Proto commits in GerritCodeReview/gerrit

These commits are when the Protocol Buffers files have changed: (only the last 100 relevant commits are shown)

2025-05-06

Add file sha to the FileInfo. With this information we can implement the feature, where the patchset picker in gr-diff-view shows between which patchset the file has been edited. Making it easier to pick the base patchset to diff against. Google-Bug-Id: b/304433985 Release-Notes: skip Change-Id: I821978d09353fc7d10496b511287d5da96310765

The documentation is generated from this commit.

2025-03-08

Merge "Expose atom explanation in submit requirement check."

Expose atom explanation in submit requirement check. To improve debuggability of unsatisfied submit requirements, we expose explanations for submit requirement predicate atoms, which can then be exposed by the UI. The API that will be affected: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#check-submit-requirement Forward-Compatible: checked Release-Notes: Allow SubmitRequirement atoms to return an explanation Bug: b/369788358 Change-Id: Iafad2063ced0fdba62cf3c2fb15406123e282d9e

2025-03-07

HumanComment proto: Add fix-suggestions field Also change the HumanComment constructor to include all fields used in CommentUtils#newHumanComment, to reduce the risk of such discrepancy between the java class and the proto will happen again. Release-Notes: skip Google-Bug-Id: b/395582358 Change-Id: Iefd66518f2ef85df38cff316e88ef222dd7b7d55 Forward-Compatible: checked

2025-01-10

entities.proto: Make Conflicts.containsConflicts field optional New fields are no longer allowed to be required. Bug: Google b/373350443 Forward-Compatible: checked Release-Notes: skip Change-Id: Ife57b838c000b6e350b4f38d7556f8861dc72919 Signed-off-by: Edwin Kempin <ekempin@google.com>

2025-01-09

Store/provide merge conflict information for revisions created by Gerrit It's possible that files in changes contain Git conflicts. In this case users should resolve the conflicts before submitting the change. Code editors may support users with resolving conflicts, but for this they need to know whether a change (or rather the current revision of the change) contains conflicts, and if yes, which commits were used as 'ours' and 'theirs' during the Git merge that created the conflicts. For revisions that are created by Gerrit, we know whether conflicts are present and which commits were used as 'ours' and 'theirs' during the Git merge that created the conflicts. For those revisions we can store this information in NoteDb and make it available to callers in RevisionInfo. At this point we are mostly interested in making the conflicts information available in RevisionInfo so that code editors can offer conflict resolution to users. In addition, Gerrit itself may make use of this to: * show a warning on changes that contain conflicts * prevent submitting changes that contain conflicts but no work on this is currently planned. Information about conflicts is only stored for revisions that are created by Gerrit as a result of performing a Git merge. Note that this does not only cover revisions for merge commits, as they are created by the Create Change REST endpoint when a MergePatchsetInput is specified and the Create Merge Patch Set For Change REST endpoint, but also revisions for non-merge commits that are created by performing a Git merge, such as rebased and cherry-picked commits. For other revisions information about conflicts is not available. If conflict information is not available it's unknown whether the revision contains any file with conflicts. For example, revisions that are uploaded by users do not have conflict information available (making it available would require scanning all files for conflicts on upload, which would be too expensive and error-prone to do). When Gerrit performs a Git merge to create a revision (MergeUtil, RebaseChangeOp, ChangeEditModifier) we already know whether the merge resulted into conflicts and which commits were used as 'ours' and 'theirs'. The result of performing the Git merge is a RevCommit that may contain files with Git conflicts. In Gerrit we use CodeReviewCommit which is an extension of RevCommit to represent the result of the merge. CodeReviewCommit is a RevCommit with Gerrit metadata and already contains information about the conflicting files. We extend CodeReviewCommit to also store the commits that were used as 'ours' and 'theirs', so that we can store them in NoteDb later. In order to store the conflicts information in NoteDb the callers of the merge logic (CreateChange, CreateMergePatchSet, RebaseChangeOp, CherryPickChange) need to pass the conflicts information from CodeReviewCommit to ChangeInserter/PatchSetInserter that pass them to ChangeUpdate. ChangeUpdate is the class that writes the NoteDb commits. To store the conflicts information in NoteDb it writes 3 new footers: "Contains-Conflicts: <true|false>", "Ours: <sha1>", "Theirs: <sha1>". The new footers are optional, hence the new code is backwards compatible and can still parse old changes where these footers are not set. Ideally we would also like to store the files that contain conflicts, but NoteDb is not a suitable storage for this, hence this information is not persisted. ChangeNotesParser is extended to parse these footers and store the parsed conflicts information in the PatchSet's. From there RevisionJson can read the conflicts information and populate a new 'conflicts' field in RevisionInfo. The conflicts information is represented by ConflictsInfo which has fields for whether there are conflicts ('contains_conflicts') and the commits that were used as 'ours' and 'theirs' ('ours', 'theirs'). With this change the 'conflicts' field in RevisionInfo is only set for revisions that were created by Gerrit as a result of performing a Git merge. For those revisions all fields in ConflictsInfo ('contains_conflicts', 'ours', 'theirs') are always set. In the future we may also set the 'conflicts' field in RevisionInfo for revisions that are created by Gerrit, but which are not the result of performing a Git merge, to inform callers that these revisions are known to not contain any conflicts. In this case 'contains_conflicts' would be 'false' while 'ours' and 'theirs' is not set. To be able to do this later without breaking the API, we make the 'ours' and 'theirs' fields in ConflictsInfo optional, although currently they are always set. This change doesn't change which commit is used as "ours" and "theirs" during the different operations, but we only make this information available to users. However since it's not obvious for all operations which commit is used as "ours" and "theirs" we document this clearly in the REST API documentation. Note, the following cases where conflict may occur are not covered in this change, but our stakeholders are fine with these limitations: * conflicts in rebased change edits are not stored (hence when a change edit is published, and becomes a regular patch set, conflicts information is not available) * conflicts are not stored for patch sets that are created by applying a patch (e.g. via the Create patch-set from patch REST endpoint) Conflicts information is not back-filled for old changes. Forward-Compatible: checked Release-Notes: Store/provide merge conflict information for revisions created by Gerrit Bug: Google b/373350443 Change-Id: I15e48ba87d6d58f4a069f1f6095e090c8ed6961e Signed-off-by: Edwin Kempin <ekempin@google.com>

2024-05-27

Allow applying patches that conflict This makes use of a new JGit feature added in [1]. It allows applying patches that conflict which results in conflict markers being added to the resulting files. As with other patch logic, we add a disclaimer to the commit message (this logic is unchanged). [1] https://gerrithub.io/c/eclipse-jgit/jgit/+/200480 Forward-Compatible: checked Release-Notes: Add API option to apply patches that conflict Change-Id: Ia0b59d0a2cb006eba6bc7a958e08c363a0965cd4

2024-05-16

Make UserPreferences converters safe Release-Notes: skip Google-Bug-Id: b/335372403 Change-Id: I7ad898dfb5272d964a6065bb48403aad9603481e

2024-05-10

SafeProtoConverterTest: test proto defaults Release-Notes: skip Google-Bug-Id: b/335372403 Change-Id: I0f296df66e7b41d2bc3584c8fc3ab23e3db20ec1 Forward-Compatible: checked'

2024-04-09

Increment change_notes cache version for new server_id field in protobuf Follow-up of Ic0e2cf4d8 introducing the server_id in the protobuf definition of Change message and ChangeColumns, with the update of the change_notes version number. Release-Notes: Introduce server_id in change_notes cache and update its version number Forward-Compatible: checked Change-Id: I828d39c64d0265fd82aa42baf5713513cb1a3ff2

2024-03-27

Add uniqueTag to the Account class. There is some use-cases (e.g. ETag calculation) when some unique version string for an account is usefult. The metaId has been used before, but now google internally doesn't store accounts in noteDb and as a result metaId is not set. Adding a separate uniqueTag property allows to use a different value as a unique version string. The change is forward and backward compatible: old versions will ignore uniqueTag in proto, newer version will automatically set uniqueId to metaId if proto doesn't contain a value for uniqueId. Google-Bug-Id: b/289356590 Release-Notes: skip Forward-Compatible: checked Change-Id: I277951ef8f8cf9d93abbd065ba4781907576727c

2024-03-14

