We can just rely on the toString formatting of the List. This makes the
code more readable.
Change-Id: I98f0da6b5180198b9fcf0e45271e12e9ab182a52
Signed-off-by: Edwin Kempin <ekempin@google.com>
"log and throw" is considered a poor practice. The logging is uneeded
since the thrown exception will be logged somewhere else.
Change-Id: I82c210fe1e053a4ffd7a58b7fc7de6056f9875a7
Signed-off-by: Edwin Kempin <ekempin@google.com>
We used to think adding this method to the top level is enough
and could avoid misuses. But it turns out we still need them in
other permission backend classes so that we only need to rescope
a user when necessary, which could provide better performance.
See I67b72b59 as an example.
Change-Id: I9ef30fb38315250fa63fb8b3d27e19e19d5e3e22
A colon is used as separator between the scheme and id. The logic in
ExternalId.Key#parse(String) that parses an external ID string looks for
the first occurrance of a colon to split the string. This means the
scheme must not contain any colon. Since a scheme is optional also the
id must not contain any colon.
We can't enforce this since internally at Google we violate this
assumption, but we also have our own parse logic and don't rely on the
parse logic in ExternalId.Key#parse(String).
Change-Id: I9a20776c6546eb09562f3da8f8e41506a74ba1e1
Signed-off-by: Edwin Kempin <ekempin@google.com>
* changes:
Move ReviewDb ProtobufCodecs to a common class
Add equals/hashCode to PatchSet and ChangeMessage
ChangeNotesState: Add a Builder
ChangeNotesState: Remove changeMessagesByPatchSet field
Submit: Remove code scanning for failed submission message
ChangeNotes: Remove fields that read from ChangeColumns
ChangeNotesState: Remove redundant revertOf field
Clarify ChangeNotesState private/WIP/started bits
ChangeNotesState: Remove some @Nullable annotations from create
Move ByteString bytes(int... ints) to test util
Add tests for fields declared in types with custom serializers
Convert OAuthTokenCache serialization to protobuf
Convert MergeabilityCacheImpl.Key serialization to protobuf
Simplify and clarify exception throwing of CacheSerializer
Some internal classes (namely PluginGuiceEnvironment and
PrivateInternals_DynamicTypes) rely on the fact that all DynamicMap
instances are DynamicMapImpl, which is not true if we have anonymous
subclasses of DynamicMap.
This is the case when #emptyMap is called, for instance from
BatchProgramModule.
Change-Id: I0e7d06bee2c13788e5c327fb70a60b7e86653d72
This allows us to replace them with custom bindings at Google.
Change-Id: I084ad42395882fd10d8cb9c8f3b842faac6dc52a
Signed-off-by: Edwin Kempin <ekempin@google.com>
This is a preparation to move the bindings for the REST API out of
GerritGlobalModule so that at Google we can replace them with custom
bindings.
Change-Id: I36b24206375ee0db4f14e8d50de7cd2721236747
Signed-off-by: Edwin Kempin <ekempin@google.com>
This makes the name consistent with the other REST API modules.
Change-Id: I5723685fddea36d1a4f777103b8f5bd27ac230ac
Signed-off-by: Edwin Kempin <ekempin@google.com>
This reverts commit 727d718b77d53e19ba14161ced72e0125087c220.
Reason for revert: We actually don't need this class to be public.
Change-Id: I25ceacb6e29f8e655bc5a28343d15b2e65aa3981
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
