Merge branch 'stable-3.9'
* stable-3.9:
PolyGerrit: use project~changeNum when fetching suggested accounts
Set version to 3.9.1-SNAPSHOT
Set version to 3.9.0
Make sure to discard a draft whenever we're saving.
Position tooltip in front of the dialog.
Assume relative links without / prefix are absolute
Set version to 3.8.4-SNAPSHOT
Set version to 3.8.3
Set version to 3.7.7-SNAPSHOT
Set version to 3.7.6
Set version to 3.6.9-SNAPSHOT
Set version to 3.6.8
Update jgit to 82e277c81339
Fire `ref-updated` stream events on online edit refs update
Fix documentation attributes bug
Introduce Documentation:searchfree_safe with local-only fonts
Release-Notes: skip
Change-Id: Iead90cc3dd6c74229332a3f94c209349e688b5d5
diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt
index 19a19dd..b37ee73 100644
--- a/Documentation/access-control.txt
+++ b/Documentation/access-control.txt
@@ -264,6 +264,7 @@
`-2..+2`, as the user's membership of `Foo Leads` effectively grant
them access to the entire reference space, thanks to the wildcard.
+[[exclusive]]
Gerrit also supports exclusive reference-level access control.
It is possible to configure Gerrit to grant an exclusive ref level
@@ -443,13 +444,13 @@
[[category_create]]
-=== Create Reference
+=== Create (aka Create Reference)
-The create reference category controls whether it is possible to
-create new references, branches or tags. This implies that the
-reference must not already exist, it's not a destructive permission
-in that you can't overwrite or remove any previously existing
-references (and also discard any commits in the process).
+The create category controls whether it is possible to create new
+references, branches or tags. This implies that the reference must not
+already exist, it's not a destructive permission in that you can't
+overwrite or remove any previously existing references (and also
+discard any commits in the process).
It's probably most common to either permit the creation of a single
branch in many gits (by granting permission on a parent project), or
@@ -484,9 +485,9 @@
`${username}` are still reachable by the users.
[[category_delete]]
-=== Delete Reference
+=== Delete (aka Delete Reference)
-The delete reference category controls whether it is possible to delete
+The delete category controls whether it is possible to delete
references, branches or tags. It doesn't allow any other update of
references.
@@ -1618,6 +1619,7 @@
Access to refs can be blocked, allowed or denied.
+[[block-rule]]
==== BLOCK
For blocking access, all rules marked BLOCK are tested, and if one
@@ -1643,6 +1645,7 @@
permissions when they are processed in the ALLOW/DENY processing, as
described in the next subsection.
+[[allow-rule]]
==== ALLOW
For allowing access, all ALLOW/DENY rules that might apply to a ref
@@ -1656,6 +1659,7 @@
This ordering lets project owners apply permissions specific to their
project, overwriting the site defaults specified in All-Projects.
+[[deny-rule]]
==== DENY
DENY is processed together with ALLOW.
diff --git a/Documentation/config-project-config.txt b/Documentation/config-project-config.txt
index 187cd0f..45004b0 100644
--- a/Documentation/config-project-config.txt
+++ b/Documentation/config-project-config.txt
@@ -1,34 +1,239 @@
-= Gerrit Code Review - Project Configuration File Format
+= Gerrit Code Review - Project Configuration
-This page explains the storage format of Gerrit's project configuration
-and access control models.
+Every project has a configuration that defines access rights and controls
+project-specific behavior.
-The web UI access control panel is a front end for human-readable
-configuration files under the +refs/meta/config+ namespace in the
-affected project. Direct manipulation of these files is mainly
-relevant in an automation scenario of the access controls.
-
+The project configuration is stored inside the git repository inside the
+`refs/meta/config` branch.
[[refs-meta-config]]
-== The +refs/meta/config+ namespace
+== The `refs/meta/config` branch
-The namespace contains three different files that play different
-roles in the permission model. With read permission to that reference,
-it is possible to fetch the +refs/meta/config+ reference to a local
-repository. A nice side effect is that you can also upload changes
-to project permissions and review them just like with regular code
-changes. The preview changes option is also provided on the UI. Please note
-that you will have to configure push rights for the +refs/meta/config+ name
-space if you'd like to use the possibility to automate permission updates.
+The `refs/meta/config` branch contains configuration files. It is disconnected
+from the normal branches under `refs/heads/` that contain the source code.
-== Property inheritance
+The files inside the `refs/meta/config` branch are versioned just like any
+other file in the repository. This means from the git history of the
+`refs/meta/config` branch it can be seen how the configuration changed
+over time and which configuration was active when.
-If a property is set to INHERIT, then the value from the parent project is
-used. If the property is not set in any parent project, the default value is
-FALSE.
+[[configuration-files]]
+== Configuration files
+
+The project configuration is stored in the following files (these files are
+stored inside the `refs/meta/config` branch, see
+link:#refs-meta-config[above]):
+
+* link:#file-project_config[project.config]:
+ Contains the link to the parent project, the project description, access
+ rights and options to control project-specific behavior.
+* link:#file-groups[groups]:
+ Resolves group names that are mentioned in `project.config` to group UUIDs.
+* [DEPRECATED] link:#file-rules_pl[rules.pl]:
+ Contains Prolog rules that control when a change becomes ready to submit.
+ Note, Prolog rules are deprecated and have been replaced by
+ link:config-submit-requirements.html[submit requirements].
+
+In addition, there can be configuration files from Gerrit plugins, usually they
+are named `<PLUGIN-NAME>.config`.
+
+[[update]]
+== Updating the project configuration
+
+There are several possibilities to update the project configuration:
+
+[[update-through-web-ui]]
+* Through the Gerrit web UI:
++
+[[update-through-general-screen]]
+--
+* On the project's `General` screen:
++
+The `General` screen can be found under:
+`BROWSE` > `Repositories` > `<project-name>` > `General`
++
+In the `Configurations` section this screen allows to edit the project
+description and change repository options.
++
+Note, this screen only supports changing the most important repository options,
+but it doesn't expose all options that exist.
++
+All changes are directly applied (without code review).
+--
++
+[[update-through-access-screen]]
+--
+* On the project's `Access` screen:
++
+The `Access` screen can be found under:
+`BROWSE` > `Repositories` > `<project-name>` > `Access`
++
+This screen allows to edit the project's access rights.
++
+Updating access rights via the `Access` screen updates the
+link:#file-groups[groups] file automatically.
++
+Modifications can either be applied directly (`SAVE` button) or saved for
+review (`SAVE FOR REVIEW` button).
++
+Saving access modification for review (`SAVE FOR REVIEW` button) creates an
+open change for the `refs/meta/config` branch where the modifications can be
+reviewed by a project owner. The modifications become effective only after a
+project owner submits the change.
+--
++
+[[update-via-online-editing]]
+--
+** Via online editing:
++
+The project's `Commands` screen, that can be found under `BROWSE` >
+`Repositories` > `<project-name>` > `Commands`, offers an `EDIT REPO CONFIG`
+command that allows to edit the `project.config` file directly in the Gerrit
+web UI.
++
+The `EDIT REPO CONFIG` command creates a new work-in-progress change with a
+change edit that contains the `project.config` file. Clicking on the command
+opens an online editor for the `project.config` file, allowing the user to make
+modifications to it.
++
+Modifications need to be saved and published ("saving" saves the modifications
+in the change edit, "publishing" publishes the change edit to make it visible
+to other users). While the change edit is not published yet, it's possible to
+add further files to it (e.g. the link:#file-groups[groups] file if it needs to
+be modified, in contrast to making access rights changes through the `Access`
+screen the `groups` file is not automatically updated). For further details how
+to work with change edits see the link:user-inline-edit.html[inline edit user
+guide]. After publishing the change edit, the change should be set to ready by
+clicking on `START REVIEW` > `SEND AND START REVIEW`. At this time you also
+want to add a project owner as a reviewer so that they can review and approve
+the change.
+--
+
+[[update-by-git-push]]
+* By pushing updates via Git:
++
+Since the configuration files are stored in a git branch, it's possible to
+update them via normal git operations:
++
+--
+1. Clone the repository if you don't have it available yet. The clone command
+ can be found in the `Download` section of project's `General` screen
+ (`BROWSE` > `Repositories` > `<project-name>` > `General`).
+2. Fetch and checkout the `refs/meta/config branch`, e.g. by `git fetch origin
+ refs/meta/config && git checkout FETCH_HEAD`.
+3. Edit the `project.config` file or other configuration files and commit the
+ changes, e.g. by `git commit --all`. Note, since the `project.config` file
+ uses the format of a git config file you can also edit it via the
+ `git config` command (e.g. to set a project description do: `git config -f
+ project.config project.description "My project description"`).
+4. Push the newly created commit, either to update the `refs/meta/config`
+ branch directly without code-review (e.g. `git push origin
+ HEAD:refs/meta/config`), or for review (e.g. `git push origin
+ HEAD:refs/for/refs/meta/config`).
+--
++
+[NOTE]
+Updates to access right may require changes in the link:#file-groups[groups]
+file. In contrast to making access rights changes through the `Access` screen
+the `groups` file is not automatically updated.
+
+[[update-through-rest-api]]
+* Through the Gerrit REST API:
++
+Gerrit offers several REST endpoints to modify the project configuration:
++
+** link:rest-api-projects.html#set-config[Set Config] REST endpoint:
+ Allows to edit the project description and change repository options.
+** link:rest-api-projects.html#set-access[Add, Update and Delete Access Rights
+ for Project] REST endpoint:
+ Allows to edit the access rights of the project.
+** link:rest-api-projects.html#create-access-change[Create Access Rights Change
+ for review] REST endpoint:
+ Allows to create a change with access right modifications that can be
+ reviewed and submitted by a project owner.
+
+[[required-permissions]]
+== Required permissions
+
+Depending on how the project configuration is changed different access rights
+are required:
+
+* Direct updates through the web UI (link:#update-through-general-screen[
+ updates through the `General` screen] and link:#update-through-access-screen[
+ direct updates of access rights through the `Access` screen via the `SAVE`
+ button]) and direct updates through the REST API (via the
+ link:rest-api-projects.html#set-config[Set Config] REST endpoint or the
+ link:rest-api-projects.html#set-access[Add, Update and Delete Access Rights
+ for Project] REST endpoint) require the user to be a project owner (have the
+ link:access-control.html#category_owner[Owner] access right assigned on
+ `refs/*` or have the link:access-control.html#capability_administrateServer[
+ Administrate Server] global capability assigned on the `All-Projects` root
+ project).
+* link:#update-by-git-push[Direct updates through `git push`] require the
+ user to have the link:access-control.html#category_push[Push] access right
+ assigned on `refs/meta/config` and be a project owner (have the
+ link:access-control.html#category_owner[Owner] access right assigned on
+ `refs/*` or have the link:access-control.html#capability_administrateServer[
+ Administrate Server] global capability assigned on the `All-Projects` root
+ project).
+* Creating changes for updates through the web UI
+ (link:#update-through-access-screen[proposing updates of access rights
+ through the `Access` screen via the `SAVE FOR REVIEW` button] and
+ link:#update-via-online-editing[proposing updates via the `EDIT REPO CONFIG`
+ command]), creating changes for updates through the REST API (via the
+ link:rest-api-projects.html#create-access-change[Create Access Rights Change
+ for review] REST endpoint) and link:#update-by-git-push[pushing changes for
+ review] require the user to be able to see the `refs/meta/config` branch
+ (have the link:access-control.html#category_read[Read] access right assigned
+ on `refs/meta/config`) and be allowed to create changes for it (have the
+ link:access-control.html#category_push[Push] access right assigned on
+ `refs/for/refs/meta/config` or be a project owner or be an administrator).
+
+[[comments]]
+=== Comments in project configuration files
+
+In principle it's possible to have comments in the project configuration files
+(lines starting with '#'), however if any Gerrit API is used that lead to
+modifications in the configuration files the comments may be dropped. This is
+because when Gerrit parses the configuration files and writes them back with
+updates, comments are not preserved.
+
+[TIP]
+When updating the project configuration use the commit message to record the
+reason for the settings so that this information is preserved in the git
+history. For example, this allows using `git blame` to inspect why
+configuration values have been set.
+
+[[inheritance]]
+== Inheritance
+
+Projects in Gerrit are organized hierarchically in a tree with the
+`All-Projects` project as the root project. The parent project is defined in
+the link:#access.inheritFrom[access section] of the `project.config` file.
+
+Projects inherit access rights and options from their parent project, but not
+all options are inheritable. See the description of the options in the
+link:#file-project_config[project.config] file to learn whether they are
+inherited or not.
+
+Options with boolean values support a special `INHERIT` value to make them
+inherit the value that is set in the parent project.
+
+Some settings can be enforced for child projects (or if set on the
+`All-Projects` root project for all projects), e.g. access right restrictions
+via link:access-control#block[BLOCK rules] or
+link:config-submit-requirements.html#submit_requirement_can_override_in_child_projects[
+non-overridable] submit requirements.
+
+[NOTE]
+Project owners can be allowed to change the parent of projects that they own
+(see config-gerrit.html#receive.allowProjectOwnersToChangeParent[
+receive.allowProjectOwnersToChangeParent] setting which is `false` by default).
+In this case project owners may escape the settings that are enforced by their
+parent project by choosing a different parent project.
[[file-project_config]]
-== The file +project.config+
+== The file `project.config`
The +project.config+ file contains the link between groups and their
permitted actions on reference patterns in this project and any projects
@@ -647,13 +852,58 @@
[[access-subsection]]
==== Access subsection
-+access+ subsections for references connect access rights to groups. Each group
-listed must exist in the link:#file-groups[+groups+ file].
+`access` subsections define access rules for a ref or a ref namespace. The ref
+or ref namespace is specified as the subsection name and can be a concrete ref
+(e.g. `refs/heads/master`), a ref pattern (last path segment is '\*', e.g.
+`refs/heads/*`) or a regular expression (must start with '^', e.g.
+`^refs/heads/rel-.*`).
-Please refer to the
-link:access-control.html#access_categories[Access Categories]
-documentation for a full list of available access rights.
+[NOTE]
+For ref patterns '\*' can only appear as the last path segment. If a '*' is
+required in any other place the ref namespace must be specified as a regular
+expression (must start with '^', '\*' must follow what's being matched, e.g.
+".*" to match any string).
+The `access` subsections contain access rules that apply to the ref or ref
+namespace of the `access` subsections. The format of the access rules is: +
+`<accessCategoryId> = (block|deny)? <range>? group <group-name>`
+
+* `<accessCategoryId>`: ID of the link:access-control.html#access_categories[
+ access category] for which the access rule should be defined. The ID of the
+ access category is the name of the access category in lowerCamelCase (e.g.
+ `createTag`), except for label permissions where it is `label-<label-Name>`
+ (e.g. `label-Code-Review`).
+* `(block|deny)?`: `block` defines a link:access-control.html#block-rule[BLOCK]
+ rule, `deny` defines a link:access-control.html#deny-rule[DENY] rule, if
+ neither `block` or `deny` is specified an link:access-control.html#allow-rule[
+ ALLOW] rule is defined.
+* `<range>?`: Only set for label permission. The voting range in the format
+ `<min-vote>..<max-vote>` (e.g. `-1..+1`).
+* `group <group-name>`: The (local) name of the group to which the access rule
+ should apply (e.g. `group Foo Bar`). The (local) group name must exist in the
+ link:#file-groups[groups] file, so that Gerrit can resolve it to the group
+ UUID.
+
+To make access rules link:access-control.html#exclusive[exclusive] they need to
+be included into the value of the `exclusiveGroupPermissions` key: +
+`exclusiveGroupPermissions = <space-separated-list-of-access-category-ids>`
+
+.Example access subsections
+----
+ [access "refs/heads/*"]
+ create = group Administrators
+ delete = group Administrators
+ deleteChanges = group Administrators
+ label-Code-Review = -2..+2 group Maintainers
+ label-Code-Review = -1..+1 group Registered Users
+ label-Verified = -1..+1 group Registered Users
+ push = block Registered Users
+ submit = group Maintainers
+ exclusiveGroupPermissions = deleteChanges submit
+ [access "^refs/tags/rel-.*"]
+ createTag = group Maintainers
+ createSignedTag = group Maintainers
+----
[[mimetype-section]]
=== MIME Types section
@@ -753,13 +1003,25 @@
default value is `FALSE`.
[[file-groups]]
-== The file +groups+
+== The file `groups`
-Each group in this list is linked with its UUID so that renaming of
-groups is possible without having to rewrite every +groups+ file
-in every repository where it's used.
+The `groups` file resolves group names that are mentioned in
+link:#access-subsection[access subsections] of the link:#file-project_config[
+project.config] file to group UUIDs.
-This is what the default groups file for +All-Projects.git+ looks like:
+In the access subsections access rights are assigned to group names. These
+group names are local to the project configuration and do not need to match
+with the actual group names. To enable Gerrit to resolve the local group names
+they must be mapped to group UUIDs in the `groups` file.
+
+The access sections use local group names, rather than requiring the actual
+group names, to allow renaming groups in Gerrit without having to rewrite every
+`project.config` file using the group.
+
+The content of the `groups` file is a simple table of group UUID to group name,
+separated by a tab.
+
+This is how the default `groups` file for `All-Projects` project looks like:
----
# UUID Group Name
@@ -771,29 +1033,37 @@
global:Registered-Users Registered Users
----
-This file can't be written to by the +git config+ command.
+Since the `groups` file has a custom format it can't be edited using the
+`git config` command.
-In order to reference a group in +project.config+, it must be listed in
-the +groups+ file. When editing permissions through the web UI this
-file is maintained automatically, but when pushing updates to
-+refs/meta/config+ this must be dealt with by hand. Gerrit will refuse
-+project.config+ files that refer to groups not listed in +groups+.
+Whenever access rights in the `project.config` file are assigned to new groups
+mapping entries for the new groups must be added to the `groups` file. The
+modifications to the `groups` file must be included in the same commit that
+updates the `project.config` file. Pushing updates to `project.config` files
+that refer to groups not listed in the `groups` file are rejected by Gerrit.
-The UUID of a group can be found on the General tab of the group's page
-in the web UI or via the +-v+ option to
-link:cmd-ls-groups.html[the +ls-groups+ SSH command].
+When link:#update-through-access-screen[editing access rights through the web
+UI] the `groups` file is automatically updated by Gerrit.
+The UUID of a group can be found on the group screen (`BROWSE` > `Groups` >
+`<group-name>` ). Alternatively the group can be looked up via the
+link:rest-api-groups.html#get-group[Get Group] REST endpoint (note that the
+`group-id` in the URL can be the group name). The group UUID is contained as
+`id` field in the return link:rest-api-groups.html#group-info[GroupInfo] JSON.
[[file-rules_pl]]
-== The file +rules.pl+
+== The file `rules.pl`
-The +rules.pl+ files allows you to replace or amend the default Prolog
-rules that control e.g. what conditions need to be fulfilled for a
-change to be submittable. This file content should be
-interpretable by the 'Prolog Cafe' interpreter.
+The `rules.pl` file allows to replace or amend the default Prolog rules that
+control what conditions need to be fulfilled for a change to be submittable.
+This file should be interpretable by the 'Prolog Cafe' interpreter.
-You can read more about the +rules.pl+ file and the prolog rules on
-link:prolog-cookbook.html[the Prolog cookbook page].
+You can read more about prolog rules on the link:prolog-cookbook.html[Prolog
+cookbook] page.
+
+[NOTE]
+Prolog rules are deprecated and have been replaced by
+link:config-submit-requirements.html[submit requirements].
GERRIT
------
diff --git a/WORKSPACE b/WORKSPACE
index 1b70606..d60916e 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -52,10 +52,10 @@
http_archive(
name = "com_google_protobuf",
- sha256 = "3bd7828aa5af4b13b99c191e8b1e884ebfa9ad371b0ce264605d347f135d2568",
- strip_prefix = "protobuf-3.19.4",
+ sha256 = "75be42bd736f4df6d702a0e4e4d30de9ee40eac024c4b845d17ae4cc831fe4ae",
+ strip_prefix = "protobuf-21.7",
urls = [
- "https://github.com/protocolbuffers/protobuf/archive/v3.19.4.tar.gz",
+ "https://github.com/protocolbuffers/protobuf/archive/v21.7.tar.gz",
],
)
diff --git a/java/com/google/gerrit/entities/Comment.java b/java/com/google/gerrit/entities/Comment.java
index e1e143c..baac18f 100644
--- a/java/com/google/gerrit/entities/Comment.java
+++ b/java/com/google/gerrit/entities/Comment.java
@@ -216,7 +216,7 @@
public int lineNbr;
public Identity author;
- protected Identity realAuthor;
+ public Identity realAuthor;
// TODO(issue-15525): Migrate this field from Timestamp to Instant
public Timestamp writtenOn;
diff --git a/java/com/google/gerrit/entities/converter/HumanCommentProtoConverter.java b/java/com/google/gerrit/entities/converter/HumanCommentProtoConverter.java
new file mode 100644
index 0000000..d14aa97
--- /dev/null
+++ b/java/com/google/gerrit/entities/converter/HumanCommentProtoConverter.java
@@ -0,0 +1,142 @@
+// Copyright (C) 2023 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.entities.converter;
+
+import static com.google.gerrit.entities.Patch.PATCHSET_LEVEL;
+
+import com.google.errorprone.annotations.Immutable;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.Comment.Range;
+import com.google.gerrit.entities.HumanComment;
+import com.google.gerrit.proto.Entities;
+import com.google.gerrit.proto.Entities.HumanComment.InFilePosition;
+import com.google.gerrit.proto.Entities.HumanComment.InFilePosition.Side;
+import com.google.protobuf.Parser;
+import java.time.Instant;
+import java.util.Optional;
+import org.eclipse.jgit.lib.ObjectId;
+
+/** Proto converter between {@link HumanComment} and {@link Entities.HumanComment}. */
+@Immutable
+public enum HumanCommentProtoConverter
+ implements ProtoConverter<Entities.HumanComment, HumanComment> {
+ INSTANCE;
+
+ private final ProtoConverter<Entities.Account_Id, Account.Id> accountIdConverter =
+ AccountIdProtoConverter.INSTANCE;
+ private final ProtoConverter<Entities.ObjectId, ObjectId> objectIdConverter =
+ ObjectIdProtoConverter.INSTANCE;
+
+ @Override
+ public Entities.HumanComment toProto(HumanComment val) {
+
+ Entities.HumanComment.Builder res =
+ Entities.HumanComment.newBuilder()
+ .setPatchsetId(val.key.patchSetId)
+ .setAccountId(accountIdConverter.toProto(val.author.getId()))
+ .setCommentUuid(val.key.uuid)
+ .setCommentText(val.message)
+ .setUnresolved(val.unresolved)
+ .setWrittenOnMillis(val.writtenOn.toInstant().toEpochMilli())
+ .setServerId(val.serverId);
+ if (!val.key.filename.equals(PATCHSET_LEVEL)) {
+ InFilePosition.Builder inFilePos =
+ InFilePosition.newBuilder()
+ .setFilePath(val.key.filename)
+ .setSide(val.side <= 0 ? Side.PARENT : Side.REVISION);
+ if (val.range != null) {
+ inFilePos.setPositionRange(
+ InFilePosition.Range.newBuilder()
+ .setStartLine(val.range.startLine)
+ .setStartChar(val.range.startChar)
+ .setEndLine(val.range.endLine)
+ .setEndChar(val.range.endChar));
+ }
+ if (val.lineNbr != 0) {
+ inFilePos.setLineNumber(val.lineNbr);
+ }
+ res.setInFilePosition(inFilePos);
+ }
+
+ if (val.parentUuid != null) {
+ res.setParentCommentUuid(val.parentUuid);
+ }
+ if (val.tag != null) {
+ res.setTag(val.tag);
+ }
+ if (val.realAuthor != null) {
+ res.setRealAuthor(accountIdConverter.toProto(val.realAuthor.getId()));
+ }
+ if (val.getCommitId() != null) {
+ res.setDestCommitId(objectIdConverter.toProto(val.getCommitId()));
+ }
+
+ return res.build();
+ }
+
+ @Override
+ public HumanComment fromProto(Entities.HumanComment proto) {
+ Optional<InFilePosition> optInFilePosition =
+ proto.hasInFilePosition() ? Optional.of(proto.getInFilePosition()) : Optional.empty();
+ Comment.Key key =
+ new Comment.Key(
+ proto.getCommentUuid(),
+ optInFilePosition.isPresent() ? optInFilePosition.get().getFilePath() : PATCHSET_LEVEL,
+ proto.getPatchsetId());
+ HumanComment res =
+ new HumanComment(
+ key,
+ accountIdConverter.fromProto(proto.getAccountId()),
+ Instant.ofEpochMilli(proto.getWrittenOnMillis()),
+ optInFilePosition.isPresent()
+ ? (short) optInFilePosition.get().getSide().getNumber()
+ : Side.REVISION_VALUE,
+ proto.getCommentText(),
+ proto.getServerId(),
+ proto.getUnresolved());
+
+ res.parentUuid = proto.hasParentCommentUuid() ? proto.getParentCommentUuid() : null;
+ res.tag = proto.hasTag() ? proto.getTag() : null;
+ if (proto.hasRealAuthor()) {
+ res.realAuthor = new Comment.Identity(accountIdConverter.fromProto(proto.getRealAuthor()));
+ }
+ if (proto.hasDestCommitId()) {
+ res.setCommitId(objectIdConverter.fromProto(proto.getDestCommitId()));
+ }
+
+ optInFilePosition.ifPresent(
+ inFilePosition -> {
+ if (inFilePosition.hasPositionRange()) {
+ var range = inFilePosition.getPositionRange();
+ res.range =
+ new Range(
+ range.getStartLine(),
+ range.getStartChar(),
+ range.getEndLine(),
+ range.getEndChar());
+ }
+ if (inFilePosition.hasLineNumber()) {
+ res.lineNbr = inFilePosition.getLineNumber();
+ }
+ });
+ return res;
+ }
+
+ @Override
+ public Parser<Entities.HumanComment> getParser() {
+ return Entities.HumanComment.parser();
+ }
+}
diff --git a/java/com/google/gerrit/server/change/RebaseChangeOp.java b/java/com/google/gerrit/server/change/RebaseChangeOp.java
index 054a6dc9..0187c16 100644
--- a/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -114,6 +114,7 @@
private boolean matchAuthorToCommitterDate = false;
private ImmutableListMultimap<String, String> validationOptions = ImmutableListMultimap.of();
private String mergeStrategy;
+ private boolean verifyNeedsRebase = true;
private CodeReviewCommit rebasedCommit;
private PatchSet.Id rebasedPatchSetId;
@@ -275,6 +276,11 @@
return this;
}
+ public RebaseChangeOp setVerifyNeedsRebase(boolean verifyNeedsRebase) {
+ this.verifyNeedsRebase = verifyNeedsRebase;
+ return this;
+ }
+
@Override
public void updateRepo(RepoContext ctx)
throws InvalidChangeOperationException, RestApiException, IOException, NoSuchChangeException,
@@ -441,7 +447,7 @@
throws ResourceConflictException, IOException {
RevCommit parentCommit = original.getParent(0);
- if (base.equals(parentCommit)) {
+ if (verifyNeedsRebase && base.equals(parentCommit)) {
throw new ResourceConflictException("Change is already up to date.");
}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index d27b13ad..c783e40 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -3024,7 +3024,11 @@
reject(inputCommand, "change " + ontoChange + " closed");
return false;
} else if (revisions.containsKey(newCommit)) {
- reject(inputCommand, "commit already exists (in the change)");
+ reject(
+ inputCommand,
+ String.format(
+ "commit %s already exists in change %s",
+ newCommit.name().substring(0, 10), change.getId()));
return false;
}
diff --git a/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java b/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java
index 87de810..cd17fe7 100644
--- a/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java
+++ b/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java
@@ -17,7 +17,6 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.gerrit.server.submit.CommitMergeStatus.EMPTY_COMMIT;
-import static com.google.gerrit.server.submit.CommitMergeStatus.SKIPPED_IDENTICAL_TREE;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
@@ -41,11 +40,8 @@
import java.io.IOException;
import java.util.Collection;
import java.util.List;
-import java.util.Optional;
-import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevCommit;
/** This strategy covers RebaseAlways and RebaseIfNecessary ones. */
public class RebaseSubmitStrategy extends SubmitStrategy {
@@ -145,90 +141,61 @@
public void updateRepoImpl(RepoContext ctx)
throws InvalidChangeOperationException, RestApiException, IOException,
PermissionBackendException {
- if (args.mergeUtil.canFastForward(
- args.mergeSorter, args.mergeTip.getCurrentTip(), args.rw, toMerge)) {
- if (!rebaseAlways) {
- if (args.project.is(BooleanProjectConfig.REJECT_EMPTY_COMMIT)
- && toMerge.getTree().equals(toMerge.getParent(0).getTree())) {
- toMerge.setStatusCode(EMPTY_COMMIT);
- return;
- }
+ if (!rebaseAlways
+ && args.mergeUtil.canFastForward(
+ args.mergeSorter, args.mergeTip.getCurrentTip(), args.rw, toMerge)) {
+ if (args.project.is(BooleanProjectConfig.REJECT_EMPTY_COMMIT)
+ && toMerge.getTree().equals(toMerge.getParent(0).getTree())) {
+ toMerge.setStatusCode(EMPTY_COMMIT);
+ return;
+ }
- args.mergeTip.moveTipTo(amendGitlink(toMerge), toMerge);
- toMerge.setStatusCode(CommitMergeStatus.CLEAN_MERGE);
- acceptMergeTip(args.mergeTip);
- return;
- }
- // RebaseAlways means we modify commit message.
- args.rw.parseBody(toMerge);
- newPatchSetId =
- ChangeUtil.nextPatchSetIdFromChangeRefs(
- ctx.getRepoView().getRefs(getId().toRefPrefix()).keySet(),
- toMerge.change().currentPatchSetId());
- RevCommit mergeTip = args.mergeTip.getCurrentTip();
- args.rw.parseBody(mergeTip);
- String cherryPickCmtMsg = args.mergeUtil.createCommitMessageOnSubmit(toMerge, mergeTip);
- PersonIdent committer =
- Optional.ofNullable(toMerge.getCommitterIdent())
- .map(ident -> ctx.newCommitterIdent(ident.getEmailAddress(), args.caller))
- .orElseGet(() -> ctx.newCommitterIdent(args.caller));
- try {
- newCommit =
- args.mergeUtil.createCherryPickFromCommit(
- ctx.getInserter(),
- ctx.getRepoView().getConfig(),
- args.mergeTip.getCurrentTip(),
- toMerge,
- committer,
- cherryPickCmtMsg,
- args.rw,
- 0,
- true,
- false);
- } catch (MergeConflictException mce) {
- // Unlike in Cherry-pick case, this should never happen.
- toMerge.setStatusCode(CommitMergeStatus.REBASE_MERGE_CONFLICT);
- throw new IllegalStateException(
- "MergeConflictException on message edit must not happen", mce);
- } catch (MergeIdenticalTreeException mie) {
- // this should not happen
- toMerge.setStatusCode(SKIPPED_IDENTICAL_TREE);
- return;
- }
- ctx.addRefUpdate(ObjectId.zeroId(), newCommit, newPatchSetId.toRefName());
- } else {
- // Stale read of patch set is ok; see comments in RebaseChangeOp.
- PatchSet origPs = args.psUtil.get(toMerge.getNotes(), toMerge.getPatchsetId());
- rebaseOp =
- args.rebaseFactory
- .create(toMerge.notes(), origPs, args.mergeTip.getCurrentTip())
- .setFireRevisionCreated(false)
- // Bypass approval copier since SubmitStrategyOp copy all approvals
- // later anyway.
- .setValidate(false)
- .setCheckAddPatchSetPermission(false)
- // RebaseAlways should set always modify commit message like
- // Cherry-Pick strategy.
- .setDetailedCommitMessage(rebaseAlways)
- // Do not post message after inserting new patchset because there
- // will be one about change being merged already.
- .setPostMessage(false)
- .setSendEmail(false)
- .setMatchAuthorToCommitterDate(
- args.project.is(BooleanProjectConfig.MATCH_AUTHOR_TO_COMMITTER_DATE))
- // The votes are automatically copied and they don't count as copied votes. See
- // method's javadoc.
- .setStoreCopiedVotes(/* storeCopiedVotes = */ false);
- try {
- rebaseOp.updateRepo(ctx);
- } catch (MergeConflictException | NoSuchChangeException e) {
- toMerge.setStatusCode(CommitMergeStatus.REBASE_MERGE_CONFLICT);
- throw new IntegrationConflictException(
- "Cannot rebase " + toMerge.name() + ": " + e.getMessage(), e);
- }
- newCommit = args.rw.parseCommit(rebaseOp.getRebasedCommit());
- newPatchSetId = rebaseOp.getPatchSetId();
+ args.mergeTip.moveTipTo(amendGitlink(toMerge), toMerge);
+ toMerge.setStatusCode(CommitMergeStatus.CLEAN_MERGE);
+ acceptMergeTip(args.mergeTip);
+ return;
}
+
+ args.rw.parseBody(toMerge);
+ newPatchSetId =
+ ChangeUtil.nextPatchSetIdFromChangeRefs(
+ ctx.getRepoView().getRefs(getId().toRefPrefix()).keySet(),
+ toMerge.change().currentPatchSetId());
+ // Stale read of patch set is ok; see comments in RebaseChangeOp.
+ PatchSet origPs = args.psUtil.get(toMerge.getNotes(), toMerge.getPatchsetId());
+ rebaseOp =
+ args.rebaseFactory
+ .create(toMerge.notes(), origPs, args.mergeTip.getCurrentTip())
+ .setFireRevisionCreated(false)
+ // Bypass approval copier since SubmitStrategyOp copy all approvals
+ // later anyway.
+ .setValidate(false)
+ .setCheckAddPatchSetPermission(false)
+ // RebaseAlways should set always modify commit message like
+ // Cherry-Pick strategy.
+ .setDetailedCommitMessage(rebaseAlways)
+ // Do not post message after inserting new patchset because there
+ // will be one about change being merged already.
+ .setPostMessage(false)
+ .setSendEmail(false)
+ .setMatchAuthorToCommitterDate(
+ args.project.is(BooleanProjectConfig.MATCH_AUTHOR_TO_COMMITTER_DATE))
+ // The votes are automatically copied and they don't count as copied votes. See
+ // method's javadoc.
+ .setStoreCopiedVotes(/* storeCopiedVotes = */ false)
+ .setVerifyNeedsRebase(/* verifyNeedsRebase= */ !rebaseAlways);
+
+ try {
+ rebaseOp.updateRepo(ctx);
+ } catch (MergeConflictException | NoSuchChangeException e) {
+ toMerge.setStatusCode(CommitMergeStatus.REBASE_MERGE_CONFLICT);
+ throw new IntegrationConflictException(
+ "Cannot rebase " + toMerge.name() + ": " + e.getMessage(), e);
+ }
+
+ newCommit = args.rw.parseCommit(rebaseOp.getRebasedCommit());
+ newPatchSetId = rebaseOp.getPatchSetId();
+
if (args.project.is(BooleanProjectConfig.REJECT_EMPTY_COMMIT)
&& newCommit.getTree().equals(newCommit.getParent(0).getTree())) {
toMerge.setStatusCode(EMPTY_COMMIT);
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 7628978..c37be7e 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -237,6 +237,8 @@
@Inject protected ProjectOperations projectOperations;
@Inject protected Emails emails;
+ @Inject protected ExtensionRegistry extensionRegistry;
+ @Inject protected RequestScopeOperations requestScopeOperations;
@Inject protected GroupOperations groupOperations;
@@ -248,12 +250,10 @@
@Inject private Provider<InternalAccountQuery> accountQueryProvider;
@Inject private Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
@Inject private Provider<PublicKeyStore> publicKeyStoreProvider;
- @Inject private RequestScopeOperations requestScopeOperations;
@Inject private RetryHelper.Metrics retryMetrics;
@Inject private Sequences seq;
@Inject private StalenessChecker stalenessChecker;
@Inject private VersionedAuthorizedKeys.Accessor authorizedKeys;
- @Inject private ExtensionRegistry extensionRegistry;
@Inject private PluginSetContext<ExceptionHook> exceptionHooks;
@Inject private ExternalIdKeyFactory externalIdKeyFactory;
@Inject private ExternalIdFactoryNoteDbImpl externalIdFactory;
@@ -549,7 +549,7 @@
AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
try (Registration registration =
extensionRegistry.newRegistration().add(accountIndexedCounter)) {
- AccountInfo info = gApi.accounts().id("admin").get();
+ AccountInfo info = gApi.accounts().id(admin.id().get()).get();
assertThat(info.name).isEqualTo("Administrator");
assertThat(info.email).isEqualTo("admin@example.com");
assertThat(info.username).isEqualTo("admin");
@@ -562,7 +562,7 @@
AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
try (Registration registration =
extensionRegistry.newRegistration().add(accountIndexedCounter)) {
- AccountInfo info = gApi.accounts().id("admin").get();
+ AccountInfo info = gApi.accounts().id(admin.id().get()).get();
AccountInfo infoByIntId = gApi.accounts().id(info._accountId).get();
assertThat(info.name).isEqualTo(infoByIntId.name);
accountIndexedCounter.assertNoReindex();
@@ -577,7 +577,7 @@
AccountInfo info = gApi.accounts().self().get();
assertUser(info, admin);
- info = gApi.accounts().id("self").get();
+ info = gApi.accounts().self().get();
assertUser(info, admin);
accountIndexedCounter.assertNoReindex();
}
@@ -588,9 +588,9 @@
AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
try (Registration registration =
extensionRegistry.newRegistration().add(accountIndexedCounter)) {
- int id = gApi.accounts().id(user.username()).get()._accountId;
- assertThat(gApi.accounts().id(user.username()).getActive()).isTrue();
- gApi.accounts().id(user.username()).setActive(false);
+ int id = gApi.accounts().id(user.id().get()).get()._accountId;
+ assertThat(gApi.accounts().id(user.id().get()).getActive()).isTrue();
+ gApi.accounts().id(user.id().get()).setActive(false);
accountIndexedCounter.assertReindexOf(user);
// Inactive users may only be resolved by ID.
@@ -608,7 +608,7 @@
assertThat(gApi.accounts().id(id).getActive()).isFalse();
gApi.accounts().id(id).setActive(true);
- assertThat(gApi.accounts().id(user.username()).getActive()).isTrue();
+ assertThat(gApi.accounts().id(user.id().get()).getActive()).isTrue();
accountIndexedCounter.assertReindexOf(user);
}
}
@@ -778,9 +778,9 @@
@Test
public void deactivateNotActive() throws Exception {
- int id = gApi.accounts().id(user.username()).get()._accountId;
- assertThat(gApi.accounts().id(user.username()).getActive()).isTrue();
- gApi.accounts().id(user.username()).setActive(false);
+ int id = gApi.accounts().id(user.id().get()).get()._accountId;
+ assertThat(gApi.accounts().id(user.id().get()).getActive()).isTrue();
+ gApi.accounts().id(user.id().get()).setActive(false);
assertThat(gApi.accounts().id(id).getActive()).isFalse();
ResourceConflictException thrown =
assertThrows(
@@ -1078,7 +1078,7 @@
TestAccount account = accountCreator.create(name("user"));
EmailInput input = newEmailInput("test@example.com");
requestScopeOperations.setApiUser(user.id());
- assertThrows(AuthException.class, () -> gApi.accounts().id(account.username()).addEmail(input));
+ assertThrows(AuthException.class, () -> gApi.accounts().id(account.id().get()).addEmail(input));
}
@Test
@@ -1089,7 +1089,7 @@
ResourceConflictException thrown =
assertThrows(
ResourceConflictException.class,
- () -> gApi.accounts().id(user.username()).addEmail(input));
+ () -> gApi.accounts().id(user.id().get()).addEmail(input));
assertThat(thrown)
.hasMessageThat()
.contains("Identity 'mailto:" + email + "' in use by another account");
@@ -1478,8 +1478,8 @@
@Test
public void adminCanSetNameOfOtherUser() throws Exception {
- gApi.accounts().id(user.username()).setName("User McUserface");
- assertThat(gApi.accounts().id(user.username()).get().name).isEqualTo("User McUserface");
+ gApi.accounts().id(user.id().get()).setName("User McUserface");
+ assertThat(gApi.accounts().id(user.id().get()).get().name).isEqualTo("User McUserface");
}
@Test
@@ -1487,7 +1487,7 @@
requestScopeOperations.setApiUser(user.id());
assertThrows(
AuthException.class,
- () -> gApi.accounts().id(admin.username()).setName("Admin McAdminface"));
+ () -> gApi.accounts().id(admin.id().get()).setName("Admin McAdminface"));
}
@Test
@@ -1497,8 +1497,8 @@
.allProjectsForUpdate()
.add(allowCapability(GlobalCapability.MODIFY_ACCOUNT).group(REGISTERED_USERS))
.update();
- gApi.accounts().id(admin.username()).setName("Admin McAdminface");
- assertThat(gApi.accounts().id(admin.username()).get().name).isEqualTo("Admin McAdminface");
+ gApi.accounts().id(admin.id().get()).setName("Admin McAdminface");
+ assertThat(gApi.accounts().id(admin.id().get()).get().name).isEqualTo("Admin McAdminface");
}
@Test
@@ -1932,8 +1932,8 @@
// Add a new key
sender.clear();
String newKey = TestSshKeys.publicKey(SshSessionFactory.genSshKey(), user.email());
- gApi.accounts().id(user.username()).addSshKey(newKey);
- info = gApi.accounts().id(user.username()).listSshKeys();
+ gApi.accounts().id(user.id().get()).addSshKey(newKey);
+ info = gApi.accounts().id(user.id().get()).listSshKeys();
assertThat(info).hasSize(2);
assertSequenceNumbers(info);
accountIndexedCounter.assertReindexOf(user);
@@ -1945,8 +1945,8 @@
// Delete key
sender.clear();
- gApi.accounts().id(user.username()).deleteSshKey(1);
- info = gApi.accounts().id(user.username()).listSshKeys();
+ gApi.accounts().id(user.id().get()).deleteSshKey(1);
+ info = gApi.accounts().id(user.id().get()).listSshKeys();
assertThat(info).hasSize(1);
accountIndexedCounter.assertReindexOf(user);
@@ -1962,7 +1962,7 @@
public void userCannotAddSshKeyToOtherAccount() throws Exception {
String newKey = TestSshKeys.publicKey(SshSessionFactory.genSshKey(), admin.email());
requestScopeOperations.setApiUser(user.id());
- assertThrows(AuthException.class, () -> gApi.accounts().id(admin.username()).addSshKey(newKey));
+ assertThrows(AuthException.class, () -> gApi.accounts().id(admin.id().get()).addSshKey(newKey));
}
@Test
@@ -1971,7 +1971,7 @@
requestScopeOperations.setApiUser(user.id());
assertThrows(
ResourceNotFoundException.class,
- () -> gApi.accounts().id(admin.username()).deleteSshKey(0));
+ () -> gApi.accounts().id(admin.id().get()).deleteSshKey(0));
}
// reindex is tested by {@link AbstractQueryAccountsTest#reindex}
@@ -1982,7 +1982,7 @@
extensionRegistry.newRegistration().add(accountIndexedCounter)) {
// admin can reindex any account
requestScopeOperations.setApiUser(admin.id());
- gApi.accounts().id(user.username()).index();
+ gApi.accounts().id(user.id().get()).index();
accountIndexedCounter.assertReindexOf(user);
// user can reindex own account
@@ -1992,7 +1992,7 @@
// user cannot reindex any account
AuthException thrown =
- assertThrows(AuthException.class, () -> gApi.accounts().id(admin.username()).index());
+ assertThrows(AuthException.class, () -> gApi.accounts().id(admin.id().get()).index());
assertThat(thrown).hasMessageThat().contains("modify account not permitted");
}
}
@@ -2045,10 +2045,10 @@
assertThat(accountQueryProvider.get().byDefault(name, true)).isEmpty();
TestAccount foo1 = accountCreator.create(name + "-1");
- assertThat(gApi.accounts().id(foo1.username()).getActive()).isTrue();
+ assertThat(gApi.accounts().id(foo1.id().get()).getActive()).isTrue();
TestAccount foo2 = accountCreator.create(name + "-2");
- gApi.accounts().id(foo2.username()).setActive(false);
+ gApi.accounts().id(foo2.id().get()).setActive(false);
assertThat(gApi.accounts().id(foo2.id().get()).getActive()).isFalse();
assertThat(accountQueryProvider.get().byDefault(name, true)).hasSize(2);
@@ -2081,7 +2081,7 @@
return input;
}
- private EmailInput newEmailInput(String email) {
+ protected EmailInput newEmailInput(String email) {
return newEmailInput(email, true);
}
@@ -2097,7 +2097,7 @@
@Test
public void allGroupsForAnAdminAccountCanBeRetrieved() throws Exception {
- List<GroupInfo> groups = gApi.accounts().id(admin.username()).getGroups();
+ List<GroupInfo> groups = gApi.accounts().id(admin.id().get()).getGroups();
assertThat(groups)
.comparingElementsUsing(getGroupToNameCorrespondence())
.containsAtLeast("Anonymous Users", "Registered Users", "Administrators");
@@ -2135,13 +2135,13 @@
@Test
public void allGroupsForAUserAccountCanBeRetrieved() throws Exception {
String username = name("user1");
- accountOperations.newAccount().username(username).create();
+ Account.Id id = accountOperations.newAccount().username(username).create();
AccountGroup.UUID groupID = groupOperations.newGroup().name("group").create();
String group = groupOperations.group(groupID).get().name();
gApi.groups().id(group).addMembers(username);
- List<GroupInfo> allGroups = gApi.accounts().id(username).getGroups();
+ List<GroupInfo> allGroups = gApi.accounts().id(id.get()).getGroups();
assertThat(allGroups)
.comparingElementsUsing(getGroupToNameCorrespondence())
.containsExactly("Anonymous Users", "Registered Users", group);
@@ -2629,7 +2629,7 @@
public void adminCanGenerateNewHttpPasswordForUser() throws Exception {
requestScopeOperations.setApiUser(admin.id());
sender.clear();
- String newPassword = gApi.accounts().id(user.username()).generateHttpPassword();
+ String newPassword = gApi.accounts().id(user.id().get()).generateHttpPassword();
assertThat(newPassword).isNotNull();
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).body()).contains("HTTP password was added or updated");
@@ -2639,7 +2639,7 @@
public void userCannotGenerateNewHttpPasswordForOtherUser() throws Exception {
requestScopeOperations.setApiUser(user.id());
assertThrows(
- AuthException.class, () -> gApi.accounts().id(admin.username()).generateHttpPassword());
+ AuthException.class, () -> gApi.accounts().id(admin.id().get()).generateHttpPassword());
}
@Test
@@ -2654,7 +2654,7 @@
requestScopeOperations.setApiUser(user.id());
assertThrows(
AuthException.class,
- () -> gApi.accounts().id(admin.username()).setHttpPassword("my-new-password"));
+ () -> gApi.accounts().id(admin.id().get()).setHttpPassword("my-new-password"));
}
@Test
@@ -2670,7 +2670,7 @@
public void userCannotRemoveHttpPasswordForOtherUser() throws Exception {
requestScopeOperations.setApiUser(user.id());
assertThrows(
- AuthException.class, () -> gApi.accounts().id(admin.username()).setHttpPassword(null));
+ AuthException.class, () -> gApi.accounts().id(admin.id().get()).setHttpPassword(null));
}
@Test
@@ -2678,7 +2678,7 @@
requestScopeOperations.setApiUser(admin.id());
String httpPassword = "new-password-for-user";
sender.clear();
- assertThat(gApi.accounts().id(user.username()).setHttpPassword(httpPassword))
+ assertThat(gApi.accounts().id(user.id().get()).setHttpPassword(httpPassword))
.isEqualTo(httpPassword);
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).body()).contains("HTTP password was added or updated");
@@ -2688,7 +2688,7 @@
public void adminCanRemoveHttpPasswordForUser() throws Exception {
requestScopeOperations.setApiUser(admin.id());
sender.clear();
- assertThat(gApi.accounts().id(user.username()).setHttpPassword(null)).isNull();
+ assertThat(gApi.accounts().id(user.id().get()).setHttpPassword(null)).isNull();
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).body()).contains("HTTP password was deleted");
}
@@ -3590,9 +3590,9 @@
extensionRegistry.newRegistration().add(accountIndexedCounter)) {
Map<String, GpgKeyInfo> gpgKeys =
gApi.accounts()
- .id(account.username())
+ .id(account.id().get())
.putGpgKeys(ImmutableList.of(armored), ImmutableList.<String>of());
- accountIndexedCounter.assertReindexOf(gApi.accounts().id(account.username()).get());
+ accountIndexedCounter.assertReindexOf(gApi.accounts().id(account.id().get()).get());
return gpgKeys;
}
});
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
index 9b77b01..b55a535 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
@@ -202,7 +202,7 @@
AuthException thrown =
assertThrows(
AuthException.class,
- () -> gApi.accounts().id("admin").signAgreement(caAutoVerify.getName()));
+ () -> gApi.accounts().id(admin.id().get()).signAgreement(caAutoVerify.getName()));
assertThat(thrown).hasMessageThat().contains("not allowed to enter contributor agreement");
}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/DiffPreferencesIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/DiffPreferencesIT.java
index a442ddd..e3380c0 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/DiffPreferencesIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/DiffPreferencesIT.java
@@ -28,7 +28,7 @@
@Test
public void getDiffPreferences() throws Exception {
DiffPreferencesInfo d = DiffPreferencesInfo.defaults();
- DiffPreferencesInfo o = gApi.accounts().id(admin.id().toString()).getDiffPreferences();
+ DiffPreferencesInfo o = gApi.accounts().id(admin.id().get()).getDiffPreferences();
assertPrefs(o, d);
}
@@ -62,13 +62,13 @@
i.matchBrackets ^= true;
i.lineWrapping ^= true;
- DiffPreferencesInfo o = gApi.accounts().id(admin.id().toString()).setDiffPreferences(i);
+ DiffPreferencesInfo o = gApi.accounts().id(admin.id().get()).setDiffPreferences(i);
assertPrefs(o, i);
// Partially fill input record
i = new DiffPreferencesInfo();
i.tabSize = 42;
- DiffPreferencesInfo a = gApi.accounts().id(admin.id().toString()).setDiffPreferences(i);
+ DiffPreferencesInfo a = gApi.accounts().id(admin.id().get()).setDiffPreferences(i);
assertPrefs(a, o, "tabSize");
assertThat(a.tabSize).isEqualTo(42);
}
@@ -85,7 +85,7 @@
update.fontSize = newFontSize;
gApi.config().server().setDefaultDiffPreferences(update);
- DiffPreferencesInfo o = gApi.accounts().id(admin.id().toString()).getDiffPreferences();
+ DiffPreferencesInfo o = gApi.accounts().id(admin.id().get()).getDiffPreferences();
// assert configured defaults
assertThat(o.lineLength).isEqualTo(newLineLength);
@@ -104,29 +104,29 @@
update.lineLength = configuredDefaultLineLength;
gApi.config().server().setDefaultDiffPreferences(update);
- DiffPreferencesInfo o = gApi.accounts().id(admin.id().toString()).getDiffPreferences();
+ DiffPreferencesInfo o = gApi.accounts().id(admin.id().get()).getDiffPreferences();
assertThat(o.lineLength).isEqualTo(configuredDefaultLineLength);
assertPrefs(o, d, "lineLength");
int newLineLength = configuredDefaultLineLength + 10;
DiffPreferencesInfo i = new DiffPreferencesInfo();
i.lineLength = newLineLength;
- DiffPreferencesInfo a = gApi.accounts().id(admin.id().toString()).setDiffPreferences(i);
+ DiffPreferencesInfo a = gApi.accounts().id(admin.id().get()).setDiffPreferences(i);
assertThat(a.lineLength).isEqualTo(newLineLength);
assertPrefs(a, d, "lineLength");
- a = gApi.accounts().id(admin.id().toString()).getDiffPreferences();
+ a = gApi.accounts().id(admin.id().get()).getDiffPreferences();
assertThat(a.lineLength).isEqualTo(newLineLength);
assertPrefs(a, d, "lineLength");
// overwrite the configured default with original hard-coded default
i = new DiffPreferencesInfo();
i.lineLength = d.lineLength;
- a = gApi.accounts().id(admin.id().toString()).setDiffPreferences(i);
+ a = gApi.accounts().id(admin.id().get()).setDiffPreferences(i);
assertThat(a.lineLength).isEqualTo(d.lineLength);
assertPrefs(a, d, "lineLength");
- a = gApi.accounts().id(admin.id().toString()).getDiffPreferences();
+ a = gApi.accounts().id(admin.id().get()).getDiffPreferences();
assertThat(a.lineLength).isEqualTo(d.lineLength);
assertPrefs(a, d, "lineLength");
}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/EditPreferencesIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/EditPreferencesIT.java
index f142c08..6802333 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/EditPreferencesIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/EditPreferencesIT.java
@@ -25,7 +25,7 @@
public class EditPreferencesIT extends AbstractDaemonTest {
@Test
public void getSetEditPreferences() throws Exception {
- EditPreferencesInfo out = gApi.accounts().id(admin.id().toString()).getEditPreferences();
+ EditPreferencesInfo out = gApi.accounts().id(admin.id().get()).getEditPreferences();
assertThat(out.lineLength).isEqualTo(100);
assertThat(out.indentUnit).isEqualTo(2);
@@ -58,7 +58,7 @@
out.autoCloseBrackets = true;
out.showBase = true;
- EditPreferencesInfo info = gApi.accounts().id(admin.id().toString()).setEditPreferences(out);
+ EditPreferencesInfo info = gApi.accounts().id(admin.id().get()).setEditPreferences(out);
assertEditPreferences(info, out);
@@ -66,7 +66,7 @@
EditPreferencesInfo in = new EditPreferencesInfo();
in.tabSize = 42;
- info = gApi.accounts().id(admin.id().toString()).setEditPreferences(in);
+ info = gApi.accounts().id(admin.id().get()).setEditPreferences(in);
out.tabSize = in.tabSize;
assertEditPreferences(info, out);
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
index eed9de8..4a28303 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
@@ -53,7 +53,7 @@
@Test
public void getAndSetPreferences() throws Exception {
- GeneralPreferencesInfo o = gApi.accounts().id(user42.id().toString()).getPreferences();
+ GeneralPreferencesInfo o = gApi.accounts().id(user42.id().get()).getPreferences();
assertPrefs(o, GeneralPreferencesInfo.defaults(), "my", "changeTable");
assertThat(o.my)
.containsExactly(
@@ -91,7 +91,7 @@
i.changeTable = new ArrayList<>();
i.changeTable.add("Status");
- o = gApi.accounts().id(user42.id().toString()).setPreferences(i);
+ o = gApi.accounts().id(user42.id().get()).setPreferences(i);
assertPrefs(o, i, "my");
assertThat(o.my).containsExactlyElementsIn(i.my);
assertThat(o.changeTable).containsExactlyElementsIn(i.changeTable);
@@ -109,7 +109,7 @@
update.changesPerPage = newChangesPerPage;
gApi.config().server().setDefaultPreferences(update);
- GeneralPreferencesInfo o = gApi.accounts().id(user42.id().toString()).getPreferences();
+ GeneralPreferencesInfo o = gApi.accounts().id(user42.id().get()).getPreferences();
// assert configured defaults
assertThat(o.changesPerPage).isEqualTo(newChangesPerPage);
@@ -126,29 +126,29 @@
update.changesPerPage = configuredChangesPerPage;
gApi.config().server().setDefaultPreferences(update);
- GeneralPreferencesInfo o = gApi.accounts().id(admin.id().toString()).getPreferences();
+ GeneralPreferencesInfo o = gApi.accounts().id(admin.id().get()).getPreferences();
assertThat(o.changesPerPage).isEqualTo(configuredChangesPerPage);
assertPrefs(o, d, "my", "changeTable", "changesPerPage");
int newChangesPerPage = configuredChangesPerPage * 2;
GeneralPreferencesInfo i = new GeneralPreferencesInfo();
i.changesPerPage = newChangesPerPage;
- GeneralPreferencesInfo a = gApi.accounts().id(admin.id().toString()).setPreferences(i);
+ GeneralPreferencesInfo a = gApi.accounts().id(admin.id().get()).setPreferences(i);
assertThat(a.changesPerPage).isEqualTo(newChangesPerPage);
assertPrefs(a, d, "my", "changeTable", "changesPerPage");
- a = gApi.accounts().id(admin.id().toString()).getPreferences();
+ a = gApi.accounts().id(admin.id().get()).getPreferences();
assertThat(a.changesPerPage).isEqualTo(newChangesPerPage);
assertPrefs(a, d, "my", "changeTable", "changesPerPage");
// overwrite the configured default with original hard-coded default
i = new GeneralPreferencesInfo();
i.changesPerPage = d.changesPerPage;
- a = gApi.accounts().id(admin.id().toString()).setPreferences(i);
+ a = gApi.accounts().id(admin.id().get()).setPreferences(i);
assertThat(a.changesPerPage).isEqualTo(d.changesPerPage);
assertPrefs(a, d, "my", "changeTable", "changesPerPage");
- a = gApi.accounts().id(admin.id().toString()).getPreferences();
+ a = gApi.accounts().id(admin.id().get()).getPreferences();
assertThat(a.changesPerPage).isEqualTo(d.changesPerPage);
assertPrefs(a, d, "my", "changeTable", "changesPerPage");
}
@@ -162,7 +162,7 @@
BadRequestException thrown =
assertThrows(
BadRequestException.class,
- () -> gApi.accounts().id(user42.id().toString()).setPreferences(i));
+ () -> gApi.accounts().id(user42.id().get()).setPreferences(i));
assertThat(thrown).hasMessageThat().contains("name for menu item is required");
}
@@ -175,7 +175,7 @@
BadRequestException thrown =
assertThrows(
BadRequestException.class,
- () -> gApi.accounts().id(user42.id().toString()).setPreferences(i));
+ () -> gApi.accounts().id(user42.id().get()).setPreferences(i));
assertThat(thrown).hasMessageThat().contains("URL for menu item is required");
}
@@ -185,7 +185,7 @@
i.my = new ArrayList<>();
i.my.add(new MenuItem(" name\t", " url\t", " _blank\t", " id\t"));
- GeneralPreferencesInfo o = gApi.accounts().id(user42.id().toString()).setPreferences(i);
+ GeneralPreferencesInfo o = gApi.accounts().id(user42.id().get()).setPreferences(i);
assertThat(o.my).containsExactly(new MenuItem("name", "url", "_blank", "id"));
}
@@ -197,7 +197,7 @@
BadRequestException thrown =
assertThrows(
BadRequestException.class,
- () -> gApi.accounts().id(user42.id().toString()).setPreferences(i));
+ () -> gApi.accounts().id(user42.id().get()).setPreferences(i));
assertThat(thrown)
.hasMessageThat()
.contains("Unsupported download scheme: " + i.downloadScheme);
@@ -211,10 +211,10 @@
GeneralPreferencesInfo i = GeneralPreferencesInfo.defaults();
i.downloadScheme = schemeName;
- GeneralPreferencesInfo o = gApi.accounts().id(user42.id().toString()).setPreferences(i);
+ GeneralPreferencesInfo o = gApi.accounts().id(user42.id().get()).setPreferences(i);
assertThat(o.downloadScheme).isEqualTo(schemeName);
- o = gApi.accounts().id(user42.id().toString()).getPreferences();
+ o = gApi.accounts().id(user42.id().get()).getPreferences();
assertThat(o.downloadScheme).isEqualTo(schemeName);
}
}
@@ -225,7 +225,7 @@
// becomes unsupported.
setDownloadScheme();
- GeneralPreferencesInfo o = gApi.accounts().id(user42.id().toString()).getPreferences();
+ GeneralPreferencesInfo o = gApi.accounts().id(user42.id().get()).getPreferences();
assertThat(o.downloadScheme).isNull();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmitByRebase.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmitByRebase.java
index 7183f9e..9bb1aa4 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmitByRebase.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmitByRebase.java
@@ -169,17 +169,75 @@
}
@Test
- public void submitWithRebaseMergeCommit() throws Throwable {
+ public void submitMergeCommitThatDependsOnNormalChangeViaTheFirstParent() throws Throwable {
/*
- * (HEAD, origin/master, origin/HEAD) Merge changes X,Y
+ * merge created by Gerrit to integrate change 1 and change 2
+ /|
+ / |
+ | |
+ * | change3 (new tip)
+ | |
+ | * change2 (merge)
+ | |\
+ | | |
+ | * | change1
+ \|/
+ * initialHead
+ */
+ RevCommit initialHead = projectOperations.project(project).getHead("master");
+ PushOneCommit.Result change1 = createChange("Added a", "a.txt", "");
+
+ PushOneCommit change2Push =
+ pushFactory.create(admin.newIdent(), testRepo, "Merge to master", "m.txt", "");
+ change2Push.setParents(ImmutableList.of(change1.getCommit(), initialHead));
+ PushOneCommit.Result change2 = change2Push.to("refs/for/master");
+
+ testRepo.reset(initialHead);
+ PushOneCommit.Result change3 = createChange("New tip", "b.txt", "");
+
+ approve(change3.getChangeId());
+ submit(change3.getChangeId());
+
+ approve(change1.getChangeId());
+ approve(change2.getChangeId());
+ submit(change2.getChangeId());
+
+ RevCommit newHead = projectOperations.project(project).getHead("master");
+ assertThat(newHead.getParentCount()).isEqualTo(2);
+
+ RevCommit headParent1 = parse(newHead.getParent(0).getId());
+ RevCommit headParent2 = parse(newHead.getParent(1).getId());
+
+ if (getSubmitType() == SubmitType.REBASE_ALWAYS) {
+ assertCurrentRevision(change3.getChangeId(), 2, headParent1.getId());
+ } else {
+ assertThat(change3.getCommit().getId()).isEqualTo(headParent1.getId());
+ }
+ assertThat(headParent1.getParentCount()).isEqualTo(1);
+ assertThat(headParent1.getParent(0)).isEqualTo(initialHead);
+
+ assertThat(headParent2.getId()).isEqualTo(change2.getCommit().getId());
+ assertThat(headParent2.getParentCount()).isEqualTo(2);
+
+ RevCommit headGrandparent1 = parse(headParent2.getParent(0).getId());
+ RevCommit headGrandparent2 = parse(headParent2.getParent(1).getId());
+
+ assertThat(headGrandparent1.getId()).isEqualTo(change1.getCommit().getId());
+ assertThat(headGrandparent2.getId()).isEqualTo(initialHead.getId());
+ }
+
+ @Test
+ public void submitMergeCommitThatDependsOnNormalChangeViaTheSecondParent() throws Throwable {
+ /*
+ * merge created by Gerrit to integrate change 1 and change 2
|\
- | * Merge branch 'master' into origin/master
+ | * change2 (merge)
| |\
- | | * SHA Added a
+ | | * change1
| |/
- * | Before
+ * | change3 (new tip)
|/
- * Initial empty repository
+ * initialHead
*/
RevCommit initialHead = projectOperations.project(project).getHead("master");
PushOneCommit.Result change1 = createChange("Added a", "a.txt", "");
@@ -190,7 +248,7 @@
PushOneCommit.Result change2 = change2Push.to("refs/for/master");
testRepo.reset(initialHead);
- PushOneCommit.Result change3 = createChange("Before", "b.txt", "");
+ PushOneCommit.Result change3 = createChange("New tip", "b.txt", "");
approve(change3.getChangeId());
submit(change3.getChangeId());
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
index 6dfa82b..10f4942 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
@@ -476,17 +476,17 @@
String name = name("foo");
TestAccount foo1 = accountCreator.create(name + "-1");
reviewChange(changeIdReviewed, foo1);
- assertThat(gApi.accounts().id(foo1.username()).getActive()).isTrue();
+ assertThat(gApi.accounts().id(foo1.id().get()).getActive()).isTrue();
TestAccount foo2 = accountCreator.create(name + "-2");
reviewChange(changeIdReviewed, foo2);
- assertThat(gApi.accounts().id(foo2.username()).getActive()).isTrue();
+ assertThat(gApi.accounts().id(foo2.id().get()).getActive()).isTrue();
assertReviewers(
suggestReviewers(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of());
requestScopeOperations.setApiUser(user.id());
- gApi.accounts().id(foo2.username()).setActive(false);
+ gApi.accounts().id(foo2.id().get()).setActive(false);
assertThat(gApi.accounts().id(foo2.id().get()).getActive()).isFalse();
assertReviewers(suggestReviewers(changeId, name), ImmutableList.of(foo1), ImmutableList.of());
assertReviewers(
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/SuggestBranchReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/project/SuggestBranchReviewersIT.java
index 98f5716..8311d96 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/SuggestBranchReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/SuggestBranchReviewersIT.java
@@ -150,15 +150,15 @@
String name = name("foo");
TestAccount foo1 = accountCreator.create(name + "-1");
- assertThat(gApi.accounts().id(foo1.username()).getActive()).isTrue();
+ assertThat(gApi.accounts().id(foo1.id().get()).getActive()).isTrue();
TestAccount foo2 = accountCreator.create(name + "-2");
- assertThat(gApi.accounts().id(foo2.username()).getActive()).isTrue();
+ assertThat(gApi.accounts().id(foo2.id().get()).getActive()).isTrue();
assertReviewers(suggestReviewers(name), ImmutableList.of(foo1, foo2), ImmutableList.of());
requestScopeOperations.setApiUser(user.id());
- gApi.accounts().id(foo2.username()).setActive(false);
+ gApi.accounts().id(foo2.id().get()).setActive(false);
assertThat(gApi.accounts().id(foo2.id().get()).getActive()).isFalse();
assertReviewers(suggestReviewers(name), ImmutableList.of(foo1), ImmutableList.of());
}
diff --git a/javatests/com/google/gerrit/acceptance/server/account/AccountResolverIT.java b/javatests/com/google/gerrit/acceptance/server/account/AccountResolverIT.java
index b150491..3664cdc 100644
--- a/javatests/com/google/gerrit/acceptance/server/account/AccountResolverIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/account/AccountResolverIT.java
@@ -106,7 +106,7 @@
gApi.accounts().id(user.id().get()).setActive(false);
requestScopeOperations.setApiUser(user.id());
- assertThat(gApi.accounts().id("self").getActive()).isFalse();
+ assertThat(gApi.accounts().self().getActive()).isFalse();
Result result = resolveAsResult("self");
assertThat(result.asIdSet()).containsExactly(user.id());
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/NotificationMailFormatIT.java b/javatests/com/google/gerrit/acceptance/server/mail/NotificationMailFormatIT.java
index 65b1d4f..f17013d 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/NotificationMailFormatIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/NotificationMailFormatIT.java
@@ -39,7 +39,7 @@
// Set user preference to receive only plaintext content
GeneralPreferencesInfo i = new GeneralPreferencesInfo();
i.emailFormat = EmailFormat.PLAINTEXT;
- gApi.accounts().id(admin.id().toString()).setPreferences(i);
+ gApi.accounts().id(admin.id().get()).setPreferences(i);
// Create change as admin and review as user
PushOneCommit.Result r = createChange();
@@ -57,7 +57,7 @@
// Reset user preference
requestScopeOperations.setApiUser(admin.id());
i.emailFormat = EmailFormat.HTML_PLAINTEXT;
- gApi.accounts().id(admin.id().toString()).setPreferences(i);
+ gApi.accounts().id(admin.id().get()).setPreferences(i);
}
@Test
diff --git a/javatests/com/google/gerrit/entities/converter/BUILD b/javatests/com/google/gerrit/entities/converter/BUILD
index 6c4d1e4..0ca9478 100644
--- a/javatests/com/google/gerrit/entities/converter/BUILD
+++ b/javatests/com/google/gerrit/entities/converter/BUILD
@@ -5,6 +5,7 @@
srcs = glob(["*.java"]),
deps = [
"//java/com/google/gerrit/entities",
+ "//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/proto/testing",
"//java/com/google/gerrit/server",
"//lib:guava",
diff --git a/javatests/com/google/gerrit/entities/converter/HumanCommentProtoConverterTest.java b/javatests/com/google/gerrit/entities/converter/HumanCommentProtoConverterTest.java
new file mode 100644
index 0000000..a6aaf36
--- /dev/null
+++ b/javatests/com/google/gerrit/entities/converter/HumanCommentProtoConverterTest.java
@@ -0,0 +1,130 @@
+// Copyright (C) 2023 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.entities.converter;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.entities.Patch.PATCHSET_LEVEL;
+
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.CommentRange;
+import com.google.gerrit.entities.HumanComment;
+import java.time.Instant;
+import org.eclipse.jgit.lib.ObjectId;
+import org.junit.Test;
+
+public class HumanCommentProtoConverterTest {
+ private static final ObjectId VALID_OBJECT_ID =
+ ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef");
+
+ private final HumanCommentProtoConverter converter = HumanCommentProtoConverter.INSTANCE;
+
+ @Test
+ public void fileLevelCommentWithAllOptionalFields() {
+ HumanComment orig =
+ new HumanComment(
+ new Comment.Key("uuid", "a.txt", 42),
+ Account.id(314),
+ Instant.ofEpochMilli(12345),
+ (short) 1,
+ "message",
+ "server",
+ /* unresolved= */ true);
+ orig.tag = "tag";
+ orig.setCommitId(VALID_OBJECT_ID);
+ orig.setRealAuthor(Account.id(271));
+
+ HumanComment res = converter.fromProto(converter.toProto(orig));
+
+ assertThat(res).isEqualTo(orig);
+ }
+
+ @Test
+ public void patchsetLevelComment() {
+ HumanComment orig =
+ new HumanComment(
+ new Comment.Key("uuid", PATCHSET_LEVEL, 42),
+ Account.id(314),
+ Instant.ofEpochMilli(12345),
+ (short) 1,
+ "message",
+ "server",
+ /* unresolved= */ false);
+
+ HumanComment res = converter.fromProto(converter.toProto(orig));
+
+ assertThat(res).isEqualTo(orig);
+ }
+
+ @Test
+ public void lineComment() {
+ HumanComment orig =
+ new HumanComment(
+ new Comment.Key("uuid", "a.txt", 42),
+ Account.id(314),
+ Instant.ofEpochMilli(12345),
+ (short) 1,
+ "message",
+ "server",
+ /* unresolved= */ true);
+ orig.setLineNbrAndRange(7, null);
+
+ HumanComment res = converter.fromProto(converter.toProto(orig));
+
+ assertThat(res).isEqualTo(orig);
+ }
+
+ @Test
+ public void rangeComment() {
+ HumanComment orig =
+ new HumanComment(
+ new Comment.Key("uuid", "a.txt", 42),
+ Account.id(314),
+ Instant.ofEpochMilli(12345),
+ (short) 1,
+ "message",
+ "server",
+ /* unresolved= */ true);
+ orig.setRange(new CommentRange(2, 3, 5, 7));
+
+ HumanComment res = converter.fromProto(converter.toProto(orig));
+
+ assertThat(res).isEqualTo(orig);
+ }
+
+ @Test
+ public void extensionRangeComment() {
+ HumanComment orig =
+ new HumanComment(
+ new Comment.Key("uuid", "a.txt", 42),
+ Account.id(314),
+ Instant.ofEpochMilli(12345),
+ (short) 1,
+ "message",
+ "server",
+ /* unresolved= */ false);
+ com.google.gerrit.extensions.client.Comment.Range range =
+ new com.google.gerrit.extensions.client.Comment.Range();
+ range.startLine = 2;
+ range.startCharacter = 3;
+ range.endLine = 5;
+ range.endCharacter = 7;
+ orig.setLineNbrAndRange(null, range);
+
+ HumanComment res = converter.fromProto(converter.toProto(orig));
+
+ assertThat(res).isEqualTo(orig);
+ }
+}
diff --git a/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java b/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
index d40f2a1..f33dc8d 100644
--- a/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
+++ b/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
@@ -127,11 +127,11 @@
"ssh://%s@" + sshAddress.getHostName() + ":" + sshAddress.getPort() + "/" + project.get();
// Admin user was already created by the base class
- setUpUserAuthentication(admin.username());
+ setUpUserAuthentication(admin.username(), admin.id().get());
// Create non-admin user
TestAccount user = accountCreator.user1();
- setUpUserAuthentication(user.username());
+ setUpUserAuthentication(user.username(), user.id().get());
// Prepare data for new change on master branch
ChangeInput in = new ChangeInput(project.get(), "master", "Test public change");
@@ -271,7 +271,7 @@
ctx.getInjector().injectMembers(this);
// Setup admin password
- gApi.accounts().id(admin.username()).setHttpPassword(ADMIN_PASSWORD);
+ gApi.accounts().id(admin.id().get()).setHttpPassword(ADMIN_PASSWORD);
// Get authenticated Git/HTTP URL
String urlWithCredentials =
@@ -336,7 +336,7 @@
ctx.getInjector().injectMembers(this);
// Setup admin password
- gApi.accounts().id(admin.username()).setHttpPassword(ADMIN_PASSWORD);
+ gApi.accounts().id(admin.id().get()).setHttpPassword(ADMIN_PASSWORD);
// Get authenticated Git/HTTP URL
String urlWithCredentials =
@@ -405,7 +405,7 @@
ctx.getInjector().injectMembers(this);
// Setup admin password
- gApi.accounts().id(admin.username()).setHttpPassword(ADMIN_PASSWORD);
+ gApi.accounts().id(admin.id().get()).setHttpPassword(ADMIN_PASSWORD);
// Get authenticated Git/HTTP URL
String urlWithCredentials =
@@ -488,7 +488,7 @@
ctx.getInjector().injectMembers(this);
// Setup admin password
- gApi.accounts().id(admin.username()).setHttpPassword(ADMIN_PASSWORD);
+ gApi.accounts().id(admin.id().get()).setHttpPassword(ADMIN_PASSWORD);
String url = config.getString("gerrit", null, "canonicalweburl");
@@ -560,9 +560,9 @@
}
}
- private void setUpUserAuthentication(String username) throws Exception {
+ private void setUpUserAuthentication(String username, int accountId) throws Exception {
// Assign HTTP password to user
- gApi.accounts().id(username).setHttpPassword(ADMIN_PASSWORD);
+ gApi.accounts().id(accountId).setHttpPassword(ADMIN_PASSWORD);
// Generate private/public key for user
execute(
@@ -573,7 +573,7 @@
// Read the content of generated public key and add it for the user in Gerrit
gApi.accounts()
- .id(username)
+ .id(accountId)
.addSshKey(
new String(
java.nio.file.Files.readAllBytes(
diff --git a/javatests/com/google/gerrit/integration/git/NoAccessSameAsNotFoundIT.java b/javatests/com/google/gerrit/integration/git/NoAccessSameAsNotFoundIT.java
index ad8a486..78fd35f 100644
--- a/javatests/com/google/gerrit/integration/git/NoAccessSameAsNotFoundIT.java
+++ b/javatests/com/google/gerrit/integration/git/NoAccessSameAsNotFoundIT.java
@@ -65,7 +65,7 @@
ctx.getInjector().injectMembers(this);
TestAccount user = accountCreator.user1();
- gApi.accounts().id(user.username()).setHttpPassword(PASSWORD);
+ gApi.accounts().id(user.id().get()).setHttpPassword(PASSWORD);
String canonical = config.getString("gerrit", null, "canonicalweburl");
repoUrl =
diff --git a/javatests/com/google/gerrit/integration/git/PushToRefsUsersIT.java b/javatests/com/google/gerrit/integration/git/PushToRefsUsersIT.java
index c42f00d..6ccc7cd 100644
--- a/javatests/com/google/gerrit/integration/git/PushToRefsUsersIT.java
+++ b/javatests/com/google/gerrit/integration/git/PushToRefsUsersIT.java
@@ -55,7 +55,7 @@
ctx.getInjector().injectMembers(this);
// Setup admin password
- gApi.accounts().id(admin.username()).setHttpPassword(ADMIN_PASSWORD);
+ gApi.accounts().id(admin.id().get()).setHttpPassword(ADMIN_PASSWORD);
// Get authenticated Git/HTTP URL
String urlWithCredentials =
diff --git a/javatests/com/google/gerrit/integration/git/UploadArchiveIT.java b/javatests/com/google/gerrit/integration/git/UploadArchiveIT.java
index c720905..b2be85f 100644
--- a/javatests/com/google/gerrit/integration/git/UploadArchiveIT.java
+++ b/javatests/com/google/gerrit/integration/git/UploadArchiveIT.java
@@ -154,7 +154,7 @@
.add(String.format("id_rsa_%s", admin.username()))
.build());
gApi.accounts()
- .id(admin.username())
+ .id(admin.id().get())
.addSshKey(
new String(
java.nio.file.Files.readAllBytes(
diff --git a/javatests/com/google/gerrit/integration/ssh/NoShellIT.java b/javatests/com/google/gerrit/integration/ssh/NoShellIT.java
index 2bbbf1a..f25db6e 100644
--- a/javatests/com/google/gerrit/integration/ssh/NoShellIT.java
+++ b/javatests/com/google/gerrit/integration/ssh/NoShellIT.java
@@ -66,7 +66,7 @@
.add(String.format("id_rsa_%s", "admin"))
.build());
gApi.accounts()
- .id("admin")
+ .id(admin.id().get())
.addSshKey(
new String(
java.nio.file.Files.readAllBytes(
diff --git a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
index c530c79..88a3afe 100644
--- a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
+++ b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
@@ -667,8 +667,8 @@
// we load AccountStates from the cache after reading documents from the index
// which means we always read fresh data when matching.
//
- // Reindex document
- gApi.accounts().id(user1.username).index();
+ // Reindex account document
+ gApi.accounts().id(user1._accountId).index();
assertQuery("name:" + quote(user1.name));
assertQuery("name:" + quote(newName), user1);
}
diff --git a/plugins/replication b/plugins/replication
index 8fd3c27..c7c36d6 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 8fd3c271ce0a21480e3d04da5ad2112efea3bedf
+Subproject commit c7c36d62cf61d1588e9cdeab3e89b77a65f44c79
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index 31c2b660..12c4202 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -370,15 +370,6 @@
@query('#secondaryActions') secondaryActions?: HTMLElement;
- // TODO(TS): Ensure that ActionType, ChangeActions and RevisionActions
- // properties are replaced with enums everywhere and remove them from
- // the GrChangeActions class
- ActionType = ActionType;
-
- ChangeActions = ChangeActions;
-
- RevisionActions = RevisionActions;
-
@state() change?: ParsedChangeInfo;
@state() actions: ActionNameToActionInfoMap = {};
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
index b953eec..b6a1a6a 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
@@ -39,7 +39,7 @@
ReviewInput,
TopicName,
} from '../../../types/common';
-import {ActionType} from '../../../api/change-actions';
+import {ActionType, RevisionActions} from '../../../api/change-actions';
import {SinonFakeTimers, SinonStubbedMember} from 'sinon';
import {IronAutogrowTextareaElement} from '@polymer/iron-autogrow-textarea';
import {GrButton} from '../../shared/gr-button/gr-button';
@@ -409,8 +409,8 @@
);
assert.isOk(buttonEl);
element.setActionHidden(
- element.ActionType.REVISION,
- element.RevisionActions.SUBMIT,
+ ActionType.REVISION,
+ RevisionActions.SUBMIT,
true
);
assert.lengthOf(element.hiddenActions, 1);
@@ -419,8 +419,8 @@
assert.isNotOk(buttonEl);
element.setActionHidden(
- element.ActionType.REVISION,
- element.RevisionActions.SUBMIT,
+ ActionType.REVISION,
+ RevisionActions.SUBMIT,
false
);
await element.updateComplete;
@@ -1235,7 +1235,7 @@
test('custom actions', async () => {
// Add a button with the same key as a server-based one to ensure
// collisions are taken care of.
- const key = element.addActionButton(element.ActionType.REVISION, 'Bork!');
+ const key = element.addActionButton(ActionType.REVISION, 'Bork!');
const keyTapped = mockPromise();
element.addEventListener(key + '-tap', async e => {
assert.equal(
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index 532052d..7c893f2 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -1078,7 +1078,7 @@
return '(0)';
}
}
- if (!this.generatedSuggestion) {
+ if (this.generatedSuggestion) {
return '(1)';
} else {
return '(0)';
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.ts
index f0143a6..523cc59 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.ts
@@ -9,10 +9,8 @@
import {
ActionPriority,
ActionType,
- ChangeActions,
ChangeActionsPluginApi,
PrimaryActionKey,
- RevisionActions,
} from '../../../api/change-actions';
import {PropertyDeclaration} from 'lit';
import {JsApiService} from './gr-js-api-types';
@@ -28,9 +26,6 @@
// This interface is required to avoid circular dependencies between files;
export interface GrChangeActionsElement extends Element {
- RevisionActions?: Record<string, string>;
- ChangeActions: Record<string, string>;
- ActionType: Record<string, string>;
primaryActionKeys: string[];
hideQuickApproveAction(): void;
setActionOverflow(type: ActionType, key: string, overflow: boolean): void;
@@ -58,12 +53,6 @@
export class GrChangeActionsInterface implements ChangeActionsPluginApi {
private el?: GrChangeActionsElement;
- RevisionActions = RevisionActions;
-
- ChangeActions = ChangeActions;
-
- ActionType = ActionType;
-
private readonly reporting = getAppContext().reportingService;
constructor(
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts
index e557ca8..0cc4f7f 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts
@@ -69,14 +69,6 @@
testResolver(pluginLoaderToken).loadPlugins([]);
});
- test('property existence', () => {
- const properties = ['ActionType', 'ChangeActions', 'RevisionActions'];
- for (const p of properties) {
- // Have to type as any to prevent 'has no index signature.'
- assert.deepEqual((changeActions as any)[p], (element as any)[p]);
- }
- });
-
test('add/remove primary action keys', () => {
element.primaryActionKeys = [];
changeActions.addPrimaryActionKey('foo' as PrimaryActionKey);
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
index 1501205..c488dfb 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
@@ -358,8 +358,12 @@
}
private handleTabKey(e: KeyboardEvent) {
- // Tab should have normal behavior if the picker is closed.
- if (!this.isDropdownVisible()) {
+ // Tab should have normal behavior if the picker is closed, or "open" but
+ // with no results.
+ if (
+ !this.isDropdownVisible() ||
+ this.getVisibleDropdown().getCurrentText() === ''
+ ) {
return;
}
e.preventDefault();
diff --git a/proto/entities.proto b/proto/entities.proto
index 3412291..335db62 100644
--- a/proto/entities.proto
+++ b/proto/entities.proto
@@ -311,3 +311,64 @@
}
optional EditPreferencesInfo edit_preferences_info = 3;
}
+
+// Next Id: 13
+message HumanComment {
+ // Required. Note that the equivalent Java struct does not contain the change
+ // ID, so we keep the same format here.
+ optional int32 patchset_id = 1;
+ optional ObjectId dest_commit_id = 2;
+ // Required.
+ optional Account_Id account_id = 3;
+ optional Account_Id real_author = 4;
+
+ // Next Id: 5
+ message InFilePosition {
+ optional string file_path = 1;
+ enum Side {
+ // Should match the logic in
+ // http://google3/third_party/java_src/gerritcodereview/gerrit/java/com/google/gerrit/extensions/client/Side.java?rcl=579772037&l=24
+ PARENT = 0;
+ REVISION = 1;
+ }
+ // Default should match
+ // http://google3/third_party/java_src/gerritcodereview/gerrit/Documentation/rest-api-changes.txt?l=7423
+ optional Side side = 2 [default = REVISION];
+ message Range {
+ // 1-based
+ optional int32 start_line = 1;
+ // 0-based
+ optional int32 start_char = 2;
+ // 1-based
+ optional int32 end_line = 3;
+ // 0-based
+ optional int32 end_char = 4;
+ }
+ // If neither range nor line number set, the comment is on the file level. It is possible
+ // (though not required) for both values to be set. in this case, it is expected that the line
+ // number is identical to the range's end line.
+ optional Range position_range = 3;
+ // 1-based
+ optional int32 line_number = 4;
+ }
+
+ // If not set, the comment is on the patchset level.
+ optional InFilePosition in_file_position = 5;
+
+ // Required.
+ optional string comment_text = 6;
+ // Might be set by the user while creating the draft.
+ // See http://go/gerrit-rest-api-change#comment-info.
+ optional string tag = 7;
+ optional bool unresolved = 8 [default = false];
+
+ // Required.
+ optional string comment_uuid = 9;
+ // Required.
+ optional string parent_comment_uuid = 10;
+
+ // Required. Epoch millis.
+ optional fixed64 written_on_millis = 11;
+ // Required.
+ optional string server_id = 12;
+}
\ No newline at end of file