Add ChangeInput definition to gerrit entities. NOTE FOR REVIEWERS - original patch and result patch are not identical. PLEASE REVIEW CAREFULLY. Diffs between the patches: // Next ID: 25 > +message ChangeInput { > + // The change repository. > + optional string project = 1; > + > + // The name of the target branch. The `refs/heads/` prefix is omitted. > + optional string branch = 2; > + > + // Commit message. > + optional string subject = 3; > + > + optional string topic = 4; > + optional ChangeStatus status = 5; > + optional bool is_private = 6; > + optional bool work_in_progress = 7; > + > + // The below fields are only relevant for creating change requests. > + > + oneof Base { > + // The new change will be created onto this Gerrit change. > + optional string base_change = 8; > + > + // A 40-digit hex SHA-1 of the commit which will be the parent commit of the newly > + // created change. If set, it must be a merged commit on the destination branch. > + // This field is mutually exclusive with `base_change`. > + // If neither `base_commit` nor `base_change` are set, the target branch tip will > + // be used as the parent commit. > + optional string base_commit = 9; > + } > + optional bool new_branch = 10; > + map<string, string> validation_options = 11; > + > + // Maps custom keys to custom values that are tied to a specific change, > + // both in the form of strings. > + // For Cog, this is needed to set the cogws key. > + // This can be deprecated once b/288098006 is resolved. > + map<string, string> custom_keyed_values = 12; > + optional MergeInput merge = 13; > + optional ApplyPatchInput patch = 14; > + optional AccountInput author = 15; > + Repeated ListChangesOption response_format_options = 16; > +} > + > +// Serialized form of com.google.gerrit.enities.ChangeMessage. > +// Serialized form of com.google.gerrit.extensions.client.ChangeStatus. > +// Next ID: 4 > +enum ChangeStatus { > + NEW = 0; > + MERGED = 1; > + ABANDONED = 2; > +} > + > +// Serialized form of com.google.gerrit.extensions.common.MergeInput. > +// Next ID: 5 > +message MergeInput { > + optional string source = 1; > + optional string source_branch = 2; > + optional string strategy = 3; > + optional bool allow_conflicts = 4; > +} > + > +// Serialized form of com.google.gerrit.extensions.api.changes.ApplyPatchInput. > +// Next ID: 2 > +message ApplyPatchInput { > + optional string patch = 1; > +} > + > +// Serialized form of com.google.gerrit.extensions.api.accounts.AccountInput. > +// Next ID: 8 > +message AccountInput { > + optional string username = 1; > + optional string name = 2; > + optional string display_name = 3; > + optional string email = 4; > + optional string ssh_key = 5; > + optional string http_password = 6; > + repeated string groups = 7; > +} > + > +// Serialized form of com.google.gerrit.extensions.client.ListChangesOption. > +// Next ID: 28 > +enum ListChangesOption { > + LABELS = 0; > + CURRENT_REVISION = 1; > + ALL_REVISIONS = 2; > + CURRENT_COMMIT = 3; > + ALL_COMMITS = 4; > + CURRENT_FILES = 5; > + ALL_FILES = 6; > + DETAILED_ACCOUNTS = 7; > + DETAILED_LABELS = 8; > + MESSAGES = 9; > + CURRENT_ACTIONS = 10; > + REVIEWED = 11; > + DRAFT_COMMENTS = 12; > + DOWNLOAD_COMMANDS = 13; > + WEB_LINKS = 14; > + CHECK = 15; > + CHANGE_ACTIONS = 16; > + COMMIT_FOOTERS = 17; > + PUSH_CERTIFICATES = 18; > + REVIEWER_UPDATES = 19; > + SUBMITTABLE = 20; > + TRACKING_IDS = 21; > + SKIP_MERGEABLE = 22; > + SKIP_DIFFSTAT = 23; > + SUBMIT_REQUIREMENTS = 24; > + CUSTOM_KEYED_VALUES = 25; > + STAR = 26; > + PARENTS = 27; > +} > + Original patch: diff --git a/proto/entities.proto b/proto/entities.proto old mode 100644 new mode 100644 --- a/proto/entities.proto +++ b/proto/entities.proto @@ -61,6 +61,50 @@ reserved 19; // assignee reserved 101; // note_db_state } + +// Serialized form of com.google.gerrit.enities.ChangeMessage. +// Next ID: 25 +message ChangeInput { + // The change repository. + optional string project = 1; + + // The name of the target branch. The `refs/heads/` prefix is omitted. + optional string branch = 2; + + // Commit message. + optional string subject = 3; + + optional string topic = 4; + optional ChangeStatus status = 5; + optional bool is_private = 6; + optional bool work_in_progress = 7; + + // The below fields are only relevant for creating change requests. + + oneof Base { + // The new change will be created onto this Gerrit change. + optional string base_change = 8; + + // A 40-digit hex SHA-1 of the commit which will be the parent commit of the newly + // created change. If set, it must b [[[Original patch trimmed due to size. Decoded string size: 3740. Decoded string SHA1: 0df815b94ab12c881e1d5d7fd403ac3713a54569.]]] Result patch: diff --git a/proto/entities.proto b/proto/entities.proto index 335db62..607c78d 100644 --- a/proto/entities.proto +++ b/proto/entities.proto @@ -63,6 +63,50 @@ } // Serialized form of com.google.gerrit.enities.ChangeMessage. +// Next ID: 25 +message ChangeInput { + // The change repository. + optional string project = 1; + + // The name of the target branch. The `refs/heads/` prefix is omitted. + optional string branch = 2; + + // Commit message. + optional string subject = 3; + + optional string topic = 4; + optional ChangeStatus status = 5; + optional bool is_private = 6; + optional bool work_in_progress = 7; + + // The below fields are only relevant for creating change requests. + + oneof Base { + // The new change will be created onto this Gerrit change. + optional string base_change = 8; + + // A 40-digit hex SHA-1 of the commit which will be the parent commit of the newly + // created change. If set, it must be a merged commit on the destination branch. + // This field is mu [[[Result patch trimmed due to size. Decoded string size: 3735. Decoded string SHA1: e4666adb9ebe76f851621fdabbf0cea00f0375fc.]]] NOTE FOR REVIEWERS - original patch and result patch are not identical. PLEASE REVIEW CAREFULLY. Diffs between the patches: // Serialized form of com.google.gerrit.extensions.api.changes.NotifyHandling. > +// Next ID: 4 > +enum NotifyHandling { > + NONE = 0; > + OWNER = 1; > + OWNER_REVIEWERS = 2; > + ALL = 3; > +} > + > +// Serialized form of com.google.gerrit.extensions.api.changes.RecipientType. > +// Next ID: 3 > +enum RecipientType { > + TO = 0; > + CC = 1; > + BCC = 2; > +} > + > +// Serialized form of com.google.gerrit.extensions.api.changes.NotifyInfo. > +// Next ID: 2 > +message NotifyInfo { > + repeated string accounts = 1; > +} > + Original patch: diff --git a/proto/entities.proto b/proto/entities.proto old mode 100644 new mode 100644 --- a/proto/entities.proto +++ b/proto/entities.proto @@ -62,8 +62,8 @@ reserved 101; // note_db_state } -// Serialized form of com.google.gerrit.enities.ChangeMessage. -// Next ID: 17 +// Serialized form of com.google.gerrit.extensions.common.ChangeInput. +// Next ID: 19 message ChangeInput { optional string project = 1; optional string branch = 2; @@ -81,6 +81,8 @@ optional ApplyPatchInput patch = 14; optional AccountInput author = 15; repeated ListChangesOption response_format_options = 16; + optional NotifyHandling notify = 17; + map<RecipientType, NotifyInfo> notify_details = 18; } // Serialized form of com.google.gerrit.enities.ChangeMessage. @@ -169,6 +171,29 @@ STAR = 26; PARENTS = 27; } + +// Serialized form of com.google.gerrit.extensions.api.changes.NotifyHandling. +// Next ID: 4 +enum NotifyHandling { + NONE = 0; + OWNER = 1; + OWNER_REVIEWERS = 2; + ALL = 3; +} + +// Se [[[Original patch trimmed due to size. Decoded string size: 1401. Decoded string SHA1: 008a5766e4b4e990dc35840f56b168b7d5dd0fc4.]]] Result patch: diff --git a/proto/entities.proto b/proto/entities.proto index 96e41a1..ed5ae5d 100644 --- a/proto/entities.proto +++ b/proto/entities.proto @@ -62,8 +62,8 @@ reserved 101; // note_db_state } -// Serialized form of com.google.gerrit.enities.ChangeMessage. -// Next ID: 17 +// Serialized form of com.google.gerrit.extensions.common.ChangeInput. +// Next ID: 19 message ChangeInput { optional string project = 1; optional string branch = 2; @@ -81,6 +81,8 @@ optional ApplyPatchInput patch = 14; optional AccountInput author = 15; repeated ListChangesOption response_format_options = 16; + optional NotifyHandling notify = 17; + map<RecipientType, NotifyInfo> notify_details = 18; } // Serialized form of com.google.gerrit.enities.ChangeMessage. @@ -170,6 +172,29 @@ PARENTS = 27; } +// Serialized form of com.google.gerrit.extensions.api.changes.NotifyHandling. +// Next ID: 4 +enum NotifyHandling { + NONE = 0; + OWNER = 1; + OWNER_REVIEWERS = 2; + ALL = 3; +} + +// Serialized form of [[[Result patch trimmed due to size. Decoded string size: 1409. Decoded string SHA1: d685a2917c95d7715b301a2b25fddc601409dbf0.]]] Release-Notes:skip Change-Id: I561cf42fa746fef1b1185a97328fb5c5e5d672e7

2024-02-02

