Earlier in this series as I was changing fields in this AutoValue class,
I kept messing up the ginormous constructor. It would have been nice to
have this builder to make editing the fields easier; better late than
never.
Change-Id: I4eaea9f8d4fa3920a7c5325729ce463f3ce13583
The only caller outside of tests that used this field was
Submit#getConflictMessage, which no longer exists.
Rename the remaining field from allChangeMessages to changeMessages,
since there is no longer overlap with another name.
Change-Id: I8f2846d7b21b41a6e0c934fd73ff87f6dc8e58d9
Back when we had separate SUBMITTED/MERGED states, the merge attempt
process would write a ChangeMessage from the server describing the
problem. This code was intended to pull out that message and report it
as the contents of a ResourceConflictException. The submit process no
longer works like this: errors during submission are supposed to be
thrown directly as IntegrationExceptions, and the code should not reach
this point. At any rate, on changes created in the past few years, this
code will never find a relevant message.
Change the case where change remains NEW after submission to be a 500, since it
reflects an actual server-side error.
Change-Id: Idbd27f7ed8daaf67318c391e9dbb1c8e02ed9e81
The point of ChangeColumns is that the contents are copied into the
relevant columns in the ChangeNotes's Change instance upon loading.
Callers should just be getting these fields from the Change instead;
there weren't any callers outside of tests doing the wrong thing anyway.
Change-Id: Ib6a592e4a5b28add9ed117dbcffb03ac7a507582
This is a @Column in Change and thus belongs in ChangeColumns. In
ChangeNotes#create it reuses the same value in both places, so they
should always be equal, modulo missing ChangeColumns, in which case the
field in ChangeNotesState should never be accessed.
Change-Id: I63eb5acdcd6c47ea2ceea7d830d5b5b6e4b28d2f
There were redundant fields in the ChangeNotesState and the contained
ChangeColumns. They properly belong in ChangeColumns, since they
correspond to actual @Columns in the Change class. Also in the Change
class, they are not nullable (they lack "notNull = false"), so remove
@Nullable in ChangeColumns as well.
A few callers were explicitly depending on the fields in
ChangeNotesState rather than in ChangeColumns; those are easily updated.
Note that the ChangeNotesState#create method always just passed in a
single value for each of these fields, which was then passed to two
different constructors, so they were always in sync.
The only time when the ChangeNotesState and ChangeColumns might
potentially be out of sync is when calling ChangeNotesState#empty, which
would pass a non-null value for these bits to the ChangeNotesState
constructor and leave the ChangeColumns null. However, this is only used
from ChangeNotes when NoteDb is disabled, in which case the
ChangeNotesState fields are never exposed to callers. In this case they
get these bits by calling the relevant methods on getChange(), which
will contain the ReviewDb change (and not have columns populated from
NoteDb via ChangeNotesState#copyColumnsTo).
Change-Id: Id4771f3e305417dc7f8d2040575d0c6746944946
These args were marked @Nullable, but the corresponding fields in
AutoValue were non-nullable, so remove @Nullable to make them
consistent.
For pastAssignees, any callers passing null would have already
encountered NPE due to the ImmutableSet#copyOf (and, if not that, the
check in the auto-generated constructor). For hashtags, push the logic
guarding against null into the one caller in
ChangeNotesParser#buildState, which already has similar logic to
construct its arguments.
Change-Id: Idc5123ccab4e1d21f932067d9ffff0f98afa5011
When the set of fields changed, it's a very good sign that something
needs to change in the serializer implementation, or at least bumping
the version number in the CacheMoule. Add a simple test utility that
maps declared field names to types, and list the expected types
explicitly in each of the relevant tests.
This simple mechanism can't catch everything, for example:
* If the serializer implementation changes for an existing type in an
incompatible way, without altering any fields. In this case, the
{key,value}Serializer in the CacheModule will change, so hopefully a
reviewer will notice and ask the author to bump the version as well.
* If semantics of individual fields change without changing the type,
such as making a field newly @Nullable.
But it's much better than nothing.
Change-Id: I51e60b83f9c3291d2f8f9ac19ce0192678211676
The serializer class lives in OAuthTokenCache.java so the extension
framework doesn't acquire a dep on the cache serialization code, which
currently lives in the server package.
Proto3 doesn't have optional fields, so we can't distinguish between
empty and null for the provider_id field (at least without adding a
separate has_provider_id field). Explicitly treat these the same in
OAuthToken itself, and clarify that other fields are not nullable.
Change-Id: I37bd47ac0c8491e5212fcec4885e063ae50d714d
Previously, CacheSerializer threw IOException for the convenience of
implementors, since some implementations were based on APIs like
InputStream/OutputStream that threw IOException. However, this didn't
cover all the possible exceptions that could be thrown; callers also
neded to be prepared to catch RuntimeException. Throwing checked
exceptions in some case and unchecked exceptions in others might have
given the impression that these cases were fundamentally different and
required different handling, which is not the case.
In addition, generally speaking, these classes are null-hostile. Use
checkNotNull in EnumCacheSerializer so the intent is clear.
Also in EnumCacheSerializer, delegate to Enums#stringConverter rather
than rolling our own.
Change-Id: I37f1cf73d961be477af389c4043bb1c10c45972c
This allows us to bind this class in our custom Guice stack at
Google.
Change-Id: I2307538a30e80f388346455ff17ef3710bc9ce8c
Signed-off-by: Edwin Kempin <ekempin@google.com>
All logging of this class should be done through the locally defined
'sshDaemonLog' Logger (and not through the inherited 'log' Logger from
AbstractLoggingBean).
Change-Id: I2d1031fe51601f11911b110c482482ff2482ce1a
Signed-off-by: Edwin Kempin <ekempin@google.com>
Because of problems in the GWT UI's display of blame annotations, the
change.allowBlame config was used to disable the Gerrit feature
altogether. Since then, the PolyGerrit UI can display blame annotations
without these problems, but the feature remains disabled.
The change.allowBlame config handling is changed to only disable blame
in the frontend of the GWT UI. Note: this removes the ability to disable
the blame API via server config.
Change-Id: Ic25aed5a533e0044956359587321676b022c21ec
This was wrongly changed in I601ea1200a. This commit changes it back to
the correct value (a provider of the CurrentUser).
Change-Id: I6cbee0cd3013e6fc53514db4c1b855e8da52e63b
When calling operations that create a lot of ProjectControls (e.g.
ListProjects) we observe OOMs in our Gerrit jobs from time to time. This
commit puts a max. limit on the cache size to prevent this from
happening.
The approach implemented is not an LRU. On purpose, we want to keep
PerThreadCache simple and don't add a dependency on e.g. Guava's LRU.
Change-Id: Ic2f6e11719b52ee2112f72698e887cbf8ca254d0
* stable-2.15:
Revert "Filter account entry suggestions"
ChangeIndexedListener: pass project name information
Set version to 2.15.2-SNAPSHOT
Change-Id: Id36088686621e5ce4c61633d8df732a49d205209
Every time a lambda taking an argument is created, an object is created.
This object is unique and makes it harder for the JIT compilation to
optimize the code.
By replacing lambdas with method references, we help the JIT better
understand our code, and reduce object creations* and garbage
collection.
Using method references also helps humans better understand the code,
and might even make compilation faster: the method, and where it lives,
is explicitely written: no need to guess it.
repoUpgraded.stream().map(n -> n.get()).collect(...);
Where does the get method come from? Hard to tell.
repoUpgraded.stream().map(NameKey::get).collect()
Same line of code, using a method reference. It is clear that the input
objects are NameKey-s, and we call the get method on them.
* This is technically wrong, as a pointer is created even when we use a
method reference, but it is at least smaller than the lambda
counterpart.
Change-Id: Ic85b1b593d42968f262d3afe49a2ad87a4b766b9
UIActions (GetRevisionActions) calls have a 99.9%ile latency > 1 minute.
We only have metrics on the REST endpoint which does not give much
indication on why this endpoint is slow.
UIActions is implemented in a way that it calls #getDescription on all
RestViews that implement the UIActions interface. This means we do
serial calls to a lot of these methods. This commit adds a latency
metric to instrument these calls. Knowing which actions are slow can
guide futher steps that we take.
Possibilities include reworking slow actions or looking into more
parallelism.
Change-Id: I261bfb1a5a43b81f11dba019d405126198908c5f
The log message had 3 parameters, but only 2 placeholders in the format
string.
Change-Id: I2ffbba76837adb92d924a8692913b6ef3e675f30
Signed-off-by: Edwin Kempin <ekempin@google.com>
We must use '{}' as placeholders for the parameters.
Change-Id: I729e7856882c0b37a4c6e19f022c981876ef8c95
Signed-off-by: Edwin Kempin <ekempin@google.com>
The debug logs say 'event listener', but the warning logs just
'listener'. Make this consistent and always use 'event listener' which
is more precise.
Change-Id: I447a00cce0b11e5337801ce5d84c7954c1340f59
Signed-off-by: Edwin Kempin <ekempin@google.com>
There is only one place where PatchListNotAvailableException is logged
as a warning, everywhere else (EventFactory, ChangeAbandoned,
ChangeMerged, ...) it is logged as an error. Make this consistent.
Change-Id: I0104a280c984e2c8244618fa518b320bad0407af
Signed-off-by: Edwin Kempin <ekempin@google.com>
Without the stacktrace the error log is not very useful.
Change-Id: I29688404e51f999bc06c6ec47b2de5bd1a6e238d
Signed-off-by: Edwin Kempin <ekempin@google.com>
