Merge "Fix the running of plugin acceptance tests in eclipse such as CookbookIT"
diff --git a/Documentation/cmd-review.txt b/Documentation/cmd-review.txt
index 70a695e..0590337 100644
--- a/Documentation/cmd-review.txt
+++ b/Documentation/cmd-review.txt
@@ -128,6 +128,12 @@
or invalid value) and votes that are not permitted for the user are
silently ignored.
+--strict-labels::
+ Require ability to vote on all specified labels before reviewing change.
+ If the vote is invalid (invalid label or invalid name), the vote is not
+ permitted for the user, or the vote is on an outdated or closed patch set,
+ return an error instead of silently discarding the vote.
+
== ACCESS
Any user who has configured an SSH key.
diff --git a/Documentation/prolog-cookbook.txt b/Documentation/prolog-cookbook.txt
index b53da4b..d68dc8a 100644
--- a/Documentation/prolog-cookbook.txt
+++ b/Documentation/prolog-cookbook.txt
@@ -48,6 +48,13 @@
Prolog based submit type computes a submit type for each change. The computed
submit type is shown on the change screen for each change.
+When submitting changes in a batch using "Submit including ancestors" or "Submit
+whole topic", submit type rules may not be used to mix submit types on a single
+branch, and trying to submit such a batch will fail. This avoids potentially
+confusing behavior and spurious submit failures. It is recommended to only use
+submit type rules to change submit types for an entire branch, which avoids this
+situation.
+
== Prolog Language
This document is not a complete Prolog tutorial.
link:http://en.wikipedia.org/wiki/Prolog[This Wikipedia page on Prolog] is a
@@ -999,37 +1006,13 @@
submit_type(fast_forward_only) :-
gerrit:change_branch(B), regex_matches('refs/heads/stable.*', B),
!.
-submit_type(T) :- gerrit:project_default_submit_type(T)
+submit_type(T) :- gerrit:project_default_submit_type(T).
----
The first `submit_type` predicate defines the `Fast Forward Only` submit type
for `+refs/heads/stable.*+` branches. The second `submit_type` predicate returns
the project's default submit type.
-=== Example 3: Don't require `Fast Forward Only` if only documentation was changed
-Like in the previous example we want the `Fast Forward Only` submit type for the
-`+refs/heads/stable*+` branches. However, if only documentation was changed
-(only `+*.txt+` files), then we allow project's default submit type for such
-changes.
-
-`rules.pl`
-[source,prolog]
-----
-submit_type(fast_forward_only) :-
- gerrit:commit_delta('(?<!\.txt)$'),
- gerrit:change_branch(B), regex_matches('refs/heads/stable.*', B),
- !.
-submit_type(T) :- gerrit:project_default_submit_type(T)
-----
-
-The `gerrit:commit_delta('(?<!\.txt)$')` succeeds if the change contains a file
-whose name doesn't end with `.txt` The rest of this rule is same like in the
-previous example.
-
-If all file names in the change end with `.txt`, then the
-`gerrit:commit_delta('(?<!\.txt)$')` will fail as no file name will match this
-regular expression.
-
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/ReleaseNotes/ReleaseNotes-2.13.txt b/ReleaseNotes/ReleaseNotes-2.13.txt
index 0f08b56..aba6131 100644
--- a/ReleaseNotes/ReleaseNotes-2.13.txt
+++ b/ReleaseNotes/ReleaseNotes-2.13.txt
@@ -66,6 +66,13 @@
* TODO add more
+Changes
+~~~~~~~
+
+In order to avoid potentially confusing behavior, when submitting changes in a
+batch, submit type rules may not be used to mix submit types on a single branch,
+and trying to submit such a batch will fail.
+
Bug Fixes
---------
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 3cea4a2..4414593 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -552,11 +552,8 @@
protected void saveProjectConfig(Project.NameKey p, ProjectConfig cfg)
throws Exception {
- MetaDataUpdate md = metaDataUpdateFactory.create(p);
- try {
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(p)) {
cfg.commit(md);
- } finally {
- md.close();
}
projectCache.evict(cfg.getProject());
}
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 617315c..ec429c4 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
@@ -470,9 +470,7 @@
// notedb and this record stays even when all votes of that user have been
// deleted, hence there is no dummy 0 approval left when a vote is
// deleted.
- // TODO(dborowitz): Support modifying other users' labels in notedb
- // format.
- //assertThat(m).isEmpty();
+ assertThat(m).isEmpty();
} else {
// 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
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/SubmitTypeRuleIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/SubmitTypeRuleIT.java
new file mode 100644
index 0000000..7fc4719
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/SubmitTypeRuleIT.java
@@ -0,0 +1,264 @@
+// Copyright (C) 2015 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.api.change;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.extensions.client.SubmitType.CHERRY_PICK;
+import static com.google.gerrit.extensions.client.SubmitType.FAST_FORWARD_ONLY;
+import static com.google.gerrit.extensions.client.SubmitType.MERGE_ALWAYS;
+import static com.google.gerrit.extensions.client.SubmitType.MERGE_IF_NECESSARY;
+import static com.google.gerrit.extensions.client.SubmitType.REBASE_IF_NECESSARY;
+import static org.junit.Assert.fail;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.projects.BranchInput;
+import com.google.gerrit.extensions.client.SubmitType;
+import com.google.gerrit.extensions.common.TestSubmitRuleInput;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.git.IntegrationException;
+import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gerrit.server.git.VersionedMetaData;
+import com.google.gerrit.testutil.ConfigSuite;
+
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+
+@NoHttpd
+public class SubmitTypeRuleIT extends AbstractDaemonTest {
+ @ConfigSuite.Default
+ public static Config submitWholeTopicEnabled() {
+ return submitWholeTopicEnabledConfig();
+ }
+
+ private class RulesPl extends VersionedMetaData {
+ private static final String FILENAME = "rules.pl";
+
+ private String rule;
+
+ @Override
+ protected String getRefName() {
+ return RefNames.REFS_CONFIG;
+ }
+
+ @Override
+ protected void onLoad() throws IOException, ConfigInvalidException {
+ rule = readUTF8(FILENAME);
+ }
+
+ @Override
+ protected boolean onSave(CommitBuilder commit)
+ throws IOException, ConfigInvalidException {
+ TestSubmitRuleInput in = new TestSubmitRuleInput();
+ in.rule = rule;
+ try {
+ gApi.changes().id(testChangeId.get()).current().testSubmitType(in);
+ } catch (RestApiException e) {
+ throw new ConfigInvalidException("Invalid submit type rule", e);
+ }
+
+ saveUTF8(FILENAME, rule);
+ return true;
+ }
+ }
+
+ private AtomicInteger fileCounter;
+ private Change.Id testChangeId;
+
+ @Before
+ public void setUp() throws Exception {
+ fileCounter = new AtomicInteger();
+ gApi.projects().name(project.get()).branch("test")
+ .create(new BranchInput());
+ testChangeId = createChange("test", "test change").getChange().getId();
+ }
+
+ private void setRulesPl(String rule) throws Exception {
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
+ RulesPl r = new RulesPl();
+ r.load(md);
+ r.rule = rule;
+ r.commit(md);
+ }
+ }
+
+ private static final String SUBMIT_TYPE_FROM_SUBJECT =
+ "submit_type(fast_forward_only) :-"
+ + "gerrit:commit_message(M),"
+ + "regex_matches('.*FAST_FORWARD_ONLY.*', M),"
+ + "!.\n"
+ + "submit_type(merge_if_necessary) :-"
+ + "gerrit:commit_message(M),"
+ + "regex_matches('.*MERGE_IF_NECESSARY.*', M),"
+ + "!.\n"
+ + "submit_type(rebase_if_necessary) :-"
+ + "gerrit:commit_message(M),"
+ + "regex_matches('.*REBASE_IF_NECESSARY.*', M),"
+ + "!.\n"
+ + "submit_type(merge_always) :-"
+ + "gerrit:commit_message(M),"
+ + "regex_matches('.*MERGE_ALWAYS.*', M),"
+ + "!.\n"
+ + "submit_type(cherry_pick) :-"
+ + "gerrit:commit_message(M),"
+ + "regex_matches('.*CHERRY_PICK.*', M),"
+ + "!.\n"
+ + "submit_type(T) :- gerrit:project_default_submit_type(T).";
+
+ private PushOneCommit.Result createChange(String dest, String subject)
+ throws Exception {
+ PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo,
+ subject, "file" + fileCounter.incrementAndGet(),
+ PushOneCommit.FILE_CONTENT);
+ PushOneCommit.Result r = push.to("refs/for/" + dest);
+ r.assertOkStatus();
+ return r;
+ }
+
+ @Test
+ public void unconditionalCherryPick() throws Exception {
+ PushOneCommit.Result r = createChange();
+ assertSubmitType(MERGE_IF_NECESSARY, r.getChangeId());
+ setRulesPl("submit_type(cherry_pick).");
+ assertSubmitType(CHERRY_PICK, r.getChangeId());
+ }
+
+ @Test
+ public void submitTypeFromSubject() throws Exception {
+ PushOneCommit.Result r1 = createChange("master", "Default 1");
+ PushOneCommit.Result r2 = createChange("master", "FAST_FORWARD_ONLY 2");
+ PushOneCommit.Result r3 = createChange("master", "MERGE_IF_NECESSARY 3");
+ PushOneCommit.Result r4 = createChange("master", "REBASE_IF_NECESSARY 4");
+ PushOneCommit.Result r5 = createChange("master", "MERGE_ALWAYS 5");
+ PushOneCommit.Result r6 = createChange("master", "CHERRY_PICK 6");
+
+ assertSubmitType(MERGE_IF_NECESSARY, r1.getChangeId());
+ assertSubmitType(MERGE_IF_NECESSARY, r2.getChangeId());
+ assertSubmitType(MERGE_IF_NECESSARY, r3.getChangeId());
+ assertSubmitType(MERGE_IF_NECESSARY, r4.getChangeId());
+ assertSubmitType(MERGE_IF_NECESSARY, r5.getChangeId());
+ assertSubmitType(MERGE_IF_NECESSARY, r6.getChangeId());
+
+ setRulesPl(SUBMIT_TYPE_FROM_SUBJECT);
+
+ assertSubmitType(MERGE_IF_NECESSARY, r1.getChangeId());
+ assertSubmitType(FAST_FORWARD_ONLY, r2.getChangeId());
+ assertSubmitType(MERGE_IF_NECESSARY, r3.getChangeId());
+ assertSubmitType(REBASE_IF_NECESSARY, r4.getChangeId());
+ assertSubmitType(MERGE_ALWAYS, r5.getChangeId());
+ assertSubmitType(CHERRY_PICK, r6.getChangeId());
+ }
+
+ @Test
+ public void submitTypeIsUsedForSubmit() throws Exception {
+ setRulesPl(SUBMIT_TYPE_FROM_SUBJECT);
+
+ PushOneCommit.Result r = createChange("master", "CHERRY_PICK 1");
+
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(r.getChangeId()).current().submit();
+
+ List<RevCommit> log = log("master", 1);
+ assertThat(log.get(0).getShortMessage()).isEqualTo("CHERRY_PICK 1");
+ assertThat(log.get(0).name()).isNotEqualTo(r.getCommit().name());
+ assertThat(log.get(0).getFullMessage())
+ .contains("Change-Id: " + r.getChangeId());
+ assertThat(log.get(0).getFullMessage()).contains("Reviewed-on: ");
+ }
+
+ @Test
+ public void mixingSubmitTypesAcrossBranchesSucceeds() throws Exception {
+ setRulesPl(SUBMIT_TYPE_FROM_SUBJECT);
+
+ PushOneCommit.Result r1 = createChange("master", "MERGE_IF_NECESSARY 1");
+
+ RevCommit initialCommit = r1.getCommit().getParent(0);
+ BranchInput bin = new BranchInput();
+ bin.revision = initialCommit.name();
+ gApi.projects().name(project.get()).branch("branch").create(bin);
+
+ testRepo.reset(initialCommit);
+ PushOneCommit.Result r2 = createChange("branch", "MERGE_ALWAYS 1");
+
+ gApi.changes().id(r1.getChangeId()).topic(name("topic"));
+ gApi.changes().id(r1.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(r2.getChangeId()).topic(name("topic"));
+ gApi.changes().id(r2.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(r2.getChangeId()).current().submit();
+
+ assertThat(log("master", 1).get(0).name()).isEqualTo(r1.getCommit().name());
+
+ List<RevCommit> branchLog = log("branch", 1);
+ assertThat(branchLog.get(0).getParents()).hasLength(2);
+ assertThat(branchLog.get(0).getParent(1).name())
+ .isEqualTo(r2.getCommit().name());
+ }
+
+ @Test
+ public void mixingSubmitTypesOnOneBranchFails() throws Exception {
+ setRulesPl(SUBMIT_TYPE_FROM_SUBJECT);
+
+ PushOneCommit.Result r1 = createChange("master", "CHERRY_PICK 1");
+ PushOneCommit.Result r2 = createChange("master", "MERGE_IF_NECESSARY 2");
+
+ gApi.changes().id(r1.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(r2.getChangeId()).current().review(ReviewInput.approve());
+
+ try {
+ gApi.changes().id(r2.getChangeId()).current().submit();
+ fail("Expected ResourceConflictException");
+ } catch (ResourceConflictException e) {
+ assertThat(e).hasMessage("Merge Conflict");
+ Throwable t = e.getCause();
+ assertThat(t).isInstanceOf(IntegrationException.class);
+ assertThat(t.getMessage()).isEqualTo(
+ "Change " + r1.getChange().getId() + " has submit type CHERRY_PICK, "
+ + "but previously chose submit type MERGE_IF_NECESSARY from change "
+ + r2.getChange().getId() + " in the same batch");
+ }
+ }
+
+ private List<RevCommit> log(String commitish, int n) throws Exception {
+ try (Repository repo = repoManager.openRepository(project);
+ Git git = new Git(repo)) {
+ ObjectId id = repo.resolve(commitish);
+ assertThat(id).isNotNull();
+ return ImmutableList.copyOf(git.log().add(id).setMaxCount(n).call());
+ }
+ }
+
+ private void assertSubmitType(SubmitType expected, String id)
+ throws Exception {
+ assertThat(gApi.changes().id(id).current().submitType())
+ .isEqualTo(expected);
+ }
+}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 73a02a5..d104a41 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -83,11 +83,8 @@
}
private void saveProjectConfig(ProjectConfig cfg) throws Exception {
- MetaDataUpdate md = metaDataUpdateFactory.create(project);
- try {
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
cfg.commit(md);
- } finally {
- md.close();
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java
index f7e11fb..534afcb 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java
@@ -321,11 +321,8 @@
}
private void saveProjectConfig(ProjectConfig cfg) throws Exception {
- MetaDataUpdate md = metaDataUpdateFactory.create(project);
- try {
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
cfg.commit(md);
- } finally {
- md.close();
}
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
index 44c2ba4..6ff7889 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
@@ -14,10 +14,12 @@
package com.google.gerrit.extensions.api.changes;
+import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ActionInfo;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.FileInfo;
import com.google.gerrit.extensions.common.MergeableInfo;
+import com.google.gerrit.extensions.common.TestSubmitRuleInput;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.NotImplementedException;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -28,6 +30,7 @@
public interface RevisionApi {
void delete() throws RestApiException;
+
void review(ReviewInput in) throws RestApiException;
void submit() throws RestApiException;
@@ -65,6 +68,9 @@
Map<String, ActionInfo> actions() throws RestApiException;
+ SubmitType submitType() throws RestApiException;
+ SubmitType testSubmitType(TestSubmitRuleInput in) throws RestApiException;
+
/**
* A default implementation which allows source compatibility
* when adding new methods to the interface.
@@ -194,5 +200,16 @@
public Map<String, ActionInfo> actions() throws RestApiException {
throw new NotImplementedException();
}
+
+ @Override
+ public SubmitType submitType() throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
+ public SubmitType testSubmitType(TestSubmitRuleInput in)
+ throws RestApiException {
+ throw new NotImplementedException();
+ }
}
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/TestSubmitRuleInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/TestSubmitRuleInput.java
new file mode 100644
index 0000000..96a1626
--- /dev/null
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/TestSubmitRuleInput.java
@@ -0,0 +1,27 @@
+// Copyright (C) 2015 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.common;
+
+import com.google.gerrit.extensions.restapi.DefaultInput;
+
+public class TestSubmitRuleInput {
+ public enum Filters {
+ RUN, SKIP
+ }
+
+ @DefaultInput
+ public String rule;
+ public Filters filters;
+}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
index aee238d..ed2a4f9 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
@@ -100,8 +100,7 @@
// state, force a cache flush now.
//
ProjectConfig config;
- MetaDataUpdate md = metaDataUpdateFactory.create(projectName);
- try {
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(projectName)) {
config = ProjectConfig.read(md);
if (config.updateGroupNames(groupBackend)) {
@@ -115,8 +114,6 @@
projectCache.evict(config.getProject());
pc = open();
}
- } finally {
- md.close();
}
final RefControl metaConfigControl = pc.controlForRef(RefNames.REFS_CONFIG);
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
index ba4f012..f8d3e70 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
@@ -92,13 +92,7 @@
final ProjectControl projectControl =
projectControlFactory.controlFor(projectName);
- final MetaDataUpdate md;
- try {
- md = metaDataUpdateFactory.create(projectName);
- } catch (RepositoryNotFoundException notFound) {
- throw new NoSuchProjectException(projectName);
- }
- try {
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(projectName)) {
ProjectConfig config = ProjectConfig.read(md, base);
Set<String> toDelete = scanSectionNames(config);
@@ -163,8 +157,8 @@
return updateProjectConfig(projectControl, config, md,
parentProjectUpdate);
- } finally {
- md.close();
+ } catch (RepositoryNotFoundException notFound) {
+ throw new NoSuchProjectException(projectName);
}
}
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java
index bb69533bf..ac43363 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java
@@ -243,6 +243,9 @@
} else if (isGenAvailableNowForCurrentSearcher()) {
set(null);
return true;
+ } else if (!reopenThread.isAlive()) {
+ setException(new IllegalStateException("NRT thread is dead"));
+ return true;
}
return false;
}
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
index 2815099..5108315 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
@@ -333,6 +333,11 @@
modules.add(SchemaVersionCheck.module());
modules.add(new DropWizardMetricMaker.RestModule());
modules.add(new LogFileCompressor.Module());
+
+ // Index module shutdown must happen before work queue shutdown, otherwise
+ // work queue can get stuck waiting on index futures that will never return.
+ modules.add(createIndexModule());
+
modules.add(new WorkQueue.Module());
modules.add(new ChangeHookRunner.Module());
modules.add(new ReceiveCommitsExecutorModule());
@@ -351,7 +356,6 @@
modules.add(new PluginRestApiModule());
modules.add(new RestCacheAdminModule());
modules.add(new GpgModule(config));
- modules.add(createIndexModule());
if (MoreObjects.firstNonNull(httpd, true)) {
modules.add(new CanonicalWebUrlModule() {
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/access/ListAccess.java b/gerrit-server/src/main/java/com/google/gerrit/server/access/ListAccess.java
index 93a3814..04546aa 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/access/ListAccess.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/access/ListAccess.java
@@ -93,40 +93,32 @@
throws ResourceNotFoundException, ResourceConflictException, IOException {
Map<String, ProjectAccessInfo> access = Maps.newTreeMap();
for (String p: projects) {
+ // Load the current configuration from the repository, ensuring it's the most
+ // recent version available. If it differs from what was in the project
+ // state, force a cache flush now.
+ //
Project.NameKey projectName = new Project.NameKey(p);
- ProjectControl pc = open(projectName);
- ProjectConfig config;
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(projectName)) {
+ ProjectControl pc = open(projectName);
+ ProjectConfig config = ProjectConfig.read(md);
- try {
- // Load the current configuration from the repository, ensuring it's the most
- // recent version available. If it differs from what was in the project
- // state, force a cache flush now.
- //
- MetaDataUpdate md = metaDataUpdateFactory.create(projectName);
- try {
- config = ProjectConfig.read(md);
-
- if (config.updateGroupNames(groupBackend)) {
- md.setMessage("Update group names\n");
- config.commit(md);
- projectCache.evict(config.getProject());
- pc = open(projectName);
- } else if (config.getRevision() != null
- && !config.getRevision().equals(
- pc.getProjectState().getConfig().getRevision())) {
- projectCache.evict(config.getProject());
- pc = open(projectName);
- }
- } catch (ConfigInvalidException e) {
- throw new ResourceConflictException(e.getMessage());
- } finally {
- md.close();
+ if (config.updateGroupNames(groupBackend)) {
+ md.setMessage("Update group names\n");
+ config.commit(md);
+ projectCache.evict(config.getProject());
+ pc = open(projectName);
+ } else if (config.getRevision() != null
+ && !config.getRevision().equals(
+ pc.getProjectState().getConfig().getRevision())) {
+ projectCache.evict(config.getProject());
+ pc = open(projectName);
}
+ access.put(p, new ProjectAccessInfo(pc, config));
+ } catch (ConfigInvalidException e) {
+ throw new ResourceConflictException(e.getMessage());
} catch (RepositoryNotFoundException e) {
throw new ResourceNotFoundException(p);
}
-
- access.put(p, new ProjectAccessInfo(pc, config));
}
return access;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthRequest.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthRequest.java
index 35d7c70..c585f97 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthRequest.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthRequest.java
@@ -14,9 +14,9 @@
package com.google.gerrit.server.account;
+import static com.google.gerrit.reviewdb.client.AccountExternalId.SCHEME_EXTERNAL;
import static com.google.gerrit.reviewdb.client.AccountExternalId.SCHEME_GERRIT;
import static com.google.gerrit.reviewdb.client.AccountExternalId.SCHEME_MAILTO;
-import static com.google.gerrit.reviewdb.client.AccountExternalId.SCHEME_EXTERNAL;
import com.google.gerrit.reviewdb.client.AccountExternalId;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetDiffPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetDiffPreferences.java
index e14791c..9617d94 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetDiffPreferences.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetDiffPreferences.java
@@ -77,10 +77,8 @@
private DiffPreferencesInfo writeToGit(DiffPreferencesInfo in,
Account.Id userId) throws RepositoryNotFoundException, IOException,
ConfigInvalidException {
- MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName);
-
DiffPreferencesInfo out = new DiffPreferencesInfo();
- try {
+ try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
VersionedAccountPreferences prefs = VersionedAccountPreferences.forUser(
userId);
prefs.load(md);
@@ -90,8 +88,6 @@
prefs.commit(md);
loadSection(prefs.getConfig(), UserConfigSections.DIFF, null, out,
DiffPreferencesInfo.defaults(), null);
- } finally {
- md.close();
}
return out;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetEditPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetEditPreferences.java
index eabe31d..d51e24a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetEditPreferences.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetEditPreferences.java
@@ -71,18 +71,15 @@
}
Account.Id accountId = rsrc.getUser().getAccountId();
- MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName);
VersionedAccountPreferences prefs;
- try {
+ try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
prefs = VersionedAccountPreferences.forUser(accountId);
prefs.load(md);
storeSection(prefs.getConfig(), UserConfigSections.EDIT, null,
readFromGit(accountId, gitMgr, allUsersName, in),
EditPreferencesInfo.defaults());
prefs.commit(md);
- } finally {
- md.close();
}
return Response.none();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
index 7042076..51c54ac 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
@@ -115,9 +115,8 @@
Account.Id accountId = rsrc.getUser().getAccountId();
AccountGeneralPreferences p;
VersionedAccountPreferences versionedPrefs;
- MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName);
db.get().accounts().beginTransaction(accountId);
- try {
+ try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
Account a = db.get().accounts().get(accountId);
if (a == null) {
throw new ResourceNotFoundException();
@@ -184,7 +183,6 @@
return new GetPreferences.PreferenceInfo(
p, versionedPrefs, md.getRepository());
} finally {
- md.close();
db.get().rollback();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
index 0ca43f5..eb16ffb 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
@@ -27,10 +27,12 @@
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.api.changes.SubmitInput;
+import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ActionInfo;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.FileInfo;
import com.google.gerrit.extensions.common.MergeableInfo;
+import com.google.gerrit.extensions.common.TestSubmitRuleInput;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -54,6 +56,7 @@
import com.google.gerrit.server.change.Reviewed;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.Submit;
+import com.google.gerrit.server.change.TestSubmitType;
import com.google.gerrit.server.git.UpdateException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -94,6 +97,8 @@
private final Comments comments;
private final CommentApiImpl.Factory commentFactory;
private final GetRevisionActions revisionActions;
+ private final Provider<TestSubmitType> testSubmitType;
+ private final TestSubmitType.Get getSubmitType;
@Inject
RevisionApiImpl(Changes changes,
@@ -119,6 +124,8 @@
Comments comments,
CommentApiImpl.Factory commentFactory,
GetRevisionActions revisionActions,
+ Provider<TestSubmitType> testSubmitType,
+ TestSubmitType.Get getSubmitType,
@Assisted RevisionResource r) {
this.changes = changes;
this.cherryPick = cherryPick;
@@ -143,6 +150,8 @@
this.comments = comments;
this.commentFactory = commentFactory;
this.revisionActions = revisionActions;
+ this.testSubmitType = testSubmitType;
+ this.getSubmitType = getSubmitType;
this.revision = r;
}
@@ -375,4 +384,23 @@
public Map<String, ActionInfo> actions() throws RestApiException {
return revisionActions.apply(revision).value();
}
+
+ @Override
+ public SubmitType submitType() throws RestApiException {
+ try {
+ return getSubmitType.apply(revision);
+ } catch (OrmException e) {
+ throw new RestApiException("Cannot get submit type", e);
+ }
+ }
+
+ @Override
+ public SubmitType testSubmitType(TestSubmitRuleInput in)
+ throws RestApiException {
+ try {
+ return testSubmitType.get().apply(revision, in);
+ } catch (OrmException e) {
+ throw new RestApiException("Cannot test submit type", e);
+ }
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
index cf08a81..0d685df 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
@@ -112,9 +112,7 @@
psa = a;
a.setValue((short)0);
ctx.getChangeUpdate().setPatchSetId(psId);
- // TODO(dborowitz): Support modifying other users' labels in notedb
- // format.
- ctx.getChangeUpdate().removeApproval(label);
+ ctx.getChangeUpdate().removeApprovalFor(a.getAccountId(), label);
break;
}
} else {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitRule.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitRule.java
index 95a701e..3eecea3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitRule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitRule.java
@@ -19,13 +19,13 @@
import com.google.common.collect.Maps;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.common.TestSubmitRuleInput;
+import com.google.gerrit.extensions.common.TestSubmitRuleInput.Filters;
import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.extensions.restapi.DefaultInput;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.rules.RulesCache;
import com.google.gerrit.server.account.AccountLoader;
-import com.google.gerrit.server.change.TestSubmitRule.Input;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
@@ -37,17 +37,8 @@
import java.util.List;
import java.util.Map;
-public class TestSubmitRule implements RestModifyView<RevisionResource, Input> {
- public enum Filters {
- RUN, SKIP
- }
-
- public static class Input {
- @DefaultInput
- public String rule;
- public Filters filters;
- }
-
+public class TestSubmitRule
+ implements RestModifyView<RevisionResource, TestSubmitRuleInput> {
private final Provider<ReviewDb> db;
private final ChangeData.Factory changeDataFactory;
private final RulesCache rules;
@@ -68,10 +59,10 @@
}
@Override
- public List<Record> apply(RevisionResource rsrc, Input input)
+ public List<Record> apply(RevisionResource rsrc, TestSubmitRuleInput input)
throws AuthException, OrmException {
if (input == null) {
- input = new Input();
+ input = new TestSubmitRuleInput();
}
if (input.rule != null && !rules.isProjectRulesEnabled()) {
throw new AuthException("project rules are disabled");
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitType.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitType.java
index f6016b5..4855012 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitType.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitType.java
@@ -17,14 +17,14 @@
import com.google.common.base.MoreObjects;
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.extensions.client.SubmitType;
+import com.google.gerrit.extensions.common.TestSubmitRuleInput;
+import com.google.gerrit.extensions.common.TestSubmitRuleInput.Filters;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.rules.RulesCache;
-import com.google.gerrit.server.change.TestSubmitRule.Filters;
-import com.google.gerrit.server.change.TestSubmitRule.Input;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
@@ -33,7 +33,8 @@
import org.kohsuke.args4j.Option;
-public class TestSubmitType implements RestModifyView<RevisionResource, Input> {
+public class TestSubmitType
+ implements RestModifyView<RevisionResource, TestSubmitRuleInput> {
private final Provider<ReviewDb> db;
private final ChangeData.Factory changeDataFactory;
private final RulesCache rules;
@@ -51,10 +52,10 @@
}
@Override
- public SubmitType apply(RevisionResource rsrc, Input input)
+ public SubmitType apply(RevisionResource rsrc, TestSubmitRuleInput input)
throws AuthException, BadRequestException, OrmException {
if (input == null) {
- input = new Input();
+ input = new TestSubmitRuleInput();
}
if (input.rule != null && !rules.isProjectRulesEnabled()) {
throw new AuthException("project rules are disabled");
@@ -71,13 +72,13 @@
if (rec.status != SubmitTypeRecord.Status.OK) {
throw new BadRequestException(String.format(
"rule %s produced invalid result: %s",
- evaluator.getSubmitRule(), rec));
+ evaluator.getSubmitRuleName(), rec));
}
return rec.type;
}
- static class Get implements RestReadView<RevisionResource> {
+ public static class Get implements RestReadView<RevisionResource> {
private final TestSubmitType test;
@Inject
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java
index d6693ae..07d22b3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java
@@ -60,15 +60,12 @@
}
VersionedAccountPreferences p;
- MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName);
- try {
+ try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
p = VersionedAccountPreferences.forDefault();
p.load(md);
com.google.gerrit.server.account.SetPreferences.storeMyMenus(p, i.my);
p.commit(md);
return new PreferenceInfo(null, p, md.getRepository());
- } finally {
- md.close();
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java
index 66417fb..da64332 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java
@@ -48,9 +48,6 @@
REVISION_GONE(""),
/** */
- NO_SUBMIT_TYPE(""),
-
- /** */
MANUAL_RECURSIVE_MERGE("The change requires a local merge to resolve.\n"
+ "\n"
+ "Please merge (or rebase) the change locally and upload the resolution for review."),
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
index 61689a1..7857fb9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
@@ -16,15 +16,14 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
-import static org.eclipse.jgit.lib.RefDatabase.ALL;
+import com.google.auto.value.AutoValue;
import com.google.common.base.Optional;
import com.google.common.base.Predicate;
-import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
-import com.google.common.collect.ListMultimap;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.common.collect.Table;
@@ -392,8 +391,7 @@
throws IntegrationException, NoSuchChangeException,
ResourceConflictException {
logDebug("Beginning merge attempt on {}", cs);
- Map<Branch.NameKey, ListMultimap<SubmitType, ChangeData>> toSubmit =
- new HashMap<>();
+ Map<Branch.NameKey, BranchBatch> toSubmit = new HashMap<>();
logDebug("Perform the merges");
try {
Multimap<Project.NameKey, Branch.NameKey> br = cs.branchesByProject();
@@ -402,22 +400,15 @@
openRepository(project);
for (Branch.NameKey branch : br.get(project)) {
setDestProject(branch);
-
- ListMultimap<SubmitType, ChangeData> submitting =
- validateChangeList(cbb.get(branch));
+ BranchBatch submitting = validateChangeList(cbb.get(branch));
toSubmit.put(branch, submitting);
- Set<SubmitType> submitTypes = new HashSet<>(submitting.keySet());
- for (SubmitType submitType : submitTypes) {
- SubmitStrategy strategy = createStrategy(branch, submitType,
- getBranchTip(branch), caller);
-
- MergeTip mergeTip = preMerge(strategy, submitting.get(submitType),
- getBranchTip(branch));
- mergeTips.put(branch, mergeTip);
- updateChangeStatus(submitting.get(submitType), branch,
- true, caller);
- }
+ SubmitStrategy strategy = createStrategy(
+ branch, submitting.submitType(), getBranchTip(branch), caller);
+ MergeTip mergeTip = preMerge(strategy, submitting.changes(),
+ getBranchTip(branch));
+ mergeTips.put(branch, mergeTip);
+ updateChangeStatus(submitting.changes(), branch, true, caller);
inserter.flush();
}
closeRepository();
@@ -431,12 +422,9 @@
pendingRefUpdates.remove(branch);
setDestProject(branch);
- ListMultimap<SubmitType, ChangeData> submitting = toSubmit.get(branch);
- for (SubmitType submitType : submitting.keySet()) {
- updateChangeStatus(submitting.get(submitType), branch,
- false, caller);
- updateSubmoduleSubscriptions(subOp, branch, getBranchTip(branch));
- }
+ BranchBatch submitting = toSubmit.get(branch);
+ updateChangeStatus(submitting.changes(), branch, false, caller);
+ updateSubmoduleSubscriptions(subOp, branch, getBranchTip(branch));
if (update != null) {
fireRefUpdated(branch, update);
}
@@ -583,30 +571,26 @@
return alreadyAccepted;
}
- private ListMultimap<SubmitType, ChangeData> validateChangeList(
+ @AutoValue
+ static abstract class BranchBatch {
+ abstract SubmitType submitType();
+ abstract List<ChangeData> changes();
+ }
+
+ private BranchBatch validateChangeList(
Collection<ChangeData> submitted) throws IntegrationException {
logDebug("Validating {} changes", submitted.size());
- ListMultimap<SubmitType, ChangeData> toSubmit = ArrayListMultimap.create();
+ List<ChangeData> toSubmit = new ArrayList<>(submitted.size());
+ Multimap<ObjectId, PatchSet.Id> revisions = getRevisions(submitted);
- Map<String, Ref> allRefs;
- try {
- allRefs = repo.getRefDatabase().getRefs(ALL);
- } catch (IOException e) {
- throw new IntegrationException(e.getMessage(), e);
- }
-
- Set<ObjectId> tips = new HashSet<>();
- for (Ref r : allRefs.values()) {
- tips.add(r.getObjectId());
- }
-
+ SubmitType submitType = null;
+ ChangeData choseSubmitTypeFrom = null;
for (ChangeData cd : submitted) {
ChangeControl ctl;
Change chg;
try {
ctl = cd.changeControl();
- // Reload change in case index was stale.
- chg = cd.reloadChange();
+ chg = cd.change();
} catch (OrmException e) {
throw new IntegrationException("Failed to validate changes", e);
}
@@ -645,18 +629,13 @@
continue;
}
- if (!tips.contains(id)) {
- // TODO Technically the proper way to do this test is to use a
- // RevWalk on "$id --not --all" and test for an empty set. But
- // that is way slower than looking for a ref directly pointing
- // at the desired tip. We should always have a ref available.
- //
+ if (!revisions.containsEntry(id, ps.getId())) {
// TODO this is actually an error, the branch is gone but we
// want to merge the issue. We can't safely do that if the
// tip is not reachable.
//
logError("Revision " + idstr + " of patch set " + ps.getId()
- + " is not contained in any ref");
+ + " does not match " + ps.getId().toRefName());
commits.put(changeId, CodeReviewCommit.revisionGone(ctl));
continue;
}
@@ -686,20 +665,52 @@
continue;
}
- SubmitType submitType;
- submitType = getSubmitType(commit.getControl(), ps);
- if (submitType == null) {
+ SubmitType st = getSubmitType(commit.getControl(), ps);
+ if (st == null) {
logError("No submit type for revision " + idstr + " of patch set "
+ ps.getId());
- commit.setStatusCode(CommitMergeStatus.NO_SUBMIT_TYPE);
- continue;
+ throw new IntegrationException(
+ "No submit type for change " + cd.getId());
}
-
+ if (submitType == null) {
+ submitType = st;
+ choseSubmitTypeFrom = cd;
+ } else if (st != submitType) {
+ throw new IntegrationException(String.format(
+ "Change %s has submit type %s, but previously chose submit type %s "
+ + "from change %s in the same batch",
+ cd.getId(), st, submitType, choseSubmitTypeFrom.getId()));
+ }
commit.add(canMergeFlag);
- toSubmit.put(submitType, cd);
+ toSubmit.add(cd);
}
logDebug("Submitting on this run: {}", toSubmit);
- return toSubmit;
+ return new AutoValue_MergeOp_BranchBatch(submitType, toSubmit);
+ }
+
+ private Multimap<ObjectId, PatchSet.Id> getRevisions(
+ Collection<ChangeData> cds) throws IntegrationException {
+ try {
+ List<String> refNames = new ArrayList<>(cds.size());
+ for (ChangeData cd : cds) {
+ // Reload change in case index was stale.
+ cd.reloadChange();
+ Change c = cd.change();
+ if (c != null) {
+ refNames.add(c.currentPatchSetId().toRefName());
+ }
+ }
+ Multimap<ObjectId, PatchSet.Id> revisions =
+ HashMultimap.create(cds.size(), 1);
+ for (Map.Entry<String, Ref> e : repo.getRefDatabase().exactRef(
+ refNames.toArray(new String[refNames.size()])).entrySet()) {
+ revisions.put(
+ e.getValue().getObjectId(), PatchSet.Id.fromRef(e.getKey()));
+ }
+ return revisions;
+ } catch (IOException | OrmException e) {
+ throw new IntegrationException("Failed to validate changes", e);
+ }
}
private SubmitType getSubmitType(ChangeControl ctl, PatchSet ps) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java
index 840b167..540d479 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java
@@ -34,7 +34,7 @@
import java.io.IOException;
/** Helps with the updating of a {@link VersionedMetaData}. */
-public class MetaDataUpdate {
+public class MetaDataUpdate implements AutoCloseable {
public static class User {
private final InternalFactory factory;
private final GitRepositoryManager mgr;
@@ -89,6 +89,33 @@
* multiple commits to a single metadata ref, see
* {@link VersionedMetaData#openUpdate(MetaDataUpdate)}.
*
+ * Important: Create a new MetaDataUpdate instance for each update:
+ * <pre>
+ * <code>
+ * try (Repository repo = repoMgr.openRepository(allUsersName);
+ * RevWalk rw = new RevWalk(repo) {
+ * BatchRefUpdate batchUpdate = repo.getRefDatabase().newBatchUpdate();
+ * // WRONG: create the MetaDataUpdate instance here and reuse it for
+ * // all updates in the loop
+ * for{@code (Map.Entry<Account.Id, DiffPreferencesInfo> e : diffPrefsFromDb)} {
+ * // CORRECT: create a new MetaDataUpdate instance for each update
+ * try (MetaDataUpdate md =
+ * metaDataUpdateFactory.create(allUsersName, batchUpdate)) {
+ * md.setMessage("Import diff preferences from reviewdb\n");
+ * VersionedAccountPreferences vPrefs =
+ * VersionedAccountPreferences.forUser(e.getKey());
+ * storeSection(vPrefs.getConfig(), UserConfigSections.DIFF, null,
+ * e.getValue(), DiffPreferencesInfo.defaults());
+ * vPrefs.commit(md);
+ * } catch (ConfigInvalidException e) {
+ * // TODO handle exception
+ * }
+ * }
+ * batchUpdate.execute(rw, NullProgressMonitor.INSTANCE);
+ * }
+ * </code>
+ * </pre>
+ *
* @param name project name.
* @param repository GIT respository
* @param user user for the update.
@@ -191,6 +218,7 @@
}
/** Close the cached Repository handle. */
+ @Override
public void close() {
getRepository().close();
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/RenameGroupOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/RenameGroupOp.java
index 06314db..00c9a7c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/RenameGroupOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/RenameGroupOp.java
@@ -83,13 +83,8 @@
continue;
}
- try {
- MetaDataUpdate md = metaDataUpdateFactory.create(projectName);
- try {
- rename(md);
- } finally {
- md.close();
- }
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(projectName)) {
+ rename(md);
} catch (RepositoryNotFoundException noProject) {
continue;
} catch (ConfigInvalidException | IOException err) {
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 9d759ed..3bca5d5 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
@@ -27,6 +27,7 @@
import com.google.common.base.Splitter;
import com.google.common.base.Supplier;
import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableSet;
@@ -38,6 +39,7 @@
import com.google.common.collect.Table;
import com.google.common.collect.Tables;
import com.google.common.primitives.Ints;
+import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
@@ -85,8 +87,9 @@
private final ObjectId tip;
private final RevWalk walk;
private final Repository repo;
- private final Map<PatchSet.Id,
- Table<Account.Id, String, Optional<PatchSetApproval>>> approvals;
+ private final Map<PatchSet.Id, Table<Account.Id, String, PatchSetApproval>>
+ approvals;
+ private final Map<PatchSet.Id, Multimap<Account.Id, String>> removedApprovals;
private final List<ChangeMessage> allChangeMessages;
private final Multimap<PatchSet.Id, ChangeMessage> changeMessagesByPatchSet;
@@ -99,6 +102,7 @@
this.repo =
repoManager.openMetadataRepository(ChangeNotes.getProjectName(change));
approvals = Maps.newHashMap();
+ removedApprovals = Maps.newHashMap();
reviewers = Maps.newLinkedHashMap();
allPastReviewers = Lists.newArrayList();
submitRecords = Lists.newArrayListWithExpectedSize(1);
@@ -126,9 +130,8 @@
buildApprovals() {
Multimap<PatchSet.Id, PatchSetApproval> result =
ArrayListMultimap.create(approvals.keySet().size(), 3);
- for (Table<?, ?, Optional<PatchSetApproval>> curr
- : approvals.values()) {
- for (PatchSetApproval psa : Optional.presentInstances(curr.values())) {
+ for (Table<?, ?, PatchSetApproval> curr : approvals.values()) {
+ for (PatchSetApproval psa : curr.values()) {
result.put(psa.getPatchSetId(), psa);
}
}
@@ -305,46 +308,99 @@
private void parseApproval(PatchSet.Id psId, Account.Id accountId,
RevCommit commit, String line) throws ConfigInvalidException {
- Table<Account.Id, String, Optional<PatchSetApproval>> curr =
- approvals.get(psId);
- if (curr == null) {
- curr = Tables.newCustomTable(
- Maps.<Account.Id, Map<String, Optional<PatchSetApproval>>>
+ if (line.startsWith("-")) {
+ parseRemoveApproval(psId, accountId, line);
+ } else {
+ parseAddApproval(psId, accountId, commit, line);
+ }
+ }
+
+ private void parseAddApproval(PatchSet.Id psId, Account.Id accountId,
+ RevCommit commit, String line) throws ConfigInvalidException {
+ LabelVote l;
+ try {
+ l = LabelVote.parseWithEquals(line);
+ } catch (IllegalArgumentException e) {
+ ConfigInvalidException pe =
+ parseException("invalid %s: %s", FOOTER_LABEL, line);
+ pe.initCause(e);
+ throw pe;
+ }
+ if (isApprovalRemoved(psId, accountId, l.label())) {
+ return;
+ }
+
+ Table<Account.Id, String, PatchSetApproval> curr = approvals.get(psId);
+ if (curr != null) {
+ if (curr.contains(accountId, l.label())) {
+ return;
+ }
+ } else {
+ curr = newApprovalsTable();
+ approvals.put(psId, curr);
+ }
+ curr.put(accountId, l.label(), new PatchSetApproval(
+ new PatchSetApproval.Key(
+ psId,
+ accountId,
+ new LabelId(l.label())),
+ l.value(),
+ new Timestamp(commit.getCommitterIdent().getWhen().getTime())));
+ }
+
+ private static Table<Account.Id, String, PatchSetApproval>
+ newApprovalsTable() {
+ return Tables.newCustomTable(
+ Maps.<Account.Id, Map<String, PatchSetApproval>>
newHashMapWithExpectedSize(2),
- new Supplier<Map<String, Optional<PatchSetApproval>>>() {
+ new Supplier<Map<String, PatchSetApproval>>() {
@Override
- public Map<String, Optional<PatchSetApproval>> get() {
+ public Map<String, PatchSetApproval> get() {
return Maps.newLinkedHashMap();
}
});
- approvals.put(psId, curr);
+ }
+
+ private void parseRemoveApproval(PatchSet.Id psId, Account.Id accountId,
+ String line) throws ConfigInvalidException {
+ Multimap<Account.Id, String> curr = removedApprovals.get(psId);
+ if (curr == null) {
+ curr = HashMultimap.create(1, 1);
+ removedApprovals.put(psId, curr);
+ }
+ String label;
+ Account.Id removedAccountId;
+ line = line.substring(1);
+ int s = line.indexOf(' ');
+ if (s > 0) {
+ label = line.substring(0, s);
+ PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1));
+ checkFooter(ident != null, FOOTER_LABEL, line);
+ removedAccountId = parseIdent(ident);
+ } else {
+ label = line;
+ removedAccountId = accountId;
}
- if (line.startsWith("-")) {
- String label = line.substring(1);
- if (!curr.contains(accountId, label)) {
- curr.put(accountId, label, Optional.<PatchSetApproval> absent());
- }
- } else {
- LabelVote l;
- try {
- l = LabelVote.parseWithEquals(line);
- } catch (IllegalArgumentException e) {
- ConfigInvalidException pe =
- parseException("invalid %s: %s", FOOTER_LABEL, line);
- pe.initCause(e);
- throw pe;
- }
- if (!curr.contains(accountId, l.label())) {
- curr.put(accountId, l.label(), Optional.of(new PatchSetApproval(
- new PatchSetApproval.Key(
- psId,
- accountId,
- new LabelId(l.label())),
- l.value(),
- new Timestamp(commit.getCommitterIdent().getWhen().getTime()))));
- }
+ Table<Account.Id, String, PatchSetApproval> added = approvals.get(psId);
+ if (added != null && added.contains(accountId, label)) {
+ return;
}
+
+ try {
+ curr.put(removedAccountId, LabelType.checkNameInternal(label));
+ } catch (IllegalArgumentException e) {
+ ConfigInvalidException pe =
+ parseException("invalid %s: %s", FOOTER_LABEL, line);
+ pe.initCause(e);
+ throw pe;
+ }
+ }
+
+ private boolean isApprovalRemoved(PatchSet.Id psId, Account.Id accountId,
+ String label) {
+ Multimap<Account.Id, String> curr = removedApprovals.get(psId);
+ return curr != null && curr.containsEntry(accountId, label);
}
private void parseSubmitRecords(List<String> lines)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 7c011b6..f1843bd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -25,11 +25,12 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
-import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.MultimapBuilder;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
@@ -87,8 +88,9 @@
}
private final AccountCache accountCache;
- private final Map<String, Optional<Short>> approvals;
+ private final Map<String, Short> approvals;
private final Map<Account.Id, ReviewerStateInternal> reviewers;
+ private final Multimap<Account.Id, String> removedApprovals;
private Change.Status status;
private String subject;
private List<SubmitRecord> submitRecords;
@@ -163,6 +165,8 @@
this.commentsUtil = commentsUtil;
this.approvals = Maps.newTreeMap(labelNameComparator);
this.reviewers = Maps.newLinkedHashMap();
+ this.removedApprovals =
+ MultimapBuilder.linkedHashKeys(1).arrayListValues(1).build();
this.comments = Lists.newArrayList();
}
@@ -177,11 +181,15 @@
}
public void putApproval(String label, short value) {
- approvals.put(label, Optional.of(value));
+ approvals.put(label, value);
}
public void removeApproval(String label) {
- approvals.put(label, Optional.<Short> absent());
+ removeApprovalFor(getUser().getAccountId(), label);
+ }
+
+ public void removeApprovalFor(Account.Id reviewer, String label) {
+ removedApprovals.put(reviewer, label);
}
public void merge(Iterable<SubmitRecord> submitRecords) {
@@ -434,20 +442,22 @@
}
for (Map.Entry<Account.Id, ReviewerStateInternal> e : reviewers.entrySet()) {
- Account account = accountCache.get(e.getKey()).getAccount();
- PersonIdent ident = newIdent(account, when);
- addFooter(msg, e.getValue().getFooterKey())
- .append(ident.getName())
- .append(" <").append(ident.getEmailAddress()).append(">\n");
+ addFooter(msg, e.getValue().getFooterKey());
+ addIdent(msg, e.getKey()).append('\n');
}
- for (Map.Entry<String, Optional<Short>> e : approvals.entrySet()) {
- if (!e.getValue().isPresent()) {
- addFooter(msg, FOOTER_LABEL, '-', e.getKey());
- } else {
- addFooter(msg, FOOTER_LABEL, LabelVote.create(
- e.getKey(), e.getValue().get()).formatWithEquals());
+ for (Map.Entry<String, Short> e : approvals.entrySet()) {
+ addFooter(msg, FOOTER_LABEL, LabelVote.create(
+ e.getKey(), e.getValue()).formatWithEquals());
+ }
+
+ for (Map.Entry<Account.Id, String> e : removedApprovals.entries()) {
+ addFooter(msg, FOOTER_LABEL).append('-').append(e.getValue());
+ Account.Id id = e.getKey();
+ if (!id.equals(ctl.getUser().getAccountId())) {
+ addIdent(msg.append(' '), id);
}
+ msg.append('\n');
}
if (submitRecords != null) {
@@ -486,6 +496,7 @@
private boolean isEmpty() {
return approvals.isEmpty()
+ && removedApprovals.isEmpty()
&& changeMessage == null
&& comments.isEmpty()
&& reviewers.isEmpty()
@@ -509,6 +520,14 @@
sb.append('\n');
}
+ private StringBuilder addIdent(StringBuilder sb, Account.Id accountId) {
+ Account account = accountCache.get(accountId).getAccount();
+ PersonIdent ident = newIdent(account, when);
+ sb.append(ident.getName()).append(" <").append(ident.getEmailAddress())
+ .append('>');
+ return sb;
+ }
+
private static String sanitizeFooter(String value) {
return value.replace('\n', ' ').replace('\0', ' ');
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java
index c4b686c..c460fd0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java
@@ -284,9 +284,7 @@
}
private void createProjectConfig(CreateProjectArgs args) throws IOException, ConfigInvalidException {
- MetaDataUpdate md =
- metaDataUpdateFactory.create(args.getProject());
- try {
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(args.getProject())) {
ProjectConfig config = ProjectConfig.read(md);
config.load(md);
@@ -321,8 +319,6 @@
md.setMessage("Created project\n");
config.commit(md);
- } finally {
- md.close();
}
projectCache.onCreateProject(args.getProject());
repoManager.setProjectDescription(args.getProject(),
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java
index 5f921b1..fd7539f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java
@@ -139,15 +139,7 @@
throw new BadRequestException("config is required");
}
- final MetaDataUpdate md;
- try {
- md = metaDataUpdateFactory.get().create(projectName);
- } catch (RepositoryNotFoundException notFound) {
- throw new ResourceNotFoundException(projectName.get());
- } catch (IOException e) {
- throw new ResourceNotFoundException(projectName.get(), e);
- }
- try {
+ try (MetaDataUpdate md = metaDataUpdateFactory.get().create(projectName)) {
ProjectConfig projectConfig = ProjectConfig.read(md);
Project p = projectConfig.getProject();
@@ -226,12 +218,12 @@
return new ConfigInfo(serverEnableSignedPush,
state.controlFor(user.get()), config, pluginConfigEntries,
cfgFactory, allProjects, views);
+ } catch (RepositoryNotFoundException notFound) {
+ throw new ResourceNotFoundException(projectName.get());
} catch (ConfigInvalidException err) {
throw new ResourceConflictException("Cannot read project " + projectName, err);
} catch (IOException err) {
throw new ResourceConflictException("Cannot update project " + projectName, err);
- } finally {
- md.close();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/PutDescription.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/PutDescription.java
index 2d407bd..7cf426d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/PutDescription.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/PutDescription.java
@@ -76,42 +76,37 @@
throw new AuthException("not project owner");
}
- try {
- MetaDataUpdate md = updateFactory.create(resource.getNameKey());
- try {
- ProjectConfig config = ProjectConfig.read(md);
- Project project = config.getProject();
- project.setDescription(Strings.emptyToNull(input.description));
+ try (MetaDataUpdate md = updateFactory.create(resource.getNameKey())) {
+ ProjectConfig config = ProjectConfig.read(md);
+ Project project = config.getProject();
+ project.setDescription(Strings.emptyToNull(input.description));
- String msg = MoreObjects.firstNonNull(
- Strings.emptyToNull(input.commitMessage),
- "Updated description.\n");
- if (!msg.endsWith("\n")) {
- msg += "\n";
- }
- md.setAuthor(user);
- md.setMessage(msg);
- ObjectId baseRev = config.getRevision();
- ObjectId commitRev = config.commit(md);
- // Only fire hook if project was actually changed.
- if (!Objects.equals(baseRev, commitRev)) {
- gitRefUpdated.fire(resource.getNameKey(), RefNames.REFS_CONFIG,
- baseRev, commitRev);
- hooks.doRefUpdatedHook(
- new Branch.NameKey(resource.getNameKey(), RefNames.REFS_CONFIG),
- baseRev, commitRev, user.getAccount());
- }
- cache.evict(ctl.getProject());
- gitMgr.setProjectDescription(
- resource.getNameKey(),
- project.getDescription());
-
- return Strings.isNullOrEmpty(project.getDescription())
- ? Response.<String>none()
- : Response.ok(project.getDescription());
- } finally {
- md.close();
+ String msg = MoreObjects.firstNonNull(
+ Strings.emptyToNull(input.commitMessage),
+ "Updated description.\n");
+ if (!msg.endsWith("\n")) {
+ msg += "\n";
}
+ md.setAuthor(user);
+ md.setMessage(msg);
+ ObjectId baseRev = config.getRevision();
+ ObjectId commitRev = config.commit(md);
+ // Only fire hook if project was actually changed.
+ if (!Objects.equals(baseRev, commitRev)) {
+ gitRefUpdated.fire(resource.getNameKey(), RefNames.REFS_CONFIG,
+ baseRev, commitRev);
+ hooks.doRefUpdatedHook(
+ new Branch.NameKey(resource.getNameKey(), RefNames.REFS_CONFIG),
+ baseRev, commitRev, user.getAccount());
+ }
+ cache.evict(ctl.getProject());
+ gitMgr.setProjectDescription(
+ resource.getNameKey(),
+ project.getDescription());
+
+ return Strings.isNullOrEmpty(project.getDescription())
+ ? Response.<String>none()
+ : Response.ok(project.getDescription());
} catch (RepositoryNotFoundException notFound) {
throw new ResourceNotFoundException(resource.getName());
} catch (ConfigInvalidException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java
index ac01de5..641c3a7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetDefaultDashboard.java
@@ -84,39 +84,34 @@
}
}
- try {
- MetaDataUpdate md = updateFactory.create(ctl.getProject().getNameKey());
- try {
- ProjectConfig config = ProjectConfig.read(md);
- Project project = config.getProject();
- if (inherited) {
- project.setDefaultDashboard(input.id);
- } else {
- project.setLocalDefaultDashboard(input.id);
- }
-
- String msg = MoreObjects.firstNonNull(
- Strings.emptyToNull(input.commitMessage),
- input.id == null
- ? "Removed default dashboard.\n"
- : String.format("Changed default dashboard to %s.\n", input.id));
- if (!msg.endsWith("\n")) {
- msg += "\n";
- }
- md.setAuthor(ctl.getUser().asIdentifiedUser());
- md.setMessage(msg);
- config.commit(md);
- cache.evict(ctl.getProject());
-
- if (target != null) {
- DashboardInfo info = get.get().apply(target);
- info.isDefault = true;
- return Response.ok(info);
- }
- return Response.none();
- } finally {
- md.close();
+ try (MetaDataUpdate md = updateFactory.create(ctl.getProject().getNameKey())) {
+ ProjectConfig config = ProjectConfig.read(md);
+ Project project = config.getProject();
+ if (inherited) {
+ project.setDefaultDashboard(input.id);
+ } else {
+ project.setLocalDefaultDashboard(input.id);
}
+
+ String msg = MoreObjects.firstNonNull(
+ Strings.emptyToNull(input.commitMessage),
+ input.id == null
+ ? "Removed default dashboard.\n"
+ : String.format("Changed default dashboard to %s.\n", input.id));
+ if (!msg.endsWith("\n")) {
+ msg += "\n";
+ }
+ md.setAuthor(ctl.getUser().asIdentifiedUser());
+ md.setMessage(msg);
+ config.commit(md);
+ cache.evict(ctl.getProject());
+
+ if (target != null) {
+ DashboardInfo info = get.get().apply(target);
+ info.isDefault = true;
+ return Response.ok(info);
+ }
+ return Response.none();
} catch (RepositoryNotFoundException notFound) {
throw new ResourceNotFoundException(ctl.getProject().getName());
} catch (ConfigInvalidException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetParent.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetParent.java
index 15c977f..8113c01 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetParent.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetParent.java
@@ -71,32 +71,27 @@
ResourceNotFoundException, UnprocessableEntityException, IOException {
ProjectControl ctl = rsrc.getControl();
validateParentUpdate(ctl, input.parent, checkIfAdmin);
- try {
- MetaDataUpdate md = updateFactory.create(rsrc.getNameKey());
- try {
- ProjectConfig config = ProjectConfig.read(md);
- Project project = config.getProject();
- project.setParentName(Strings.emptyToNull(input.parent));
+ try (MetaDataUpdate md = updateFactory.create(rsrc.getNameKey())) {
+ ProjectConfig config = ProjectConfig.read(md);
+ Project project = config.getProject();
+ project.setParentName(Strings.emptyToNull(input.parent));
- String msg = Strings.emptyToNull(input.commitMessage);
- if (msg == null) {
- msg = String.format(
- "Changed parent to %s.\n",
- MoreObjects.firstNonNull(project.getParentName(),
- allProjects.get()));
- } else if (!msg.endsWith("\n")) {
- msg += "\n";
- }
- md.setAuthor(ctl.getUser().asIdentifiedUser());
- md.setMessage(msg);
- config.commit(md);
- cache.evict(ctl.getProject());
-
- Project.NameKey parentName = project.getParent(allProjects);
- return parentName != null ? parentName.get() : "";
- } finally {
- md.close();
+ String msg = Strings.emptyToNull(input.commitMessage);
+ if (msg == null) {
+ msg = String.format(
+ "Changed parent to %s.\n",
+ MoreObjects.firstNonNull(project.getParentName(),
+ allProjects.get()));
+ } else if (!msg.endsWith("\n")) {
+ msg += "\n";
}
+ md.setAuthor(ctl.getUser().asIdentifiedUser());
+ md.setMessage(msg);
+ config.commit(md);
+ cache.evict(ctl.getProject());
+
+ Project.NameKey parentName = project.getParent(allProjects);
+ return parentName != null ? parentName.get() : "";
} catch (RepositoryNotFoundException notFound) {
throw new ResourceNotFoundException(rsrc.getName());
} catch (ConfigInvalidException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
index 2b35cef..e827d3b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
@@ -231,7 +231,8 @@
// required for this change to be submittable. Each label will indicate
// whether or not that is actually possible given the permissions.
return ruleError(String.format("Submit rule '%s' for change %s of %s has "
- + "no solution.", getSubmitRule(), cd.getId(), getProjectName()));
+ + "no solution.", getSubmitRuleName(), cd.getId(),
+ getProjectName()));
}
return resultsToSubmitRecord(getSubmitRule(), results);
@@ -410,13 +411,13 @@
if (results.isEmpty()) {
// Should never occur for a well written rule
- return typeError("Submit rule '" + getSubmitRule() + "' for change "
+ return typeError("Submit rule '" + getSubmitRuleName() + "' for change "
+ cd.getId() + " of " + getProjectName() + " has no solution.");
}
Term typeTerm = results.get(0);
if (!(typeTerm instanceof SymbolTerm)) {
- return typeError("Submit rule '" + getSubmitRule() + "' for change "
+ return typeError("Submit rule '" + getSubmitRuleName() + "' for change "
+ cd.getId() + " of " + getProjectName()
+ " did not return a symbol.");
}
@@ -609,6 +610,10 @@
return submitRule;
}
+ public String getSubmitRuleName() {
+ return submitRule != null ? submitRule.toString() : "<unknown rule>";
+ }
+
private void initPatchSet() throws OrmException {
if (patchSet == null) {
patchSet = cd.currentPatchSet();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_115.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_115.java
index 920160b..30479b7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_115.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_115.java
@@ -131,18 +131,18 @@
try (Repository git = mgr.openRepository(allUsersName);
RevWalk rw = new RevWalk(git)) {
BatchRefUpdate bru = git.getRefDatabase().newBatchUpdate();
- MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED,
- allUsersName, git, bru);
- md.getCommitBuilder().setAuthor(serverUser);
- md.getCommitBuilder().setCommitter(serverUser);
-
for (Map.Entry<Account.Id, DiffPreferencesInfo> e : imports.entrySet()) {
- VersionedAccountPreferences p =
- VersionedAccountPreferences.forUser(e.getKey());
- p.load(md);
- storeSection(p.getConfig(), UserConfigSections.DIFF, null,
- e.getValue(), DiffPreferencesInfo.defaults());
- p.commit(md);
+ try(MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED,
+ allUsersName, git, bru)) {
+ md.getCommitBuilder().setAuthor(serverUser);
+ md.getCommitBuilder().setCommitter(serverUser);
+ VersionedAccountPreferences p =
+ VersionedAccountPreferences.forUser(e.getKey());
+ p.load(md);
+ storeSection(p.getConfig(), UserConfigSections.DIFF, null,
+ e.getValue(), DiffPreferencesInfo.defaults());
+ p.commit(md);
+ }
}
bru.execute(rw, NullProgressMonitor.INSTANCE);
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index 0f3fee4..5af78461 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -112,6 +112,11 @@
+ "Label: Label2=1\n"
+ "Label: Label3=0\n"
+ "Label: Label4=-1\n");
+ assertParseSucceeds("Update change\n"
+ + "\n"
+ + "Patch-Set: 1\n"
+ + "Label: -Label1\n"
+ + "Label: -Label1 Other Account <2@gerrit>\n");
assertParseFails("Update change\n"
+ "\n"
+ "Patch-Set: 1\n"
@@ -124,6 +129,18 @@
+ "\n"
+ "Patch-Set: 1\n"
+ "Label: X+Y\n");
+ assertParseFails("Update change\n"
+ + "\n"
+ + "Patch-Set: 1\n"
+ + "Label: Label1 Other Account <2@gerrit>\n");
+ assertParseFails("Update change\n"
+ + "\n"
+ + "Patch-Set: 1\n"
+ + "Label: -Label!1\n");
+ assertParseFails("Update change\n"
+ + "\n"
+ + "Patch-Set: 1\n"
+ + "Label: -Label!1 Other Account <2@gerrit>\n");
}
@Test
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 c68c283..bfb16c1 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
@@ -197,6 +197,41 @@
}
@Test
+ public void removeOtherUsersApprovals() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, otherUser);
+ update.putApproval("Not-For-Long", (short) 1);
+ update.commit();
+
+ ChangeNotes notes = newNotes(c);
+ PatchSetApproval psa = Iterables.getOnlyElement(
+ notes.getApprovals().get(c.currentPatchSetId()));
+ assertThat(psa.getAccountId()).isEqualTo(otherUserId);
+ assertThat(psa.getLabel()).isEqualTo("Not-For-Long");
+ assertThat(psa.getValue()).isEqualTo((short) 1);
+
+ update = newUpdate(c, changeOwner);
+ update.removeApprovalFor(otherUserId, "Not-For-Long");
+ update.commit();
+
+ notes = newNotes(c);
+ assertThat(notes.getApprovals()).isEmpty();
+
+ // Add back approval on same label.
+ update = newUpdate(c, otherUser);
+ update.putApproval("Not-For-Long", (short) 2);
+ update.commit();
+
+ notes = newNotes(c);
+ psa = Iterables.getOnlyElement(
+ notes.getApprovals().get(c.currentPatchSetId()));
+ assertThat(psa.getAccountId()).isEqualTo(otherUserId);
+ assertThat(psa.getLabel()).isEqualTo("Not-For-Long");
+ assertThat(psa.getValue()).isEqualTo((short) 2);
+
+ }
+
+ @Test
public void multipleReviewers() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
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 6ed70a2..2fb91f6 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
@@ -145,17 +145,12 @@
continue;
}
- try {
- MetaDataUpdate md = metaDataUpdateFactory.create(nameKey);
- try {
- ProjectConfig config = ProjectConfig.read(md);
- config.getProject().setParentName(newParentKey);
- md.setMessage("Inherit access from "
- + (newParentKey != null ? newParentKey.get() : allProjectsName.get()) + "\n");
- config.commit(md);
- } finally {
- md.close();
- }
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(nameKey)) {
+ ProjectConfig config = ProjectConfig.read(md);
+ config.getProject().setParentName(newParentKey);
+ md.setMessage("Inherit access from "
+ + (newParentKey != null ? newParentKey.get() : allProjectsName.get()) + "\n");
+ config.commit(md);
} catch (RepositoryNotFoundException notFound) {
err.append("error: Project ").append(name).append(" not found\n");
} catch (IOException | ConfigInvalidException e) {
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 5eda57c..301bc0e 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
@@ -14,6 +14,8 @@
package com.google.gerrit.sshd.commands;
+import com.google.gerrit.extensions.common.TestSubmitRuleInput;
+import com.google.gerrit.extensions.common.TestSubmitRuleInput.Filters;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
@@ -21,8 +23,6 @@
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.Revisions;
-import com.google.gerrit.server.change.TestSubmitRule.Filters;
-import com.google.gerrit.server.change.TestSubmitRule.Input;
import com.google.gerrit.sshd.SshCommand;
import com.google.inject.Inject;
@@ -34,7 +34,7 @@
import java.nio.ByteBuffer;
abstract class BaseTestPrologCommand extends SshCommand {
- private Input input = new Input();
+ private TestSubmitRuleInput input = new TestSubmitRuleInput();
@Inject
private ChangesCollection changes;
@@ -55,7 +55,7 @@
input.filters = no ? Filters.SKIP : Filters.RUN;
}
- protected abstract RestModifyView<RevisionResource, Input> createView();
+ protected abstract RestModifyView<RevisionResource, TestSubmitRuleInput> createView();
@Override
protected final void run() throws UnloggedFailure {
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 4570085..54e07aa3 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
@@ -122,6 +122,10 @@
@Option(name = "--json", aliases = "-j", usage = "read review input json from stdin")
private boolean json;
+ @Option(name = "--strict-labels", usage = "Strictly check if the labels "
+ + "specified can be applied to the given patch set(s)")
+ private boolean strictLabels;
+
@Option(name = "--label", aliases = "-l", usage = "custom label(s) to assign", metaVar = "LABEL=VALUE")
void addLabel(final String token) {
LabelVote v = LabelVote.parseWithEquals(token);
@@ -272,7 +276,7 @@
review.notify = notify;
review.labels = Maps.newTreeMap();
review.drafts = ReviewInput.DraftHandling.PUBLISH;
- review.strictLabels = false;
+ review.strictLabels = strictLabels;
for (ApproveOption ao : optionList) {
Short v = ao.value();
if (v != null) {
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 ab1353b..589fbf0 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
@@ -124,41 +124,36 @@
String name = ctlProject.getName();
final StringBuilder err = new StringBuilder();
- try {
- MetaDataUpdate md = metaDataUpdateFactory.create(nameKey);
- try {
- ProjectConfig config = ProjectConfig.read(md);
- Project project = config.getProject();
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(nameKey)) {
+ ProjectConfig config = ProjectConfig.read(md);
+ Project project = config.getProject();
- if (requireChangeID != null) {
- project.setRequireChangeID(requireChangeID);
- }
- if (submitType != null) {
- project.setSubmitType(submitType);
- }
- if (contentMerge != null) {
- project.setUseContentMerge(contentMerge);
- }
- if (contributorAgreements != null) {
- project.setUseContributorAgreements(contributorAgreements);
- }
- if (signedOffBy != null) {
- project.setUseSignedOffBy(signedOffBy);
- }
- if (projectDescription != null) {
- project.setDescription(projectDescription);
- }
- if (state != null) {
- project.setState(state);
- }
- if (maxObjectSizeLimit != null) {
- project.setMaxObjectSizeLimit(maxObjectSizeLimit);
- }
- md.setMessage("Project settings updated");
- config.commit(md);
- } finally {
- md.close();
+ if (requireChangeID != null) {
+ project.setRequireChangeID(requireChangeID);
}
+ if (submitType != null) {
+ project.setSubmitType(submitType);
+ }
+ if (contentMerge != null) {
+ project.setUseContentMerge(contentMerge);
+ }
+ if (contributorAgreements != null) {
+ project.setUseContributorAgreements(contributorAgreements);
+ }
+ if (signedOffBy != null) {
+ project.setUseSignedOffBy(signedOffBy);
+ }
+ if (projectDescription != null) {
+ project.setDescription(projectDescription);
+ }
+ if (state != null) {
+ project.setState(state);
+ }
+ if (maxObjectSizeLimit != null) {
+ project.setMaxObjectSizeLimit(maxObjectSizeLimit);
+ }
+ md.setMessage("Project settings updated");
+ config.commit(md);
} catch (RepositoryNotFoundException notFound) {
err.append("error: Project ").append(name).append(" not found\n");
} catch (IOException | ConfigInvalidException e) {
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TestSubmitRuleCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TestSubmitRuleCommand.java
index b957a7a..9c4a680 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TestSubmitRuleCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TestSubmitRuleCommand.java
@@ -14,10 +14,10 @@
package com.google.gerrit.sshd.commands;
+import com.google.gerrit.extensions.common.TestSubmitRuleInput;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.TestSubmitRule;
-import com.google.gerrit.server.change.TestSubmitRule.Input;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -29,7 +29,7 @@
private Provider<TestSubmitRule> view;
@Override
- protected RestModifyView<RevisionResource, Input> createView() {
+ protected RestModifyView<RevisionResource, TestSubmitRuleInput> createView() {
return view.get();
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TestSubmitTypeCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TestSubmitTypeCommand.java
index 2e7f0df..34d4bdc 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TestSubmitTypeCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TestSubmitTypeCommand.java
@@ -15,9 +15,9 @@
package com.google.gerrit.sshd.commands;
+import com.google.gerrit.extensions.common.TestSubmitRuleInput;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.change.RevisionResource;
-import com.google.gerrit.server.change.TestSubmitRule.Input;
import com.google.gerrit.server.change.TestSubmitType;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.inject.Inject;
@@ -29,7 +29,7 @@
private Provider<TestSubmitType> view;
@Override
- protected RestModifyView<RevisionResource, Input> createView() {
+ protected RestModifyView<RevisionResource, TestSubmitRuleInput> createView() {
return view.get();
}
}
diff --git a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
index a2947af..48f7767 100644
--- a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
+++ b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
@@ -292,7 +292,6 @@
private Injector createSysInjector() {
final List<Module> modules = new ArrayList<>();
modules.add(new DropWizardMetricMaker.RestModule());
- modules.add(new WorkQueue.Module());
modules.add(new ChangeHookRunner.Module());
modules.add(new ReceiveCommitsExecutorModule());
modules.add(new DiffExecutorModule());
@@ -306,13 +305,12 @@
modules.add(new PluginRestApiModule());
modules.add(new RestCacheAdminModule());
modules.add(new GpgModule(config));
- switch (indexType) {
- case LUCENE:
- modules.add(new LuceneIndexModule());
- break;
- default:
- throw new IllegalStateException("unsupported index.type = " + indexType);
- }
+
+ // Index module shutdown must happen before work queue shutdown, otherwise
+ // work queue can get stuck waiting on index futures that will never return.
+ modules.add(createIndexModule());
+
+ modules.add(new WorkQueue.Module());
modules.add(new CanonicalWebUrlModule() {
@Override
protected Class<? extends Provider<String>> provider() {
@@ -332,6 +330,15 @@
return cfgInjector.createChildInjector(modules);
}
+ private Module createIndexModule() {
+ switch (indexType) {
+ case LUCENE:
+ return new LuceneIndexModule();
+ default:
+ throw new IllegalStateException("unsupported index.type = " + indexType);
+ }
+ }
+
private void initIndexType() {
indexType = IndexModule.getIndexType(cfgInjector);
}
diff --git a/polygerrit-ui/app/elements/gr-ajax.html b/polygerrit-ui/app/elements/gr-ajax.html
index 9671b37..97f3528 100644
--- a/polygerrit-ui/app/elements/gr-ajax.html
+++ b/polygerrit-ui/app/elements/gr-ajax.html
@@ -79,6 +79,13 @@
},
},
+ ready: function() {
+ // Used for debugging which element a request came from.
+ var headers = this.$.xhr.headers;
+ headers['x-requesting-element-id'] = this.id || 'gr-ajax (no id)';
+ this.$.xhr.headers = headers;
+ },
+
generateRequest: function() {
return this.$.xhr.generateRequest();
},
diff --git a/polygerrit-ui/app/elements/gr-change-view.html b/polygerrit-ui/app/elements/gr-change-view.html
index 8bef101..35153d4 100644
--- a/polygerrit-ui/app/elements/gr-change-view.html
+++ b/polygerrit-ui/app/elements/gr-change-view.html
@@ -38,6 +38,7 @@
height: 4.1em;
}
.header {
+ align-items: center;
background-color: var(--view-background-color);
display: flex;
max-width: var(--max-constrained-width);
@@ -57,6 +58,15 @@
overflow: hidden;
text-overflow: ellipsis;
}
+ .patchSelectLabel {
+ margin-left: var(--default-horizontal-margin);
+ }
+ .header select {
+ margin-left: .5em;
+ }
+ .header gr-reply-dropdown {
+ margin-left: var(--default-horizontal-margin);
+ }
section {
margin: 10px 0;
padding: 10px var(--default-horizontal-margin);
@@ -87,26 +97,39 @@
}
</style>
<gr-ajax id="detailXHR"
- url="[[_computeDetailPath(changeNum)]]"
+ url="[[_computeDetailPath(_changeNum)]]"
params="[[_computeDetailQueryParams()]]"
- last-response="{{change}}"
+ last-response="{{_change}}"
loading="{{_loading}}"></gr-ajax>
<gr-ajax id="commentsXHR"
- url="[[_computeCommentsPath(changeNum)]]"
- last-response="{{comments}}"></gr-ajax>
+ url="[[_computeCommentsPath(_changeNum)]]"
+ last-response="{{_comments}}"></gr-ajax>
+ <gr-ajax id="commitInfoXHR"
+ url="[[_computeCommitInfoPath(_changeNum, _patchNum)]]"
+ last-response="{{_commitInfo}}"></gr-ajax>
<div class="container loading" hidden$="{{!_loading}}">Loading...</div>
<div class="container" hidden$="{{_loading}}">
<div class="headerContainer">
<div class="header">
<h2>
- <a href$="[[_computeChangePath(change._number)]]">[[change._number]]</a><span>:</span>
- <span>[[change.subject]]</span>
+ <a href$="[[_computeChangePath(_change._number)]]">[[_change._number]]</a><span>:</span>
+ <span>[[_change.subject]]</span>
</h2>
+ <label class="patchSelectLabel" for="patchSetSelect">Patch set</label>
+ <select id="patchSetSelect" on-change="_handlePatchChange">
+ <template is="dom-repeat" items="{{_allPatchSets}}" as="patchNumber">
+ <option value$="[[patchNumber]]" selected$="[[_computePatchIndexIsSelected(index, _patchNum)]]">
+ <span>[[patchNumber]]</span>
+ /
+ <span>[[_computeLatestPatchNum(_change)]]</span>
+ </option>
+ </template>
+ </select>
<gr-reply-dropdown id="replyDropdown"
- change-num="[[changeNum]]"
- patch-num="[[_computePatchNum(change.current_revision)]]"
- labels="[[change.labels]]"
- permitted-labels="[[change.permitted_labels]]"
+ change-num="[[_changeNum]]"
+ patch-num="[[_patchNum]]"
+ labels="[[_change.labels]]"
+ permitted-labels="[[_change.permitted_labels]]"
on-send="_handleReplySent"
hidden$="[[!_loggedIn]]">Reply</gr-reply-dropdown>
</div>
@@ -115,13 +138,13 @@
<table>
<tr>
<td class="changeInfo-label">Owner</td>
- <td>[[change.owner.name]]</td>
+ <td>[[_change.owner.name]]</td>
</tr>
<tr>
<td class="changeInfo-label">Reviewers</td>
<td>
<template is="dom-repeat"
- items="[[_computeReviewers(change.labels, change.owner)]]"
+ items="[[_computeReviewers(_change.labels, _change.owner)]]"
as="reviewer">
<div>[[reviewer.name]]</div>
</template>
@@ -129,15 +152,15 @@
</tr>
<tr>
<td class="changeInfo-label">Project</td>
- <td>[[change.project]]</td>
+ <td>[[_change.project]]</td>
</tr>
<tr>
<td class="changeInfo-label">Branch</td>
- <td>[[change.branch]]</td>
+ <td>[[_change.branch]]</td>
</tr>
<tr>
<td class="changeInfo-label">Topic</td>
- <td>[[change.topic]]</td>
+ <td>[[_change.topic]]</td>
</tr>
<tr>
<td class="changeInfo-label">Strategy</td>
@@ -147,21 +170,20 @@
<td class="changeInfo-label">Updated</td>
<td>
<gr-date-formatter
- date-str="[[change.updated]]"></gr-date-formatter>
+ date-str="[[_change.updated]]"></gr-date-formatter>
</td>
</tr>
</table>
</section>
- <section class="summary">[[_computeCurrentRevisionMessage(change)]]</section>
+ <section class="summary">[[_commitInfo.message]]</section>
<gr-file-list id="fileList"
- change-num="[[changeNum]]"
- patch-num="[[_computePatchNum(change.current_revision)]]"
- revision="[[change.current_revision]]"
- comments="[[comments]]"></gr-file-list>
+ change-num="[[_changeNum]]"
+ patch-num="[[_patchNum]]"
+ comments="[[_comments]]"></gr-file-list>
<gr-messages-list
- change-num="[[changeNum]]"
- messages="[[change.messages]]"
- comments="[[comments]]"></gr-messages-list>
+ change-num="[[_changeNum]]"
+ messages="[[_change.messages]]"
+ comments="[[_comments]]"></gr-messages-list>
</div>
</template>
<script>
@@ -189,9 +211,19 @@
type: Object,
observer: '_paramsChanged',
},
- changeNum: Number,
- comments: Object,
+ _comments: Object,
+ _change: {
+ type: Object,
+ observer: '_changeChanged',
+ },
+ _commitInfo: Object,
+ _changeNum: String,
+ _patchNum: String,
+ _allPatchSets: {
+ type: Array,
+ computed: '_computeAllPatchSets(_change)',
+ },
_loggedIn: {
type: Boolean,
value: false,
@@ -240,20 +272,38 @@
el.classList.toggle('pinned', window.scrollY >= top);
},
+ _handlePatchChange: function(e) {
+ var patchNum = e.target.value;
+ var currentPatchNum =
+ this._change.revisions[this._change.current_revision]._number
+ if (patchNum == currentPatchNum) {
+ page.show(this._computeChangePath(this._changeNum));
+ return;
+ }
+ page.show(this._computeChangePath(this._changeNum) + '/' + patchNum);
+ },
+
_handleReplySent: function(e) {
this._reload();
},
_paramsChanged: function(value) {
- this.changeNum = value.changeNum;
- if (!this.changeNum) {
- this.change = null;
- this.comments = null;
+ this._changeNum = value.changeNum;
+ this._patchNum = value.patchNum;
+ if (!this._changeNum) {
+ this._change = null;
+ this._comments = null;
return;
}
this._reload();
},
+ _changeChanged: function(change) {
+ if (!change) { return; }
+ this._patchNum = this._patchNum ||
+ change.revisions[change.current_revision]._number;
+ },
+
_computeChangePath: function(changeNum) {
return '/c/' + changeNum;
},
@@ -262,32 +312,22 @@
return '/changes/' + changeNum + '/detail';
},
- _computeCommitInfoPath: function(changeNum, commitHash) {
- return '/changes/' + changeNum + '/revisions/' + commitHash + '/commit';
+ _computeCommitInfoPath: function(changeNum, patchNum) {
+ return Changes.baseURL(changeNum, patchNum) + '/commit?links';
},
_computeCommentsPath: function(changeNum) {
return '/changes/' + changeNum + '/comments';
},
- _computePatchNum: function(revision) {
- return this.change && this.change.revisions[revision]._number;
- },
-
_computeDetailQueryParams: function() {
var options = Changes.listChangesOptionsToHex(
- Changes.ListChangesOption.CURRENT_REVISION,
- Changes.ListChangesOption.CURRENT_COMMIT,
+ Changes.ListChangesOption.ALL_REVISIONS,
Changes.ListChangesOption.CHANGE_ACTIONS
);
return { O: options };
},
- _computeCurrentRevisionMessage: function(change) {
- return change &&
- change.revisions[change.current_revision].commit.message;
- },
-
_computeReviewers: function(labels, owner) {
var reviewers =
(labels['Code-Review'] && labels['Code-Review'].all) || [];
@@ -297,6 +337,22 @@
});
},
+ _computeLatestPatchNum: function(change) {
+ return change.revisions[change.current_revision]._number;
+ },
+
+ _computeAllPatchSets: function(change) {
+ var patchNums = [];
+ for (var rev in change.revisions) {
+ patchNums.push(change.revisions[rev]._number);
+ }
+ return patchNums.sort();
+ },
+
+ _computePatchIndexIsSelected: function(index, patchNum) {
+ return this._allPatchSets[index] == patchNum;
+ },
+
_handleKey: function(e) {
if (util.shouldSupressKeyboardShortcut(e)) { return; }
e.preventDefault();
@@ -311,9 +367,19 @@
},
_reload: function() {
- this.$.detailXHR.generateRequest();
+ var detailCompletes = this.$.detailXHR.generateRequest().completes;
this.$.commentsXHR.generateRequest();
- this.$.fileList.reload();
+ var reloadCommitInfoAndFileList = function() {
+ this.$.commitInfoXHR.generateRequest();
+ this.$.fileList.reload();
+ }.bind(this);
+
+ if (this._patchNum) {
+ reloadCommitInfoAndFileList();
+ } else {
+ // The patch number is reliant on the change detail request.
+ detailCompletes.then(reloadCommitInfoAndFileList);
+ }
},
});
diff --git a/polygerrit-ui/app/elements/gr-diff-side.html b/polygerrit-ui/app/elements/gr-diff-side.html
index 6e150d6..0120d3b 100644
--- a/polygerrit-ui/app/elements/gr-diff-side.html
+++ b/polygerrit-ui/app/elements/gr-diff-side.html
@@ -60,6 +60,11 @@
/* Line feed */
content: '\A';
}
+ .tab:before {
+ color: #C62828;
+ /* >> character */
+ content: '\00BB';
+ }
.filler {
background: #eee;
}
@@ -80,6 +85,8 @@
SEMICOLON: ';'.charCodeAt(0),
};
+ var TAB_REGEX = /\t/g;
+
Polymer({
is: 'gr-diff-side',
@@ -201,6 +208,7 @@
if (numLines > 1) {
html = this._addNewLines(code.content, html, numLines);
}
+ html = this._addTabIndicators(code.content, html);
// If the html is equivalent to the text then it didn't get highlighted
// or escaped. Use textContent which is faster than innerHTML.
@@ -300,6 +308,11 @@
return result;
},
+ _addTabIndicators: function(content, html) {
+ return html.replace(TAB_REGEX,
+ '<span class="style-scope gr-diff-side tab">\t</span>');
+ },
+
_createElement: function(tagName, className) {
var el = document.createElement(tagName);
// When Shady DOM is being used, these classes are added to account for
diff --git a/polygerrit-ui/app/elements/gr-diff-view.html b/polygerrit-ui/app/elements/gr-diff-view.html
index 4945a67..9829aeb 100644
--- a/polygerrit-ui/app/elements/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/gr-diff-view.html
@@ -55,6 +55,7 @@
change-num="[[_changeNum]]"
patch-range="[[_patchRange]]"
path="[[_path]]"
+ available-patches="[[_computeAvailablePatches(_change.revisions)]]"
on-render="_handleDiffRender">
</gr-diff>
</template>
@@ -156,6 +157,14 @@
}
},
+ _computeAvailablePatches: function(revisions) {
+ var patchNums = [];
+ for (var rev in revisions) {
+ patchNums.push(revisions[rev]._number);
+ }
+ return patchNums.sort(function(a, b) { return a - b; });
+ },
+
_computeChangePath: function(changeNum) {
return '/c/' + changeNum;
},
diff --git a/polygerrit-ui/app/elements/gr-diff.html b/polygerrit-ui/app/elements/gr-diff.html
index 23d14b5..ebc67b9 100644
--- a/polygerrit-ui/app/elements/gr-diff.html
+++ b/polygerrit-ui/app/elements/gr-diff.html
@@ -17,12 +17,16 @@
<link rel="import" href="../bower_components/polymer/polymer.html">
<link rel="import" href="gr-ajax.html">
<link rel="import" href="gr-diff-side.html">
+<link rel="import" href="gr-patch-range-select.html">
<link rel="import" href="gr-request.html">
<dom-module id="gr-diff">
<template>
<style>
- :host {
+ gr-patch-range-select {
+ margin: 0 var(--default-horizontal-margin) .75em;
+ }
+ .diffContainer {
border-bottom: 1px solid #eee;
border-top: 1px solid #eee;
font-family: 'Source Code Pro', monospace;
@@ -51,14 +55,22 @@
url="[[_computeDraftsPath(changeNum, patchRange.basePatchNum)]]"></gr-ajax>
<gr-ajax id="draftsXHR"
url="[[_computeDraftsPath(changeNum, patchRange.patchNum)]]"></gr-ajax>
- <gr-diff-side id="leftDiff"
- content="{{_diff.leftSide}}"
- width="[[sideWidth]]"
- can-comment="[[_loggedIn]]"></gr-diff-side>
- <gr-diff-side id="rightDiff"
- content="{{_diff.rightSide}}"
- width="[[sideWidth]]"
- can-comment="[[_loggedIn]]"></gr-diff-side>
+
+ <gr-patch-range-select
+ path="[[path]]"
+ change-num="[[changeNum]]"
+ patch-range="[[patchRange]]"
+ available-patches="[[availablePatches]]"></gr-patch-range-select>
+ <div class="diffContainer">
+ <gr-diff-side id="leftDiff"
+ content="{{_diff.leftSide}}"
+ width="[[sideWidth]]"
+ can-comment="[[_loggedIn]]"></gr-diff-side>
+ <gr-diff-side id="rightDiff"
+ content="{{_diff.rightSide}}"
+ width="[[sideWidth]]"
+ can-comment="[[_loggedIn]]"></gr-diff-side>
+ </div>
</template>
<script>
(function() {
@@ -78,6 +90,7 @@
type: Boolean,
value: false,
},
+ availablePatches: Array,
changeNum: String,
/*
* A single object to encompass basePatchNum and patchNum is used
@@ -190,7 +203,7 @@
}.bind(this)));
if (basePatchNum != 'PARENT') {
- draftsPromise = this.$baseDraftsXHR.generateRequest().completes;
+ draftsPromise = this.$.baseDraftsXHR.generateRequest().completes;
promises.push(draftsPromise.then(function(req) {
this._baseDrafts =
(req.response[this.path] || []).filter(withoutParent);
diff --git a/polygerrit-ui/app/elements/gr-file-list.html b/polygerrit-ui/app/elements/gr-file-list.html
index dcc96ac..de04f55 100644
--- a/polygerrit-ui/app/elements/gr-file-list.html
+++ b/polygerrit-ui/app/elements/gr-file-list.html
@@ -49,10 +49,10 @@
}
</style>
<gr-ajax id="filesXHR"
- url="[[_computeFilesURL(changeNum, revision)]]"
+ url="[[_computeFilesURL(changeNum, patchNum)]]"
on-response="_handleResponse"></gr-ajax>
<gr-ajax id="draftsXHR"
- url="[[_computeDraftsURL(changeNum, revision)]]"
+ url="[[_computeDraftsURL(changeNum, patchNum)]]"
last-response="{{_drafts}}"></gr-ajax>
</gr-ajax>
<div class="tableContainer">
@@ -73,8 +73,8 @@
href$="[[_computeDiffURL(changeNum, patchNum, file.__path)]]">[[file.__path]]</a>
</td>
<td>
- <span class="drafts">[[_computeDraftsString(_drafts, file.__path)]]</span>
- <span class="comments">[[_computeCommentsString(comments, file.__path)]]</span>
+ <span class="drafts">[[_computeDraftsString(_drafts, patchNum, file.__path)]]</span>
+ <span class="comments">[[_computeCommentsString(comments, patchNum, file.__path)]]</span>
</td>
<td>
+<span>[[file.lines_inserted]]</span> lines,
@@ -93,46 +93,46 @@
is: 'gr-file-list',
properties: {
- patchNum: Number,
- changeNum: Number,
- revision: String,
+ patchNum: String,
+ changeNum: String,
comments: Object,
_drafts: Object,
},
- observers: [
- '_changeNumOrRevisionChanged(changeNum, revision)',
- ],
-
reload: function() {
- if (!!this.changeNum && !!this.revision) {
+ if (!!this.changeNum && !!this.patchNum) {
this.$.filesXHR.generateRequest();
- this.$.draftsXHR.generateRequest();
+ app.accountReady.then(function() {
+ if (!app.loggedIn) { return; }
+ this.$.draftsXHR.generateRequest();
+ }.bind(this));
}
},
- _changeNumOrRevisionChanged: function(changeNum, revision) {
- this.reload();
+ _computeFilesURL: function(changeNum, patchNum) {
+ return Changes.baseURL(changeNum, patchNum) + '/files';
},
- _computeFilesURL: function(changeNum, revision) {
- return Changes.baseURL(changeNum, revision) + '/files';
- },
-
- _computeCommentsString: function(comments, path) {
- var num = (comments[path] || []).length;
+ _computeCommentsString: function(comments, patchNum, path) {
+ var patchComments = (comments[path] || []).filter(function(c) {
+ return c.patch_set == patchNum;
+ });
+ var num = patchComments.length;
if (num == 0) { return ''; }
if (num == 1) { return '1 comment'; }
if (num > 1) { return num + ' comments'; };
},
- _computeDraftsURL: function(changeNum, revision) {
- return Changes.baseURL(changeNum, revision) + '/drafts';
+ _computeDraftsURL: function(changeNum, patchNum) {
+ return Changes.baseURL(changeNum, patchNum) + '/drafts';
},
- _computeDraftsString: function(drafts, path) {
- var num = (drafts[path] || []).length;
+ _computeDraftsString: function(drafts, patchNum, path) {
+ var patchDrafts = (drafts[path] || []).filter(function(d) {
+ return d.patch_set == patchNum;
+ });
+ var num = patchDrafts.length;
if (num == 0) { return ''; }
if (num == 1) { return '1 draft'; }
if (num > 1) { return num + ' drafts'; };
diff --git a/polygerrit-ui/app/elements/gr-patch-range-select.html b/polygerrit-ui/app/elements/gr-patch-range-select.html
new file mode 100644
index 0000000..a2a5dc4
--- /dev/null
+++ b/polygerrit-ui/app/elements/gr-patch-range-select.html
@@ -0,0 +1,95 @@
+<!--
+Copyright (C) 2015 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.
+-->
+
+<link rel="import" href="../bower_components/polymer/polymer.html">
+
+<dom-module id="gr-patch-range-select">
+ <template>
+ <style>
+ :host {
+ display: block;
+ }
+ .patchRange {
+ display: inline-block;
+ }
+ </style>
+ Patch set:
+ <span class="patchRange">
+ <select id="leftPatchSelect" on-change="_handlePatchChange">
+ <option value="PARENT"
+ selected$="[[_computeLeftSelected('PARENT', patchRange)]]">Base</option>
+ <template is="dom-repeat" items="{{availablePatches}}" as="patchNum">
+ <option value$="[[patchNum]]"
+ selected$="[[_computeLeftSelected(patchNum, patchRange)]]"
+ disabled$="[[_computeLeftDisabled(patchNum, patchRange)]]">[[patchNum]]</option>
+ </template>
+ </select>
+ </span>
+ →
+ <span class="patchRange">
+ <select id="rightPatchSelect" on-change="_handlePatchChange">
+ <template is="dom-repeat" items="{{availablePatches}}" as="patchNum">
+ <option value$="[[patchNum]]"
+ selected$="[[_computeRightSelected(patchNum, patchRange)]]"
+ disabled$="[[_computeRightDisabled(patchNum, patchRange)]]">[[patchNum]]</option>
+ </template>
+ </select>
+ </span>
+ </template>
+ <script>
+ (function() {
+ 'use strict';
+
+ Polymer({
+ is: 'gr-patch-range-select',
+
+ properties: {
+ availablePatches: Array,
+ changeNum: String,
+ patchRange: Object,
+ path: String,
+ },
+
+ _handlePatchChange: function(e) {
+ var leftPatch = this.$.leftPatchSelect.value;
+ var rightPatch = this.$.rightPatchSelect.value;
+ var rangeStr = rightPatch;
+ if (leftPatch != 'PARENT') {
+ rangeStr = leftPatch + '..' + rangeStr;
+ }
+ page.show('/c/' + this.changeNum + '/' + rangeStr + '/' + this.path);
+ },
+
+ _computeLeftSelected: function(patchNum, patchRange) {
+ return patchNum == patchRange.basePatchNum;
+ },
+
+ _computeRightSelected: function(patchNum, patchRange) {
+ return patchNum == patchRange.patchNum;
+ },
+
+ _computeLeftDisabled: function(patchNum, patchRange) {
+ return parseInt(patchNum, 10) >= parseInt(patchRange.patchNum, 10);
+ },
+
+ _computeRightDisabled: function(patchNum, patchRange) {
+ if (patchRange.basePatchNum == 'PARENT') { return false; }
+ return parseInt(patchNum, 10) <= parseInt(patchRange.basePatchNum, 10);
+ },
+ });
+ })();
+ </script>
+</dom-module>
diff --git a/polygerrit-ui/app/scripts/app.js b/polygerrit-ui/app/scripts/app.js
index 3ceb90d..2ab2c5c 100644
--- a/polygerrit-ui/app/scripts/app.js
+++ b/polygerrit-ui/app/scripts/app.js
@@ -61,7 +61,7 @@
page.redirect('/c/' + ctx.params[0]);
});
- page('/c/:changeNum', function(data) {
+ page('/c/:changeNum/:patchNum?', function(data) {
app.route = 'gr-change-view';
app.params = data.params;
});
diff --git a/polygerrit-ui/app/test/gr-change-view-test.html b/polygerrit-ui/app/test/gr-change-view-test.html
index 4cf1d4c..8671560 100644
--- a/polygerrit-ui/app/test/gr-change-view-test.html
+++ b/polygerrit-ui/app/test/gr-change-view-test.html
@@ -37,9 +37,25 @@
<script>
suite('gr-change-view tests', function() {
var element;
+ var server;
setup(function() {
element = fixture('basic');
+ server = sinon.fakeServer.create();
+ // Eat any requests made by elements in this suite.
+ server.respondWith(
+ 'GET',
+ /\/changes\/(.*)/,
+ [
+ 200,
+ { 'Content-Type': 'application/json' },
+ ')]}\'\n{}',
+ ]
+ );
+ });
+
+ teardown(function() {
+ server.restore();
});
test('keyboard shortcuts', function() {
@@ -57,5 +73,49 @@
assert.isFalse(dropdownEl.opened);
});
+ test('patch num change', function(done) {
+ element._changeNum = '42';
+ element._patchNum = 2;
+ element._change = {
+ revisions: {
+ rev2: { _number: 2 },
+ rev1: { _number: 1 },
+ rev3: { _number: 3 },
+ },
+ current_revision: 'rev3',
+ };
+ flushAsynchronousOperations();
+ var selectEl = element.$$('.header select');
+ assert.ok(selectEl);
+ var optionEls =
+ Polymer.dom(element.root).querySelectorAll('.header option');
+ assert.equal(optionEls.length, 3);
+ assert.isFalse(
+ element.$$('.header option[value="1"]').hasAttribute('selected'));
+ assert.isTrue(
+ element.$$('.header option[value="2"]').hasAttribute('selected'));
+ assert.isFalse(
+ element.$$('.header option[value="3"]').hasAttribute('selected'));
+
+ var showStub = sinon.stub(page, 'show');
+
+ var numEvents = 0;
+ selectEl.addEventListener('change', function(e) {
+ numEvents++;
+ if (numEvents == 1) {
+ assert(showStub.lastCall.calledWithExactly('/c/42/1'),
+ 'Should navigate to /c/42/1');
+ selectEl.value = '3';
+ element.fire('change', {}, {node: selectEl});
+ } else if (numEvents == 2) {
+ assert(showStub.lastCall.calledWithExactly('/c/42'),
+ 'Should navigate to /c/42');
+ showStub.restore();
+ done();
+ }
+ });
+ selectEl.value = '1';
+ element.fire('change', {}, {node: selectEl});
+ });
});
</script>
diff --git a/polygerrit-ui/app/test/gr-file-list-test.html b/polygerrit-ui/app/test/gr-file-list-test.html
new file mode 100644
index 0000000..f474f15
--- /dev/null
+++ b/polygerrit-ui/app/test/gr-file-list-test.html
@@ -0,0 +1,126 @@
+<!DOCTYPE html>
+<!--
+Copyright (C) 2015 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.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-file-list</title>
+
+<script src="../../bower_components/webcomponentsjs/webcomponents.min.js"></script>
+<script src="../../bower_components/web-component-tester/browser.js"></script>
+<script src="../../bower_components/page/page.js"></script>
+<script src="../scripts/changes.js"></script>
+<script src="../scripts/fake-app.js"></script>
+<script src="../scripts/util.js"></script>
+
+<link rel="import" href="../../bower_components/iron-test-helpers/iron-test-helpers.html">
+<link rel="import" href="../elements/gr-file-list.html">
+
+<test-fixture id="basic">
+ <template>
+ <gr-file-list></gr-file-list>
+ </template>
+</test-fixture>
+
+<script>
+ suite('gr-file-list tests', function() {
+ var element;
+ var server;
+
+ setup(function() {
+ element = fixture('basic');
+ server = sinon.fakeServer.create();
+ server.respondWith(
+ 'GET',
+ '/changes/42/revisions/1/files',
+ [
+ 200,
+ { 'Content-Type': 'application/json' },
+ ')]}\'\n' +
+ JSON.stringify({
+ '/COMMIT_MSG': {
+ status: 'A',
+ lines_inserted: 9,
+ size_delta: 317,
+ size: 317
+ },
+ 'myfile.txt': {
+ lines_inserted: 35,
+ size_delta: 1146,
+ size: 1167
+ }
+ }),
+ ]
+ );
+ server.respondWith(
+ 'GET',
+ '/changes/42/revisions/2/files',
+ [
+ 200,
+ { 'Content-Type': 'application/json' },
+ ')]}\'\n' +
+ JSON.stringify({
+ '/COMMIT_MSG': {
+ status: 'A',
+ lines_inserted: 9,
+ size_delta: 317,
+ size: 317
+ },
+ 'myfile.txt': {
+ lines_inserted: 35,
+ size_delta: 1146,
+ size: 1167
+ },
+ 'file_added_in_rev2.txt': {
+ lines_inserted: 98,
+ size_delta: 234,
+ size: 136
+ }
+ }),
+ ]
+ );
+ });
+
+ teardown(function() {
+ server.restore();
+ });
+
+ test('requests', function(done) {
+ element.changeNum = '42';
+ element.patchNum = '1';
+ element.reload();
+ server.respond();
+
+ element.async(function() {
+ var filenames = element.files.map(function(f) {
+ return f.__path;
+ });
+ assert.deepEqual(filenames, ['/COMMIT_MSG', 'myfile.txt']);
+
+ element.patchNum = '2';
+ element.reload();
+ server.respond();
+ element.async(function() {
+ filenames = element.files.map(function(f) {
+ return f.__path;
+ });
+ assert.deepEqual(filenames,
+ ['/COMMIT_MSG', 'file_added_in_rev2.txt', 'myfile.txt']);
+ done();
+ }, 1);
+ }, 1)
+ });
+ });
+</script>
diff --git a/polygerrit-ui/app/test/gr-patch-range-select-test.html b/polygerrit-ui/app/test/gr-patch-range-select-test.html
new file mode 100644
index 0000000..c2b19e7
--- /dev/null
+++ b/polygerrit-ui/app/test/gr-patch-range-select-test.html
@@ -0,0 +1,93 @@
+<!DOCTYPE html>
+<!--
+Copyright (C) 2015 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.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-patch-range-select</title>
+
+<script src="../../bower_components/webcomponentsjs/webcomponents.min.js"></script>
+<script src="../../bower_components/web-component-tester/browser.js"></script>
+<script src="../../bower_components/page/page.js"></script>
+
+<link rel="import" href="../../bower_components/iron-test-helpers/iron-test-helpers.html">
+<link rel="import" href="../elements/gr-patch-range-select.html">
+
+<test-fixture id="basic">
+ <template>
+ <gr-patch-range-select auto></gr-patch-range-select>
+ </template>
+</test-fixture>
+
+<script>
+ suite('gr-patch-range-select tests', function() {
+ var element;
+
+ setup(function() {
+ element = fixture('basic');
+ });
+
+ test('enabled/disabled options', function() {
+ var patchRange = {
+ basePatchNum: 'PARENT',
+ patchNum: '3',
+ };
+ ['1', '2', '3'].forEach(function(patchNum) {
+ assert.isFalse(element._computeRightDisabled(patchNum, patchRange));
+ });
+ ['PARENT', '1', '2'].forEach(function(patchNum) {
+ assert.isFalse(element._computeLeftDisabled(patchNum, patchRange));
+ });
+ assert.isTrue(element._computeLeftDisabled('3', patchRange));
+
+ patchRange.basePatchNum = '2';
+ assert.isTrue(element._computeLeftDisabled('3', patchRange));
+ assert.isTrue(element._computeRightDisabled('1', patchRange));
+ assert.isTrue(element._computeRightDisabled('2', patchRange));
+ assert.isFalse(element._computeRightDisabled('3', patchRange));
+ });
+
+ test('navigation', function(done) {
+ var showStub = sinon.stub(page, 'show');
+ var leftSelectEl = element.$.leftPatchSelect;
+ var rightSelectEl = element.$.rightPatchSelect;
+ element.changeNum = '42';
+ element.path = 'path/to/file.txt';
+ element.availablePatches = ['1', '2', '3'];
+ flushAsynchronousOperations();
+
+ var numEvents = 0;
+ leftSelectEl.addEventListener('change', function(e) {
+ numEvents++;
+ if (numEvents == 1) {
+ assert(showStub.lastCall.calledWithExactly(
+ '/c/42/3/path/to/file.txt'),
+ 'Should navigate to /c/42/3/path/to/file.txt');
+ leftSelectEl.value = '1';
+ element.fire('change', {}, {node: leftSelectEl});
+ } else if (numEvents == 2) {
+ assert(showStub.lastCall.calledWithExactly(
+ '/c/42/1..3/path/to/file.txt'),
+ 'Should navigate to /c/42/1..3/path/to/file.txt');
+ showStub.restore();
+ done();
+ }
+ });
+ leftSelectEl.value = 'PARENT';
+ rightSelectEl.value = '3';
+ element.fire('change', {}, {node: leftSelectEl});
+ });
+ });
+</script>
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 42cd24f..1ba06b0 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -33,6 +33,8 @@
'gr-diff-side-test.html',
'gr-diff-test.html',
'gr-diff-view-test.html',
+ 'gr-file-list-test.html',
+ 'gr-patch-range-select-test.html',
'gr-reply-dropdown-test.html',
'gr-search-bar-test.html',
].forEach(function(file) {