Return reviewer updates for reviewers by email in change details (part 1/2) ChangeInfo that is returned to the frontend when it requests change details contains reviewer updates as ReviewerUpdateInfo's. The frontend relies on getting ReviewerUpdateInfo to show reviewer updates in the Change Log (the change messages that Gerrit adds for reviewer updates are filtered out since they are tagged with "autogenerated:..."). So far Gerrit only returns reviewer updates for reviewers that are Gerrit accounts but not for reviewers by email. Due to this updates about reviewers by email are missing in the Change Log. Reviewer updates are recorded in NoteDb and ChangeNotesParser parses them from there. At the moment ChangeNotesParser only creates ReviewerStatusUpdate's for reviewers that are Gerrit accounts (method parseReviewer), but not for reviewers by email (method parseReviewerByEmail). In NoteDb reviewers that are Gerrit accounts are recorded as account IDs and reviewers by email are recorded as Address strings, consisting out of a full name and an email ("Full Name <full.name@example.com>") or only an email ("Full Name <full.name@example.com>"). ReviewerStatusUpdate currently refers to the reviewer by account ID (field reviewer). To be able to also refer to reviewers by email, we make the account ID optional (we change the type of reviewer field from Account.Id to Optional<Account.Id>) and we add a new optional field for the reviewer address (we add a new reviewerByEmail field of type Optional<Address>). ReviewerStatusUpdate's are a part of ChangeNotesState that is converted into a proto to be cached. This means the changes to ReviewerStatusUpdate must be reflected in the ReviewerStatusUpdateProto proto. In the ChangeNotesStateProto optional fields are accompanied by a "has" boolean field that tells whether the optional field has been set (see comment on ChangeNotesStateProto). This means to make the ReviewerStatusUpdateProto.reviewer field optional we add a boolean ReviewerStatusUpdateProto.has_reviewer field. For storing reviewers by email we add a ReviewerStatusUpdateProto.reviewer_by_email field and a ReviewerStatusUpdateProto.has_reviewer_by_email field. ChangeNoteState.toReviewerStatusUpdateProto takes care to populate the new fields. When rolling out this change at Google we must ensure that: 1. the new code can read old protos 2. there are no new protos that the old code cannot read, until all datacenters have been updated with the new code 1. is ensured in ChangeNotesState.toReviewerStatusUpdateList that reads the reviewer field if neither has_reviewer_by_email nor has_reviewer have been set, which is the case for old protos. 2. is ensured by not creating any ReviewerStatusUpdate for reviewers by email yet (see code in ChangeNotesParser.parseReviewerByEmail that is commented out). This means all ReviewerStatusUpdate's still have the reviewer field set which is unconditionally read by the old code. Creating ReviewerStatusUpdate's for reviewers by email will be done by the follow-up change (uncomment the code in ChangeNotesParser.parseReviewerByEmail). That change must only be submitted after this change has been rolled out to all datacenters, so that all datacenters have the new code and can read the new protos. Finally, ChangeJson is updated to be able to emit ReviewerUpdateInfo's for reviewer updates that are done for reviewers by email, and hence have no account ID. ChangeJson already had code to create AccountInfo's for reviewers by email that only have the name and email set, but no account ID. We refactored this a bit so that we can use this code to create the AccountInfo's in ReviewerUpdateInfo for reviewer updates that are done for reviewers by email. A new test in ChangeReviewersByEmailIT, checks that still no reviewer updates for reviewers by email are returned, but the assertions for when we can enable this are already prepared as comments. The new test follows the example of the test that checks reviewer updates for reviewers that are Gerrit accounts (see ChangeReviewersIT.addReviewerToReviewerUpdateInfo()). Release-Notes: skip Bug: Google b/240676565 Forward-Compatible: checked Change-Id: If1dfb3c710b5fbd9babddc669cfc85020dab14fd Signed-off-by: Edwin Kempin <ekempin@google.com>

2023-11-24

Add proto representation for HumanComment along with a converter Bug: Google b/289357382 Release-Notes: skip Change-Id: Ib0407a7113e58925bbab30af9adb503b347ef74f Forward-Compatible: checked

2023-10-19

Introduce a user preference for sidebar on diff page This will be used to automatically open the user's preferred sidebar on diff pages in the UI. Google-Bug-Id: b/275059901 Release-Notes: Add a user preference for sidebar on the diff page. Change-Id: Idd8c492aa6cd4cc90bd1b6d581a7cafeb9adee13

2023-09-25

CachedPreferences: change the cached format to CachedPreferencesProto This is replacing the legacy string formatting. Advanced the AccountCache version to avoid mishmash. This is a part of a series of changes intended to add support for different account storage systems. Bug: Google b/289357382 Release-Notes: skip Change-Id: Ic6674e67a737d1268f24d19194fb35466a174ea4

CachedPreferences: add support for proto based preferences CachedPreferences originally only supported `jgit.lib.Config` data. This change is adding support for proto-based ones. The new protos are completely equivalent to the Java preference classes, and they should be kept in sync (enforced by the converter test). The change only applies for account-level preferences. Default host preferences are still assumed to be git configs. This is a part of a series of changes intended to add support for different account storage systems. Bug: Google b/289357382 Release-Notes: skip Change-Id: I99ed211f3f25d59f03d70ccd05c860be5c747b3d

2023-09-11

List Branches - add a next-page-token request parameter We add a next-page-token parameter to the request and response of 'List Branches' endpoint. This would optimize the performance for pagination across this API in contrast with the start & limit parameters. Background ---------- The reason is that the current start/skip parameters process the request as follows: 1) Load all the branches from the repo and get the set S 2) Filter S against caller visibility to get S'. If start and limit parameters are supplied, we filter branches until we get as many as 'limit' entries starting from 'start'. Visibility checks on the remaining branches is not necessary. Because of the visibility checks in (2), it needs to be executed for all skipped branches + the branches that are returned, so a client migrating from a one call to 'n' paginated calls will perform a quadratic computations of visibility. This is bad since the majority of the request time is spent on visibility checks. With continuation tokens, the request can be executed as follows: 1) Load all the branches from the repo and get the set S 2) Drop all refs before (and including) the continuation token, get S' 3) Filter S' against caller visibility to get S''. If start and limit parameters are supplied, we filter branches until we get as many as 'limit' entries starting from 'start'. Visibility checks on the remaining branches is not necessary. 4) Return the refs to the user and continuation_token = last item Implementation -------------- We accept a new 'next-page-token' request parameter which is mutually exclusive with 'start' (or otherwise the request would fail). If the limit parameter is set, and the number of branches is larger than the limit, we set a new response header 'X-GERRIT-NEXT-PAGE-TOKEN' to the string representation of the branch name that's last in the return list. Otherwise, this header will not be set. The response header is made opaque by proto-serializing it then encoding it with base64. If the token corresponded to a non-existent branch, the server will skip over to the next greater branch. We also prepend an encoded string to the token value as a lightweight way to indicate it was generated by the server, otherwise the request would also fail if an invalid token (not generated by Gerrit) was supplied in the request parameters. Change-Id: Ib3942a4024e4975f04eced3873ebb004cfe609ba Google-Bug-Id: b/288895555 Release-Notes: Allow pagination using continuation token for 'list branches'

2023-07-26

Use RoaringBitmap instead of BitSet in TagSet Leverage the RoaringBitmap libary [1] to optimize usage and storage of TagSets. This makes a significant improvement (see below) particularly for repos with a combination of many tags and many non-tag/non-change refs. Also increment the git_tags cache version since the storage format is changed. This change intends to only do a straightforward replacement of BitSet and does not attempt any further optimizations RoaringBitmap may provide. Performance comparison: These numbers were gathered after running `git ls-remote` against a very large repo with 1.3M refs (including ~375K tags and ~26.5K other non-skippable refs). | | BitSets | RoaringBitmap (this change) | | h2 PageStore heap memory used | 2.5 GB | 155 MB | | disk size of git_tags cache | 1.5 GB | 58 MB | | runtime | 5.75 min | 5.5 min | Further, with Gerrit's heapLimit (java -Xmx) setting set to 26g, the BitSet implementation was not able to successfully execute the ls-remote, whereas the RoaringBitmap implementation continued to perform with a consistent runtime down to at least 16g and even still worked at 10g (lowest tested) but runtime then increased to 6.5 min. [1] https://github.com/RoaringBitmap/RoaringBitmap Change-Id: Ief36b7bb2312899a64efb9a10565b3d095ed5584 Release-Notes: Reduced memory and storage needs for TagSet by using RoaringBitmaps

2023-06-21

Add the 'branch' as a field of RevisionInfo Currently the branch field is only part of the ChangeInfo/ChangeData, however the target branch can be different if the change is moved between patchsets. We keep history of the target branch changes in NoteDb. In this change, we parse and set the 'branch' as a new field in PatchSet. We also surface the 'branch' field on the API in RevisionJson. A note on compatibility: PatchSet is an entity that's serialized to proto (See PatchSetProtoConverter) in two places: 1) in ChangeField to be stored in the change index. 2) as a field in ChangeNotesState, then stored as value in ChangeNotesCache. For [1], until we upgrade the version of the change index, existing patchsets stored in the index will not have the new 'branch' field set, hence any requests for patchsets data that go through the change index will not have the `branch` field set. If the per-patch-set branch does not exist, we only set the branch value to the per-change value if this is the last patch-set. Otherwise, the branch field will be null. For [2], we increased the cache version to force loading the new 'branch' field. The modification to the serialization in this change is forward compatible: an old version of the server code can deal with (deserialize) entities written with the new 'branch' field, since this field is optional. Release-Notes: Add 'branch' as a new field in RevisionInfo Forward-Compatible: checked Google-Bug-Id: b/282076066 Change-Id: I166e86d0e52dbaa9b9c3757d2034827653f745fb

