Merge "Stop query from executing if predicate is empty"
diff --git a/.gitignore b/.gitignore
index 75cdfb5..93c1339 100644
--- a/.gitignore
+++ b/.gitignore
@@ -9,7 +9,6 @@
/.apt_generated
/.apt_generated_tests
/.bazel_path
-/.buckd
/.classpath
/.factorypath
/.idea
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 00350d1..37bf3e1 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1565,12 +1565,16 @@
Execute `java -jar gerrit.war daemon --help` to see all possible
options.
-[[container.slave]]container.slave::
+[[container.replica]]container.replica::
+
Used on Gerrit replica installations. If set to true the Gerrit JVM is
called with the '--replica' switch, enabling replica mode. If no value is
set (or any other value), Gerrit defaults to master mode.
+[[container.slave]]container.slave::
++
+Backward compatibility for 'container.slave' config setting.
+
[[container.startupTimeout]]container.startupTimeout::
+
The maximum time (in seconds) to wait for a gerrit.sh start command
@@ -3640,7 +3644,10 @@
[[plugins.mandatory]]plugins.mandatory::
+
List of mandatory plugins. If a plugin from this list does not load,
-Gerrit start will fail.
+Gerrit will fail to start.
++
+Disabling and restarting of a mandatory plugin is rejected, but reloading
+of a mandatory plugin is still possible.
[[plugins.jsLoadTimeout]]plugins.jsLoadTimeout::
+
diff --git a/Documentation/dev-build-plugins.txt b/Documentation/dev-build-plugins.txt
index 8139743..219861f 100644
--- a/Documentation/dev-build-plugins.txt
+++ b/Documentation/dev-build-plugins.txt
@@ -69,7 +69,7 @@
The output can be normally found in the following directory:
----
-bazel-genfiles/plugins/<plugin-name>/<plugin-name>.jar
+bazel-bin/plugins/<plugin-name>/<plugin-name>.jar
----
Some plugins describe their build process in `src/main/resources/Documentation/build.md`
diff --git a/Documentation/dev-release.txt b/Documentation/dev-release.txt
index 48bafdf..9e1744c 100644
--- a/Documentation/dev-release.txt
+++ b/Documentation/dev-release.txt
@@ -98,7 +98,7 @@
Tag the plugins:
----
- git submodule foreach git tag -s -m "v$version" "v$version"
+ git submodule foreach '[ "$path" == "modules/jgit" ] || git tag -s -m "v$version" "v$version"'
----
[[build-gerrit]]
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 6b8281a..10fb741 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -1501,6 +1501,90 @@
change is new
----
+[[revert-submission]]
+=== Revert Submission
+--
+'POST /changes/link:#change-id[\{change-id\}]/revert_submission'
+--
+
+Creates open revert changes for all of the changes of a certain submission.
+
+Details for the revert can be specified in the request body inside a link:#revert-input[
+RevertInput] The topic of all created revert changes will be
+`revert-{submission_id}-{random_string_of_size_10}`.
+
+The changes will not be rebased on onto the destination branch so the users may still
+have to manually rebase them to resolve conflicts and make them submittable.
+
+.Request
+----
+ POST /changes/myProject~master~I1ffe09a505e25f15ce1521bcfb222e51e62c2a14/revert_submission HTTP/1.0
+----
+
+As response link:#revert-submission-info[RevertSubmissionInfo] entity
+is returned. That entity describes the revert changes.
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ "revert_changes":
+ [
+ {
+ "id": "myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940",
+ "project": "myProject",
+ "branch": "master",
+ "topic": "revert--1571043962462-3640749-ABCEEZGHIJ",
+ "change_id": "I8473b95934b5732ac55d26311a706c9c2bde9940",
+ "subject": "Revert \"Implementing Feature X\"",
+ "status": "NEW",
+ "created": "2013-02-01 09:59:32.126000000",
+ "updated": "2013-02-21 11:16:36.775000000",
+ "mergeable": true,
+ "insertions": 6,
+ "deletions": 4,
+ "_number": 3965,
+ "owner": {
+ "name": "John Doe"
+ }
+ },
+ {
+ "id": "anyProject~master~1eee2c9d8f352483781e772f35dc586a69ff5646",
+ "project": "anyProject",
+ "branch": "master",
+ "topic": "revert--1571043962462-3640749-ABCEEZGHIJ",
+ "change_id": "I1eee2c9d8f352483781e772f35dc586a69ff5646",
+ "subject": "Revert \"Implementing Feature Y\"",
+ "status": "NEW",
+ "created": "2013-02-04 09:59:33.126000000",
+ "updated": "2013-02-21 11:16:37.775000000",
+ "mergeable": true,
+ "insertions": 62,
+ "deletions": 11,
+ "_number": 3966,
+ "owner": {
+ "name": "Jane Doe"
+ }
+ }
+ ]
+----
+
+If any of the changes cannot be reverted because the change state doesn't
+allow reverting the change, the response is "`409 Conflict`" and
+the error message is contained in the response body.
+
+.Response
+----
+ HTTP/1.1 409 Conflict
+ Content-Disposition: attachment
+ Content-Type: text/plain; charset=UTF-8
+
+ change is new
+----
+
[[submit-change]]
=== Submit Change
--
@@ -6931,8 +7015,22 @@
Additional information about whom to notify about the revert as a map
of recipient type to link:#notify-info[NotifyInfo] entity.
|`topic` |optional|
-Name of the topic for the revert change. If not set, the default is the topic
-of the change being reverted.
+Name of the topic for the revert change. If not set, the default for Revert
+endpoint is the topic of the change being reverted, and the default for the
+RevertSubmission endpoint is `revert-{submission_id}-{timestamp.now}`.
+|===========================
+
+[[revert-submission-info]]
+=== RevertSubmissionInfo
+The `RevertSubmissionInfo` describes the revert changes.
+
+[options="header",cols="1,6"]
+|===========================
+|Field Name | Description
+|`revert_changes` |
+A list of #change-info[ChangeInfo] that describes the revert
+changes. Each entity in that list is a revert change that was created in that
+revert submission.
|=============================
[[review-info]]
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index c1349aa..b70dfea 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -1252,6 +1252,57 @@
}
----
+[[create-change]]
+=== Create Change for review.
+
+This endpoint is functionally equivalent to
+link:rest-api-changes.html#create-change[create change in the change
+API], but it has the project name in the URL, which is easier to route
+in sharded deployments.
+
+.Request
+----
+ POST /projects/myProject/create.change HTTP/1.0
+ Content-Type: application/json; charset=UTF-8
+
+ {
+ "subject" : "Let's support 100% Gerrit workflow direct in browser",
+ "branch" : "master",
+ "topic" : "create-change-in-browser",
+ "status" : "NEW"
+ }
+----
+
+As response a link:#change-info[ChangeInfo] entity is returned that describes
+the resulting change.
+
+.Response
+----
+ HTTP/1.1 201 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "id": "myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9941",
+ "project": "myProject",
+ "branch": "master",
+ "topic": "create-change-in-browser",
+ "change_id": "I8473b95934b5732ac55d26311a706c9c2bde9941",
+ "subject": "Let's support 100% Gerrit workflow direct in browser",
+ "status": "NEW",
+ "created": "2014-05-05 07:15:44.639000000",
+ "updated": "2014-05-05 07:15:44.639000000",
+ "mergeable": true,
+ "insertions": 0,
+ "deletions": 0,
+ "_number": 4711,
+ "owner": {
+ "name": "John Doe"
+ }
+ }
+----
+
[[create-access-change]]
=== Create Access Rights Change for review.
--
diff --git a/WORKSPACE b/WORKSPACE
index 00c905c..e623e42 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -760,7 +760,7 @@
sha1 = "f7be08ec23c21485b9b5a1cf1654c2ec8c58168d",
)
-GITILES_VERS = "0.3-4"
+GITILES_VERS = "0.3-5"
GITILES_REPO = GERRIT
@@ -769,14 +769,14 @@
artifact = "com.google.gitiles:blame-cache:" + GITILES_VERS,
attach_source = False,
repository = GITILES_REPO,
- sha1 = "d1d62c9905b0cc9e066d337b33480599f430eb87",
+ sha1 = "22d5e48827bd48b9e7b049bb9726ef017fda9eca",
)
maven_jar(
name = "gitiles-servlet",
artifact = "com.google.gitiles:gitiles-servlet:" + GITILES_VERS,
repository = GITILES_REPO,
- sha1 = "7360cb90576a813cd1288cf853c59824fe92467e",
+ sha1 = "061de6d5ef22be870300cc01a6fb205bb7782eae",
)
# prettify must match the version used in Gitiles
diff --git a/contrib/hooks/post-receive-move-tmp-refs b/contrib/hooks/post-receive-move-tmp-refs
index c4a53db..c99a3e5 100755
--- a/contrib/hooks/post-receive-move-tmp-refs
+++ b/contrib/hooks/post-receive-move-tmp-refs
@@ -23,6 +23,8 @@
# remote.NAME.push = +refs/changes/*:refs/tmp/changes/*
# remote.NAME.push = +refs/heads/*:refs/heads/*
# remote.NAME.push = +refs/tags/*:refs/tags/*
+# And if it's a Gerrit mirror:
+# remote.NAME.push = +refs/meta/*:refs/meta/*
#
# In the replicated repository in the gerrit slave configure
# receive.hideRefs = refs/changes/
@@ -47,18 +49,15 @@
# automatically have the "tmp-refs" commit hook installed.
# See https://git-scm.com/docs/git-init#_template_directory for details.
-readonly NULL_SHA1=0000000000000000000000000000000000000000
-
# move new changes arriving under refs/tmp/changes/ to refs/changes/
mv_tmp_refs()
{
oldrev=$1
newrev=$2
refname=$3
- case "$refname","$oldrev" in
- refs/tmp/changes/*,$NULL_SHA1)
+ case "$refname" in refs/tmp/changes/*)
short_refname=${refname##refs/tmp/changes/}
- $(git update-ref refs/changes/$short_refname $newrev $NULL_SHA1 2>/dev/null)
+ $(git update-ref refs/changes/$short_refname $newrev 2>/dev/null)
$(git update-ref -d $refname $newrev 2>/dev/null)
echo "moved \"$refname\" to \"refs/changes/$short_refname\""
;;
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index a5066b7..678bc31 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -44,6 +44,7 @@
import com.google.gerrit.server.git.receive.AsyncReceiveCommits;
import com.google.gerrit.server.schema.JdbcAccountPatchReviewStore;
import com.google.gerrit.server.ssh.NoSshModule;
+import com.google.gerrit.server.util.ReplicaUtil;
import com.google.gerrit.server.util.SocketUtil;
import com.google.gerrit.server.util.SystemLog;
import com.google.gerrit.testing.FakeEmailSender;
@@ -450,7 +451,7 @@
@Nullable InMemoryRepositoryManager inMemoryRepoManager)
throws Exception {
Config cfg = desc.buildConfig(baseConfig);
- daemon.setReplica(isReplica(baseConfig) || isReplica(cfg));
+ daemon.setReplica(ReplicaUtil.isReplica(baseConfig) || ReplicaUtil.isReplica(cfg));
mergeTestConfig(cfg);
// Set the log4j configuration to an invalid one to prevent system logs
// from getting configured and creating log files.
@@ -463,7 +464,8 @@
cfg.setString(
"accountPatchReviewDb", null, "url", JdbcAccountPatchReviewStore.TEST_IN_MEMORY_URL);
daemon.setEnableHttpd(desc.httpd());
- daemon.setLuceneModule(LuceneIndexModule.singleVersionAllLatest(0, isReplica(baseConfig)));
+ daemon.setLuceneModule(
+ LuceneIndexModule.singleVersionAllLatest(0, ReplicaUtil.isReplica(baseConfig)));
daemon.setDatabaseForTesting(
ImmutableList.of(
new InMemoryTestingDatabaseModule(cfg, site, inMemoryRepoManager),
@@ -479,10 +481,6 @@
return new GerritServer(desc, null, createTestInjector(daemon), daemon, null);
}
- private static boolean isReplica(Config baseConfig) {
- return baseConfig.getBoolean("container", "slave", false);
- }
-
private static GerritServer startOnDisk(
Description desc,
Path site,
@@ -662,7 +660,7 @@
Path site = server.testInjector.getInstance(Key.get(Path.class, SitePath.class));
Config cfg = server.testInjector.getInstance(Key.get(Config.class, GerritServerConfig.class));
- cfg.setBoolean("container", null, "slave", true);
+ cfg.setBoolean("container", null, "replica", true);
InMemoryRepositoryManager inMemoryRepoManager = null;
if (hasBinding(server.testInjector, InMemoryRepositoryManager.class)) {
diff --git a/java/com/google/gerrit/acceptance/ReindexGroupsAtStartup.java b/java/com/google/gerrit/acceptance/ReindexGroupsAtStartup.java
index bd8a926..b985e40 100644
--- a/java/com/google/gerrit/acceptance/ReindexGroupsAtStartup.java
+++ b/java/com/google/gerrit/acceptance/ReindexGroupsAtStartup.java
@@ -20,6 +20,7 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.group.db.Groups;
import com.google.gerrit.server.index.group.GroupIndexer;
+import com.google.gerrit.server.util.ReplicaUtil;
import com.google.inject.Inject;
import com.google.inject.Scopes;
import java.io.IOException;
@@ -50,8 +51,8 @@
@Override
public void start() {
- // Gerrit slaves without a reindex
- if (cfg.getBoolean("container", "slave", false)
+ // Gerrit replicas without a reindex
+ if (ReplicaUtil.isReplica(cfg)
&& !cfg.getBoolean("index", "scheduledIndexer", "runOnStartup", true)) {
return;
}
diff --git a/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java b/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java
index 58a9e1c..a06f90f 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java
@@ -14,8 +14,6 @@
package com.google.gerrit.elasticsearch;
-import static com.google.gerrit.server.index.account.AccountField.ID;
-
import com.google.gerrit.elasticsearch.ElasticMapping.MappingProperties;
import com.google.gerrit.elasticsearch.bulk.BulkRequest;
import com.google.gerrit.elasticsearch.bulk.IndexRequest;
@@ -92,8 +90,16 @@
@Override
public DataSource<AccountState> getSource(Predicate<AccountState> p, QueryOptions opts)
throws QueryParseException {
- JsonArray sortArray = getSortArray(AccountField.ID.getName());
- return new ElasticQuerySource(p, opts.filterFields(IndexUtils::accountFields), type, sortArray);
+ JsonArray sortArray =
+ getSortArray(
+ schema.useLegacyNumericFields()
+ ? AccountField.ID.getName()
+ : AccountField.ID_STR.getName());
+ return new ElasticQuerySource(
+ p,
+ opts.filterFields(o -> IndexUtils.accountFields(o, schema.useLegacyNumericFields())),
+ type,
+ sortArray);
}
@Override
@@ -118,7 +124,15 @@
source = json.getAsJsonObject().get("fields");
}
- Account.Id id = Account.id(source.getAsJsonObject().get(ID.getName()).getAsInt());
+ Account.Id id =
+ Account.id(
+ source
+ .getAsJsonObject()
+ .get(
+ schema.useLegacyNumericFields()
+ ? AccountField.ID.getName()
+ : AccountField.ID_STR.getName())
+ .getAsInt());
// Use the AccountCache rather than depending on any stored fields in the document (of which
// there shouldn't be any). The most expensive part to compute anyway is the effective group
// IDs, and we don't have a good way to reindex when those change.
diff --git a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
index 1e35191..6151de2 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
@@ -40,6 +40,7 @@
import com.google.gerrit.entities.converter.PatchSetApprovalProtoConverter;
import com.google.gerrit.entities.converter.PatchSetProtoConverter;
import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.query.DataSource;
@@ -91,6 +92,7 @@
private final ChangeMapping mapping;
private final ChangeData.Factory changeDataFactory;
private final Schema<ChangeData> schema;
+ private final FieldDef<ChangeData, ?> idField;
@Inject
ElasticChangeIndex(
@@ -102,7 +104,9 @@
super(cfg, sitePaths, schema, clientBuilder, CHANGES);
this.changeDataFactory = changeDataFactory;
this.schema = schema;
- mapping = new ChangeMapping(schema, client.adapter());
+ this.mapping = new ChangeMapping(schema, client.adapter());
+ this.idField =
+ this.schema.useLegacyNumericFields() ? ChangeField.LEGACY_ID : ChangeField.LEGACY_ID_STR;
}
@Override
@@ -157,7 +161,8 @@
}
}
- QueryOptions filteredOpts = opts.filterFields(IndexUtils::changeFields);
+ QueryOptions filteredOpts =
+ opts.filterFields(o -> IndexUtils.changeFields(o, schema.useLegacyNumericFields()));
return new ElasticQuerySource(p, filteredOpts, getURI(indexes), getSortArray());
}
@@ -167,7 +172,7 @@
JsonArray sortArray = new JsonArray();
addNamedElement(ChangeField.UPDATED.getName(), properties, sortArray);
- addNamedElement(ChangeField.LEGACY_ID.getName(), properties, sortArray);
+ addNamedElement(idField.getName(), properties, sortArray);
return sortArray;
}
@@ -206,7 +211,7 @@
JsonElement c = source.get(ChangeField.CHANGE.getName());
if (c == null) {
- int id = source.get(ChangeField.LEGACY_ID.getName()).getAsInt();
+ int id = source.get(idField.getName()).getAsInt();
// IndexUtils#changeFields ensures either CHANGE or PROJECT is always present.
String projectName = requireNonNull(source.get(ChangeField.PROJECT.getName()).getAsString());
return changeDataFactory.create(Project.nameKey(projectName), Change.id(id));
diff --git a/java/com/google/gerrit/elasticsearch/ElasticQueryBuilder.java b/java/com/google/gerrit/elasticsearch/ElasticQueryBuilder.java
index 394158d..d05e91c 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticQueryBuilder.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticQueryBuilder.java
@@ -145,7 +145,7 @@
String name = p.getField().getName();
String value = p.getValue();
- if (value.isEmpty()) {
+ if (!p.getField().isRepeatable() && value.isEmpty()) {
return new BoolQueryBuilder().mustNot(QueryBuilders.existsQuery(name));
} else if (p instanceof RegexPredicate) {
if (value.startsWith("^")) {
diff --git a/java/com/google/gerrit/elasticsearch/bulk/UpdateRequest.java b/java/com/google/gerrit/elasticsearch/bulk/UpdateRequest.java
index a693f6d..2f0bd01 100644
--- a/java/com/google/gerrit/elasticsearch/bulk/UpdateRequest.java
+++ b/java/com/google/gerrit/elasticsearch/bulk/UpdateRequest.java
@@ -40,11 +40,7 @@
for (Values<V> values : schema.buildFields(v)) {
String name = values.getField().getName();
if (values.getField().isRepeatable()) {
- builder.field(
- name,
- Streams.stream(values.getValues())
- .filter(e -> shouldAddElement(e))
- .collect(toList()));
+ builder.field(name, Streams.stream(values.getValues()).collect(toList()));
} else {
Object element = Iterables.getOnlyElement(values.getValues(), "");
if (shouldAddElement(element)) {
diff --git a/java/com/google/gerrit/entities/Change.java b/java/com/google/gerrit/entities/Change.java
index ced3823..739bd38 100644
--- a/java/com/google/gerrit/entities/Change.java
+++ b/java/com/google/gerrit/entities/Change.java
@@ -44,7 +44,7 @@
* |
* +- {@link PatchSetApproval}: a +/- vote on the change's current state.
* |
- * +- {@link PatchLineComment}: comment about a specific line
+ * +- {@link Comment}: comment about a specific line
* </pre>
*
* <p>
@@ -60,8 +60,8 @@
* commit can contain zero patches, if the merge has no conflicts, or has no impact other than to
* cut off a line of development.
*
- * <p>Each PatchLineComment is a draft or a published comment about a single line of the associated
- * file. These are the inline comment entities created by users as they perform a review.
+ * <p>Each Comment is a draft or a published comment about a single line of the associated file.
+ * These are the inline comment entities created by users as they perform a review.
*
* <p>When additional PatchSets appear under a change, these PatchSets reference <i>replacement</i>
* commits; alternative commits that could be made to the project instead of the original commit
@@ -76,10 +76,10 @@
* <h5>ChangeMessage</h5>
*
* <p>The ChangeMessage entity is a general free-form comment about the whole change, rather than
- * PatchLineComment's file and line specific context. The ChangeMessage appears at the start of any
- * email generated by Gerrit, and is shown on the change overview page, rather than in a
- * file-specific context. Users often use this entity to describe general remarks about the overall
- * concept proposed by the change.
+ * Comment's file and line specific context. The ChangeMessage appears at the start of any email
+ * generated by Gerrit, and is shown on the change overview page, rather than in a file-specific
+ * context. Users often use this entity to describe general remarks about the overall concept
+ * proposed by the change.
*
* <p>
*
diff --git a/java/com/google/gerrit/entities/Comment.java b/java/com/google/gerrit/entities/Comment.java
index db62beea..65c642c 100644
--- a/java/com/google/gerrit/entities/Comment.java
+++ b/java/com/google/gerrit/entities/Comment.java
@@ -29,30 +29,33 @@
*
* <p>Changing fields in this class changes the storage format of inline comments in NoteDb and may
* require a corresponding data migration (adding new optional fields is generally okay).
- *
- * <p>{@link PatchLineComment} historically represented comments in ReviewDb. There are a few
- * notable differences:
- *
- * <ul>
- * <li>PatchLineComment knows the comment status (published or draft). For comments in NoteDb the
- * status is determined by the branch in which they are stored (published comments are stored
- * in the change meta ref; draft comments are store in refs/draft-comments branches in
- * All-Users). Hence Comment doesn't need to contain the status, but the status is implicitly
- * known by where the comments are read from.
- * <li>PatchLineComment knows the change ID. For comments in NoteDb, the change ID is determined
- * by the branch in which they are stored (the ref name contains the change ID). Hence Comment
- * doesn't need to contain the change ID, but the change ID is implicitly known by where the
- * comments are read from.
- * </ul>
- *
- * <p>For all utility classes and middle layer functionality using Comment over PatchLineComment is
- * preferred, as ReviewDb is gone so PatchLineComment is slated for deletion as well. This means
- * Comment should be used everywhere and only for storing inline comment in ReviewDb a conversion to
- * PatchLineComment is done. Converting Comments to PatchLineComments and vice verse is done by
- * CommentsUtil#toPatchLineComments(Change.Id, PatchLineComment.Status, Iterable) and
- * CommentsUtil#toComments(String, Iterable).
*/
public class Comment {
+ public enum Status {
+ DRAFT('d'),
+
+ PUBLISHED('P');
+
+ private final char code;
+
+ Status(char c) {
+ code = c;
+ }
+
+ public char getCode() {
+ return code;
+ }
+
+ public static Status forCode(char c) {
+ for (Status s : Status.values()) {
+ if (s.code == c) {
+ return s;
+ }
+ }
+ return null;
+ }
+ }
+
public static class Key {
public String uuid;
public String filename;
diff --git a/java/com/google/gerrit/entities/PatchLineComment.java b/java/com/google/gerrit/entities/PatchLineComment.java
deleted file mode 100644
index 2182589..0000000
--- a/java/com/google/gerrit/entities/PatchLineComment.java
+++ /dev/null
@@ -1,327 +0,0 @@
-// Copyright (C) 2008 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;
-
-import com.google.gerrit.common.Nullable;
-import com.google.gerrit.extensions.client.Comment.Range;
-import java.sql.Timestamp;
-import java.util.Objects;
-import org.eclipse.jgit.lib.AnyObjectId;
-import org.eclipse.jgit.lib.ObjectId;
-
-/**
- * A comment left by a user on a specific line of a {@link Patch}.
- *
- * <p>New APIs should not expose this class.
- *
- * @see Comment
- */
-public final class PatchLineComment {
- public static final char STATUS_DRAFT = 'd';
- public static final char STATUS_PUBLISHED = 'P';
-
- public enum Status {
- DRAFT(STATUS_DRAFT),
-
- PUBLISHED(STATUS_PUBLISHED);
-
- private final char code;
-
- Status(char c) {
- code = c;
- }
-
- public char getCode() {
- return code;
- }
-
- public static Status forCode(char c) {
- for (Status s : Status.values()) {
- if (s.code == c) {
- return s;
- }
- }
- return null;
- }
- }
-
- public static PatchLineComment from(
- Change.Id changeId, PatchLineComment.Status status, Comment c) {
- Patch.Key patchKey = Patch.key(PatchSet.id(changeId, c.key.patchSetId), c.key.filename);
- PatchLineComment plc =
- new PatchLineComment(
- patchKey, c.key.uuid, c.lineNbr, c.author.getId(), c.parentUuid, c.writtenOn);
- plc.setSide(c.side);
- plc.setMessage(c.message);
- if (c.range != null) {
- Comment.Range r = c.range;
- plc.setRange(new CommentRange(r.startLine, r.startChar, r.endLine, r.endChar));
- }
- plc.setTag(c.tag);
- plc.setCommitId(c.getCommitId());
- plc.setStatus(status);
- plc.setRealAuthor(c.getRealAuthor().getId());
- plc.setUnresolved(c.unresolved);
- return plc;
- }
-
- protected Patch.Key patchKey;
-
- protected String uuid;
-
- /** Line number this comment applies to; it should display after the line. */
- protected int lineNbr;
-
- /** Who wrote this comment. */
- protected Account.Id author;
-
- /** When this comment was drafted. */
- protected Timestamp writtenOn;
-
- /** Current publication state of the comment; see {@link Status}. */
- protected char status;
-
- /** Which file is this comment; 0 is ancestor, 1 is new version. */
- protected short side;
-
- /** The text left by the user. */
- @Nullable protected String message;
-
- /** The parent of this comment, or null if this is the first comment on this line */
- @Nullable protected String parentUuid;
-
- @Nullable protected CommentRange range;
-
- @Nullable protected String tag;
-
- /** Real user that added this comment on behalf of the user recorded in {@link #author}. */
- @Nullable protected Account.Id realAuthor;
-
- /** True if this comment requires further action. */
- protected boolean unresolved;
-
- /** The ID of the commit to which this comment is referring. */
- protected ObjectId commitId;
-
- protected PatchLineComment() {}
-
- public PatchLineComment(
- Patch.Key patchKey, String uuid, int line, Account.Id a, String parentUuid, Timestamp when) {
- this.patchKey = patchKey;
- this.uuid = uuid;
- lineNbr = line;
- author = a;
- setParentUuid(parentUuid);
- setStatus(Status.DRAFT);
- setWrittenOn(when);
- }
-
- public PatchLineComment(PatchLineComment o) {
- patchKey = o.patchKey;
- uuid = o.uuid;
- lineNbr = o.lineNbr;
- author = o.author;
- realAuthor = o.realAuthor;
- writtenOn = o.writtenOn;
- status = o.status;
- side = o.side;
- message = o.message;
- parentUuid = o.parentUuid;
- commitId = o.commitId;
- if (o.range != null) {
- range =
- new CommentRange(
- o.range.getStartLine(),
- o.range.getStartCharacter(),
- o.range.getEndLine(),
- o.range.getEndCharacter());
- }
- }
-
- public PatchSet.Id getPatchSetId() {
- return patchKey.patchSetId();
- }
-
- public int getLine() {
- return lineNbr;
- }
-
- public void setLine(int line) {
- lineNbr = line;
- }
-
- public Account.Id getAuthor() {
- return author;
- }
-
- public Account.Id getRealAuthor() {
- return realAuthor != null ? realAuthor : getAuthor();
- }
-
- public void setRealAuthor(Account.Id id) {
- // Use null for same real author, as before the column was added.
- realAuthor = Objects.equals(getAuthor(), id) ? null : id;
- }
-
- public Timestamp getWrittenOn() {
- return writtenOn;
- }
-
- public Status getStatus() {
- return Status.forCode(status);
- }
-
- public void setStatus(Status s) {
- status = s.getCode();
- }
-
- public short getSide() {
- return side;
- }
-
- public void setSide(short s) {
- side = s;
- }
-
- public String getMessage() {
- return message;
- }
-
- public void setMessage(String s) {
- message = s;
- }
-
- public void setWrittenOn(Timestamp ts) {
- writtenOn = ts;
- }
-
- public String getParentUuid() {
- return parentUuid;
- }
-
- public void setParentUuid(String inReplyTo) {
- parentUuid = inReplyTo;
- }
-
- public void setRange(Range r) {
- if (r != null) {
- range =
- new CommentRange(
- r.startLine, r.startCharacter,
- r.endLine, r.endCharacter);
- } else {
- range = null;
- }
- }
-
- public void setRange(CommentRange r) {
- range = r;
- }
-
- public CommentRange getRange() {
- return range;
- }
-
- public void setCommitId(AnyObjectId commitId) {
- this.commitId = commitId.copy();
- }
-
- public ObjectId getCommitId() {
- return commitId;
- }
-
- public void setTag(String tag) {
- this.tag = tag;
- }
-
- public String getTag() {
- return tag;
- }
-
- public void setUnresolved(Boolean unresolved) {
- this.unresolved = unresolved;
- }
-
- public Boolean getUnresolved() {
- return unresolved;
- }
-
- public Comment asComment(String serverId) {
- Comment c =
- new Comment(
- new Comment.Key(uuid, patchKey.fileName(), patchKey.patchSetId().get()),
- author,
- writtenOn,
- side,
- message,
- serverId,
- unresolved);
- c.setCommitId(commitId);
- c.setRange(range);
- c.lineNbr = lineNbr;
- c.parentUuid = parentUuid;
- c.tag = tag;
- c.setRealAuthor(getRealAuthor());
- return c;
- }
-
- @Override
- public boolean equals(Object o) {
- if (o instanceof PatchLineComment) {
- PatchLineComment c = (PatchLineComment) o;
- return Objects.equals(patchKey, c.patchKey)
- && Objects.equals(uuid, c.uuid)
- && Objects.equals(lineNbr, c.getLine())
- && Objects.equals(author, c.getAuthor())
- && Objects.equals(writtenOn, c.getWrittenOn())
- && Objects.equals(status, c.getStatus().getCode())
- && Objects.equals(side, c.getSide())
- && Objects.equals(message, c.getMessage())
- && Objects.equals(parentUuid, c.getParentUuid())
- && Objects.equals(range, c.getRange())
- && Objects.equals(commitId, c.getCommitId())
- && Objects.equals(tag, c.getTag())
- && Objects.equals(unresolved, c.getUnresolved());
- }
- return false;
- }
-
- @Override
- public int hashCode() {
- return Objects.hash(patchKey, uuid);
- }
-
- @Override
- public String toString() {
- StringBuilder builder = new StringBuilder();
- builder.append("PatchLineComment{");
- builder.append("patchKey=").append(patchKey).append(',');
- builder.append("uuid=").append(uuid).append(',');
- builder.append("lineNbr=").append(lineNbr).append(',');
- builder.append("author=").append(author.get()).append(',');
- builder.append("realAuthor=").append(realAuthor != null ? realAuthor.get() : "").append(',');
- builder.append("writtenOn=").append(writtenOn.toString()).append(',');
- builder.append("status=").append(status).append(',');
- builder.append("side=").append(side).append(',');
- builder.append("message=").append(Objects.toString(message, "")).append(',');
- builder.append("parentUuid=").append(Objects.toString(parentUuid, "")).append(',');
- builder.append("range=").append(Objects.toString(range, "")).append(',');
- builder.append("revId=").append(commitId != null ? commitId.name() : "").append(',');
- builder.append("tag=").append(Objects.toString(tag, "")).append(',');
- builder.append("unresolved=").append(unresolved);
- builder.append('}');
- return builder.toString();
- }
-}
diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
index 26a1a27..119d941 100644
--- a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
@@ -27,6 +27,7 @@
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.common.MergePatchSetInput;
import com.google.gerrit.extensions.common.PureRevertInfo;
+import com.google.gerrit.extensions.common.RevertSubmissionInfo;
import com.google.gerrit.extensions.common.RobotCommentInfo;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
import com.google.gerrit.extensions.restapi.NotImplementedException;
@@ -161,6 +162,12 @@
*/
ChangeApi revert(RevertInput in) throws RestApiException;
+ default RevertSubmissionInfo revertSubmission() throws RestApiException {
+ return revertSubmission(new RevertInput());
+ }
+
+ RevertSubmissionInfo revertSubmission(RevertInput in) throws RestApiException;
+
/** Create a merge patch set for the change. */
ChangeInfo createMergePatchSet(MergePatchSetInput in) throws RestApiException;
@@ -502,6 +509,11 @@
}
@Override
+ public RevertSubmissionInfo revertSubmission(RevertInput in) throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public void rebase(RebaseInput in) throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/common/RevertSubmissionInfo.java b/java/com/google/gerrit/extensions/common/RevertSubmissionInfo.java
new file mode 100644
index 0000000..dabd035
--- /dev/null
+++ b/java/com/google/gerrit/extensions/common/RevertSubmissionInfo.java
@@ -0,0 +1,21 @@
+// Copyright (C) 2019 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.extensions.common;
+
+import java.util.List;
+
+public class RevertSubmissionInfo {
+ public List<ChangeInfo> revertChanges;
+}
diff --git a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
index 40939b3..b9b66bc 100644
--- a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
+++ b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
@@ -112,9 +112,6 @@
SanitizedContent sanitizedStaticPath = urlInScriptTagOrdainer.apply(staticPath);
ImmutableMap.Builder<String, Object> data = ImmutableMap.builder();
- // TODO(taoalpha): Remove once p2 fully rolled out
- data.put("polymer2", "true");
-
if (canonicalPath != null) {
data.put("canonicalPath", canonicalPath);
}
diff --git a/java/com/google/gerrit/index/IndexType.java b/java/com/google/gerrit/index/IndexType.java
index a92b734..ee44deb 100644
--- a/java/com/google/gerrit/index/IndexType.java
+++ b/java/com/google/gerrit/index/IndexType.java
@@ -52,10 +52,6 @@
return type.equals(ELASTICSEARCH);
}
- public static boolean isElasticsearch(String type) {
- return type.toLowerCase().equals(ELASTICSEARCH);
- }
-
@Override
public String toString() {
return type;
diff --git a/java/com/google/gerrit/index/Schema.java b/java/com/google/gerrit/index/Schema.java
index ed7ae30..f9f8c48 100644
--- a/java/com/google/gerrit/index/Schema.java
+++ b/java/com/google/gerrit/index/Schema.java
@@ -34,6 +34,7 @@
public static class Builder<T> {
private final List<FieldDef<T, ?>> fields = new ArrayList<>();
+ private boolean useLegacyNumericFields;
public Builder<T> add(Schema<T> schema) {
this.fields.addAll(schema.getFields().values());
@@ -52,8 +53,13 @@
return this;
}
+ public Builder<T> legacyNumericFields(boolean useLegacyNumericFields) {
+ this.useLegacyNumericFields = useLegacyNumericFields;
+ return this;
+ }
+
public Schema<T> build() {
- return new Schema<>(ImmutableList.copyOf(fields));
+ return new Schema<>(useLegacyNumericFields, ImmutableList.copyOf(fields));
}
}
@@ -82,14 +88,15 @@
private final ImmutableMap<String, FieldDef<T, ?>> fields;
private final ImmutableMap<String, FieldDef<T, ?>> storedFields;
+ private final boolean useLegacyNumericFields;
private int version;
- public Schema(Iterable<FieldDef<T, ?>> fields) {
- this(0, fields);
+ public Schema(boolean useLegacyNumericFields, Iterable<FieldDef<T, ?>> fields) {
+ this(0, useLegacyNumericFields, fields);
}
- public Schema(int version, Iterable<FieldDef<T, ?>> fields) {
+ public Schema(int version, boolean useLegacyNumericFields, Iterable<FieldDef<T, ?>> fields) {
this.version = version;
ImmutableMap.Builder<String, FieldDef<T, ?>> b = ImmutableMap.builder();
ImmutableMap.Builder<String, FieldDef<T, ?>> sb = ImmutableMap.builder();
@@ -101,12 +108,17 @@
}
this.fields = b.build();
this.storedFields = sb.build();
+ this.useLegacyNumericFields = useLegacyNumericFields;
}
public final int getVersion() {
return version;
}
+ public final boolean useLegacyNumericFields() {
+ return useLegacyNumericFields;
+ }
+
/**
* Get all fields in this schema.
*
diff --git a/java/com/google/gerrit/index/SchemaUtil.java b/java/com/google/gerrit/index/SchemaUtil.java
index c59f251..9599d6a 100644
--- a/java/com/google/gerrit/index/SchemaUtil.java
+++ b/java/com/google/gerrit/index/SchemaUtil.java
@@ -67,12 +67,19 @@
}
public static <V> Schema<V> schema(Collection<FieldDef<V, ?>> fields) {
- return new Schema<>(ImmutableList.copyOf(fields));
+ return new Schema<>(true, ImmutableList.copyOf(fields));
+ }
+
+ public static <V> Schema<V> schema(Schema<V> schema, boolean useLegacyNumericFields) {
+ return new Schema<>(
+ useLegacyNumericFields,
+ new ImmutableList.Builder<FieldDef<V, ?>>().addAll(schema.getFields().values()).build());
}
@SafeVarargs
public static <V> Schema<V> schema(Schema<V> schema, FieldDef<V, ?>... moreFields) {
return new Schema<>(
+ true,
new ImmutableList.Builder<FieldDef<V, ?>>()
.addAll(schema.getFields().values())
.addAll(ImmutableList.copyOf(moreFields))
@@ -81,7 +88,7 @@
@SafeVarargs
public static <V> Schema<V> schema(FieldDef<V, ?>... fields) {
- return schema(ImmutableList.copyOf(fields));
+ return new Schema<>(true, ImmutableList.copyOf(fields));
}
public static Set<String> getPersonParts(PersonIdent person) {
diff --git a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
index 7a0430c..deb3203 100644
--- a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
+++ b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
@@ -63,8 +63,10 @@
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field;
import org.apache.lucene.document.Field.Store;
+import org.apache.lucene.document.IntPoint;
import org.apache.lucene.document.LegacyIntField;
import org.apache.lucene.document.LegacyLongField;
+import org.apache.lucene.document.LongPoint;
import org.apache.lucene.document.StoredField;
import org.apache.lucene.document.StringField;
import org.apache.lucene.document.TextField;
@@ -334,15 +336,23 @@
if (type == FieldType.INTEGER || type == FieldType.INTEGER_RANGE) {
for (Object value : values.getValues()) {
- doc.add(new LegacyIntField(name, (Integer) value, store));
+ Integer intValue = (Integer) value;
+ if (schema.useLegacyNumericFields()) {
+ doc.add(new LegacyIntField(name, intValue, store));
+ } else {
+ doc.add(new IntPoint(name, intValue));
+ if (store == Store.YES) {
+ doc.add(new StoredField(name, intValue));
+ }
+ }
}
} else if (type == FieldType.LONG) {
for (Object value : values.getValues()) {
- doc.add(new LegacyLongField(name, (Long) value, store));
+ addLongField(doc, name, store, (Long) value);
}
} else if (type == FieldType.TIMESTAMP) {
for (Object value : values.getValues()) {
- doc.add(new LegacyLongField(name, ((Timestamp) value).getTime(), store));
+ addLongField(doc, name, store, ((Timestamp) value).getTime());
}
} else if (type == FieldType.EXACT || type == FieldType.PREFIX) {
for (Object value : values.getValues()) {
@@ -361,6 +371,17 @@
}
}
+ private void addLongField(Document doc, String name, Store store, Long longValue) {
+ if (schema.useLegacyNumericFields()) {
+ doc.add(new LegacyLongField(name, longValue, store));
+ } else {
+ doc.add(new LongPoint(name, longValue));
+ if (store == Store.YES) {
+ doc.add(new StoredField(name, longValue));
+ }
+ }
+ }
+
protected FieldBundle toFieldBundle(Document doc) {
Map<String, FieldDef<V, ?>> allFields = getSchema().getFields();
ListMultimap<String, Object> rawFields = ArrayListMultimap.create();
diff --git a/java/com/google/gerrit/lucene/ChangeSubIndex.java b/java/com/google/gerrit/lucene/ChangeSubIndex.java
index 778c0e5..fd439f1 100644
--- a/java/com/google/gerrit/lucene/ChangeSubIndex.java
+++ b/java/com/google/gerrit/lucene/ChangeSubIndex.java
@@ -15,6 +15,7 @@
package com.google.gerrit.lucene;
import static com.google.common.collect.Iterables.getOnlyElement;
+import static com.google.gerrit.lucene.LuceneChangeIndex.ID2_SORT_FIELD;
import static com.google.gerrit.lucene.LuceneChangeIndex.ID_SORT_FIELD;
import static com.google.gerrit.lucene.LuceneChangeIndex.UPDATED_SORT_FIELD;
import static com.google.gerrit.server.index.change.ChangeSchemaDefinitions.NAME;
@@ -99,6 +100,9 @@
if (f == ChangeField.LEGACY_ID) {
int v = (Integer) getOnlyElement(values.getValues());
doc.add(new NumericDocValuesField(ID_SORT_FIELD, v));
+ } else if (f == ChangeField.LEGACY_ID_STR) {
+ String v = (String) getOnlyElement(values.getValues());
+ doc.add(new NumericDocValuesField(ID2_SORT_FIELD, Integer.valueOf(v)));
} else if (f == ChangeField.UPDATED) {
long t = ((Timestamp) getOnlyElement(values.getValues())).getTime();
doc.add(new NumericDocValuesField(UPDATED_SORT_FIELD, t));
diff --git a/java/com/google/gerrit/lucene/LuceneAccountIndex.java b/java/com/google/gerrit/lucene/LuceneAccountIndex.java
index ccbe8a3..efd7ea3 100644
--- a/java/com/google/gerrit/lucene/LuceneAccountIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneAccountIndex.java
@@ -17,6 +17,7 @@
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.gerrit.server.index.account.AccountField.FULL_NAME;
import static com.google.gerrit.server.index.account.AccountField.ID;
+import static com.google.gerrit.server.index.account.AccountField.ID_STR;
import static com.google.gerrit.server.index.account.AccountField.PREFERRED_EMAIL_EXACT;
import com.google.gerrit.entities.Account;
@@ -60,13 +61,18 @@
private static final String FULL_NAME_SORT_FIELD = sortFieldName(FULL_NAME);
private static final String EMAIL_SORT_FIELD = sortFieldName(PREFERRED_EMAIL_EXACT);
private static final String ID_SORT_FIELD = sortFieldName(ID);
+ private static final String ID2_SORT_FIELD = sortFieldName(ID_STR);
- private static Term idTerm(AccountState as) {
- return idTerm(as.account().id());
+ private static Term idTerm(boolean useLegacyNumericFields, AccountState as) {
+ return idTerm(useLegacyNumericFields, as.account().id());
}
- private static Term idTerm(Account.Id id) {
- return QueryBuilder.intTerm(ID.getName(), id.get());
+ private static Term idTerm(boolean useLegacyNumericFields, Account.Id id) {
+ FieldDef<AccountState, ?> idField = useLegacyNumericFields ? ID : ID_STR;
+ if (useLegacyNumericFields) {
+ return QueryBuilder.intTerm(idField.getName(), id.get());
+ }
+ return QueryBuilder.stringTerm(idField.getName(), Integer.toString(id.get()));
}
private final GerritIndexWriterConfig indexWriterConfig;
@@ -110,6 +116,9 @@
if (f == ID) {
int v = (Integer) getOnlyElement(values.getValues());
doc.add(new NumericDocValuesField(ID_SORT_FIELD, v));
+ } else if (f == ID_STR) {
+ String v = (String) getOnlyElement(values.getValues());
+ doc.add(new NumericDocValuesField(ID2_SORT_FIELD, Integer.valueOf(v)));
} else if (f == FULL_NAME) {
String value = (String) getOnlyElement(values.getValues());
doc.add(new SortedDocValuesField(FULL_NAME_SORT_FIELD, new BytesRef(value)));
@@ -123,7 +132,7 @@
@Override
public void replace(AccountState as) {
try {
- replace(idTerm(as), toDocument(as)).get();
+ replace(idTerm(getSchema().useLegacyNumericFields(), as), toDocument(as)).get();
} catch (ExecutionException | InterruptedException e) {
throw new StorageException(e);
}
@@ -132,7 +141,7 @@
@Override
public void delete(Account.Id key) {
try {
- delete(idTerm(key)).get();
+ delete(idTerm(getSchema().useLegacyNumericFields(), key)).get();
} catch (ExecutionException | InterruptedException e) {
throw new StorageException(e);
}
@@ -141,20 +150,29 @@
@Override
public DataSource<AccountState> getSource(Predicate<AccountState> p, QueryOptions opts)
throws QueryParseException {
+ queryBuilder.getSchema().useLegacyNumericFields();
return new LuceneQuerySource(
- opts.filterFields(IndexUtils::accountFields), queryBuilder.toQuery(p), getSort());
+ opts.filterFields(o -> IndexUtils.accountFields(o, getSchema().useLegacyNumericFields())),
+ queryBuilder.toQuery(p),
+ getSort());
}
private Sort getSort() {
+ String idSortField = getSchema().useLegacyNumericFields() ? ID_SORT_FIELD : ID2_SORT_FIELD;
return new Sort(
new SortField(FULL_NAME_SORT_FIELD, SortField.Type.STRING, false),
new SortField(EMAIL_SORT_FIELD, SortField.Type.STRING, false),
- new SortField(ID_SORT_FIELD, SortField.Type.LONG, false));
+ new SortField(idSortField, SortField.Type.LONG, false));
}
@Override
protected AccountState fromDocument(Document doc) {
- Account.Id id = Account.id(doc.getField(ID.getName()).numericValue().intValue());
+ FieldDef<AccountState, ?> idField = getSchema().useLegacyNumericFields() ? ID : ID_STR;
+ Account.Id id =
+ Account.id(
+ getSchema().useLegacyNumericFields()
+ ? doc.getField(idField.getName()).numericValue().intValue()
+ : Integer.valueOf(doc.getField(idField.getName()).stringValue()));
// Use the AccountCache rather than depending on any stored fields in the document (of which
// there shouldn't be any). The most expensive part to compute anyway is the effective group
// IDs, and we don't have a good way to reindex when those change.
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index 80adaf1..16d66b6 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -18,6 +18,7 @@
import static com.google.gerrit.lucene.AbstractLuceneIndex.sortFieldName;
import static com.google.gerrit.server.git.QueueProvider.QueueType.INTERACTIVE;
import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID;
+import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID_STR;
import static com.google.gerrit.server.index.change.ChangeField.PROJECT;
import static com.google.gerrit.server.index.change.ChangeIndexRewriter.CLOSED_STATUSES;
import static com.google.gerrit.server.index.change.ChangeIndexRewriter.OPEN_STATUSES;
@@ -44,6 +45,7 @@
import com.google.gerrit.entities.converter.PatchSetProtoConverter;
import com.google.gerrit.entities.converter.ProtoConverter;
import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.query.FieldBundle;
@@ -106,6 +108,7 @@
static final String UPDATED_SORT_FIELD = sortFieldName(ChangeField.UPDATED);
static final String ID_SORT_FIELD = sortFieldName(ChangeField.LEGACY_ID);
+ static final String ID2_SORT_FIELD = sortFieldName(ChangeField.LEGACY_ID_STR);
private static final String CHANGES = "changes";
private static final String CHANGES_OPEN = "open";
@@ -134,12 +137,22 @@
private static final String UNRESOLVED_COMMENT_COUNT_FIELD =
ChangeField.UNRESOLVED_COMMENT_COUNT.getName();
- static Term idTerm(ChangeData cd) {
- return QueryBuilder.intTerm(LEGACY_ID.getName(), cd.getId().get());
+ @FunctionalInterface
+ static interface IdTerm {
+ Term get(String name, int id);
}
- static Term idTerm(Change.Id id) {
- return QueryBuilder.intTerm(LEGACY_ID.getName(), id.get());
+ static Term idTerm(IdTerm idTerm, FieldDef<ChangeData, ?> idField, ChangeData cd) {
+ return idTerm(idTerm, idField, cd.getId());
+ }
+
+ static Term idTerm(IdTerm idTerm, FieldDef<ChangeData, ?> idField, Change.Id id) {
+ return idTerm.get(idField.getName(), id.get());
+ }
+
+ @FunctionalInterface
+ static interface ChangeIdExtractor {
+ Change.Id extract(IndexableField f);
}
private final ListeningExecutorService executor;
@@ -149,6 +162,12 @@
private final ChangeSubIndex openIndex;
private final ChangeSubIndex closedIndex;
+ // TODO(davido): Remove the below fields when support for legacy numeric fields is removed.
+ private final FieldDef<ChangeData, ?> idField;
+ private final String idSortFieldName;
+ private final IdTerm idTerm;
+ private final ChangeIdExtractor extractor;
+
@Inject
LuceneChangeIndex(
@GerritServerConfig Config cfg,
@@ -183,6 +202,20 @@
new ChangeSubIndex(
schema, sitePaths, dir.resolve(CHANGES_CLOSED), closedConfig, searcherFactory);
}
+
+ idField = this.schema.useLegacyNumericFields() ? LEGACY_ID : LEGACY_ID_STR;
+ idSortFieldName = schema.useLegacyNumericFields() ? ID_SORT_FIELD : ID2_SORT_FIELD;
+ idTerm =
+ (name, id) ->
+ this.schema.useLegacyNumericFields()
+ ? QueryBuilder.intTerm(name, id)
+ : QueryBuilder.stringTerm(name, Integer.toString(id));
+ extractor =
+ (f) ->
+ Change.id(
+ this.schema.useLegacyNumericFields()
+ ? f.numericValue().intValue()
+ : Integer.valueOf(f.stringValue()));
}
@Override
@@ -201,7 +234,7 @@
@Override
public void replace(ChangeData cd) {
- Term id = LuceneChangeIndex.idTerm(cd);
+ Term id = LuceneChangeIndex.idTerm(idTerm, idField, cd);
// toDocument is essentially static and doesn't depend on the specific
// sub-index, so just pick one.
Document doc = openIndex.toDocument(cd);
@@ -217,10 +250,10 @@
}
@Override
- public void delete(Change.Id id) {
- Term idTerm = LuceneChangeIndex.idTerm(id);
+ public void delete(Change.Id changeId) {
+ Term id = LuceneChangeIndex.idTerm(idTerm, idField, changeId);
try {
- Futures.allAsList(openIndex.delete(idTerm), closedIndex.delete(idTerm)).get();
+ Futures.allAsList(openIndex.delete(id), closedIndex.delete(id)).get();
} catch (ExecutionException | InterruptedException e) {
throw new StorageException(e);
}
@@ -256,7 +289,7 @@
private Sort getSort() {
return new Sort(
new SortField(UPDATED_SORT_FIELD, SortField.Type.LONG, true),
- new SortField(ID_SORT_FIELD, SortField.Type.LONG, true));
+ new SortField(idSortFieldName, SortField.Type.LONG, true));
}
private class QuerySource implements ChangeDataSource {
@@ -304,7 +337,7 @@
throw new StorageException("interrupted");
}
- final Set<String> fields = IndexUtils.changeFields(opts);
+ final Set<String> fields = IndexUtils.changeFields(opts, schema.useLegacyNumericFields());
return new ChangeDataResults(
executor.submit(
new Callable<List<Document>>() {
@@ -325,7 +358,7 @@
public ResultSet<FieldBundle> readRaw() {
List<Document> documents;
try {
- documents = doRead(IndexUtils.changeFields(opts));
+ documents = doRead(IndexUtils.changeFields(opts, schema.useLegacyNumericFields()));
} catch (IOException e) {
throw new StorageException(e);
}
@@ -403,9 +436,8 @@
List<Document> docs = future.get();
ImmutableList.Builder<ChangeData> result =
ImmutableList.builderWithExpectedSize(docs.size());
- String idFieldName = LEGACY_ID.getName();
for (Document doc : docs) {
- result.add(toChangeData(fields(doc, fields), fields, idFieldName));
+ result.add(toChangeData(fields(doc, fields), fields, idField.getName()));
}
return result.build();
} catch (InterruptedException e) {
@@ -446,10 +478,10 @@
cd = changeDataFactory.create(parseProtoFrom(proto, ChangeProtoConverter.INSTANCE));
} else {
IndexableField f = Iterables.getFirst(doc.get(idFieldName), null);
- Change.Id id = Change.id(f.numericValue().intValue());
+
// IndexUtils#changeFields ensures either CHANGE or PROJECT is always present.
IndexableField project = doc.get(PROJECT.getName()).iterator().next();
- cd = changeDataFactory.create(Project.nameKey(project.stringValue()), id);
+ cd = changeDataFactory.create(Project.nameKey(project.stringValue()), extractor.extract(f));
}
// Any decoding that is done here must also be done in {@link ElasticChangeIndex}.
diff --git a/java/com/google/gerrit/lucene/QueryBuilder.java b/java/com/google/gerrit/lucene/QueryBuilder.java
index ce5ba98..e8ef95f 100644
--- a/java/com/google/gerrit/lucene/QueryBuilder.java
+++ b/java/com/google/gerrit/lucene/QueryBuilder.java
@@ -36,6 +36,8 @@
import java.util.Date;
import java.util.List;
import org.apache.lucene.analysis.Analyzer;
+import org.apache.lucene.document.IntPoint;
+import org.apache.lucene.document.LongPoint;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.LegacyNumericRangeQuery;
@@ -49,6 +51,21 @@
@SuppressWarnings("deprecation")
public class QueryBuilder<V> {
+ @FunctionalInterface
+ static interface IntTermQuery {
+ Query get(String name, int value);
+ }
+
+ @FunctionalInterface
+ static interface IntRangeQuery {
+ Query get(String name, int min, int max);
+ }
+
+ @FunctionalInterface
+ static interface LongRangeQuery {
+ Query get(String name, long min, long max);
+ }
+
static Term intTerm(String name, int value) {
BytesRefBuilder builder = new BytesRefBuilder();
LegacyNumericUtils.intToPrefixCoded(value, 0, builder);
@@ -61,12 +78,36 @@
return new Term(name, builder.get());
}
+ static Query intPoint(String name, int value) {
+ return IntPoint.newExactQuery(name, value);
+ }
+
private final Schema<V> schema;
private final org.apache.lucene.util.QueryBuilder queryBuilder;
+ // TODO(davido): Remove the below fields when support for legacy numeric fields is removed.
+ private final IntTermQuery intTermQuery;
+ private final IntRangeQuery intRangeTermQuery;
+ private final LongRangeQuery longRangeQuery;
+
public QueryBuilder(Schema<V> schema, Analyzer analyzer) {
this.schema = schema;
queryBuilder = new org.apache.lucene.util.QueryBuilder(analyzer);
+ intTermQuery =
+ (name, value) ->
+ this.schema.useLegacyNumericFields()
+ ? new TermQuery(intTerm(name, value))
+ : intPoint(name, value);
+ intRangeTermQuery =
+ (name, min, max) ->
+ this.schema.useLegacyNumericFields()
+ ? LegacyNumericRangeQuery.newIntRange(name, min, max, true, true)
+ : IntPoint.newRangeQuery(name, min, max);
+ longRangeQuery =
+ (name, min, max) ->
+ this.schema.useLegacyNumericFields()
+ ? LegacyNumericRangeQuery.newLongRange(name, min, max, true, true)
+ : LongPoint.newRangeQuery(name, min, max);
}
public Query toQuery(Predicate<V> p) throws QueryParseException {
@@ -169,20 +210,20 @@
} catch (NumberFormatException e) {
throw new QueryParseException("not an integer: " + p.getValue());
}
- return new TermQuery(intTerm(p.getField().getName(), value));
+ return intTermQuery.get(p.getField().getName(), value);
}
private Query intRangeQuery(IndexPredicate<V> p) throws QueryParseException {
if (p instanceof IntegerRangePredicate) {
IntegerRangePredicate<V> r = (IntegerRangePredicate<V>) p;
+ String name = r.getField().getName();
int minimum = r.getMinimumValue();
int maximum = r.getMaximumValue();
if (minimum == maximum) {
// Just fall back to a standard integer query.
- return new TermQuery(intTerm(p.getField().getName(), minimum));
+ return intTermQuery.get(name, minimum);
}
- return LegacyNumericRangeQuery.newIntRange(
- r.getField().getName(), minimum, maximum, true, true);
+ return intRangeTermQuery.get(name, minimum, maximum);
}
throw new QueryParseException("not an integer range: " + p);
}
@@ -190,20 +231,16 @@
private Query timestampQuery(IndexPredicate<V> p) throws QueryParseException {
if (p instanceof TimestampRangePredicate) {
TimestampRangePredicate<V> r = (TimestampRangePredicate<V>) p;
- return LegacyNumericRangeQuery.newLongRange(
- r.getField().getName(),
- r.getMinTimestamp().getTime(),
- r.getMaxTimestamp().getTime(),
- true,
- true);
+ return longRangeQuery.get(
+ r.getField().getName(), r.getMinTimestamp().getTime(), r.getMaxTimestamp().getTime());
}
throw new QueryParseException("not a timestamp: " + p);
}
private Query notTimestamp(TimestampRangePredicate<V> r) throws QueryParseException {
if (r.getMinTimestamp().getTime() == 0) {
- return LegacyNumericRangeQuery.newLongRange(
- r.getField().getName(), r.getMaxTimestamp().getTime(), null, true, true);
+ return longRangeQuery.get(
+ r.getField().getName(), r.getMaxTimestamp().getTime(), Long.MAX_VALUE);
}
throw new QueryParseException("cannot negate: " + r);
}
@@ -245,4 +282,8 @@
public int toIndexTimeInMinutes(Date ts) {
return (int) (ts.getTime() / 60000);
}
+
+ public Schema<V> getSchema() {
+ return schema;
+ }
}
diff --git a/java/com/google/gerrit/pgm/Init.java b/java/com/google/gerrit/pgm/Init.java
index 0537fe9..799377c 100644
--- a/java/com/google/gerrit/pgm/Init.java
+++ b/java/com/google/gerrit/pgm/Init.java
@@ -32,6 +32,7 @@
import com.google.gerrit.server.config.SitePath;
import com.google.gerrit.server.ioutil.HostPlatform;
import com.google.gerrit.server.securestore.SecureStoreClassName;
+import com.google.gerrit.server.util.ReplicaUtil;
import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.google.inject.Inject;
@@ -144,7 +145,7 @@
});
modules.add(new GerritServerConfigModule());
Guice.createInjector(modules).injectMembers(this);
- if (!run.flags.cfg.getBoolean("container", "slave", false)) {
+ if (!ReplicaUtil.isReplica(run.flags.cfg)) {
reindexProjects();
}
start(run);
diff --git a/java/com/google/gerrit/pgm/Reindex.java b/java/com/google/gerrit/pgm/Reindex.java
index 3cc9315..2e526bb 100644
--- a/java/com/google/gerrit/pgm/Reindex.java
+++ b/java/com/google/gerrit/pgm/Reindex.java
@@ -34,6 +34,7 @@
import com.google.gerrit.server.index.IndexModule;
import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
import com.google.gerrit.server.plugins.PluginGuiceEnvironment;
+import com.google.gerrit.server.util.ReplicaUtil;
import com.google.inject.Inject;
import com.google.inject.Injector;
import com.google.inject.Key;
@@ -144,14 +145,15 @@
if (changesVersion != null) {
versions.put(ChangeSchemaDefinitions.INSTANCE.getName(), changesVersion);
}
- boolean slave = globalConfig.getBoolean("container", "slave", false);
+ boolean replica = ReplicaUtil.isReplica(globalConfig);
List<Module> modules = new ArrayList<>();
Module indexModule;
IndexType indexType = IndexModule.getIndexType(dbInjector);
if (indexType.isLucene()) {
- indexModule = LuceneIndexModule.singleVersionWithExplicitVersions(versions, threads, slave);
+ indexModule = LuceneIndexModule.singleVersionWithExplicitVersions(versions, threads, replica);
} else if (indexType.isElasticsearch()) {
- indexModule = ElasticIndexModule.singleVersionWithExplicitVersions(versions, threads, slave);
+ indexModule =
+ ElasticIndexModule.singleVersionWithExplicitVersions(versions, threads, replica);
} else {
throw new IllegalStateException("unsupported index.type = " + indexType);
}
diff --git a/java/com/google/gerrit/server/AssigneeStatusUpdate.java b/java/com/google/gerrit/server/AssigneeStatusUpdate.java
new file mode 100644
index 0000000..0da5edf
--- /dev/null
+++ b/java/com/google/gerrit/server/AssigneeStatusUpdate.java
@@ -0,0 +1,21 @@
+package com.google.gerrit.server;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.entities.Account;
+import java.sql.Timestamp;
+import java.util.Optional;
+
+/** Change to an assignee's status. */
+@AutoValue
+public abstract class AssigneeStatusUpdate {
+ public static AssigneeStatusUpdate create(
+ Timestamp ts, Account.Id updatedBy, Optional<Account.Id> currentAssignee) {
+ return new AutoValue_AssigneeStatusUpdate(ts, updatedBy, currentAssignee);
+ }
+
+ public abstract Timestamp date();
+
+ public abstract Account.Id updatedBy();
+
+ public abstract Optional<Account.Id> currentAssignee();
+}
diff --git a/java/com/google/gerrit/server/CommentsUtil.java b/java/com/google/gerrit/server/CommentsUtil.java
index c858e1a..ae4ba4b 100644
--- a/java/com/google/gerrit/server/CommentsUtil.java
+++ b/java/com/google/gerrit/server/CommentsUtil.java
@@ -19,7 +19,6 @@
import static java.util.stream.Collectors.toList;
import com.google.common.collect.ComparisonChain;
-import com.google.common.collect.FluentIterable;
import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
import com.google.gerrit.common.Nullable;
@@ -27,7 +26,6 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.Patch;
-import com.google.gerrit.entities.PatchLineComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.entities.RobotComment;
@@ -259,8 +257,7 @@
return sort(comments);
}
- public void putComments(
- ChangeUpdate update, PatchLineComment.Status status, Iterable<Comment> comments) {
+ public void putComments(ChangeUpdate update, Comment.Status status, Iterable<Comment> comments) {
for (Comment c : comments) {
update.putComment(status, c);
}
@@ -353,15 +350,4 @@
comments.sort(COMMENT_ORDER);
return comments;
}
-
- public static Iterable<PatchLineComment> toPatchLineComments(
- Change.Id changeId, PatchLineComment.Status status, Iterable<Comment> comments) {
- return FluentIterable.from(comments).transform(c -> PatchLineComment.from(changeId, status, c));
- }
-
- public static List<Comment> toComments(
- final String serverId, Iterable<PatchLineComment> comments) {
- return COMMENT_ORDER.sortedCopy(
- FluentIterable.from(comments).transform(plc -> plc.asComment(serverId)));
- }
}
diff --git a/java/com/google/gerrit/server/PublishCommentUtil.java b/java/com/google/gerrit/server/PublishCommentUtil.java
index 6ebebcf..c446c92 100644
--- a/java/com/google/gerrit/server/PublishCommentUtil.java
+++ b/java/com/google/gerrit/server/PublishCommentUtil.java
@@ -15,12 +15,13 @@
package com.google.gerrit.server;
import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.gerrit.entities.PatchLineComment.Status.PUBLISHED;
import static java.util.stream.Collectors.toSet;
import com.google.common.collect.ImmutableList;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.Comment.Status;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.validators.CommentForValidation;
@@ -34,10 +35,14 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.util.Collection;
+import java.util.HashSet;
import java.util.Map;
+import java.util.Set;
@Singleton
public class PublishCommentUtil {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
private final PatchListCache patchListCache;
private final PatchSetUtil psUtil;
private final CommentsUtil commentsUtil;
@@ -51,32 +56,58 @@
}
public void publish(
- ChangeContext ctx, PatchSet.Id psId, Collection<Comment> drafts, @Nullable String tag) {
+ ChangeContext ctx,
+ PatchSet.Id psId,
+ Collection<Comment> draftComments,
+ @Nullable String tag) {
ChangeNotes notes = ctx.getNotes();
checkArgument(notes != null);
- if (drafts.isEmpty()) {
+ if (draftComments.isEmpty()) {
return;
}
Map<PatchSet.Id, PatchSet> patchSets =
- psUtil.getAsMap(notes, drafts.stream().map(d -> psId(notes, d)).collect(toSet()));
- for (Comment d : drafts) {
- PatchSet ps = patchSets.get(psId(notes, d));
+ psUtil.getAsMap(notes, draftComments.stream().map(d -> psId(notes, d)).collect(toSet()));
+ Set<Comment> commentsToPublish = new HashSet<>();
+ for (Comment draftComment : draftComments) {
+ PatchSet.Id psIdOfDraftComment = psId(notes, draftComment);
+ PatchSet ps = patchSets.get(psIdOfDraftComment);
if (ps == null) {
- throw new StorageException("patch set " + ps + " not found");
+ // This can happen if changes with the same numeric ID exist:
+ // - change 12345 has 3 patch sets in repo X
+ // - another change 12345 has 7 patch sets in repo Y
+ // - the user saves a draft comment on patch set 6 of the change in repo Y
+ // - this draft comment gets stored in:
+ // AllUsers -> refs/draft-comments/45/12345/<account-id>
+ // - when posting a review with draft handling PUBLISH_ALL_REVISIONS on the change in
+ // repo X, the draft comments are loaded from
+ // AllUsers -> refs/draft-comments/45/12345/<account-id>, including the draft
+ // comment that was saved for patch set 6 of the change in repo Y
+ // - patch set 6 does not exist for the change in repo x, hence we get null for the patch
+ // set here
+ // Instead of failing hard (and returning an Internal Server Error) to the caller,
+ // just ignore that comment.
+ // Gerrit ensures that numeric change IDs are unique, but you can get duplicates if
+ // change refs of one repo are copied/pushed to another repo on the same host (this
+ // should never be done, but we know it happens).
+ logger.atWarning().log(
+ "Ignoring draft comment %s on non existing patch set %s (repo = %s)",
+ draftComment, psIdOfDraftComment, notes.getProjectName());
+ continue;
}
- d.writtenOn = ctx.getWhen();
- d.tag = tag;
+ draftComment.writtenOn = ctx.getWhen();
+ draftComment.tag = tag;
// Draft may have been created by a different real user; copy the current real user. (Only
// applies to X-Gerrit-RunAs, since modifying drafts via on_behalf_of is not allowed.)
- ctx.getUser().updateRealAccountId(d::setRealAuthor);
+ ctx.getUser().updateRealAccountId(draftComment::setRealAuthor);
try {
- CommentsUtil.setCommentCommitId(d, patchListCache, notes.getChange(), ps);
+ CommentsUtil.setCommentCommitId(draftComment, patchListCache, notes.getChange(), ps);
} catch (PatchListNotAvailableException e) {
throw new StorageException(e);
}
+ commentsToPublish.add(draftComment);
}
- commentsUtil.putComments(ctx.getUpdate(psId), PUBLISHED, drafts);
+ commentsUtil.putComments(ctx.getUpdate(psId), Status.PUBLISHED, commentsToPublish);
}
private static PatchSet.Id psId(ChangeNotes notes, Comment c) {
diff --git a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index a04be30..d0af686 100644
--- a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -50,6 +50,7 @@
import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.common.MergePatchSetInput;
import com.google.gerrit.extensions.common.PureRevertInfo;
+import com.google.gerrit.extensions.common.RevertSubmissionInfo;
import com.google.gerrit.extensions.common.RobotCommentInfo;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
import com.google.gerrit.extensions.registration.DynamicMap;
@@ -96,6 +97,7 @@
import com.google.gerrit.server.restapi.change.Rebase;
import com.google.gerrit.server.restapi.change.Restore;
import com.google.gerrit.server.restapi.change.Revert;
+import com.google.gerrit.server.restapi.change.RevertSubmission;
import com.google.gerrit.server.restapi.change.Reviewers;
import com.google.gerrit.server.restapi.change.Revisions;
import com.google.gerrit.server.restapi.change.SetReadyForReview;
@@ -132,6 +134,7 @@
private final ChangeResource change;
private final Abandon abandon;
private final Revert revert;
+ private final RevertSubmission revertSubmission;
private final Restore restore;
private final CreateMergePatchSet updateByMerge;
private final Provider<SubmittedTogether> submittedTogether;
@@ -181,6 +184,7 @@
ListReviewers listReviewers,
Abandon abandon,
Revert revert,
+ RevertSubmission revertSubmission,
Restore restore,
CreateMergePatchSet updateByMerge,
Provider<SubmittedTogether> submittedTogether,
@@ -219,6 +223,7 @@
@Assisted ChangeResource change) {
this.changeApi = changeApi;
this.revert = revert;
+ this.revertSubmission = revertSubmission;
this.reviewers = reviewers;
this.revisions = revisions;
this.reviewerApi = reviewerApi;
@@ -358,6 +363,15 @@
}
@Override
+ public RevertSubmissionInfo revertSubmission(RevertInput in) throws RestApiException {
+ try {
+ return revertSubmission.apply(change, in).value();
+ } catch (Exception e) {
+ throw asRestApiException("Cannot revert a change submission", e);
+ }
+ }
+
+ @Override
public ChangeInfo createMergePatchSet(MergePatchSetInput in) throws RestApiException {
try {
return updateByMerge.apply(change, in).value();
diff --git a/java/com/google/gerrit/server/change/AbandonUtil.java b/java/com/google/gerrit/server/change/AbandonUtil.java
index cb89440..1bc1fad 100644
--- a/java/com/google/gerrit/server/change/AbandonUtil.java
+++ b/java/com/google/gerrit/server/change/AbandonUtil.java
@@ -40,7 +40,10 @@
private final ChangeCleanupConfig cfg;
private final Provider<ChangeQueryProcessor> queryProvider;
- private final ChangeQueryBuilder queryBuilder;
+ // Provider is needed, because AbandonUtil is singleton, but ChangeQueryBuilder accesses
+ // index collection, that is only provided when multiversion index module is started.
+ // TODO(davido); Remove provider again, when support for legacy numeric fields is removed.
+ private final Provider<ChangeQueryBuilder> queryBuilderProvider;
private final BatchAbandon batchAbandon;
private final InternalUser internalUser;
@@ -49,11 +52,11 @@
ChangeCleanupConfig cfg,
InternalUser.Factory internalUserFactory,
Provider<ChangeQueryProcessor> queryProvider,
- ChangeQueryBuilder queryBuilder,
+ Provider<ChangeQueryBuilder> queryBuilderProvider,
BatchAbandon batchAbandon) {
this.cfg = cfg;
this.queryProvider = queryProvider;
- this.queryBuilder = queryBuilder;
+ this.queryBuilderProvider = queryBuilderProvider;
this.batchAbandon = batchAbandon;
internalUser = internalUserFactory.create();
}
@@ -71,7 +74,11 @@
}
List<ChangeData> changesToAbandon =
- queryProvider.get().enforceVisibility(false).query(queryBuilder.parse(query)).entities();
+ queryProvider
+ .get()
+ .enforceVisibility(false)
+ .query(queryBuilderProvider.get().parse(query))
+ .entities();
ImmutableListMultimap.Builder<Project.NameKey, ChangeData> builder =
ImmutableListMultimap.builder();
for (ChangeData cd : changesToAbandon) {
@@ -111,7 +118,7 @@
queryProvider
.get()
.enforceVisibility(false)
- .query(queryBuilder.parse(newQuery))
+ .query(queryBuilderProvider.get().parse(newQuery))
.entities();
if (!changesToAbandon.isEmpty()) {
validChanges.add(cd);
diff --git a/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index afaf695..c05a47d 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -17,12 +17,14 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.RawInput;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.git.LockFailureException;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
@@ -228,7 +230,12 @@
createCommit(repository, basePatchSetCommit, baseTree, newCommitMessage, nowTimestamp);
if (optionalChangeEdit.isPresent()) {
- updateEdit(repository, optionalChangeEdit.get(), newEditCommit, nowTimestamp);
+ updateEdit(
+ notes.getProjectName(),
+ repository,
+ optionalChangeEdit.get(),
+ newEditCommit,
+ nowTimestamp);
} else {
createEdit(repository, notes, basePatchSet, newEditCommit, nowTimestamp);
}
@@ -331,7 +338,12 @@
createCommit(repository, basePatchSetCommit, newTreeId, commitMessage, nowTimestamp);
if (optionalChangeEdit.isPresent()) {
- updateEdit(repository, optionalChangeEdit.get(), newEditCommit, nowTimestamp);
+ updateEdit(
+ notes.getProjectName(),
+ repository,
+ optionalChangeEdit.get(),
+ newEditCommit,
+ nowTimestamp);
} else {
createEdit(repository, notes, basePatchSet, newEditCommit, nowTimestamp);
}
@@ -384,7 +396,12 @@
createCommit(repository, patchSetCommit, newTreeId, commitMessage, nowTimestamp);
if (optionalChangeEdit.isPresent()) {
- return updateEdit(repository, optionalChangeEdit.get(), newEditCommit, nowTimestamp);
+ return updateEdit(
+ notes.getProjectName(),
+ repository,
+ optionalChangeEdit.get(),
+ newEditCommit,
+ nowTimestamp);
}
return createEdit(repository, notes, patchSet, newEditCommit, nowTimestamp);
}
@@ -531,7 +548,13 @@
throws IOException {
Change change = notes.getChange();
String editRefName = getEditRefName(change, basePatchSet);
- updateReference(repository, editRefName, ObjectId.zeroId(), newEditCommitId, timestamp);
+ updateReference(
+ notes.getProjectName(),
+ repository,
+ editRefName,
+ ObjectId.zeroId(),
+ newEditCommitId,
+ timestamp);
reindex(change);
RevCommit newEditCommit = lookupCommit(repository, newEditCommitId);
@@ -544,11 +567,16 @@
}
private ChangeEdit updateEdit(
- Repository repository, ChangeEdit changeEdit, ObjectId newEditCommitId, Timestamp timestamp)
+ Project.NameKey projectName,
+ Repository repository,
+ ChangeEdit changeEdit,
+ ObjectId newEditCommitId,
+ Timestamp timestamp)
throws IOException {
String editRefName = changeEdit.getRefName();
RevCommit currentEditCommit = changeEdit.getEditCommit();
- updateReference(repository, editRefName, currentEditCommit, newEditCommitId, timestamp);
+ updateReference(
+ projectName, repository, editRefName, currentEditCommit, newEditCommitId, timestamp);
reindex(changeEdit.getChange());
RevCommit newEditCommit = lookupCommit(repository, newEditCommitId);
@@ -557,6 +585,7 @@
}
private void updateReference(
+ Project.NameKey projectName,
Repository repository,
String refName,
ObjectId currentObjectId,
@@ -571,14 +600,12 @@
ru.setForceUpdate(true);
try (RevWalk revWalk = new RevWalk(repository)) {
RefUpdate.Result res = ru.update(revWalk);
+ String message = "cannot update " + ru.getName() + " in " + projectName + ": " + res;
+ if (res == RefUpdate.Result.LOCK_FAILURE) {
+ throw new LockFailureException(message, ru);
+ }
if (res != RefUpdate.Result.NEW && res != RefUpdate.Result.FORCED) {
- throw new IOException(
- "cannot update "
- + ru.getName()
- + " in "
- + repository.getDirectory()
- + ": "
- + ru.getResult());
+ throw new IOException(message);
}
}
}
diff --git a/java/com/google/gerrit/server/index/IndexUtils.java b/java/com/google/gerrit/server/index/IndexUtils.java
index 4b5cd49..a45777e 100644
--- a/java/com/google/gerrit/server/index/IndexUtils.java
+++ b/java/com/google/gerrit/server/index/IndexUtils.java
@@ -16,18 +16,21 @@
import static com.google.gerrit.server.index.change.ChangeField.CHANGE;
import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID;
+import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID_STR;
import static com.google.gerrit.server.index.change.ChangeField.PROJECT;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.project.ProjectField;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.index.account.AccountField;
import com.google.gerrit.server.index.group.GroupField;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.SingleGroupUser;
import java.io.IOException;
import java.util.Set;
@@ -56,17 +59,18 @@
}
}
- public static Set<String> accountFields(QueryOptions opts) {
- return accountFields(opts.fields());
+ public static Set<String> accountFields(QueryOptions opts, boolean useLegacyNumericFields) {
+ return accountFields(opts.fields(), useLegacyNumericFields);
}
- public static Set<String> accountFields(Set<String> fields) {
- return fields.contains(AccountField.ID.getName())
- ? fields
- : Sets.union(fields, ImmutableSet.of(AccountField.ID.getName()));
+ public static Set<String> accountFields(Set<String> fields, boolean useLegacyNumericFields) {
+ String idFieldName =
+ useLegacyNumericFields ? AccountField.ID.getName() : AccountField.ID_STR.getName();
+ return fields.contains(idFieldName) ? fields : Sets.union(fields, ImmutableSet.of(idFieldName));
}
- public static Set<String> changeFields(QueryOptions opts) {
+ public static Set<String> changeFields(QueryOptions opts, boolean useLegacyNumericFields) {
+ FieldDef<ChangeData, ?> idField = useLegacyNumericFields ? LEGACY_ID : LEGACY_ID_STR;
// Ensure we request enough fields to construct a ChangeData. We need both
// change ID and project, which can either come via the Change field or
// separate fields.
@@ -75,10 +79,10 @@
// A Change is always sufficient.
return fs;
}
- if (fs.contains(PROJECT.getName()) && fs.contains(LEGACY_ID.getName())) {
+ if (fs.contains(PROJECT.getName()) && fs.contains(idField.getName())) {
return fs;
}
- return Sets.union(fs, ImmutableSet.of(LEGACY_ID.getName(), PROJECT.getName()));
+ return Sets.union(fs, ImmutableSet.of(idField.getName(), PROJECT.getName()));
}
public static Set<String> groupFields(QueryOptions opts) {
diff --git a/java/com/google/gerrit/server/index/account/AccountField.java b/java/com/google/gerrit/server/index/account/AccountField.java
index ae94283..0dd22ce 100644
--- a/java/com/google/gerrit/server/index/account/AccountField.java
+++ b/java/com/google/gerrit/server/index/account/AccountField.java
@@ -46,6 +46,9 @@
public static final FieldDef<AccountState, Integer> ID =
integer("id").stored().build(a -> a.account().id().get());
+ public static final FieldDef<AccountState, String> ID_STR =
+ exact("id_str").stored().build(a -> String.valueOf(a.account().id().get()));
+
/**
* External IDs.
*
diff --git a/java/com/google/gerrit/server/index/account/AccountIndex.java b/java/com/google/gerrit/server/index/account/AccountIndex.java
index 153bf17..1838edf 100644
--- a/java/com/google/gerrit/server/index/account/AccountIndex.java
+++ b/java/com/google/gerrit/server/index/account/AccountIndex.java
@@ -27,6 +27,6 @@
@Override
default Predicate<AccountState> keyPredicate(Account.Id id) {
- return AccountPredicates.id(id);
+ return AccountPredicates.id(getSchema(), id);
}
}
diff --git a/java/com/google/gerrit/server/index/account/AccountSchemaDefinitions.java b/java/com/google/gerrit/server/index/account/AccountSchemaDefinitions.java
index c41814f..157e290 100644
--- a/java/com/google/gerrit/server/index/account/AccountSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/account/AccountSchemaDefinitions.java
@@ -49,7 +49,18 @@
@Deprecated static final Schema<AccountState> V9 = schema(V8);
// Lucene index was changed to add additional fields for sorting.
- static final Schema<AccountState> V10 = schema(V9);
+ @Deprecated static final Schema<AccountState> V10 = schema(V9);
+
+ // New numeric types: use dimensional points using the k-d tree geo-spatial data structure
+ // to offer fast single- and multi-dimensional numeric range. As the consequense, integer
+ // document id type is replaced with string document id type.
+ static final Schema<AccountState> V11 =
+ new Schema.Builder<AccountState>()
+ .add(V10)
+ .remove(AccountField.ID)
+ .add(AccountField.ID_STR)
+ .legacyNumericFields(false)
+ .build();
public static final String NAME = "accounts";
public static final AccountSchemaDefinitions INSTANCE = new AccountSchemaDefinitions();
diff --git a/java/com/google/gerrit/server/index/account/StalenessChecker.java b/java/com/google/gerrit/server/index/account/StalenessChecker.java
index fc86c8d..aad9527 100644
--- a/java/com/google/gerrit/server/index/account/StalenessChecker.java
+++ b/java/com/google/gerrit/server/index/account/StalenessChecker.java
@@ -61,6 +61,12 @@
AccountField.REF_STATE.getName(),
AccountField.EXTERNAL_ID_STATE.getName());
+ public static final ImmutableSet<String> FIELDS2 =
+ ImmutableSet.of(
+ AccountField.ID_STR.getName(),
+ AccountField.REF_STATE.getName(),
+ AccountField.EXTERNAL_ID_STATE.getName());
+
private final AccountIndexCollection indexes;
private final GitRepositoryManager repoManager;
private final AllUsersName allUsersName;
@@ -93,8 +99,13 @@
return false;
}
+ boolean useLegacyNumericFields = i.getSchema().useLegacyNumericFields();
+ ImmutableSet<String> fields = useLegacyNumericFields ? FIELDS : FIELDS2;
Optional<FieldBundle> result =
- i.getRaw(id, QueryOptions.create(indexConfig, 0, 1, IndexUtils.accountFields(FIELDS)));
+ i.getRaw(
+ id,
+ QueryOptions.create(
+ indexConfig, 0, 1, IndexUtils.accountFields(fields, useLegacyNumericFields)));
if (!result.isPresent()) {
// The document is missing in the index.
try (Repository repo = repoManager.openRepository(allUsersName)) {
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index c62b4df..07e9b9e4 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -107,6 +107,9 @@
public static final FieldDef<ChangeData, Integer> LEGACY_ID =
integer("legacy_id").stored().build(cd -> cd.getId().get());
+ public static final FieldDef<ChangeData, String> LEGACY_ID_STR =
+ exact("legacy_id_str").stored().build(cd -> String.valueOf(cd.getId().get()));
+
/** Newer style Change-Id key. */
public static final FieldDef<ChangeData, String> ID =
prefix(ChangeQueryBuilder.FIELD_CHANGE_ID).build(changeGetter(c -> c.getKey().get()));
diff --git a/java/com/google/gerrit/server/index/change/ChangeIndex.java b/java/com/google/gerrit/server/index/change/ChangeIndex.java
index 1a01417..29bff0a 100644
--- a/java/com/google/gerrit/server/index/change/ChangeIndex.java
+++ b/java/com/google/gerrit/server/index/change/ChangeIndex.java
@@ -20,6 +20,7 @@
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.LegacyChangeIdPredicate;
+import com.google.gerrit.server.query.change.LegacyChangeIdStrPredicate;
public interface ChangeIndex extends Index<Change.Id, ChangeData> {
public interface Factory
@@ -27,6 +28,8 @@
@Override
default Predicate<ChangeData> keyPredicate(Change.Id id) {
- return new LegacyChangeIdPredicate(id);
+ return getSchema().useLegacyNumericFields()
+ ? new LegacyChangeIdPredicate(id)
+ : new LegacyChangeIdStrPredicate(id);
}
}
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index bf93edb..72153c4 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -85,7 +85,18 @@
ChangeField.WIP);
// The computation of the 'extension' field is changed, hence reindexing is required.
- static final Schema<ChangeData> V56 = schema(V55);
+ @Deprecated static final Schema<ChangeData> V56 = schema(V55);
+
+ // New numeric types: use dimensional points using the k-d tree geo-spatial data structure
+ // to offer fast single- and multi-dimensional numeric range. As the consequense, integer
+ // document id type is replaced with string document id type.
+ static final Schema<ChangeData> V57 =
+ new Schema.Builder<ChangeData>()
+ .add(V56)
+ .remove(ChangeField.LEGACY_ID)
+ .add(ChangeField.LEGACY_ID_STR)
+ .legacyNumericFields(false)
+ .build();
public static final String NAME = "changes";
public static final ChangeSchemaDefinitions INSTANCE = new ChangeSchemaDefinitions();
diff --git a/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java b/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java
index 6d0f3b6..e64e2eb 100644
--- a/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java
@@ -43,7 +43,11 @@
@Deprecated static final Schema<InternalGroup> V6 = schema(V5);
// Lucene index was changed to add an additional field for sorting.
- static final Schema<InternalGroup> V7 = schema(V6);
+ @Deprecated static final Schema<InternalGroup> V7 = schema(V6);
+
+ // New numeric types: use dimensional points using the k-d tree geo-spatial data structure
+ // to offer fast single- and multi-dimensional numeric range.
+ static final Schema<InternalGroup> V8 = schema(V7, false);
public static final GroupSchemaDefinitions INSTANCE = new GroupSchemaDefinitions();
diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
index 81d5381..158db1c 100644
--- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java
+++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
@@ -24,7 +24,6 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
-import com.google.gerrit.entities.PatchLineComment.Status;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
@@ -335,7 +334,7 @@
persistentCommentFromMailComment(ctx, c, targetPatchSetForComment(ctx, c, patchSet)));
}
commentsUtil.putComments(
- ctx.getUpdate(ctx.getChange().currentPatchSetId()), Status.PUBLISHED, comments);
+ ctx.getUpdate(ctx.getChange().currentPatchSetId()), Comment.Status.PUBLISHED, comments);
return true;
}
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
index dbd1abb..0acf20e 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
@@ -50,7 +50,6 @@
public final ChangeNoteJson changeNoteJson;
public final GitRepositoryManager repoManager;
public final AllUsersName allUsers;
- public final LegacyChangeNoteRead legacyChangeNoteRead;
public final NoteDbMetrics metrics;
public final String serverId;
@@ -64,14 +63,12 @@
GitRepositoryManager repoManager,
AllUsersName allUsers,
ChangeNoteJson changeNoteJson,
- LegacyChangeNoteRead legacyChangeNoteRead,
NoteDbMetrics metrics,
Provider<ChangeNotesCache> cache,
@GerritServerId String serverId) {
this.failOnLoadForTest = new AtomicBoolean();
this.repoManager = repoManager;
this.allUsers = allUsers;
- this.legacyChangeNoteRead = legacyChangeNoteRead;
this.changeNoteJson = changeNoteJson;
this.metrics = metrics;
this.cache = cache;
diff --git a/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java b/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
index d0819d7..877022e 100644
--- a/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
@@ -23,7 +23,6 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
-import com.google.gerrit.entities.PatchLineComment;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.StorageException;
@@ -265,12 +264,7 @@
// Even though reading from changes might not be enabled, we need to
// parse any existing revision notes so we can merge them.
return RevisionNoteMap.parse(
- noteUtil.getChangeNoteJson(),
- noteUtil.getLegacyChangeNoteRead(),
- getId(),
- rw.getObjectReader(),
- noteMap,
- PatchLineComment.Status.DRAFT);
+ noteUtil.getChangeNoteJson(), rw.getObjectReader(), noteMap, Comment.Status.DRAFT);
}
@Override
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
index 7aa3bc8..b221ef5 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
@@ -63,22 +63,13 @@
static final String UNRESOLVED = "Unresolved";
static final String TAG = FOOTER_TAG.getName();
- private final LegacyChangeNoteRead legacyChangeNoteRead;
private final ChangeNoteJson changeNoteJson;
private final String serverId;
@Inject
- public ChangeNoteUtil(
- ChangeNoteJson changeNoteJson,
- LegacyChangeNoteRead legacyChangeNoteRead,
- @GerritServerId String serverId) {
+ public ChangeNoteUtil(ChangeNoteJson changeNoteJson, @GerritServerId String serverId) {
this.serverId = serverId;
this.changeNoteJson = changeNoteJson;
- this.legacyChangeNoteRead = legacyChangeNoteRead;
- }
-
- public LegacyChangeNoteRead getLegacyChangeNoteRead() {
- return legacyChangeNoteRead;
}
public ChangeNoteJson getChangeNoteJson() {
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index 384eebc..8cf0046 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -16,6 +16,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.entities.RefNames.changeMetaRef;
import static java.util.Comparator.comparing;
import static java.util.Objects.requireNonNull;
@@ -29,6 +30,7 @@
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.ListMultimap;
+import com.google.common.collect.Lists;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.Multimaps;
import com.google.common.collect.Ordering;
@@ -48,6 +50,7 @@
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.entities.RobotComment;
import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.server.AssigneeStatusUpdate;
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
@@ -366,9 +369,24 @@
return state.reviewerUpdates();
}
- /** @return an ImmutableSet of Account.Ids of all users that have been assigned to this change. */
+ /**
+ * @return an ImmutableSet of Account.Ids of all users that have been assigned to this change. The
+ * order of the set is the order in which they were assigned.
+ */
public ImmutableSet<Account.Id> getPastAssignees() {
- return state.pastAssignees();
+ return Lists.reverse(state.assigneeUpdates()).stream()
+ .map(AssigneeStatusUpdate::currentAssignee)
+ .filter(Optional::isPresent)
+ .map(Optional::get)
+ .collect(toImmutableSet());
+ }
+
+ /**
+ * @return an ImmutableList of AssigneeStatusUpdate of all the updates to the assignee field to
+ * this change. The order of the list is from most recent updates to least recent.
+ */
+ public ImmutableList<AssigneeStatusUpdate> getAssigneeUpdates() {
+ return state.assigneeUpdates();
}
/** @return a ImmutableSet of all hashtags for this change sorted in alphabetical order. */
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
index 03073d2..7fde297 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
@@ -61,7 +61,7 @@
.weigher(Weigher.class)
.maximumWeight(10 << 20)
.diskLimit(-1)
- .version(1)
+ .version(2)
.keySerializer(Key.Serializer.INSTANCE)
.valueSerializer(ChangeNotesState.Serializer.INSTANCE);
}
@@ -148,11 +148,8 @@
+ str(state.columns().originalSubject())
+ P
+ str(state.columns().submissionId())
- + ptr(state.columns().assignee(), K) // assignee
+ P // status
+ P
- + set(state.pastAssignees(), K)
- + P
+ set(state.hashtags(), str(10))
+ P
+ list(state.patchSets(), patchSet())
@@ -169,6 +166,8 @@
+ P
+ list(state.reviewerUpdates(), 4 * O + K + K + P)
+ P
+ + list(state.assigneeUpdates(), 4 * O + K + K)
+ + P
+ list(state.submitRecords(), P + list(2, str(4) + P + K) + P)
+ P
+ list(state.changeMessages(), changeMessage())
@@ -180,10 +179,6 @@
+ I; // updateCount
}
- private static int ptr(Object o, int size) {
- return o != null ? P + size : P;
- }
-
private static int str(String s) {
if (s == null) {
return P;
@@ -355,12 +350,7 @@
"Load change notes for change %s of project %s", key.changeId(), key.project());
ChangeNotesParser parser =
new ChangeNotesParser(
- key.changeId(),
- key.id(),
- walkSupplier.get(),
- args.changeNoteJson,
- args.legacyChangeNoteRead,
- args.metrics);
+ key.changeId(), key.id(), walkSupplier.get(), args.changeNoteJson, args.metrics);
ChangeNotesState result = parser.parseAll();
// This assignment only happens if call() was actually called, which only
// happens when Cache#get(K, Callable<V>) incurs a cache miss.
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index c4c227e..9c45aaf 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -61,12 +61,12 @@
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.LabelId;
-import com.google.gerrit.entities.PatchLineComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.mail.Address;
import com.google.gerrit.metrics.Timer1;
+import com.google.gerrit.server.AssigneeStatusUpdate;
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
@@ -102,7 +102,6 @@
// Private final members initialized in the constructor.
private final ChangeNoteJson changeNoteJson;
- private final LegacyChangeNoteRead legacyChangeNoteRead;
private final NoteDbMetrics metrics;
private final Change.Id id;
@@ -115,6 +114,7 @@
private final Table<Address, ReviewerStateInternal, Timestamp> reviewersByEmail;
private final List<Account.Id> allPastReviewers;
private final List<ReviewerStatusUpdate> reviewerUpdates;
+ private final List<AssigneeStatusUpdate> assigneeUpdates;
private final List<SubmitRecord> submitRecords;
private final ListMultimap<ObjectId, Comment> comments;
private final Map<PatchSet.Id, PatchSet.Builder> patchSets;
@@ -129,8 +129,6 @@
private String branch;
private Change.Status status;
private String topic;
- private Optional<Account.Id> assignee;
- private List<Account.Id> pastAssignees;
private Set<String> hashtags;
private Timestamp createdOn;
private Timestamp lastUpdatedOn;
@@ -156,13 +154,11 @@
ObjectId tip,
ChangeNotesRevWalk walk,
ChangeNoteJson changeNoteJson,
- LegacyChangeNoteRead legacyChangeNoteRead,
NoteDbMetrics metrics) {
this.id = changeId;
this.tip = tip;
this.walk = walk;
this.changeNoteJson = changeNoteJson;
- this.legacyChangeNoteRead = legacyChangeNoteRead;
this.metrics = metrics;
approvals = new LinkedHashMap<>();
bufferedApprovals = new ArrayList<>();
@@ -172,6 +168,7 @@
pendingReviewersByEmail = ReviewerByEmailSet.empty();
allPastReviewers = new ArrayList<>();
reviewerUpdates = new ArrayList<>();
+ assigneeUpdates = new ArrayList<>();
submitRecords = Lists.newArrayListWithExpectedSize(1);
allChangeMessages = new ArrayList<>();
comments = MultimapBuilder.hashKeys().arrayListValues().build();
@@ -231,9 +228,7 @@
topic,
originalSubject,
submissionId,
- assignee != null ? assignee.orElse(null) : null,
status,
- Sets.newLinkedHashSet(Lists.reverse(pastAssignees)),
firstNonNull(hashtags, ImmutableSet.of()),
buildPatchSets(),
buildApprovals(),
@@ -243,6 +238,7 @@
pendingReviewersByEmail,
allPastReviewers,
buildReviewerUpdates(),
+ assigneeUpdates,
submitRecords,
buildAllMessages(),
comments,
@@ -361,7 +357,7 @@
}
parseHashtags(commit);
- parseAssignee(commit);
+ parseAssigneeUpdates(ts, commit);
if (submissionId == null) {
submissionId = parseSubmissionId(commit);
@@ -444,7 +440,7 @@
return effectiveAccountId;
}
PersonIdent ident = RawParseUtils.parsePersonIdent(realUser);
- return legacyChangeNoteRead.parseIdent(ident, id);
+ return parseIdent(ident);
}
private String parseTopic(ChangeNotesCommit commit) throws ConfigInvalidException {
@@ -566,10 +562,8 @@
}
}
- private void parseAssignee(ChangeNotesCommit commit) throws ConfigInvalidException {
- if (pastAssignees == null) {
- pastAssignees = Lists.newArrayList();
- }
+ private void parseAssigneeUpdates(Timestamp ts, ChangeNotesCommit commit)
+ throws ConfigInvalidException {
String assigneeValue = parseOneFooter(commit, FOOTER_ASSIGNEE);
if (assigneeValue != null) {
Optional<Account.Id> parsedAssignee;
@@ -578,14 +572,9 @@
parsedAssignee = Optional.empty();
} else {
PersonIdent ident = RawParseUtils.parsePersonIdent(assigneeValue);
- parsedAssignee = Optional.ofNullable(legacyChangeNoteRead.parseIdent(ident, id));
+ parsedAssignee = Optional.ofNullable(parseIdent(ident));
}
- if (assignee == null) {
- assignee = parsedAssignee;
- }
- if (parsedAssignee.isPresent()) {
- pastAssignees.add(parsedAssignee.get());
- }
+ assigneeUpdates.add(AssigneeStatusUpdate.create(ts, ownerId, parsedAssignee));
}
}
@@ -716,12 +705,7 @@
ChangeNotesCommit tipCommit = walk.parseCommit(tip);
revisionNoteMap =
RevisionNoteMap.parse(
- changeNoteJson,
- legacyChangeNoteRead,
- id,
- reader,
- NoteMap.read(reader, tipCommit),
- PatchLineComment.Status.PUBLISHED);
+ changeNoteJson, reader, NoteMap.read(reader, tipCommit), Comment.Status.PUBLISHED);
Map<ObjectId, ChangeRevisionNote> rns = revisionNoteMap.revisionNotes;
for (Map.Entry<ObjectId, ChangeRevisionNote> e : rns.entrySet()) {
@@ -780,7 +764,7 @@
labelVoteStr = line.substring(0, s);
PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1));
checkFooter(ident != null, FOOTER_LABEL, line);
- effectiveAccountId = legacyChangeNoteRead.parseIdent(ident, id);
+ effectiveAccountId = parseIdent(ident);
} else {
labelVoteStr = line;
effectiveAccountId = committerId;
@@ -819,7 +803,7 @@
label = line.substring(1, s);
PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1));
checkFooter(ident != null, FOOTER_LABEL, line);
- effectiveAccountId = legacyChangeNoteRead.parseIdent(ident, id);
+ effectiveAccountId = parseIdent(ident);
} else {
label = line.substring(1);
effectiveAccountId = committerId;
@@ -878,7 +862,7 @@
label.label = line.substring(c + 2, c2);
PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(c2 + 2));
checkFooter(ident != null, FOOTER_SUBMITTED_WITH, line);
- label.appliedBy = legacyChangeNoteRead.parseIdent(ident, id);
+ label.appliedBy = parseIdent(ident);
} else {
label.label = line.substring(c + 2);
}
@@ -894,7 +878,7 @@
if (a.getName().equals(c.getName()) && a.getEmailAddress().equals(c.getEmailAddress())) {
return null;
}
- return legacyChangeNoteRead.parseIdent(commit.getAuthorIdent(), id);
+ return parseIdent(commit.getAuthorIdent());
}
private void parseReviewer(Timestamp ts, ReviewerStateInternal state, String line)
@@ -903,7 +887,7 @@
if (ident == null) {
throw invalidFooter(state.getFooterKey(), line);
}
- Account.Id accountId = legacyChangeNoteRead.parseIdent(ident, id);
+ Account.Id accountId = parseIdent(ident);
reviewerUpdates.add(ReviewerStatusUpdate.create(ts, ownerId, accountId, state));
if (!reviewers.containsRow(accountId)) {
reviewers.put(accountId, state, ts);
@@ -1103,4 +1087,10 @@
private ConfigInvalidException parseException(String fmt, Object... args) {
return ChangeNotes.parseException(id, fmt, args);
}
+
+ private Account.Id parseIdent(PersonIdent ident) throws ConfigInvalidException {
+ return NoteDbUtil.parseIdent(ident)
+ .orElseThrow(
+ () -> parseException("cannot retrieve account id: %s", ident.getEmailAddress()));
+ }
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index 5396508..896cca3 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -18,7 +18,6 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableListMultimap.toImmutableListMultimap;
-import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.Objects.requireNonNull;
import com.google.auto.value.AutoValue;
@@ -50,10 +49,12 @@
import com.google.gerrit.json.OutputFormat;
import com.google.gerrit.mail.Address;
import com.google.gerrit.proto.Protos;
+import com.google.gerrit.server.AssigneeStatusUpdate;
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto;
+import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AssigneeStatusUpdateProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ChangeColumnsProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerSetEntryProto;
@@ -67,6 +68,7 @@
import java.sql.Timestamp;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import org.eclipse.jgit.lib.ObjectId;
@@ -103,9 +105,7 @@
@Nullable String topic,
@Nullable String originalSubject,
@Nullable String submissionId,
- @Nullable Account.Id assignee,
@Nullable Change.Status status,
- Set<Account.Id> pastAssignees,
Set<String> hashtags,
Map<PatchSet.Id, PatchSet> patchSets,
ListMultimap<PatchSet.Id, PatchSetApproval> approvals,
@@ -115,6 +115,7 @@
ReviewerByEmailSet pendingReviewersByEmail,
List<Account.Id> allPastReviewers,
List<ReviewerStatusUpdate> reviewerUpdates,
+ List<AssigneeStatusUpdate> assigneeUpdates,
List<SubmitRecord> submitRecords,
List<ChangeMessage> changeMessages,
ListMultimap<ObjectId, Comment> publishedComments,
@@ -147,13 +148,11 @@
.topic(topic)
.originalSubject(originalSubject)
.submissionId(submissionId)
- .assignee(assignee)
.isPrivate(isPrivate)
.workInProgress(workInProgress)
.reviewStarted(reviewStarted)
.revertOf(revertOf)
.build())
- .pastAssignees(pastAssignees)
.hashtags(hashtags)
.serverId(serverId)
.patchSets(patchSets.entrySet())
@@ -164,6 +163,7 @@
.pendingReviewersByEmail(pendingReviewersByEmail)
.allPastReviewers(allPastReviewers)
.reviewerUpdates(reviewerUpdates)
+ .assigneeUpdates(assigneeUpdates)
.submitRecords(submitRecords)
.changeMessages(changeMessages)
.publishedComments(publishedComments)
@@ -211,9 +211,6 @@
@Nullable
abstract String submissionId();
- @Nullable
- abstract Account.Id assignee();
-
abstract boolean isPrivate();
abstract boolean workInProgress();
@@ -247,8 +244,6 @@
abstract Builder submissionId(@Nullable String submissionId);
- abstract Builder assignee(@Nullable Account.Id assignee);
-
abstract Builder status(@Nullable Change.Status status);
abstract Builder isPrivate(boolean isPrivate);
@@ -274,8 +269,6 @@
abstract ChangeColumns columns();
// Other related to this Change.
- abstract ImmutableSet<Account.Id> pastAssignees();
-
abstract ImmutableSet<String> hashtags();
@Nullable
@@ -297,6 +290,8 @@
abstract ImmutableList<ReviewerStatusUpdate> reviewerUpdates();
+ abstract ImmutableList<AssigneeStatusUpdate> assigneeUpdates();
+
abstract ImmutableList<SubmitRecord> submitRecords();
abstract ImmutableList<ChangeMessage> changeMessages();
@@ -339,7 +334,9 @@
change.setTopic(Strings.emptyToNull(c.topic()));
change.setLastUpdatedOn(c.lastUpdatedOn());
change.setSubmissionId(c.submissionId());
- change.setAssignee(c.assignee());
+ if (!assigneeUpdates().isEmpty()) {
+ change.setAssignee(assigneeUpdates().get(0).currentAssignee().orElse(null));
+ }
change.setPrivate(c.isPrivate());
change.setWorkInProgress(c.workInProgress());
change.setReviewStarted(c.reviewStarted());
@@ -359,7 +356,6 @@
static Builder empty(Change.Id changeId) {
return new AutoValue_ChangeNotesState.Builder()
.changeId(changeId)
- .pastAssignees(ImmutableSet.of())
.hashtags(ImmutableSet.of())
.patchSets(ImmutableList.of())
.approvals(ImmutableList.of())
@@ -369,6 +365,7 @@
.pendingReviewersByEmail(ReviewerByEmailSet.empty())
.allPastReviewers(ImmutableList.of())
.reviewerUpdates(ImmutableList.of())
+ .assigneeUpdates(ImmutableList.of())
.submitRecords(ImmutableList.of())
.changeMessages(ImmutableList.of())
.publishedComments(ImmutableListMultimap.of())
@@ -383,8 +380,6 @@
abstract Builder serverId(String serverId);
- abstract Builder pastAssignees(Set<Account.Id> pastAssignees);
-
abstract Builder hashtags(Iterable<String> hashtags);
abstract Builder patchSets(Iterable<Map.Entry<PatchSet.Id, PatchSet>> patchSets);
@@ -403,6 +398,8 @@
abstract Builder reviewerUpdates(List<ReviewerStatusUpdate> reviewerUpdates);
+ abstract Builder assigneeUpdates(List<AssigneeStatusUpdate> assigneeUpdates);
+
abstract Builder submitRecords(List<SubmitRecord> submitRecords);
abstract Builder changeMessages(List<ChangeMessage> changeMessages);
@@ -438,7 +435,6 @@
b.setServerId(object.serverId());
b.setHasServerId(true);
}
- object.pastAssignees().forEach(a -> b.addPastAssignee(a.get()));
object.hashtags().forEach(b::addHashtag);
object
.patchSets()
@@ -469,6 +465,7 @@
object.allPastReviewers().forEach(a -> b.addPastReviewer(a.get()));
object.reviewerUpdates().forEach(u -> b.addReviewerUpdate(toReviewerStatusUpdateProto(u)));
+ object.assigneeUpdates().forEach(u -> b.addAssigneeUpdate(toAssigneeStatusUpdateProto(u)));
object
.submitRecords()
.forEach(r -> b.addSubmitRecord(GSON.toJson(new StoredSubmitRecord(r))));
@@ -508,9 +505,6 @@
if (cols.submissionId() != null) {
b.setSubmissionId(cols.submissionId()).setHasSubmissionId(true);
}
- if (cols.assignee() != null) {
- b.setAssignee(cols.assignee().get()).setHasAssignee(true);
- }
if (cols.status() != null) {
b.setStatus(STATUS_CONVERTER.reverse().convert(cols.status())).setHasStatus(true);
}
@@ -550,6 +544,17 @@
.build();
}
+ private static AssigneeStatusUpdateProto toAssigneeStatusUpdateProto(AssigneeStatusUpdate u) {
+ AssigneeStatusUpdateProto.Builder builder =
+ AssigneeStatusUpdateProto.newBuilder()
+ .setDate(u.date().getTime())
+ .setUpdatedBy(u.updatedBy().get())
+ .setHasCurrentAssignee(u.currentAssignee().isPresent());
+
+ u.currentAssignee().ifPresent(assignee -> builder.setCurrentAssignee(assignee.get()));
+ return builder.build();
+ }
+
@Override
public ChangeNotesState deserialize(byte[] in) {
ChangeNotesStateProto proto = Protos.parseUnchecked(ChangeNotesStateProto.parser(), in);
@@ -561,8 +566,6 @@
.changeId(changeId)
.columns(toChangeColumns(changeId, proto.getColumns()))
.serverId(proto.getHasServerId() ? proto.getServerId() : null)
- .pastAssignees(
- proto.getPastAssigneeList().stream().map(Account::id).collect(toImmutableSet()))
.hashtags(proto.getHashtagList())
.patchSets(
proto.getPatchSetList().stream()
@@ -581,6 +584,7 @@
.allPastReviewers(
proto.getPastReviewerList().stream().map(Account::id).collect(toImmutableList()))
.reviewerUpdates(toReviewerStatusUpdateList(proto.getReviewerUpdateList()))
+ .assigneeUpdates(toAssigneeStatusUpdateList(proto.getAssigneeUpdateList()))
.submitRecords(
proto.getSubmitRecordList().stream()
.map(r -> GSON.fromJson(r, StoredSubmitRecord.class).toSubmitRecord())
@@ -624,9 +628,6 @@
if (proto.getHasSubmissionId()) {
b.submissionId(proto.getSubmissionId());
}
- if (proto.getHasAssignee()) {
- b.assignee(Account.id(proto.getAssignee()));
- }
if (proto.getHasStatus()) {
b.status(STATUS_CONVERTER.convert(proto.getStatus()));
}
@@ -677,5 +678,20 @@
}
return b.build();
}
+
+ private static ImmutableList<AssigneeStatusUpdate> toAssigneeStatusUpdateList(
+ List<AssigneeStatusUpdateProto> protos) {
+ ImmutableList.Builder<AssigneeStatusUpdate> b = ImmutableList.builder();
+ for (AssigneeStatusUpdateProto proto : protos) {
+ b.add(
+ AssigneeStatusUpdate.create(
+ new Timestamp(proto.getDate()),
+ Account.id(proto.getUpdatedBy()),
+ proto.getHasCurrentAssignee()
+ ? Optional.of(Account.id(proto.getCurrentAssignee()))
+ : Optional.empty()));
+ }
+ return b.build();
+ }
}
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java b/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
index a9f2357..b6443f1 100644
--- a/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
+++ b/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
@@ -16,10 +16,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
-import com.google.common.primitives.Bytes;
-import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
-import com.google.gerrit.entities.PatchLineComment;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
@@ -30,30 +27,16 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.util.MutableInteger;
-import org.eclipse.jgit.util.RawParseUtils;
class ChangeRevisionNote extends RevisionNote<Comment> {
- private static final byte[] CERT_HEADER = "certificate version ".getBytes(UTF_8);
- // See org.eclipse.jgit.transport.PushCertificateParser.END_SIGNATURE
- private static final byte[] END_SIGNATURE = "-----END PGP SIGNATURE-----\n".getBytes(UTF_8);
-
private final ChangeNoteJson noteJson;
- private final LegacyChangeNoteRead legacyChangeNoteRead;
- private final Change.Id changeId;
- private final PatchLineComment.Status status;
+ private final Comment.Status status;
private String pushCert;
ChangeRevisionNote(
- ChangeNoteJson noteJson,
- LegacyChangeNoteRead legacyChangeNoteRead,
- Change.Id changeId,
- ObjectReader reader,
- ObjectId noteId,
- PatchLineComment.Status status) {
+ ChangeNoteJson noteJson, ObjectReader reader, ObjectId noteId, Comment.Status status) {
super(reader, noteId);
- this.legacyChangeNoteRead = legacyChangeNoteRead;
this.noteJson = noteJson;
- this.changeId = changeId;
this.status = status;
}
@@ -67,29 +50,13 @@
MutableInteger p = new MutableInteger();
p.value = offset;
- if (isJson(raw, p.value)) {
- RevisionNoteData data = parseJson(noteJson, raw, p.value);
- if (status == PatchLineComment.Status.PUBLISHED) {
- pushCert = data.pushCert;
- } else {
- pushCert = null;
- }
- return data.comments;
- }
-
- if (status == PatchLineComment.Status.PUBLISHED) {
- pushCert = parsePushCert(changeId, raw, p);
- trimLeadingEmptyLines(raw, p);
+ RevisionNoteData data = parseJson(noteJson, raw, p.value);
+ if (status == Comment.Status.PUBLISHED) {
+ pushCert = data.pushCert;
} else {
pushCert = null;
}
- List<Comment> comments = legacyChangeNoteRead.parseNote(raw, p, changeId);
- comments.forEach(c -> c.legacyFormat = true);
- return comments;
- }
-
- static boolean isJson(byte[] raw, int offset) {
- return raw[offset] == '{' || raw[offset] == '[';
+ return data.comments;
}
private RevisionNoteData parseJson(ChangeNoteJson noteUtil, byte[] raw, int offset)
@@ -99,18 +66,4 @@
return noteUtil.getGson().fromJson(r, RevisionNoteData.class);
}
}
-
- private static String parsePushCert(Change.Id changeId, byte[] bytes, MutableInteger p)
- throws ConfigInvalidException {
- if (RawParseUtils.match(bytes, p.value, CERT_HEADER) < 0) {
- return null;
- }
- int end = Bytes.indexOf(bytes, END_SIGNATURE);
- if (end < 0) {
- throw ChangeNotes.parseException(changeId, "invalid push certificate in note");
- }
- int start = p.value;
- p.value = end + END_SIGNATURE.length;
- return new String(bytes, start, p.value, UTF_8);
- }
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index c51f7fb..c0cd173 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -53,7 +53,6 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
-import com.google.gerrit.entities.PatchLineComment;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RobotComment;
import com.google.gerrit.exceptions.StorageException;
@@ -273,10 +272,10 @@
this.psDescription = psDescription;
}
- public void putComment(PatchLineComment.Status status, Comment c) {
+ public void putComment(Comment.Status status, Comment c) {
verifyComment(c);
createDraftUpdateIfNull();
- if (status == PatchLineComment.Status.DRAFT) {
+ if (status == Comment.Status.DRAFT) {
draftUpdate.putComment(c);
} else {
comments.add(c);
@@ -462,12 +461,7 @@
// Even though reading from changes might not be enabled, we need to
// parse any existing revision notes so we can merge them.
return RevisionNoteMap.parse(
- noteUtil.getChangeNoteJson(),
- noteUtil.getLegacyChangeNoteRead(),
- getId(),
- rw.getObjectReader(),
- noteMap,
- PatchLineComment.Status.PUBLISHED);
+ noteUtil.getChangeNoteJson(), rw.getObjectReader(), noteMap, Comment.Status.PUBLISHED);
}
private void checkComments(
diff --git a/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java b/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
index 4ec4f20..9c8b369 100644
--- a/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
+++ b/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
@@ -15,7 +15,7 @@
package com.google.gerrit.server.notedb;
import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.gerrit.entities.PatchLineComment.Status.PUBLISHED;
+import static com.google.gerrit.entities.Comment.Status;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toMap;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
@@ -95,13 +95,13 @@
ObjectReader reader = revWalk.getObjectReader();
RevCommit newTipCommit = revWalk.next(); // The first commit will not be rewritten.
Map<String, Comment> parentComments =
- getPublishedComments(noteUtil, changeId, reader, NoteMap.read(reader, newTipCommit));
+ getPublishedComments(noteUtil, reader, NoteMap.read(reader, newTipCommit));
boolean rewrite = false;
RevCommit originalCommit;
while ((originalCommit = revWalk.next()) != null) {
NoteMap noteMap = NoteMap.read(reader, originalCommit);
- Map<String, Comment> currComments = getPublishedComments(noteUtil, changeId, reader, noteMap);
+ Map<String, Comment> currComments = getPublishedComments(noteUtil, reader, noteMap);
if (!rewrite && currComments.containsKey(uuid)) {
rewrite = true;
@@ -131,28 +131,18 @@
*/
@VisibleForTesting
public static Map<String, Comment> getPublishedComments(
- ChangeNoteJson changeNoteJson,
- LegacyChangeNoteRead legacyChangeNoteRead,
- Change.Id changeId,
- ObjectReader reader,
- NoteMap noteMap)
+ ChangeNoteJson changeNoteJson, ObjectReader reader, NoteMap noteMap)
throws IOException, ConfigInvalidException {
- return RevisionNoteMap.parse(
- changeNoteJson, legacyChangeNoteRead, changeId, reader, noteMap, PUBLISHED)
- .revisionNotes.values().stream()
+ return RevisionNoteMap.parse(changeNoteJson, reader, noteMap, Status.PUBLISHED).revisionNotes
+ .values().stream()
.flatMap(n -> n.getEntities().stream())
.collect(toMap(c -> c.key.uuid, Function.identity()));
}
public static Map<String, Comment> getPublishedComments(
- ChangeNoteUtil noteUtil, Change.Id changeId, ObjectReader reader, NoteMap noteMap)
+ ChangeNoteUtil noteUtil, ObjectReader reader, NoteMap noteMap)
throws IOException, ConfigInvalidException {
- return getPublishedComments(
- noteUtil.getChangeNoteJson(),
- noteUtil.getLegacyChangeNoteRead(),
- changeId,
- reader,
- noteMap);
+ return getPublishedComments(noteUtil.getChangeNoteJson(), reader, noteMap);
}
/**
* Gets the comments put in by the current commit. The message of the target comment will be
@@ -215,11 +205,9 @@
RevisionNoteMap<ChangeRevisionNote> revNotesMap =
RevisionNoteMap.parse(
noteUtil.getChangeNoteJson(),
- noteUtil.getLegacyChangeNoteRead(),
- changeId,
reader,
NoteMap.read(reader, parentCommit),
- PUBLISHED);
+ Status.PUBLISHED);
RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(revNotesMap);
for (Comment c : putInComments) {
diff --git a/java/com/google/gerrit/server/notedb/DraftCommentNotes.java b/java/com/google/gerrit/server/notedb/DraftCommentNotes.java
index e473768..3966396 100644
--- a/java/com/google/gerrit/server/notedb/DraftCommentNotes.java
+++ b/java/com/google/gerrit/server/notedb/DraftCommentNotes.java
@@ -27,7 +27,6 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
-import com.google.gerrit.entities.PatchLineComment;
import com.google.gerrit.entities.Project;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -121,12 +120,7 @@
ObjectReader reader = handle.walk().getObjectReader();
revisionNoteMap =
RevisionNoteMap.parse(
- args.changeNoteJson,
- args.legacyChangeNoteRead,
- getChangeId(),
- reader,
- NoteMap.read(reader, tipCommit),
- PatchLineComment.Status.DRAFT);
+ args.changeNoteJson, reader, NoteMap.read(reader, tipCommit), Comment.Status.DRAFT);
ListMultimap<ObjectId, Comment> cs = MultimapBuilder.hashKeys().arrayListValues().build();
for (ChangeRevisionNote rn : revisionNoteMap.revisionNotes.values()) {
for (Comment c : rn.getEntities()) {
diff --git a/java/com/google/gerrit/server/notedb/LegacyChangeNoteRead.java b/java/com/google/gerrit/server/notedb/LegacyChangeNoteRead.java
deleted file mode 100644
index e663149..0000000
--- a/java/com/google/gerrit/server/notedb/LegacyChangeNoteRead.java
+++ /dev/null
@@ -1,399 +0,0 @@
-// Copyright (C) 2018 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.server.notedb;
-
-import static com.google.gerrit.server.notedb.ChangeNotes.parseException;
-import static java.nio.charset.StandardCharsets.UTF_8;
-
-import com.google.common.collect.ImmutableList;
-import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.Comment;
-import com.google.gerrit.entities.CommentRange;
-import com.google.gerrit.entities.PatchSet;
-import com.google.gerrit.server.config.GerritServerId;
-import com.google.inject.Inject;
-import java.sql.Timestamp;
-import java.text.ParseException;
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Locale;
-import java.util.Set;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.util.GitDateParser;
-import org.eclipse.jgit.util.MutableInteger;
-import org.eclipse.jgit.util.QuotedString;
-import org.eclipse.jgit.util.RawParseUtils;
-
-public class LegacyChangeNoteRead {
- private final String serverId;
-
- @Inject
- public LegacyChangeNoteRead(@GerritServerId String serverId) {
- this.serverId = serverId;
- }
-
- public Account.Id parseIdent(PersonIdent ident, Change.Id changeId)
- throws ConfigInvalidException {
- return NoteDbUtil.parseIdent(ident)
- .orElseThrow(
- () ->
- parseException(
- changeId, "cannot retrieve account id: %s", ident.getEmailAddress()));
- }
-
- private static boolean match(byte[] note, MutableInteger p, byte[] expected) {
- int m = RawParseUtils.match(note, p.value, expected);
- return m == p.value + expected.length;
- }
-
- public List<Comment> parseNote(byte[] note, MutableInteger p, Change.Id changeId)
- throws ConfigInvalidException {
- if (p.value >= note.length) {
- return ImmutableList.of();
- }
- Set<Comment.Key> seen = new HashSet<>();
- List<Comment> result = new ArrayList<>();
- int sizeOfNote = note.length;
- byte[] psb = ChangeNoteUtil.PATCH_SET.getBytes(UTF_8);
- byte[] bpsb = ChangeNoteUtil.BASE_PATCH_SET.getBytes(UTF_8);
- byte[] bpn = ChangeNoteUtil.PARENT_NUMBER.getBytes(UTF_8);
-
- ObjectId commitId =
- ObjectId.fromString(parseStringField(note, p, changeId, ChangeNoteUtil.REVISION));
- String fileName = null;
- PatchSet.Id psId = null;
- boolean isForBase = false;
- Integer parentNumber = null;
-
- while (p.value < sizeOfNote) {
- boolean matchPs = match(note, p, psb);
- boolean matchBase = match(note, p, bpsb);
- if (matchPs) {
- fileName = null;
- psId = parsePsId(note, p, changeId, ChangeNoteUtil.PATCH_SET);
- isForBase = false;
- } else if (matchBase) {
- fileName = null;
- psId = parsePsId(note, p, changeId, ChangeNoteUtil.BASE_PATCH_SET);
- isForBase = true;
- if (match(note, p, bpn)) {
- parentNumber = parseParentNumber(note, p, changeId);
- }
- } else if (psId == null) {
- throw parseException(
- changeId,
- "missing %s or %s header",
- ChangeNoteUtil.PATCH_SET,
- ChangeNoteUtil.BASE_PATCH_SET);
- }
-
- Comment c = parseComment(note, p, fileName, psId, commitId, isForBase, parentNumber);
- fileName = c.key.filename;
- if (!seen.add(c.key)) {
- throw parseException(changeId, "multiple comments for %s in note", c.key);
- }
- result.add(c);
- }
- return result;
- }
-
- private Comment parseComment(
- byte[] note,
- MutableInteger curr,
- String currentFileName,
- PatchSet.Id psId,
- ObjectId commitId,
- boolean isForBase,
- Integer parentNumber)
- throws ConfigInvalidException {
- Change.Id changeId = psId.changeId();
-
- // Check if there is a new file.
- boolean newFile =
- (RawParseUtils.match(note, curr.value, ChangeNoteUtil.FILE.getBytes(UTF_8))) != -1;
- if (newFile) {
- // If so, parse the new file name.
- currentFileName = parseFilename(note, curr, changeId);
- } else if (currentFileName == null) {
- throw parseException(changeId, "could not parse %s", ChangeNoteUtil.FILE);
- }
-
- CommentRange range = parseCommentRange(note, curr);
- if (range == null) {
- throw parseException(changeId, "could not parse %s", ChangeNoteUtil.COMMENT_RANGE);
- }
-
- Timestamp commentTime = parseTimestamp(note, curr, changeId);
- Account.Id aId = parseAuthor(note, curr, changeId, ChangeNoteUtil.AUTHOR);
- boolean hasRealAuthor =
- (RawParseUtils.match(note, curr.value, ChangeNoteUtil.REAL_AUTHOR.getBytes(UTF_8))) != -1;
- Account.Id raId = null;
- if (hasRealAuthor) {
- raId = parseAuthor(note, curr, changeId, ChangeNoteUtil.REAL_AUTHOR);
- }
-
- boolean hasParent =
- (RawParseUtils.match(note, curr.value, ChangeNoteUtil.PARENT.getBytes(UTF_8))) != -1;
- String parentUUID = null;
- boolean unresolved = false;
- if (hasParent) {
- parentUUID = parseStringField(note, curr, changeId, ChangeNoteUtil.PARENT);
- }
- boolean hasUnresolved =
- (RawParseUtils.match(note, curr.value, ChangeNoteUtil.UNRESOLVED.getBytes(UTF_8))) != -1;
- if (hasUnresolved) {
- unresolved = parseBooleanField(note, curr, changeId, ChangeNoteUtil.UNRESOLVED);
- }
-
- String uuid = parseStringField(note, curr, changeId, ChangeNoteUtil.UUID);
-
- boolean hasTag =
- (RawParseUtils.match(note, curr.value, ChangeNoteUtil.TAG.getBytes(UTF_8))) != -1;
- String tag = null;
- if (hasTag) {
- tag = parseStringField(note, curr, changeId, ChangeNoteUtil.TAG);
- }
-
- int commentLength = parseCommentLength(note, curr, changeId);
-
- String message = RawParseUtils.decode(UTF_8, note, curr.value, curr.value + commentLength);
- checkResult(message, "message contents", changeId);
-
- Comment c =
- new Comment(
- new Comment.Key(uuid, currentFileName, psId.get()),
- aId,
- commentTime,
- isForBase ? (short) (parentNumber == null ? 0 : -parentNumber) : (short) 1,
- message,
- serverId,
- unresolved);
- c.lineNbr = range.getEndLine();
- c.parentUuid = parentUUID;
- c.tag = tag;
- c.setCommitId(commitId);
- if (raId != null) {
- c.setRealAuthor(raId);
- }
-
- if (range.getStartCharacter() != -1) {
- c.setRange(range);
- }
-
- curr.value = RawParseUtils.nextLF(note, curr.value + commentLength);
- curr.value = RawParseUtils.nextLF(note, curr.value);
- return c;
- }
-
- private static String parseStringField(
- byte[] note, MutableInteger curr, Change.Id changeId, String fieldName)
- throws ConfigInvalidException {
- int endOfLine = RawParseUtils.nextLF(note, curr.value);
- checkHeaderLineFormat(note, curr, fieldName, changeId);
- int startOfField = RawParseUtils.endOfFooterLineKey(note, curr.value) + 2;
- curr.value = endOfLine;
- return RawParseUtils.decode(UTF_8, note, startOfField, endOfLine - 1);
- }
-
- /**
- * @return a comment range. If the comment range line in the note only has one number, we return a
- * CommentRange with that one number as the end line and the other fields as -1. If the
- * comment range line in the note contains a whole comment range, then we return a
- * CommentRange with all fields set. If the line is not correctly formatted, return null.
- */
- private static CommentRange parseCommentRange(byte[] note, MutableInteger ptr) {
- CommentRange range = new CommentRange(-1, -1, -1, -1);
-
- int last = ptr.value;
- int startLine = RawParseUtils.parseBase10(note, ptr.value, ptr);
- if (ptr.value == last) {
- return null;
- } else if (note[ptr.value] == '\n') {
- range.setEndLine(startLine);
- ptr.value += 1;
- return range;
- } else if (note[ptr.value] == ':') {
- range.setStartLine(startLine);
- ptr.value += 1;
- } else {
- return null;
- }
-
- last = ptr.value;
- int startChar = RawParseUtils.parseBase10(note, ptr.value, ptr);
- if (ptr.value == last) {
- return null;
- } else if (note[ptr.value] == '-') {
- range.setStartCharacter(startChar);
- ptr.value += 1;
- } else {
- return null;
- }
-
- last = ptr.value;
- int endLine = RawParseUtils.parseBase10(note, ptr.value, ptr);
- if (ptr.value == last) {
- return null;
- } else if (note[ptr.value] == ':') {
- range.setEndLine(endLine);
- ptr.value += 1;
- } else {
- return null;
- }
-
- last = ptr.value;
- int endChar = RawParseUtils.parseBase10(note, ptr.value, ptr);
- if (ptr.value == last) {
- return null;
- } else if (note[ptr.value] == '\n') {
- range.setEndCharacter(endChar);
- ptr.value += 1;
- } else {
- return null;
- }
- return range;
- }
-
- private static PatchSet.Id parsePsId(
- byte[] note, MutableInteger curr, Change.Id changeId, String fieldName)
- throws ConfigInvalidException {
- checkHeaderLineFormat(note, curr, fieldName, changeId);
- int startOfPsId = RawParseUtils.endOfFooterLineKey(note, curr.value) + 1;
- MutableInteger i = new MutableInteger();
- int patchSetId = RawParseUtils.parseBase10(note, startOfPsId, i);
- int endOfLine = RawParseUtils.nextLF(note, curr.value);
- if (i.value != endOfLine - 1) {
- throw parseException(changeId, "could not parse %s", fieldName);
- }
- checkResult(patchSetId, "patchset id", changeId);
- curr.value = endOfLine;
- return PatchSet.id(changeId, patchSetId);
- }
-
- private static Integer parseParentNumber(byte[] note, MutableInteger curr, Change.Id changeId)
- throws ConfigInvalidException {
- checkHeaderLineFormat(note, curr, ChangeNoteUtil.PARENT_NUMBER, changeId);
-
- int start = RawParseUtils.endOfFooterLineKey(note, curr.value) + 1;
- MutableInteger i = new MutableInteger();
- int parentNumber = RawParseUtils.parseBase10(note, start, i);
- int endOfLine = RawParseUtils.nextLF(note, curr.value);
- if (i.value != endOfLine - 1) {
- throw parseException(changeId, "could not parse %s", ChangeNoteUtil.PARENT_NUMBER);
- }
- checkResult(parentNumber, "parent number", changeId);
- curr.value = endOfLine;
- return Integer.valueOf(parentNumber);
- }
-
- private static String parseFilename(byte[] note, MutableInteger curr, Change.Id changeId)
- throws ConfigInvalidException {
- checkHeaderLineFormat(note, curr, ChangeNoteUtil.FILE, changeId);
- int startOfFileName = RawParseUtils.endOfFooterLineKey(note, curr.value) + 2;
- int endOfLine = RawParseUtils.nextLF(note, curr.value);
- curr.value = endOfLine;
- curr.value = RawParseUtils.nextLF(note, curr.value);
- return QuotedString.GIT_PATH.dequote(
- RawParseUtils.decode(UTF_8, note, startOfFileName, endOfLine - 1));
- }
-
- private static Timestamp parseTimestamp(byte[] note, MutableInteger curr, Change.Id changeId)
- throws ConfigInvalidException {
- int endOfLine = RawParseUtils.nextLF(note, curr.value);
- Timestamp commentTime;
- String dateString = RawParseUtils.decode(UTF_8, note, curr.value, endOfLine - 1);
- try {
- commentTime = new Timestamp(GitDateParser.parse(dateString, null, Locale.US).getTime());
- } catch (ParseException e) {
- throw new ConfigInvalidException("could not parse comment timestamp", e);
- }
- curr.value = endOfLine;
- return checkResult(commentTime, "comment timestamp", changeId);
- }
-
- private Account.Id parseAuthor(
- byte[] note, MutableInteger curr, Change.Id changeId, String fieldName)
- throws ConfigInvalidException {
- checkHeaderLineFormat(note, curr, fieldName, changeId);
- int startOfAccountId = RawParseUtils.endOfFooterLineKey(note, curr.value) + 2;
- PersonIdent ident = RawParseUtils.parsePersonIdent(note, startOfAccountId);
- Account.Id aId = parseIdent(ident, changeId);
- curr.value = RawParseUtils.nextLF(note, curr.value);
- return checkResult(aId, fieldName, changeId);
- }
-
- private static int parseCommentLength(byte[] note, MutableInteger curr, Change.Id changeId)
- throws ConfigInvalidException {
- checkHeaderLineFormat(note, curr, ChangeNoteUtil.LENGTH, changeId);
- int startOfLength = RawParseUtils.endOfFooterLineKey(note, curr.value) + 1;
- MutableInteger i = new MutableInteger();
- i.value = startOfLength;
- int commentLength = RawParseUtils.parseBase10(note, startOfLength, i);
- if (i.value == startOfLength) {
- throw parseException(changeId, "could not parse %s", ChangeNoteUtil.LENGTH);
- }
- int endOfLine = RawParseUtils.nextLF(note, curr.value);
- if (i.value != endOfLine - 1) {
- throw parseException(changeId, "could not parse %s", ChangeNoteUtil.LENGTH);
- }
- curr.value = endOfLine;
- return checkResult(commentLength, "comment length", changeId);
- }
-
- private boolean parseBooleanField(
- byte[] note, MutableInteger curr, Change.Id changeId, String fieldName)
- throws ConfigInvalidException {
- String str = parseStringField(note, curr, changeId, fieldName);
- if ("true".equalsIgnoreCase(str)) {
- return true;
- } else if ("false".equalsIgnoreCase(str)) {
- return false;
- }
- throw parseException(changeId, "invalid boolean for %s: %s", fieldName, str);
- }
-
- private static <T> T checkResult(T o, String fieldName, Change.Id changeId)
- throws ConfigInvalidException {
- if (o == null) {
- throw parseException(changeId, "could not parse %s", fieldName);
- }
- return o;
- }
-
- private static int checkResult(int i, String fieldName, Change.Id changeId)
- throws ConfigInvalidException {
- if (i <= 0) {
- throw parseException(changeId, "could not parse %s", fieldName);
- }
- return i;
- }
-
- private static void checkHeaderLineFormat(
- byte[] note, MutableInteger curr, String fieldName, Change.Id changeId)
- throws ConfigInvalidException {
- boolean correct = RawParseUtils.match(note, curr.value, fieldName.getBytes(UTF_8)) != -1;
- int p = curr.value + fieldName.length();
- correct &= (p < note.length && note[p] == ':');
- p++;
- correct &= (p < note.length && note[p] == ' ');
- if (!correct) {
- throw parseException(changeId, "could not parse %s", fieldName);
- }
- }
-}
diff --git a/java/com/google/gerrit/server/notedb/LegacyChangeNoteWrite.java b/java/com/google/gerrit/server/notedb/LegacyChangeNoteWrite.java
deleted file mode 100644
index a31a68b..0000000
--- a/java/com/google/gerrit/server/notedb/LegacyChangeNoteWrite.java
+++ /dev/null
@@ -1,198 +0,0 @@
-// Copyright (C) 2018 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.server.notedb;
-
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.collect.ImmutableList.toImmutableList;
-import static com.google.gerrit.server.CommentsUtil.COMMENT_ORDER;
-import static java.nio.charset.StandardCharsets.UTF_8;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ListMultimap;
-import com.google.gerrit.common.UsedAt;
-import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.Comment;
-import com.google.gerrit.server.GerritPersonIdent;
-import com.google.gerrit.server.config.GerritServerId;
-import com.google.inject.Inject;
-import java.io.OutputStream;
-import java.io.OutputStreamWriter;
-import java.io.PrintWriter;
-import java.sql.Timestamp;
-import java.util.Date;
-import java.util.List;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.util.QuotedString;
-
-public class LegacyChangeNoteWrite {
-
- private final PersonIdent serverIdent;
- private final String serverId;
-
- @Inject
- public LegacyChangeNoteWrite(
- @GerritPersonIdent PersonIdent serverIdent, @GerritServerId String serverId) {
- this.serverIdent = serverIdent;
- this.serverId = serverId;
- }
-
- public PersonIdent newIdent(Account.Id authorId, Date when, PersonIdent serverIdent) {
- return new PersonIdent(
- authorId.toString(), authorId.get() + "@" + serverId, when, serverIdent.getTimeZone());
- }
-
- @VisibleForTesting
- public PersonIdent newIdent(Account author, Date when, PersonIdent serverIdent) {
- return new PersonIdent(
- author.toString(), author.id().get() + "@" + serverId, when, serverIdent.getTimeZone());
- }
-
- public String getServerId() {
- return serverId;
- }
-
- private void appendHeaderField(PrintWriter writer, String field, String value) {
- writer.print(field);
- writer.print(": ");
- writer.print(value);
- writer.print('\n');
- }
-
- /**
- * Build a note that contains the metadata for and the contents of all of the comments in the
- * given comments.
- *
- * @param comments Comments to be written to the output stream, keyed by patch set ID; multiple
- * patch sets are allowed since base revisions may be shared across patch sets. All of the
- * comments must share the same commitId, and all the comments for a given patch set must have
- * the same side.
- * @param out output stream to write to.
- */
- @UsedAt(UsedAt.Project.GOOGLE)
- public void buildNote(ListMultimap<Integer, Comment> comments, OutputStream out) {
- if (comments.isEmpty()) {
- return;
- }
-
- ImmutableList<Integer> psIds = comments.keySet().stream().sorted().collect(toImmutableList());
-
- OutputStreamWriter streamWriter = new OutputStreamWriter(out, UTF_8);
- try (PrintWriter writer = new PrintWriter(streamWriter)) {
- ObjectId commitId = comments.values().iterator().next().getCommitId();
- String commitName = commitId.name();
- appendHeaderField(writer, ChangeNoteUtil.REVISION, commitName);
-
- for (int psId : psIds) {
- List<Comment> psComments = COMMENT_ORDER.sortedCopy(comments.get(psId));
- Comment first = psComments.get(0);
-
- short side = first.side;
- appendHeaderField(
- writer,
- side <= 0 ? ChangeNoteUtil.BASE_PATCH_SET : ChangeNoteUtil.PATCH_SET,
- Integer.toString(psId));
- if (side < 0) {
- appendHeaderField(writer, ChangeNoteUtil.PARENT_NUMBER, Integer.toString(-side));
- }
-
- String currentFilename = null;
-
- for (Comment c : psComments) {
- checkArgument(
- commitId.equals(c.getCommitId()),
- "All comments being added must have all the same commitId. The "
- + "comment below does not have the same commitId as the others "
- + "(%s).\n%s",
- commitId,
- c);
- checkArgument(
- side == c.side,
- "All comments being added must all have the same side. The "
- + "comment below does not have the same side as the others "
- + "(%s).\n%s",
- side,
- c);
- String commentFilename = QuotedString.GIT_PATH.quote(c.key.filename);
-
- if (!commentFilename.equals(currentFilename)) {
- currentFilename = commentFilename;
- writer.print("File: ");
- writer.print(commentFilename);
- writer.print("\n\n");
- }
-
- appendOneComment(writer, c);
- }
- }
- }
- }
-
- private void appendOneComment(PrintWriter writer, Comment c) {
- // The CommentRange field for a comment is allowed to be null. If it is
- // null, then in the first line, we simply use the line number field for a
- // comment instead. If it isn't null, we write the comment range itself.
- Comment.Range range = c.range;
- if (range != null) {
- writer.print(range.startLine);
- writer.print(':');
- writer.print(range.startChar);
- writer.print('-');
- writer.print(range.endLine);
- writer.print(':');
- writer.print(range.endChar);
- } else {
- writer.print(c.lineNbr);
- }
- writer.print("\n");
-
- writer.print(NoteDbUtil.formatTime(serverIdent, c.writtenOn));
- writer.print("\n");
-
- appendIdent(writer, ChangeNoteUtil.AUTHOR, c.author.getId(), c.writtenOn);
- if (!c.getRealAuthor().equals(c.author)) {
- appendIdent(writer, ChangeNoteUtil.REAL_AUTHOR, c.getRealAuthor().getId(), c.writtenOn);
- }
-
- String parent = c.parentUuid;
- if (parent != null) {
- appendHeaderField(writer, ChangeNoteUtil.PARENT, parent);
- }
-
- appendHeaderField(writer, ChangeNoteUtil.UNRESOLVED, Boolean.toString(c.unresolved));
- appendHeaderField(writer, ChangeNoteUtil.UUID, c.key.uuid);
-
- if (c.tag != null) {
- appendHeaderField(writer, ChangeNoteUtil.TAG, c.tag);
- }
-
- byte[] messageBytes = c.message.getBytes(UTF_8);
- appendHeaderField(writer, ChangeNoteUtil.LENGTH, Integer.toString(messageBytes.length));
-
- writer.print(c.message);
- writer.print("\n\n");
- }
-
- private void appendIdent(PrintWriter writer, String header, Account.Id id, Timestamp ts) {
- PersonIdent ident = newIdent(id, ts, serverIdent);
- StringBuilder name = new StringBuilder();
- PersonIdent.appendSanitized(name, ident.getName());
- name.append(" <");
- PersonIdent.appendSanitized(name, ident.getEmailAddress());
- name.append('>');
- appendHeaderField(writer, header, name.toString());
- }
-}
diff --git a/java/com/google/gerrit/server/notedb/RevisionNoteMap.java b/java/com/google/gerrit/server/notedb/RevisionNoteMap.java
index bb464f6..3e1bad1 100644
--- a/java/com/google/gerrit/server/notedb/RevisionNoteMap.java
+++ b/java/com/google/gerrit/server/notedb/RevisionNoteMap.java
@@ -15,9 +15,7 @@
package com.google.gerrit.server.notedb;
import com.google.common.collect.ImmutableMap;
-import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
-import com.google.gerrit.entities.PatchLineComment;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
@@ -32,18 +30,11 @@
final ImmutableMap<ObjectId, T> revisionNotes;
static RevisionNoteMap<ChangeRevisionNote> parse(
- ChangeNoteJson noteJson,
- LegacyChangeNoteRead legacyChangeNoteRead,
- Change.Id changeId,
- ObjectReader reader,
- NoteMap noteMap,
- PatchLineComment.Status status)
+ ChangeNoteJson noteJson, ObjectReader reader, NoteMap noteMap, Comment.Status status)
throws ConfigInvalidException, IOException {
Map<ObjectId, ChangeRevisionNote> result = new HashMap<>();
for (Note note : noteMap) {
- ChangeRevisionNote rn =
- new ChangeRevisionNote(
- noteJson, legacyChangeNoteRead, changeId, reader, note.getData(), status);
+ ChangeRevisionNote rn = new ChangeRevisionNote(noteJson, reader, note.getData(), status);
rn.parse();
result.put(note.copy(), rn);
}
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 3779fbf..44d9d98 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -217,6 +217,14 @@
r.load(update, id);
return r;
}
+
+ @UsedAt(UsedAt.Project.COLLABNET)
+ public ProjectConfig read(Repository repo, Project.NameKey name)
+ throws IOException, ConfigInvalidException {
+ ProjectConfig r = create(name);
+ r.load(repo);
+ return r;
+ }
}
private final StoredConfig baseConfig;
diff --git a/java/com/google/gerrit/server/query/account/AccountPredicates.java b/java/com/google/gerrit/server/query/account/AccountPredicates.java
index 647d70d..1eed7ea 100644
--- a/java/com/google/gerrit/server/query/account/AccountPredicates.java
+++ b/java/com/google/gerrit/server/query/account/AccountPredicates.java
@@ -44,7 +44,7 @@
List<Predicate<AccountState>> preds = Lists.newArrayListWithCapacity(3);
Integer id = Ints.tryParse(query);
if (id != null) {
- preds.add(id(Account.id(id)));
+ preds.add(id(schema, Account.id(id)));
}
if (canSeeSecondaryEmails) {
preds.add(equalsNameIncludingSecondaryEmails(query));
@@ -64,9 +64,11 @@
return Predicate.or(preds);
}
- public static Predicate<AccountState> id(Account.Id accountId) {
+ public static Predicate<AccountState> id(Schema<AccountState> schema, Account.Id accountId) {
return new AccountPredicate(
- AccountField.ID, AccountQueryBuilder.FIELD_ACCOUNT, accountId.toString());
+ schema.useLegacyNumericFields() ? AccountField.ID : AccountField.ID_STR,
+ AccountQueryBuilder.FIELD_ACCOUNT,
+ accountId.toString());
}
public static Predicate<AccountState> emailIncludingSecondaryEmails(String email) {
diff --git a/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java b/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java
index 699c988..664df70 100644
--- a/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java
@@ -202,7 +202,7 @@
if ("self".equalsIgnoreCase(query) || "me".equalsIgnoreCase(query)) {
try {
- return Predicate.or(defaultPredicate, AccountPredicates.id(self()));
+ return Predicate.or(defaultPredicate, AccountPredicates.id(args.schema(), self()));
} catch (QueryParseException e) {
// Skip.
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index f44a54d..d2fc77d 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -38,7 +38,6 @@
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.index.IndexConfig;
-import com.google.gerrit.index.IndexType;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.SchemaUtil;
import com.google.gerrit.index.query.LimitPredicate;
@@ -461,7 +460,9 @@
if (PAT_LEGACY_ID.matcher(query).matches()) {
Integer id = Ints.tryParse(query);
if (id != null) {
- return new LegacyChangeIdPredicate(Change.id(id));
+ return args.getSchema().useLegacyNumericFields()
+ ? new LegacyChangeIdPredicate(Change.id(id))
+ : new LegacyChangeIdStrPredicate(Change.id(id));
}
} else if (PAT_CHANGE_ID.matcher(query).matches()) {
return new ChangeIdPredicate(parseChangeId(query));
@@ -736,9 +737,6 @@
@Operator
public Predicate<ChangeData> extension(String ext) throws QueryParseException {
if (args.getSchema().hasField(ChangeField.EXTENSION)) {
- if (ext.isEmpty() && IndexType.isElasticsearch(args.indexConfig.type())) {
- return new FileWithNoExtensionInElasticPredicate();
- }
return new FileExtensionPredicate(ext);
}
throw new QueryParseException("'extension' operator is not supported by change index version");
@@ -777,11 +775,6 @@
if (directory.startsWith("^")) {
return new RegexDirectoryPredicate(directory);
}
-
- if (IndexType.isElasticsearch(args.indexConfig.type())
- && (directory.isEmpty() || directory.equals("/"))) {
- return Predicate.any();
- }
return new DirectoryPredicate(directory);
}
throw new QueryParseException("'directory' operator is not supported by change index version");
diff --git a/java/com/google/gerrit/server/query/change/CommentPredicate.java b/java/com/google/gerrit/server/query/change/CommentPredicate.java
index d193bb6..4b14f08 100644
--- a/java/com/google/gerrit/server/query/change/CommentPredicate.java
+++ b/java/com/google/gerrit/server/query/change/CommentPredicate.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.query.change;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryParseException;
@@ -32,9 +33,15 @@
@Override
public boolean match(ChangeData object) {
try {
- Predicate<ChangeData> p = Predicate.and(new LegacyChangeIdPredicate(object.getId()), this);
+ Change.Id id = object.getId();
+ Predicate<ChangeData> p =
+ Predicate.and(
+ index.getSchema().useLegacyNumericFields()
+ ? new LegacyChangeIdPredicate(id)
+ : new LegacyChangeIdStrPredicate(id),
+ this);
for (ChangeData cData : index.getSource(p, IndexedChangeQuery.oneResult()).read()) {
- if (cData.getId().equals(object.getId())) {
+ if (cData.getId().equals(id)) {
return true;
}
}
diff --git a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
index 5ec1e87..17e4a59 100644
--- a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
@@ -87,7 +87,11 @@
List<Predicate<ChangeData>> and = new ArrayList<>(5);
and.add(new ProjectPredicate(c.getProject().get()));
and.add(new RefPredicate(c.getDest().branch()));
- and.add(Predicate.not(new LegacyChangeIdPredicate(c.getId())));
+ and.add(
+ Predicate.not(
+ args.getSchema().useLegacyNumericFields()
+ ? new LegacyChangeIdPredicate(c.getId())
+ : new LegacyChangeIdStrPredicate(c.getId())));
and.add(Predicate.or(filePredicates));
ChangeDataCache changeDataCache = new ChangeDataCache(cd, args.projectCache);
diff --git a/java/com/google/gerrit/server/query/change/FileWithNoExtensionInElasticPredicate.java b/java/com/google/gerrit/server/query/change/FileWithNoExtensionInElasticPredicate.java
deleted file mode 100644
index d886baf..0000000
--- a/java/com/google/gerrit/server/query/change/FileWithNoExtensionInElasticPredicate.java
+++ /dev/null
@@ -1,37 +0,0 @@
-// Copyright (C) 2019 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.server.query.change;
-
-import com.google.gerrit.index.query.PostFilterPredicate;
-import com.google.gerrit.server.index.change.ChangeField;
-
-public class FileWithNoExtensionInElasticPredicate extends PostFilterPredicate<ChangeData> {
-
- private static final String NO_EXT = "";
-
- public FileWithNoExtensionInElasticPredicate() {
- super(ChangeField.EXTENSION.getName(), NO_EXT);
- }
-
- @Override
- public boolean match(ChangeData cd) {
- return ChangeField.getExtensions(cd).contains(NO_EXT);
- }
-
- @Override
- public int getCost() {
- return 1;
- }
-}
diff --git a/java/com/google/gerrit/server/query/change/FuzzyTopicPredicate.java b/java/com/google/gerrit/server/query/change/FuzzyTopicPredicate.java
index a5838e5..d558b0f 100644
--- a/java/com/google/gerrit/server/query/change/FuzzyTopicPredicate.java
+++ b/java/com/google/gerrit/server/query/change/FuzzyTopicPredicate.java
@@ -43,7 +43,10 @@
return false;
}
try {
- Predicate<ChangeData> thisId = new LegacyChangeIdPredicate(cd.getId());
+ Predicate<ChangeData> thisId =
+ index.getSchema().useLegacyNumericFields()
+ ? new LegacyChangeIdPredicate(cd.getId())
+ : new LegacyChangeIdStrPredicate(cd.getId());
Iterable<ChangeData> results =
index.getSource(and(thisId, this), IndexedChangeQuery.oneResult()).read();
return !Iterables.isEmpty(results);
diff --git a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
index 0bef209..6605c23 100644
--- a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
+++ b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
@@ -55,6 +55,11 @@
* holding on to a single instance.
*/
public class InternalChangeQuery extends InternalQuery<ChangeData, InternalChangeQuery> {
+ @FunctionalInterface
+ static interface ChangeIdPredicateFactory {
+ Predicate<ChangeData> create(Change.Id id);
+ }
+
private static Predicate<ChangeData> ref(BranchNameKey branch) {
return new RefPredicate(branch.branch());
}
@@ -78,6 +83,9 @@
private final ChangeData.Factory changeDataFactory;
private final ChangeNotes.Factory notesFactory;
+ // TODO(davido): Remove the below fields when support for legacy numeric fields is removed.
+ private final ChangeIdPredicateFactory predicateFactory;
+
@Inject
InternalChangeQuery(
ChangeQueryProcessor queryProcessor,
@@ -88,6 +96,11 @@
super(queryProcessor, indexes, indexConfig);
this.changeDataFactory = changeDataFactory;
this.notesFactory = notesFactory;
+ predicateFactory =
+ (id) ->
+ schema().useLegacyNumericFields()
+ ? new LegacyChangeIdPredicate(id)
+ : new LegacyChangeIdStrPredicate(id);
}
public List<ChangeData> byKey(Change.Key key) {
@@ -99,13 +112,13 @@
}
public List<ChangeData> byLegacyChangeId(Change.Id id) {
- return query(new LegacyChangeIdPredicate(id));
+ return query(predicateFactory.create(id));
}
public List<ChangeData> byLegacyChangeIds(Collection<Change.Id> ids) {
List<Predicate<ChangeData>> preds = new ArrayList<>(ids.size());
for (Change.Id id : ids) {
- preds.add(new LegacyChangeIdPredicate(id));
+ preds.add(predicateFactory.create(id));
}
return query(or(preds));
}
diff --git a/java/com/google/gerrit/server/query/change/LegacyChangeIdStrPredicate.java b/java/com/google/gerrit/server/query/change/LegacyChangeIdStrPredicate.java
new file mode 100644
index 0000000..bae9c0d
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/LegacyChangeIdStrPredicate.java
@@ -0,0 +1,39 @@
+// Copyright (C) 2019 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.server.query.change;
+
+import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID_STR;
+
+import com.google.gerrit.entities.Change;
+
+/** Predicate over change number (aka legacy ID or Change.Id). */
+public class LegacyChangeIdStrPredicate extends ChangeIndexPredicate {
+ protected final Change.Id id;
+
+ public LegacyChangeIdStrPredicate(Change.Id id) {
+ super(LEGACY_ID_STR, ChangeQueryBuilder.FIELD_CHANGE, id.toString());
+ this.id = id;
+ }
+
+ @Override
+ public boolean match(ChangeData object) {
+ return id.equals(object.getId());
+ }
+
+ @Override
+ public int getCost() {
+ return 1;
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/MessagePredicate.java b/java/com/google/gerrit/server/query/change/MessagePredicate.java
index 0bd8c88..44cbd8e 100644
--- a/java/com/google/gerrit/server/query/change/MessagePredicate.java
+++ b/java/com/google/gerrit/server/query/change/MessagePredicate.java
@@ -33,7 +33,12 @@
@Override
public boolean match(ChangeData object) {
try {
- Predicate<ChangeData> p = Predicate.and(new LegacyChangeIdPredicate(object.getId()), this);
+ Predicate<ChangeData> p =
+ Predicate.and(
+ index.getSchema().useLegacyNumericFields()
+ ? new LegacyChangeIdPredicate(object.getId())
+ : new LegacyChangeIdStrPredicate(object.getId()),
+ this);
for (ChangeData cData : index.getSource(p, IndexedChangeQuery.oneResult()).read()) {
if (cData.getId().equals(object.getId())) {
return true;
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
index b326834..1973b00 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
@@ -70,6 +70,7 @@
import java.util.TimeZone;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.InvalidObjectIdException;
+import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectReader;
@@ -381,7 +382,14 @@
throw new BadRequestException(String.format("Base %s doesn't represent a valid SHA-1", base));
}
- RevCommit baseCommit = revWalk.parseCommit(baseObjectId);
+ RevCommit baseCommit;
+ try {
+ baseCommit = revWalk.parseCommit(baseObjectId);
+ } catch (MissingObjectException e) {
+ throw new UnprocessableEntityException(
+ String.format("Base %s doesn't exist", baseObjectId.name()), e);
+ }
+
InternalChangeQuery changeQuery = queryProvider.get();
changeQuery.enforceVisibility(true);
List<ChangeData> changeDatas = changeQuery.byBranchCommit(project, destRef.getName(), base);
diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java
index acc6465..758cf47 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -170,16 +170,28 @@
BatchUpdate.Factory updateFactory, TopLevelResource parent, ChangeInput input)
throws IOException, InvalidChangeOperationException, RestApiException, UpdateException,
PermissionBackendException, ConfigInvalidException {
+ if (Strings.isNullOrEmpty(input.project)) {
+ throw new BadRequestException("project must be non-empty");
+ }
+
+ return execute(updateFactory, input, projectsCollection.parse(input.project));
+ }
+
+ /** Creates the changes in the given project. This is public for reuse in the project API. */
+ public Response<ChangeInfo> execute(
+ BatchUpdate.Factory updateFactory, ChangeInput input, ProjectResource projectResource)
+ throws IOException, RestApiException, UpdateException, PermissionBackendException,
+ ConfigInvalidException {
if (!user.get().isIdentifiedUser()) {
throw new AuthException("Authentication required");
}
- IdentifiedUser me = user.get().asIdentifiedUser();
- checkAndSanitizeChangeInput(input, me);
- ProjectResource projectResource = projectsCollection.parse(input.project);
ProjectState projectState = projectResource.getProjectState();
projectState.checkStatePermitsWrite();
+ IdentifiedUser me = user.get().asIdentifiedUser();
+ checkAndSanitizeChangeInput(input, me);
+
Project.NameKey project = projectResource.getNameKey();
contributorAgreements.check(project, user.get());
@@ -202,10 +214,6 @@
*/
private void checkAndSanitizeChangeInput(ChangeInput input, IdentifiedUser me)
throws RestApiException, PermissionBackendException, IOException {
- if (Strings.isNullOrEmpty(input.project)) {
- throw new BadRequestException("project must be non-empty");
- }
-
if (Strings.isNullOrEmpty(input.branch)) {
throw new BadRequestException("branch must be non-empty");
}
diff --git a/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java b/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java
index 88938b6..f434e31 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java
@@ -18,7 +18,6 @@
import com.google.common.base.Strings;
import com.google.gerrit.entities.Comment;
-import com.google.gerrit.entities.PatchLineComment.Status;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.common.CommentInfo;
@@ -121,7 +120,8 @@
setCommentCommitId(comment, patchListCache, ctx.getChange(), ps);
- commentsUtil.putComments(ctx.getUpdate(psId), Status.DRAFT, Collections.singleton(comment));
+ commentsUtil.putComments(
+ ctx.getUpdate(psId), Comment.Status.DRAFT, Collections.singleton(comment));
return true;
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/Module.java b/java/com/google/gerrit/server/restapi/change/Module.java
index a57bd64..539463f 100644
--- a/java/com/google/gerrit/server/restapi/change/Module.java
+++ b/java/com/google/gerrit/server/restapi/change/Module.java
@@ -96,6 +96,7 @@
post(CHANGE_KIND, "hashtags").to(PostHashtags.class);
post(CHANGE_KIND, "restore").to(Restore.class);
post(CHANGE_KIND, "revert").to(Revert.class);
+ post(CHANGE_KIND, "revert_submission").to(RevertSubmission.class);
post(CHANGE_KIND, "submit").to(Submit.CurrentRevision.class);
get(CHANGE_KIND, "submitted_together").to(SubmittedTogether.class);
post(CHANGE_KIND, "rebase").to(Rebase.CurrentRevision.class);
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index a38e88f..974a72c 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -49,7 +49,6 @@
import com.google.gerrit.entities.FixSuggestion;
import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.Patch;
-import com.google.gerrit.entities.PatchLineComment.Status;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.RobotComment;
@@ -249,7 +248,10 @@
}
ProjectState projectState = projectCache.checkedGet(revision.getProject());
LabelTypes labelTypes = projectState.getLabelTypes(revision.getNotes());
+
input.drafts = firstNonNull(input.drafts, DraftHandling.KEEP);
+ logger.atFine().log("draft handling = %s", input.drafts);
+
if (input.onBehalfOf != null) {
revision = onBehalfOf(revision, labelTypes, input);
}
@@ -971,7 +973,7 @@
break;
}
ChangeUpdate changeUpdate = ctx.getUpdate(psId);
- commentsUtil.putComments(changeUpdate, Status.PUBLISHED, toPublish);
+ commentsUtil.putComments(changeUpdate, Comment.Status.PUBLISHED, toPublish);
comments.addAll(toPublish);
return !toPublish.isEmpty();
}
diff --git a/java/com/google/gerrit/server/restapi/change/PutDraftComment.java b/java/com/google/gerrit/server/restapi/change/PutDraftComment.java
index 948ad89..5696fcb 100644
--- a/java/com/google/gerrit/server/restapi/change/PutDraftComment.java
+++ b/java/com/google/gerrit/server/restapi/change/PutDraftComment.java
@@ -17,7 +17,6 @@
import static com.google.gerrit.server.CommentsUtil.setCommentCommitId;
import com.google.gerrit.entities.Comment;
-import com.google.gerrit.entities.PatchLineComment.Status;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.common.CommentInfo;
@@ -140,7 +139,7 @@
}
setCommentCommitId(comment, patchListCache, ctx.getChange(), ps);
commentsUtil.putComments(
- update, Status.DRAFT, Collections.singleton(update(comment, in, ctx.getWhen())));
+ update, Comment.Status.DRAFT, Collections.singleton(update(comment, in, ctx.getWhen())));
return true;
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
new file mode 100644
index 0000000..2c3930a
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -0,0 +1,166 @@
+// Copyright (C) 2019 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.server.restapi.change;
+
+import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
+import static com.google.gerrit.server.permissions.RefPermission.CREATE_CHANGE;
+import static java.util.Objects.requireNonNull;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.extensions.api.changes.RevertInput;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.RevertSubmissionInfo;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.webui.UiAction;
+import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.project.ContributorAgreementsChecker;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gerrit.server.update.BatchUpdate;
+import com.google.gerrit.server.update.RetryHelper;
+import com.google.gerrit.server.update.RetryingRestModifyView;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.commons.lang.RandomStringUtils;
+
+@Singleton
+public class RevertSubmission
+ extends RetryingRestModifyView<ChangeResource, RevertInput, RevertSubmissionInfo>
+ implements UiAction<ChangeResource> {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final Revert revert;
+ private final Provider<InternalChangeQuery> queryProvider;
+ private final ChangeResource.Factory changeResourceFactory;
+ private final Provider<CurrentUser> user;
+ private final PermissionBackend permissionBackend;
+ private final ProjectCache projectCache;
+ private final PatchSetUtil psUtil;
+ private final ContributorAgreementsChecker contributorAgreements;
+
+ @Inject
+ RevertSubmission(
+ RetryHelper retryHelper,
+ Revert revert,
+ Provider<InternalChangeQuery> queryProvider,
+ ChangeResource.Factory changeResourceFactory,
+ Provider<CurrentUser> user,
+ PermissionBackend permissionBackend,
+ ProjectCache projectCache,
+ PatchSetUtil psUtil,
+ ContributorAgreementsChecker contributorAgreements) {
+ super(retryHelper);
+ this.revert = revert;
+ this.queryProvider = queryProvider;
+ this.changeResourceFactory = changeResourceFactory;
+ this.user = user;
+ this.permissionBackend = permissionBackend;
+ this.projectCache = projectCache;
+ this.psUtil = psUtil;
+ this.contributorAgreements = contributorAgreements;
+ }
+
+ @Override
+ public Response<RevertSubmissionInfo> applyImpl(
+ BatchUpdate.Factory updateFactory, ChangeResource changeResource, RevertInput input)
+ throws Exception {
+
+ if (!changeResource.getChange().isMerged()) {
+ throw new ResourceConflictException(
+ String.format("change is %s.", ChangeUtil.status(changeResource.getChange())));
+ }
+
+ String submissionId =
+ requireNonNull(
+ changeResource.getChange().getSubmissionId(),
+ String.format("merged change %s has no submission ID", changeResource.getId()));
+
+ List<ChangeData> changeDatas = queryProvider.get().bySubmissionId(submissionId);
+
+ for (ChangeData changeData : changeDatas) {
+ Change change = changeData.change();
+
+ // Might do the permission tests multiple times, but these are necessary to ensure that the
+ // user has permissions to revert all changes. If they lack any permission, no revert will be
+ // done.
+
+ contributorAgreements.check(change.getProject(), changeResource.getUser());
+ permissionBackend.currentUser().ref(change.getDest()).check(CREATE_CHANGE);
+ permissionBackend.currentUser().change(changeData).check(ChangePermission.READ);
+ projectCache.checkedGet(change.getProject()).checkStatePermitsWrite();
+
+ requireNonNull(
+ psUtil.get(changeData.notes(), change.currentPatchSetId()),
+ String.format(
+ "current patch set %s of change %s not found",
+ change.currentPatchSetId(), change.currentPatchSetId()));
+ }
+ return Response.ok(revertSubmission(changeDatas, input, submissionId));
+ }
+
+ private RevertSubmissionInfo revertSubmission(
+ List<ChangeData> changeDatas, RevertInput input, String submissionId) throws Exception {
+ List<ChangeInfo> results;
+ results = new ArrayList<>();
+ if (input.topic == null) {
+ input.topic =
+ String.format(
+ "revert-%s-%s", submissionId, RandomStringUtils.randomAlphabetic(10).toUpperCase());
+ }
+ for (ChangeData changeData : changeDatas) {
+ ChangeResource change = changeResourceFactory.create(changeData.notes(), user.get());
+ // Reverts are done with retrying by using RetryingRestModifyView.
+ results.add(revert.apply(change, input).value());
+ }
+ RevertSubmissionInfo revertSubmissionInfo = new RevertSubmissionInfo();
+ revertSubmissionInfo.revertChanges = results;
+ return revertSubmissionInfo;
+ }
+
+ @Override
+ public Description getDescription(ChangeResource rsrc) {
+ Change change = rsrc.getChange();
+ boolean projectStatePermitsWrite = false;
+ try {
+ projectStatePermitsWrite = projectCache.checkedGet(rsrc.getProject()).statePermitsWrite();
+ } catch (IOException e) {
+ logger.atSevere().withCause(e).log(
+ "Failed to check if project state permits write: %s", rsrc.getProject());
+ }
+ return new UiAction.Description()
+ .setLabel(
+ "Revert this change and all changes that have been submitted together with this change")
+ .setTitle("Revert submission")
+ .setVisible(
+ and(
+ projectStatePermitsWrite,
+ permissionBackend
+ .user(rsrc.getUser())
+ .ref(change.getDest())
+ .testCond(CREATE_CHANGE)));
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
index 26f7873..676cc07 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
@@ -32,6 +32,7 @@
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Url;
+import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.query.FieldBundle;
@@ -227,6 +228,13 @@
return suggestedReviewers;
}
+ private static Account.Id fromIdField(FieldBundle f, boolean useLegacyNumericFields) {
+ if (useLegacyNumericFields) {
+ return Account.id(f.getValue(AccountField.ID).intValue());
+ }
+ return Account.id(Integer.valueOf(f.getValue(AccountField.ID_STR)));
+ }
+
private List<Account.Id> suggestAccounts(SuggestReviewers suggestReviewers)
throws BadRequestException {
try (Timer0.Context ctx = metrics.queryAccountsLatency.start()) {
@@ -239,6 +247,10 @@
accountQueryBuilder.defaultQuery(suggestReviewers.getQuery()));
logger.atFine().log("accounts index query: %s", pred);
accountIndexRewriter.validateMaxTermsInQuery(pred);
+ boolean useLegacyNumericFields =
+ accountIndexes.getSearchIndex().getSchema().useLegacyNumericFields();
+ FieldDef<AccountState, ?> idField =
+ useLegacyNumericFields ? AccountField.ID : AccountField.ID_STR;
ResultSet<FieldBundle> result =
accountIndexes
.getSearchIndex()
@@ -248,11 +260,11 @@
indexConfig,
0,
suggestReviewers.getLimit(),
- ImmutableSet.of(AccountField.ID.getName())))
+ ImmutableSet.of(idField.getName())))
.readRaw();
List<Account.Id> matches =
result.toList().stream()
- .map(f -> Account.id(f.getValue(AccountField.ID).intValue()))
+ .map(f -> fromIdField(f, useLegacyNumericFields))
.collect(toList());
logger.atFine().log("Matches: %s", matches);
return matches;
diff --git a/java/com/google/gerrit/server/restapi/project/CreateChange.java b/java/com/google/gerrit/server/restapi/project/CreateChange.java
new file mode 100644
index 0000000..de11ffe
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/project/CreateChange.java
@@ -0,0 +1,70 @@
+// Copyright (C) 2019 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.server.restapi.project;
+
+import com.google.common.base.Strings;
+import com.google.gerrit.exceptions.InvalidNameException;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.ChangeInput;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.project.InvalidChangeOperationException;
+import com.google.gerrit.server.project.ProjectResource;
+import com.google.gerrit.server.update.BatchUpdate;
+import com.google.gerrit.server.update.RetryHelper;
+import com.google.gerrit.server.update.RetryingRestModifyView;
+import com.google.gerrit.server.update.UpdateException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+
+@Singleton
+public class CreateChange extends RetryingRestModifyView<ProjectResource, ChangeInput, ChangeInfo> {
+ private final com.google.gerrit.server.restapi.change.CreateChange changeCreateChange;
+ private final Provider<CurrentUser> user;
+
+ @Inject
+ public CreateChange(
+ RetryHelper retryHelper,
+ Provider<CurrentUser> user,
+ com.google.gerrit.server.restapi.change.CreateChange changeCreateChange) {
+ super(retryHelper);
+ this.changeCreateChange = changeCreateChange;
+ this.user = user;
+ }
+
+ @Override
+ public Response<ChangeInfo> applyImpl(
+ BatchUpdate.Factory updateFactory, ProjectResource rsrc, ChangeInput input)
+ throws PermissionBackendException, IOException, ConfigInvalidException,
+ InvalidChangeOperationException, InvalidNameException, UpdateException, RestApiException {
+ if (!user.get().isIdentifiedUser()) {
+ throw new AuthException("Authentication required");
+ }
+
+ if (!Strings.isNullOrEmpty(input.project)) {
+ throw new BadRequestException("may not specify project");
+ }
+
+ input.project = rsrc.getName();
+ return changeCreateChange.execute(updateFactory, input, rsrc);
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/project/Module.java b/java/com/google/gerrit/server/restapi/project/Module.java
index de5661d..065facd 100644
--- a/java/com/google/gerrit/server/restapi/project/Module.java
+++ b/java/com/google/gerrit/server/restapi/project/Module.java
@@ -77,6 +77,7 @@
child(PROJECT_KIND, "branches").to(BranchesCollection.class);
create(BRANCH_KIND).to(CreateBranch.class);
+ post(PROJECT_KIND, "create.change").to(CreateChange.class);
put(BRANCH_KIND).to(PutBranch.class);
get(BRANCH_KIND).to(GetBranch.class);
delete(BRANCH_KIND).to(DeleteBranch.class);
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index a06027a..406c0a1 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -504,7 +504,7 @@
try {
integrateIntoHistory(cs);
} catch (IntegrationException e) {
- logger.atSevere().withCause(e).log("Error from integrateIntoHistory");
+ logger.atWarning().withCause(e).log("Error from integrateIntoHistory");
throw new ResourceConflictException(e.getMessage(), e);
}
return null;
diff --git a/java/com/google/gerrit/server/util/ReplicaUtil.java b/java/com/google/gerrit/server/util/ReplicaUtil.java
new file mode 100644
index 0000000..bf6111a
--- /dev/null
+++ b/java/com/google/gerrit/server/util/ReplicaUtil.java
@@ -0,0 +1,25 @@
+// Copyright (C) 2019 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.server.util;
+
+import org.eclipse.jgit.lib.Config;
+
+public class ReplicaUtil {
+ /** Provides backward compatibility for container.slave property. */
+ public static boolean isReplica(Config cfg) {
+ return cfg.getBoolean("container", "slave", false)
+ || cfg.getBoolean("container", "replica", false);
+ }
+}
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index 08835cc..fa9685e 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -86,6 +86,7 @@
import com.google.gerrit.server.securestore.SecureStore;
import com.google.gerrit.server.ssh.NoSshKeyCache;
import com.google.gerrit.server.submit.LocalMergeSuperSetComputation;
+import com.google.gerrit.server.util.ReplicaUtil;
import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.google.inject.Injector;
@@ -299,11 +300,10 @@
private Module indexModule(String moduleClassName) {
try {
- boolean slave = cfg.getBoolean("container", "slave", false);
Class<?> clazz = Class.forName(moduleClassName);
Method m =
clazz.getMethod("singleVersionWithExplicitVersions", Map.class, int.class, boolean.class);
- return (Module) m.invoke(null, getSingleSchemaVersions(), 0, slave);
+ return (Module) m.invoke(null, getSingleSchemaVersions(), 0, ReplicaUtil.isReplica(cfg));
} catch (ClassNotFoundException
| SecurityException
| NoSuchMethodException
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
index 5550d98..9059cb1 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
@@ -239,6 +239,28 @@
}
@Test
+ public void revertSubmissionWithoutCLA() throws Exception {
+ assume().that(isContributorAgreementsEnabled()).isTrue();
+
+ // Create a change succeeds when agreement is not required
+ setUseContributorAgreements(InheritableBoolean.FALSE);
+ ChangeInfo change = gApi.changes().create(newChangeInput()).get();
+
+ // Approve and submit it
+ requestScopeOperations.setApiUser(admin.id());
+ gApi.changes().id(change.changeId).current().review(ReviewInput.approve());
+ gApi.changes().id(change.changeId).current().submit(new SubmitInput());
+
+ // Revert Submission is not allowed when CLA is required but not signed
+ requestScopeOperations.setApiUser(user.id());
+ setUseContributorAgreements(InheritableBoolean.TRUE);
+ AuthException thrown =
+ assertThrows(
+ AuthException.class, () -> gApi.changes().id(change.changeId).revertSubmission());
+ assertThat(thrown).hasMessageThat().contains("Contributor Agreement");
+ }
+
+ @Test
public void revertExcludedProjectChangeWithoutCLA() throws Exception {
// Contributor agreements configured with excludeProjects = ExcludedProject
// in AbstractDaemonTest.configureContributorAgreement(...)
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
index 0607a3c..0e89de2 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
@@ -21,12 +21,16 @@
import static java.util.stream.Collectors.toList;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestProjectInput;
+import com.google.gerrit.acceptance.UseClockStep;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RevertInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -36,6 +40,8 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.PureRevertInfo;
+import com.google.gerrit.extensions.common.RevertSubmissionInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -45,8 +51,10 @@
import com.google.inject.Inject;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Repository;
import org.junit.Test;
@@ -361,17 +369,20 @@
@Test
public void cantCreateRevertWithoutProjectWritePermission() throws Exception {
- PushOneCommit.Result r = createChange();
- gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.approve());
- gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).submit();
+ PushOneCommit.Result result = createChange();
+ gApi.changes()
+ .id(result.getChangeId())
+ .revision(result.getCommit().name())
+ .review(ReviewInput.approve());
+ gApi.changes().id(result.getChangeId()).revision(result.getCommit().name()).submit();
projectCache.checkedGet(project).getProject().setState(ProjectState.READ_ONLY);
+ String expected = "project state " + ProjectState.READ_ONLY + " does not permit write";
ResourceConflictException thrown =
assertThrows(
- ResourceConflictException.class, () -> gApi.changes().id(r.getChangeId()).revert());
- assertThat(thrown)
- .hasMessageThat()
- .contains("project state " + ProjectState.READ_ONLY + " does not permit write");
+ ResourceConflictException.class,
+ () -> gApi.changes().id(result.getChangeId()).revert());
+ assertThat(thrown).hasMessageThat().contains(expected);
}
@Test
@@ -412,6 +423,339 @@
assertThat(thrown).hasMessageThat().contains("Not found: " + r.getChangeId());
}
+ @Test
+ @GerritConfig(name = "change.submitWholeTopic", value = "true")
+ public void cantCreateRevertSubmissionWithoutProjectWritePermission() throws Exception {
+ String secondProject = "secondProject";
+ projectOperations.newProject().name(secondProject).create();
+ TestRepository<InMemoryRepository> secondRepo =
+ cloneProject(Project.nameKey("secondProject"), admin);
+ String topic = "topic";
+ PushOneCommit.Result result1 =
+ createChange(testRepo, "master", "first change", "a.txt", "message", topic);
+ PushOneCommit.Result result2 =
+ createChange(secondRepo, "master", "second change", "b.txt", "message", topic);
+ gApi.changes().id(result1.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(result2.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(result1.getChangeId()).revision(result1.getCommit().name()).submit();
+
+ // revoke write permissions for the first repository.
+ projectCache.checkedGet(project).getProject().setState(ProjectState.READ_ONLY);
+
+ String expected = "project state " + ProjectState.READ_ONLY + " does not permit write";
+
+ // assert that if first repository has no write permissions, it will fail.
+ ResourceConflictException thrown =
+ assertThrows(
+ ResourceConflictException.class,
+ () -> gApi.changes().id(result1.getChangeId()).revertSubmission());
+ assertThat(thrown).hasMessageThat().contains(expected);
+
+ // assert that if the first repository has no write permissions and a change from another
+ // repository is trying to revert the submission, it will fail.
+ thrown =
+ assertThrows(
+ ResourceConflictException.class,
+ () -> gApi.changes().id(result2.getChangeId()).revertSubmission());
+ assertThat(thrown).hasMessageThat().contains(expected);
+ }
+
+ @Test
+ @GerritConfig(name = "change.submitWholeTopic", value = "true")
+ public void cantCreateRevertSubmissionWithoutCreateChangePermission() throws Exception {
+ String secondProject = "secondProject";
+ projectOperations.newProject().name(secondProject).create();
+ TestRepository<InMemoryRepository> secondRepo =
+ cloneProject(Project.nameKey("secondProject"), admin);
+ String topic = "topic";
+ PushOneCommit.Result result1 =
+ createChange(testRepo, "master", "first change", "a.txt", "message", topic);
+ PushOneCommit.Result result2 =
+ createChange(secondRepo, "master", "second change", "b.txt", "message", topic);
+ gApi.changes().id(result1.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(result2.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(result1.getChangeId()).revision(result1.getCommit().name()).submit();
+
+ // revoke create change permissions for the first repository.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.PUSH).ref("refs/for/*").group(REGISTERED_USERS))
+ .update();
+
+ // assert that if first repository has no write create change, it will fail.
+ PermissionDeniedException thrown =
+ assertThrows(
+ PermissionDeniedException.class,
+ () -> gApi.changes().id(result1.getChangeId()).revertSubmission());
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("not permitted: create change on refs/heads/master");
+
+ // assert that if the first repository has no create change permissions and a change from
+ // another repository is trying to revert the submission, it will fail.
+ thrown =
+ assertThrows(
+ PermissionDeniedException.class,
+ () -> gApi.changes().id(result2.getChangeId()).revertSubmission());
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("not permitted: create change on refs/heads/master");
+ }
+
+ @Test
+ @GerritConfig(name = "change.submitWholeTopic", value = "true")
+ public void cantCreateRevertSubmissionWithoutReadPermission() throws Exception {
+ String secondProject = "secondProject";
+ projectOperations.newProject().name(secondProject).create();
+ TestRepository<InMemoryRepository> secondRepo =
+ cloneProject(Project.nameKey("secondProject"), admin);
+ String topic = "topic";
+ PushOneCommit.Result result1 =
+ createChange(testRepo, "master", "first change", "a.txt", "message", topic);
+ PushOneCommit.Result result2 =
+ createChange(secondRepo, "master", "second change", "b.txt", "message", topic);
+ gApi.changes().id(result1.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(result2.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(result1.getChangeId()).revision(result1.getCommit().name()).submit();
+
+ // revoke read permissions for the first repository.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref("refs/heads/master").group(REGISTERED_USERS))
+ .update();
+
+ // assert that if first repository has no read permissions, it will fail.
+ ResourceNotFoundException resourceNotFoundException =
+ assertThrows(
+ ResourceNotFoundException.class,
+ () -> gApi.changes().id(result1.getChangeId()).revertSubmission());
+ assertThat(resourceNotFoundException)
+ .hasMessageThat()
+ .isEqualTo("Not found: " + result1.getChangeId());
+
+ // assert that if the first repository has no READ permissions and a change from another
+ // repository is trying to revert the submission, it will fail.
+ AuthException authException =
+ assertThrows(
+ AuthException.class, () -> gApi.changes().id(result2.getChangeId()).revertSubmission());
+ assertThat(authException).hasMessageThat().isEqualTo("read not permitted");
+ }
+
+ @Test
+ public void revertSubmissionPreservesReviewersAndCcs() throws Exception {
+ PushOneCommit.Result r = createChange("first change", "a.txt", "message");
+
+ ReviewInput in = ReviewInput.approve();
+ in.reviewer(user.email());
+ in.reviewer(accountCreator.user2().email(), ReviewerState.CC, true);
+ // Add user as reviewer that will create the revert
+ in.reviewer(accountCreator.admin2().email());
+
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(in);
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).submit();
+
+ // expect both the original reviewers and CCs to be preserved
+ // original owner should be added as reviewer, user requesting the revert (new owner) removed
+ requestScopeOperations.setApiUser(accountCreator.admin2().id());
+ Map<ReviewerState, Collection<AccountInfo>> result =
+ getChangeApis(gApi.changes().id(r.getChangeId()).revertSubmission()).get(0).get().reviewers;
+ assertThat(result).containsKey(ReviewerState.REVIEWER);
+
+ List<Integer> reviewers =
+ result.get(ReviewerState.REVIEWER).stream().map(a -> a._accountId).collect(toList());
+ assertThat(result).containsKey(ReviewerState.CC);
+ List<Integer> ccs =
+ result.get(ReviewerState.CC).stream().map(a -> a._accountId).collect(toList());
+ assertThat(ccs).containsExactly(accountCreator.user2().id().get());
+ assertThat(reviewers).containsExactly(user.id().get(), admin.id().get());
+ }
+
+ @Test
+ public void revertSubmissionNotifications() throws Exception {
+ PushOneCommit.Result firstResult = createChange("first change", "a.txt", "message");
+ approve(firstResult.getChangeId());
+ gApi.changes().id(firstResult.getChangeId()).addReviewer(user.email());
+ PushOneCommit.Result secondResult = createChange("second change", "b.txt", "other");
+ approve(secondResult.getChangeId());
+ gApi.changes().id(secondResult.getChangeId()).addReviewer(user.email());
+
+ gApi.changes()
+ .id(secondResult.getChangeId())
+ .revision(secondResult.getCommit().name())
+ .submit();
+
+ sender.clear();
+ RevertInput revertInput = new RevertInput();
+ revertInput.notify = NotifyHandling.ALL;
+
+ RevertSubmissionInfo revertChanges =
+ gApi.changes().id(secondResult.getChangeId()).revertSubmission(revertInput);
+
+ List<Message> messages = sender.getMessages();
+
+ assertThat(messages).hasSize(4);
+ assertThat(sender.getMessages(revertChanges.revertChanges.get(0).changeId, "newchange"))
+ .hasSize(1);
+ assertThat(sender.getMessages(firstResult.getChangeId(), "revert")).hasSize(1);
+ assertThat(sender.getMessages(revertChanges.revertChanges.get(1).changeId, "newchange"))
+ .hasSize(1);
+ assertThat(sender.getMessages(secondResult.getChangeId(), "revert")).hasSize(1);
+ }
+
+ @Test
+ public void suppressRevertSubmissionNotifications() throws Exception {
+ PushOneCommit.Result firstResult = createChange("first change", "a.txt", "message");
+ approve(firstResult.getChangeId());
+ gApi.changes().id(firstResult.getChangeId()).addReviewer(user.email());
+ PushOneCommit.Result secondResult = createChange("second change", "b.txt", "other");
+ approve(secondResult.getChangeId());
+ gApi.changes().id(secondResult.getChangeId()).addReviewer(user.email());
+
+ gApi.changes()
+ .id(secondResult.getChangeId())
+ .revision(secondResult.getCommit().name())
+ .submit();
+
+ RevertInput revertInput = new RevertInput();
+ revertInput.notify = NotifyHandling.NONE;
+
+ sender.clear();
+ gApi.changes().id(secondResult.getChangeId()).revertSubmission(revertInput);
+ assertThat(sender.getMessages()).isEmpty();
+ }
+
+ @Test
+ public void revertSubmissionOfSingleChange() throws Exception {
+ PushOneCommit.Result result = createChange("Change", "a.txt", "message");
+ approve(result.getChangeId());
+ gApi.changes().id(result.getChangeId()).current().submit();
+ List<ChangeApi> revertChanges =
+ getChangeApis(gApi.changes().id(result.getChangeId()).revertSubmission());
+
+ String sha1Commit = result.getCommit().getName();
+
+ assertThat(revertChanges.get(0).current().commit(false).parents.get(0).commit)
+ .isEqualTo(sha1Commit);
+
+ assertThat(revertChanges.get(0).current().files().get("a.txt").linesDeleted).isEqualTo(1);
+
+ assertThat(revertChanges.get(0).get().revertOf)
+ .isEqualTo(result.getChange().change().getChangeId());
+ assertThat(revertChanges.get(0).get().topic)
+ .startsWith("revert-" + result.getChange().change().getSubmissionId() + "-");
+ }
+
+ @Test
+ public void revertSubmissionWithSetTopic() throws Exception {
+ PushOneCommit.Result result = createChange();
+ gApi.changes().id(result.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(result.getChangeId()).topic("topic");
+ gApi.changes().id(result.getChangeId()).revision(result.getCommit().name()).submit();
+ RevertInput revertInput = new RevertInput();
+ revertInput.topic = "reverted-not-default";
+ assertThat(
+ gApi.changes()
+ .id(result.getChangeId())
+ .revertSubmission(revertInput)
+ .revertChanges
+ .get(0)
+ .topic)
+ .isEqualTo(revertInput.topic);
+ }
+
+ @Test
+ public void revertSubmissionWithSetMessage() throws Exception {
+ PushOneCommit.Result result = createChange();
+ gApi.changes().id(result.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(result.getChangeId()).revision(result.getCommit().name()).submit();
+ RevertInput revertInput = new RevertInput();
+ revertInput.message = "Message from input";
+ assertThat(
+ gApi.changes()
+ .id(result.getChangeId())
+ .revertSubmission(revertInput)
+ .revertChanges
+ .get(0)
+ .subject)
+ .isEqualTo(revertInput.message);
+ }
+
+ @Test
+ @GerritConfig(name = "change.submitWholeTopic", value = "true")
+ @UseClockStep
+ public void revertSubmissionDifferentRepositoriesWithDependantChange() throws Exception {
+ projectOperations.newProject().name("secondProject").create();
+ TestRepository<InMemoryRepository> secondRepo =
+ cloneProject(Project.nameKey("secondProject"), admin);
+ List<PushOneCommit.Result> resultCommits = new ArrayList<>();
+ String topic = "topic";
+ resultCommits.add(createChange(testRepo, "master", "first change", "a.txt", "message", topic));
+ resultCommits.add(
+ createChange(secondRepo, "master", "first change", "a.txt", "message", topic));
+ resultCommits.add(
+ createChange(secondRepo, "master", "second change", "b.txt", "Other message", topic));
+ for (PushOneCommit.Result result : resultCommits) {
+ approve(result.getChangeId());
+ }
+ // submit all changes
+ gApi.changes().id(resultCommits.get(1).getChangeId()).current().submit();
+ List<ChangeApi> revertChanges =
+ getChangeApis(gApi.changes().id(resultCommits.get(1).getChangeId()).revertSubmission());
+
+ // The reverts are by update time, so the reversal ensures that
+ // revertChanges[i] is the revert of resultCommits[i]
+ Collections.reverse(revertChanges);
+
+ assertThat(revertChanges.get(0).current().files().get("a.txt").linesDeleted).isEqualTo(1);
+ assertThat(revertChanges.get(1).current().files().get("a.txt").linesDeleted).isEqualTo(1);
+ assertThat(revertChanges.get(2).current().files().get("b.txt").linesDeleted).isEqualTo(1);
+ // has size 3 because of the same topic, and submitWholeTopic is true.
+ assertThat(gApi.changes().id(revertChanges.get(0).get()._number).submittedTogether())
+ .hasSize(3);
+
+ // expected messages on source change:
+ // 1. Uploaded patch set 1.
+ // 2. Patch Set 1: Code-Review+2
+ // 3. Change has been successfully merged by Administrator
+ // 4. Created a revert of this change as %s
+
+ for (int i = 0; i < resultCommits.size(); i++) {
+ assertThat(revertChanges.get(i).current().commit(false).parents.get(0).commit)
+ .isEqualTo(resultCommits.get(i).getCommit().getName());
+ assertThat(revertChanges.get(i).get().revertOf)
+ .isEqualTo(resultCommits.get(i).getChange().change().getChangeId());
+ List<ChangeMessageInfo> sourceMessages =
+ new ArrayList<>(gApi.changes().id(resultCommits.get(i).getChangeId()).get().messages);
+ assertThat(sourceMessages).hasSize(4);
+ String expectedMessage =
+ String.format(
+ "Created a revert of this change as %s", revertChanges.get(i).get().changeId);
+ assertThat(sourceMessages.get(3).message).isEqualTo(expectedMessage);
+ // Expected message on the created change: "Uploaded patch set 1."
+ List<ChangeMessageInfo> messages =
+ revertChanges.get(i).get().messages.stream().collect(toList());
+ assertThat(messages).hasSize(1);
+ assertThat(messages.get(0).message).isEqualTo("Uploaded patch set 1.");
+ assertThat(revertChanges.get(i).get().revertOf)
+ .isEqualTo(gApi.changes().id(resultCommits.get(i).getChangeId()).get()._number);
+ assertThat(revertChanges.get(i).get().topic)
+ .startsWith("revert-" + resultCommits.get(0).getChange().change().getSubmissionId());
+ }
+ }
+
+ @Test
+ public void cantRevertSubmissionWithAnOpenChange() throws Exception {
+ PushOneCommit.Result result = createChange("first change", "a.txt", "message");
+ approve(result.getChangeId());
+ ResourceConflictException thrown =
+ assertThrows(
+ ResourceConflictException.class,
+ () -> gApi.changes().id(result.getChangeId()).revertSubmission());
+ assertThat(thrown).hasMessageThat().isEqualTo("change is new.");
+ }
+
@Override
protected PushOneCommit.Result createChange() throws Exception {
return createChange("refs/for/master");
@@ -451,4 +795,13 @@
.create();
}
}
+
+ private List<ChangeApi> getChangeApis(RevertSubmissionInfo revertSubmissionInfo)
+ throws Exception {
+ List<ChangeApi> results = new ArrayList<>();
+ for (ChangeInfo changeInfo : revertSubmissionInfo.revertChanges) {
+ results.add(gApi.changes().id(changeInfo._number));
+ }
+ return results;
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index 2d94566..32941ff 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -53,6 +53,7 @@
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSetApproval;
+import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.CherryPickInput;
import com.google.gerrit.extensions.api.changes.DraftApi;
@@ -974,6 +975,42 @@
}
@Test
+ public void cherryPickToNonExistingBranch() throws Exception {
+ PushOneCommit.Result result = createChange();
+
+ CherryPickInput input = new CherryPickInput();
+ input.message = "foo bar";
+ input.destination = "non-existing";
+ // TODO(ekempin): This should rather result in an UnprocessableEntityException.
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.changes().id(result.getChangeId()).current().cherryPick(input));
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo(
+ String.format("Branch %s does not exist.", RefNames.REFS_HEADS + input.destination));
+ }
+
+ @Test
+ public void cherryPickToNonExistingBaseCommit() throws Exception {
+ createBranch(BranchNameKey.create(project, "foo"));
+ PushOneCommit.Result result = createChange();
+
+ CherryPickInput input = new CherryPickInput();
+ input.message = "foo bar";
+ input.destination = "foo";
+ input.base = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
+ UnprocessableEntityException thrown =
+ assertThrows(
+ UnprocessableEntityException.class,
+ () -> gApi.changes().id(result.getChangeId()).current().cherryPick(input));
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo(String.format("Base %s doesn't exist", input.base));
+ }
+
+ @Test
public void canRebase() throws Exception {
PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
PushOneCommit.Result r1 = push.to("refs/for/master");
diff --git a/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java b/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java
index b77825a..cad0b83 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java
+++ b/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java
@@ -241,7 +241,7 @@
}
private void enableSlaveMode() throws Exception {
- updateConfig(config -> config.setBoolean("container", null, "slave", true));
+ updateConfig(config -> config.setBoolean("container", null, "replica", true));
}
private void updateConfig(Consumer<Config> configConsumer) throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/rest/binding/ChangesRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/binding/ChangesRestApiBindingsIT.java
index 8a284d9..83bc3eb 100644
--- a/javatests/com/google/gerrit/acceptance/rest/binding/ChangesRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/binding/ChangesRestApiBindingsIT.java
@@ -84,6 +84,7 @@
RestCall.post("/changes/%s/rebase"),
RestCall.post("/changes/%s/restore"),
RestCall.post("/changes/%s/revert"),
+ RestCall.post("/changes/%s/revert_submission"),
RestCall.get("/changes/%s/pure_revert"),
RestCall.post("/changes/%s/submit"),
RestCall.get("/changes/%s/submitted_together"),
diff --git a/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java
index f48a603..48dc89f 100644
--- a/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java
@@ -69,6 +69,7 @@
RestCall.get("/projects/%s/statistics.git"),
RestCall.post("/projects/%s/index"),
RestCall.post("/projects/%s/gc"),
+ RestCall.post("/projects/%s/create.change"),
RestCall.get("/projects/%s/children"),
RestCall.get("/projects/%s/branches"),
RestCall.post("/projects/%s/branches:delete"),
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
index dda7bbd..8b51e7f 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
@@ -63,11 +63,25 @@
return gApi.changes().id(id).revision(1).actions();
}
+ protected Map<String, ActionInfo> getChangeActions(String id) throws Exception {
+ return gApi.changes().id(id).get().actions;
+ }
+
protected String getETag(String id) throws Exception {
return gApi.changes().id(id).current().etag();
}
@Test
+ public void changeActionOneMergedChangeHasReverts() throws Exception {
+ String changeId = createChangeWithTopic().getChangeId();
+ gApi.changes().id(changeId).current().review(ReviewInput.approve());
+ gApi.changes().id(changeId).current().submit();
+ Map<String, ActionInfo> actions = getChangeActions(changeId);
+ assertThat(actions).containsKey("revert");
+ assertThat(actions).containsKey("revert_submission");
+ }
+
+ @Test
public void revisionActionsOneChangePerTopicUnapproved() throws Exception {
String changeId = createChangeWithTopic().getChangeId();
Map<String, ActionInfo> actions = getActions(changeId);
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateChangeIT.java
new file mode 100644
index 0000000..ca3707d
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateChangeIT.java
@@ -0,0 +1,46 @@
+// Copyright (C) 2019 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.acceptance.rest.project;
+
+import static com.google.common.truth.Truth8.assertThat;
+import static com.google.gerrit.entities.RefNames.REFS_HEADS;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.extensions.api.projects.BranchInput;
+import com.google.gerrit.extensions.common.ChangeInput;
+import org.junit.Test;
+
+public class CreateChangeIT extends AbstractDaemonTest {
+
+ // Just a basic test. The real functionality is tested under the restapi.change acceptance tests.
+ @Test
+ public void basic() throws Exception {
+ BranchInput branchInput = new BranchInput();
+ branchInput.ref = "foo";
+ assertThat(gApi.projects().name(project.get()).branches().get().stream().map(i -> i.ref))
+ .doesNotContain(REFS_HEADS + branchInput.ref);
+ RestResponse r =
+ adminRestSession.put(
+ "/projects/" + project.get() + "/branches/" + branchInput.ref, branchInput);
+ r.assertCreated();
+
+ ChangeInput input = new ChangeInput();
+ input.branch = "foo";
+ input.subject = "subject";
+ RestResponse cr = adminRestSession.post("/projects/" + project.get() + "/create.change", input);
+ cr.assertCreated();
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index b22d319..bf2b915 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -1058,10 +1058,10 @@
Map<String, com.google.gerrit.entities.Comment> commentMapBefore =
DeleteCommentRewriter.getPublishedComments(
- noteUtil, changeId, reader, NoteMap.read(reader, commitBefore));
+ noteUtil, reader, NoteMap.read(reader, commitBefore));
Map<String, com.google.gerrit.entities.Comment> commentMapAfter =
DeleteCommentRewriter.getPublishedComments(
- noteUtil, changeId, reader, NoteMap.read(reader, commitAfter));
+ noteUtil, reader, NoteMap.read(reader, commitAfter));
if (commentMapBefore.containsKey(targetCommentUuid)) {
assertThat(commentMapAfter).containsKey(targetCommentUuid);
diff --git a/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java b/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java
index 7be90134..15c66eb 100644
--- a/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java
+++ b/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java
@@ -29,7 +29,7 @@
assertThat(
staticTemplateData(
"http://example.com/", null, null, new HashMap<>(), IndexHtmlUtilTest::ordain))
- .containsExactly("canonicalPath", "", "polymer2", "true", "staticResourcePath", ordain(""));
+ .containsExactly("canonicalPath", "", "staticResourcePath", ordain(""));
}
@Test
@@ -41,13 +41,7 @@
null,
new HashMap<>(),
IndexHtmlUtilTest::ordain))
- .containsExactly(
- "canonicalPath",
- "/gerrit",
- "polymer2",
- "true",
- "staticResourcePath",
- ordain("/gerrit"));
+ .containsExactly("canonicalPath", "/gerrit", "staticResourcePath", ordain("/gerrit"));
}
@Test
@@ -60,12 +54,7 @@
new HashMap<>(),
IndexHtmlUtilTest::ordain))
.containsExactly(
- "canonicalPath",
- "",
- "polymer2",
- "true",
- "staticResourcePath",
- ordain("http://my-cdn.com/foo/bar/"));
+ "canonicalPath", "", "staticResourcePath", ordain("http://my-cdn.com/foo/bar/"));
}
@Test
@@ -78,12 +67,7 @@
new HashMap<>(),
IndexHtmlUtilTest::ordain))
.containsExactly(
- "canonicalPath",
- "/gerrit",
- "polymer2",
- "true",
- "staticResourcePath",
- ordain("http://my-cdn.com/foo/bar/"));
+ "canonicalPath", "/gerrit", "staticResourcePath", ordain("http://my-cdn.com/foo/bar/"));
}
private static SanitizedContent ordain(String s) {
diff --git a/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java b/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
index c46c3da..a23ccab 100644
--- a/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
+++ b/javatests/com/google/gerrit/server/index/change/FakeChangeIndex.java
@@ -28,10 +28,11 @@
@Ignore
public class FakeChangeIndex implements ChangeIndex {
- static final Schema<ChangeData> V1 = new Schema<>(1, ImmutableList.of(ChangeField.STATUS));
+ static final Schema<ChangeData> V1 = new Schema<>(1, false, ImmutableList.of(ChangeField.STATUS));
static final Schema<ChangeData> V2 =
- new Schema<>(2, ImmutableList.of(ChangeField.STATUS, ChangeField.PATH, ChangeField.UPDATED));
+ new Schema<>(
+ 2, false, ImmutableList.of(ChangeField.STATUS, ChangeField.PATH, ChangeField.UPDATED));
private static class Source implements ChangeDataSource {
private final Predicate<ChangeData> p;
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index ed79be6..013939a 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -214,6 +214,26 @@
}
@Test
+ public void parseAssignee() throws Exception {
+ assertParseSucceeds(
+ "Update change\n"
+ + "\n"
+ + "Branch: refs/heads/master\n"
+ + "Change-id: I577fb248e474018276351785930358ec0450e9f7\n"
+ + "Patch-set: 1\n"
+ + "Assignee: Change Owner <1@gerrit>\n"
+ + "Subject: This is a test change\n");
+ assertParseSucceeds(
+ "Update change\n"
+ + "\n"
+ + "Branch: refs/heads/master\n"
+ + "Change-id: I577fb248e474018276351785930358ec0450e9f7\n"
+ + "Patch-set: 2\n"
+ + "Assignee:\n"
+ + "Subject: This is a test change\n");
+ }
+
+ @Test
public void parseTopic() throws Exception {
assertParseSucceeds(
"Update change\n"
@@ -551,8 +571,6 @@
private ChangeNotesParser newParser(ObjectId tip) throws Exception {
walk.reset();
ChangeNoteJson changeNoteJson = injector.getInstance(ChangeNoteJson.class);
- LegacyChangeNoteRead reader = injector.getInstance(LegacyChangeNoteRead.class);
- return new ChangeNotesParser(
- newChange().getId(), tip, walk, changeNoteJson, reader, args.metrics);
+ return new ChangeNotesParser(newChange().getId(), tip, walk, changeNoteJson, args.metrics);
}
}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
index d381e99..0d7f2bd 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
@@ -38,10 +38,12 @@
import com.google.gerrit.entities.converter.PatchSetApprovalProtoConverter;
import com.google.gerrit.entities.converter.PatchSetProtoConverter;
import com.google.gerrit.mail.Address;
+import com.google.gerrit.server.AssigneeStatusUpdate;
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto;
+import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AssigneeStatusUpdateProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ChangeColumnsProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerSetEntryProto;
@@ -242,17 +244,6 @@
}
@Test
- public void serializeAssignee() throws Exception {
- assertRoundTrip(
- newBuilder().columns(cols.toBuilder().assignee(Account.id(2000)).build()).build(),
- ChangeNotesStateProto.newBuilder()
- .setMetaId(SHA_BYTES)
- .setChangeId(ID.get())
- .setColumns(colsProto.toBuilder().setAssignee(2000).setHasAssignee(true))
- .build());
- }
-
- @Test
public void serializeStatus() throws Exception {
assertRoundTrip(
newBuilder().columns(cols.toBuilder().status(Change.Status.MERGED).build()).build(),
@@ -308,19 +299,6 @@
}
@Test
- public void serializePastAssignees() throws Exception {
- assertRoundTrip(
- newBuilder().pastAssignees(ImmutableSet.of(Account.id(2002), Account.id(2001))).build(),
- ChangeNotesStateProto.newBuilder()
- .setMetaId(SHA_BYTES)
- .setChangeId(ID.get())
- .setColumns(colsProto)
- .addPastAssignee(2002)
- .addPastAssignee(2001)
- .build());
- }
-
- @Test
public void serializeHashtags() throws Exception {
assertRoundTrip(
newBuilder().hashtags(ImmutableSet.of("tag2", "tag1")).build(),
@@ -611,6 +589,35 @@
}
@Test
+ public void serializeAssigneeUpdates() throws Exception {
+ assertRoundTrip(
+ newBuilder()
+ .assigneeUpdates(
+ ImmutableList.of(
+ AssigneeStatusUpdate.create(
+ new Timestamp(1212L), Account.id(1000), Optional.of(Account.id(2001))),
+ AssigneeStatusUpdate.create(
+ new Timestamp(3434L), Account.id(1000), Optional.empty())))
+ .build(),
+ ChangeNotesStateProto.newBuilder()
+ .setMetaId(SHA_BYTES)
+ .setChangeId(ID.get())
+ .setColumns(colsProto)
+ .addAssigneeUpdate(
+ AssigneeStatusUpdateProto.newBuilder()
+ .setDate(1212L)
+ .setUpdatedBy(1000)
+ .setCurrentAssignee(2001)
+ .setHasCurrentAssignee(true))
+ .addAssigneeUpdate(
+ AssigneeStatusUpdateProto.newBuilder()
+ .setDate(3434L)
+ .setUpdatedBy(1000)
+ .setHasCurrentAssignee(false))
+ .build());
+ }
+
+ @Test
public void serializeSubmitRecords() throws Exception {
SubmitRecord sr1 = new SubmitRecord();
sr1.status = SubmitRecord.Status.OK;
@@ -721,7 +728,6 @@
.put("changeId", Change.Id.class)
.put("serverId", String.class)
.put("columns", ChangeColumns.class)
- .put("pastAssignees", new TypeLiteral<ImmutableSet<Account.Id>>() {}.getType())
.put("hashtags", new TypeLiteral<ImmutableSet<String>>() {}.getType())
.put(
"patchSets",
@@ -738,6 +744,9 @@
.put(
"reviewerUpdates",
new TypeLiteral<ImmutableList<ReviewerStatusUpdate>>() {}.getType())
+ .put(
+ "assigneeUpdates",
+ new TypeLiteral<ImmutableList<AssigneeStatusUpdate>>() {}.getType())
.put("submitRecords", new TypeLiteral<ImmutableList<SubmitRecord>>() {}.getType())
.put("changeMessages", new TypeLiteral<ImmutableList<ChangeMessage>>() {}.getType())
.put(
@@ -762,7 +771,6 @@
.put("topic", String.class)
.put("originalSubject", String.class)
.put("submissionId", String.class)
- .put("assignee", Account.Id.class)
.put("status", Change.Status.class)
.put("isPrivate", boolean.class)
.put("workInProgress", boolean.class)
@@ -844,6 +852,19 @@
}
@Test
+ public void assigneeStatusUpdateMethods() throws Exception {
+ assertThatSerializedClass(AssigneeStatusUpdate.class)
+ .hasAutoValueMethods(
+ ImmutableMap.of(
+ "date",
+ Timestamp.class,
+ "updatedBy",
+ Account.Id.class,
+ "currentAssignee",
+ new TypeLiteral<Optional<Account.Id>>() {}.getType()));
+ }
+
+ @Test
public void submitRecordFields() throws Exception {
assertThatSerializedClass(SubmitRecord.class)
.hasFields(
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 2ed145e..5993206 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -43,11 +43,11 @@
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.CommentRange;
-import com.google.gerrit.entities.PatchLineComment.Status;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.mail.Address;
+import com.google.gerrit.server.AssigneeStatusUpdate;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ReviewerSet;
@@ -74,7 +74,6 @@
@Inject private DraftCommentNotes.Factory draftNotesFactory;
@Inject private ChangeNoteJson changeNoteJson;
- @Inject private LegacyChangeNoteRead legacyChangeNoteRead;
@Inject private @GerritServerId String serverId;
@@ -120,7 +119,7 @@
RevCommit commit = tr.commit().message("PS2").create();
ChangeUpdate update = newUpdate(c, changeOwner);
update.putComment(
- Status.PUBLISHED,
+ Comment.Status.PUBLISHED,
newComment(
c.currentPatchSetId(),
"a.txt",
@@ -182,7 +181,7 @@
RevCommit commit = tr.commit().message("PS2").create();
update = newUpdate(c, changeOwner);
update.putComment(
- Status.PUBLISHED,
+ Comment.Status.PUBLISHED,
newComment(
c.currentPatchSetId(),
"a.txt",
@@ -745,6 +744,35 @@
}
@Test
+ public void assigneeStatusUpdateChangeNotes() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, otherUser);
+ update.setAssignee(otherUserId);
+ update.commit();
+
+ update = newUpdate(c, changeOwner);
+ update.removeAssignee();
+ update.commit();
+
+ update = newUpdate(c, changeOwner);
+ update.setAssignee(changeOwner.getAccountId());
+ update.commit();
+
+ update = newUpdate(c, changeOwner);
+ update.setAssignee(otherUserId);
+ update.commit();
+
+ ChangeNotes notes = newNotes(c);
+ ImmutableList<AssigneeStatusUpdate> statusUpdates = notes.getAssigneeUpdates();
+ assertThat(statusUpdates).hasSize(4);
+ assertThat(statusUpdates.get(3).updatedBy()).isEqualTo(otherUserId);
+ assertThat(statusUpdates.get(3).currentAssignee()).hasValue(otherUserId);
+ assertThat(statusUpdates.get(2).currentAssignee()).isEmpty();
+ assertThat(statusUpdates.get(1).currentAssignee()).hasValue(changeOwner.getAccountId());
+ assertThat(statusUpdates.get(0).currentAssignee()).hasValue(otherUserId);
+ }
+
+ @Test
public void hashtagCommit() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
@@ -1061,7 +1089,7 @@
update.putApproval("Code-Review", (short) 1);
update.setChangeMessage("This is a message");
update.putComment(
- Status.PUBLISHED,
+ Comment.Status.PUBLISHED,
newComment(
c.currentPatchSetId(),
"a.txt",
@@ -1161,7 +1189,7 @@
update.setPatchSetId(psId2);
Timestamp ts = TimeUtil.nowTs();
update.putComment(
- Status.PUBLISHED,
+ Comment.Status.PUBLISHED,
newComment(
psId2,
"a.txt",
@@ -1246,7 +1274,7 @@
ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234"),
false);
update1.setPatchSetId(psId);
- update1.putComment(Status.PUBLISHED, comment1);
+ update1.putComment(Comment.Status.PUBLISHED, comment1);
updateManager.add(update1);
ChangeUpdate update2 = newUpdate(c, otherUser);
@@ -1268,12 +1296,7 @@
try (ChangeNotesRevWalk rw = ChangeNotesCommit.newRevWalk(repo)) {
ChangeNotesParser notesWithComments =
new ChangeNotesParser(
- c.getId(),
- commitWithComments.copy(),
- rw,
- changeNoteJson,
- legacyChangeNoteRead,
- args.metrics);
+ c.getId(), commitWithComments.copy(), rw, changeNoteJson, args.metrics);
ChangeNotesState state = notesWithComments.parseAll();
assertThat(state.approvals()).isEmpty();
assertThat(state.publishedComments()).hasSize(1);
@@ -1282,12 +1305,7 @@
try (ChangeNotesRevWalk rw = ChangeNotesCommit.newRevWalk(repo)) {
ChangeNotesParser notesWithApprovals =
new ChangeNotesParser(
- c.getId(),
- commitWithApprovals.copy(),
- rw,
- changeNoteJson,
- legacyChangeNoteRead,
- args.metrics);
+ c.getId(), commitWithApprovals.copy(), rw, changeNoteJson, args.metrics);
ChangeNotesState state = notesWithApprovals.parseAll();
assertThat(state.approvals()).hasSize(1);
@@ -1470,7 +1488,7 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment);
+ update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1500,7 +1518,7 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment);
+ update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1530,7 +1548,7 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment);
+ update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1560,7 +1578,7 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment);
+ update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1629,9 +1647,9 @@
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(psId2);
- update.putComment(Status.PUBLISHED, comment3);
- update.putComment(Status.PUBLISHED, comment2);
- update.putComment(Status.PUBLISHED, comment1);
+ update.putComment(Comment.Status.PUBLISHED, comment3);
+ update.putComment(Comment.Status.PUBLISHED, comment2);
+ update.putComment(Comment.Status.PUBLISHED, comment1);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1671,7 +1689,7 @@
false);
comment.setRealAuthor(changeOwner.getAccountId());
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment);
+ update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1709,7 +1727,7 @@
ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234"),
false);
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment);
+ update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1747,7 +1765,7 @@
commitId1,
false);
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, commentForBase);
+ update.putComment(Comment.Status.PUBLISHED, commentForBase);
update.commit();
update = newUpdate(c, otherUser);
@@ -1766,7 +1784,7 @@
commitId2,
false);
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, commentForPS);
+ update.putComment(Comment.Status.PUBLISHED, commentForPS);
update.commit();
assertThat(newNotes(c).getComments())
@@ -1805,7 +1823,7 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment1);
+ update.putComment(Comment.Status.PUBLISHED, comment1);
update.commit();
update = newUpdate(c, otherUser);
@@ -1824,7 +1842,7 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment2);
+ update.putComment(Comment.Status.PUBLISHED, comment2);
update.commit();
assertThat(newNotes(c).getComments())
@@ -1863,7 +1881,7 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment1);
+ update.putComment(Comment.Status.PUBLISHED, comment1);
update.commit();
update = newUpdate(c, otherUser);
@@ -1882,7 +1900,7 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment2);
+ update.putComment(Comment.Status.PUBLISHED, comment2);
update.commit();
assertThat(newNotes(c).getComments())
@@ -1921,7 +1939,7 @@
commitId1,
false);
update.setPatchSetId(ps1);
- update.putComment(Status.PUBLISHED, comment1);
+ update.putComment(Comment.Status.PUBLISHED, comment1);
update.commit();
incrementPatchSet(c);
@@ -1944,7 +1962,7 @@
commitId2,
false);
update.setPatchSetId(ps2);
- update.putComment(Status.PUBLISHED, comment2);
+ update.putComment(Comment.Status.PUBLISHED, comment2);
update.commit();
assertThat(newNotes(c).getComments())
@@ -1981,7 +1999,7 @@
commitId,
false);
update.setPatchSetId(ps1);
- update.putComment(Status.DRAFT, comment1);
+ update.putComment(Comment.Status.DRAFT, comment1);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1991,7 +2009,7 @@
update = newUpdate(c, otherUser);
update.setPatchSetId(ps1);
- update.putComment(Status.PUBLISHED, comment1);
+ update.putComment(Comment.Status.PUBLISHED, comment1);
update.commit();
notes = newNotes(c);
@@ -2044,8 +2062,8 @@
side,
commitId,
false);
- update.putComment(Status.DRAFT, comment1);
- update.putComment(Status.DRAFT, comment2);
+ update.putComment(Comment.Status.DRAFT, comment1);
+ update.putComment(Comment.Status.DRAFT, comment2);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2060,7 +2078,7 @@
// Publish first draft.
update = newUpdate(c, otherUser);
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment1);
+ update.putComment(Comment.Status.PUBLISHED, comment1);
update.commit();
notes = newNotes(c);
@@ -2115,8 +2133,8 @@
commitId2,
false);
- update.putComment(Status.DRAFT, baseComment);
- update.putComment(Status.DRAFT, psComment);
+ update.putComment(Comment.Status.DRAFT, baseComment);
+ update.putComment(Comment.Status.DRAFT, psComment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2131,8 +2149,8 @@
update = newUpdate(c, otherUser);
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, baseComment);
- update.putComment(Status.PUBLISHED, psComment);
+ update.putComment(Comment.Status.PUBLISHED, baseComment);
+ update.putComment(Comment.Status.PUBLISHED, psComment);
update.commit();
notes = newNotes(c);
@@ -2171,7 +2189,7 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Status.DRAFT, comment);
+ update.putComment(Comment.Status.DRAFT, comment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2216,7 +2234,7 @@
commitId1,
false);
update.setPatchSetId(ps1);
- update.putComment(Status.DRAFT, comment1);
+ update.putComment(Comment.Status.DRAFT, comment1);
update.commit();
incrementPatchSet(c);
@@ -2239,7 +2257,7 @@
commitId2,
false);
update.setPatchSetId(ps2);
- update.putComment(Status.DRAFT, comment2);
+ update.putComment(Comment.Status.DRAFT, comment2);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2283,7 +2301,7 @@
side,
commitId,
false);
- update.putComment(Status.PUBLISHED, comment);
+ update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
assertThat(repo.exactRef(changeMetaRef(c.getId()))).isNotNull();
@@ -2316,7 +2334,7 @@
side,
commitId,
false);
- update.putComment(Status.DRAFT, draft);
+ update.putComment(Comment.Status.DRAFT, draft);
update.commit();
String draftRef = refsDraftComments(c.getId(), otherUser.getAccountId());
@@ -2338,7 +2356,7 @@
side,
commitId,
false);
- update.putComment(Status.PUBLISHED, pub);
+ update.putComment(Comment.Status.PUBLISHED, pub);
update.commit();
assertThat(exactRefAllUsers(draftRef)).isEqualTo(old);
@@ -2369,7 +2387,7 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment);
+ update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
assertThat(newNotes(c).getComments())
@@ -2401,7 +2419,7 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment);
+ update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
assertThat(newNotes(c).getComments())
@@ -2453,8 +2471,8 @@
side,
commitId2,
false);
- update.putComment(Status.DRAFT, comment1);
- update.putComment(Status.DRAFT, comment2);
+ update.putComment(Comment.Status.DRAFT, comment1);
+ update.putComment(Comment.Status.DRAFT, comment2);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2463,8 +2481,8 @@
update = newUpdate(c, otherUser);
update.setPatchSetId(ps2);
- update.putComment(Status.PUBLISHED, comment1);
- update.putComment(Status.PUBLISHED, comment2);
+ update.putComment(Comment.Status.PUBLISHED, comment1);
+ update.putComment(Comment.Status.PUBLISHED, comment2);
update.commit();
notes = newNotes(c);
@@ -2511,8 +2529,8 @@
side,
commitId1,
false);
- update.putComment(Status.DRAFT, comment1);
- update.putComment(Status.DRAFT, comment2);
+ update.putComment(Comment.Status.DRAFT, comment1);
+ update.putComment(Comment.Status.DRAFT, comment2);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2522,7 +2540,7 @@
update = newUpdate(c, otherUser);
update.setPatchSetId(ps1);
- update.putComment(Status.PUBLISHED, comment2);
+ update.putComment(Comment.Status.PUBLISHED, comment2);
update.commit();
notes = newNotes(c);
@@ -2584,8 +2602,8 @@
side,
commitId1,
false);
- update.putComment(Status.DRAFT, comment1);
- update.putComment(Status.DRAFT, comment2);
+ update.putComment(Comment.Status.DRAFT, comment1);
+ update.putComment(Comment.Status.DRAFT, comment2);
update.commit();
String refName = refsDraftComments(c.getId(), otherUserId);
@@ -2593,7 +2611,7 @@
update = newUpdate(c, otherUser);
update.setPatchSetId(ps1);
- update.putComment(Status.PUBLISHED, comment2);
+ update.putComment(Comment.Status.PUBLISHED, comment2);
update.commit();
assertThat(exactRefAllUsers(refName)).isNotNull();
assertThat(exactRefAllUsers(refName)).isNotEqualTo(oldDraftId);
@@ -2619,7 +2637,7 @@
update = newUpdate(c, otherUser);
update.setPatchSetId(ps1);
- update.putComment(Status.PUBLISHED, comment1);
+ update.putComment(Comment.Status.PUBLISHED, comment1);
update.commit();
// Updating an unrelated comment causes the zombie comment to get fixed up.
@@ -2647,7 +2665,7 @@
(short) 1,
commitId,
false);
- update1.putComment(Status.PUBLISHED, comment1);
+ update1.putComment(Comment.Status.PUBLISHED, comment1);
ChangeUpdate update2 = newUpdate(c, otherUser);
Comment comment2 =
@@ -2664,7 +2682,7 @@
(short) 1,
commitId,
false);
- update2.putComment(Status.PUBLISHED, comment2);
+ update2.putComment(Comment.Status.PUBLISHED, comment2);
try (NoteDbUpdateManager manager = updateManagerFactory.create(project)) {
manager.add(update1);
@@ -2721,7 +2739,7 @@
(short) 1,
ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234"),
false);
- update.putComment(Status.PUBLISHED, comment);
+ update.putComment(Comment.Status.PUBLISHED, comment);
update.commit();
notes = newNotes(c);
diff --git a/javatests/com/google/gerrit/server/notedb/DraftCommentNotesTest.java b/javatests/com/google/gerrit/server/notedb/DraftCommentNotesTest.java
index e53da4c..bf49884 100644
--- a/javatests/com/google/gerrit/server/notedb/DraftCommentNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/DraftCommentNotesTest.java
@@ -18,7 +18,6 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
-import com.google.gerrit.entities.PatchLineComment.Status;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.server.util.time.TimeUtil;
import org.eclipse.jgit.lib.ObjectId;
@@ -32,7 +31,7 @@
Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(c.currentPatchSetId());
- update.putComment(Status.PUBLISHED, comment(c.currentPatchSetId()));
+ update.putComment(Comment.Status.PUBLISHED, comment(c.currentPatchSetId()));
update.commit();
assertThat(newNotes(c).getDraftComments(otherUserId)).isEmpty();
@@ -45,13 +44,13 @@
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(c.currentPatchSetId());
- update.putComment(Status.DRAFT, comment(c.currentPatchSetId()));
+ update.putComment(Comment.Status.DRAFT, comment(c.currentPatchSetId()));
update.commit();
assertThat(newNotes(c).getDraftComments(otherUserId)).hasSize(1);
assertableFanOutExecutor.assertInteractions(0);
update = newUpdate(c, otherUser);
- update.putComment(Status.PUBLISHED, comment(c.currentPatchSetId()));
+ update.putComment(Comment.Status.PUBLISHED, comment(c.currentPatchSetId()));
update.commit();
assertThat(newNotes(c).getDraftComments(otherUserId)).isEmpty();
@@ -64,7 +63,7 @@
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(c.currentPatchSetId());
- update.putComment(Status.DRAFT, comment(c.currentPatchSetId()));
+ update.putComment(Comment.Status.DRAFT, comment(c.currentPatchSetId()));
update.commit();
ChangeNotes notes = newNotes(c);
diff --git a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
index 0f3e148..e7f0812 100644
--- a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
+++ b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
@@ -631,19 +631,22 @@
public void rawDocument() throws Exception {
AccountInfo userInfo = gApi.accounts().id(admin.getAccountId().get()).get();
+ Schema<AccountState> schema = indexes.getSearchIndex().getSchema();
Optional<FieldBundle> rawFields =
indexes
.getSearchIndex()
.getRaw(
Account.id(userInfo._accountId),
QueryOptions.create(
- IndexConfig.createDefault(),
- 0,
- 1,
- indexes.getSearchIndex().getSchema().getStoredFields().keySet()));
+ IndexConfig.createDefault(), 0, 1, schema.getStoredFields().keySet()));
assertThat(rawFields).isPresent();
- assertThat(rawFields.get().getValue(AccountField.ID)).isEqualTo(userInfo._accountId);
+ if (schema.useLegacyNumericFields()) {
+ assertThat(rawFields.get().getValue(AccountField.ID)).isEqualTo(userInfo._accountId);
+ } else {
+ assertThat(Integer.valueOf(rawFields.get().getValue(AccountField.ID_STR)))
+ .isEqualTo(userInfo._accountId);
+ }
// The field EXTERNAL_ID_STATE is only supported from schema version 6.
if (getSchemaVersion() < 6) {
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index d4f836e..2c5fcc4 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -154,7 +154,7 @@
@Inject protected AllUsersName allUsersName;
@Inject protected BatchUpdate.Factory updateFactory;
@Inject protected ChangeInserter.Factory changeFactory;
- @Inject protected ChangeQueryBuilder queryBuilder;
+ @Inject protected Provider<ChangeQueryBuilder> queryBuilderProvider;
@Inject protected GerritApi gApi;
@Inject protected IdentifiedUser.GenericFactory userFactory;
@Inject protected ChangeIndexCollection indexes;
@@ -3052,6 +3052,7 @@
assertQuery(ChangeIndexPredicate.none());
+ ChangeQueryBuilder queryBuilder = queryBuilderProvider.get();
for (Predicate<ChangeData> matchingOneChange :
ImmutableList.of(
// One index query, one post-filtering query.
diff --git a/modules/jgit b/modules/jgit
index 009e078..8e356fc 160000
--- a/modules/jgit
+++ b/modules/jgit
@@ -1 +1 @@
-Subproject commit 009e07882fee2b0ad2ebe7ba44e2ff52c101f858
+Subproject commit 8e356fc45e2b169572088ebdfb931ca788a5e220
diff --git a/plugins/replication b/plugins/replication
index e5881ac..fefba17 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit e5881ace0e39fdd15a3ce42d1626ebaf62595cbf
+Subproject commit fefba17b04224e9346a50f6e66944142f4878179
diff --git a/polygerrit-ui/app/BUILD b/polygerrit-ui/app/BUILD
index 54b780b..6d30a14 100644
--- a/polygerrit-ui/app/BUILD
+++ b/polygerrit-ui/app/BUILD
@@ -18,8 +18,7 @@
],
),
outs = ["polygerrit_ui.zip"],
- # TODO(taoalpha): replace with gr-app.html once p2 fully rolled out
- app = "elements/gr-app-p2.html",
+ app = "elements/gr-app.html",
)
bower_component_bundle(
diff --git a/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html b/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html
index 7e68669..5e4e8fd 100644
--- a/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html
+++ b/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html
@@ -136,6 +136,7 @@
* * num {number} The number identifying the patch set
* * desc {!string} Optional patch set description
* * wip {boolean} If true, this patch set was never subject to review.
+ * * sha {string} hash of the commit
*
* The wip property is determined by the change's current work_in_progress
* property and its log of change messages.
@@ -148,14 +149,18 @@
if (!change) { return []; }
let patchNums = [];
if (change.revisions && Object.keys(change.revisions).length) {
+ const revisions = Object.keys(change.revisions).map(sha => {
+ return Object.assign({sha}, change.revisions[sha]);
+ });
patchNums =
- Gerrit.PatchSetBehavior.sortRevisions(Object.values(change.revisions))
+ Gerrit.PatchSetBehavior.sortRevisions(revisions)
.map(e => {
// TODO(kaspern): Mark which patchset an edit was made on, if an
// edit exists -- perhaps with a temporary description.
return {
num: e._number,
desc: e.description,
+ sha: e.sha,
};
});
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
index 8803c83..085ce6e 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
@@ -366,9 +366,10 @@
_sectionsChanged() {
// Flush DOM operations so that the list item elements will be loaded.
- Polymer.dom.flush();
- this.$.cursor.stops = this._getListItems();
- this.$.cursor.moveToStart();
+ Polymer.RenderStatus.afterNextRender(this, () => {
+ this.$.cursor.stops = this._getListItems();
+ this.$.cursor.moveToStart();
+ });
},
_isOutgoing(section) {
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
index dce2a55..4599f01 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
@@ -172,11 +172,11 @@
{_number: 2},
];
flushAsynchronousOperations();
- const elementItems = Polymer.dom(element.root).querySelectorAll(
- 'gr-change-list-item');
- assert.equal(elementItems.length, 3);
+ Polymer.RenderStatus.afterNextRender(element, () => {
+ const elementItems = Polymer.dom(element.root).querySelectorAll(
+ 'gr-change-list-item');
+ assert.equal(elementItems.length, 3);
- flush(() => {
assert.isTrue(elementItems[0].hasAttribute('selected'));
MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
assert.equal(element.selectedIndex, 1);
@@ -470,42 +470,44 @@
},
];
flushAsynchronousOperations();
- const elementItems = Polymer.dom(element.root).querySelectorAll(
- 'gr-change-list-item');
- assert.equal(elementItems.length, 9);
+ Polymer.RenderStatus.afterNextRender(element, () => {
+ const elementItems = Polymer.dom(element.root).querySelectorAll(
+ 'gr-change-list-item');
+ assert.equal(elementItems.length, 9);
- MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
- assert.equal(element.selectedIndex, 1);
- MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
+ MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
+ assert.equal(element.selectedIndex, 1);
+ MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
- const navStub = sandbox.stub(Gerrit.Nav, 'navigateToChange');
- assert.equal(element.selectedIndex, 2);
+ const navStub = sandbox.stub(Gerrit.Nav, 'navigateToChange');
+ assert.equal(element.selectedIndex, 2);
- MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
- assert.deepEqual(navStub.lastCall.args[0], {_number: 2},
- 'Should navigate to /c/2/');
+ MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
+ assert.deepEqual(navStub.lastCall.args[0], {_number: 2},
+ 'Should navigate to /c/2/');
- MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
- assert.equal(element.selectedIndex, 1);
- MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
- assert.deepEqual(navStub.lastCall.args[0], {_number: 1},
- 'Should navigate to /c/1/');
+ MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
+ assert.equal(element.selectedIndex, 1);
+ MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
+ assert.deepEqual(navStub.lastCall.args[0], {_number: 1},
+ 'Should navigate to /c/1/');
- MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
- MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
- MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
- assert.equal(element.selectedIndex, 4);
- MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
- assert.deepEqual(navStub.lastCall.args[0], {_number: 4},
- 'Should navigate to /c/4/');
+ MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
+ MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
+ MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
+ assert.equal(element.selectedIndex, 4);
+ MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
+ assert.deepEqual(navStub.lastCall.args[0], {_number: 4},
+ 'Should navigate to /c/4/');
- MockInteractions.pressAndReleaseKeyOn(element, 82); // 'r'
- const change = element._changeForIndex(element.selectedIndex);
- assert.equal(change.reviewed, true,
- 'Should mark change as reviewed');
- MockInteractions.pressAndReleaseKeyOn(element, 82); // 'r'
- assert.equal(change.reviewed, false,
- 'Should mark change as unreviewed');
+ MockInteractions.pressAndReleaseKeyOn(element, 82); // 'r'
+ const change = element._changeForIndex(element.selectedIndex);
+ assert.equal(change.reviewed, true,
+ 'Should mark change as reviewed');
+ MockInteractions.pressAndReleaseKeyOn(element, 82); // 'r'
+ assert.equal(change.reviewed, false,
+ 'Should mark change as unreviewed');
+ });
});
test('highlight attribute is updated correctly', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 9f842c1..3b09270 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -912,7 +912,7 @@
_handleActionTap(e) {
e.preventDefault();
let el = Polymer.dom(e).localTarget;
- while (el.is !== 'gr-button') {
+ while (el.tagName.toLowerCase() !== 'gr-button') {
if (!el.parentElement) { return; }
el = el.parentElement;
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 2994c8d..00d5b42 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -120,6 +120,8 @@
}
.changeMetadata {
border-right: 1px solid var(--border-color);
+ /* Limit meta section to half of the screen at max */
+ max-width: 50%;
}
.commitMessage {
font-family: var(--monospace-font-family);
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 6ac2195..7378acd 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -293,6 +293,12 @@
<div class="comments">Comments</div>
<div class="sizeBars">Size</div>
<div class="header-stats">Delta</div>
+ <template is="dom-if" if="[[_showDynamicColumns]]">
+ <template is="dom-repeat" items="[[_dynamicHeaderEndpoints]]" as="headerEndpoint">
+ <gr-endpoint-decorator name$="[[headerEndpoint]]">
+ </gr-endpoint-decorator>
+ </template>
+ </template>
<!-- Empty div here exists to keep spacing in sync with file rows. -->
<div class="reviewed hideOnEdit" hidden$="[[!_loggedIn]]"></div>
<div class="editFileControls showOnEdit"></div>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 561e21c..6b84901 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -183,7 +183,8 @@
_showDynamicColumns: {
type: Boolean,
- computed: '_computeShowDynamicColumns(_dynamicHeaderEndpoints)',
+ computed: '_computeShowDynamicColumns(_dynamicHeaderEndpoints, ' +
+ '_dynamicContentEndpoints, _dynamicSummaryEndpoints)',
},
/** @type {Array<string>} */
_dynamicHeaderEndpoints: {
@@ -1297,13 +1298,17 @@
return `sizeBars desktop ${hideClass}`;
},
- _computeShowDynamicColumns(dynamicHeaderEndpoints) {
- // During a design review, it was decided that dynamic columns should
- // remain hidden until column headers (including existing columns such as
- // "Comments") are in place to avoid confusion.
- // TODO(crbug.com/939904): Enable dispaying dynamic columns when there is
- // at least one of them registered.
- return false;
+ /**
+ * Shows registered dynamic columns iff the 'header', 'content' and
+ * 'summary' endpoints are regiestered the exact same number of times.
+ * Ideally, there should be a better way to enforce the expectation of the
+ * dependencies between dynamic endpoints.
+ */
+ _computeShowDynamicColumns(
+ headerEndpoints, contentEndpoints, summaryEndpoints) {
+ return headerEndpoints && contentEndpoints && summaryEndpoints &&
+ headerEndpoints.length === contentEndpoints.length &&
+ headerEndpoints.length === summaryEndpoints.length;
},
/**
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index 8db000c..bb1743b 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -901,17 +901,17 @@
test('patch set from revisions', () => {
const expected = [
- {num: 4, desc: 'test'},
- {num: 3, desc: 'test'},
- {num: 2, desc: 'test'},
- {num: 1, desc: 'test'},
+ {num: 4, desc: 'test', sha: 'rev4'},
+ {num: 3, desc: 'test', sha: 'rev3'},
+ {num: 2, desc: 'test', sha: 'rev2'},
+ {num: 1, desc: 'test', sha: 'rev1'},
];
const patchNums = element.computeAllPatchSets({
revisions: {
- rev3: {_number: 3, description: 'test'},
- rev1: {_number: 1, description: 'test'},
- rev4: {_number: 4, description: 'test'},
- rev2: {_number: 2, description: 'test'},
+ rev3: {_number: 3, description: 'test', date: 3},
+ rev1: {_number: 1, description: 'test', date: 1},
+ rev4: {_number: 4, description: 'test', date: 4},
+ rev2: {_number: 2, description: 'test', date: 2},
},
});
assert.equal(patchNums.length, expected.length);
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
index a9fde2b..f889bfc 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
@@ -62,6 +62,10 @@
behaviors: [Gerrit.PatchSetBehavior],
+ _getShaForPatch(patch) {
+ return patch.sha.substring(0, 10);
+ },
+
_computeBaseDropdownContent(availablePatches, patchNum, _sortedRevisions,
changeComments, revisionInfo) {
// Polymer 2: check for undefined
@@ -85,7 +89,7 @@
for (const basePatch of availablePatches) {
const basePatchNum = basePatch.num;
const entry = this._createDropdownEntry(basePatchNum, 'Patchset ',
- _sortedRevisions, changeComments);
+ _sortedRevisions, changeComments, this._getShaForPatch(basePatch));
dropdownContent.push(Object.assign({}, entry, {
disabled: this._computeLeftDisabled(
basePatch.num, patchNum, _sortedRevisions),
@@ -133,7 +137,7 @@
const patchNum = patch.num;
const entry = this._createDropdownEntry(
patchNum, patchNum === 'edit' ? '' : 'Patchset ', _sortedRevisions,
- changeComments);
+ changeComments, this._getShaForPatch(patch));
dropdownContent.push(Object.assign({}, entry, {
disabled: this._computeRightDisabled(basePatchNum, patchNum,
_sortedRevisions),
@@ -142,12 +146,17 @@
return dropdownContent;
},
- _createDropdownEntry(patchNum, prefix, sortedRevisions, changeComments) {
+ _computeText(patchNum, prefix, changeComments, sha) {
+ return `${prefix}${patchNum}` +
+ `${this._computePatchSetCommentsString(changeComments, patchNum)}`
+ + (` | ${sha}`);
+ },
+
+ _createDropdownEntry(patchNum, prefix, sortedRevisions, changeComments,
+ sha) {
const entry = {
triggerText: `${prefix}${patchNum}`,
- text: `${prefix}${patchNum}` +
- `${this._computePatchSetCommentsString(
- changeComments, patchNum)}`,
+ text: this._computeText(patchNum, prefix, changeComments, sha),
mobileText: this._computeMobileText(patchNum, changeComments,
sortedRevisions),
bottomText: `${this._computePatchSetDescription(
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html
index 922a11c..66fa974 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html
@@ -120,10 +120,10 @@
test('_computeBaseDropdownContent', () => {
const availablePatches = [
- {num: 'edit'},
- {num: 3},
- {num: 2},
- {num: 1},
+ {num: 'edit', sha: '1'},
+ {num: 3, sha: '2'},
+ {num: 2, sha: '3'},
+ {num: 1, sha: '4'},
];
const revisions = [
{
@@ -147,7 +147,7 @@
{
disabled: true,
triggerText: 'Patchset edit',
- text: 'Patchset edit',
+ text: 'Patchset edit | 1',
mobileText: 'edit',
bottomText: '',
value: 'edit',
@@ -155,7 +155,7 @@
{
disabled: true,
triggerText: 'Patchset 3',
- text: 'Patchset 3',
+ text: 'Patchset 3 | 2',
mobileText: '3',
bottomText: '',
value: 3,
@@ -164,7 +164,7 @@
{
disabled: true,
triggerText: 'Patchset 2',
- text: 'Patchset 2',
+ text: 'Patchset 2 | 3',
mobileText: '2 description',
bottomText: 'description',
value: 2,
@@ -172,7 +172,7 @@
{
disabled: true,
triggerText: 'Patchset 1',
- text: 'Patchset 1',
+ text: 'Patchset 1 | 4',
mobileText: '1',
bottomText: '',
value: 1,
@@ -197,10 +197,10 @@
];
element.revisionInfo = getInfo(element.revisions);
element.availablePatches = [
- {num: 1},
- {num: 2},
- {num: 3},
- {num: 'edit'},
+ {num: 1, sha: '1'},
+ {num: 2, sha: '2'},
+ {num: 3, sha: '3'},
+ {num: 'edit', sha: '4'},
];
element.patchNum = 2;
element.basePatchNum = 'PARENT';
@@ -223,10 +223,10 @@
];
element.revisionInfo = getInfo(element.revisions);
element.availablePatches = [
- {num: 'edit'},
- {num: 3},
- {num: 2},
- {num: 1},
+ {num: 'edit', sha: '1'},
+ {num: 3, sha: '2'},
+ {num: 2, sha: '3'},
+ {num: 1, sha: '4'},
];
element.patchNum = 2;
element.basePatchNum = 'PARENT';
@@ -250,10 +250,10 @@
];
element.revisionInfo = getInfo(element.revisions);
element.availablePatches = [
- {num: 1},
- {num: 2},
- {num: 3},
- {num: 'edit'},
+ {num: 1, sha: '1'},
+ {num: 2, sha: '2'},
+ {num: 3, sha: '3'},
+ {num: 'edit', sha: '4'},
];
element.patchNum = 2;
element.basePatchNum = 'PARENT';
@@ -274,10 +274,10 @@
];
element.revisionInfo = getInfo(element.revisions);
element.availablePatches = [
- {num: 1},
- {num: 2},
- {num: 3},
- {num: 'edit'},
+ {num: 1, sha: '1'},
+ {num: 2, sha: '2'},
+ {num: 3, sha: '3'},
+ {num: 'edit', sha: '4'},
];
element.patchNum = 2;
element.basePatchNum = 'PARENT';
@@ -293,10 +293,10 @@
test('_computePatchDropdownContent', () => {
const availablePatches = [
- {num: 'edit'},
- {num: 3},
- {num: 2},
- {num: 1},
+ {num: 'edit', sha: '1'},
+ {num: 3, sha: '2'},
+ {num: 2, sha: '3'},
+ {num: 1, sha: '4'},
];
const basePatchNum = 1;
const sortedRevisions = [
@@ -310,7 +310,7 @@
{
disabled: false,
triggerText: 'edit',
- text: 'edit',
+ text: 'edit | 1',
mobileText: 'edit',
bottomText: '',
value: 'edit',
@@ -318,7 +318,7 @@
{
disabled: false,
triggerText: 'Patchset 3',
- text: 'Patchset 3',
+ text: 'Patchset 3 | 2',
mobileText: '3',
bottomText: '',
value: 3,
@@ -327,7 +327,7 @@
{
disabled: false,
triggerText: 'Patchset 2',
- text: 'Patchset 2',
+ text: 'Patchset 2 | 3',
mobileText: '2 description',
bottomText: 'description',
value: 2,
@@ -335,7 +335,7 @@
{
disabled: true,
triggerText: 'Patchset 1',
- text: 'Patchset 1',
+ text: 'Patchset 1 | 4',
mobileText: '1',
bottomText: '',
value: 1,
diff --git a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
index 383687f..25c7738 100644
--- a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
+++ b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
@@ -259,12 +259,14 @@
lastNotify: {left: 1, right: 1},
};
+ const rangesCache = new Map();
+
this._processPromise = util.makeCancelable(this._loadHLJS()
.then(() => {
return new Promise(resolve => {
const nextStep = () => {
this._processHandle = null;
- this._processNextLine(state);
+ this._processNextLine(state, rangesCache);
// Move to the next line in the section.
state.lineIndex++;
@@ -321,12 +323,21 @@
* Highlight.js emits and emit a list of text ranges and classes for the
* markers.
* @param {string} str The string of HTML.
+ * @param {Map<string, !Array<!Object>>} rangesCache A map for caching
+ * ranges for each string. A cache is read and written by this method.
+ * Since diff is mostly comparing same file on two sides, there is good rate
+ * of duplication at least for parts that are on left and right parts.
* @return {!Array<!Object>} The list of ranges.
*/
- _rangesFromString(str) {
+ _rangesFromString(str, rangesCache) {
+ const cached = rangesCache.get(str);
+ if (cached) return cached;
+
const div = document.createElement('div');
div.innerHTML = str;
- return this._rangesFromElement(div, 0);
+ const ranges = this._rangesFromElement(div, 0);
+ rangesCache.set(str, ranges);
+ return ranges;
},
_rangesFromElement(elem, offset) {
@@ -357,7 +368,7 @@
* lines).
* @param {!Object} state The processing state for the layer.
*/
- _processNextLine(state) {
+ _processNextLine(state, rangesCache) {
let baseLine;
let revisionLine;
@@ -386,7 +397,8 @@
baseLine = this._workaround(this._baseLanguage, baseLine);
result = this._hljs.highlight(this._baseLanguage, baseLine, true,
state.baseContext);
- this.push('_baseRanges', this._rangesFromString(result.value));
+ this.push('_baseRanges',
+ this._rangesFromString(result.value, rangesCache));
state.baseContext = result.top;
}
@@ -395,7 +407,8 @@
revisionLine = this._workaround(this._revisionLanguage, revisionLine);
result = this._hljs.highlight(this._revisionLanguage, revisionLine,
true, state.revisionContext);
- this.push('_revisionRanges', this._rangesFromString(result.value));
+ this.push('_revisionRanges',
+ this._rangesFromString(result.value, rangesCache));
state.revisionContext = result.top;
}
},
diff --git a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer_test.html b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer_test.html
index 472db21..4e3492f 100644
--- a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer_test.html
@@ -388,10 +388,20 @@
'<span class="non-whtelisted-class">',
'<span class="gr-diff gr-syntax gr-syntax-keyword">public</span>',
'</span>'].join('');
- const result = element._rangesFromString(str);
+ const result = element._rangesFromString(str, new Map());
assert.notEqual(result.length, 0);
});
+ test('_rangesFromString cache same syntax markers', () => {
+ sandbox.spy(element, '_rangesFromElement');
+ const str =
+ '<span class="gr-diff gr-syntax gr-syntax-keyword">public</span>';
+ const cacheMap = new Map();
+ element._rangesFromString(str, cacheMap);
+ element._rangesFromString(str, cacheMap);
+ assert.isTrue(element._rangesFromElement.calledOnce);
+ });
+
test('_isSectionDone', () => {
let state = {sectionIndex: 0, lineIndex: 0};
assert.isFalse(element._isSectionDone(state));
diff --git a/polygerrit-ui/app/elements/gr-app-p2.html b/polygerrit-ui/app/elements/gr-app-p2.html
deleted file mode 100644
index 01727f3..0000000
--- a/polygerrit-ui/app/elements/gr-app-p2.html
+++ /dev/null
@@ -1,47 +0,0 @@
-<!--
-@license
-Copyright (C) 2019 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.
--->
-
-<!--
- TODO(taoalpha): This file only used by google,
- remove once polymer 2 fully rolled out.
--->
-<script>
- window.Gerrit = window.Gerrit || {};
-
- // Disable extra font load from paper-styles
- window.polymerSkipLoadingFontRoboto = true;
-</script>
-
-<link rel="import" href="/bower_components/polymer/polymer.html">
-<link rel="import" href="/bower_components/polymer-resin/standalone/polymer-resin.html">
-<link rel="import" href="/bower_components/polymer/lib/legacy/legacy-data-mixin.html">
-<link rel="import" href="../behaviors/safe-types-behavior/safe-types-behavior.html">
-<script>
- security.polymer_resin.install({
- allowedIdentifierPrefixes: [''],
- reportHandler: security.polymer_resin.CONSOLE_LOGGING_REPORT_HANDLER,
- safeTypesBridge: Gerrit.SafeTypes.safeTypesBridge,
- });
-</script>
-
-<link rel="import" href="./gr-app-element.html">
-<dom-module id="gr-app-p2">
- <template>
- <gr-app-element id="app-element"></gr-app-element>
- </template>
- <script src="gr-app-p2.js" crossorigin="anonymous"></script>
-</dom-module>
diff --git a/polygerrit-ui/app/elements/gr-app-p2.js b/polygerrit-ui/app/elements/gr-app-p2.js
deleted file mode 100644
index 638a1bc..0000000
--- a/polygerrit-ui/app/elements/gr-app-p2.js
+++ /dev/null
@@ -1,26 +0,0 @@
-/**
- * @license
- * Copyright (C) 2019 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.
- */
-
-// TODO(taoalpha): This file is only used by google,
-// remove once polymer fully rolled out.
-(function() {
- 'use strict';
-
- Polymer({
- is: 'gr-app-p2',
- });
-})();
diff --git a/polygerrit-ui/app/elements/gr-app.html b/polygerrit-ui/app/elements/gr-app.html
index a3364b4..d23de3d 100644
--- a/polygerrit-ui/app/elements/gr-app.html
+++ b/polygerrit-ui/app/elements/gr-app.html
@@ -15,26 +15,13 @@
limitations under the License.
-->
<script>
- // TODO(taoalpha): clean up after p2 fully rolled out
- if (!window.POLYMER2) {
- // This must be set prior to loading Polymer for the first time.
- if (localStorage.getItem('USE_SHADOW_DOM') === 'true') {
- window.Polymer = {
- dom: 'shadow',
- passiveTouchGestures: true,
- lazyRegister: true,
- };
- } else if (!window.Polymer) {
- window.Polymer = {
- passiveTouchGestures: true,
- lazyRegister: true,
- };
- }
+ if (!window.Polymer) {
+ window.Polymer = {
+ passiveTouchGestures: true,
+ lazyRegister: true,
+ };
}
window.Gerrit = window.Gerrit || {};
-
- // Disable extra font load from paper-styles
- window.polymerSkipLoadingFontRoboto = true;
</script>
<link rel="import" href="/bower_components/polymer/polymer.html">
diff --git a/polygerrit-ui/app/elements/gr-app.js b/polygerrit-ui/app/elements/gr-app.js
index 1f6c4a0..ac8ea1a 100644
--- a/polygerrit-ui/app/elements/gr-app.js
+++ b/polygerrit-ui/app/elements/gr-app.js
@@ -17,15 +17,6 @@
(function() {
'use strict';
- // Eagerly render Polymer components when backgrounded. (Skips
- // requestAnimationFrame.)
- // @see https://github.com/Polymer/polymer/issues/3851
- // @see Issue 4699
- // TODO(taoalpha): Remove once p2 fully rolled out
- if (!window.POLYMER2) {
- Polymer.RenderStatus._makeReady();
- }
-
Polymer({
is: 'gr-app',
});
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list_test.html b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list_test.html
index 312cc34..2b63d99 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list_test.html
@@ -68,7 +68,7 @@
assert.equal(element._computeMobileText(item), item.mobileText);
});
- test('options are selected and laid out correctly', () => {
+ test('options are selected and laid out correctly', done => {
element.value = 2;
element.items = [
{
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index f0df879..dec5b1d 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -1776,7 +1776,7 @@
opt_workInProgress, opt_baseChange, opt_baseCommit) {
return this._restApiHelper.send({
method: 'POST',
- url: '/changes/',
+ url: `/projects/${encodeURIComponent(project)}/create.change`,
body: {
project,
branch,
diff --git a/polygerrit-ui/server.go b/polygerrit-ui/server.go
index 1a2d299..35bdefd 100644
--- a/polygerrit-ui/server.go
+++ b/polygerrit-ui/server.go
@@ -43,6 +43,8 @@
host = flag.String("host", "gerrit-review.googlesource.com", "Host to proxy requests to")
scheme = flag.String("scheme", "https", "URL scheme")
cdnPattern = regexp.MustCompile("https://cdn.googlesource.com/polygerrit_ui/[0-9.]*")
+ webComponentPattern = regexp.MustCompile("webcomponentsjs-p2")
+ grAppPattern = regexp.MustCompile("gr-app-p2")
bundledPluginsPattern = regexp.MustCompile("https://cdn.googlesource.com/polygerrit_assets/[0-9.]*")
)
@@ -184,9 +186,13 @@
buf.ReadFrom(reader)
original := buf.String()
+ // Replace the webcomponentsjs-p2 with webcomponentsjs
+ replaced := webComponentPattern.ReplaceAllString(original, "webcomponentsjs")
+ replaced = grAppPattern.ReplaceAllString(replaced, "gr-app")
+
// Simply remove all CDN references, so files are loaded from the local file system or the proxy
// server instead.
- replaced := cdnPattern.ReplaceAllString(original, "")
+ replaced = cdnPattern.ReplaceAllString(replaced, "")
// Modify window.INITIAL_DATA so that it has the same effect as injectLocalPlugins. To achieve
// this let's add JavaScript lines at the end of the <script>...</script> snippet that also
diff --git a/proto/cache.proto b/proto/cache.proto
index 2a07640..10e0216 100644
--- a/proto/cache.proto
+++ b/proto/cache.proto
@@ -75,7 +75,7 @@
// Instead, we just take the tedious yet simple approach of having a "has_foo"
// field for each nullable field "foo", indicating whether or not foo is null.
//
-// Next ID: 20
+// Next ID: 23
message ChangeNotesStateProto {
// Effectively required, even though the corresponding ChangeNotesState field
// is optional, since the field is only absent when NoteDb is disabled, in
@@ -110,8 +110,8 @@
string submission_id = 13;
bool has_submission_id = 14;
- int32 assignee = 15;
- bool has_assignee = 16;
+ reserved 15; // assignee
+ reserved 16; // has_assignee
string status = 17;
bool has_status = 18;
@@ -130,7 +130,7 @@
// which case attempting to use the ChangeNotesCache is programmer error.
ChangeColumnsProto columns = 3;
- repeated int32 past_assignee = 4;
+ reserved 4; // past_assignee
repeated string hashtag = 5;
@@ -189,6 +189,14 @@
string server_id = 20;
bool has_server_id = 21;
+
+ message AssigneeStatusUpdateProto {
+ int64 date = 1;
+ int32 updated_by = 2;
+ int32 current_assignee = 3;
+ bool has_current_assignee = 4;
+ }
+ repeated AssigneeStatusUpdateProto assignee_update = 22;
}
diff --git a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
index 8598229b..3feb1b4 100644
--- a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
+++ b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
@@ -24,7 +24,6 @@
{@param? assetsBundle: ?} /** {string} Assets bundle .html file, served from $assetsPath. */
{@param? faviconPath: ?}
{@param? versionInfo: ?}
- {@param? polymer2: ?}
{@param? polyfillCE: ?}
{@param? polyfillSD: ?}
{@param? polyfillSC: ?}
@@ -40,9 +39,9 @@
</noscript>
<script>
+ // Disable extra font load from paper-styles
+ window.polymerSkipLoadingFontRoboto = true;
window.CLOSURE_NO_DEPS = true;
- // TODO(taoalpha): clean up once p2 fully rolled out
- {if $polymer2}window.POLYMER2 = true;{/if}
{if $canonicalPath != ''}window.CANONICAL_PATH = '{$canonicalPath}';{/if}
{if $versionInfo}window.VERSION_INFO = '{$versionInfo}';{/if}
{if $staticResourcePath != ''}window.STATIC_RESOURCE_PATH = '{$staticResourcePath}';{/if}
@@ -92,17 +91,8 @@
<link rel="import" href="{$assetsPath}/{$assetsBundle}">{\n}
{/if}
- // TODO(taoalpha): only used by Google, remove once polymer 2 fully rolled out
- {if $polymer2}
- <link rel="import" href="{$staticResourcePath}/elements/gr-app-p2.html">{\n}
- {else}
- <link rel="import" href="{$staticResourcePath}/elements/gr-app.html">{\n}
- {/if}
+ <link rel="import" href="{$staticResourcePath}/elements/gr-app.html">{\n}
<body unresolved>{\n}
- {if $polymer2}
- <gr-app-p2 id="app"></gr-app-p2>{\n}
- {else}
- <gr-app id="app"></gr-app>{\n}
- {/if}
+ <gr-app id="app"></gr-app>{\n}
{/template}
diff --git a/tools/eclipse/project.py b/tools/eclipse/project.py
index 0bc41fc..7f0e88b 100755
--- a/tools/eclipse/project.py
+++ b/tools/eclipse/project.py
@@ -203,7 +203,7 @@
testAtt.setAttribute('name', 'test')
testAtt.setAttribute('value', 'true')
atts.appendChild(testAtt)
- if "apt_generated" in path:
+ if "apt_generated" in path or "modules/jgit" in path:
if not atts:
atts = doc.createElement('attributes')
ignoreOptionalProblems = doc.createElement('attribute')
diff --git a/tools/maven/api.sh b/tools/maven/api.sh
index e72e3cb..c6049bf 100755
--- a/tools/maven/api.sh
+++ b/tools/maven/api.sh
@@ -60,7 +60,13 @@
set -o xtrace
fi
-bazel build //tools/maven:gen_${command} "$@" || \
- { echo "bazel failed to build gen_${command}. Use VERBOSE=1 for more info" ; exit 1 ; }
+if [[ `which bazelisk` ]]; then
+ BAZEL_CMD=bazelisk
+else
+ BAZEL_CMD=bazel
+fi
-./bazel-genfiles/tools/maven/${command}.sh
+${BAZEL_CMD} build //tools/maven:gen_${command} "$@" || \
+ { echo "${BAZEL_CMD} failed to build gen_${command}. Use VERBOSE=1 for more info" ; exit 1 ; }
+
+./bazel-bin/tools/maven/${command}.sh
diff --git a/tools/maven/gerrit-acceptance-framework_pom.xml b/tools/maven/gerrit-acceptance-framework_pom.xml
index 14a07d1..4240a9b 100644
--- a/tools/maven/gerrit-acceptance-framework_pom.xml
+++ b/tools/maven/gerrit-acceptance-framework_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-acceptance-framework</artifactId>
- <version>3.1.0-SNAPSHOT</version>
+ <version>3.2.0-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Acceptance Test Framework</name>
<description>Framework for Gerrit's acceptance tests</description>
diff --git a/tools/maven/gerrit-extension-api_pom.xml b/tools/maven/gerrit-extension-api_pom.xml
index 0cf1448..cf2b080 100644
--- a/tools/maven/gerrit-extension-api_pom.xml
+++ b/tools/maven/gerrit-extension-api_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-extension-api</artifactId>
- <version>3.1.0-SNAPSHOT</version>
+ <version>3.2.0-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Extension API</name>
<description>API for Gerrit Extensions</description>
diff --git a/tools/maven/gerrit-plugin-api_pom.xml b/tools/maven/gerrit-plugin-api_pom.xml
index ebb66b4..7d3c4f0 100644
--- a/tools/maven/gerrit-plugin-api_pom.xml
+++ b/tools/maven/gerrit-plugin-api_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-plugin-api</artifactId>
- <version>3.1.0-SNAPSHOT</version>
+ <version>3.2.0-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Plugin API</name>
<description>API for Gerrit Plugins</description>
diff --git a/tools/maven/gerrit-war_pom.xml b/tools/maven/gerrit-war_pom.xml
index 51e517b..9478283 100644
--- a/tools/maven/gerrit-war_pom.xml
+++ b/tools/maven/gerrit-war_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-war</artifactId>
- <version>3.1.0-SNAPSHOT</version>
+ <version>3.2.0-SNAPSHOT</version>
<packaging>war</packaging>
<name>Gerrit Code Review - WAR</name>
<description>Gerrit WAR</description>
diff --git a/version.bzl b/version.bzl
index 42a576c..fb1e5ca 100644
--- a/version.bzl
+++ b/version.bzl
@@ -2,4 +2,4 @@
# Used by :api_install and :api_deploy targets
# when talking to the destination repository.
#
-GERRIT_VERSION = "3.1.0-SNAPSHOT"
+GERRIT_VERSION = "3.2.0-SNAPSHOT"
