Some tests want to configure the server ID via the Gerrit config.
InMemoryModule should respect this setting if it is there and only
fallback to the hard-coded server ID if this config is missing.
Change-Id: I8eace895978d221b8d4e726a4c5428bc16f9e77b
Signed-off-by: Edwin Kempin <ekempin@google.com>
E.g. GroupConfigCommitMessage#getFooters is using Sets for the footers
and hence the order was not guaranteed. As result of this tests checking
group audit logs could be flaky.
The AuditLogFormatter in AbstractGroupTest is now loading the real group
names instead of using 'Group <uuid>'. This is needed to control the
order of the subgroup modification footers from the AuditLogReaderTest.
If the real name is not included into these footers the sort order
depends on the generated UUIDs. Since the UUIDs are generated based on
the group name and the server ID they are stable for each run, but
relying on the order of the UUIDs makes the test at least less readable.
The AuditLogFormatter in AbstractGroupTest is loading the group from the
repo each time a group name is needed. Since it's an in memory
repository and there are only few tests using this AuditLogFormatter we
don't bother about caching here.
Reading footers from group config commit messages works regardless of
the order of the footers. This means this change doesn't require
rewriting already existing group refs.
Change-Id: I3eafef10e916890b90d9f9ac222595eaf2246e27
Signed-off-by: Edwin Kempin <ekempin@google.com>
Create project could fail when there are concurrent requests.
For example, in the test
CreateProjectIT#createSameProjectFromTwoConcurrentRequests.
Like other places, it's good to check whether ProjectState is
null before use.
Change-Id: I9dc590912f6ffa1878a3974991f78ccf51ca9ad1
Sometimes this can be useful, e.g. if one developers starts a WIP
change, goes to vacation and another developer makes the change ready.
At the moment the WIP flag cannot be removed by anyone else than the
change owner.
Change-Id: I4878f066b633b349dbfe927480ebb143539bf4d3
Signed-off-by: Edwin Kempin <ekempin@google.com>
Gerrit users active on several Gerrit servers may find it hard to
determine the gerrit instance related to an email.
This commit fixes it by adding a Gerrit instance name to the email
titles, right before the project's short name.
For instance, for a Gerrit instance called "Pear" and the project
"website/forum", the notification email titles will contain "Pear/forum".
Also add configuration to disable this behavior.
Change-Id: I6c842f33ce605125ec64ca7d09643f59aa96a02d
GroupRebuilder and GroupBundle are supposed to be only used by schema
migrations. Make sure that they are not used otherwise by moving them
into the schema package.
Change-Id: I094043259720edec9b60309f0ec0535bf0505d9e
Signed-off-by: Edwin Kempin <ekempin@google.com>
This is better than checking a constant group name that is the same for
all groups.
Change-Id: Ie92327bb1d5f285f3ef65fa229f1b3ee863cd8f6
Signed-off-by: Edwin Kempin <ekempin@google.com>
We want to use GroupRebuilder from a schema migration to migrate Gerrit
groups to NoteDb. For this GroupRebuilder must not depend on classes
which are not available during init:
- Don't use MetaDataUpdate.InternalFactory but instead instantiate
MetaDataUpdate directly (it's okay to use GitReferenceUpdated.DISABLED
since we don't fire events during init and init is the only place
where GroupRebuilder is used).
- Don't create an AuditLogFormatter from account cache, group cache and
server ID but instead require that the AuditLogFormatter is created
and passed in by the caller.
Change-Id: Ib43e3121ec99c38ef4c1a1879c48d879118fb4c4
Signed-off-by: Edwin Kempin <ekempin@google.com>
This will make it easier to use this method from the schema migration
that implements the migration of Gerrit groups to NoteDb.
Change-Id: Ic15d54c240998796bb5e8ad91a8b8144674c8af0
Signed-off-by: Edwin Kempin <ekempin@google.com>
This will allow us to use this method from the schema migration that
implements the migration of Gerrit groups to NoteDb.
Change-Id: I12ba4a0217ae9479f32c6c5f3fc0a834fd127e30
Signed-off-by: Edwin Kempin <ekempin@google.com>
The Gerrit groups will be migrated to NoteDb. Once this is done reading
groups from ReviewDb will be no longer supported and the
AccountGroup*Access classes will be removed.
The GroupBundle class will be used by the schema migration that migrates
the Gerrit groups from ReviewDb to NoteDb and we need to keep this
migration running for some longer time to support Gerrit upgrades.
This means GroupBundle will still exist when the AccountGroup*Access
classes are already gone. Hence to read group data it cannot rely on the
AccountGroup*Access classes, but must use plain SQL to retrieve this
data from ReviewDb.
Change-Id: If4bc99191bc7cd0e713c9666c2d52b278fe3a246
Signed-off-by: Edwin Kempin <ekempin@google.com>
This reverts commit 809de7e70c9a974df0a4c467ab731938c8b81d85.
Reason for revert: Groups are migrated offline with change I530116c8c5a.
Hence, we don't need to prevent any intermediate group updates.
Change-Id: I28113f8dbca7698a2335ae315405e7893636a745
This change partially reverts I84201c0c9d.
A follow-up change will migrate all groups from ReviewDb to NoteDb.
Further follow-up changes will remove all ReviewDb code for groups.
Hence, we don't need this REST endpoint anymore, which only existed
temporarily while implementing groups in NoteDb.
Change-Id: Ia2cf0c75a80e34ef9a8d8c8063b08388fa5fae9c
The only usage of this class is to create a ProjectState from a
ProjectConfig acquired from a ProjectState returned by the ProjectCache.
No need for this indirection; just use the original ProjectState.
Change-Id: I53434efc3c59bb2992f54ef262076030db55badd
This is similar to the split between index and server.index: these
pieces of the index code do not depend on Gerrit internals and may be
used to implement a project index in non-Gerrit servers. In other words,
this is the logical continuation of the work started in I07beec95.
Change-Id: Ic56eb46a5386cdd72f62117ad36b24ca85659809
* stable-2.15:
Fix formatting in MergeInput documentation
Fix assertions on iterable size in tests
Fix example ref in 'Create Merge Patch Set For Change' documentation
Fix documentation for CreateMergePatchSetForChange REST API endpoint.
LightweightPluginDaemonTest: Expose plugin guice injectors
Change-Id: Ief9930217e4769aac629b076316cdb3b47edfd11
* stable-2.15:
Skip migrating inline comments on missing patch set parents
Temporarily increase heap size of NoteDb migration tests
Log the reason why a project cannot be found
Change kind cache: short-circuit on root commits
Document that gitweb.type must be set
Tidy up config-gitweb
Change kind cache: short-circuit on root commits
Do not abort indexing if < 50% projects failed
Improve documentation of `index.maxLimit` for Elasticsearch
InitIndex: Set Elasticsearch index config under elasticsearch section
Link to hashtag intro docs from more places
user-upload.txt: Document setting hashtags on push
intro-user.txt: Document hashtags
user-search.txt: Document hashtag operator
intro-user.txt: Mention that topics may affect submission
Add NoteDb migration test for change with no patch set refs
NoteDbMigrator: Totally skip changes with no patch sets
Add more tests for rebuilding changes missing some entities
Fix Change-Id in revert email
Widen set of My Drafts menus that are automatically removed
Migrate old My Drafts menus in refs/users/default
This partially reverts commit e518d9dacc9d4cc547cb5935101859e36072ccb0
because Schema_159_to_160_Test references PREFERENCES which was made
private, and uses the forDefault method which was removed. This commit
makes PREFERENCES package visible and re-adds the forDefault method as
a package visible method.
Change-Id: Ifba662a47197b3a5f17988fc69896cdca1ff853b
This change does the same for accounts what was done for groups by
change I1965b20c3.
The race condition which exists for the change index (see I04ac36ea55a)
doesn't exist for the account index. Contrary to the change index, the
account index always uses the current value from the primary storage
when updating the index.
Let's assume that we have two concurrent updates of an account.
1. Updater A writes account in state A to primary storage.
2. Updater B writes account in state B to primary storage.
3. Updater B updates the index with state B from primary storage.
4. Updater A updates the index with state B from primary storage.
So, even when the writes to the storage and the updates of the index are
executed in an arbitrary order, the index still contains the correct
value in the end.
Change-Id: I43df0fa08ce88360cd4e2eb6f94de23ca2d464a8
This change does the same for accounts what was done for groups by
change I0b281d7b72.
Indexed accounts are retrieved from the account cache. As the account
cache may possibly have stale entries, we have to be careful not to use
outdated values. All code paths which referred to the AccountIndexer
correctly invalidated the account cache. Requiring the cache evictions
in other classes is a bit dangerous, though.
This situation becomes even worse with the recently added
AccountIndexer#reindexIfStale method (see change I66d5bb09e4). Whether
the account index is stale is determined by comparing the account stored
in the index with the one stored in NoteDb. For updating a stale index
entry, we didn't ensure that the account was freshly read from NoteDb.
This means that users of AccountIndexer#reindexIfStale which care about
up-to-date values would need to manually invalidate the cache, which
would trigger an indexing without the stale check, and hence make the
call to AccountIndexer#reindexIfStale obsolete. Alternatively, they
would need to accept outdated values without any guarantee when they are
updated.
To prevent future errors due to missing cache invalidations and to allow
calling AccountIndexer#index and AccountIndexer#reindexIfStale even with
stale caches, we explicitly use a non-cached value for indexing now.
As the necessity for manual cache evictions is removed, it makes sense
in other parts of the code to not indirectly trigger an indexing for an
account by evicting it from the cache but to explicitly call the
indexer.
As a result, evicting an element from the cache doesn't need to trigger
an indexing anymore. It would also have created an infinite loop for the
new eviction in the indexer.
Since the AccountCache is no longer doing any reindex on eviction, we
can now rename AccountCache#evictAllNoReindex() to
AccountCache#evictAll().
For AllAccountsIndexer, there was even a possibly unintended
side-effect: The cache invalidation triggered an indirect indexing for
the account before the account was retrieved from the cache and
explicitly put in the index. The adjusted code should avoid this
situation by doing all interaction with the index in AllAccountsIndexer.
We also use the opportunity to add some tests for the AccountIndexer and
the adjusted behavior.
Change-Id: I8eba8b0c5e1d65ad63c15970275b2a597d475c9d
Signed-off-by: Edwin Kempin <ekempin@google.com>
When a subgroup is not available any more, we couldn't load the
group from GroupBackend. Thus in this case, we should not try
to set the name of the subgroup.
Change-Id: I2be5e620fba847cb3fd8d0e364926141e4c33300
When adding DeleteCommentRewriter to NoteDbUpdateManager, a check
was added to prevent updating and rewriting one ref in a single
BatchUpdate. However, the check doesn't work as intended because
"draftUpdates" and "robotCommentUpdates" update different refs
("refs/draft-comments/xx" and "refs/changes/xx/robot-comments")
from DeleteCommentRewriter("refs/changes/xx/meta").
This commit fixes this by adding the check in #add. The idea is:
* Prevent adding a ChangeUpdate if there is an earlier NoteDbRewriter
for the same ref.
* Prevent adding a NoteDbRewriter if there is an earlier ChangeUpdate
for the same ref.
Change-Id: Ia1def2cbefce92f7ca612a33bcf0560d2cbd0158
Previously, admins were only allowed to see the internal groups a user
belongs to. System groups as well as those from other GroupBackends were
excluded from the result. This impeded investigations in case of ACL
issues.
Allowing admins to see all groups shouldn't create a new security issue.
Even previously, admins had ways to track down any external groups which
were mentioned in Gerrit ACLs. If implemented correctly, external
GroupBackends should only provide groups which are mentioned in the
Gerrit ACLs of the corresponding project. In that case, listing all
groups for another user as admin shouldn't disclose any new information
to admins.
This change doesn't only allow admins to see all groups of a user but
also allows them to see all users who belong to an internal group even
when users are listed as members on subgroups. There is no apparent
reason to allow one direction without the other and that's why we don't
restrict it.
Change-Id: Ia71c59af6035ea23bad5a3156d1522f7dac6424b
This commit makes sure that the commit message for the
super repo is created with an sorted list of subscriptions.
Without this fix, the following test could fail because the
commit message may change for different runs.
SubmoduleSubscriptionsWholeTopicMergeIT#updateManySubmodules
Change-Id: If84195c04fb89b1397295d0d80b6b697f20598e4
There are 2 places that take care about evicting and reindexing accounts
after an account update:
- ReindexAfterRefUpdate if the user branch is touched
- ExternalIdNotes if external IDs are updated (needed because external
ID updates don't touch the user branch)
This means to disable eviction and reindexing two things need to be
done:
- bind GitReferenceUpdated#DISABLED
- create AccountsUpdate with ExternalIdNotes.FactoryNoReindex
The JavaDoc of AccountsUpdate wasn't precise about this.
Change-Id: I62e5d8cea4da4b94435f57b590cf5f0b2fa781b3
Signed-off-by: Edwin Kempin <ekempin@google.com>