2023-04-26

Add Key-value pairs to notedb These can be used to store metadata on a change. If the value is empty, it's considered a deletion of the given key. The format on the change-message is for a given <key> <value> is: Custom-Values: <key>=<value> Given this, keys and values may not contain any of `\r\n\0`. Some example use-cases: - Storing an issue - Storing a project name or namespace Future use-cases might require indexing by key (or key-value pair) but this is currently not in scope. Key-value pairs may not contain personal identifiable information. They can be fetched along with the ChangeInfo if the option `CUSTOM_KEYED_VALUES` is passed along. Forward-Compatible: true Release-Notes: Adds a new footer for changes for key-value storage. Change-Id: I4f00073f2d523d38d53424000e415e974e9df1dc

2023-02-07

Remove assignee from Change value classes. This is part of the series of changes removing assignee functionality. Forward-Compatible: checked Google-Bug-Id: b/267456422 Release-Notes: skip Change-Id: I06fee5509e443a45afc631859faabc7589138a72

2023-02-06

Remove ability to read and write assignee. This is part of the series of changes removing assignee functionality. We keep CommitRewriter to be able to rewrite databases written by older versions. But remove assertions in the tests that require ChangeNotes api calls Forward-Compatible: checked Google-Bug-Id: b/267456422 Release-Notes: skip Change-Id: I37ed3e5b64695bfe340c91d5556072162965e2ee

2023-02-01

