Merge "Add support for images in diffs"
diff --git a/Documentation/cmd-index-changes.txt b/Documentation/cmd-index-changes.txt
new file mode 100644
index 0000000..8566827
--- /dev/null
+++ b/Documentation/cmd-index-changes.txt
@@ -0,0 +1,40 @@
+= gerrit index changes
+
+== NAME
+gerrit index changes - Index one or more changes.
+
+== SYNOPSIS
+--
+'ssh' -p <port> <host> 'gerrit index changes' <CHANGE> [<CHANGE> ...]
+--
+
+== DESCRIPTION
+Indexes one or more changes.
+
+Changes can be specified in the link:rest-api-changes.html#change-id[same format]
+supported by the REST API.
+
+== ACCESS
+Caller must have the 'Maintain Server' capability, or be the owner of the change
+to be indexed.
+
+== SCRIPTING
+This command is intended to be used in scripts.
+
+== OPTIONS
+--CHANGE::
+ Required; changes to be indexed.
+
+== EXAMPLES
+Index changes with legacy ID numbers 1 and 2.
+
+====
+ $ ssh -p 29418 user@review.example.com gerrit index changes 1 2
+====
+
+GERRIT
+------
+Part of link:index.html[Gerrit Code Review]
+
+SEARCHBOX
+---------
diff --git a/Documentation/cmd-index.txt b/Documentation/cmd-index.txt
index 90212fb..e244228 100644
--- a/Documentation/cmd-index.txt
+++ b/Documentation/cmd-index.txt
@@ -126,6 +126,9 @@
link:cmd-index-start.html[gerrit index start]::
Start the online indexer.
+link:cmd-index-changes.html[gerrit index changes]::
+ Index one or more changes.
+
link:cmd-logging-ls-level.html[gerrit logging ls-level]::
List loggers and their logging level.
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
index 84e7557..f2c701b 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
@@ -298,7 +298,7 @@
throws OrmException, NoSuchChangeException {
Iterable<Account.Id> actualIds = approvalsUtil
.getReviewers(db, notesFactory.createChecked(db, c))
- .values();
+ .all();
assertThat(actualIds).containsExactlyElementsIn(
Sets.newHashSet(TestAccount.ids(expectedReviewers)));
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index dfa0336c..5d038c4 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -629,7 +629,7 @@
.votes();
assertThat(m).hasSize(1);
- assertThat(m).containsEntry("Code-Review", new Short((short)2));
+ assertThat(m).containsEntry("Code-Review", Short.valueOf((short)2));
setApiUser(user);
gApi.changes()
@@ -643,7 +643,7 @@
.votes();
assertThat(m).hasSize(1);
- assertThat(m).containsEntry("Code-Review", new Short((short)-1));
+ assertThat(m).containsEntry("Code-Review", Short.valueOf((short)-1));
}
@Test
@@ -681,7 +681,7 @@
// When NoteDb is disabled there is a dummy 0 approval on the change so
// that the user is still returned as CC when all votes of that user have
// been deleted.
- assertThat(m).containsEntry("Code-Review", new Short((short)0));
+ assertThat(m).containsEntry("Code-Review", Short.valueOf((short)0));
}
ChangeInfo c = gApi.changes()
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmoduleSectionParserIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmoduleSectionParserIT.java
new file mode 100644
index 0000000..6a4454f
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmoduleSectionParserIT.java
@@ -0,0 +1,370 @@
+// Copyright (C) 2016 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.git;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.Sets;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.SubmoduleSubscription;
+import com.google.gerrit.server.util.SubmoduleSectionParser;
+
+import org.eclipse.jgit.lib.Config;
+import org.junit.Test;
+
+import java.util.Set;
+
+public class SubmoduleSectionParserIT extends AbstractDaemonTest {
+ private static final String THIS_SERVER = "http://localhost/";
+
+ @Test
+ public void testFollowMasterBranch() throws Exception {
+ Project.NameKey p = createProject("a");
+ Config cfg = new Config();
+ cfg.fromText(""
+ + "[submodule \"a\"]\n"
+ + "path = localpath-to-a\n"
+ + "url = ssh://localhost/" + p.get() + "\n"
+ + "branch = master\n");
+ Branch.NameKey targetBranch = new Branch.NameKey(
+ new Project.NameKey("project"), "master");
+
+ Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
+ cfg, THIS_SERVER, targetBranch).parseAllSections();
+
+ Set<SubmoduleSubscription> expected = Sets.newHashSet(
+ new SubmoduleSubscription(targetBranch, new Branch.NameKey(
+ p, "master"), "localpath-to-a"));
+
+ assertThat(res).containsExactlyElementsIn(expected);
+ }
+
+ @Test
+ public void testFollowMatchingBranch() throws Exception {
+ Project.NameKey p = createProject("a");
+ Config cfg = new Config();
+ cfg.fromText(""
+ + "[submodule \"a\"]\n"
+ + "path = a\n"
+ + "url = ssh://localhost/" + p.get() + "\n"
+ + "branch = .\n");
+
+ Branch.NameKey targetBranch1 = new Branch.NameKey(
+ new Project.NameKey("project"), "master");
+
+ Set<SubmoduleSubscription> res1 = new SubmoduleSectionParser(
+ cfg, THIS_SERVER, targetBranch1).parseAllSections();
+
+ Set<SubmoduleSubscription> expected1 = Sets.newHashSet(
+ new SubmoduleSubscription(targetBranch1, new Branch.NameKey(
+ p, "master"), "a"));
+
+ assertThat(res1).containsExactlyElementsIn(expected1);
+
+ Branch.NameKey targetBranch2 = new Branch.NameKey(
+ new Project.NameKey("project"), "somebranch");
+
+ Set<SubmoduleSubscription> res2 = new SubmoduleSectionParser(
+ cfg, THIS_SERVER, targetBranch2).parseAllSections();
+
+ Set<SubmoduleSubscription> expected2 = Sets.newHashSet(
+ new SubmoduleSubscription(targetBranch2, new Branch.NameKey(
+ p, "somebranch"), "a"));
+
+ assertThat(res2).containsExactlyElementsIn(expected2);
+ }
+
+ @Test
+ public void testFollowAnotherBranch() throws Exception {
+ Project.NameKey p = createProject("a");
+ Config cfg = new Config();
+ cfg.fromText(""
+ + "[submodule \"a\"]\n"
+ + "path = a\n"
+ + "url = ssh://localhost/" + p.get() + "\n"
+ + "branch = anotherbranch\n");
+
+ Branch.NameKey targetBranch = new Branch.NameKey(
+ new Project.NameKey("project"), "master");
+
+ Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
+ cfg, THIS_SERVER, targetBranch).parseAllSections();
+
+ Set<SubmoduleSubscription> expected = Sets.newHashSet(
+ new SubmoduleSubscription(targetBranch, new Branch.NameKey(
+ p, "anotherbranch"), "a"));
+
+ assertThat(res).containsExactlyElementsIn(expected);
+ }
+
+ @Test
+ public void testWithAnotherURI() throws Exception {
+ Project.NameKey p = createProject("a");
+ Config cfg = new Config();
+ cfg.fromText(""
+ + "[submodule \"a\"]\n"
+ + "path = a\n"
+ + "url = http://localhost:80/" + p.get() + "\n"
+ + "branch = master\n");
+
+ Branch.NameKey targetBranch = new Branch.NameKey(
+ new Project.NameKey("project"), "master");
+
+ Set<SubmoduleSubscription> res =new SubmoduleSectionParser(
+ cfg, THIS_SERVER, targetBranch).parseAllSections();
+
+ Set<SubmoduleSubscription> expected = Sets.newHashSet(
+ new SubmoduleSubscription(targetBranch, new Branch.NameKey(
+ p, "master"), "a"));
+
+ assertThat(res).containsExactlyElementsIn(expected);
+ }
+
+ @Test
+ public void testWithSlashesInProjectName() throws Exception {
+ Project.NameKey p = createProject("project/with/slashes/a");
+ Config cfg = new Config();
+ cfg.fromText(""
+ + "[submodule \"project/with/slashes/a\"]\n"
+ + "path = a\n"
+ + "url = http://localhost:80/" + p.get() + "\n"
+ + "branch = master\n");
+
+ Branch.NameKey targetBranch = new Branch.NameKey(
+ new Project.NameKey("project"), "master");
+
+ Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
+ cfg, THIS_SERVER, targetBranch).parseAllSections();
+
+ Set<SubmoduleSubscription> expected = Sets.newHashSet(
+ new SubmoduleSubscription(targetBranch, new Branch.NameKey(
+ p, "master"), "a"));
+
+ assertThat(res).containsExactlyElementsIn(expected);
+ }
+
+ @Test
+ public void testWithSlashesInPath() throws Exception {
+ Project.NameKey p = createProject("a");
+ Config cfg = new Config();
+ cfg.fromText(""
+ + "[submodule \"a\"]\n"
+ + "path = a/b/c/d/e\n"
+ + "url = http://localhost:80/" + p.get() + "\n"
+ + "branch = master\n");
+
+ Branch.NameKey targetBranch = new Branch.NameKey(
+ new Project.NameKey("project"), "master");
+
+ Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
+ cfg, THIS_SERVER, targetBranch).parseAllSections();
+
+ Set<SubmoduleSubscription> expected = Sets.newHashSet(
+ new SubmoduleSubscription(targetBranch, new Branch.NameKey(
+ p, "master"), "a/b/c/d/e"));
+
+ assertThat(res).containsExactlyElementsIn(expected);
+ }
+
+ @Test
+ public void testWithMoreSections() throws Exception {
+ Project.NameKey p1 = createProject("a");
+ Project.NameKey p2 = createProject("b");
+ Config cfg = new Config();
+ cfg.fromText(""
+ + "[submodule \"a\"]\n"
+ + " path = a\n"
+ + " url = ssh://localhost/" + p1.get() + "\n"
+ + " branch = .\n"
+ + "[submodule \"b\"]\n"
+ + " path = b\n"
+ + " url = http://localhost:80/" + p2.get() + "\n"
+ + " branch = master\n");
+
+ Branch.NameKey targetBranch = new Branch.NameKey(
+ new Project.NameKey("project"), "master");
+
+ Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
+ cfg, THIS_SERVER, targetBranch).parseAllSections();
+
+ Set<SubmoduleSubscription> expected = Sets.newHashSet(
+ new SubmoduleSubscription(targetBranch, new Branch.NameKey(
+ p1, "master"), "a"),
+ new SubmoduleSubscription(targetBranch, new Branch.NameKey(
+ p2, "master"), "b"));
+
+ assertThat(res).containsExactlyElementsIn(expected);
+ }
+
+ @Test
+ public void testWithSubProjectFound() throws Exception {
+ Project.NameKey p1 = createProject("a/b");
+ Project.NameKey p2 = createProject("b");
+ Config cfg = new Config();
+ cfg.fromText("\n"
+ + "[submodule \"a/b\"]\n"
+ + "path = a/b\n"
+ + "url = ssh://localhost/" + p1.get() + "\n"
+ + "branch = .\n"
+ + "[submodule \"b\"]\n"
+ + "path = b\n"
+ + "url = http://localhost/" + p2.get() + "\n"
+ + "branch = .\n");
+
+ Branch.NameKey targetBranch = new Branch.NameKey(
+ new Project.NameKey("project"), "master");
+
+ Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
+ cfg, THIS_SERVER, targetBranch).parseAllSections();
+
+ Set<SubmoduleSubscription> expected = Sets.newHashSet(
+ new SubmoduleSubscription(targetBranch, new Branch.NameKey(
+ p2, "master"), "b"),
+ new SubmoduleSubscription(targetBranch, new Branch.NameKey(
+ p1, "master"), "a/b"));
+
+ assertThat(res).containsExactlyElementsIn(expected);
+ }
+
+ @Test
+ public void testWithAnInvalidSection() throws Exception {
+ Project.NameKey p1 = createProject("a");
+ Project.NameKey p2 = createProject("b");
+ Project.NameKey p3 = createProject("d");
+ Project.NameKey p4 = createProject("e");
+ Config cfg = new Config();
+ cfg.fromText("\n"
+ + "[submodule \"a\"]\n"
+ + " path = a\n"
+ + " url = ssh://localhost/" + p1.get() + "\n"
+ + " branch = .\n"
+ + "[submodule \"b\"]\n"
+ // path missing
+ + " url = http://localhost:80/" + p2.get() + "\n"
+ + " branch = master\n"
+ + "[submodule \"c\"]\n"
+ + " path = c\n"
+ // url missing
+ + " branch = .\n"
+ + "[submodule \"d\"]\n"
+ + " path = d-parent/the-d-folder\n"
+ + " url = ssh://localhost/" + p3.get() + "\n"
+ // branch missing
+ + "[submodule \"e\"]\n"
+ + " path = e\n"
+ + " url = ssh://localhost/" + p4.get() + "\n"
+ + " branch = refs/heads/master\n");
+
+ Branch.NameKey targetBranch = new Branch.NameKey(
+ new Project.NameKey("project"), "master");
+
+ Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
+ cfg, THIS_SERVER, targetBranch).parseAllSections();
+
+ Set<SubmoduleSubscription> expected = Sets.newHashSet(
+ new SubmoduleSubscription(targetBranch, new Branch.NameKey(
+ p1, "master"), "a"),
+ new SubmoduleSubscription(targetBranch, new Branch.NameKey(
+ p4, "master"), "e"));
+
+ assertThat(res).containsExactlyElementsIn(expected);
+ }
+
+ @Test
+ public void testWithSectionOfNonexistingProject() throws Exception {
+ Config cfg = new Config();
+ cfg.fromText("\n"
+ + "[submodule \"a\"]\n"
+ + "path = a\n"
+ + "url = ssh://non-localhost/a\n"
+ // Project "a" doesn't exist
+ + "branch = .\\n");
+
+ Branch.NameKey targetBranch = new Branch.NameKey(
+ new Project.NameKey("project"), "master");
+
+ Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
+ cfg, THIS_SERVER, targetBranch).parseAllSections();
+
+ assertThat(res).isEmpty();
+ }
+
+ @Test
+ public void testWithSectionToOtherServer() throws Exception {
+ Project.NameKey p1 = createProject("a");
+ Config cfg = new Config();
+ cfg.fromText(""
+ + "[submodule \"a\"]"
+ + "path = a"
+ + "url = ssh://non-localhost/" + p1.get() + "\n"
+ + "branch = .");
+
+ Branch.NameKey targetBranch = new Branch.NameKey(
+ new Project.NameKey("project"), "master");
+
+ Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
+ cfg, THIS_SERVER, targetBranch).parseAllSections();
+
+ assertThat(res).isEmpty();
+ }
+
+ @Test
+ public void testWithRelativeURI() throws Exception {
+ Project.NameKey p1 = createProject("a");
+ Config cfg = new Config();
+ cfg.fromText(""
+ + "[submodule \"a\"]\n"
+ + "path = a\n"
+ + "url = ../" + p1.get() + "\n"
+ + "branch = master\n");
+
+ Branch.NameKey targetBranch = new Branch.NameKey(
+ new Project.NameKey("project"), "master");
+
+ Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
+ cfg, THIS_SERVER, targetBranch).parseAllSections();
+
+ Set<SubmoduleSubscription> expected = Sets.newHashSet(
+ new SubmoduleSubscription(targetBranch, new Branch.NameKey(
+ p1, "master"), "a"));
+
+ assertThat(res).containsExactlyElementsIn(expected);
+ }
+
+ @Test
+ public void testWithDeepRelativeURI() throws Exception {
+ Project.NameKey p1 = createProject("a");
+ Config cfg = new Config();
+ cfg.fromText(""
+ + "[submodule \"a\"]\n"
+ + "path = a\n"
+ + "url = ../../" + p1.get() + "\n"
+ + "branch = master\n");
+
+ Branch.NameKey targetBranch = new Branch.NameKey(
+ new Project.NameKey("nested/project"), "master");
+
+ Set<SubmoduleSubscription> res = new SubmoduleSectionParser(
+ cfg, THIS_SERVER, targetBranch).parseAllSections();
+
+ Set<SubmoduleSubscription> expected = Sets.newHashSet(
+ new SubmoduleSubscription(targetBranch, new Branch.NameKey(
+ p1, "master"), "a"));
+
+ assertThat(res).containsExactlyElementsIn(expected);
+ }
+}
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/SubscribeSection.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/SubscribeSection.java
index b05f335..6cee630 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/SubscribeSection.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/SubscribeSection.java
@@ -68,4 +68,18 @@
public Collection<RefSpec> getRefSpecs() {
return Collections.unmodifiableCollection(refSpecs);
}
+
+ @Override
+ public String toString() {
+ StringBuilder ret = new StringBuilder();
+ ret.append("[SubscribeSection, project=");
+ ret.append(project);
+ ret.append(", refs=[");
+ for (RefSpec r : refSpecs) {
+ ret.append(r.toString());
+ ret.append(", ");
+ }
+ ret.append("]");
+ return ret.toString();
+ }
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/QueryScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/QueryScreen.java
index e7fba34..80117be 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/QueryScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/QueryScreen.java
@@ -20,11 +20,23 @@
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gwt.regexp.shared.RegExp;
import com.google.gwt.user.client.rpc.AsyncCallback;
import com.google.gwtorm.client.KeyUtil;
public class QueryScreen extends PagedSingleListScreen implements
ChangeListScreen {
+ // Legacy numeric identifier.
+ private static final RegExp NUMERIC_ID = RegExp.compile("^[1-9][0-9]*$");
+ // Commit SHA1 hash
+ private static final RegExp COMMIT_SHA1 =
+ RegExp.compile("^([0-9a-fA-F]{4," + RevId.LEN + "})$");
+ // Change-Id
+ private static final String ID_PATTERN = "[iI][0-9a-f]{4,}$";
+ private static final RegExp CHANGE_ID = RegExp.compile("^" + ID_PATTERN);
+ private static final RegExp CHANGE_ID_TRIPLET =
+ RegExp.compile("^(.)+~(.)+~" + ID_PATTERN);
+
public static QueryScreen forQuery(String query) {
return forQuery(query, 0);
}
@@ -80,24 +92,9 @@
}
private static boolean isSingleQuery(String query) {
- if (query.matches("^[1-9][0-9]*$")) {
- // Legacy numeric identifier.
- //
- return true;
- }
-
- if (query.matches("^[iI][0-9a-f]{4,}$")) {
- // Newer style Change-Id.
- //
- return true;
- }
-
- if (query.matches("^([0-9a-fA-F]{4," + RevId.LEN + "})$")) {
- // Commit SHA-1 of any change.
- //
- return true;
- }
-
- return false;
+ return NUMERIC_ID.test(query)
+ || CHANGE_ID.test(query)
+ || CHANGE_ID_TRIPLET.test(query)
+ || COMMIT_SHA1.test(query);
}
}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java
index 7d8d22c..fa2e0e3 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java
@@ -161,6 +161,7 @@
key = manager.createKey(id);
val = manager.createVal(key, id, rememberMe, identity, null, null);
saveCookie();
+ user = identified.create(val.getAccountId());
}
/** Set the user account for this current request only. */
@@ -178,6 +179,7 @@
key = null;
val = null;
saveCookie();
+ user = anonymousProvider.get();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
index dc3a6b1..847d559 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
@@ -14,8 +14,6 @@
package com.google.gerrit.server;
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import com.google.common.annotations.VisibleForTesting;
@@ -23,13 +21,10 @@
import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
-import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.Iterables;
-import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
-import com.google.common.collect.SetMultimap;
import com.google.common.collect.Sets;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
@@ -45,7 +40,6 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NotesMigration;
-import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.util.LabelVote;
import com.google.gwtorm.server.OrmException;
@@ -115,15 +109,14 @@
*
* @param db review database.
* @param notes change notes.
- * @return multimap of reviewers keyed by state, where each account appears
- * exactly once in {@link SetMultimap#values()}, and
- * {@link ReviewerStateInternal#REMOVED} is not present.
+ * @return reviewers for the change.
* @throws OrmException if reviewers for the change could not be read.
*/
- public ImmutableSetMultimap<ReviewerStateInternal, Account.Id> getReviewers(
- ReviewDb db, ChangeNotes notes) throws OrmException {
+ public ReviewerSet getReviewers(ReviewDb db, ChangeNotes notes)
+ throws OrmException {
if (!migration.readChanges()) {
- return getReviewers(db.patchSetApprovals().byChange(notes.getChangeId()));
+ return ReviewerSet.fromApprovals(
+ db.patchSetApprovals().byChange(notes.getChangeId()));
}
return notes.load().getReviewers();
}
@@ -133,44 +126,18 @@
*
* @param allApprovals all approvals to consider; must all belong to the same
* change.
- * @return multimap of reviewers keyed by state, where each account appears
- * exactly once in {@link SetMultimap#values()}, and
- * {@link ReviewerStateInternal#REMOVED} is not present.
+ * @return reviewers for the change.
+ * @throws OrmException if reviewers for the change could not be read.
*/
- public ImmutableSetMultimap<ReviewerStateInternal, Account.Id> getReviewers(
- ChangeNotes notes, Iterable<PatchSetApproval> allApprovals)
+ public ReviewerSet getReviewers(ChangeNotes notes,
+ Iterable<PatchSetApproval> allApprovals)
throws OrmException {
if (!migration.readChanges()) {
- return getReviewers(allApprovals);
+ return ReviewerSet.fromApprovals(allApprovals);
}
return notes.load().getReviewers();
}
- private static ImmutableSetMultimap<ReviewerStateInternal, Account.Id> getReviewers(
- Iterable<PatchSetApproval> allApprovals) {
- PatchSetApproval first = null;
- SetMultimap<ReviewerStateInternal, Account.Id> reviewers =
- LinkedHashMultimap.create();
- for (PatchSetApproval psa : allApprovals) {
- if (first == null) {
- first = psa;
- } else {
- checkArgument(
- first.getKey().getParentKey().getParentKey().equals(
- psa.getKey().getParentKey().getParentKey()),
- "multiple change IDs: %s, %s", first.getKey(), psa.getKey());
- }
- Account.Id id = psa.getAccountId();
- if (psa.getValue() != 0) {
- reviewers.put(REVIEWER, id);
- reviewers.remove(CC, id);
- } else if (!reviewers.containsEntry(REVIEWER, id)) {
- reviewers.put(CC, id);
- }
- }
- return ImmutableSetMultimap.copyOf(reviewers);
- }
-
public List<PatchSetApproval> addReviewers(ReviewDb db,
ChangeUpdate update, LabelTypes labelTypes, Change change, PatchSet ps,
PatchSetInfo info, Iterable<Account.Id> wantReviewers,
@@ -185,7 +152,7 @@
Iterable<Account.Id> wantReviewers) throws OrmException {
PatchSet.Id psId = change.currentPatchSetId();
return addReviewers(db, update, labelTypes, change, psId, false, null, null,
- wantReviewers, getReviewers(db, notes).values());
+ wantReviewers, getReviewers(db, notes).all());
}
private List<PatchSetApproval> addReviewers(ReviewDb db, ChangeUpdate update,
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ReviewerSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/ReviewerSet.java
new file mode 100644
index 0000000..515cef7
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ReviewerSet.java
@@ -0,0 +1,115 @@
+// Copyright (C) 2016 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;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
+
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableTable;
+import com.google.common.collect.Table;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
+
+import java.sql.Timestamp;
+
+/**
+ * Set of reviewers on a change.
+ * <p>
+ * A given account may appear in multiple states and at different timestamps. No
+ * reviewers with state {@link ReviewerStateInternal#REMOVED} are ever exposed
+ * by this interface.
+ */
+public class ReviewerSet {
+ private static final ReviewerSet EMPTY = new ReviewerSet(
+ ImmutableTable.<ReviewerStateInternal, Account.Id, Timestamp>of());
+
+ public static ReviewerSet fromApprovals(
+ Iterable<PatchSetApproval> approvals) {
+ PatchSetApproval first = null;
+ Table<ReviewerStateInternal, Account.Id, Timestamp> reviewers =
+ HashBasedTable.create();
+ for (PatchSetApproval psa : approvals) {
+ if (first == null) {
+ first = psa;
+ } else {
+ checkArgument(
+ first.getKey().getParentKey().getParentKey().equals(
+ psa.getKey().getParentKey().getParentKey()),
+ "multiple change IDs: %s, %s", first.getKey(), psa.getKey());
+ }
+ Account.Id id = psa.getAccountId();
+ if (psa.getValue() != 0) {
+ reviewers.put(REVIEWER, id, psa.getGranted());
+ reviewers.remove(CC, id);
+ } else if (!reviewers.contains(REVIEWER, id)) {
+ reviewers.put(CC, id, psa.getGranted());
+ }
+ }
+ return new ReviewerSet(reviewers);
+ }
+
+ public static ReviewerSet fromTable(
+ Table<ReviewerStateInternal, Account.Id, Timestamp> table) {
+ return new ReviewerSet(table);
+ }
+
+ public static ReviewerSet empty() {
+ return EMPTY;
+ }
+
+ private final ImmutableTable<ReviewerStateInternal, Account.Id, Timestamp>
+ table;
+ private ImmutableSet<Account.Id> accounts;
+
+ private ReviewerSet(Table<ReviewerStateInternal, Account.Id, Timestamp> table) {
+ this.table = ImmutableTable.copyOf(table);
+ }
+
+ public ImmutableSet<Account.Id> all() {
+ if (accounts == null) {
+ // Idempotent and immutable, don't bother locking.
+ accounts = ImmutableSet.copyOf(table.columnKeySet());
+ }
+ return accounts;
+ }
+
+ public ImmutableSet<Account.Id> byState(ReviewerStateInternal state) {
+ return table.row(state).keySet();
+ }
+
+ public ImmutableTable<ReviewerStateInternal, Account.Id, Timestamp>
+ asTable() {
+ return table;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ return (o instanceof ReviewerSet) && table.equals(((ReviewerSet) o).table);
+ }
+
+ @Override
+ public int hashCode() {
+ return table.hashCode();
+ }
+
+ @Override
+ public String toString() {
+ return getClass().getSimpleName() + table;
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index 568f816..f93bb72 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -442,10 +442,10 @@
out.removableReviewers = removableReviewers(ctl, out.labels.values());
out.reviewers = new HashMap<>();
- for (Map.Entry<ReviewerStateInternal, Collection<Account.Id>> e
- : cd.reviewers().asMap().entrySet()) {
+ for (Map.Entry<ReviewerStateInternal, Map<Account.Id, Timestamp>> e
+ : cd.reviewers().asTable().rowMap().entrySet()) {
out.reviewers.put(e.getKey().asReviewerState(),
- toAccountInfo(e.getValue()));
+ toAccountInfo(e.getValue().keySet()));
}
}
@@ -617,7 +617,7 @@
// - They are an explicit reviewer.
// - They ever voted on this change.
Set<Account.Id> allUsers = new HashSet<>();
- allUsers.addAll(cd.reviewers().values());
+ allUsers.addAll(cd.reviewers().all());
for (PatchSetApproval psa : cd.approvals().values()) {
allUsers.add(psa.getAccountId());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java
index 1e3480f..9a0c691 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java
@@ -51,7 +51,7 @@
Map<Account.Id, ReviewerResource> reviewers = new LinkedHashMap<>();
ReviewDb db = dbProvider.get();
for (Account.Id accountId
- : approvalsUtil.getReviewers(db, rsrc.getNotes()).values()) {
+ : approvalsUtil.getReviewers(db, rsrc.getNotes()).all()) {
if (!reviewers.containsKey(accountId)) {
reviewers.put(accountId, resourceFactory.create(rsrc, accountId));
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
index 3defdd7..ffbfc36 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -19,10 +19,8 @@
import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
-import com.google.common.collect.SetMultimap;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
-import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -33,6 +31,7 @@
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.BanCommit;
import com.google.gerrit.server.git.BatchUpdate;
@@ -43,7 +42,6 @@
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.mail.ReplacePatchSetSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.RefControl;
@@ -106,7 +104,7 @@
private PatchSet patchSet;
private PatchSetInfo patchSetInfo;
private ChangeMessage changeMessage;
- private SetMultimap<ReviewerStateInternal, Account.Id> oldReviewers;
+ private ReviewerSet oldReviewers;
@AssistedInject
public PatchSetInserter(ChangeHooks hooks,
@@ -264,8 +262,8 @@
cm.setFrom(ctx.getUser().getAccountId());
cm.setPatchSet(patchSet, patchSetInfo);
cm.setChangeMessage(changeMessage);
- cm.addReviewers(oldReviewers.get(REVIEWER));
- cm.addExtraCC(oldReviewers.get(CC));
+ cm.addReviewers(oldReviewers.byState(REVIEWER));
+ cm.addExtraCC(oldReviewers.byState(CC));
cm.send();
} catch (Exception err) {
log.error("Cannot send email for new patch set on change "
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java
index ff5185e..4304669 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java
@@ -207,7 +207,7 @@
throws OrmException, IOException {
LabelTypes labelTypes = ctx.getControl().getLabelTypes();
Collection<Account.Id> oldReviewers = approvalsUtil.getReviewers(
- ctx.getDb(), ctx.getNotes()).values();
+ ctx.getDb(), ctx.getNotes()).all();
RevCommit commit = ctx.getRevWalk().parseCommit(
ObjectId.fromString(patchSet.getRevision().get()));
patchSetInfo = patchSetInfoFactory.get(ctx.getRevWalk(), commit, psId);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java
index 74a8866..d45d260 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java
@@ -83,6 +83,6 @@
private Collection<Account.Id> fetchAccountIds(ChangeResource rsrc)
throws OrmException {
return approvalsUtil.getReviewers(
- dbProvider.get(), rsrc.getNotes()).values();
+ dbProvider.get(), rsrc.getNotes()).all();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 73954b5..5bbfd3d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -142,7 +142,6 @@
import com.google.gerrit.server.ssh.SshAddressesModule;
import com.google.gerrit.server.tools.ToolsCatalog;
import com.google.gerrit.server.util.IdGenerator;
-import com.google.gerrit.server.util.SubmoduleSectionParser;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.gerrit.server.validators.GroupCreationValidationListener;
import com.google.gerrit.server.validators.HashtagValidationListener;
@@ -332,7 +331,6 @@
factory(MergeValidators.Factory.class);
factory(ProjectConfigValidator.Factory.class);
factory(NotesBranchUtil.Factory.class);
- factory(SubmoduleSectionParser.Factory.class);
factory(ReplaceOp.Factory.class);
factory(GitModules.Factory.class);
factory(VersionedAuthorizedKeys.Factory.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java
index 5807c26..56daccc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java
@@ -203,7 +203,7 @@
public void addAllReviewers(ReviewDb db, ChangeAttribute a, ChangeNotes notes)
throws OrmException {
Collection<Account.Id> reviewers =
- approvalsUtil.getReviewers(db, notes).values();
+ approvalsUtil.getReviewers(db, notes).all();
if (!reviewers.isEmpty()) {
a.allReviewers = Lists.newArrayListWithCapacity(reviewers.size());
for (Account.Id id : reviewers) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/GitModules.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/GitModules.java
index eb359e6..e6ce074 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/GitModules.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/GitModules.java
@@ -35,8 +35,6 @@
import org.slf4j.LoggerFactory;
import java.io.IOException;
-import java.net.URI;
-import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Set;
@@ -55,8 +53,7 @@
private static final String GIT_MODULES = ".gitmodules";
- private final String thisServer;
- private final SubmoduleSectionParser.Factory subSecParserFactory;
+ private final String canonicalWebUrl;
private final Branch.NameKey branch;
private final String submissionId;
private final MergeOpRepoManager orm;
@@ -66,20 +63,13 @@
@AssistedInject
GitModules(
@CanonicalWebUrl @Nullable String canonicalWebUrl,
- SubmoduleSectionParser.Factory subSecParserFactory,
@Assisted Branch.NameKey branch,
@Assisted String submissionId,
- @Assisted MergeOpRepoManager orm) throws SubmoduleException {
- this.subSecParserFactory = subSecParserFactory;
+ @Assisted MergeOpRepoManager orm) {
this.orm = orm;
this.branch = branch;
this.submissionId = submissionId;
- try {
- this.thisServer = new URI(canonicalWebUrl).getHost();
- } catch (URISyntaxException e) {
- throw new SubmoduleException("Incorrect Gerrit canonical web url " +
- "provided in gerrit.config file.", e);
- }
+ this.canonicalWebUrl = canonicalWebUrl;
}
void load() throws IOException {
@@ -106,7 +96,7 @@
try {
BlobBasedConfig bbc =
new BlobBasedConfig(null, or.repo, commit, GIT_MODULES);
- subscriptions = subSecParserFactory.create(bbc, thisServer,
+ subscriptions = new SubmoduleSectionParser(bbc, canonicalWebUrl,
branch).parseAllSections();
} catch (ConfigInvalidException e) {
throw new IOException(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
index 4e804df..0a9c839 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
@@ -145,12 +145,19 @@
ProjectConfig cfg = projectCache.get(project).getConfig();
for (SubscribeSection s : projectStateFactory.create(cfg)
.getSubscribeSections(branch)) {
+ logDebug("Checking subscribe section " + s);
Collection<Branch.NameKey> branches =
getDestinationBranches(branch, s, orm);
for (Branch.NameKey targetBranch : branches) {
GitModules m = gitmodulesFactory.create(targetBranch, updateId, orm);
m.load();
- ret.addAll(m.subscribedTo(branch));
+ for (SubmoduleSubscription ss : m.subscribedTo(branch)) {
+ logDebug("Checking SubmoduleSubscription " + ss);
+ if (projectCache.get(ss.getSubmodule().getParentKey()) != null) {
+ logDebug("Adding SubmoduleSubscription " + ss);
+ ret.add(ss);
+ }
+ }
}
}
logDebug("Calculated superprojects for " + branch + " are " + ret);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
index 367159d..ad543ea 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
@@ -111,7 +111,7 @@
appendText(velocifyFile("ChangeFooter.vm"));
try {
TreeSet<String> names = new TreeSet<>();
- for (Account.Id who : changeData.reviewers().values()) {
+ for (Account.Id who : changeData.reviewers().all()) {
names.add(getNameEmailFor(who));
}
for (String name : names) {
@@ -337,7 +337,7 @@
}
try {
- for (Account.Id id : changeData.reviewers().values()) {
+ for (Account.Id id : changeData.reviewers().all()) {
add(RecipientType.CC, id);
}
} catch (OrmException err) {
@@ -353,7 +353,7 @@
}
try {
- for (Account.Id id : changeData.reviewers().get(REVIEWER)) {
+ for (Account.Id id : changeData.reviewers().byState(REVIEWER)) {
add(RecipientType.CC, id);
}
} catch (OrmException err) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java
index af8a3f69..a1274c3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java
@@ -17,12 +17,11 @@
import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
-import com.google.common.collect.Multimap;
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.errors.NoSuchAccountException;
import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.account.AccountResolver;
-import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gwtorm.server.OrmException;
import org.eclipse.jgit.revwalk.FooterKey;
@@ -58,10 +57,10 @@
}
public static MailRecipients getRecipientsFromReviewers(
- Multimap<ReviewerStateInternal, Account.Id> reviewers) {
+ ReviewerSet reviewers) {
MailRecipients recipients = new MailRecipients();
- recipients.reviewers.addAll(reviewers.get(REVIEWER));
- recipients.cc.addAll(reviewers.get(CC));
+ recipients.reviewers.addAll(reviewers.byState(REVIEWER));
+ recipients.cc.addAll(reviewers.byState(CC));
return recipients;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java
index 0dfd8c9..c59a0ac 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java
@@ -27,6 +27,7 @@
import com.google.common.base.CharMatcher;
import com.google.common.base.Function;
import com.google.common.base.Predicate;
+import com.google.common.base.Strings;
import com.google.common.collect.Collections2;
import com.google.common.collect.ComparisonChain;
import com.google.common.collect.ImmutableCollection;
@@ -40,6 +41,7 @@
import com.google.common.collect.Multimap;
import com.google.common.collect.Ordering;
import com.google.common.collect.Sets;
+import com.google.common.collect.Table;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
@@ -50,6 +52,7 @@
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.PatchLineCommentsUtil;
+import com.google.gerrit.server.ReviewerSet;
import com.google.gwtorm.client.Column;
import com.google.gwtorm.server.OrmException;
@@ -59,6 +62,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Comparator;
+import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
@@ -84,12 +88,15 @@
throws OrmException {
db.changes().beginTransaction(id);
try {
+ List<PatchSetApproval> approvals =
+ db.patchSetApprovals().byChange(id).toList();
return new ChangeBundle(
db.changes().get(id),
db.changeMessages().byChange(id),
db.patchSets().byChange(id),
- db.patchSetApprovals().byChange(id),
+ approvals,
db.patchComments().byChange(id),
+ ReviewerSet.fromApprovals(approvals),
Source.REVIEW_DB);
} finally {
db.rollback();
@@ -106,6 +113,7 @@
Iterables.concat(
plcUtil.draftByChange(null, notes),
plcUtil.publishedByChange(null, notes)),
+ notes.getReviewers(),
Source.NOTE_DB);
}
@@ -156,7 +164,6 @@
return CHANGE_MESSAGE_ORDER.immutableSortedCopy(in);
}
-
private static TreeMap<PatchSet.Id, PatchSet> patchSetMap(Iterable<PatchSet> in) {
TreeMap<PatchSet.Id, PatchSet> out = new TreeMap<>(
new Comparator<PatchSet.Id>() {
@@ -256,6 +263,7 @@
patchSetApprovals;
private final ImmutableMap<PatchLineComment.Key, PatchLineComment>
patchLineComments;
+ private final ReviewerSet reviewers;
private final Source source;
public ChangeBundle(
@@ -264,6 +272,7 @@
Iterable<PatchSet> patchSets,
Iterable<PatchSetApproval> patchSetApprovals,
Iterable<PatchLineComment> patchLineComments,
+ ReviewerSet reviewers,
Source source) {
this.change = checkNotNull(change);
this.changeMessages = changeMessageList(changeMessages);
@@ -272,6 +281,7 @@
ImmutableMap.copyOf(patchSetApprovalMap(patchSetApprovals));
this.patchLineComments =
ImmutableMap.copyOf(patchLineCommentMap(patchLineComments));
+ this.reviewers = checkNotNull(reviewers);
this.source = checkNotNull(source);
for (ChangeMessage m : this.changeMessages) {
@@ -309,6 +319,10 @@
return patchLineComments.values();
}
+ public ReviewerSet getReviewers() {
+ return reviewers;
+ }
+
public Source getSource() {
return source;
}
@@ -319,6 +333,7 @@
diffChangeMessages(diffs, this, o);
diffPatchSets(diffs, this, o);
diffPatchSetApprovals(diffs, this, o);
+ diffReviewers(diffs, this, o);
diffPatchLineComments(diffs, this, o);
return ImmutableList.copyOf(diffs);
}
@@ -425,8 +440,10 @@
CharMatcher s = CharMatcher.is(' ');
boolean excludeSubject = false;
boolean excludeOrigSubj = false;
- String aSubj = a.getSubject();
- String bSubj = b.getSubject();
+ // Subject is not technically a nullable field, but we observed some null
+ // subjects in the wild on googlesource.com, so treat null as empty.
+ String aSubj = Strings.nullToEmpty(a.getSubject());
+ String bSubj = Strings.nullToEmpty(b.getSubject());
// Allow created timestamp in NoteDb to be either the created timestamp of
// the change, or the timestamp of the first remaining patch set.
@@ -661,6 +678,39 @@
}
}
+ @AutoValue
+ static abstract class ReviewerKey {
+ private static Map<ReviewerKey, Timestamp> toMap(ReviewerSet reviewers) {
+ Map<ReviewerKey, Timestamp> result = new HashMap<>();
+ for (Table.Cell<ReviewerStateInternal, Account.Id, Timestamp> c :
+ reviewers.asTable().cellSet()) {
+ result.put(new AutoValue_ChangeBundle_ReviewerKey(
+ c.getRowKey(), c.getColumnKey()), c.getValue());
+ }
+ return result;
+ }
+
+ abstract ReviewerStateInternal state();
+ abstract Account.Id account();
+
+ @Override
+ public String toString() {
+ return state() + "," + account();
+ }
+ }
+
+ private static void diffReviewers(List<String> diffs,
+ ChangeBundle bundleA, ChangeBundle bundleB) {
+ Map<ReviewerKey, Timestamp> as = ReviewerKey.toMap(bundleA.reviewers);
+ Map<ReviewerKey, Timestamp> bs = ReviewerKey.toMap(bundleB.reviewers);
+ for (ReviewerKey k : diffKeySets(diffs, as, bs)) {
+ Timestamp a = as.get(k);
+ Timestamp b = bs.get(k);
+ String desc = describe(k);
+ diffTimestamps(diffs, desc, bundleA, a, bundleB, b, "timestamp");
+ }
+ }
+
private static void diffPatchLineComments(List<String> diffs,
ChangeBundle bundleA, ChangeBundle bundleB) {
Map<PatchLineComment.Key, PatchLineComment> as =
@@ -825,9 +875,14 @@
private static String keyClass(Object obj) {
Class<?> clazz = obj.getClass();
String name = clazz.getSimpleName();
- checkArgument(name.equals("Key") || name.equals("Id"),
+ checkArgument(name.endsWith("Key") || name.endsWith("Id"),
"not an Id/Key class: %s", name);
- return clazz.getEnclosingClass().getSimpleName() + "." + name;
+ if (name.equals("Key") || name.equals("Id")) {
+ return clazz.getEnclosingClass().getSimpleName() + "." + name;
+ } else if (name.startsWith("AutoValue_")) {
+ return name.substring(name.lastIndexOf('_') + 1);
+ }
+ return name;
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
index a6cd8fa..926194b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -28,7 +28,6 @@
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Iterables;
@@ -36,6 +35,7 @@
import com.google.common.collect.Multimap;
import com.google.common.collect.Multimaps;
import com.google.common.collect.Ordering;
+import com.google.common.collect.Tables;
import com.google.common.util.concurrent.AsyncFunction;
import com.google.common.util.concurrent.CheckedFuture;
import com.google.common.util.concurrent.Futures;
@@ -55,6 +55,7 @@
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.ReviewDbUtil;
+import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.git.RefCache;
import com.google.gerrit.server.git.RepoRefCache;
import com.google.gerrit.server.project.NoSuchChangeException;
@@ -381,7 +382,7 @@
private ImmutableSortedMap<PatchSet.Id, PatchSet> patchSets;
private ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals;
- private ImmutableSetMultimap<ReviewerStateInternal, Account.Id> reviewers;
+ private ReviewerSet reviewers;
private ImmutableList<Account.Id> allPastReviewers;
private ImmutableList<SubmitRecord> submitRecords;
private ImmutableList<ChangeMessage> allChangeMessages;
@@ -420,7 +421,7 @@
return approvals;
}
- public ImmutableSetMultimap<ReviewerStateInternal, Account.Id> getReviewers() {
+ public ReviewerSet getReviewers() {
return reviewers;
}
@@ -583,13 +584,7 @@
} else {
hashtags = ImmutableSet.of();
}
- ImmutableSetMultimap.Builder<ReviewerStateInternal, Account.Id> reviewers =
- ImmutableSetMultimap.builder();
- for (Map.Entry<Account.Id, ReviewerStateInternal> e
- : parser.reviewers.entrySet()) {
- reviewers.put(e.getValue(), e.getKey());
- }
- this.reviewers = reviewers.build();
+ this.reviewers = ReviewerSet.fromTable(Tables.transpose(parser.reviewers));
this.allPastReviewers = ImmutableList.copyOf(parser.allPastReviewers);
submitRecords = ImmutableList.copyOf(parser.submitRecords);
@@ -598,7 +593,7 @@
@Override
protected void loadDefaults() {
approvals = ImmutableListMultimap.of();
- reviewers = ImmutableSetMultimap.of();
+ reviewers = ReviewerSet.empty();
submitRecords = ImmutableList.of();
allChangeMessages = ImmutableList.of();
changeMessagesByPatchSet = ImmutableListMultimap.of();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 9d9e180..02dd441 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -36,6 +36,7 @@
import com.google.common.base.Splitter;
import com.google.common.base.Supplier;
import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableSet;
@@ -94,7 +95,7 @@
private static final RevId PARTIAL_PATCH_SET =
new RevId("INVALID PARTIAL PATCH SET");
- final Map<Account.Id, ReviewerStateInternal> reviewers;
+ final Table<Account.Id, ReviewerStateInternal, Timestamp> reviewers;
final List<Account.Id> allPastReviewers;
final List<SubmitRecord> submitRecords;
final Multimap<RevId, PatchLineComment> comments;
@@ -134,7 +135,7 @@
this.noteUtil = noteUtil;
this.metrics = metrics;
approvals = new HashMap<>();
- reviewers = new LinkedHashMap<>();
+ reviewers = HashBasedTable.create();
allPastReviewers = new ArrayList<>();
submitRecords = Lists.newArrayListWithExpectedSize(1);
allChangeMessages = new ArrayList<>();
@@ -156,7 +157,7 @@
parse(commit);
}
parseNotes();
- allPastReviewers.addAll(reviewers.keySet());
+ allPastReviewers.addAll(reviewers.rowKeySet());
pruneReviewers();
updatePatchSetStates();
checkMandatoryFooters();
@@ -262,7 +263,7 @@
for (ReviewerStateInternal state : ReviewerStateInternal.values()) {
for (String line : commit.getFooterLineValues(state.getFooterKey())) {
- parseReviewer(state, line);
+ parseReviewer(ts, state, line);
}
// Don't update timestamp when a reviewer was added, matching RevewDb
// behavior.
@@ -698,27 +699,27 @@
return noteUtil.parseIdent(commit.getAuthorIdent(), id);
}
- private void parseReviewer(ReviewerStateInternal state, String line)
- throws ConfigInvalidException {
+ private void parseReviewer(Timestamp ts, ReviewerStateInternal state,
+ String line) throws ConfigInvalidException {
PersonIdent ident = RawParseUtils.parsePersonIdent(line);
if (ident == null) {
throw invalidFooter(state.getFooterKey(), line);
}
Account.Id accountId = noteUtil.parseIdent(ident, id);
- if (!reviewers.containsKey(accountId)) {
- reviewers.put(accountId, state);
+ if (!reviewers.containsRow(accountId)) {
+ reviewers.put(accountId, state, ts);
}
}
private void pruneReviewers() {
- Iterator<Map.Entry<Account.Id, ReviewerStateInternal>> rit =
- reviewers.entrySet().iterator();
+ Iterator<Table.Cell<Account.Id, ReviewerStateInternal, Timestamp>> rit =
+ reviewers.cellSet().iterator();
while (rit.hasNext()) {
- Map.Entry<Account.Id, ReviewerStateInternal> e = rit.next();
- if (e.getValue() == ReviewerStateInternal.REMOVED) {
+ Table.Cell<Account.Id, ReviewerStateInternal, Timestamp> e = rit.next();
+ if (e.getColumnKey() == ReviewerStateInternal.REMOVED) {
rit.remove();
for (Table<Account.Id, ?, ?> curr : approvals.values()) {
- curr.rowKeySet().remove(e.getKey());
+ curr.rowKeySet().remove(e.getRowKey());
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java
index 9aa69bf..5f16eb4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java
@@ -36,6 +36,7 @@
import com.google.common.collect.Multimap;
import com.google.common.collect.Ordering;
import com.google.common.collect.Sets;
+import com.google.common.collect.Table;
import com.google.common.primitives.Ints;
import com.google.gerrit.common.FormatUtil;
import com.google.gerrit.common.Nullable;
@@ -277,6 +278,11 @@
}
}
+ for (Table.Cell<ReviewerStateInternal, Account.Id, Timestamp> r :
+ bundle.getReviewers().asTable().cellSet()) {
+ events.add(new ReviewerEvent(r, change.getCreatedOn()));
+ }
+
Change noteDbChange = new Change(null, null, null, null, null);
for (ChangeMessage msg : bundle.getChangeMessages()) {
if (msg.getPatchSetId() == null || psIds.contains(msg.getPatchSetId())) {
@@ -714,6 +720,33 @@
}
}
+ private static class ReviewerEvent extends Event {
+ private Table.Cell<ReviewerStateInternal, Account.Id, Timestamp> reviewer;
+
+ ReviewerEvent(
+ Table.Cell<ReviewerStateInternal, Account.Id, Timestamp> reviewer,
+ Timestamp changeCreatedOn) {
+ super(
+ // Reviewers aren't generally associated with a particular patch set
+ // (although as an implementation detail they were in ReviewDb). Just
+ // use the latest patch set at the time of the event.
+ null,
+ reviewer.getColumnKey(), reviewer.getValue(), changeCreatedOn, null);
+ this.reviewer = reviewer;
+ }
+
+ @Override
+ boolean uniquePerUpdate() {
+ return false;
+ }
+
+ @Override
+ void apply(ChangeUpdate update) throws IOException, OrmException {
+ checkUpdate(update);
+ update.putReviewer(reviewer.getColumnKey(), reviewer.getRowKey());
+ }
+ }
+
private static class PatchSetEvent extends Event {
private final Change change;
private final PatchSet ps;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
index 4bda245..62c6d5e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
@@ -340,7 +340,7 @@
public boolean isReviewer(ReviewDb db, @Nullable ChangeData cd)
throws OrmException {
if (getUser().isIdentifiedUser()) {
- Collection<Account.Id> results = changeData(db, cd).reviewers().values();
+ Collection<Account.Id> results = changeData(db, cd).reviewers().all();
return results.contains(getUser().getAccountId());
}
return false;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index ed7b134..93a16ad 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -30,7 +30,6 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
-import com.google.common.collect.SetMultimap;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitTypeRecord;
@@ -50,13 +49,13 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.change.MergeabilityCache;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NotesMigration;
-import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListEntry;
@@ -905,8 +904,7 @@
return Optional.absent();
}
- public SetMultimap<ReviewerStateInternal, Account.Id> reviewers()
- throws OrmException {
+ public ReviewerSet reviewers() throws OrmException {
return approvalsUtil.getReviewers(notes(), approvals().values());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java
index eb07250..2da8c54 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java
@@ -40,7 +40,7 @@
object.change().getStatus() == Change.Status.DRAFT) {
return false;
}
- for (Account.Id accountId : object.reviewers().values()) {
+ for (Account.Id accountId : object.reviewers().all()) {
if (id.equals(accountId)) {
return true;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerinPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerinPredicate.java
index eb93451..76a02432 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerinPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerinPredicate.java
@@ -37,7 +37,7 @@
@Override
public boolean match(final ChangeData object) throws OrmException {
- for (Account.Id accountId : object.reviewers().values()) {
+ for (Account.Id accountId : object.reviewers().all()) {
IdentifiedUser reviewer = userFactory.create(accountId);
if (reviewer.getEffectiveGroups().contains(uuid)) {
return true;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/util/SubmoduleSectionParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/util/SubmoduleSectionParser.java
index 907ef70..6b5c991 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/util/SubmoduleSectionParser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/util/SubmoduleSectionParser.java
@@ -17,11 +17,8 @@
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.SubmoduleSubscription;
-import com.google.gerrit.server.project.ProjectCache;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
-import org.eclipse.jgit.lib.BlobBasedConfig;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import java.net.URI;
@@ -48,24 +45,15 @@
*/
public class SubmoduleSectionParser {
- public interface Factory {
- SubmoduleSectionParser create(BlobBasedConfig bbc, String thisServer,
- Branch.NameKey superProjectBranch);
- }
-
- private final ProjectCache projectCache;
- private final BlobBasedConfig bbc;
- private final String thisServer;
+ private final Config bbc;
+ private final String canonicalWebUrl;
private final Branch.NameKey superProjectBranch;
- @Inject
- public SubmoduleSectionParser(ProjectCache projectCache,
- @Assisted BlobBasedConfig bbc,
- @Assisted String thisServer,
- @Assisted Branch.NameKey superProjectBranch) {
- this.projectCache = projectCache;
+ public SubmoduleSectionParser(Config bbc,
+ String canonicalWebUrl,
+ Branch.NameKey superProjectBranch) {
this.bbc = bbc;
- this.thisServer = thisServer;
+ this.canonicalWebUrl = canonicalWebUrl;
this.superProjectBranch = superProjectBranch;
}
@@ -84,51 +72,74 @@
final String url = bbc.getString("submodule", id, "url");
final String path = bbc.getString("submodule", id, "path");
String branch = bbc.getString("submodule", id, "branch");
- SubmoduleSubscription ss = null;
try {
if (url != null && url.length() > 0 && path != null && path.length() > 0
&& branch != null && branch.length() > 0) {
// All required fields filled.
+ String project;
- boolean urlIsRelative = url.startsWith("../");
- String server = null;
- if (!urlIsRelative) {
+ if (branch.equals(".")) {
+ branch = superProjectBranch.get();
+ }
+
+ // relative URL
+ if (url.startsWith("../")) {
+ // prefix with a slash for easier relative path walks
+ project = '/' + superProjectBranch.getParentKey().get();
+ String hostPart = url;
+ while (hostPart.startsWith("../")) {
+ int lastSlash = project.lastIndexOf('/');
+ if (lastSlash < 0) {
+ // too many levels up, ignore for now
+ return null;
+ }
+ project = project.substring(0, lastSlash);
+ hostPart = hostPart.substring(3);
+ }
+ project = project + "/" + hostPart;
+
+ // remove leading '/'
+ project = project.substring(1);
+ } else {
// It is actually an URI. It could be ssh://localhost/project-a.
- server = new URI(url).getHost();
- }
- if ((urlIsRelative)
- || (server != null && server.equalsIgnoreCase(thisServer))) {
- // Subscription really related to this running server.
- if (branch.equals(".")) {
- branch = superProjectBranch.get();
+ URI targetServerURI = new URI(url);
+ URI thisServerURI = new URI(canonicalWebUrl);
+ String thisHost = thisServerURI.getHost();
+ String targetHost = targetServerURI.getHost();
+ if (thisHost == null || targetHost == null ||
+ !targetHost.equalsIgnoreCase(thisHost)) {
+ return null;
}
-
- final String urlExtractedPath = new URI(url).getPath();
- String projectName;
- int fromIndex = urlExtractedPath.length() - 1;
- while (fromIndex > 0) {
- fromIndex = urlExtractedPath.lastIndexOf('/', fromIndex - 1);
- projectName = urlExtractedPath.substring(fromIndex + 1);
-
- if (projectName.endsWith(Constants.DOT_GIT_EXT)) {
- projectName = projectName.substring(0, //
- projectName.length() - Constants.DOT_GIT_EXT.length());
- }
- Project.NameKey projectKey = new Project.NameKey(projectName);
- if (projectCache.get(projectKey) != null) {
- ss = new SubmoduleSubscription(
- superProjectBranch,
- new Branch.NameKey(new Project.NameKey(projectName), branch),
- path);
- }
+ String p1 = targetServerURI.getPath();
+ String p2 = thisServerURI.getPath();
+ if (!p1.startsWith(p2)) {
+ // When we are running the server at
+ // http://server/my-gerrit/ but the subscription is for
+ // http://server/other-teams-gerrit/
+ return null;
}
+ // skip common part
+ project = p1.substring(p2.length());
}
+
+ while (project.startsWith("/")) {
+ project = project.substring(1);
+ }
+
+ if (project.endsWith(Constants.DOT_GIT_EXT)) {
+ project = project.substring(0, //
+ project.length() - Constants.DOT_GIT_EXT.length());
+ }
+ Project.NameKey projectKey = new Project.NameKey(project);
+ return new SubmoduleSubscription(
+ superProjectBranch,
+ new Branch.NameKey(projectKey, branch),
+ path);
}
} catch (URISyntaxException e) {
// Error in url syntax (in fact it is uri syntax)
}
-
- return ss;
+ return null;
}
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java
index ea1120f..4306d74 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java
@@ -14,14 +14,18 @@
package com.google.gerrit.server.notedb;
+import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.common.TimeUtil.roundToSecond;
import static com.google.gerrit.server.notedb.ChangeBundle.Source.NOTE_DB;
import static com.google.gerrit.server.notedb.ChangeBundle.Source.REVIEW_DB;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;
+import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Table;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
@@ -33,6 +37,7 @@
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.testutil.TestChanges;
import com.google.gerrit.testutil.TestTimeUtil;
import com.google.gwtorm.client.KeyUtil;
@@ -113,9 +118,9 @@
Change c2 = TestChanges.newChange(project, accountId);
int id2 = c2.getId().get();
ChangeBundle b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"changeId differs for Changes: {" + id1 + "} != {" + id2 + "}",
@@ -131,9 +136,9 @@
new Project.NameKey("project"), new Account.Id(100));
Change c2 = clone(c1);
ChangeBundle b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
@@ -153,9 +158,9 @@
// Both are ReviewDb, exact timestamp match is required.
ChangeBundle b1 = new ChangeBundle(c1, messages(), patchSets(),
- approvals(), comments(), REVIEW_DB);
+ approvals(), comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c2, messages(), patchSets(),
- approvals(), comments(), REVIEW_DB);
+ approvals(), comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"createdOn differs for Change.Id " + c1.getId() + ":"
+ " {2009-09-30 17:00:01.0} != {2009-09-30 17:00:02.0}",
@@ -164,9 +169,9 @@
// One NoteDb, slop is allowed.
b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
- comments(), NOTE_DB);
+ comments(), reviewers(), NOTE_DB);
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
assertNoDiffs(b2, b1);
@@ -175,9 +180,9 @@
Change c3 = clone(c1);
c3.setLastUpdatedOn(TimeUtil.nowTs());
b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
- comments(), NOTE_DB);
+ comments(), reviewers(), NOTE_DB);
ChangeBundle b3 = new ChangeBundle(c3, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
String msg = "effective last updated time differs for Change.Id "
+ c1.getId() + " in NoteDb vs. ReviewDb:"
+ " {2009-09-30 17:00:01.0} != {2009-09-30 17:00:10.0}";
@@ -197,27 +202,27 @@
// Both ReviewDb, exact match required.
ChangeBundle b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"originalSubject differs for Change.Id " + c1.getId() + ":"
+ " {Original A} != {Original B}");
// Both NoteDb, exact match required.
b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
assertDiffs(b1, b2,
"originalSubject differs for Change.Id " + c1.getId() + ":"
+ " {Original A} != {Original B}");
// One ReviewDb, one NoteDb, original subject is ignored.
b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(), comments(),
- REVIEW_DB);
+ reviewers(), REVIEW_DB);
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
assertNoDiffs(b1, b2);
assertNoDiffs(b2, b1);
}
@@ -233,25 +238,25 @@
// Both ReviewDb, exact match required.
ChangeBundle b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"topic differs for Change.Id " + c1.getId() + ":"
+ " {} != {null}");
// Topic ignored if ReviewDb is empty and NoteDb is null.
b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(), comments(),
- REVIEW_DB);
+ reviewers(), REVIEW_DB);
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
assertNoDiffs(b1, b2);
// Exact match still required if NoteDb has empty value (not realistic).
b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(), comments(),
- REVIEW_DB);
+ reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"topic differs for Change.Id " + c1.getId() + ":"
+ " {} != {null}");
@@ -260,9 +265,9 @@
Change c3 = clone(c1);
c3.setTopic("topic");
b1 = new ChangeBundle(c3, messages(), patchSets(), approvals(), comments(),
- REVIEW_DB);
+ reviewers(), REVIEW_DB);
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
assertDiffs(b1, b2,
"topic differs for Change.Id " + c1.getId() + ":"
+ " {topic} != {null}");
@@ -284,16 +289,16 @@
// Both ReviewDb, exact match required.
ChangeBundle b1 = new ChangeBundle(c1, messages(), patchSets(),
- approvals(a), comments(), REVIEW_DB);
+ approvals(a), comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c2, messages(), patchSets(),
- approvals(a), comments(), REVIEW_DB);
+ approvals(a), comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"effective last updated time differs for Change.Id " + c1.getId() + ":"
+ " {2009-09-30 17:00:00.0} != {2009-09-30 17:00:06.0}");
// NoteDb allows latest timestamp from all entities in bundle.
b2 = new ChangeBundle(c2, messages(), patchSets(),
- approvals(a), comments(), NOTE_DB);
+ approvals(a), comments(), reviewers(), NOTE_DB);
assertNoDiffs(b1, b2);
}
@@ -314,18 +319,18 @@
// ReviewDb has later lastUpdatedOn timestamp than NoteDb, allowed since
// NoteDb matches the latest timestamp of a non-Change entity.
ChangeBundle b1 = new ChangeBundle(c2, messages(), patchSets(),
- approvals(a), comments(), REVIEW_DB);
+ approvals(a), comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c1, messages(), patchSets(),
- approvals(a), comments(), NOTE_DB);
+ approvals(a), comments(), reviewers(), NOTE_DB);
assertThat(b1.getChange().getLastUpdatedOn())
.isGreaterThan(b2.getChange().getLastUpdatedOn());
assertNoDiffs(b1, b2);
// Timestamps must actually match if Change is the only entity.
b1 = new ChangeBundle(c2, messages(), patchSets(), approvals(), comments(),
- REVIEW_DB);
+ reviewers(), REVIEW_DB);
b2 = new ChangeBundle(c1, messages(), patchSets(), approvals(), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
assertDiffs(b1, b2,
"effective last updated time differs for Change.Id " + c1.getId()
+ " in NoteDb vs. ReviewDb:"
@@ -344,25 +349,25 @@
// Both ReviewDb, exact match required.
ChangeBundle b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"subject differs for Change.Id " + c1.getId() + ":"
+ " {Change subject} != {Change sub}");
// ReviewDb has shorter subject, allowed.
b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
- comments(), NOTE_DB);
+ comments(), reviewers(), NOTE_DB);
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
// NoteDb has shorter subject, not allowed.
b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
- comments(), NOTE_DB);
+ comments(), reviewers(), NOTE_DB);
assertDiffs(b1, b2,
"subject differs for Change.Id " + c1.getId() + ":"
+ " {Change subject} != {Change sub}");
@@ -379,18 +384,18 @@
// Both ReviewDb, exact match required.
ChangeBundle b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"subject differs for Change.Id " + c1.getId() + ":"
+ " {Change subject} != { Change subject}");
// ReviewDb is missing leading spaces, allowed.
b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
- comments(), NOTE_DB);
+ comments(), reviewers(), NOTE_DB);
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
assertNoDiffs(b2, b1);
}
@@ -406,18 +411,18 @@
// Both ReviewDb.
ChangeBundle b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"subject differs for Change.Id " + c1.getId() + ":"
+ " {Change subject} != {\tChange subject}");
// One NoteDb.
b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
- comments(), NOTE_DB);
+ comments(), reviewers(), NOTE_DB);
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"subject differs for Change.Id " + c1.getId() + ":"
+ " {Change subject} != {\tChange subject}");
@@ -437,9 +442,9 @@
new ChangeMessage.Key(c.getId(), "uuid2"),
accountId, TimeUtil.nowTs(), c.currentPatchSetId());
ChangeBundle b1 = new ChangeBundle(c, messages(cm1), latest(c), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(cm2), latest(c), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"ChangeMessage.Key sets differ:"
@@ -455,9 +460,9 @@
cm1.setMessage("message 1");
ChangeMessage cm2 = clone(cm1);
ChangeBundle b1 = new ChangeBundle(c, messages(cm1), latest(c), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(cm2), latest(c), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
@@ -479,9 +484,9 @@
cm2.getKey().set("uuid2");
ChangeBundle b1 = new ChangeBundle(c, messages(cm1), latest(c), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(cm2), latest(c), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
// Both are ReviewDb, exact UUID match is required.
assertDiffs(b1, b2,
"ChangeMessage.Key sets differ:"
@@ -489,9 +494,9 @@
// One NoteDb, UUIDs are ignored.
b1 = new ChangeBundle(c, messages(cm1), latest(c), approvals(), comments(),
- REVIEW_DB);
+ reviewers(), REVIEW_DB);
b2 = new ChangeBundle(c, messages(cm2), latest(c), approvals(), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
assertNoDiffs(b1, b2);
}
@@ -510,18 +515,18 @@
// Both ReviewDb: Uses same keySet diff as other types.
ChangeBundle b1 = new ChangeBundle(c, messages(cm1, cm2), latest(c),
- approvals(), comments(), REVIEW_DB);
+ approvals(), comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(cm1), latest(c), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"ChangeMessage.Key sets differ: [" + id
+ ",uuid2] only in A; [] only in B");
// One NoteDb: UUIDs in keys can't be used for comparison, just diff counts.
b1 = new ChangeBundle(c, messages(cm1, cm2), latest(c), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
b2 = new ChangeBundle(c, messages(cm1), latest(c), approvals(), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
assertDiffs(b1, b2,
"ChangeMessages differ for Change.Id " + id + "\n"
+ "Only in A:\n " + cm2);
@@ -544,9 +549,9 @@
cm3.getKey().set("uuid2"); // Differs only in UUID.
ChangeBundle b1 = new ChangeBundle(c, messages(cm1, cm3), latest(c),
- approvals(), comments(), REVIEW_DB);
+ approvals(), comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(cm2, cm3), latest(c),
- approvals(), comments(), NOTE_DB);
+ approvals(), comments(), reviewers(), NOTE_DB);
// Implementation happens to pair up cm1 in b1 with cm3 in b2 because it
// depends on iteration order and doesn't care about UUIDs. The important
// thing is that there's some diff.
@@ -572,18 +577,18 @@
// Both are ReviewDb, exact timestamp match is required.
ChangeBundle b1 = new ChangeBundle(c, messages(cm1), latest(c), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(cm2), latest(c), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"writtenOn differs for ChangeMessage.Key " + c.getId() + ",uuid1:"
+ " {2009-09-30 17:00:02.0} != {2009-09-30 17:00:03.0}");
// One NoteDb, slop is allowed.
b1 = new ChangeBundle(c, messages(cm1), latest(c), approvals(), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
b2 = new ChangeBundle(c, messages(cm2), latest(c), approvals(), comments(),
- REVIEW_DB);
+ reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
assertNoDiffs(b2, b1);
@@ -592,9 +597,9 @@
ChangeMessage cm3 = clone(cm1);
cm3.setWrittenOn(TimeUtil.nowTs());
b1 = new ChangeBundle(c, messages(cm1), latest(c), approvals(), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
ChangeBundle b3 = new ChangeBundle(c, messages(cm3), latest(c), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
int id = c.getId().get();
assertDiffs(b1, b3,
"ChangeMessages differ for Change.Id " + id + "\n"
@@ -619,9 +624,9 @@
cm2.setPatchSetId(null);
ChangeBundle b1 = new ChangeBundle(c, messages(cm1), latest(c), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(cm2), latest(c), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
// Both are ReviewDb, exact patch set ID match is required.
assertDiffs(b1, b2,
@@ -630,16 +635,16 @@
// Null patch set ID on ReviewDb is ignored.
b1 = new ChangeBundle(c, messages(cm1), latest(c), approvals(), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
b2 = new ChangeBundle(c, messages(cm2), latest(c), approvals(), comments(),
- REVIEW_DB);
+ reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
// Null patch set ID on NoteDb is not ignored (but is not realistic).
b1 = new ChangeBundle(c, messages(cm1), latest(c), approvals(), comments(),
- REVIEW_DB);
+ reviewers(), REVIEW_DB);
b2 = new ChangeBundle(c, messages(cm2), latest(c), approvals(), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
assertDiffs(b1, b2,
"ChangeMessages differ for Change.Id " + id + "\n"
+ "Only in A:\n " + cm1 + "\n"
@@ -665,9 +670,9 @@
ps2.setCreatedOn(TimeUtil.nowTs());
ChangeBundle b1 = new ChangeBundle(c, messages(), patchSets(ps2),
- approvals(), comments(), REVIEW_DB);
+ approvals(), comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(), patchSets(ps1, ps2),
- approvals(), comments(), REVIEW_DB);
+ approvals(), comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"PatchSet.Id sets differ:"
@@ -683,9 +688,9 @@
ps1.setCreatedOn(TimeUtil.nowTs());
PatchSet ps2 = clone(ps1);
ChangeBundle b1 = new ChangeBundle(c, messages(), patchSets(ps1),
- approvals(), comments(), REVIEW_DB);
+ approvals(), comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(), patchSets(ps2),
- approvals(), comments(), REVIEW_DB);
+ approvals(), comments(), reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
@@ -709,18 +714,18 @@
// Both are ReviewDb, exact timestamp match is required.
ChangeBundle b1 = new ChangeBundle(c, messages(), patchSets(ps1),
- approvals(), comments(), REVIEW_DB);
+ approvals(), comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(), patchSets(ps2),
- approvals(), comments(), REVIEW_DB);
+ approvals(), comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"createdOn differs for PatchSet.Id " + c.getId() + ",1:"
+ " {2009-09-30 17:00:02.0} != {2009-09-30 17:00:03.0}");
// One NoteDb, slop is allowed.
b1 = new ChangeBundle(c, messages(), patchSets(ps1), approvals(),
- comments(), NOTE_DB);
+ comments(), reviewers(), NOTE_DB);
b2 = new ChangeBundle(c, messages(), patchSets(ps2), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
// But not too much slop.
@@ -728,9 +733,9 @@
PatchSet ps3 = clone(ps1);
ps3.setCreatedOn(TimeUtil.nowTs());
b1 = new ChangeBundle(c, messages(), patchSets(ps1), approvals(),
- comments(), NOTE_DB);
+ comments(), reviewers(), NOTE_DB);
ChangeBundle b3 = new ChangeBundle(c, messages(), patchSets(ps3),
- approvals(), comments(), REVIEW_DB);
+ approvals(), comments(), reviewers(), REVIEW_DB);
String msg = "createdOn differs for PatchSet.Id " + c.getId()
+ ",1 in NoteDb vs. ReviewDb:"
+ " {2009-09-30 17:00:02.0} != {2009-09-30 17:00:10.0}";
@@ -752,16 +757,16 @@
ps2.setPushCertificate(ps2.getPushCertificate() + "\n\n");
ChangeBundle b1 = new ChangeBundle(c, messages(), patchSets(ps1),
- approvals(), comments(), NOTE_DB);
+ approvals(), comments(), reviewers(), NOTE_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(), patchSets(ps2),
- approvals(), comments(), REVIEW_DB);
+ approvals(), comments(), reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
assertNoDiffs(b2, b1);
b1 = new ChangeBundle(c, messages(), patchSets(ps1), approvals(),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
b2 = new ChangeBundle(c, messages(), patchSets(ps2), approvals(),
- comments(), NOTE_DB);
+ comments(), reviewers(), NOTE_DB);
assertNoDiffs(b1, b2);
assertNoDiffs(b2, b1);
}
@@ -793,24 +798,24 @@
// Both ReviewDb.
ChangeBundle b1 = new ChangeBundle(c, messages(), patchSets(ps1),
- approvals(a1), comments(), REVIEW_DB);
+ approvals(a1), comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(), patchSets(ps1, ps2),
- approvals(a1, a2), comments(), REVIEW_DB);
+ approvals(a1, a2), comments(), reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
// One NoteDb.
b1 = new ChangeBundle(c, messages(), patchSets(ps1), approvals(a1),
- comments(), NOTE_DB);
+ comments(), reviewers(), NOTE_DB);
b2 = new ChangeBundle(c, messages(), patchSets(ps1, ps2), approvals(a1, a2),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
assertNoDiffs(b2, b1);
// Both NoteDb.
b1 = new ChangeBundle(c, messages(), patchSets(ps1), approvals(a1),
- comments(), NOTE_DB);
+ comments(), reviewers(), NOTE_DB);
b2 = new ChangeBundle(c, messages(), patchSets(ps1, ps2), approvals(a1, a2),
- comments(), NOTE_DB);
+ comments(), reviewers(), NOTE_DB);
assertNoDiffs(b1, b2);
}
@@ -830,9 +835,9 @@
TimeUtil.nowTs());
ChangeBundle b1 = new ChangeBundle(c, messages(), latest(c), approvals(a1),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(), latest(c), approvals(a2),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"PatchSetApproval.Key sets differ:"
@@ -850,9 +855,9 @@
TimeUtil.nowTs());
PatchSetApproval a2 = clone(a1);
ChangeBundle b1 = new ChangeBundle(c, messages(), latest(c), approvals(a1),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(), latest(c), approvals(a2),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
@@ -877,9 +882,9 @@
// Both are ReviewDb, exact timestamp match is required.
ChangeBundle b1 = new ChangeBundle(c, messages(), latest(c), approvals(a1),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(), latest(c), approvals(a2),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"granted differs for PatchSetApproval.Key "
+ c.getId() + "%2C1,100,Code-Review:"
@@ -887,9 +892,9 @@
// One NoteDb, slop is allowed.
b1 = new ChangeBundle(c, messages(), latest(c), approvals(a1), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
b2 = new ChangeBundle(c, messages(), latest(c), approvals(a2), comments(),
- REVIEW_DB);
+ reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
// But not too much slop.
@@ -897,9 +902,9 @@
PatchSetApproval a3 = clone(a1);
a3.setGranted(TimeUtil.nowTs());
b1 = new ChangeBundle(c, messages(), latest(c), approvals(a1), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
ChangeBundle b3 = new ChangeBundle(c, messages(), latest(c), approvals(a3),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
String msg = "granted differs for PatchSetApproval.Key "
+ c.getId() + "%2C1,100,Code-Review in NoteDb vs. ReviewDb:"
+ " {2009-09-30 17:00:07.0} != {2009-09-30 17:00:15.0}";
@@ -923,9 +928,9 @@
// Both are ReviewDb, exact match is required.
ChangeBundle b1 = new ChangeBundle(c, messages(), latest(c), approvals(a1),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(), latest(c), approvals(a2),
- comments(), REVIEW_DB);
+ comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"granted differs for PatchSetApproval.Key "
+ c.getId() + "%2C1,100,Code-Review:"
@@ -933,14 +938,91 @@
// Truncating NoteDb timestamp is allowed.
b1 = new ChangeBundle(c, messages(), latest(c), approvals(a1), comments(),
- NOTE_DB);
+ reviewers(), NOTE_DB);
b2 = new ChangeBundle(c, messages(), latest(c), approvals(a2), comments(),
- REVIEW_DB);
+ reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
assertNoDiffs(b2, b1);
}
@Test
+ public void diffReviewerKeySets() throws Exception {
+ Change c = TestChanges.newChange(project, accountId);
+ Timestamp now = TimeUtil.nowTs();
+ ReviewerSet r1 = reviewers(REVIEWER, new Account.Id(1), now);
+ ReviewerSet r2 = reviewers(REVIEWER, new Account.Id(2), now);
+
+ ChangeBundle b1 = new ChangeBundle(c, messages(), latest(c), approvals(),
+ comments(), r1, REVIEW_DB);
+ ChangeBundle b2 = new ChangeBundle(c, messages(), latest(c), approvals(),
+ comments(), r2, REVIEW_DB);
+ assertNoDiffs(b1, b1);
+ assertNoDiffs(b2, b2);
+ assertDiffs(b1, b2,
+ "ReviewerKey sets differ:"
+ + " [REVIEWER,1] only in A;"
+ + " [REVIEWER,2] only in B");
+ }
+
+ @Test
+ public void diffReviewerTimestamps() throws Exception {
+ Change c = TestChanges.newChange(project, accountId);
+ ReviewerSet r1 = reviewers(REVIEWER, new Account.Id(1), TimeUtil.nowTs());
+ ReviewerSet r2 = reviewers(REVIEWER, new Account.Id(1), TimeUtil.nowTs());
+
+ ChangeBundle b1 = new ChangeBundle(c, messages(), latest(c), approvals(),
+ comments(), r1, REVIEW_DB);
+ ChangeBundle b2 = new ChangeBundle(c, messages(), latest(c), approvals(),
+ comments(), r2, REVIEW_DB);
+ assertDiffs(b1, b2,
+ "timestamp differs for ReviewerKey REVIEWER,1:"
+ + " {2009-09-30 17:00:06.0} != {2009-09-30 17:00:12.0}");
+
+ b1 = new ChangeBundle(c, messages(), latest(c), approvals(), comments(), r1,
+ REVIEW_DB);
+ b2 = new ChangeBundle(c, messages(), latest(c), approvals(), comments(), r2,
+ NOTE_DB);
+ assertDiffs(b1, b2,
+ "timestamp differs for ReviewerKey REVIEWER,1 in NoteDb vs. ReviewDb:"
+ + " {2009-09-30 17:00:12.0} != {2009-09-30 17:00:06.0}");
+ }
+
+ @Test
+ public void diffReviewerTimestampsAllowsSlop() throws Exception {
+ subWindowResolution();
+ Change c = TestChanges.newChange(project, accountId);
+ ReviewerSet r1 = reviewers(REVIEWER, new Account.Id(1), TimeUtil.nowTs());
+ ReviewerSet r2 = reviewers(REVIEWER, new Account.Id(1), TimeUtil.nowTs());
+
+ // Both are ReviewDb, exact timestamp match is required.
+ ChangeBundle b1 = new ChangeBundle(c, messages(), latest(c), approvals(),
+ comments(), r1, REVIEW_DB);
+ ChangeBundle b2 = new ChangeBundle(c, messages(), latest(c), approvals(),
+ comments(), r2, REVIEW_DB);
+ assertDiffs(b1, b2,
+ "timestamp differs for ReviewerKey REVIEWER,1:"
+ + " {2009-09-30 17:00:02.0} != {2009-09-30 17:00:03.0}");
+
+ // One NoteDb, slop is allowed
+ b1 = new ChangeBundle(c, messages(), latest(c), approvals(), comments(), r1,
+ NOTE_DB);
+ b2 = new ChangeBundle(c, messages(), latest(c), approvals(), comments(), r2,
+ REVIEW_DB);
+ assertNoDiffs(b1, b2);
+
+ // But not too much slop.
+ superWindowResolution();
+ ReviewerSet r3 = reviewers(REVIEWER, new Account.Id(1), TimeUtil.nowTs());
+ b1 = new ChangeBundle(c, messages(), latest(c), approvals(), comments(), r1,
+ NOTE_DB);
+ ChangeBundle b3 = new ChangeBundle(c, messages(), latest(c), approvals(),
+ comments(), r3, REVIEW_DB);
+ assertDiffs(b1, b3,
+ "timestamp differs for ReviewerKey REVIEWER,1 in NoteDb vs. ReviewDb:"
+ + " {2009-09-30 17:00:02.0} != {2009-09-30 17:00:10.0}");
+ }
+
+ @Test
public void diffPatchLineCommentKeySets() throws Exception {
Change c = TestChanges.newChange(project, accountId);
int id = c.getId().get();
@@ -954,9 +1036,9 @@
5, accountId, null, TimeUtil.nowTs());
ChangeBundle b1 = new ChangeBundle(c, messages(), latest(c), approvals(),
- comments(c1), REVIEW_DB);
+ comments(c1), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(), latest(c), approvals(),
- comments(c2), REVIEW_DB);
+ comments(c2), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"PatchLineComment.Key sets differ:"
@@ -973,9 +1055,9 @@
5, accountId, null, TimeUtil.nowTs());
PatchLineComment c2 = clone(c1);
ChangeBundle b1 = new ChangeBundle(c, messages(), latest(c), approvals(),
- comments(c1), REVIEW_DB);
+ comments(c1), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(), latest(c), approvals(),
- comments(c2), REVIEW_DB);
+ comments(c2), reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
@@ -999,9 +1081,9 @@
// Both are ReviewDb, exact timestamp match is required.
ChangeBundle b1 = new ChangeBundle(c, messages(), latest(c), approvals(),
- comments(c1), REVIEW_DB);
+ comments(c1), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(), latest(c), approvals(),
- comments(c2), REVIEW_DB);
+ comments(c2), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"writtenOn differs for PatchLineComment.Key "
+ c.getId() + ",1,filename,uuid:"
@@ -1009,9 +1091,9 @@
// One NoteDb, slop is allowed.
b1 = new ChangeBundle(c, messages(), latest(c), approvals(), comments(c1),
- NOTE_DB);
+ reviewers(), NOTE_DB);
b2 = new ChangeBundle(c, messages(), latest(c), approvals(), comments(c2),
- REVIEW_DB);
+ reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
// But not too much slop.
@@ -1019,9 +1101,9 @@
PatchLineComment c3 = clone(c1);
c3.setWrittenOn(TimeUtil.nowTs());
b1 = new ChangeBundle(c, messages(), latest(c), approvals(), comments(c1),
- NOTE_DB);
+ reviewers(), NOTE_DB);
ChangeBundle b3 = new ChangeBundle(c, messages(), latest(c), approvals(),
- comments(c3), REVIEW_DB);
+ comments(c3), reviewers(), REVIEW_DB);
String msg = "writtenOn differs for PatchLineComment.Key " + c.getId()
+ ",1,filename,uuid in NoteDb vs. ReviewDb:"
+ " {2009-09-30 17:00:02.0} != {2009-09-30 17:00:10.0}";
@@ -1043,9 +1125,9 @@
5, accountId, null, TimeUtil.nowTs());
ChangeBundle b1 = new ChangeBundle(c, messages(), latest(c), approvals(),
- comments(c1, c2), REVIEW_DB);
+ comments(c1, c2), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(), latest(c), approvals(),
- comments(c1), REVIEW_DB);
+ comments(c1), reviewers(), REVIEW_DB);
assertNoDiffs(b1, b2);
}
@@ -1085,6 +1167,17 @@
return Arrays.asList(ents);
}
+ private static ReviewerSet reviewers(Object... ents) {
+ checkArgument(ents.length % 3 == 0);
+ Table<ReviewerStateInternal, Account.Id, Timestamp> t =
+ HashBasedTable.create();
+ for (int i = 0; i < ents.length; i += 3) {
+ t.put((ReviewerStateInternal) ents[i], (Account.Id) ents[i + 1],
+ (Timestamp) ents[i + 2]);
+ }
+ return ReviewerSet.fromTable(t);
+ }
+
private static List<PatchLineComment> comments(PatchLineComment... ents) {
return Arrays.asList(ents);
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 3f77498..1f54f55 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -28,7 +28,7 @@
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.ImmutableSetMultimap;
+import com.google.common.collect.ImmutableTable;
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
@@ -46,6 +46,7 @@
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
import com.google.gerrit.testutil.TestChanges;
import com.google.gwtorm.server.OrmException;
@@ -392,10 +393,12 @@
update.commit();
ChangeNotes notes = newNotes(c);
- assertThat(notes.getReviewers()).isEqualTo(
- ImmutableSetMultimap.of(
- REVIEWER, new Account.Id(1),
- REVIEWER, new Account.Id(2)));
+ Timestamp ts = new Timestamp(update.getWhen().getTime());
+ assertThat(notes.getReviewers()).isEqualTo(ReviewerSet.fromTable(
+ ImmutableTable.<ReviewerStateInternal, Account.Id, Timestamp>builder()
+ .put(REVIEWER, new Account.Id(1), ts)
+ .put(REVIEWER, new Account.Id(2), ts)
+ .build()));
}
@Test
@@ -407,10 +410,12 @@
update.commit();
ChangeNotes notes = newNotes(c);
- assertThat(notes.getReviewers()).isEqualTo(
- ImmutableSetMultimap.of(
- REVIEWER, new Account.Id(1),
- CC, new Account.Id(2)));
+ Timestamp ts = new Timestamp(update.getWhen().getTime());
+ assertThat(notes.getReviewers()).isEqualTo(ReviewerSet.fromTable(
+ ImmutableTable.<ReviewerStateInternal, Account.Id, Timestamp>builder()
+ .put(REVIEWER, new Account.Id(1), ts)
+ .put(CC, new Account.Id(2), ts)
+ .build()));
}
@Test
@@ -421,16 +426,18 @@
update.commit();
ChangeNotes notes = newNotes(c);
- assertThat(notes.getReviewers()).isEqualTo(
- ImmutableSetMultimap.of(REVIEWER, new Account.Id(2)));
+ Timestamp ts = new Timestamp(update.getWhen().getTime());
+ assertThat(notes.getReviewers()).isEqualTo(ReviewerSet.fromTable(
+ ImmutableTable.of(REVIEWER, new Account.Id(2), ts)));
update = newUpdate(c, otherUser);
update.putReviewer(otherUser.getAccount().getId(), CC);
update.commit();
notes = newNotes(c);
- assertThat(notes.getReviewers()).isEqualTo(
- ImmutableSetMultimap.of(CC, new Account.Id(2)));
+ ts = new Timestamp(update.getWhen().getTime());
+ assertThat(notes.getReviewers()).isEqualTo(ReviewerSet.fromTable(
+ ImmutableTable.of(CC, new Account.Id(2), ts)));
}
@Test
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index ec9b716..dad134b 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -571,7 +571,7 @@
.reviewer(user.getAccountId().toString())
.votes();
assertThat(m).hasSize(1);
- assertThat(m).containsEntry("Code-Review", new Short((short)1));
+ assertThat(m).containsEntry("Code-Review", Short.valueOf((short) 1));
Map<Integer, Change> changes = new LinkedHashMap<>(5);
changes.put(2, reviewPlus2Change);
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/util/SubmoduleSectionParserTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/util/SubmoduleSectionParserTest.java
deleted file mode 100644
index ba62cf7..0000000
--- a/gerrit-server/src/test/java/com/google/gerrit/server/util/SubmoduleSectionParserTest.java
+++ /dev/null
@@ -1,290 +0,0 @@
-// Copyright (C) 2011 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 static org.easymock.EasyMock.createNiceMock;
-import static org.easymock.EasyMock.createStrictMock;
-import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.verify;
-import static org.junit.Assert.assertEquals;
-
-import com.google.gerrit.reviewdb.client.Branch;
-import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.SubmoduleSubscription;
-import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.project.ProjectState;
-
-import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase;
-import org.eclipse.jgit.lib.BlobBasedConfig;
-import org.eclipse.jgit.lib.Constants;
-import org.junit.Before;
-import org.junit.Test;
-
-import java.net.URI;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
-import java.util.TreeMap;
-
-public class SubmoduleSectionParserTest extends LocalDiskRepositoryTestCase {
- private static final String THIS_SERVER = "localhost";
- private ProjectCache projectCache;
- private BlobBasedConfig bbc;
-
- @Override
- @Before
- public void setUp() throws Exception {
- super.setUp();
-
- projectCache = createStrictMock(ProjectCache.class);
- bbc = createStrictMock(BlobBasedConfig.class);
- }
-
- private void doReplay() {
- replay(projectCache, bbc);
- }
-
- private void doVerify() {
- verify(projectCache, bbc);
- }
-
- @Test
- public void testSubmodulesParseWithCorrectSections() throws Exception {
- final Map<String, SubmoduleSection> sectionsToReturn = new TreeMap<>();
- sectionsToReturn.put("a", new SubmoduleSection("ssh://localhost/a", "a",
- "."));
- sectionsToReturn.put("b", new SubmoduleSection("ssh://localhost/b", "b",
- "."));
- sectionsToReturn.put("c", new SubmoduleSection("ssh://localhost/test/c",
- "c-path", "refs/heads/master"));
- sectionsToReturn.put("d", new SubmoduleSection("ssh://localhost/d",
- "d-parent/the-d-folder", "refs/heads/test"));
- sectionsToReturn.put("e", new SubmoduleSection("ssh://localhost/e.git", "e",
- "."));
-
- Map<String, String> reposToBeFound = new HashMap<>();
- reposToBeFound.put("a", "a");
- reposToBeFound.put("b", "b");
- reposToBeFound.put("c", "test/c");
- reposToBeFound.put("d", "d");
- reposToBeFound.put("e", "e");
-
- final Branch.NameKey superBranchNameKey =
- new Branch.NameKey(new Project.NameKey("super-project"),
- "refs/heads/master");
-
- Set<SubmoduleSubscription> expectedSubscriptions = new HashSet<>();
- expectedSubscriptions
- .add(new SubmoduleSubscription(superBranchNameKey, new Branch.NameKey(
- new Project.NameKey("a"), "refs/heads/master"), "a"));
- expectedSubscriptions
- .add(new SubmoduleSubscription(superBranchNameKey, new Branch.NameKey(
- new Project.NameKey("b"), "refs/heads/master"), "b"));
- expectedSubscriptions.add(new SubmoduleSubscription(superBranchNameKey,
- new Branch.NameKey(new Project.NameKey("test/c"), "refs/heads/master"),
- "c-path"));
- expectedSubscriptions.add(new SubmoduleSubscription(superBranchNameKey,
- new Branch.NameKey(new Project.NameKey("d"), "refs/heads/test"),
- "d-parent/the-d-folder"));
- expectedSubscriptions
- .add(new SubmoduleSubscription(superBranchNameKey, new Branch.NameKey(
- new Project.NameKey("e"), "refs/heads/master"), "e"));
-
- execute(superBranchNameKey, sectionsToReturn, reposToBeFound,
- expectedSubscriptions);
- }
-
- @Test
- public void testSubmodulesParseWithAnInvalidSection() throws Exception {
- final Map<String, SubmoduleSection> sectionsToReturn = new TreeMap<>();
- sectionsToReturn.put("a", new SubmoduleSection("ssh://localhost/a", "a",
- "."));
- // This one is invalid since "b" is not a recognized project
- sectionsToReturn.put("b", new SubmoduleSection("ssh://localhost/b", "b",
- "."));
- sectionsToReturn.put("c", new SubmoduleSection("ssh://localhost/test/c",
- "c-path", "refs/heads/master"));
- sectionsToReturn.put("d", new SubmoduleSection("ssh://localhost/d",
- "d-parent/the-d-folder", "refs/heads/test"));
- sectionsToReturn.put("e", new SubmoduleSection("ssh://localhost/e.git", "e",
- "."));
-
- // "b" will not be in this list
- Map<String, String> reposToBeFound = new HashMap<>();
- reposToBeFound.put("a", "a");
- reposToBeFound.put("c", "test/c");
- reposToBeFound.put("d", "d");
- reposToBeFound.put("e", "e");
-
- final Branch.NameKey superBranchNameKey =
- new Branch.NameKey(new Project.NameKey("super-project"),
- "refs/heads/master");
-
- Set<SubmoduleSubscription> expectedSubscriptions = new HashSet<>();
- expectedSubscriptions
- .add(new SubmoduleSubscription(superBranchNameKey, new Branch.NameKey(
- new Project.NameKey("a"), "refs/heads/master"), "a"));
- expectedSubscriptions.add(new SubmoduleSubscription(superBranchNameKey,
- new Branch.NameKey(new Project.NameKey("test/c"), "refs/heads/master"),
- "c-path"));
- expectedSubscriptions.add(new SubmoduleSubscription(superBranchNameKey,
- new Branch.NameKey(new Project.NameKey("d"), "refs/heads/test"),
- "d-parent/the-d-folder"));
- expectedSubscriptions
- .add(new SubmoduleSubscription(superBranchNameKey, new Branch.NameKey(
- new Project.NameKey("e"), "refs/heads/master"), "e"));
-
- execute(superBranchNameKey, sectionsToReturn, reposToBeFound,
- expectedSubscriptions);
- }
-
- @Test
- public void testSubmoduleSectionToOtherServer() throws Exception {
- Map<String, SubmoduleSection> sectionsToReturn = new HashMap<>();
- // The url is not to this server.
- sectionsToReturn.put("a", new SubmoduleSection("ssh://review.source.com/a",
- "a", "."));
-
- Set<SubmoduleSubscription> expectedSubscriptions = Collections.emptySet();
- execute(new Branch.NameKey(new Project.NameKey("super-project"),
- "refs/heads/master"), sectionsToReturn, new HashMap<String, String>(),
- expectedSubscriptions);
- }
-
- @Test
- public void testProjectNotFound() throws Exception {
- Map<String, SubmoduleSection> sectionsToReturn = new HashMap<>();
- sectionsToReturn.put("a", new SubmoduleSection("ssh://localhost/a", "a",
- "."));
-
- Set<SubmoduleSubscription> expectedSubscriptions = Collections.emptySet();
- execute(new Branch.NameKey(new Project.NameKey("super-project"),
- "refs/heads/master"), sectionsToReturn, new HashMap<String, String>(),
- expectedSubscriptions);
- }
-
- @Test
- public void testProjectWithSlashesNotFound() throws Exception {
- Map<String, SubmoduleSection> sectionsToReturn = new HashMap<>();
- sectionsToReturn.put("project", new SubmoduleSection(
- "ssh://localhost/company/tools/project", "project", "."));
-
- Set<SubmoduleSubscription> expectedSubscriptions = Collections.emptySet();
- execute(new Branch.NameKey(new Project.NameKey("super-project"),
- "refs/heads/master"), sectionsToReturn, new HashMap<String, String>(),
- expectedSubscriptions);
- }
-
- @Test
- public void testSubmodulesParseWithSubProjectFound() throws Exception {
- Map<String, SubmoduleSection> sectionsToReturn = new TreeMap<>();
- sectionsToReturn.put("a/b", new SubmoduleSection(
- "ssh://localhost/a/b", "a/b", "."));
-
- Map<String, String> reposToBeFound = new HashMap<>();
- reposToBeFound.put("a/b", "a/b");
- reposToBeFound.put("b", "b");
-
- Branch.NameKey superBranchNameKey =
- new Branch.NameKey(new Project.NameKey("super-project"),
- "refs/heads/master");
-
- Set<SubmoduleSubscription> expectedSubscriptions = new HashSet<>();
- expectedSubscriptions
- .add(new SubmoduleSubscription(superBranchNameKey, new Branch.NameKey(
- new Project.NameKey("a/b"), "refs/heads/master"), "a/b"));
- execute(superBranchNameKey, sectionsToReturn, reposToBeFound,
- expectedSubscriptions);
- }
-
- private void execute(final Branch.NameKey superProjectBranch,
- final Map<String, SubmoduleSection> sectionsToReturn,
- final Map<String, String> reposToBeFound,
- final Set<SubmoduleSubscription> expectedSubscriptions) throws Exception {
- expect(bbc.getSubsections("submodule"))
- .andReturn(sectionsToReturn.keySet());
-
- for (Map.Entry<String, SubmoduleSection> entry : sectionsToReturn.entrySet()) {
- String id = entry.getKey();
- final SubmoduleSection section = entry.getValue();
- expect(bbc.getString("submodule", id, "url")).andReturn(section.getUrl());
- expect(bbc.getString("submodule", id, "path")).andReturn(
- section.getPath());
- expect(bbc.getString("submodule", id, "branch")).andReturn(
- section.getBranch());
-
- if (THIS_SERVER.equals(new URI(section.getUrl()).getHost())) {
- String projectNameCandidate;
- final String urlExtractedPath = new URI(section.getUrl()).getPath();
- int fromIndex = urlExtractedPath.length() - 1;
- while (fromIndex > 0) {
- fromIndex = urlExtractedPath.lastIndexOf('/', fromIndex - 1);
- projectNameCandidate = urlExtractedPath.substring(fromIndex + 1);
- if (projectNameCandidate.endsWith(Constants.DOT_GIT_EXT)) {
- projectNameCandidate = projectNameCandidate.substring(0, //
- projectNameCandidate.length() - Constants.DOT_GIT_EXT.length());
- }
- if (reposToBeFound.containsValue(projectNameCandidate)) {
- expect(projectCache.get(new Project.NameKey(projectNameCandidate)))
- .andReturn(createNiceMock(ProjectState.class));
- } else {
- expect(projectCache.get(new Project.NameKey(projectNameCandidate)))
- .andReturn(null);
- }
- }
- }
- }
-
- doReplay();
-
- final SubmoduleSectionParser ssp =
- new SubmoduleSectionParser(projectCache, bbc, THIS_SERVER,
- superProjectBranch);
-
- Set<SubmoduleSubscription> returnedSubscriptions = ssp.parseAllSections();
-
- doVerify();
-
- assertEquals(expectedSubscriptions, returnedSubscriptions);
- }
-
- private static final class SubmoduleSection {
- private final String url;
- private final String path;
- private final String branch;
-
- SubmoduleSection(final String url, final String path,
- final String branch) {
- this.url = url;
- this.path = path;
- this.branch = branch;
- }
-
- public String getUrl() {
- return url;
- }
-
- public String getPath() {
- return path;
- }
-
- public String getBranch() {
- return branch;
- }
- }
-}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AliasCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AliasCommand.java
index bbad2be..fde3a66 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AliasCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AliasCommand.java
@@ -63,19 +63,19 @@
for (String name : chain(command)) {
CommandProvider p = map.get(name);
if (p == null) {
- throw new UnloggedFailure(1, getName() + ": not found");
+ throw die(getName() + ": not found");
}
Command cmd = p.getProvider().get();
if (!(cmd instanceof DispatchCommand)) {
- throw new UnloggedFailure(1, getName() + ": not found");
+ throw die(getName() + ": not found");
}
map = ((DispatchCommand) cmd).getMap();
}
CommandProvider p = map.get(command.value());
if (p == null) {
- throw new UnloggedFailure(1, getName() + ": not found");
+ throw die(getName() + ": not found");
}
Command cmd = p.getProvider().get();
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java
index f296ef3..2873c37 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java
@@ -377,6 +377,14 @@
return new UnloggedFailure(1, "fatal: " + why.getMessage(), why);
}
+ protected void writeError(String type, String msg) {
+ try {
+ err.write((type + ": " + msg + "\n").getBytes(ENC));
+ } catch (IOException e) {
+ // Ignored
+ }
+ }
+
public void checkExclusivity(final Object arg1, final String arg1name,
final Object arg2, final String arg2name) throws UnloggedFailure {
if (arg1 != null && arg2 != null) {
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java
new file mode 100644
index 0000000..aa361af
--- /dev/null
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java
@@ -0,0 +1,85 @@
+// Copyright (C) 2016 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.sshd;
+
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ChangeFinder;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.change.ChangesCollection;
+import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.sshd.BaseCommand.UnloggedFailure;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+public class ChangeArgumentParser {
+ private final CurrentUser currentUser;
+ private final ChangesCollection changesCollection;
+ private final ChangeFinder changeFinder;
+ private final ReviewDb db;
+
+ @Inject
+ ChangeArgumentParser(CurrentUser currentUser,
+ ChangesCollection changesCollection,
+ ChangeFinder changeFinder,
+ ReviewDb db) {
+ this.currentUser = currentUser;
+ this.changesCollection = changesCollection;
+ this.changeFinder = changeFinder;
+ this.db = db;
+ }
+
+ public void addChange(String id, Map<Change.Id, ChangeResource> changes)
+ throws UnloggedFailure, OrmException {
+ addChange(id, changes, null);
+ }
+
+ public void addChange(String id, Map<Change.Id, ChangeResource> changes,
+ ProjectControl projectControl) throws UnloggedFailure, OrmException {
+ List<ChangeControl> matched = changeFinder.find(id, currentUser);
+ List<ChangeControl> toAdd = new ArrayList<>(changes.size());
+ for (ChangeControl ctl : matched) {
+ if (!changes.containsKey(ctl.getId())
+ && inProject(projectControl, ctl.getProject())
+ && ctl.isVisible(db)) {
+ toAdd.add(ctl);
+ }
+ }
+
+ if (toAdd.isEmpty()) {
+ throw new UnloggedFailure(1, "\"" + id + "\" no such change");
+ } else if (toAdd.size() > 1) {
+ throw new UnloggedFailure(1, "\"" + id + "\" matches multiple changes");
+ }
+ ChangeControl ctl = toAdd.get(0);
+ changes.put(ctl.getId(), changesCollection.parse(ctl));
+ }
+
+ private boolean inProject(ProjectControl projectControl, Project project) {
+ if (projectControl != null) {
+ return projectControl.getProject().getNameKey().equals(project.getNameKey());
+ } else {
+ // No --project option, so they want every project.
+ return true;
+ }
+ }
+}
\ No newline at end of file
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DispatchCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DispatchCommand.java
index 8873be9..f2911dc 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DispatchCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DispatchCommand.java
@@ -73,7 +73,7 @@
if (Strings.isNullOrEmpty(commandName)) {
StringWriter msg = new StringWriter();
msg.write(usage());
- throw new UnloggedFailure(1, msg.toString());
+ throw die(msg.toString());
}
final CommandProvider p = commands.get(commandName);
@@ -81,7 +81,7 @@
String msg =
(getName().isEmpty() ? "Gerrit Code Review" : getName()) + ": "
+ commandName + ": not found";
- throw new UnloggedFailure(1, msg);
+ throw die(msg);
}
final Command cmd = p.getProvider().get();
@@ -96,7 +96,7 @@
bc.setArguments(args.toArray(new String[args.size()]));
} else if (!args.isEmpty()) {
- throw new UnloggedFailure(1, commandName + " does not take arguments");
+ throw die(commandName + " does not take arguments");
}
provideStateTo(cmd);
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java
index 4308db9..24bd8c2 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java
@@ -115,10 +115,9 @@
if (caller instanceof PeerDaemonUser) {
// OK.
} else if (!enableRunAs) {
- throw new UnloggedFailure(1,
- "fatal: suexec disabled by auth.enableRunAs = false");
+ throw die("suexec disabled by auth.enableRunAs = false");
} else if (!caller.getCapabilities().canRunAs()) {
- throw new UnloggedFailure(1, "fatal: suexec not permitted");
+ throw die("suexec not permitted");
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminQueryShell.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminQueryShell.java
index 850026b..237d844 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminQueryShell.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminQueryShell.java
@@ -47,7 +47,7 @@
try {
checkPermission();
} catch (PermissionDeniedException err) {
- throw new UnloggedFailure("fatal: " + err.getMessage());
+ throw die(err.getMessage());
}
QueryShell shell = factory.create(in, out);
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminSetParent.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminSetParent.java
index b4594d4..eb0d7b2 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminSetParent.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminSetParent.java
@@ -84,12 +84,11 @@
@Override
protected void run() throws Failure {
if (oldParent == null && children.isEmpty()) {
- throw new UnloggedFailure(1, "fatal: child projects have to be specified as " +
- "arguments or the --children-of option has to be set");
+ throw die("child projects have to be specified as " +
+ "arguments or the --children-of option has to be set");
}
if (oldParent == null && !excludedChildren.isEmpty()) {
- throw new UnloggedFailure(1, "fatal: --exclude can only be used together " +
- "with --children-of");
+ throw die("--exclude can only be used together with --children-of");
}
final StringBuilder err = new StringBuilder();
@@ -164,7 +163,7 @@
while (err.charAt(err.length() - 1) == '\n') {
err.setLength(err.length() - 1);
}
- throw new UnloggedFailure(1, err.toString());
+ throw die(err.toString());
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AproposCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AproposCommand.java
index cc393ce..12f69ed 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AproposCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AproposCommand.java
@@ -48,7 +48,7 @@
docResult.url));
}
} catch (DocQueryException dqe) {
- throw new UnloggedFailure(1, "fatal: " + dqe.getMessage());
+ throw die(dqe);
}
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/BaseTestPrologCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/BaseTestPrologCommand.java
index 301bc0e..9f31ddc 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/BaseTestPrologCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/BaseTestPrologCommand.java
@@ -76,7 +76,7 @@
OutputFormat.JSON.newGson().toJson(result, stdout);
stdout.print('\n');
} catch (Exception e) {
- throw new UnloggedFailure("Processing of prolog script failed: " + e);
+ throw die("Processing of prolog script failed: " + e);
}
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateBranchCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateBranchCommand.java
index b1d09e9..d3ec69c 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateBranchCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateBranchCommand.java
@@ -48,7 +48,7 @@
gApi.projects().name(project.getProject().getNameKey().get())
.branch(name).create(in);
} catch (RestApiException e) {
- throw new UnloggedFailure(1, "fatal: " + e.getMessage(), e);
+ throw die(e);
}
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateProjectCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateProjectCommand.java
index 3ad5156..4ebafb8 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateProjectCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateProjectCommand.java
@@ -134,7 +134,7 @@
try {
if (!suggestParent) {
if (projectName == null) {
- throw new UnloggedFailure(1, "fatal: Project name is required.");
+ throw die("Project name is required.");
}
ProjectInput input = new ProjectInput();
@@ -176,7 +176,7 @@
}
}
} catch (RestApiException | NoSuchProjectException err) {
- throw new UnloggedFailure(1, "fatal: " + err.getMessage(), err);
+ throw die(err);
}
}
@@ -188,7 +188,7 @@
String[] s = pluginConfigValue.split("=");
String[] s2 = s[0].split("\\.");
if (s.length != 2 || s2.length != 2) {
- throw new UnloggedFailure(1, "Invalid plugin config value '"
+ throw die("Invalid plugin config value '"
+ pluginConfigValue
+ "', expected format '<plugin-name>.<parameter-name>=<value>'"
+ " or '<plugin-name>.<parameter-name>=<value1,value2,...>'");
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/FlushCaches.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/FlushCaches.java
index fcf365c..1f03225 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/FlushCaches.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/FlushCaches.java
@@ -60,14 +60,14 @@
try {
if (list) {
if (all || caches.size() > 0) {
- throw error("error: cannot use --list with --all or --cache");
+ throw die("cannot use --list with --all or --cache");
}
doList();
return;
}
if (all && caches.size() > 0) {
- throw error("error: cannot combine --all and --cache");
+ throw die("cannot combine --all and --cache");
} else if (!all && caches.size() == 1 && caches.contains("all")) {
caches.clear();
all = true;
@@ -87,10 +87,6 @@
}
}
- private static UnloggedFailure error(String msg) {
- return new UnloggedFailure(1, msg);
- }
-
@SuppressWarnings("unchecked")
private void doList() {
for (String name : (List<String>) listCaches
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/GarbageCollectionCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/GarbageCollectionCommand.java
index a3fbcb2..520d194 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/GarbageCollectionCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/GarbageCollectionCommand.java
@@ -68,11 +68,10 @@
private void verifyCommandLine() throws UnloggedFailure {
if (!all && projects.isEmpty()) {
- throw new UnloggedFailure(1,
- "needs projects as command arguments or --all option");
+ throw die("needs projects as command arguments or --all option");
}
if (all && !projects.isEmpty()) {
- throw new UnloggedFailure(1,
+ throw die(
"either specify projects as command arguments or use --all option");
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexActivateCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexActivateCommand.java
index c508b1d..4991700 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexActivateCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexActivateCommand.java
@@ -14,8 +14,6 @@
package com.google.gerrit.sshd.commands;
-import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER;
-
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.lucene.LuceneVersionManager;
@@ -28,8 +26,7 @@
@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
@CommandMetaData(name = "activate",
- description = "Activate the latest index version available",
- runsAt = MASTER)
+ description = "Activate the latest index version available")
public class IndexActivateCommand extends SshCommand {
@Argument(index = 0, required = true, metaVar = "INDEX",
@@ -48,8 +45,7 @@
stdout.println("Not activating index, already using latest version");
}
} catch (ReindexerAlreadyRunningException e) {
- throw new UnloggedFailure("Failed to activate latest index: "
- + e.getMessage());
+ throw die("Failed to activate latest index: " + e.getMessage());
}
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java
new file mode 100644
index 0000000..f2c858e
--- /dev/null
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java
@@ -0,0 +1,71 @@
+// Copyright (C) 2016 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.sshd.commands;
+
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.change.Index;
+import com.google.gerrit.sshd.ChangeArgumentParser;
+import com.google.gerrit.sshd.CommandMetaData;
+import com.google.gerrit.sshd.SshCommand;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+
+import org.kohsuke.args4j.Argument;
+
+import java.io.IOException;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+@CommandMetaData(name = "changes", description = "Index changes")
+final class IndexChangesCommand extends SshCommand {
+ @Inject
+ private Index index;
+
+ @Inject
+ private ChangeArgumentParser changeArgumentParser;
+
+ @Argument(index = 0, required = true, multiValued = true, metaVar = "CHANGE",
+ usage = "changes to index")
+ void addChange(String token) {
+ try {
+ changeArgumentParser.addChange(token, changes);
+ } catch (UnloggedFailure e) {
+ throw new IllegalArgumentException(e.getMessage(), e);
+ } catch (OrmException e) {
+ throw new IllegalArgumentException("database is down", e);
+ }
+ }
+
+ private Map<Change.Id, ChangeResource> changes = new LinkedHashMap<>();
+
+ @Override
+ protected void run() throws UnloggedFailure {
+ boolean ok = true;
+ for (ChangeResource rsrc : changes.values()) {
+ try {
+ index.apply(rsrc, new Index.Input());
+ } catch (IOException | RestApiException e) {
+ ok = false;
+ writeError("error", String.format(
+ "failed to index change %s: %s", rsrc.getId(), e.getMessage()));
+ }
+ }
+ if (!ok) {
+ throw die("failed to index one or more changes");
+ }
+ }
+}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexCommandsModule.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexCommandsModule.java
index 3e7b293..633bca8 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexCommandsModule.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexCommandsModule.java
@@ -28,5 +28,6 @@
command(index).toProvider(new DispatchCommandProvider(index));
command(index, IndexActivateCommand.class);
command(index, IndexStartCommand.class);
+ command(index, IndexChangesCommand.class);
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexStartCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexStartCommand.java
index c2c565f..73e9f33 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexStartCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexStartCommand.java
@@ -14,8 +14,6 @@
package com.google.gerrit.sshd.commands;
-import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER;
-
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.lucene.LuceneVersionManager;
@@ -27,8 +25,7 @@
import org.kohsuke.args4j.Argument;
@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
-@CommandMetaData(name = "start", description = "Start the online reindexer",
- runsAt = MASTER)
+@CommandMetaData(name = "start", description = "Start the online reindexer")
public class IndexStartCommand extends SshCommand {
@Argument(index = 0, required = true, metaVar = "INDEX",
@@ -47,7 +44,7 @@
stdout.println("Nothing to reindex, index is already the latest version");
}
} catch (ReindexerAlreadyRunningException e) {
- throw new UnloggedFailure("Failed to start reindexer: " + e.getMessage());
+ throw die("Failed to start reindexer: " + e.getMessage());
}
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java
index bd97286..2e11ef9 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java
@@ -49,7 +49,7 @@
@Override
public void run() throws Exception {
if (impl.getUser() != null && !impl.getProjects().isEmpty()) {
- throw new UnloggedFailure(1, "fatal: --user and --project options are not compatible.");
+ throw die("--user and --project options are not compatible.");
}
impl.display(stdout);
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListProjectsCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListProjectsCommand.java
index 134a719..d81c153 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListProjectsCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListProjectsCommand.java
@@ -34,10 +34,10 @@
if (!impl.getFormat().isJson()) {
List<String> showBranch = impl.getShowBranch();
if (impl.isShowTree() && (showBranch != null) && !showBranch.isEmpty()) {
- throw new UnloggedFailure(1, "fatal: --tree and --show-branch options are not compatible.");
+ throw die("--tree and --show-branch options are not compatible.");
}
if (impl.isShowTree() && impl.isShowDescription()) {
- throw new UnloggedFailure(1, "fatal: --tree and --description options are not compatible.");
+ throw die("--tree and --description options are not compatible.");
}
}
impl.display(out);
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/LsUserRefs.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/LsUserRefs.java
index a70d581..1ac347f 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/LsUserRefs.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/LsUserRefs.java
@@ -109,11 +109,10 @@
+ projectControl.getProject().getNameKey(), e);
}
} catch (RepositoryNotFoundException e) {
- throw new UnloggedFailure("fatal: '"
- + projectControl.getProject().getNameKey() + "': not a git archive");
+ throw die("'" + projectControl.getProject().getNameKey()
+ + "': not a git archive");
} catch (IOException e) {
- throw new UnloggedFailure("fatal: Error opening: '"
- + projectControl.getProject().getNameKey());
+ throw die("Error opening: '" + projectControl.getProject().getNameKey());
}
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Query.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Query.java
index f65a0c9..8bde743 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Query.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Query.java
@@ -102,7 +102,7 @@
super.parseCommandLine();
if (processor.getIncludeFiles() &&
!(processor.getIncludePatchSets() || processor.getIncludeCurrentPatchSet())) {
- throw new UnloggedFailure(1, "--files option needs --patch-sets or --current-patch-set");
+ throw die("--files option needs --patch-sets or --current-patch-set");
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Receive.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Receive.java
index 0b12aa6..011cb91 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Receive.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Receive.java
@@ -83,7 +83,7 @@
Capable r = receive.canUpload();
if (r != Capable.OK) {
- throw new UnloggedFailure(1, "\nfatal: " + r.getMessage());
+ throw die(r.getMessage());
}
verifyProjectVisible("reviewer", reviewerId);
@@ -165,7 +165,7 @@
for (final Account.Id id : who) {
final IdentifiedUser user = identifiedUserFactory.create(id);
if (!projectControl.forUser(user).isVisible()) {
- throw new UnloggedFailure(1, type + " "
+ throw die(type + " "
+ user.getAccount().getFullName() + " cannot access the project");
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
index dad8672..3c5d5a3 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
@@ -153,68 +153,68 @@
protected void run() throws UnloggedFailure {
if (abandonChange) {
if (restoreChange) {
- throw error("abandon and restore actions are mutually exclusive");
+ throw die("abandon and restore actions are mutually exclusive");
}
if (submitChange) {
- throw error("abandon and submit actions are mutually exclusive");
+ throw die("abandon and submit actions are mutually exclusive");
}
if (publishPatchSet) {
- throw error("abandon and publish actions are mutually exclusive");
+ throw die("abandon and publish actions are mutually exclusive");
}
if (deleteDraftPatchSet) {
- throw error("abandon and delete actions are mutually exclusive");
+ throw die("abandon and delete actions are mutually exclusive");
}
if (rebaseChange) {
- throw error("abandon and rebase actions are mutually exclusive");
+ throw die("abandon and rebase actions are mutually exclusive");
}
}
if (publishPatchSet) {
if (restoreChange) {
- throw error("publish and restore actions are mutually exclusive");
+ throw die("publish and restore actions are mutually exclusive");
}
if (submitChange) {
- throw error("publish and submit actions are mutually exclusive");
+ throw die("publish and submit actions are mutually exclusive");
}
if (deleteDraftPatchSet) {
- throw error("publish and delete actions are mutually exclusive");
+ throw die("publish and delete actions are mutually exclusive");
}
}
if (json) {
if (restoreChange) {
- throw error("json and restore actions are mutually exclusive");
+ throw die("json and restore actions are mutually exclusive");
}
if (submitChange) {
- throw error("json and submit actions are mutually exclusive");
+ throw die("json and submit actions are mutually exclusive");
}
if (deleteDraftPatchSet) {
- throw error("json and delete actions are mutually exclusive");
+ throw die("json and delete actions are mutually exclusive");
}
if (publishPatchSet) {
- throw error("json and publish actions are mutually exclusive");
+ throw die("json and publish actions are mutually exclusive");
}
if (abandonChange) {
- throw error("json and abandon actions are mutually exclusive");
+ throw die("json and abandon actions are mutually exclusive");
}
if (changeComment != null) {
- throw error("json and message are mutually exclusive");
+ throw die("json and message are mutually exclusive");
}
if (rebaseChange) {
- throw error("json and rebase actions are mutually exclusive");
+ throw die("json and rebase actions are mutually exclusive");
}
if (changeTag != null) {
- throw error("json and tag actions are mutually exclusive");
+ throw die("json and tag actions are mutually exclusive");
}
}
if (rebaseChange) {
if (deleteDraftPatchSet) {
- throw error("rebase and delete actions are mutually exclusive");
+ throw die("rebase and delete actions are mutually exclusive");
}
if (submitChange) {
- throw error("rebase and submit actions are mutually exclusive");
+ throw die("rebase and submit actions are mutually exclusive");
}
}
if (deleteDraftPatchSet && submitChange) {
- throw error("delete and submit actions are mutually exclusive");
+ throw die("delete and submit actions are mutually exclusive");
}
boolean ok = true;
@@ -232,20 +232,21 @@
}
} catch (RestApiException | UnloggedFailure e) {
ok = false;
- writeError("error: " + e.getMessage() + "\n");
+ writeError("error", e.getMessage() + "\n");
} catch (NoSuchChangeException e) {
ok = false;
- writeError("no such change " + patchSet.getId().getParentKey().get());
+ writeError("error",
+ "no such change " + patchSet.getId().getParentKey().get());
} catch (Exception e) {
ok = false;
- writeError("fatal: internal server error while reviewing "
+ writeError("fatal", "internal server error while reviewing "
+ patchSet.getId() + "\n");
log.error("internal error while reviewing " + patchSet.getId(), e);
}
}
if (!ok) {
- throw error("one or more reviews failed; review output above");
+ throw die("one or more reviews failed; review output above");
}
}
@@ -262,8 +263,8 @@
return OutputFormat.JSON.newGson().
fromJson(CharStreams.toString(r), ReviewInput.class);
} catch (IOException | JsonSyntaxException e) {
- writeError(e.getMessage() + '\n');
- throw error("internal error while reading review input");
+ writeError("error", e.getMessage() + '\n');
+ throw die("internal error while reading review input");
}
}
@@ -321,7 +322,7 @@
revisionApi(patchSet).delete();
}
} catch (IllegalStateException | RestApiException e) {
- throw error(e.getMessage());
+ throw die(e);
}
}
@@ -342,7 +343,7 @@
try {
allProjectsControl = projectControlFactory.controlFor(allProjects);
} catch (NoSuchProjectException e) {
- throw new UnloggedFailure("missing " + allProjects.get());
+ throw die("missing " + allProjects.get());
}
for (LabelType type : allProjectsControl.getLabelTypes().getLabelTypes()) {
@@ -360,16 +361,4 @@
super.parseCommandLine();
}
-
- private void writeError(final String msg) {
- try {
- err.write(msg.getBytes(ENC));
- } catch (IOException e) {
- // Ignored
- }
- }
-
- private static UnloggedFailure error(final String msg) {
- return new UnloggedFailure(1, msg);
- }
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
index 6e03ac1..535f79a 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetAccountCommand.java
@@ -145,16 +145,14 @@
private void validate() throws UnloggedFailure {
if (active && inactive) {
- throw new UnloggedFailure(1,
- "--active and --inactive options are mutually exclusive.");
+ throw die("--active and --inactive options are mutually exclusive.");
}
if (clearHttpPassword && !Strings.isNullOrEmpty(httpPassword)) {
- throw new UnloggedFailure(1,
- "--http-password and --clear-http-password options are mutually " +
- "exclusive.");
+ throw die("--http-password and --clear-http-password options are "
+ + "mutually exclusive.");
}
if (addSshKeys.contains("-") && deleteSshKeys.contains("-")) {
- throw new UnloggedFailure(1, "Only one option may use the stdin");
+ throw die("Only one option may use the stdin");
}
if (deleteSshKeys.contains("ALL")) {
deleteSshKeys = Collections.singletonList("ALL");
@@ -163,8 +161,7 @@
deleteEmails = Collections.singletonList("ALL");
}
if (deleteEmails.contains(preferredEmail)) {
- throw new UnloggedFailure(1,
- "--preferred-email and --delete-email options are mutually " +
+ throw die("--preferred-email and --delete-email options are mutually " +
"exclusive for the same email address.");
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetHeadCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetHeadCommand.java
index b1d1605..4fef018 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetHeadCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetHeadCommand.java
@@ -49,7 +49,7 @@
try {
setHead.apply(new ProjectResource(project), input);
} catch (UnprocessableEntityException e) {
- throw new UnloggedFailure("fatal: " + e.getMessage());
+ throw die(e);
}
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetProjectCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetProjectCommand.java
index 589fbf0..6328fb4 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetProjectCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetProjectCommand.java
@@ -155,7 +155,7 @@
md.setMessage("Project settings updated");
config.commit(md);
} catch (RepositoryNotFoundException notFound) {
- err.append("error: Project ").append(name).append(" not found\n");
+ err.append("Project ").append(name).append(" not found\n");
} catch (IOException | ConfigInvalidException e) {
final String msg = "Cannot update project " + name;
log.error(msg, e);
@@ -167,7 +167,7 @@
while (err.charAt(err.length() - 1) == '\n') {
err.setLength(err.length() - 1);
}
- throw new UnloggedFailure(1, err.toString());
+ throw die(err.toString());
}
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java
index 63cb54f..ac64803 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java
@@ -18,17 +18,12 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.ChangeFinder;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.ChangeResource;
-import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.DeleteReviewer;
import com.google.gerrit.server.change.PostReviewers;
import com.google.gerrit.server.change.ReviewerResource;
-import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.sshd.ChangeArgumentParser;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
import com.google.gwtorm.server.OrmException;
@@ -39,7 +34,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.LinkedHashMap;
@@ -63,10 +57,10 @@
toRemove.add(who);
}
- @Argument(index = 0, required = true, multiValued = true, metaVar = "COMMIT", usage = "changes to modify")
+ @Argument(index = 0, required = true, multiValued = true, metaVar = "CHANGE", usage = "changes to modify")
void addChange(String token) {
try {
- addChangeImpl(token);
+ changeArgumentParser.addChange(token, changes, projectControl);
} catch (UnloggedFailure e) {
throw new IllegalArgumentException(e.getMessage(), e);
} catch (OrmException e) {
@@ -75,9 +69,6 @@
}
@Inject
- private ReviewDb db;
-
- @Inject
private ReviewerResource.Factory reviewerFactory;
@Inject
@@ -87,13 +78,7 @@
private DeleteReviewer deleteReviewer;
@Inject
- private CurrentUser currentUser;
-
- @Inject
- private ChangesCollection changesCollection;
-
- @Inject
- private ChangeFinder changeFinder;
+ private ChangeArgumentParser changeArgumentParser;
private Set<Account.Id> toRemove = new HashSet<>();
@@ -113,7 +98,7 @@
}
if (!ok) {
- throw error("fatal: one or more updates failed; review output above");
+ throw die("one or more updates failed; review output above");
}
}
@@ -159,48 +144,4 @@
return ok;
}
-
- private void addChangeImpl(String id) throws UnloggedFailure, OrmException {
- List<ChangeControl> matched = changeFinder.find(id, currentUser);
- List<ChangeControl> toAdd = new ArrayList<>(changes.size());
- for (ChangeControl ctl : matched) {
- if (!changes.containsKey(ctl.getId()) && inProject(ctl.getProject())
- && ctl.isVisible(db)) {
- toAdd.add(ctl);
- }
- }
- switch (toAdd.size()) {
- case 0:
- throw error("\"" + id + "\" no such change");
-
- case 1:
- ChangeControl ctl = toAdd.get(0);
- changes.put(ctl.getId(), changesCollection.parse(ctl));
- break;
-
- default:
- throw error("\"" + id + "\" matches multiple changes");
- }
- }
-
- private boolean inProject(Project project) {
- if (projectControl != null) {
- return projectControl.getProject().getNameKey().equals(project.getNameKey());
- } else {
- // No --project option, so they want every project.
- return true;
- }
- }
-
- private void writeError(String type, String msg) {
- try {
- err.write((type + ": " + msg + "\n").getBytes(ENC));
- } catch (IOException e) {
- // Ignored
- }
- }
-
- private static UnloggedFailure error(String msg) {
- return new UnloggedFailure(1, msg);
- }
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StreamEvents.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StreamEvents.java
index 2da1b6d..9e6630a 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StreamEvents.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/StreamEvents.java
@@ -14,7 +14,6 @@
package com.google.gerrit.sshd.commands;
-import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.Supplier;
@@ -48,8 +47,7 @@
import java.util.concurrent.LinkedBlockingQueue;
@RequiresCapability(GlobalCapability.STREAM_EVENTS)
-@CommandMetaData(name = "stream-events", description = "Monitor events occurring in real time",
- runsAt = MASTER)
+@CommandMetaData(name = "stream-events", description = "Monitor events occurring in real time")
final class StreamEvents extends BaseCommand {
/** Maximum number of events that may be queued up for each connection. */
private static final int MAX_EVENTS = 128;
diff --git a/lib/js/BUCK b/lib/js/BUCK
index 36a3d19..275941c 100644
--- a/lib/js/BUCK
+++ b/lib/js/BUCK
@@ -17,8 +17,8 @@
npm_binary(
name = 'bower',
- version = '1.6.5',
- sha1 = '59d457122a161e42cc1625bbab8179c214b7ac11',
+ version = '1.7.9',
+ sha1 = 'b7296c2393e0d75edaa6ca39648132dd255812b0',
)
npm_binary(
@@ -102,17 +102,9 @@
bower_component(
name = 'fetch',
package = 'fetch',
- version = '0.11.0',
+ version = '1.0.0',
license = 'fetch',
- sha1 = 'a55d4e291821958d9d400bb3184c12bb367dc670',
-)
-
-bower_component(
- name = 'font-roboto',
- package = 'polymerelements/font-roboto',
- version = '1.0.1',
- license = 'polymer',
- sha1 = '735676217f67221903d6be10cc2fb1b336bed13f',
+ sha1 = '1b05a2bb40c73232c2909dc196de7519fe4db7a9',
)
bower_component(
@@ -127,10 +119,10 @@
bower_component(
name = 'iron-a11y-keys-behavior',
package = 'polymerelements/iron-a11y-keys-behavior',
- version = '1.1.1',
+ version = '1.1.2',
deps = [':polymer'],
license = 'polymer',
- sha1 = '6bb52b967a4fb242897520dad6c366135e3813ce',
+ sha1 = '57fd39ee153ce37ed719ba3f7a405afb987d54f9',
)
bower_component(
@@ -151,19 +143,19 @@
bower_component(
name = 'iron-behaviors',
package = 'polymerelements/iron-behaviors',
- version = '1.0.13',
+ version = '1.0.16',
deps = [
':iron-a11y-keys-behavior',
':polymer',
],
license = 'polymer',
- sha1 = 'e9bcdac5414cb8282b5f75eeb51c9154380045af',
+ sha1 = 'bd70636a2c0a78c50d1a76f9b8ca1ffd815478a3',
)
bower_component(
name = 'iron-dropdown',
package = 'polymerelements/iron-dropdown',
- version = '1.3.0',
+ version = '1.4.0',
deps = [
':iron-a11y-keys-behavior',
':iron-behaviors',
@@ -173,16 +165,16 @@
':polymer',
],
license = 'polymer',
- sha1 = '08ae9c9fa2f2c19a8ab330dfe8240292c8d161cf',
+ sha1 = '63e3d669a09edaa31c4f05afc76b53b919ef0595',
)
bower_component(
name = 'iron-fit-behavior',
package = 'polymerelements/iron-fit-behavior',
- version = '1.0.6',
+ version = '1.2.2',
deps = [':polymer'],
license = 'polymer',
- sha1 = '28df0349d3cb20ac5e4aeb40651ef7d84de75fb0',
+ sha1 = 'bc53e9bab36b21f086ab8fac8c53cc7214aa1890',
)
bower_component(
@@ -206,14 +198,14 @@
bower_component(
name = 'iron-input',
package = 'polymerelements/iron-input',
- version = '1.0.9',
+ version = '1.0.10',
deps = [
':iron-a11y-announcer',
':iron-validatable-behavior',
':polymer',
],
license = 'polymer',
- sha1 = '4e415c2511ec8ff6c8b17249ec8f02e8d8b1a0d9',
+ sha1 = '9bc0c8e81de2527125383cbcf74dd9f27e7fa9ac',
)
bower_component(
@@ -228,7 +220,7 @@
bower_component(
name = 'iron-overlay-behavior',
package = 'polymerelements/iron-overlay-behavior',
- version = '1.4.2',
+ version = '1.7.6',
deps = [
':iron-a11y-keys-behavior',
':iron-fit-behavior',
@@ -236,7 +228,7 @@
':polymer',
],
license = 'polymer',
- sha1 = 'babdd95d7efd63bf3f2969a8f1036e8f324979a9',
+ sha1 = '83181085fda59446ce74fd0d5ca30c223f38ee4a',
)
bower_component(
@@ -251,31 +243,31 @@
bower_component(
name = 'iron-selector',
package = 'polymerelements/iron-selector',
- version = '1.2.5',
+ version = '1.5.2',
deps = [':polymer'],
license = 'polymer',
- sha1 = '7728750bc9dfa858915dfd25397709bdbdaee2b1',
+ sha1 = 'c57235dfda7fbb987c20ad0e97aac70babf1a1bf',
)
bower_component(
name = 'iron-test-helpers',
package = 'polymerelements/iron-test-helpers',
- version = '1.1.5',
+ version = '1.2.5',
deps = [':polymer'],
license = 'DO_NOT_DISTRIBUTE',
- sha1 = '000e2256ae487e4d24edfb6d17dc98626bb8a8e2',
+ sha1 = '433b03b106f5ff32049b84150cd70938e18b67ac',
)
bower_component(
name = 'iron-validatable-behavior',
package = 'polymerelements/iron-validatable-behavior',
- version = '1.0.5',
+ version = '1.1.1',
deps = [
':iron-meta',
':polymer',
],
license = 'polymer',
- sha1 = '5a68250d6d9abcd576f116dc4fc7312426323883',
+ sha1 = '480423380be0536f948735d91bc472f6e7ced5b4',
)
bower_component(
@@ -289,23 +281,23 @@
bower_component(
name = 'mocha',
package = 'mocha',
- version = '2.4.5',
+ version = '2.5.1',
license = 'DO_NOT_DISTRIBUTE',
- sha1 = 'efbb1675710c0ba94a44eb7a4d27040229283197',
+ sha1 = 'cb29bdd1047cfd9304659ecf10ec263f9c888c99',
)
bower_component(
name = 'moment',
package = 'moment/moment',
- version = '2.12.0',
+ version = '2.13.0',
license = 'moment',
- sha1 = '508d53de8f49ab87f03e209e5073e339107ed3e6',
+ sha1 = 'fc8ce2c799bab21f6ced7aff928244f4ca8880aa',
)
bower_component(
name = 'neon-animation',
package = 'polymerelements/neon-animation',
- version = '1.1.1',
+ version = '1.2.3',
deps = [
':iron-meta',
':iron-resizable-behavior',
@@ -314,7 +306,7 @@
':web-animations-js',
],
license = 'polymer',
- sha1 = 'd6e1b45e5a936d0ec0b66b3520e230e9d8605642',
+ sha1 = '71cc0d3e0afdf8b8563e87d2ff03a6fa19183bd9',
)
bower_component(
@@ -326,19 +318,6 @@
)
bower_component(
- name = 'paper-styles',
- package = 'polymerelements/paper-styles',
- version = '1.1.4',
- deps = [
- ':font-roboto',
- ':iron-flex-layout',
- ':polymer',
- ],
- license = 'polymer',
- sha1 = '89276c5ec18b8927a704dda2bf14ff35c310401a',
-)
-
-bower_component(
name = 'polymer',
package = 'polymer/polymer',
version = '1.4.0',
@@ -383,17 +362,17 @@
bower_component(
name = 'test-fixture',
package = 'polymerelements/test-fixture',
- version = '1.1.0',
+ version = '1.1.1',
license = 'DO_NOT_DISTRIBUTE',
- sha1 = '4afc8998ae42b0421847906a7550b997c6fdc088',
+ sha1 = 'e373bd21c069163c3a754e234d52c07c77b20d3c',
)
bower_component(
name = 'web-animations-js',
package = 'web-animations/web-animations-js',
- version = '2.1.4',
+ version = '2.2.1',
license = 'Apache2.0',
- sha1 = '92f06d8417a51f1f75c94b7a19616e19695cc6db',
+ sha1 = '0e73b263a86aa6764ad35c273eb12055f83d7eda',
)
bower_component(
@@ -418,7 +397,8 @@
bower_component(
name = 'webcomponentsjs',
package = 'webcomponentsjs',
- version = '0.7.21',
+ version = '0.7.22',
license = 'polymer',
- sha1 = 'ceb96b01c8a86b17831a25d6ab9eca95226c408e',
+ sha1 = '8ba97a4a279ec6973a19b171c462a7b5cf454fb9',
)
+
diff --git a/plugins/reviewnotes b/plugins/reviewnotes
index c167df0..3f3d572 160000
--- a/plugins/reviewnotes
+++ b/plugins/reviewnotes
@@ -1 +1 @@
-Subproject commit c167df08a8550d8c6c7ccf12b7df4fa6bfc6d432
+Subproject commit 3f3d572e9618f268b19cc54856deee4c96180e4c
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
index a6e65ef..aa5d8bd 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
@@ -60,7 +60,11 @@
return;
}
- this.push('comments', this._newDraft(opt_lineNum));
+ var draft = this._newDraft(opt_lineNum);
+ this.push('comments', draft);
+ this.async(function() {
+ this._commentElWithDraftID(draft.__draftID).editing = true;
+ }.bind(this), 1);
},
_getLoggedIn: function() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
index f7b5f01..525bac0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
@@ -71,16 +71,15 @@
_xhrPromise: Object, // Used for testing.
_messageText: {
type: String,
+ value: '',
observer: '_messageTextChanged',
},
},
- ready: function() {
- this._loadLocalDraft().then(function(loadedLocal) {
- this._messageText = (this.comment && this.comment.message) || '';
- this.editing = !this._messageText.length || loadedLocal;
- }.bind(this));
- },
+ observers: [
+ '_commentMessageChanged(comment.message)',
+ '_loadLocalDraft(changeNum, patchNum, comment)',
+ ],
save: function() {
this.comment.message = this._messageText;
@@ -151,8 +150,12 @@
}
},
+ _commentMessageChanged: function(message) {
+ this._messageText = message || '';
+ },
+
_messageTextChanged: function(newValue, oldValue) {
- if (this.comment && this.comment.id) { return; }
+ if (!this.comment || (this.comment && this.comment.id)) { return; }
this.debounce('store', function() {
var message = this._messageText;
@@ -227,8 +230,10 @@
if (!this.comment.__draft) {
throw Error('Cannot discard a non-draft comment.');
}
+ this.editing = false;
this.disabled = true;
if (!this.comment.id) {
+ this.disabled = false;
this.fire('comment-discard');
return;
}
@@ -259,34 +264,23 @@
draft);
},
- _loadLocalDraft: function() {
- // Use an async promise to avoid blocking render on potentially slow
- // localStorage calls.
- return new Promise(function(resolve) {
- this.async(function() {
- // Only apply local drafts to comments that haven't been saved
- // remotely, and haven't been given a default message already.
- if (!this.comment || this.comment.id || this.comment.message) {
- resolve(false);
- return;
- }
+ _loadLocalDraft: function(changeNum, patchNum, comment) {
+ // Only apply local drafts to comments that haven't been saved
+ // remotely, and haven't been given a default message already.
+ if (!comment || comment.id || comment.message) {
+ return;
+ }
- var draft = this.$.storage.getDraftComment({
- changeNum: this.changeNum,
- patchNum: this.patchNum,
- path: this.comment.path,
- line: this.comment.line,
- });
+ var draft = this.$.storage.getDraftComment({
+ changeNum: changeNum,
+ patchNum: patchNum,
+ path: comment.path,
+ line: comment.line,
+ });
- if (draft) {
- this.comment.message = draft.message;
- resolve(true);
- return;
- }
-
- resolve(false);
- }.bind(this));
- }.bind(this));
+ if (draft) {
+ this.set('comment.message', draft.message);
+ }
},
});
})();
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 7c87037..781f457 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -22,48 +22,49 @@
<script src="../bower_components/web-component-tester/browser.js"></script>
<script>
var testFiles = [];
+ var basePath = '../elements/';
[
- '../elements/change/gr-change-actions/gr-change-actions_test.html',
- '../elements/change/gr-change-metadata/gr-change-metadata_test.html',
- '../elements/change/gr-change-view/gr-change-view_test.html',
- '../elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.html',
- '../elements/change/gr-download-dialog/gr-download-dialog_test.html',
- '../elements/change/gr-file-list/gr-file-list_test.html',
- '../elements/change/gr-message/gr-message_test.html',
- '../elements/change/gr-messages-list/gr-messages-list_test.html',
- '../elements/change/gr-related-changes-list/gr-related-changes-list_test.html',
- '../elements/change/gr-reply-dialog/gr-reply-dialog_test.html',
- '../elements/change/gr-reviewer-list/gr-reviewer-list_test.html',
- '../elements/change-list/gr-change-list/gr-change-list_test.html',
- '../elements/change-list/gr-change-list-item/gr-change-list-item_test.html',
- '../elements/core/gr-account-dropdown/gr-account-dropdown_test.html',
- '../elements/core/gr-error-manager/gr-error-manager_test.html',
- '../elements/core/gr-main-header/gr-main-header_test.html',
- '../elements/core/gr-search-bar/gr-search-bar_test.html',
- '../elements/diff/gr-diff/gr-diff-builder_test.html',
- '../elements/diff/gr-diff/gr-diff-group_test.html',
- '../elements/diff/gr-diff/gr-diff_test.html',
- '../elements/diff/gr-diff-comment/gr-diff-comment_test.html',
- '../elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html',
- '../elements/diff/gr-diff-cursor/gr-diff-cursor_test.html',
- '../elements/diff/gr-diff-preferences/gr-diff-preferences_test.html',
- '../elements/diff/gr-diff-view/gr-diff-view_test.html',
- '../elements/diff/gr-patch-range-select/gr-patch-range-select_test.html',
- '../elements/shared/gr-alert/gr-alert_test.html',
- '../elements/shared/gr-account-label/gr-account-label_test.html',
- '../elements/shared/gr-account-link/gr-account-link_test.html',
- '../elements/shared/gr-alert/gr-alert_test.html',
- '../elements/shared/gr-avatar/gr-avatar_test.html',
- '../elements/shared/gr-change-star/gr-change-star_test.html',
- '../elements/shared/gr-confirm-dialog/gr-confirm-dialog_test.html',
- '../elements/shared/gr-cursor-manager/gr-cursor-manager_test.html',
- '../elements/shared/gr-date-formatter/gr-date-formatter_test.html',
- '../elements/shared/gr-js-api-interface/gr-js-api-interface_test.html',
- '../elements/shared/gr-linked-text/gr-linked-text_test.html',
- '../elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html',
- '../elements/shared/gr-storage/gr-storage_test.html',
+ 'change-list/gr-change-list-item/gr-change-list-item_test.html',
+ 'change-list/gr-change-list/gr-change-list_test.html',
+ 'change/gr-change-actions/gr-change-actions_test.html',
+ 'change/gr-change-metadata/gr-change-metadata_test.html',
+ 'change/gr-change-view/gr-change-view_test.html',
+ 'change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.html',
+ 'change/gr-download-dialog/gr-download-dialog_test.html',
+ 'change/gr-file-list/gr-file-list_test.html',
+ 'change/gr-message/gr-message_test.html',
+ 'change/gr-messages-list/gr-messages-list_test.html',
+ 'change/gr-related-changes-list/gr-related-changes-list_test.html',
+ 'change/gr-reply-dialog/gr-reply-dialog_test.html',
+ 'change/gr-reviewer-list/gr-reviewer-list_test.html',
+ 'core/gr-account-dropdown/gr-account-dropdown_test.html',
+ 'core/gr-error-manager/gr-error-manager_test.html',
+ 'core/gr-main-header/gr-main-header_test.html',
+ 'core/gr-search-bar/gr-search-bar_test.html',
+ 'diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html',
+ 'diff/gr-diff-comment/gr-diff-comment_test.html',
+ 'diff/gr-diff-cursor/gr-diff-cursor_test.html',
+ 'diff/gr-diff-preferences/gr-diff-preferences_test.html',
+ 'diff/gr-diff-view/gr-diff-view_test.html',
+ 'diff/gr-diff/gr-diff-builder_test.html',
+ 'diff/gr-diff/gr-diff-group_test.html',
+ 'diff/gr-diff/gr-diff_test.html',
+ 'diff/gr-patch-range-select/gr-patch-range-select_test.html',
+ 'shared/gr-account-label/gr-account-label_test.html',
+ 'shared/gr-account-link/gr-account-link_test.html',
+ 'shared/gr-alert/gr-alert_test.html',
+ 'shared/gr-avatar/gr-avatar_test.html',
+ 'shared/gr-change-star/gr-change-star_test.html',
+ 'shared/gr-confirm-dialog/gr-confirm-dialog_test.html',
+ 'shared/gr-cursor-manager/gr-cursor-manager_test.html',
+ 'shared/gr-date-formatter/gr-date-formatter_test.html',
+ 'shared/gr-js-api-interface/gr-js-api-interface_test.html',
+ 'shared/gr-linked-text/gr-linked-text_test.html',
+ 'shared/gr-rest-api-interface/gr-rest-api-interface_test.html',
+ 'shared/gr-storage/gr-storage_test.html',
].forEach(function(file) {
+ file = basePath + file;
testFiles.push(file);
testFiles.push(file + '?dom=shadow');
});