Support rebasing on behalf of uploader It is common to configure submit requirements in a way so that a change can only be submitted if two users agree to the change (2 person principle). Often this is implemented by using 'label:Code-Review=MAX,user=non_uploader' in the submit requirement. In this case a change can only be submitted if it has a Code-Review approval from a user that is not the uploader. This satisfies the 2 person principle, since 2 users, the uploader and a reviewer, agree to the change. The approval from the uploader is implicit, while the approval from the reviewer is explicit. This stops working well when the reviewer rebases the change. If the reviewer rebases the change, they become the uploader of the rebased patch set. Now the approval of the reviewer is no longer sufficiant to submit the change (since the reviewer is the uploader their approval is no longer matching the 'label:Code-Review=MAX,user=non_uploader' condition). In practise this means that after a change has been rebased by the reviewer, an approval of another user (e.g. the original uploader) is required to make the change submittable again. This is a problem, if the original uploader is in another timezone, since now you either need to wait a day with the submission to gain the approval of the original uploader or you need to get a third person involved (which often just rubber-stamps the change). Known workaround are: 1. Do a manual rebase then ask for re-approval in the team chat. 2. Ask for a rebase in the team chat (keeps the approval of the reviewer intact if it's configured to be sticky). 3. Don't let the reviewer rebase the change, but use the Rebase-If-Necessary submit strategy so that Gerrit performs an automatic rebase on submit (after the approvals were checked). 1. and 2. are extra steps in the process that slow down the submission of changes and frustrate the users. 3. doesn't work for teams that require a pre-submit verification of the exact commit that will be integrated into the target branch (with the Rebase-If-Necessary submit strategy the verification is done on the patch set that is the latest at the moment when the submit button is clicked, but not on the rebased commit that is created by the Rebase-If-Necessary submit strategy). To solve this problem we support rebasing on behalf of the uploader now. Rebasing a change on behalf of the uploader means that the uploader stays intact when the reviewer rebases the change. This means an approval of the reviewer on the rebased change is sufficient to submit it. Rebasing a change on behalf of the uploader records the real user, the user doing the rebase, in NoteDb, so that impersonated rebases can be detected there. The real user is recorded by the 'Real-user' footer which is also used if a vote is applied on behalf of another user or if a change is submitted on behalf of another user. It's worth to note that rebasing a change on behalf of the uploader multiple times in a row is possible. Rebasing on behalf of has the following limitations: 1. We only allow rebasing on behalf of the uploader, but not on behalf of any user: If rebasing on behalf of any user would be allowed, users could misuse this to self-approve their own changes: Upload a change, rebase it on behalf of another user to make them the uploader and gain their implicit approval, then approve the change to add an explicit approval (which counts now since they are no longer the uploader). To prevent this, we only allow rebasing on behalf of the user who uploaded the patch set that is being rebased. 2. Rebasing on behalf of the uploader is only allowed for trivial rebases: If the 2 person principle is implemented by a submit requirement that uses 'label:Code-Review=MAX,user=non_uploader', we require an explicit approval from a reviewer and we assume an implicit approval from the uploader. The implicit approval of the uploader only approves the exact diff that the uploader has uploaded. Hence if the rebase that is done behalf of the uploader would be allowed to include further modifications (e.g. conflict resolutions) those modifications would wrongly be considered as implicitly approved by the uploader, but the uploader never saw these modifications. Hence non-trivial rebases on behalf of the user are not allowed. In pratise this means that the 'allow_conflicts' options cannot be used when rebasing a change on behalf of the uploader. 3. Rebasing on behalf of the uploader is only allowed for the current patch set: Gerrit allows to rebase outdated patch sets (this creates a new patch set that restores the contents of the outdated patch set), but this is an edge case that is not supported in the web UI. Disallowing rebasing outdated patch sets on behalf of the uploader allows us at Google to check device trust (check that the patch set was uploaded from a trusted device) for the rebased patch set. For patch sets that were created on behalf of the uploader we don't have device information available for the uploader, but only for the rebaser. The device information for the rebaser is not relevant, since the patch was originally uploaded by the uploader, not by the rebaser. If we would trust the rebasers device for patch sets that were rebased on behalf of the uploader, it would be possible to do an upload from a untrusted device and then make it trusted by rebasing it from a trusted device (e.g. make someone with a trusted device click on rebase). If rebasing of outdated patch sets on behalf of the uploader is not allowed, we can check the device trust for patch sets that were rebased on behalf of the uploader by finding the previous patch set that was originally uploaded by the uploader and check the device trust for this patch set. In addition, support for rebasing on behalf of the uploader is not implemented (yet) for the following cases: 1. Rebase on behalf of the uploader is not supported for rebasing chains: This is not implemented for now since supporting this is more complex than supporting it for a single change and we don't want to invest time into this now. Likely we want to support this in the future. 2. Rebase on behalf of the uploader by git push: We only support rebasing on behalf of the uploader via the REST API, but not via git push. For git push it could be implemented by adding a push option, by we would need to check that the rebase is a trivial rebase and checking this may be expensive. We don't have a use case to support rebasing on behalf of the uploader via git push, hence we have no plans to implement this, but implementing it should be possible. Rebasing on behalf of the uploader is supported via REST. For this a new 'on_behalf_of_uploader' boolean flag is added to the input of the Rebase REST endpoint. Technically to do the rebase on behalf of the uploader we need to create an IdentifiedUser instance for the uploader that contains the user doing the rebase as the real user (created by runAs(...)) and use it for the rebase operation. For this we create a new RevisionResource that contains the created IdentifiedUser instance. This is similar to how voting on behalf of other users is implemented in PostReview (see onBehalfOf(...) method). Permission-wise rebasing on behalf of the uploader requires that the rebaser is allowed to rebase and that the uploader on whom's behalf the rebase is done can create the new patch set. Rebasing is allowed for users that have the Rebase or the Submit permission. In addition change owners can always rebase their own changes. See ChangeControl#canRebase(). Normal rebasing (on behalf of yourself) also requires the Create Change aka the Push permission. For rebase on behalf of the uploader we only require this permission for the uploader on whom's behalf the rebase is done (as this user is the creator of the patch set), but not for the rebaser. The user on whom's behalf the rebase is done must have the following permissions: * The Read permission that allows to see the change. * The Push permission that allows upload (see above). * The Add Patch Set permission if the change is owned by another user. * The Forge Author permission if the patch set that is rebased has a forged author (author != uploader). * The Forge Server permission if the patch set that is rebased has the server identity as the author. Usually the uploader should have all these permission since they were already required for the original upload, but there is the edge case that the uploader had the permission when doing the original upload and then the permission was revoked. Note that patch sets with a forged committer (committer != uploader) can be rebased on behalf of the uploader, even if the uploader doesn't have the Forge Committer permission. This is because on rebase on behalf of the uploader the uploader will become the committer of the new rebased patch set, hence for the rebased patch set the committer is no longer forged (committer == uploader) and hence the Forge Committer permission is not required. For rebase on behalf of the uploader we need to check these permissions explicitly since we want to return a 409 Conflict response with a proper error message if they are missing (the error message should say that the permission is missing for the uploader). The normal code path also checks these permission but the exception thrown there would result in a 403 Forbidden response and the error message would wrongly look like the caller (i.e. the rebaser) is missing the permission. Checking the permissions for the impersonated user explicitly is also what we do for other operations that are done on behalf of another user (e.g. see the label permission check for the impersonated user in PostReview#onBehalfOf(...). This means for the rebaser the required permissions are slightly different for normal rebase and rebase on behalf of the uploader: * normal rebase: rebaser must be change owner or have the 'Submit' or 'Rebase' permission, and have the 'Push' permission * rebase on behalf of the uploader: rebaser must be change owner or have the 'Submit' or 'Rebase' permission We did consider adding a dedicated 'Rebase on behalf of uploader' permission to control who can rebase on behalf of the uploader, but at the moment we don't see a good reason for such a permission. However if it turns out later that there is a use case that requires to allow normal rebase, but not rebase on behalf of the uploader, such a permission may still be added then. Since both operations, normal rebase and rebase on behalf of the uploader, are done by the same REST endpoint (the Rebase REST endpoint) there is only a single UI action for this. We now enable this UI action if the user can either rebase or rebase on behalf of the uploader. This can lead to a situation where the rebase action is enabled because the user can rebase on behalf of the uploader, but the user can't do a normal rebase. To allow the web UI to recognize and handle this situation we add a new 'enabled_options' field to ActionInfo that returns the enabled options. For the 'rebase' action we populate the following options: * 'rebase': Present if the user can do a normal rebase. * 'rebase_on_behalf_of_uploader': Present if the user can rebase on behalf of the uploader. Finally, we extend RevisionInfo and add a new 'real_uploader' field to it that returns the real uploader if a revision was created by rebasing it on behalf of the original uploader. This allows the web UI to show the real uploader for such patch sets (e.g. similar to how the web UI shows the real author for impersonated change messages). To be able to return the real uploader in RevisionInfo we must store the real uploader in PatchSet when reading patch sets from NoteDb. Having a new field in PatchSet means we also have to add it to the patch set proto. For backwards-compatibility, when reading the proto and a real uploader is not set we set the real uploader to the uploader. Release-Notes: Added support for rebasing on behalf of the uploader Change-Id: Ie11e72ec5c966a3648fd68b53ca0673ca9c80401 Signed-off-by: Edwin Kempin <ekempin@google.com> Forward-Compatible: yes

2022-11-14

Remove html commentlink functionality. Html commentlinks are too powerful and often allow for arbitrary html injection on the page. Instead they are replaced with link commentlinks, using optional `prefix`, `suffix`, `text` parameters to achieve the same functionality. The commentlinks that do not generate a link, but rather leverage the commentlink arbitrary string replace mechanism to do something else are explicitly no longer supported. For the migration of gerrit.config files. Follow instructions in tools/migration/html_to_link_commentlink.md Release-Notes: Remove html commentlink functionality. Existing configs can be migrated using scripts in tools/migration/html_to_link_commentlink.md Google-Bug-Id: b/33429040 Forward-Compatible: checked Change-Id: I66e9c83dff7b87cf566eea6a30daac6835aea9a1

2022-10-04

Merge "Include old and new file modes in diffs"

Include old and new file modes in diffs This was previously implemented and approved for the legacy diff cache (see change I79f483fa8) that we abandonned since we redesigned the diff cache. In this change, we add the old and new files modes to the diff output and surface them with the diff endpoints. Adding old/new modes to the response is essential such that if a file is modified with a change to the file mode only, the file will be listed as modified. Google-Bug-Id: b/179792565 Release-Notes: add old/new file modes to the 'List Files' diff endpoint Forward-Compatible: checked Change-Id: I04f6fd9f4fba2658f3ee5f5529a78af93d211836

2022-09-21

Add prefix, suffix and text field to the commentLinks. We are planning to deprecate html option for the commentLinks. Most common usecase for raw html matches is to make links not extend over token boundaries or to configure the text of the link. In order to fill the gap in functionality we are adding prefix, suffix and text fields. The generated link will look like this: PREFIX<a href="LINK">TEXT</a>SUFFIX Change-Id: I0ac43e39a7484dee59928958b52cf5602abcc366 Release-Notes: Add ability to specify prefix, suffix and text fields for commentLinks of type `link`. Forward-Compatible: checked Google-Bug-Id: b/216788721

2022-06-30

Remove support for deprecated approval config fields in label configs The boolean flags in label configurations to control approval copying, as well as the copyValue settings, have been deprecated in favor of the new copyCondition setting that allows to express the copy conditions as a query (see change I7af6414bd and change Ibc79273c1). Existing usages of the deprecated field have been migrated to copy conditions by change Iaf3383262. Change I59d93486c added a validator that prevents setting the deprecated fields. This means the deprecated fields are unused now and we can drop the support for them. Signed-off-by: Edwin Kempin <ekempin@google.com> Release-Notes: Removed support for deprecated approval config fields in label configs Change-Id: I921a61ff4e26ed1c731b73680f3a466bc2661a4f Forward-Compatible: checked

2022-05-24

Add 'hide' SubmitRequirementResult property to cache The property was originaly implemented in change 329952, but since it was not included in cache, the value is always 'false' and the SR is no filtered out from the results. Change-Id: I213cd5e3dd9e46a09907aace6f7304aba1360909 Release-Notes: skip

2021-12-10

Merge "Mark submit requirements as bypassed if change is pushed with %submit"

Mark submit requirements as bypassed if change is pushed with %submit If a change is uploaded for review with the "%submit" option, submit rules/requirements are bypassed. To enable after the fact auditing, we used to store an extra marker submit record with status=FORCED to indicate that all other records were forced. In this change, we implement the same behaviour for submit requirements. We also bypass submit requirements by setting a "forced" boolean in SubmitRequirementResult. After the change is merged, we return the "FORCED" status for all SRs on the API. Change-Id: I3305bfeee661cdb7af0b954c7b4d9a9348a75a58

Merge "Introduce PatchSetApproval UUID and persist it in NoteDB"

2021-12-08

Merge "Add a "description" field to the label definition"

Add a "description" field to the label definition With submit requirements, we'll have some labels associated with submit requirements while others aren't (for now we name them 'trigger votes'). This change is to provide more clarity/explanation of what trigger votes mean and show this explicit description in the UI. Bug: Google b/205810851 Change-Id: Ic7bc1a6f5938958a3c1a6de206ee226b2b1fb320

2021-12-06

Introduce PatchSetApproval UUID and persist it in NoteDB The Globally Unique Identifier for PatchSetApproval will make it possible to reference the granted PatchSetApproval from an external storage. The copy approvals have the same UUIDs as the original approvals, (reference the original vote), which allows obtaining the original entry from the external storage. The UUID is unique for each granted vote: votes, re-issued within the same patch will have different UUIDs. Change-Id: Ifb7a347b0671b1e2622fcc73fa65b64657cddad7

2021-11-30

Remove ApprovalCache In the predecessor change, we removed all uses of this cache. Now, we can also remove all references to this cache since it's no longer used. It's not needed since it was initially created as a performance optimisation, but now we persist all votes in NoteDb so we can just read the votes from there. Change-Id: I9212c5c4b01b019690aefbdafb58a2c321a0ada3

2021-09-23

Persist copying approvals to NoteDb This change persists the copied votes to NoteDb and ensures they are accessible by ChangeNotes. We do not use the copied votes in any way, and this will be implemented in a follow-up change. This is a step to stop computing copied votes on demand, but only compute them once during new patch-set creation. Advantages: 1. Better latency. Currently, to compute copied vote from ps1 to ps10, we have to compute whether we can copy to ps1 -> ps2 -> ... -> ps10, which is O(number of patch-sets), which can lead to bad latency. If we persist the copied votes, we only have to look at the previous patch-set. 2. Another argument of latency: this has to be computed every time we retrieve the change, compared to once per patch-set upload. 3. Less error prone: we had a security issue that happened because of a bug in the logic of copying across multiple patch-sets. Disadvantages: 1. Either we perform a migration to persist past votes to NoteDb, OR, at some point we stop computing the copied votes on demand, which means that for old changes we will not have copied votes. However, this issue already happens for old changes if the copy conditions for the project change. 2. We used to be able to change the copy conditions on the project and see the immediate effect on changes. That will no longer work: if a vote is copied, it will stay copied (this can also be an advantage, though). Storage Layer: We store the copied votes the same way that we store the other votes, and we just add a "copied" flag. Removed votes: When removing votes we put a "0" vote on the current patch-set. Since the keys for both votes and copied votes are the same, we don't need to implement anything to allow removing votes. Copied votes on past patch-sets will stay, but they will not be visible to the user as only the latest patch-set matters. Timestamps: The timestamp of copied votes will be the commit timestamp (and not the original vote's timestamp). This is fine especially since the timestamp is not even user visible, and it somewhat makes sense that the timestamp of the new copied vote is when the patch-set is created. Google-bug-Id: b/195909329 Change-Id: Ia1ab9b3c5680362f6db31f71b29d7a922b98254d

2021-09-14

Add auth.userNameCaseInsensitive option In Gerrit usernames are handled case sensitive, e.g. johndoe and JohnDoe can both exist as separate accounts. This could lead to issues when migrating an account from LDAP to an internal account, if ldap.localUsernameToLowerCase was set. Such usernames can also be rather confusing for users, if they try to identify authors of comments or changes. This change adds an option to handle accounts case insensitive but case preserving. To achieve this the external ID notes of external IDs using the 'gerrit:' or 'username:' scheme are named with the SHA-1 from the lowercase external ID. With this change there can be no accounts with usernames only different in capitalization. On existing Gerrit sites this may require to migrate the external ID note names to match the new scheme. A tool to do so will be provided in a separate change. Change-Id: I64db5ed5256564dbc03952e9721615f42212ad48

2021-09-06

Add a legacy flag to submit requirements results This is to differentiate between results that are created from legacy submit records from results created by evaluating submit requirements that are stored in the project config. Change-Id: Id00abed12a3ddc46bdcbe2732f78c2cae8e68c12

2021-08-02

Change: Remove unused rowVersion This field was used for optimistic locking in ReviewDb and is now unused. Change-Id: I45280c43330fbf8d5103127d2d771c50ba449328

2021-07-19

Enable parsing submit requirements results from change meta commits This change enables parsing submit requirement results from NoteDb and stores them in ChangeNotesState. Since instances of ChangeNotesState are cached, we implement serializing submit requirement results to proto. A test is added to ensure correct serialization. We also store the latest patchset commit ID as a field in SubmitRequirementResult, since it's needed later when submit requirements are stored in the change revision notes. Previously, The change meta ref contained published comments only. Since now it can contain results of submit requirements, we rename HumanCommentsRevisionNoteData to ChangeRevisionNoteData. In the next change in this series, we'll implement storing submit requirements in NoteDb when the change is merged. Change-Id: I582c12eae8089d4a774125b86b914baa29773790

2021-06-25

DiffOperations: Move the timeout logic inside the diff cache In this change we do the following: 1) We move the timeout logic from DiffOperations to the GitFileDiffCache. The timeout is enforced while computing the file header (similar to the old diff cache, see [2]). If the computation times out, we cache negative results. We enforce the timeout with a boolean variable (see (2)). The FileDiffCache (which wraps the GitFileDiffCache and performs extra logic, e.g. computing rebase edits) will also cache negative results. 2) Add an extra boolean field to the GitFileDiffCacheKey and the FileDiffCacheKey called "useTimeout". We only enforce timeouts if the diff algorithm uses the Myers diff fallback. The new logic now goes as follows: DiffOperations requests diffs using the HISTOGRAM_WITH_MYERS algorithm and with useTimeout=true. If we get back negative results (meaning that the computation times out), we request the diffs again using the HISTOGRAM_NO_FALLBACK and with useTimeouts=false (we don't want to enforce timeouts in this case). This logic is matching what was done in old diff cache. The reasonning behind this change is the following: * The timeout logic in the old diff cache was added in the past (many years ago) because of a bug in the Myers diff algorithm in JGit that causes an infinite loop. See Ib00de070 and [1]. It looks like the bug isn't fixed yet. The timeout was only employed while computing the file header (see [2]) so was not enforced on the whole diff computation. [1] suggets that if the JGit bug in Myers diff is fixed then we can remove the timeouts in Gerrit, i.e. we did introduce the timeout logic in Gerrit as a workaround for this bug. * It wasn't a good idea to keep the timeout logic in DiffOperations: This caused the @DiffExecutor to create new threads with every request to get the diff, whether it's cached or not, and caused an overload to the diff executor. With the logic of this change, once the result is cached (whether positive or negative), we will not have to go through the cache loader (or the timeout) logic again. [1] bugs.chromium.org/p/gerrit/issues/detail?id=487 [2] https://gerrit.googlesource.com/gerrit/+/0dc27d2a5b512fdcb7ce5f95b412e174fe33c7d2/java/com/google/gerrit/server/patch/filediff/PatchListLoader.java#530 Change-Id: I680454cf93f78b90025bab95ed3717c74dbf32bf

2021-06-08

Merge changes I36c23edc,I02ea6398,Ie1706b20 * changes: Submit requirements: rename blocking expression to submittability Implement evaluating submit requirements using change data Implement evaluating submit requirement expressions

2021-06-04

Cache PatchSetApprovals keyed by the SHA1 of ChangeNotes Gerrit copies approvals to newer patch sets based on a set of rules (boolean configs and query). Currently, this copying is done on demand whenever approvals are requested (e.g. when users view the change screen). The logic is quite simple and loops over past approvals and then tries to copy them forward patch set by patch set. Hence, the complexity is O(#patch-sets). The new query logic allows for more complex predicates (e.g. checking group membership) that would slow this operation down. To solve this, this commit adds a persisted cache for patch set approvals). We have a long-term vision to persist patch set approvals in NoteDb, not only on submission but also whenever a new patch set is uploaded. However, this requires backfilling data and until that has happened, we'd still suffer from the performance degradation. This cache solves our problem today and serves as intermediary state to storing approvals in NoteDb. The current on-demand logic has the advantage that when project owners change the copy configs, they affect all open changes immediately. However, this advantage is not documented, hence not promised (and even if, we could change that). With this change, we move towards a model where we store approvals. Change-Id: Ie53fbce4e0fb6a46b6b4573baf7ba5b9e3b5b581

Add 'copyCondition' to the label section in project.config With this change, we wire up logic created in predecessor changes. We allow users to configure a copyCondition. If true, any label votes are copied from one patch set to the next. copyConditions work alongside existing configs. If any of these controls allows copying the label, we copy. We will deprecate other configs (copyMinScore, ...) in the future and migrate projects to use the more expressive copyCondition. Change-Id: Ib755f3aeaad6800b17bd2ce27c972f4fbacf9e02

2021-05-27

Submit requirements: rename blocking expression to submittability We intend to define blocking expressions as becoming fulfilled when they evaluate to true. Example `label:"code-review=+2"` is fulfilled when the label is actually voted on. To avoid confusion, we rename blockingExpression to submittabilityExpression. The blocking expression notation shall not be used anymore. Change-Id: I36c23edc4e6e4add3c53e22e77f5074506c4363e

2021-04-26

Implement parsing submit requirements from project.config As per the new "Composable submit requirements" design (see I32e9863c8d), we define the new submit requirements and store them in the project's config file. In this change, submit requirements are loaded from project.config (if they exist) and cached as part of the CachedProjectConfig. Notes: 1) Later on, we should implement syntax validation of the applicability/blocking/override expressions while loading the submit requirements from the project config. 2) There is no need to increase the version of the persisted project cache. This cache is keyed by the SHA-1 of refs/mata/config making all cache entries immutable. When a new change updates refs/meta/config for a project, a reload of the CacheProjectConfig will be enforced with a new SHA-1 in the key. This change didn't document the new section definition in project.config. A follow up change should be added for the documentation after the logic for submittability is implemented. Change-Id: I9bb7d5a681aedf8ebb5361b1b5ecf2c73a4c0be3

2021-03-23

Persist group cache by uuid We split the groups by uuid cache into two caches: 1. "groups_byuuid" stays the same. Caches by uuid without reading the SHA-1 of the ref from NoteDb. 2. "groups_byuuid_persisted" is the persistent variant of the above cache. This is used in the loader of "groups_byuuid" and it has the SHA-1 of the ref in the key. The new setup eases cold start times. The split is useful to allow accessing groups without loading the SHA-1 for each lookup. As a follow-up, we can also implement another method that gets multiple groups at the same time. This will be more efficient given that we will only need to access NoteDb once instead of once per group. This will be needed when getting the subgroups. While at it, we needed to add a new method that loads a group (to load it for a specific revision) so we also add a few @Nullable for other methods (since the sha-1 can be null). Change-Id: Ia52b3a3edcae0589c785e9d093776ec26dc324ca

2021-03-01

Merge "Add the source content type to the comment context"

2021-02-26

Add the source content type to the comment context This change adds a new field "source_content_type" to the CommentInfo entity. This field contains the content type (mime type) of the source file where the comment is written. This is needed by the front-end to do syntax highlighting. We return the file content type in the Get Diff endpoint (check DiffFileMetaInfo). In this change we are reusing this small part of the diff logic to compute the mime type of the source file while loading the comment context. Also added some tests to make sure the content type is returned correctly, and another test to make sure the comment context is de/serialized correctly. Change-Id: Ib1f7281866ca6a11e88e44024a424f253efcb758

2021-02-23

Surface comparison type and old/new commit IDs in the file diff output This is needed because callers of the diff cache can request a diff against parent, hence they don't know which base commit will be picked. Surfacing this information so that callers can have access to it. Change-Id: I06f5811bdec43b4ec0b401b9ea20097ac5297424

2021-02-09

Merge "Inline entity protobufs directly into cache protobufs"

2021-02-08

Add the context-padding parameter to the comment context This parameter allows padding the context lines with extra lines before and after the comment range. This parameter works only if the enable-context parameter is set to true. This change is backward compatible: if the new parameter is not set or set to zero, only the lines of the comment range are returned. Change-Id: I2d44fb852c68ba484cbf94adff912b7ee522f9c5

Inline entity protobufs directly into cache protobufs Before, some fields of the ChangeNotesStateProto carried `bytes` fields holding serialized protocol buffers. Per https://developers.google.com/protocol-buffers/docs/encoding#embedded the encoding of an embedded message with tag X is equal to the encoding of a string/bytes with tag X. By inlining the entity directly, we make the relations explicitly visible, and enable reflective analysis of the entire cache value. Change-Id: Ifd29abe9faf215dfe21889d0bfe7ae3f5b9b1415

2021-01-19

Create a label config for copying all scores if list of files is the same This change allows creating new labels or setting to an old label a new label config: copyAllScoresIfListOfFilesDidNotChange. Labels that have this config set to true will copy all scores if the list of files did not change. Renames count as the same file (= no file change); as long as there are no explicit additions or deletions to the list of files, the scores will be copied to the next patch-set. Change-Id: I0051ea1236e6bd50a69455d799ce27081091d21a

2021-01-05

Add protobuf serializers for the FileDiffOutput and FileDiffCacheKey Change-Id: Ib722f9c895091c30274f7cc65b320844a2d43922

2020-12-01

Merge "Expose the mergedOn date of a change in ChangeNotes"

Expose the mergedOn date of a change in ChangeNotes This is required for indexing the change on the merge date to implement the merge search operator. - Currently, this value can be extracted from ChangeData.getSubmitApproval - However, it should not be used as it is planned to deprecate SUBM label from NoteDb, see LabelId.LEGACY_SUBMIT_NAME, see the change Id7fbddfa. - Also cache the mergedOn date as a part of ChangeNotesState. Change-Id: I5b54fda392f1c6ca86d7f3440a227ded2aa40eb0

2020-11-20

Add protobuf serializers for the GitFileDiff and GitFileDiffCacheKey Change-Id: Id1537c2f756732743e2ba82e8aab11c3e52f8f15

2020-10-27

Extract the keys for the (Git)ModifiedFilesCaches to separate classes Change-Id: If8ea827f82ae369eb744fe8eb60e88f528762179

Add protobuf serializers for the GitModifiedFilesCache entities This change adds protobuf serializers for the GitModifiedFilesCacheImpl#Key and to the ModifiedFile entity. The change also includes a round-trip tests for the serializers. Change-Id: I154aba001a0ba5c432fbb6941d0b4991923ea2f5

2020-10-12

Merge "Cache the comment context"

2020-10-08

Cache the comment context This change introduces a cache interface for the comment context (CommentContextCache). The request execution flow is as follows: ListChangeComments loads all comments from the change notes and passes them to the CommentJson for formatting. If the "context" flag is set, CommentJson uses the CommentContextCache to evaluate and retrieve the context for all comments. The comment context cache interface uses a persisted cache and provides two methods: get() and getAll(). The getAll() method is the one used by CommentJson. The cache loader also overrides the load() and loadAll() methods. The cache loader uses CommentContextLoader.java to load the context by opening the files at the exact commits specified for each comment. The loadAll() method groups all comments by project and change ID (both specify a certain commit). All comments in the same group are loaded simultaneouly. Change-Id: I4bc24c0556974dac99d14fef3128f6db8da18e54

2020-10-07

Maintain all attention set updates in ChangeNotesState This allows gathering change-based metrics on attention set history, and not only on the current status of the attention set (which will just be empty on the majority of cases on old changes). Change-Id: I6f8ffc72f533c4f706355820151ada74e3203421

2020-08-11

Persist ProjectCache This commit splits the 'projects' cache into two pieces: 1) The 'projects' cache: This cache continues to work as it did before. It offers a by-name caching without the need to present or load the ObjectId of refs/meta/config. 2) The 'persisted_projects' cache: This cache is the persisted variant of 'projects'. It's used in the loader for 'projects' and - as all persisted caches - has the ObjectId in the key. This new setup allows admins to persist the project cache - if desired - to ease cold start times. This is an additional performance improvement next to ProjectCacheWarmer. It might at some point replace ProjectCacheWarmer. For now, the two co-exist. The split is done in this way to make sure we can still access projects in the cache without loading a SHA1 for each lookup as that would be prohibitively slow given the amount of requests this cache serves. Change-Id: I174076acd90ae815ac66e1e0f8a72db0701bf2d3

2020-08-07

Serialize ProjectLevelConfig and PluginConfig in CachedProjectConfig Change-Id: Icba74aca874be352575537318d49b32ee5c76484

2020-07-23

Serialize CachedProjectConfig This commit is the final piece of a series that allows us to serialize CachedProjectConfig. After this commit, we can make ProjectCache a serialized cache, which speeds up cold-starts. Entities are moved to com.gerrit.entities where needed. Change-Id: I501e68ae44d4bdfe0ecc6d000c4eb5644d0bec4c

2020-07-17

Add serializer for StoredCommentLinkInfo This commit adds a serializer for the StoredCommentLinkInfo entitiy. The eventual goal is that we serialize CachedProjectConfig. The entity is too large to be serialized directly, though, so we divide and conquer. This commit moves the AutoValue representation to the entities package to allow the serializer packages to keep its dependencies minimal. Change-Id: I860e2c4c1a845d2e518c1a331711e1bc66ac85be

Add serializer for SubscribeSection This commit adds a serializer for the SubscribeSection entitiy. The eventual goal is that we serialize CachedProjectConfig. The entity is too large to be serialized directly, though, so we divide and conquer. Change-Id: I0333f8cf8582eb9d3028ea3f5b35120b43d209c4

Add serializer for ConfiguredMimeType This commit adds a serializer for the ConfiguredMimeType entitiy. The eventual goal is that we serialize CachedProjectConfig. The entity is too large to be serialized directly, though, so we divide and conquer. This commit moves the AutoValue representation to the entities package to allow the serializer packages to keep its dependencies minimal. Change-Id: I524ee5fe0358ebefa05b4a1fb24fd34631ef721d

Add serializer for LabelValue This commit adds a serializer for the LabelValue entitiy. The eventual goal is that we serialize CachedProjectConfig. The entity is too large to be serialized directly, though, so we divide and conquer. This commit moves the AutoValue representation to the entities package to allow the serializer packages to keep its dependencies minimal. Change-Id: If04e7efb06cb8c53b995296e39194556a3d97dab

Add serializer for LabelType This commit adds a serializer for the LabelType entitiy. The eventual goal is that we serialize CachedProjectConfig. The entity is too large to be serialized directly, though, so we divide an conquer. Change-Id: I6acbba0acd26de42fd285a8fe5a23ae5c8706430

Add serializer for NotifyConfig This commit adds a serializer for the NotifyConfig entitiy. The eventual goal is that we serialize CachedProjectConfig. The entity is too large to be serialized directly, though, so we divide and conquer. This commit moves the AutoValue representation to the entities package to allow the serializer packages to keep its dependencies minimal. Change-Id: I6876624bbe5d99c3ca187984f1214bf2066850fa

Add serializer for Address This commit adds a serializer for the Address entitiy. The eventual goal is that we serialize CachedProjectConfig. The entity is too large to be serialized directly, though, so we divide and conquer. This commit moves the AutoValue representation to the entities package to allow the serializer packages to keep its dependencies minimal. Change-Id: I37192013b1da23499fd6b64d012ea82696b74df4

Add serializer for ContributorAgreement This commit adds a serializer for the ContributorAgreement entitiy. The eventual goal is that we serialize CachedProjectConfig. The entity is too large to be serialized directly, though, so we divide and conquer. Change-Id: I8b4ea2933f83821a68d4f7e494e4aa3751d3eb86

Add serializer for BranchOrderSection This commit adds a serializer for the BranchOrderSection entitiy. The eventual goal is that we serialize CachedProjectConfig. The entity is too large to be serialized directly, though, so we divide and conquer. This commit moves the AutoValue representation to the entities package to allow the serializer packages to keep its dependencies minimal. Change-Id: I4a481343c687cf1adba8c77e6d75bbf9b3afab31

Add serializer for AccessSection This commit adds a serializer for the AccessSection entitiy. The eventual goal is that we serialize CachedProjectConfig. The entity is too large to be serialized directly, though, so we divide and conquer. Change-Id: Ibcf681e86c68d10225f468c7f1ee4d51ed35c365

Add serializer for Permission This commit adds a serializer for the Permission entitiy. The eventual goal is that we serialize CachedProjectConfig. The entity is too large to be serialized directly, though, so we divide and conquer. Change-Id: Ia1bfe912f15ddb3d5c2e2bab4586944550845eb5

2020-07-10

Add serializer for PermissionRule This commit adds a serializer for the PermissionRule entitiy. The eventual goal is that we serialize CachedProjectConfig. The entity is too large to be serialized directly, though, so we divide an conquer. Change-Id: Ifc691c58486c57c3a1f3ed80b731ef698011526f

Add serializer for GroupReference This commit adds a serializer for the GroupReference entitiy. The eventual goal is that we serialize CachedProjectConfig. The entity is too large to be serialized directly, though, so we divide an conquer. Change-Id: Ica3838752dba4b13661f41927f2c43ad0a3d93a9

2020-07-09

Add serializer for Project This commit adds a serializer for the Project entitiy. The eventual goal is that we serialize CachedProjectConfig. The entity is too large to be serialized directly, though, so we divide an conquer. Since we use proto3 for serialization, we have to convert null values to empty strings and back. For all the fields we serialize we do not actually care about the difference of null and empty strings. Change-Id: I1fbba29ced9ad6dd836ef43674ac033960b19edd

2020-04-07

Serialize AccountCache Data on googlesource.com suggests that we spend a significant amount of time loading accounts from NoteDb. This is true for all Gerrit installations, but especially for distributed setups or setups that restart often. This commit serializes the AccountCache using established mechanisms. To do that, we decompose AccountState - the entity that we currently cache - into smaller chunks that can be cached individually: 1) External IDs + user name (cached in ExternalIdCache) 2) CachedAccountDetails (newly cached) 3) Gerrit's default settings (we start caching this in a follow-up change) CachedAccountDetails - a new class representing all information stored under the user's ref (refs/users/<sharded-id>) is now cached in the 'accounts' cache instead of AccountState. AccountState is contructed when requested from the sources 1-3 and not cached itself as it's just a plain wrapper around other state, that we already cache. This has the following advantages: 1) CachedAccountDetails contains only details from refs/users/<sharded-id>. By that, we can use the SHA1 of that ref as cache key and start serializing the cache to eliminate cold start penalty as well as router assignment change penalty (for distributed setups). It also means that we don't have to do invalidation ourselves anymore. 2) When the server's default preferences change, we don't have to invalidate all accounts anymore. This is a shortcoming of the current approach. 3) The projected speed improvements that come from persisting the cache makes it so that we can remove the logic to load accounts in parallel. The new aproach also means that: 1) We now need to get the SHA1 from refs/users/<sharded-id> for every account that we look up. Data suggests that this is not an issue for latency as ref lookups are cheap. We retain the method in AccountCacheImpl that allows the caller to load a Set<AccountState> so that in the cases where we want many many accounts (change queries, ...) we have to open All-Users only once. In case we discover that - against our assumptions - this is a bottleneck we can add a small in-memory cache for AccountState. Related prework: The new aproach shows that the way we handle user preferences is suboptimal, because: 1) We pipe through API data types to the storage 2) We overlay defaults directly in the storage 3) Use reflection to get/set fields. I considered and prototyped a rewrite of this and initially thought I could get it done before serializing the account cache. However it turned out to be significantly more work and the impact of that work (besides being a much desired cleanup) is rather low. So I decided to get the cache serialized independently. Change-Id: I61ae57802f37c62ee9e3552e4a0f19fe3d8d762b

2020-03-31

Merge changes I67ea6014,Iac484155,I1cf3aafd * changes: Add the groups external cache to the config-gerrit documentation Split the groups external cache into 2 caches for better performance Add a test method for the groups external cache

2020-03-27

Split the groups external cache into 2 caches for better performance The current implementation of the groups external cache used an in-memory cache with a single key "EXTERNAL_NAME" that maps to the list of all external groups. In this change we almost left this cache intact, but modified its loader to retrieve the value (external groups) using a persisted external groups cache. The persistent cache has a key that represents the state of NoteDb, and specifically is the concatenation of the SHA-1 of all internal groups' refs in NoteDb. The persistent cache's loader does the expensive operation of loading the groups from NoteDb. Change-Id: Iac48415589eabec479dc658eaa20c2dd4c7b4932

2020-03-19

REST endpoints for managing the attention set Attention set is returned by default in change details. Background: https://www.gerritcodereview.com/design-docs/attention-set.html Upcoming further changes: - Add to index - Extend ReviewInput API to add reviewers - Add invariants (e.g. user must be reviewer) - Send notifications in *Attention*Op#postUpdate() - Consider adding NotificationHandling in AttentionInput - Consider adding validation listeners Change-Id: I52ae870a94852ac98f731fef63f65cd2a7064742

2020-03-11

Merge changes I5f74b124,I3f474829 * changes: Add serializer to Account Add serializer to ProjectWatchKey

2020-03-09

Postfix "Millis" to fields that store timestamps in epoch millis Change-Id: Id7803a643e45107e50df8a4d77297e9e087ac1f6

2020-03-06

Add serializer to Account This commit is the second in a series that will eventually persist the AccountCache on disk in a similar fashion as we do for other caches. As prerequisite, we want to {de}serialize the different parts of AccountState. Change-Id: I5f74b124b3b08619d1d1ad3e0b8b55a39fb85c13

2020-03-05

Add serializer to ProjectWatchKey This commit is the first in a series that will eventually persist the AccountCache on disk in a similar fashion as we do for other caches. As prerequisite, we want to {de}serialize the different parts of AccountState. Change-Id: I3f474829219a55575259505514233fb80f03bbde

2020-02-19

Read/write attention set in ChangeNotes Context: https://www.gerritcodereview.com/design-docs/attention-set.html This change adds code for NoteDb to store and read attention sets. We considered storing this data in the file that also holds the comments (adding a dedicated file would require significant refactoring). This would better accomodate the structured attention set data, but this file is attached to a specific patch set whereas the attention set relates to the change as a whole. We agreed that the commit message is the better storage location, even when this is the first footer line that will have JSON in it and be potentially long. Change-Id: I9e48cf6c1bb6d6d8f28f9ca251cac2c3529af4c8

2020-01-08

Add 'CherryPickOf' field for a change After a change is created or updated using the 'cherry-pick' functionality, this field will contain the source change number and the patchset. Having this field helps us identify changes where actual dev time was spent on by filtering out propagated changes. This is especially useful for organizations wanting to generate cost metrics. Change-Id: I782a56aa52c52670ec74fabb713fe47ecba24de1

2019-10-16

Add AssigneeStatusUpdate Keep details for assignee changes, so that it would be possible to query for changes in the assignee field. Assignee and pastAssignees are removed from ChangeNotesParser as they are not needed. Assignee and pastAssignee are also removed from cache as they can be inferred. Change-Id: I57cd4a5bd5f5886daee26f42a308f0dad1bd8b8b

2019-10-15

Rename reviewdb package to entities and dissolve client package This is long overdue renaming step to manifest that SQL database is removed from gerrit core. Moreover, client/server package division was needed due to GWT UI that was removed as well in release 3.0. Bug: Issue 11678 Change-Id: Icfd83a309a6affac54141e7284e70f1255537dc4

2019-09-24

ChangeNotesParser: Hoist server id check to ChangeNotes In some contextes (e.g. analytics plugin) allow ChangeNotesParser to return ChangeNotesStates objects containing the account instances from foreign servers. It would be bad if these objects escaped the analytics plugin and got used elsewhere in other Gerrit APIs. Any solution that allows to parse arbitrary serverIds from analytics plugin should also include some safety provisions so it doesn't cause unintended consequences elsewhere in Gerrit core/Gerrit plugin API. Here is the plan: * Add a serverId field to ChangeNotesState * Modify ChangeNotesParser/NoteDbUtil to allow any serverId during the parsing phase * In ChangeNotes, reject any ChangeNotesState that has a serverId not matching the serverId of the running server * If serverId is not present, the cached entry was populated with an earlier version and thus serverId has been already checked * Since analytics plugin won't be using ChangeNotes, it doesn't need to run the check that the serverId in the ChangeNotesState matches the current server * Outside of the NoteDb code, all or almost all Gerrit APIs use ChangeNotes, not ChangeNotesState * So with the above approach, it should be mostly impossible to ever see notes in non-analytics contextes with a mismatched serverId. Inspired-By: Dave Borowitz <dborowitz@google.com> Feature: Issue 10174 Change-Id: I9b43f8479206b6373edad857251cecdfde917269

2019-04-29

Convert RevId proto to be based on ObjectId See Iff5644e2 context on removing RevId usages. Change-Id: I635facafaa97abc3936a1ed3911c03f4b76e156d

2019-04-23

Convert Branch.NameKey to AutoValue See I6982fb24 for context. Change-Id: I7a79a374c9efdd4ccd9ab5169562fb8ae3452120

Convert PatchSet.Id to AutoValue See I6982fb24 for context. Change-Id: I681421e2e3415125dc4db44c7f3bf32d37920166

Convert PatchSetApproval.Key to AutoValue While we're in there, rename the underlying field from categoryId to labelId, including renaming the proto field. This is cleanup following from I5df6f0c5. At that time, we opted to leave the name the same because Megastore had problems with renaming proto fields and hence we couldn't easily rename @Column fields. Now that ReviewDb (including the Megastore implementation) is gone, this argument no longer applies. See I6982fb24 for context. Change-Id: I011e0bfeb74345a20b5eb0bb97ce61de7cb12bd0

2019-04-18

Record the number of updates to a NoteDb ref Eventually we may want to limit the total length of a NoteDb ref's history, to avoid having to scan and store an unbounded amount of commit data into ChangeNotes. To do that, we first need to record it. In most cases, this number will equal getChangeMessages().size(), but that's partly an artifact of the current APIs, which always accompany a change with a ChangeMessage. The underlying storage layer doesn't require it. Don't bump the ChangeNotesCache version number: old entries will just return an update count of 0. My current thinking is that populating this field is not worth flushing the persistent cache. If we change our minds, we can trivially bump the version number. Change-Id: I9fbcdfa0583a525e60b75672032f3179e5da3c79