Merge "Update label vote within reply-dialog when modified externally"
diff --git a/Documentation/config-project-config.txt b/Documentation/config-project-config.txt
index 5483d85..34f39c8 100644
--- a/Documentation/config-project-config.txt
+++ b/Documentation/config-project-config.txt
@@ -84,6 +84,11 @@
also redefine the text and behavior of the built in label types `Code-Review`
and `Verified`.
+Optionally a +commentlink+ section can be added to define project-specific
+comment links. The +commentlink+ section has the same format as the
+link:config-gerrit.html#commentlink[+commentlink+ section in gerrit.config]
+which is used to define global comment links.
+
[[project-section]]
=== Project section
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index bed3760..ccfc440 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -2417,18 +2417,21 @@
[source, java]
----
-import com.google.gerrit.server.change.ReviewerSuggestion;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.annotations.ExtensionPoint;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
import java.util.Set;
public class MyPlugin implements ReviewerSuggestion {
- public Set<SuggestedReviewer> suggestReviewers(
- Change.Id changeId, String query, Set<Account.Id> candidates){
- Set<SuggestedReviewer> suggestions = new HashSet<>();
- // Implement your ranking logic here
- return suggestions;
+ public Set<SuggestedReviewer> suggestReviewers(Project.NameKey project,
+ @Nullable Change.Id changeId, @Nullable String query,
+ Set<Account.Id> candidates) {
+ Set<SuggestedReviewer> suggestions = new HashSet<>();
+ // Implement your ranking logic here
+ return suggestions;
}
}
----
diff --git a/Documentation/dev-readme.txt b/Documentation/dev-readme.txt
index 4959ced..bd7f6e9 100644
--- a/Documentation/dev-readme.txt
+++ b/Documentation/dev-readme.txt
@@ -90,22 +90,49 @@
java -jar buck-out/gen/gerrit/gerrit.war init -d ../gerrit_testsite
----
-Accept defaults by pressing Enter until 'init' completes, or add
-the '--batch' command line option to avoid them entirely. It is
-recommended to change the listen addresses from '*' to 'localhost' to
-prevent outside connections from contacting the development instance.
+During initialization, make two changes to the default settings:
-The daemon will automatically start in the background and a web
-browser will launch to the start page, enabling login via OpenID.
+* Change the listen addresses from '*' to 'localhost' to prevent outside
+ connections from contacting the development instance; and
+* Change the auth type from 'OPENID' to 'DEVELOPMENT_BECOME_ANY_ACCOUNT' to
+ allow yourself to create and act as arbitrary test accounts on your
+ development instance.
-Shutdown the daemon after registering the administrator account
-through the web interface:
+Continue through init until it completes. The daemon will automatically start in
+the background and a web browser will launch to the start page. From here you
+can sign in as the account created during init, register additional accounts,
+create projects, and more.
+
+When you want to shut down the daemon, simply run:
----
../gerrit_testsite/bin/gerrit.sh stop
----
+[[localdev]]
+== Working with the Local Server
+
+If you need to create additional accounts on your development instance, click
+'become' in the upper right corner, select 'Switch User', and then register
+a new account.
+
+Use the `ssh` protocol to clone from and push to the local server. For
+example, to clone a repository that you've created through the admin
+interface, run:
+
+----
+git clone ssh://username@localhost:29418/projectname
+----
+
+Then you'll be able to create changes the same way users do, with
+
+----
+git push origin HEAD:refs/for/master
+----
+
+
+
== Testing
diff --git a/Documentation/intro-project-owner.txt b/Documentation/intro-project-owner.txt
index 7a724f7..72fe717 100644
--- a/Documentation/intro-project-owner.txt
+++ b/Documentation/intro-project-owner.txt
@@ -70,8 +70,8 @@
commands:
----
- $ git fetch origin refs/meta/config:config
- $ git checkout config
+ $ git fetch ssh://localhost:29418/project refs/meta/config
+ $ git checkout FETCH_HEAD
$ git log project.config
----
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index ababc16..47030cf 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -1382,7 +1382,8 @@
"show_tabs": true,
"show_whitespace_errors": true,
"syntax_highlighting": true,
- "tab_size": 8
+ "tab_size": 8,
+ "font_size": 12
}
----
@@ -1413,7 +1414,8 @@
"show_tabs": true,
"show_whitespace_errors": true,
"syntax_highlighting": true,
- "tab_size": 8
+ "tab_size": 8,
+ "font_size": 12
}
----
@@ -1437,7 +1439,8 @@
"show_tabs": true,
"show_whitespace_errors": true,
"syntax_highlighting": true,
- "tab_size": 8
+ "tab_size": 8,
+ "font_size": 12
}
----
@@ -2209,6 +2212,8 @@
If true the line numbers are hidden.
|`tab_size` ||
Number of spaces that should be used to display one tab.
+|`font_size` ||
+Default font size in pixels for change to be displayed in the diff view.
|'hide_empty_pane' |not set if `false`|
Whether empty panes should be hidden. The left pane is empty when a
file was added; the right pane is empty when a file was deleted.
@@ -2269,6 +2274,8 @@
True if the line numbers should be hidden.
|`tab_size` |optional|
Number of spaces that should be used to display one tab.
+|`font_size` |optional|
+Default font size in pixels for change to be displayed in the diff view.
|`line_wrapping` |optional|
Whether to enable line wrapping or not.
|===========================================
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index ae02475..d8214c2 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -4644,6 +4644,8 @@
while posting the review.
NOTE: To apply different tags on on different votes/comments multiple
invocations of the REST call are required.
+|`post_submit` |not set if `false`|
+If true, this vote was made after the change was submitted.
|===========================
[[assignee-input]]
@@ -4860,11 +4862,13 @@
=== CherryPickInput
The `CherryPickInput` entity contains information for cherry-picking a change to a new branch.
-[options="header",cols="1,6"]
+[options="header",cols="1,^1,5"]
|===========================
-|Field Name |Description
-|`message` |Commit message for the cherry-picked change
-|`destination` |Destination branch
+|Field Name ||Description
+|`message` ||Commit message for the cherry-picked change
+|`destination` ||Destination branch
+|`parent` |optional, defaults to 1|
+Number of the parent relative to which the cherry-pick should be considered.
|===========================
[[comment-info]]
diff --git a/WORKSPACE b/WORKSPACE
index 17fbfea..4487821 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -672,8 +672,8 @@
maven_jar(
name = 'truth',
- artifact = 'com.google.truth:truth:0.28',
- sha1 = '0a388c7877c845ff4b8e19689dda5ac9d34622c4',
+ artifact = 'com.google.truth:truth:0.30',
+ sha1 = '9d591b5a66eda81f0b88cf1c748ab8853d99b18b',
)
maven_jar(
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 c0d6f18..300eb74 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
@@ -22,7 +22,6 @@
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static org.eclipse.jgit.lib.Constants.HEAD;
-import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -144,6 +143,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.regex.Pattern;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GitUtil.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GitUtil.java
index ceab04fe..2f1463d 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GitUtil.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GitUtil.java
@@ -16,8 +16,8 @@
import static com.google.common.truth.Truth.assertThat;
-import com.google.common.base.Optional;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
import com.google.common.primitives.Ints;
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.reviewdb.client.Project;
@@ -50,6 +50,7 @@
import java.io.IOException;
import java.util.List;
+import java.util.Optional;
import java.util.Properties;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
@@ -239,10 +240,7 @@
throws IOException {
RevCommit c = tr.getRevWalk().parseCommit(id);
tr.getRevWalk().parseBody(c);
- List<String> ids = c.getFooterLines(FooterConstants.CHANGE_ID);
- if (ids.isEmpty()) {
- return Optional.absent();
- }
- return Optional.of(ids.get(ids.size() - 1));
+ return Lists.reverse(c.getFooterLines(FooterConstants.CHANGE_ID)).stream()
+ .findFirst();
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/DiffPreferencesIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/DiffPreferencesIT.java
index 9236176..bce9861 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/DiffPreferencesIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/DiffPreferencesIT.java
@@ -78,6 +78,7 @@
// change all default values
i.context *= -1;
i.tabSize *= -1;
+ i.fontSize *= -1;
i.lineLength *= -1;
i.cursorBlinkRate = 500;
i.theme = Theme.MIDNIGHT;
@@ -121,9 +122,11 @@
DiffPreferencesInfo d = DiffPreferencesInfo.defaults();
int newLineLength = d.lineLength + 10;
int newTabSize = d.tabSize * 2;
+ int newFontSize = d.fontSize - 2;
DiffPreferencesInfo update = new DiffPreferencesInfo();
update.lineLength = newLineLength;
update.tabSize = newTabSize;
+ update.fontSize = newFontSize;
gApi.config().server().setDefaultDiffPreferences(update);
DiffPreferencesInfo o = gApi.accounts()
@@ -133,8 +136,9 @@
// assert configured defaults
assertThat(o.lineLength).isEqualTo(newLineLength);
assertThat(o.tabSize).isEqualTo(newTabSize);
+ assertThat(o.fontSize).isEqualTo(newFontSize);
// assert hard-coded defaults
- assertPrefs(o, d, "lineLength", "tabSize");
+ assertPrefs(o, d, "lineLength", "tabSize", "fontSize");
}
}
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 3dd1ff3..dc829dc 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
@@ -406,15 +406,6 @@
}
@Test
- public void voteOnClosedChange() throws Exception {
- PushOneCommit.Result r = createChange();
- merge(r);
- exception.expect(ResourceConflictException.class);
- exception.expectMessage("change is closed");
- revision(r).review(ReviewInput.reject());
- }
-
- @Test
public void rebaseUpToDateChange() throws Exception {
PushOneCommit.Result r = createChange();
exception.expect(ResourceConflictException.class);
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index 7f5c6bc..f383daf 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -26,10 +26,14 @@
import static org.junit.Assert.fail;
import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Iterators;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.CherryPickInput;
import com.google.gerrit.extensions.api.changes.DraftApi;
@@ -39,25 +43,34 @@
import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.client.ChangeStatus;
+import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.SubmitType;
+import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.DiffInfo;
import com.google.gerrit.extensions.common.FileInfo;
+import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.common.MergeableInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.ETagView;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.change.GetRevisionActions;
import com.google.gerrit.server.change.RevisionResource;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.transport.RefSpec;
import org.junit.Test;
import java.io.ByteArrayOutputStream;
@@ -66,6 +79,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
+import java.util.EnumSet;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
@@ -127,6 +141,115 @@
}
@Test
+ public void postSubmitApproval() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = project.get() + "~master~" + r.getChangeId();
+ gApi.changes()
+ .id(changeId)
+ .current()
+ .review(ReviewInput.recommend());
+
+ String label = "Code-Review";
+ ApprovalInfo approval = getApproval(changeId, label);
+ assertThat(approval.value).isEqualTo(1);
+ assertThat(approval.postSubmit).isNull();
+
+ // Submit by direct push.
+ git().push()
+ .setRefSpecs(new RefSpec(r.getCommit().name() + ":refs/heads/master"))
+ .call();
+ assertThat(gApi.changes().id(changeId).get().status)
+ .isEqualTo(ChangeStatus.MERGED);
+
+ approval = getApproval(changeId, label);
+ assertThat(approval.value).isEqualTo(1);
+ assertThat(approval.postSubmit).isNull();
+
+ // Repeating the current label is allowed. Does not flip the postSubmit bit
+ // due to deduplication codepath.
+ gApi.changes()
+ .id(changeId)
+ .current()
+ .review(ReviewInput.recommend());
+ approval = getApproval(changeId, label);
+ assertThat(approval.value).isEqualTo(1);
+ assertThat(approval.postSubmit).isNull();
+
+ // Reducing vote is not allowed.
+ try {
+ gApi.changes()
+ .id(changeId)
+ .current()
+ .review(ReviewInput.dislike());
+ fail("expected ResourceConflictException");
+ } catch (ResourceConflictException e) {
+ assertThat(e).hasMessage(
+ "Cannot reduce vote on labels for closed change: Code-Review");
+ }
+ approval = getApproval(changeId, label);
+ assertThat(approval.value).isEqualTo(1);
+ assertThat(approval.postSubmit).isNull();
+
+ // Increasing vote is allowed.
+ gApi.changes()
+ .id(changeId)
+ .current()
+ .review(ReviewInput.approve());
+ approval = getApproval(changeId, label);
+ assertThat(approval.value).isEqualTo(2);
+ assertThat(approval.postSubmit).isTrue();
+
+ // Decreasing to previous post-submit vote is still not allowed.
+ try {
+ gApi.changes()
+ .id(changeId)
+ .current()
+ .review(ReviewInput.dislike());
+ fail("expected ResourceConflictException");
+ } catch (ResourceConflictException e) {
+ assertThat(e).hasMessage(
+ "Cannot reduce vote on labels for closed change: Code-Review");
+ }
+ approval = getApproval(changeId, label);
+ assertThat(approval.value).isEqualTo(2);
+ assertThat(approval.postSubmit).isTrue();
+ }
+
+ @TestProjectInput(submitType = SubmitType.CHERRY_PICK)
+ @Test
+ public void approvalCopiedDuringSubmitIsNotPostSubmit() throws Exception {
+ PushOneCommit.Result r = createChange();
+ Change.Id id = r.getChange().getId();
+ gApi.changes()
+ .id(id.get())
+ .current()
+ .review(ReviewInput.approve());
+ gApi.changes()
+ .id(id.get())
+ .current()
+ .submit();
+
+ ChangeData cd = r.getChange();
+ assertThat(cd.patchSets()).hasSize(2);
+ PatchSetApproval psa = Iterators.getOnlyElement(
+ cd.currentApprovals().stream()
+ .filter(a -> !a.isLegacySubmit()).iterator());
+ assertThat(psa.getPatchSetId().get()).isEqualTo(2);
+ assertThat(psa.getLabel()).isEqualTo("Code-Review");
+ assertThat(psa.getValue()).isEqualTo(2);
+ assertThat(psa.isPostSubmit()).isFalse();
+ }
+
+ @Test
+ public void voteOnAbandonedChange() throws Exception {
+ PushOneCommit.Result r = createChange();
+ gApi.changes().id(r.getChangeId()).abandon();
+ exception.expect(ResourceConflictException.class);
+ exception.expectMessage("change is closed");
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.reject());
+ }
+
+ @Test
public void deleteDraft() throws Exception {
PushOneCommit.Result r = createDraft();
gApi.changes()
@@ -357,6 +480,111 @@
}
@Test
+ public void cherryPickMergeRelativeToDefaultParent() throws Exception {
+ String parent1FileName = "a.txt";
+ String parent2FileName = "b.txt";
+ PushOneCommit.Result mergeChangeResult =
+ createCherryPickableMerge(parent1FileName, parent2FileName);
+
+ String cherryPickBranchName = "branch_for_cherry_pick";
+ createBranch(new Branch.NameKey(project, cherryPickBranchName));
+
+ CherryPickInput cherryPickInput = new CherryPickInput();
+ cherryPickInput.destination = cherryPickBranchName;
+ cherryPickInput.message = "Cherry-pick a merge commit to another branch";
+
+ ChangeInfo cherryPickedChangeInfo = gApi.changes()
+ .id(mergeChangeResult.getChangeId())
+ .current()
+ .cherryPick(cherryPickInput)
+ .get();
+
+ Map<String, FileInfo> cherryPickedFilesByName =
+ cherryPickedChangeInfo.revisions
+ .get(cherryPickedChangeInfo.currentRevision)
+ .files;
+ assertThat(cherryPickedFilesByName).containsKey(parent2FileName);
+ assertThat(cherryPickedFilesByName).doesNotContainKey(parent1FileName);
+ }
+
+ @Test
+ public void cherryPickMergeRelativeToSpecificParent() throws Exception {
+ String parent1FileName = "a.txt";
+ String parent2FileName = "b.txt";
+ PushOneCommit.Result mergeChangeResult =
+ createCherryPickableMerge(parent1FileName, parent2FileName);
+
+ String cherryPickBranchName = "branch_for_cherry_pick";
+ createBranch(new Branch.NameKey(project, cherryPickBranchName));
+
+ CherryPickInput cherryPickInput = new CherryPickInput();
+ cherryPickInput.destination = cherryPickBranchName;
+ cherryPickInput.message = "Cherry-pick a merge commit to another branch";
+ cherryPickInput.parent = 2;
+
+ ChangeInfo cherryPickedChangeInfo = gApi.changes()
+ .id(mergeChangeResult.getChangeId())
+ .current()
+ .cherryPick(cherryPickInput)
+ .get();
+
+ Map<String, FileInfo> cherryPickedFilesByName =
+ cherryPickedChangeInfo.revisions
+ .get(cherryPickedChangeInfo.currentRevision)
+ .files;
+ assertThat(cherryPickedFilesByName).containsKey(parent1FileName);
+ assertThat(cherryPickedFilesByName).doesNotContainKey(parent2FileName);
+ }
+
+ @Test
+ public void cherryPickMergeUsingInvalidParent() throws Exception {
+ String parent1FileName = "a.txt";
+ String parent2FileName = "b.txt";
+ PushOneCommit.Result mergeChangeResult =
+ createCherryPickableMerge(parent1FileName, parent2FileName);
+
+ String cherryPickBranchName = "branch_for_cherry_pick";
+ createBranch(new Branch.NameKey(project, cherryPickBranchName));
+
+ CherryPickInput cherryPickInput = new CherryPickInput();
+ cherryPickInput.destination = cherryPickBranchName;
+ cherryPickInput.message = "Cherry-pick a merge commit to another branch";
+ cherryPickInput.parent = 0;
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("Cherry Pick: Parent 0 does not exist. Please"
+ + " specify a parent in range [1, 2].");
+ gApi.changes()
+ .id(mergeChangeResult.getChangeId())
+ .current()
+ .cherryPick(cherryPickInput);
+ }
+
+ @Test
+ public void cherryPickMergeUsingNonExistentParent() throws Exception {
+ String parent1FileName = "a.txt";
+ String parent2FileName = "b.txt";
+ PushOneCommit.Result mergeChangeResult =
+ createCherryPickableMerge(parent1FileName, parent2FileName);
+
+ String cherryPickBranchName = "branch_for_cherry_pick";
+ createBranch(new Branch.NameKey(project, cherryPickBranchName));
+
+ CherryPickInput cherryPickInput = new CherryPickInput();
+ cherryPickInput.destination = cherryPickBranchName;
+ cherryPickInput.message = "Cherry-pick a merge commit to another branch";
+ cherryPickInput.parent = 3;
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("Cherry Pick: Parent 3 does not exist. Please"
+ + " specify a parent in range [1, 2].");
+ gApi.changes()
+ .id(mergeChangeResult.getChangeId())
+ .current()
+ .cherryPick(cherryPickInput);
+ }
+
+ @Test
public void canRebase() throws Exception {
PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
PushOneCommit.Result r1 = push.to("refs/for/master");
@@ -806,4 +1034,49 @@
assertDiffForNewFile(diff, pushResult.getCommit(), path,
expectedContentSideB);
}
+
+ private PushOneCommit.Result createCherryPickableMerge(String parent1FileName,
+ String parent2FileName) throws Exception {
+ RevCommit initialCommit = getHead(repo());
+
+ String branchAName = "branchA";
+ createBranch(new Branch.NameKey(project, branchAName));
+ String branchBName = "branchB";
+ createBranch(new Branch.NameKey(project, branchBName));
+
+ PushOneCommit.Result changeAResult = pushFactory
+ .create(db, admin.getIdent(), testRepo, "change a",
+ parent1FileName, "Content of a")
+ .to("refs/for/" + branchAName);
+
+ testRepo.reset(initialCommit);
+ PushOneCommit.Result changeBResult = pushFactory
+ .create(db, admin.getIdent(), testRepo, "change b",
+ parent2FileName, "Content of b")
+ .to("refs/for/" + branchBName);
+
+ PushOneCommit pushableMergeCommit = pushFactory.create(db, admin.getIdent(),
+ testRepo, "merge", ImmutableMap.of(parent1FileName, "Content of a",
+ parent2FileName, "Content of b"));
+ pushableMergeCommit.setParents(ImmutableList.of(changeAResult.getCommit(),
+ changeBResult.getCommit()));
+ PushOneCommit.Result mergeChangeResult =
+ pushableMergeCommit.to("refs/for/" + branchAName);
+ mergeChangeResult.assertOkStatus();
+ return mergeChangeResult;
+ }
+
+ private ApprovalInfo getApproval(String changeId, String label)
+ throws Exception {
+ ChangeInfo info = gApi.changes()
+ .id(changeId)
+ .get(EnumSet.of(ListChangesOption.DETAILED_LABELS));
+ LabelInfo li = info.labels.get(label);
+ assertThat(li).isNotNull();
+ int accountId = atrScope.get().getUser().getAccountId().get();
+ return li.all.stream()
+ .filter(a -> a._accountId == accountId)
+ .findFirst()
+ .get();
+ }
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index c611f2c..7830b17e 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -20,7 +20,6 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.concurrent.TimeUnit.SECONDS;
-import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
@@ -87,6 +86,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
public class ChangeEditIT extends AbstractDaemonTest {
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
similarity index 62%
rename from gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java
rename to gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
index fd2385b..521ccc4 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.git;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
@@ -34,18 +35,25 @@
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.edit.ChangeEditModifier;
import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.git.ReceiveCommitsAdvertiseRefsHook;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.VisibleRefFilter;
+import com.google.gerrit.server.notedb.ChangeNoteUtil;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.Util;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testutil.DisabledReviewDb;
+import com.google.gerrit.testutil.TestChanges;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
@@ -53,11 +61,12 @@
import org.junit.Test;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
@NoHttpd
-public class VisibleRefFilterIT extends AbstractDaemonTest {
+public class RefAdvertisementIT extends AbstractDaemonTest {
@Inject
private ChangeEditModifier editModifier;
@@ -74,12 +83,23 @@
@Inject
private Provider<CurrentUser> userProvider;
+ @Inject
+ private ChangeNoteUtil noteUtil;
+
+ @Inject
+ @AnonymousCowardName
+ private String anonymousCowardName;
+
private AccountGroup.UUID admins;
- private Change.Id c1;
- private Change.Id c2;
+ private ChangeData c1;
+ private ChangeData c2;
+ private ChangeData c3;
+ private ChangeData c4;
private String r1;
private String r2;
+ private String r3;
+ private String r4;
@Before
public void setUp() throws Exception {
@@ -111,17 +131,31 @@
.branch("branch")
.create(new BranchInput());
+ // First 2 changes are merged, which means the tags pointing to them are
+ // visible.
allow(Permission.SUBMIT, admins, "refs/for/refs/heads/*");
PushOneCommit.Result mr = pushFactory.create(db, admin.getIdent(), testRepo)
.to("refs/for/master%submit");
mr.assertOkStatus();
- c1 = mr.getChange().getId();
- r1 = changeRefPrefix(c1);
+ c1 = mr.getChange();
+ r1 = changeRefPrefix(c1.getId());
PushOneCommit.Result br = pushFactory.create(db, admin.getIdent(), testRepo)
.to("refs/for/branch%submit");
br.assertOkStatus();
- c2 = br.getChange().getId();
- r2 = changeRefPrefix(c2);
+ c2 = br.getChange();
+ r2 = changeRefPrefix(c2.getId());
+
+ // Second 2 changes are unmerged.
+ mr = pushFactory.create(db, admin.getIdent(), testRepo)
+ .to("refs/for/master");
+ mr.assertOkStatus();
+ c3 = mr.getChange();
+ r3 = changeRefPrefix(c3.getId());
+ br = pushFactory.create(db, admin.getIdent(), testRepo)
+ .to("refs/for/branch");
+ br.assertOkStatus();
+ c4 = br.getChange();
+ r4 = changeRefPrefix(c4.getId());
try (Repository repo = repoManager.openRepository(project)) {
// master-tag -> master
@@ -139,7 +173,7 @@
}
@Test
- public void allRefsVisibleNoRefsMetaConfig() throws Exception {
+ public void uploadPackAllRefsVisibleNoRefsMetaConfig() throws Exception {
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
Util.allow(cfg, Permission.READ, REGISTERED_USERS, "refs/*");
Util.allow(cfg, Permission.READ, admins, RefNames.REFS_CONFIG);
@@ -147,12 +181,16 @@
saveProjectConfig(project, cfg);
setApiUser(user);
- assertRefs(
+ assertUploadPackRefs(
"HEAD",
r1 + "1",
r1 + "meta",
r2 + "1",
r2 + "meta",
+ r3 + "1",
+ r3 + "meta",
+ r4 + "1",
+ r4 + "meta",
"refs/heads/branch",
"refs/heads/master",
"refs/tags/branch-tag",
@@ -160,16 +198,20 @@
}
@Test
- public void allRefsVisibleWithRefsMetaConfig() throws Exception {
+ public void uploadPackAllRefsVisibleWithRefsMetaConfig() throws Exception {
allow(Permission.READ, REGISTERED_USERS, "refs/*");
allow(Permission.READ, REGISTERED_USERS, RefNames.REFS_CONFIG);
- assertRefs(
+ assertUploadPackRefs(
"HEAD",
r1 + "1",
r1 + "meta",
r2 + "1",
r2 + "meta",
+ r3 + "1",
+ r3 + "meta",
+ r4 + "1",
+ r4 + "meta",
"refs/heads/branch",
"refs/heads/master",
RefNames.REFS_CONFIG,
@@ -178,28 +220,32 @@
}
@Test
- public void subsetOfBranchesVisibleIncludingHead() throws Exception {
+ public void uploadPackSubsetOfBranchesVisibleIncludingHead() throws Exception {
allow(Permission.READ, REGISTERED_USERS, "refs/heads/master");
deny(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
setApiUser(user);
- assertRefs(
+ assertUploadPackRefs(
"HEAD",
r1 + "1",
r1 + "meta",
+ r3 + "1",
+ r3 + "meta",
"refs/heads/master",
"refs/tags/master-tag");
}
@Test
- public void subsetOfBranchesVisibleNotIncludingHead() throws Exception {
+ public void uploadPackSubsetOfBranchesVisibleNotIncludingHead() throws Exception {
deny(Permission.READ, REGISTERED_USERS, "refs/heads/master");
allow(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
setApiUser(user);
- assertRefs(
+ assertUploadPackRefs(
r2 + "1",
r2 + "meta",
+ r4 + "1",
+ r4 + "meta",
"refs/heads/branch",
"refs/tags/branch-tag",
// master branch is not visible but master-tag is reachable from branch
@@ -208,12 +254,12 @@
}
@Test
- public void subsetOfBranchesVisibleWithEdit() throws Exception {
+ public void uploadPackSubsetOfBranchesVisibleWithEdit() throws Exception {
allow(Permission.READ, REGISTERED_USERS, "refs/heads/master");
deny(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
- Change c = notesFactory.createChecked(db, project, c1).getChange();
- PatchSet ps1 = getPatchSet(new PatchSet.Id(c1, 1));
+ Change c = notesFactory.createChecked(db, project, c1.getId()).getChange();
+ PatchSet ps1 = getPatchSet(new PatchSet.Id(c1.getId(), 1));
// Admin's edit is not visible.
setApiUser(admin);
@@ -223,59 +269,64 @@
setApiUser(user);
editModifier.createEdit(c, ps1);
- assertRefs(
+ assertUploadPackRefs(
"HEAD",
r1 + "1",
r1 + "meta",
+ r3 + "1",
+ r3 + "meta",
"refs/heads/master",
"refs/tags/master-tag",
- "refs/users/01/1000001/edit-" + c1.get() + "/1");
+ "refs/users/01/1000001/edit-" + c1.getId() + "/1");
}
@Test
- public void subsetOfRefsVisibleWithAccessDatabase() throws Exception {
+ public void uploadPackSubsetOfRefsVisibleWithAccessDatabase() throws Exception {
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
try {
deny(Permission.READ, REGISTERED_USERS, "refs/heads/master");
allow(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
- Change c = notesFactory.createChecked(db, project, c1).getChange();
- PatchSet ps1 = getPatchSet(new PatchSet.Id(c1, 1));
+ PatchSet ps1 = getPatchSet(new PatchSet.Id(c1.getId(), 1));
setApiUser(admin);
- editModifier.createEdit(c, ps1);
+ editModifier.createEdit(c1.change(), ps1);
setApiUser(user);
- assertRefs(
+ assertUploadPackRefs(
// Change 1 is visible due to accessDatabase capability, even though
// refs/heads/master is not.
r1 + "1",
r1 + "meta",
r2 + "1",
r2 + "meta",
+ r3 + "1",
+ r3 + "meta",
+ r4 + "1",
+ r4 + "meta",
"refs/heads/branch",
"refs/tags/branch-tag",
// See comment in subsetOfBranchesVisibleNotIncludingHead.
"refs/tags/master-tag",
// All edits are visible due to accessDatabase capability.
- "refs/users/00/1000000/edit-" + c1.get() + "/1");
+ "refs/users/00/1000000/edit-" + c1.getId() + "/1");
} finally {
removeGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
}
}
@Test
- public void draftRefs() throws Exception {
+ public void uploadPackDraftRefs() throws Exception {
allow(Permission.READ, REGISTERED_USERS, "refs/heads/*");
PushOneCommit.Result br = pushFactory.create(db, admin.getIdent(), testRepo)
.to("refs/drafts/master");
br.assertOkStatus();
- Change.Id c3 = br.getChange().getId();
- String r3 = changeRefPrefix(c3);
+ Change.Id c5 = br.getChange().getId();
+ String r5 = changeRefPrefix(c5);
- // Only admin can see admin's draft change.
+ // Only admin can see admin's draft change (5).
setApiUser(admin);
- assertRefs(
+ assertUploadPackRefs(
"HEAD",
r1 + "1",
r1 + "meta",
@@ -283,6 +334,10 @@
r2 + "meta",
r3 + "1",
r3 + "meta",
+ r4 + "1",
+ r4 + "meta",
+ r5 + "1",
+ r5 + "meta",
"refs/heads/branch",
"refs/heads/master",
RefNames.REFS_CONFIG,
@@ -291,12 +346,16 @@
// user can't.
setApiUser(user);
- assertRefs(
+ assertUploadPackRefs(
"HEAD",
r1 + "1",
r1 + "meta",
r2 + "1",
r2 + "meta",
+ r3 + "1",
+ r3 + "meta",
+ r4 + "1",
+ r4 + "meta",
"refs/heads/branch",
"refs/heads/master",
"refs/tags/branch-tag",
@@ -304,7 +363,7 @@
}
@Test
- public void noSearchingChangeCacheImpl() throws Exception {
+ public void uploadPackNoSearchingChangeCacheImpl() throws Exception {
allow(Permission.READ, REGISTERED_USERS, "refs/heads/*");
setApiUser(user);
@@ -320,6 +379,10 @@
r1 + "meta",
r2 + "1",
r2 + "meta",
+ r3 + "1",
+ r3 + "meta",
+ r4 + "1",
+ r4 + "meta",
"refs/heads/branch",
"refs/heads/master",
"refs/tags/branch-tag",
@@ -328,7 +391,7 @@
}
@Test
- public void sequencesWithAccessDatabase() throws Exception {
+ public void uploadPackSequencesWithAccessDatabase() throws Exception {
assume().that(notesMigration.readChangeSequence()).isTrue();
try (Repository repo = repoManager.openRepository(allProjects)) {
setApiUser(user);
@@ -348,6 +411,82 @@
}
}
+ @Test
+ public void receivePackListsOpenChangesAsAdditionalHaves() throws Exception {
+ ReceiveCommitsAdvertiseRefsHook.Result r = getReceivePackRefs();
+ assertThat(r.allRefs().keySet()).containsExactly(
+ // meta refs are excluded even when NoteDb is enabled.
+ "HEAD",
+ "refs/heads/branch",
+ "refs/heads/master",
+ "refs/meta/config",
+ "refs/tags/branch-tag",
+ "refs/tags/master-tag");
+ assertThat(r.additionalHaves()).containsExactly(obj(c3, 1), obj(c4, 1));
+ }
+
+ @Test
+ public void receivePackRespectsVisibilityOfOpenChanges() throws Exception {
+ allow(Permission.READ, REGISTERED_USERS, "refs/heads/master");
+ deny(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
+ setApiUser(user);
+
+ assertThat(getReceivePackRefs().additionalHaves())
+ .containsExactly(obj(c3, 1));
+ }
+
+ @Test
+ public void receivePackListsOnlyLatestPatchSet() throws Exception {
+ testRepo.reset(obj(c3, 1));
+ PushOneCommit.Result r = amendChange(c3.change().getKey().get());
+ r.assertOkStatus();
+ c3 = r.getChange();
+ assertThat(getReceivePackRefs().additionalHaves())
+ .containsExactly(obj(c3, 2), obj(c4, 1));
+ }
+
+ @Test
+ public void receivePackOmitsMissingObject() throws Exception {
+ // Use the tactic from ConsistencyCheckerIT to insert a new patch set with a
+ // missing object.
+ String rev = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
+ try (Repository repo = repoManager.openRepository(project)) {
+ TestRepository<?> tr = new TestRepository<>(repo);
+ String subject = "Subject for missing commit";
+ Change c = new Change(c3.change());
+ PatchSet.Id psId = new PatchSet.Id(c3.getId(), 2);
+ c.setCurrentPatchSet(psId, subject, c.getOriginalSubject());
+
+ PatchSet ps = TestChanges.newPatchSet(psId, rev, admin.getId());
+ db.patchSets().insert(Collections.singleton(ps));
+ db.changes().update(Collections.singleton(c));
+
+ if (notesMigration.commitChangeWrites()) {
+ PersonIdent committer = serverIdent.get();
+ PersonIdent author = noteUtil.newIdent(
+ accountCache.get(admin.getId()).getAccount(),
+ committer.getWhen(),
+ committer,
+ anonymousCowardName);
+ tr.branch(RefNames.changeMetaRef(c3.getId()))
+ .commit()
+ .author(author)
+ .committer(committer)
+ .message(
+ "Update patch set " + psId.get() + "\n"
+ + "\n"
+ + "Patch-set: " + psId.get() + "\n"
+ + "Commit: " + rev + "\n"
+ + "Subject: " + subject + "\n")
+ .create();
+ }
+ indexer.index(db, c.getProject(), c.getId());
+ }
+
+ assertThat(getReceivePackRefs().additionalHaves())
+ .containsExactly(obj(c4, 1));
+ }
+
/**
* Assert that refs seen by a non-admin user match expected.
*
@@ -356,7 +495,8 @@
* from the expected list before comparing to the actual results.
* @throws Exception
*/
- private void assertRefs(String... expectedWithMeta) throws Exception {
+ private void assertUploadPackRefs(String... expectedWithMeta)
+ throws Exception {
try (Repository repo = repoManager.openRepository(project)) {
assertRefs(
repo,
@@ -391,6 +531,15 @@
}
}
+ private ReceiveCommitsAdvertiseRefsHook.Result getReceivePackRefs()
+ throws Exception {
+ ReceiveCommitsAdvertiseRefsHook hook =
+ new ReceiveCommitsAdvertiseRefsHook(queryProvider, project);
+ try (Repository repo = repoManager.openRepository(project)) {
+ return hook.advertiseRefs(repo.getAllRefs());
+ }
+ }
+
private ProjectControl projectControl() throws Exception {
return projectControlFactory.controlFor(project, userProvider.get());
}
@@ -402,4 +551,12 @@
projectControlFactory.controlFor(project, userProvider.get()),
db, true);
}
+
+ private static ObjectId obj(ChangeData cd, int psNum) throws Exception {
+ PatchSet.Id psId = new PatchSet.Id(cd.getId(), psNum);
+ PatchSet ps = cd.patchSet(psId);
+ assertWithMessage("%s not found in %s", psId, cd.patchSets()).that(ps)
+ .isNotNull();
+ return ObjectId.fromString(ps.getRevision().get());
+ }
}
diff --git a/gerrit-acceptance-tests/tests.bzl b/gerrit-acceptance-tests/tests.bzl
index 62a99e3..160234f 100644
--- a/gerrit-acceptance-tests/tests.bzl
+++ b/gerrit-acceptance-tests/tests.bzl
@@ -11,7 +11,8 @@
flaky = 0,
deps = [],
labels = [],
- vm_args = ['-Xmx256m']):
+ vm_args = ['-Xmx256m'],
+ **kwargs):
junit_tests(
name = group,
srcs = srcs,
@@ -24,4 +25,5 @@
'slow',
],
jvm_flags = vm_args,
+ **kwargs
)
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java
index 7ae7ef1..2e1bb13 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java
@@ -17,4 +17,5 @@
public class CherryPickInput {
public String message;
public String destination;
+ public Integer parent;
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/DiffPreferencesInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/DiffPreferencesInfo.java
index d246996..3f5fa31 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/DiffPreferencesInfo.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/DiffPreferencesInfo.java
@@ -22,6 +22,9 @@
/** Default tab size. */
public static final int DEFAULT_TAB_SIZE = 8;
+ /** Default font size. */
+ public static final int DEFAULT_FONT_SIZE = 12;
+
/** Default line length. */
public static final int DEFAULT_LINE_LENGTH = 100;
@@ -41,6 +44,7 @@
public Integer context;
public Integer tabSize;
+ public Integer fontSize;
public Integer lineLength;
public Integer cursorBlinkRate;
public Boolean expandAllComments;
@@ -68,6 +72,7 @@
DiffPreferencesInfo i = new DiffPreferencesInfo();
i.context = DEFAULT_CONTEXT;
i.tabSize = DEFAULT_TAB_SIZE;
+ i.fontSize = DEFAULT_FONT_SIZE;
i.lineLength = DEFAULT_LINE_LENGTH;
i.cursorBlinkRate = 0;
i.ignoreWhitespace = Whitespace.IGNORE_NONE;
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ApprovalInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ApprovalInfo.java
index 6d28dbc..d59e813 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ApprovalInfo.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ApprovalInfo.java
@@ -20,6 +20,7 @@
public String tag;
public Integer value;
public Timestamp date;
+ public Boolean postSubmit;
public ApprovalInfo(Integer id) {
super(id);
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java
index cd70143..77c8bb4 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java
@@ -21,7 +21,6 @@
import static java.util.stream.Collectors.toList;
import com.google.common.base.CharMatcher;
-import com.google.common.base.Optional;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.cache.Cache;
@@ -72,6 +71,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.concurrent.ConcurrentMap;
import java.util.function.Predicate;
import java.util.jar.Attributes;
@@ -556,7 +556,7 @@
int d = file.lastIndexOf('.');
return scanner.getEntry(file.substring(0, d) + ".md");
}
- return Optional.absent();
+ return Optional.empty();
}
private void sendMarkdownAsHtml(PluginContentScanner scanner, PluginEntry entry,
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/CatServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/CatServlet.java
index 4047279..42b5e7e 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/CatServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/CatServlet.java
@@ -14,7 +14,6 @@
package com.google.gerrit.httpd.raw;
-import com.google.common.base.Optional;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Change;
@@ -33,6 +32,7 @@
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.Optional;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNoteDb.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNoteDb.java
index 030ac30..755ab1b 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNoteDb.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNoteDb.java
@@ -14,6 +14,7 @@
package com.google.gerrit.pgm;
+import static com.google.common.base.Preconditions.checkArgument;
import static com.google.gerrit.reviewdb.server.ReviewDbUtil.unwrapDb;
import static com.google.gerrit.server.schema.DataSourceProvider.Context.MULTI_USER;
@@ -28,6 +29,7 @@
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
+import com.google.gerrit.common.FormatUtil;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
@@ -42,12 +44,16 @@
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.git.ChainedReceiveCommands;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.index.DummyIndexModule;
import com.google.gerrit.server.index.change.ReindexAfterUpdate;
+import com.google.gerrit.server.notedb.ChangeBundleReader;
+import com.google.gerrit.server.notedb.NoteDbUpdateManager;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder;
+import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder.NoPatchSetsException;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
@@ -57,9 +63,12 @@
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ProgressMonitor;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.TextProgressMonitor;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.kohsuke.args4j.Option;
@@ -67,6 +76,7 @@
import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@@ -107,6 +117,9 @@
private GitRepositoryManager repoManager;
@Inject
+ private NoteDbUpdateManager.Factory updateManagerFactory;
+
+ @Inject
private NotesMigration notesMigration;
@Inject
@@ -115,6 +128,9 @@
@Inject
private WorkQueue workQueue;
+ @Inject
+ private ChangeBundleReader bundleReader;
+
@Override
public int run() throws Exception {
mustHaveValidSite();
@@ -153,7 +169,7 @@
@Override
public Boolean call() {
try (ReviewDb db = unwrapDb(schemaFactory.open())) {
- return rebuilder.rebuildProject(
+ return rebuildProject(
db, changesByProject, project, allUsersRepo);
} catch (Exception e) {
log.error("Error rebuilding project " + project, e);
@@ -257,4 +273,37 @@
return ImmutableMultimap.copyOf(changesByProject);
}
}
+
+ private boolean rebuildProject(ReviewDb db,
+ ImmutableMultimap<Project.NameKey, Change.Id> allChanges,
+ Project.NameKey project, Repository allUsersRepo)
+ throws IOException, OrmException {
+ checkArgument(allChanges.containsKey(project));
+ boolean ok = true;
+ ProgressMonitor pm = new TextProgressMonitor(new PrintWriter(System.out));
+ pm.beginTask(
+ FormatUtil.elide(project.get(), 50), allChanges.get(project).size());
+ try (NoteDbUpdateManager manager = updateManagerFactory.create(project);
+ ObjectInserter allUsersInserter = allUsersRepo.newObjectInserter();
+ RevWalk allUsersRw = new RevWalk(allUsersInserter.newReader())) {
+ manager.setAllUsersRepo(allUsersRepo, allUsersRw, allUsersInserter,
+ new ChainedReceiveCommands(allUsersRepo));
+ for (Change.Id changeId : allChanges.get(project)) {
+ try {
+ rebuilder.buildUpdates(
+ manager, bundleReader.fromReviewDb(db, changeId));
+ } catch (NoPatchSetsException e) {
+ log.warn(e.getMessage());
+ } catch (Throwable t) {
+ log.error("Failed to rebuild change " + changeId, t);
+ ok = false;
+ }
+ pm.update(1);
+ }
+ manager.execute();
+ } finally {
+ pm.endTask();
+ }
+ return ok;
+ }
}
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/VersionedAuthorizedKeysOnInit.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/VersionedAuthorizedKeysOnInit.java
index 6739ce0..e47f23a 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/VersionedAuthorizedKeysOnInit.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/VersionedAuthorizedKeysOnInit.java
@@ -16,7 +16,6 @@
import static com.google.common.base.Preconditions.checkState;
-import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.gerrit.pgm.init.api.InitFlags;
import com.google.gerrit.pgm.init.api.VersionedMetaDataOnInit;
@@ -34,6 +33,7 @@
import java.io.IOException;
import java.util.List;
+import java.util.Optional;
public class VersionedAuthorizedKeysOnInit extends VersionedMetaDataOnInit {
public interface Factory {
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java
index a2becc2..30f2e1d 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java
@@ -100,6 +100,9 @@
@Column(id = 7, notNull = false)
protected Account.Id realAccountId;
+ @Column(id = 8)
+ protected boolean postSubmit;
+
// DELETED: id = 4 (changeOpen)
// DELETED: id = 5 (changeSortKey)
@@ -119,6 +122,7 @@
granted = src.granted;
realAccountId = src.realAccountId;
tag = src.tag;
+ postSubmit = src.postSubmit;
}
public PatchSetApproval(PatchSetApproval src) {
@@ -186,14 +190,24 @@
return tag;
}
+ public void setPostSubmit(boolean postSubmit) {
+ this.postSubmit = postSubmit;
+ }
+
+ public boolean isPostSubmit() {
+ return postSubmit;
+ }
+
@Override
public String toString() {
- return "["
- + key + ": "
- + value
- + ",tag:" + tag
- + ",realAccountId:" + realAccountId
- + ']';
+ StringBuilder sb = new StringBuilder("[")
+ .append(key).append(": ").append(value)
+ .append(",tag:").append(tag)
+ .append(",realAccountId:").append(realAccountId);
+ if (postSubmit) {
+ sb.append(",postSubmit");
+ }
+ return sb.append(']').toString();
}
@Override
@@ -204,7 +218,8 @@
&& Objects.equals(value, p.value)
&& Objects.equals(granted, p.granted)
&& Objects.equals(tag, p.tag)
- && Objects.equals(realAccountId, p.realAccountId);
+ && Objects.equals(realAccountId, p.realAccountId)
+ && postSubmit == p.postSubmit;
}
return false;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeFinder.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeFinder.java
index bc6f732..2cd4df6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeFinder.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeFinder.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server;
-import com.google.common.base.Optional;
import com.google.common.primitives.Ints;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.change.ChangeTriplet;
@@ -30,6 +29,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.Optional;
@Singleton
public class ChangeFinder {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java
index 81ec4eb..d1e0ae5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java
@@ -18,7 +18,6 @@
import static com.google.common.base.Preconditions.checkArgument;
import static java.util.stream.Collectors.toList;
-import com.google.common.base.Optional;
import com.google.common.collect.ComparisonChain;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
@@ -64,6 +63,8 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.Optional;
+import java.util.function.Predicate;
import java.util.stream.StreamSupport;
/**
@@ -153,22 +154,18 @@
public Optional<Comment> get(ReviewDb db, ChangeNotes notes,
Comment.Key key) throws OrmException {
if (!migration.readChanges()) {
- PatchLineComment plc = db.patchComments()
- .get(PatchLineComment.Key.from(notes.getChangeId(), key));
- Comment c = plc != null ? plc.asComment(serverId) : null;
- return Optional.fromNullable(c);
+ return Optional.ofNullable(
+ db.patchComments()
+ .get(PatchLineComment.Key.from(notes.getChangeId(), key)))
+ .map(plc -> plc.asComment(serverId));
}
- for (Comment c : publishedByChange(db, notes)) {
- if (key.equals(c.key)) {
- return Optional.of(c);
- }
+ Predicate<Comment> p = c -> key.equals(c.key);
+ Optional<Comment> c =
+ publishedByChange(db, notes).stream().filter(p).findFirst();
+ if (c.isPresent()) {
+ return c;
}
- for (Comment c : draftByChange(db, notes)) {
- if (key.equals(c.key)) {
- return Optional.of(c);
- }
- }
- return Optional.absent();
+ return draftByChange(db, notes).stream().filter(p).findFirst();
}
public List<Comment> publishedByChange(ReviewDb db, ChangeNotes notes)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ReviewerRecommender.java b/gerrit-server/src/main/java/com/google/gerrit/server/ReviewerRecommender.java
index 7f6460d..af85201 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ReviewerRecommender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ReviewerRecommender.java
@@ -50,8 +50,8 @@
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Iterator;
-import java.util.List;
import java.util.LinkedHashMap;
+import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
@@ -68,6 +68,8 @@
private static final double BASE_REVIEWER_WEIGHT = 10;
private static final double BASE_OWNER_WEIGHT = 1;
private static final double BASE_COMMENT_WEIGHT = 0.5;
+ private static final double[] WEIGHTS = new double[] {
+ BASE_REVIEWER_WEIGHT, BASE_OWNER_WEIGHT, BASE_COMMENT_WEIGHT,};
private static final long PLUGIN_QUERY_TIMEOUT = 500; //ms
private final ChangeQueryBuilder changeQueryBuilder;
@@ -117,8 +119,9 @@
for (DynamicMap.Entry<ReviewerSuggestion> plugin :
reviewerSuggestionPluginMap) {
- tasks.add(() -> plugin.getProvider().get().suggestReviewers(
- changeNotes.getChangeId(), query, reviewerScores.keySet()));
+ tasks.add(() -> plugin.getProvider().get()
+ .suggestReviewers(projectControl.getProject().getNameKey(),
+ changeNotes.getChangeId(), query, reviewerScores.keySet()));
String pluginWeight = config.getString("addReviewer",
plugin.getPluginName() + "-" + plugin.getExportName(), "weight");
if (Strings.isNullOrEmpty(pluginWeight)) {
@@ -154,7 +157,9 @@
}
// Remove change owner
- reviewerScores.remove(changeNotes.getChange().getOwner());
+ if (changeNotes != null) {
+ reviewerScores.remove(changeNotes.getChange().getOwner());
+ }
// Sort results
Stream<Entry<Account.Id, MutableDouble>> sorted =
@@ -247,18 +252,15 @@
Iterator<List<ChangeData>> queryResultIterator = result.iterator();
Iterator<Account.Id> reviewersIterator = reviewers.keySet().iterator();
- double[] weights = new double[]{
- BASE_REVIEWER_WEIGHT, BASE_OWNER_WEIGHT, BASE_COMMENT_WEIGHT};
-
int i = 0;
Account.Id currentId = null;
while (queryResultIterator.hasNext()) {
List<ChangeData> currentResult = queryResultIterator.next();
- if (i % weights.length == 0) {
+ if (i % WEIGHTS.length == 0) {
currentId = reviewersIterator.next();
}
- reviewers.get(currentId).add(weights[i % weights.length] *
+ reviewers.get(currentId).add(WEIGHTS[i % WEIGHTS.length] *
baseWeight * currentResult.size());
i++;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
index 149931d..846b44b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.account;
-import com.google.common.base.Optional;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableSet;
@@ -52,6 +51,7 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ExecutionException;
@@ -252,17 +252,13 @@
if (accountIndexes.getSearchIndex() != null) {
AccountState accountState =
accountQueryProvider.get().oneByExternalId(key.get());
- return accountState != null
- ? Optional.of(accountState.getAccount().getId())
- : Optional.<Account.Id>absent();
+ return Optional.ofNullable(accountState)
+ .map(s -> s.getAccount().getId());
}
try (ReviewDb db = schema.open()) {
- AccountExternalId id = db.accountExternalIds().get(key);
- if (id != null) {
- return Optional.of(id.getAccountId());
- }
- return Optional.absent();
+ return Optional.ofNullable(db.accountExternalIds().get(key))
+ .map(AccountExternalId::getAccountId);
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthorizedKeys.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthorizedKeys.java
index 0e8c051..45dbe60 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthorizedKeys.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthorizedKeys.java
@@ -15,13 +15,13 @@
package com.google.gerrit.server.account;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Optional;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountSshKey;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
+import java.util.Optional;
public class AuthorizedKeys {
public static final String FILE_NAME = "authorized_keys";
@@ -47,7 +47,7 @@
key.setInvalid();
keys.add(Optional.of(key));
} else if (line.startsWith(DELETED_KEY_COMMENT)) {
- keys.add(Optional.<AccountSshKey> absent());
+ keys.add(Optional.empty());
seq++;
} else if (line.startsWith("#")) {
continue;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java
index e5e2f99..a2f4b65 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.account;
-import com.google.common.base.Optional;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -35,6 +34,7 @@
import java.util.Collections;
import java.util.List;
+import java.util.Optional;
import java.util.concurrent.ExecutionException;
/** Tracks group objects in memory for efficient access. */
@@ -130,7 +130,7 @@
return null;
}
try {
- return byName.get(name.get()).orNull();
+ return byName.get(name.get()).orElse(null);
} catch (ExecutionException e) {
log.warn(String.format("Cannot lookup group %s by name", name.get()), e);
return null;
@@ -143,7 +143,7 @@
return null;
}
try {
- return byUUID.get(uuid.get()).orNull();
+ return byUUID.get(uuid.get()).orElse(null);
} catch (ExecutionException e) {
log.warn(String.format("Cannot lookup group %s by name", uuid.get()), e);
return null;
@@ -183,7 +183,7 @@
public Optional<AccountGroup> load(final AccountGroup.Id key)
throws Exception {
try (ReviewDb db = schema.open()) {
- return Optional.fromNullable(db.accountGroups().get(key));
+ return Optional.ofNullable(db.accountGroups().get(key));
}
}
}
@@ -203,9 +203,9 @@
AccountGroup.NameKey key = new AccountGroup.NameKey(name);
AccountGroupName r = db.accountGroupNames().get(key);
if (r != null) {
- return Optional.fromNullable(db.accountGroups().get(r.getId()));
+ return Optional.ofNullable(db.accountGroups().get(r.getId()));
}
- return Optional.absent();
+ return Optional.empty();
}
}
}
@@ -228,7 +228,7 @@
if (r.size() == 1) {
return Optional.of(r.get(0));
} else if (r.size() == 0) {
- return Optional.absent();
+ return Optional.empty();
} else {
throw new OrmDuplicateKeyException("Duplicate group UUID " + uuid);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAuthorizedKeys.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAuthorizedKeys.java
index aa32d27..41ae498 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAuthorizedKeys.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAuthorizedKeys.java
@@ -16,10 +16,9 @@
import static com.google.common.base.Preconditions.checkState;
import static java.util.Comparator.comparing;
+import static java.util.stream.Collectors.toList;
-import com.google.common.base.Optional;
import com.google.common.base.Strings;
-import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
import com.google.gerrit.common.errors.InvalidSshKeyException;
import com.google.gerrit.reviewdb.client.Account;
@@ -46,6 +45,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.Optional;
/**
* 'authorized_keys' file in the refs/users/CD/ABCD branches of the All-Users
@@ -192,7 +192,8 @@
/** Returns all SSH keys. */
private List<AccountSshKey> getKeys() {
checkLoaded();
- return Lists.newArrayList(Optional.presentInstances(keys));
+ return keys.stream().filter(Optional::isPresent).map(Optional::get)
+ .collect(toList());
}
/**
@@ -205,8 +206,7 @@
*/
private AccountSshKey getKey(int seq) {
checkLoaded();
- Optional<AccountSshKey> key = keys.get(seq - 1);
- return key.orNull();
+ return keys.get(seq - 1).orElse(null);
}
/**
@@ -246,7 +246,7 @@
private boolean deleteKey(int seq) {
checkLoaded();
if (seq <= keys.size() && keys.get(seq - 1).isPresent()) {
- keys.set(seq - 1, Optional.<AccountSshKey> absent());
+ keys.set(seq - 1, Optional.empty());
return true;
}
return false;
@@ -279,8 +279,9 @@
*/
public void setKeys(Collection<AccountSshKey> newKeys) {
Ordering<AccountSshKey> o = Ordering.from(comparing(k -> k.getKey().get()));
- keys = new ArrayList<>(Collections.nCopies(o.max(newKeys).getKey().get(),
- Optional.<AccountSshKey> absent()));
+ keys = new ArrayList<>(
+ Collections.nCopies(o.max(newKeys).getKey().get(),
+ Optional.empty()));
for (AccountSshKey key : newKeys) {
keys.set(key.getKey().get() - 1, Optional.of(key));
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/WatchConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/WatchConfig.java
index a3cd0c9..8f96e92 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/WatchConfig.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/WatchConfig.java
@@ -21,7 +21,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Enums;
import com.google.common.base.Joiner;
-import com.google.common.base.Optional;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.collect.ArrayListMultimap;
@@ -319,9 +318,9 @@
if (i + 1 < notifyValue.length() - 2) {
for (String nt : Splitter.on(',').trimResults().splitToList(
notifyValue.substring(i + 1, notifyValue.length() - 1))) {
- Optional<NotifyType> notifyType =
- Enums.getIfPresent(NotifyType.class, nt);
- if (!notifyType.isPresent()) {
+ NotifyType notifyType =
+ Enums.getIfPresent(NotifyType.class, nt).orNull();
+ if (notifyType == null) {
validationErrorSink.error(new ValidationError(WATCH_CONFIG,
String.format(
"Invalid notify type %s in project watch "
@@ -329,7 +328,7 @@
nt, accountId.get(), project, notifyValue)));
continue;
}
- notifyTypes.add(notifyType.get());
+ notifyTypes.add(notifyType);
}
}
return create(filter, notifyTypes);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapModule.java
index eaaafd6..217df2f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapModule.java
@@ -16,7 +16,6 @@
import static java.util.concurrent.TimeUnit.HOURS;
-import com.google.common.base.Optional;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.reviewdb.client.Account;
@@ -27,6 +26,7 @@
import com.google.inject.Scopes;
import com.google.inject.TypeLiteral;
+import java.util.Optional;
import java.util.Set;
public class LdapModule extends CacheModule {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
index 603efe0..4148e7a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
@@ -16,7 +16,6 @@
import static com.google.gerrit.reviewdb.client.AccountExternalId.SCHEME_GERRIT;
-import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
@@ -49,6 +48,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ExecutionException;
@@ -295,7 +295,7 @@
}
try {
Optional<Account.Id> id = usernameCache.get(accountName);
- return id != null ? id.orNull() : null;
+ return id != null ? id.orElse(null) : null;
} catch (ExecutionException e) {
log.warn(String.format("Cannot lookup account %s in LDAP", accountName), e);
return null;
@@ -313,13 +313,10 @@
@Override
public Optional<Account.Id> load(String username) throws Exception {
try (ReviewDb db = schema.open()) {
- final AccountExternalId extId =
- db.accountExternalIds().get(
- new AccountExternalId.Key(SCHEME_GERRIT, username));
- if (extId != null) {
- return Optional.of(extId.getAccountId());
- }
- return Optional.absent();
+ return Optional.ofNullable(
+ db.accountExternalIds().get(
+ new AccountExternalId.Key(SCHEME_GERRIT, username)))
+ .map(AccountExternalId::getAccountId);
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java
index 878cc81..ad07e30 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.change;
-import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.FluentIterable;
import com.google.gerrit.common.Nullable;
@@ -65,6 +64,7 @@
import java.io.IOException;
import java.util.List;
+import java.util.Optional;
@Singleton
public class ChangeEdits implements
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index bf7e6b8..771b049 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.change;
+import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.extensions.client.ListChangesOption.ALL_COMMITS;
import static com.google.gerrit.extensions.client.ListChangesOption.ALL_FILES;
import static com.google.gerrit.extensions.client.ListChangesOption.ALL_REVISIONS;
@@ -40,7 +41,6 @@
import com.google.auto.value.AutoValue;
import com.google.common.base.Joiner;
import com.google.common.base.MoreObjects;
-import com.google.common.base.Optional;
import com.google.common.base.Throwables;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.HashBasedTable;
@@ -137,6 +137,7 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.TreeMap;
@@ -266,7 +267,7 @@
}
public ChangeInfo format(ChangeData cd) throws OrmException {
- return format(cd, Optional.<PatchSet.Id> absent(), true);
+ return format(cd, Optional.empty(), true);
}
private ChangeInfo format(ChangeData cd, Optional<PatchSet.Id> limitToPsId,
@@ -356,7 +357,7 @@
ChangeInfo i = out.get(cd.getId());
if (i == null) {
try {
- i = toChangeInfo(cd, Optional.<PatchSet.Id> absent());
+ i = toChangeInfo(cd, Optional.empty());
} catch (PatchListNotAvailableException | GpgException | OrmException
| IOException | RuntimeException e) {
if (has(CHECK)) {
@@ -695,6 +696,10 @@
private void setAllApprovals(ChangeControl baseCtrl, ChangeData cd,
Map<String, LabelWithStatus> labels) throws OrmException {
+ Change.Status status = cd.change().getStatus();
+ checkState(status.isOpen(),
+ "should not call setAllApprovals on %s change", status);
+
// Include a user in the output for this label if either:
// - They are an explicit reviewer.
// - They ever voted on this change.
@@ -734,6 +739,9 @@
}
tag = psa.getTag();
date = psa.getGranted();
+ if (psa.isPostSubmit()) {
+ log.warn("unexpected post-submit approval on open change: {}", psa);
+ }
} else {
// Either the user cannot vote on this label, or they were added as a
// reviewer but have not responded yet. Explicitly check whether the
@@ -816,6 +824,9 @@
info.value = Integer.valueOf(val);
info.date = psa.getGranted();
info.tag = psa.getTag();
+ if (psa.isPostSubmit()) {
+ info.postSubmit = true;
+ }
}
if (!standard) {
continue;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeTriplet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeTriplet.java
index 7069e6d..fc3e70a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeTriplet.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeTriplet.java
@@ -15,12 +15,13 @@
package com.google.gerrit.server.change;
import com.google.auto.value.AutoValue;
-import com.google.common.base.Optional;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
+import java.util.Optional;
+
@AutoValue
public abstract class ChangeTriplet {
public static String format(Change change) {
@@ -44,7 +45,7 @@
int t2 = triplet.lastIndexOf('~');
int t1 = triplet.lastIndexOf('~', t2 - 1);
if (t1 < 0 || t2 < 0) {
- return Optional.absent();
+ return Optional.empty();
}
String project = Url.decode(triplet.substring(0, t1));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java
index 1a063f4..b5eb193 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java
@@ -60,10 +60,12 @@
public ChangeInfo apply(RevisionResource revision, CherryPickInput input)
throws OrmException, IOException, UpdateException, RestApiException {
final ChangeControl control = revision.getControl();
+ int parent = input.parent == null ? 1 : input.parent;
if (input.message == null || input.message.trim().isEmpty()) {
throw new BadRequestException("message must be non-empty");
- } else if (input.destination == null || input.destination.trim().isEmpty()) {
+ } else if (input.destination == null
+ || input.destination.trim().isEmpty()) {
throw new BadRequestException("destination must be non-empty");
}
@@ -91,7 +93,7 @@
Change.Id cherryPickedChangeId =
cherryPickChange.cherryPick(revision.getChange(),
revision.getPatchSet(), input.message, refName,
- refControl);
+ refControl, parent);
return json.create(ChangeJson.NO_OPTIONS).format(revision.getProject(),
cherryPickedChangeId);
} catch (InvalidChangeOperationException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
index 248acd3..1a18357 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
@@ -114,8 +114,8 @@
}
public Change.Id cherryPick(Change change, PatchSet patch,
- final String message, final String ref,
- final RefControl refControl) throws NoSuchChangeException,
+ final String message, final String ref, final RefControl refControl,
+ int parent) throws NoSuchChangeException,
OrmException, MissingObjectException,
IncorrectObjectTypeException, IOException,
InvalidChangeOperationException, IntegrationException, UpdateException,
@@ -147,6 +147,13 @@
CodeReviewCommit commitToCherryPick =
revWalk.parseCommit(ObjectId.fromString(patch.getRevision().get()));
+ if (parent <= 0 || parent > commitToCherryPick.getParentCount()) {
+ throw new InvalidChangeOperationException(String.format(
+ "Cherry Pick: Parent %s does not exist. Please specify a parent in"
+ + " range [1, %s].",
+ parent, commitToCherryPick.getParentCount()));
+ }
+
Timestamp now = TimeUtil.nowTs();
PersonIdent committerIdent =
identifiedUser.newCommitterIdent(now, serverTimeZone);
@@ -160,10 +167,12 @@
CodeReviewCommit cherryPickCommit;
try {
- ProjectState projectState = refControl.getProjectControl().getProjectState();
- cherryPickCommit =
- mergeUtilFactory.create(projectState).createCherryPickFromCommit(git, oi, mergeTip,
- commitToCherryPick, committerIdent, commitMessage, revWalk);
+ ProjectState projectState = refControl.getProjectControl()
+ .getProjectState();
+ cherryPickCommit = mergeUtilFactory.create(projectState)
+ .createCherryPickFromCommit(git, oi, mergeTip,
+ commitToCherryPick, committerIdent, commitMessage, revWalk,
+ parent - 1);
Change.Key changeKey;
final List<String> idList = cherryPickCommit.getFooterLines(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeEdit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeEdit.java
index 7c1e959..604b615 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeEdit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeEdit.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.change;
-import com.google.common.base.Optional;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
@@ -27,6 +26,7 @@
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.Optional;
@Singleton
public class DeleteChangeEdit implements RestModifyView<ChangeResource, Input> {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftComment.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftComment.java
index 9a4c02d..37930dd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftComment.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftComment.java
@@ -16,7 +16,6 @@
import static com.google.gerrit.server.CommentsUtil.setCommentRevId;
-import com.google.common.base.Optional;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -39,6 +38,7 @@
import com.google.inject.Singleton;
import java.util.Collections;
+import java.util.Optional;
@Singleton
public class DeleteDraftComment
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetAssignee.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetAssignee.java
index d57107b..5ad259b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetAssignee.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetAssignee.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.change;
-import com.google.common.base.Optional;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -25,6 +24,8 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import java.util.Optional;
+
@Singleton
public class GetAssignee implements RestReadView<ChangeResource> {
private final AccountInfoCacheFactory.Factory accountInfo;
@@ -36,9 +37,8 @@
@Override
public Response<AccountInfo> apply(ChangeResource rsrc) throws OrmException {
-
Optional<Account.Id> assignee =
- Optional.fromNullable(rsrc.getChange().getAssignee());
+ Optional.ofNullable(rsrc.getChange().getAssignee());
if (assignee.isPresent()) {
Account account = accountInfo.create().get(assignee.get());
return Response.ok(AccountJson.toAccountInfo(account));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index 9210d2b..92617f3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -15,9 +15,11 @@
package com.google.gerrit.server.change;
import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.server.CommentsUtil.setCommentRevId;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toSet;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
@@ -55,6 +57,7 @@
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.extensions.restapi.Url;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.LabelId;
@@ -266,6 +269,10 @@
continue;
}
+ if (caller.getUser().isInternalUser()) {
+ continue;
+ }
+
PermissionRange r = caller.getRange(Permission.forLabelAs(type.getName()));
if (r == null || r.isEmpty() || !r.contains(ent.getValue())) {
throw new AuthException(String.format(
@@ -795,10 +802,7 @@
}
}
- if ((!del.isEmpty() || !ups.isEmpty())
- && ctx.getChange().getStatus().isClosed()) {
- throw new ResourceConflictException("change is closed");
- }
+ validatePostSubmitLabels(ctx, labelTypes, previous, ups, del);
// Return early if user is not a reviewer and not posting any labels.
// This allows us to preserve their CC status.
@@ -813,6 +817,49 @@
return !del.isEmpty() || !ups.isEmpty();
}
+ private void validatePostSubmitLabels(ChangeContext ctx,
+ LabelTypes labelTypes, Map<String, Short> previous,
+ List<PatchSetApproval> ups, List<PatchSetApproval> del)
+ throws ResourceConflictException {
+ if (ctx.getChange().getStatus().isOpen()) {
+ return; // Not closed, nothing to validate.
+ } else if (del.isEmpty() && ups.isEmpty()) {
+ return; // No new votes.
+ } else if (ctx.getChange().getStatus() != Change.Status.MERGED) {
+ throw new ResourceConflictException("change is closed");
+ }
+
+ // Disallow reducing votes on any labels post-submit. This assumes the
+ // high values were broadly necessary to submit, so reducing them would
+ // make it possible to take a merged change and make it no longer
+ // submittable.
+ List<PatchSetApproval> reduced = new ArrayList<>(ups.size() + del.size());
+ reduced.addAll(del);
+ for (PatchSetApproval psa : ups) {
+ LabelType lt = checkNotNull(labelTypes.byLabel(psa.getLabel()));
+ String normName = lt.getName();
+ Short prev = previous.get(normName);
+ if (prev == null) {
+ continue;
+ }
+ checkState(prev != psa.getValue()); // Should be filtered out above.
+ if (prev > psa.getValue()) {
+ reduced.add(psa);
+ } else {
+ // Set postSubmit bit in ReviewDb; not required for NoteDb, which sets
+ // it automatically.
+ psa.setPostSubmit(true);
+ }
+ }
+
+ if (!reduced.isEmpty()) {
+ throw new ResourceConflictException(
+ "Cannot reduce vote on labels for closed change: "
+ + reduced.stream().map(p -> p.getLabel()).distinct().sorted()
+ .collect(joining(", ")));
+ }
+ }
+
private void forceCallerAsReviewer(ChangeContext ctx,
Map<String, PatchSetApproval> current, List<PatchSetApproval> ups,
List<PatchSetApproval> del) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishChangeEdit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishChangeEdit.java
index 76ff15e..6875287 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishChangeEdit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishChangeEdit.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.change;
-import com.google.common.base.Optional;
import com.google.gerrit.common.data.Capable;
import com.google.gerrit.extensions.api.changes.PublishChangeEditInput;
import com.google.gerrit.extensions.registration.DynamicMap;
@@ -37,6 +36,7 @@
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.Optional;
@Singleton
public class PublishChangeEdit implements
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java
index 4298937..5002436 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutAssignee.java
@@ -14,12 +14,15 @@
package com.google.gerrit.server.change;
+import com.google.common.base.Strings;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AssigneeInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
@@ -59,10 +62,17 @@
@Override
public Response<AccountInfo> apply(ChangeResource rsrc, AssigneeInput input)
throws RestApiException, UpdateException, OrmException, IOException {
+ if (!rsrc.getControl().canEditAssignee()) {
+ throw new AuthException("Changing Assignee not permitted");
+ }
+ if (Strings.isNullOrEmpty(input.assignee)) {
+ throw new BadRequestException("missing assignee field");
+ }
+
try (BatchUpdate bu = batchUpdateFactory.create(db.get(),
rsrc.getChange().getProject(), rsrc.getControl().getUser(),
TimeUtil.nowTs())) {
- SetAssigneeOp op = assigneeFactory.create(input);
+ SetAssigneeOp op = assigneeFactory.create(input.assignee);
bu.addOp(rsrc.getId(), op);
PostReviewers.Addition reviewersAddition =
@@ -70,7 +80,6 @@
bu.addOp(rsrc.getId(), reviewersAddition.op);
bu.execute();
- reviewersAddition.gatherResults();
return Response.ok(AccountJson.toAccountInfo(op.getNewAssignee()));
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraftComment.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraftComment.java
index 91ad849..0742c6d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraftComment.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraftComment.java
@@ -16,7 +16,6 @@
import static com.google.gerrit.server.CommentsUtil.setCommentRevId;
-import com.google.common.base.Optional;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.common.CommentInfo;
@@ -44,6 +43,7 @@
import java.sql.Timestamp;
import java.util.Collections;
+import java.util.Optional;
@Singleton
public class PutDraftComment implements RestModifyView<DraftCommentResource, DraftInput> {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeEdit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeEdit.java
index baa0990..ac1b770 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeEdit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeEdit.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.change;
-import com.google.common.base.Optional;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AcceptsPost;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -39,6 +38,7 @@
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.Optional;
@Singleton
public class RebaseChangeEdit implements
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerSuggestion.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerSuggestion.java
index 379d2bd..6affd9f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerSuggestion.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerSuggestion.java
@@ -14,9 +14,11 @@
package com.google.gerrit.server.change;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.annotations.ExtensionPoint;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
import java.util.Set;
@@ -30,12 +32,14 @@
/**
* Reviewer suggestion.
*
- * @param changeId The changeId that the suggestion is for.
- * @param query The query as typed by the user. Can be an empty string.
- * @param candidates A set of candidates for the ranking.
+ * @param project The name key of the project the suggestion is for.
+ * @param changeId The changeId that the suggestion is for. Can be an {@code null}.
+ * @param query The query as typed by the user. Can be an {@code null}.
+ * @param candidates A set of candidates for the ranking. Can be empty.
* @return Set of suggested reviewers as a tuple of account id and score.
* The account ids listed here don't have to be a part of candidates.
*/
- Set<SuggestedReviewer> suggestReviewers(
- Change.Id changeId, String query, Set<Account.Id> candidates);
+ Set<SuggestedReviewer> suggestReviewers(Project.NameKey project,
+ @Nullable Change.Id changeId, @Nullable String query,
+ Set<Account.Id> candidates);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RevisionResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RevisionResource.java
index a8fd013..152563b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/RevisionResource.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/RevisionResource.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.change;
-import com.google.common.base.Optional;
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestResource.HasETag;
import com.google.gerrit.extensions.restapi.RestView;
@@ -28,6 +27,8 @@
import com.google.gerrit.server.project.ChangeControl;
import com.google.inject.TypeLiteral;
+import java.util.Optional;
+
public class RevisionResource implements RestResource, HasETag {
public static final TypeLiteral<RestView<RevisionResource>> REVISION_KIND =
new TypeLiteral<RestView<RevisionResource>>() {};
@@ -38,7 +39,7 @@
private boolean cacheable = true;
public RevisionResource(ChangeResource change, PatchSet ps) {
- this(change, ps, Optional.<ChangeEdit> absent());
+ this(change, ps, Optional.empty());
}
public RevisionResource(ChangeResource change, PatchSet ps,
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revisions.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revisions.java
index 30a09cf..4572994 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revisions.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revisions.java
@@ -15,7 +15,6 @@
package com.google.gerrit.server.change;
import com.google.common.base.Joiner;
-import com.google.common.base.Optional;
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -38,6 +37,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.Optional;
@Singleton
public class Revisions implements ChildCollection<ChangeResource, RevisionResource> {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SetAssigneeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SetAssigneeOp.java
index dba5358..43c1dee 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/SetAssigneeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SetAssigneeOp.java
@@ -14,11 +14,11 @@
package com.google.gerrit.server.change;
-import com.google.common.base.Optional;
-import com.google.gerrit.extensions.api.changes.AssigneeInput;
+import static com.google.common.base.Preconditions.checkNotNull;
+
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Account;
@@ -39,16 +39,18 @@
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
+import java.util.Optional;
+
public class SetAssigneeOp extends BatchUpdate.Op {
public interface Factory {
- SetAssigneeOp create(AssigneeInput input);
+ SetAssigneeOp create(String assignee);
}
private final AccountsCollection accounts;
private final ChangeMessagesUtil cmUtil;
private final AccountInfoCacheFactory.Factory accountInfosFactory;
private final DynamicSet<AssigneeValidationListener> validationListeners;
- private final AssigneeInput input;
+ private final String assignee;
private final String anonymousCowardName;
private final AssigneeChanged assigneeChanged;
@@ -63,37 +65,28 @@
DynamicSet<AssigneeValidationListener> validationListeners,
AssigneeChanged assigneeChanged,
@AnonymousCowardName String anonymousCowardName,
- @Assisted AssigneeInput input) {
+ @Assisted String assignee) {
this.accounts = accounts;
this.cmUtil = cmUtil;
this.accountInfosFactory = accountInfosFactory;
this.validationListeners = validationListeners;
this.assigneeChanged = assigneeChanged;
this.anonymousCowardName = anonymousCowardName;
- this.input = input;
+ this.assignee = checkNotNull(assignee);
}
@Override
public boolean updateChange(BatchUpdate.ChangeContext ctx)
throws OrmException, RestApiException {
- if (!ctx.getControl().canEditAssignee()) {
- throw new AuthException("Changing Assignee not permitted");
- }
change = ctx.getChange();
ChangeUpdate update = ctx.getUpdate(change.currentPatchSetId());
Optional<Account.Id> oldAssigneeId =
- Optional.fromNullable(change.getAssignee());
- if (input.assignee == null) {
- if (oldAssigneeId.isPresent()) {
- throw new BadRequestException("Cannot set Assignee to empty");
- }
- return false;
- }
+ Optional.ofNullable(change.getAssignee());
oldAssignee = null;
if (oldAssigneeId.isPresent()) {
oldAssignee = accountInfosFactory.create().get(oldAssigneeId.get());
}
- IdentifiedUser newAssigneeUser = accounts.parse(input.assignee);
+ IdentifiedUser newAssigneeUser = accounts.parse(assignee);
if (oldAssigneeId.isPresent() &&
oldAssigneeId.get().equals(newAssigneeUser.getAccountId())) {
newAssignee = oldAssignee;
@@ -101,20 +94,20 @@
}
if (!newAssigneeUser.getAccount().isActive()) {
throw new UnprocessableEntityException(String.format(
- "Account of %s is not active", input.assignee));
+ "Account of %s is not active", assignee));
}
if (!ctx.getControl().forUser(newAssigneeUser).isRefVisible()) {
throw new AuthException(String.format(
"Change %s is not visible to %s.",
change.getChangeId(),
- input.assignee));
+ assignee));
}
try {
for (AssigneeValidationListener validator : validationListeners) {
validator.validateAssignee(change, newAssigneeUser.getAccount());
}
} catch (ValidationException e) {
- throw new BadRequestException(e.getMessage());
+ throw new ResourceConflictException(e.getMessage());
}
// notedb
update.setAssignee(newAssigneeUser.getAccountId());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java
index cf6d307..2b1c97c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java
@@ -25,6 +25,7 @@
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.server.ChangeMessagesUtil;
@@ -85,9 +86,11 @@
@Override
public boolean updateChange(ChangeContext ctx)
- throws AuthException, BadRequestException, OrmException, IOException {
+ throws AuthException, BadRequestException, MethodNotAllowedException,
+ OrmException, IOException {
if (!notesMigration.readChanges()) {
- throw new BadRequestException("Cannot add hashtags; NoteDb is disabled");
+ throw new MethodNotAllowedException(
+ "Cannot add hashtags; NoteDb is disabled");
}
if (input == null
|| (input.add == null && input.remove == null)) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GetServerInfo.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GetServerInfo.java
index 447d1be..54fc3fa 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GetServerInfo.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GetServerInfo.java
@@ -17,7 +17,6 @@
import static java.util.stream.Collectors.toList;
import com.google.common.base.CharMatcher;
-import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.ContributorAgreement;
@@ -62,6 +61,7 @@
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Map;
+import java.util.Optional;
import java.util.concurrent.TimeUnit;
public class GetServerInfo implements RestReadView<ConfigResource> {
@@ -215,11 +215,11 @@
.hasField(ChangeField.ASSIGNEE));
info.largeChange = cfg.getInt("change", "largeChange", 500);
info.replyTooltip =
- Optional.fromNullable(cfg.getString("change", null, "replyTooltip"))
- .or("Reply and score") + " (Shortcut: a)";
+ Optional.ofNullable(cfg.getString("change", null, "replyTooltip"))
+ .orElse("Reply and score") + " (Shortcut: a)";
info.replyLabel =
- Optional.fromNullable(cfg.getString("change", null, "replyLabel"))
- .or("Reply") + "\u2026";
+ Optional.ofNullable(cfg.getString("change", null, "replyLabel"))
+ .orElse("Reply") + "\u2026";
info.updateDelay = (int) ConfigUtil.getTimeUnit(
cfg, "change", null, "updateDelay", 30, TimeUnit.SECONDS);
info.submitWholeTopic = Submit.wholeTopicEnabled(cfg);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/PluginConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/PluginConfig.java
index c7390d7..d8485fe 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/PluginConfig.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/PluginConfig.java
@@ -151,6 +151,6 @@
}
public Set<String> getNames() {
- return cfg.getNames(PLUGIN, pluginName);
+ return cfg.getNames(PLUGIN, pluginName, true);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java
index 8fca453..6beff6c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java
@@ -16,7 +16,6 @@
import static com.google.common.base.Preconditions.checkArgument;
-import com.google.common.base.Optional;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.client.ChangeKind;
@@ -56,6 +55,7 @@
import org.eclipse.jgit.revwalk.RevWalk;
import java.io.IOException;
+import java.util.Optional;
/**
* Utility functions to manipulate change edits.
@@ -144,7 +144,7 @@
}
Ref ref = repo.getRefDatabase().firstExactRef(refNames);
if (ref == null) {
- return Optional.absent();
+ return Optional.empty();
}
try (RevWalk rw = new RevWalk(repo)) {
RevCommit commit = rw.parseCommit(ref.getObjectId());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/AgreementSignup.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/AgreementSignup.java
index 2d187c0..c910a7a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/AgreementSignup.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/AgreementSignup.java
@@ -20,13 +20,7 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.inject.Inject;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
public class AgreementSignup {
- private static final Logger log =
- LoggerFactory.getLogger(AgreementSignup.class);
-
private final DynamicSet<AgreementSignupListener> listeners;
private final EventUtil util;
@@ -46,7 +40,7 @@
try {
l.onAgreementSignup(event);
} catch (Exception e) {
- util.logEventListenerError(log, e);
+ util.logEventListenerError(this, l, e);
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/AssigneeChanged.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/AssigneeChanged.java
index 2234556..53d837f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/AssigneeChanged.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/AssigneeChanged.java
@@ -43,31 +43,24 @@
this.util = util;
}
- public void fire(ChangeInfo change, AccountInfo editor, AccountInfo oldAssignee,
- Timestamp when) {
- if (!listeners.iterator().hasNext()) {
- return;
- }
- Event event = new Event(change, editor, oldAssignee, when);
- for (AssigneeChangedListener l : listeners) {
- try {
- l.onAssigneeChanged(event);
- } catch (Exception e) {
- log.warn("Error in event listener", e);
- }
- }
- }
-
public void fire(Change change, Account account, Account oldAssignee,
Timestamp when) {
if (!listeners.iterator().hasNext()) {
return;
}
try {
- fire(util.changeInfo(change),
+ Event event = new Event(
+ util.changeInfo(change),
util.accountInfo(account),
util.accountInfo(oldAssignee),
when);
+ for (AssigneeChangedListener l : listeners) {
+ try {
+ l.onAssigneeChanged(event);
+ } catch (Exception e) {
+ util.logEventListenerError(event, l, e);
+ }
+ }
} catch (OrmException e) {
log.error("Couldn't fire event", e);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java
index ad5bb98..5a7aec2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeAbandoned.java
@@ -63,7 +63,7 @@
try {
l.onChangeAbandoned(event);
} catch (Exception e) {
- util.logEventListenerError(log, e);
+ util.logEventListenerError(this, l, e);
}
}
} catch (PatchListNotAvailableException | GpgException | IOException
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeMerged.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeMerged.java
index ebfe7d6..8b4a6a0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeMerged.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeMerged.java
@@ -63,7 +63,7 @@
try {
l.onChangeMerged(event);
} catch (Exception e) {
- util.logEventListenerError(log, e);
+ util.logEventListenerError(this, l, e);
}
}
} catch (PatchListNotAvailableException | GpgException | IOException
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeRestored.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeRestored.java
index 6087070..1d2682a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeRestored.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeRestored.java
@@ -63,7 +63,7 @@
try {
l.onChangeRestored(event);
} catch (Exception e) {
- util.logEventListenerError(log, e);
+ util.logEventListenerError(this, l, e);
}
}
} catch (PatchListNotAvailableException | GpgException | IOException
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeReverted.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeReverted.java
index c4f2fc6..d963a47 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeReverted.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeReverted.java
@@ -52,7 +52,7 @@
try {
l.onChangeReverted(event);
} catch (Exception e) {
- util.logEventListenerError(log, e);
+ util.logEventListenerError(this, l, e);
}
}
} catch (OrmException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/CommentAdded.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/CommentAdded.java
index 35f2c7d..f1bb50a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/CommentAdded.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/CommentAdded.java
@@ -68,7 +68,7 @@
try {
l.onCommentAdded(event);
} catch (Exception e) {
- util.logEventListenerError(log, e);
+ util.logEventListenerError(this, l, e);
}
}
} catch (PatchListNotAvailableException | GpgException | IOException
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/DraftPublished.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/DraftPublished.java
index b5dba77..4f6d298 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/DraftPublished.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/DraftPublished.java
@@ -60,7 +60,7 @@
try {
l.onDraftPublished(event);
} catch (Exception e) {
- util.logEventListenerError(log, e);
+ util.logEventListenerError(this, l, e);
}
}
} catch (PatchListNotAvailableException | GpgException | IOException
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java
index c5eaa0c..682d9ec 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/EventUtil.java
@@ -35,6 +35,7 @@
import com.google.inject.Provider;
import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.sql.Timestamp;
@@ -43,6 +44,7 @@
import java.util.Map;
public class EventUtil {
+ private static final Logger log = LoggerFactory.getLogger(EventUtil.class);
private final ChangeData.Factory changeDataFactory;
private final Provider<ReviewDb> db;
@@ -54,8 +56,9 @@
Provider<ReviewDb> db) {
this.changeDataFactory = changeDataFactory;
this.db = db;
- this.changeJson = changeJsonFactory.create(
- EnumSet.allOf(ListChangesOption.class));
+ EnumSet<ListChangesOption> opts = EnumSet.allOf(ListChangesOption.class);
+ opts.remove(ListChangesOption.CHECK);
+ this.changeJson = changeJsonFactory.create(opts);
}
public ChangeInfo changeInfo(Change change) throws OrmException {
@@ -95,11 +98,16 @@
return result;
}
- public void logEventListenerError(Logger log, Exception error) {
+ public void logEventListenerError(Object event, Object listener,
+ Exception error) {
if (log.isDebugEnabled()) {
- log.debug("Error in event listener", error);
+ log.debug(String.format(
+ "Error in event listener %s for event %s",
+ listener.getClass().getName(), event.getClass().getName()), error);
} else {
- log.warn("Error in event listener: {}", error.getMessage());
+ log.warn("Error in listener {} for event {}: {}",
+ listener.getClass().getName(), event.getClass().getName(),
+ error.getMessage());
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/GitReferenceUpdated.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/GitReferenceUpdated.java
index 63fa391..381dced 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/GitReferenceUpdated.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/GitReferenceUpdated.java
@@ -26,13 +26,8 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.transport.ReceiveCommand;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class GitReferenceUpdated {
- private static final Logger log = LoggerFactory
- .getLogger(GitReferenceUpdated.class);
-
public static final GitReferenceUpdated DISABLED = new GitReferenceUpdated() {
@Override
public void fire(Project.NameKey project, RefUpdate refUpdate,
@@ -123,7 +118,7 @@
try {
l.onGitReferenceUpdated(event);
} catch (Exception e) {
- util.logEventListenerError(log, e);
+ util.logEventListenerError(this, l, e);
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/HashtagsEdited.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/HashtagsEdited.java
index c9cf5f1..233a89e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/HashtagsEdited.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/HashtagsEdited.java
@@ -62,7 +62,7 @@
try {
l.onHashtagsEdited(event);
} catch (Exception e) {
- util.logEventListenerError(log, e);
+ util.logEventListenerError(this, l, e);
}
}
} catch (OrmException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerAdded.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerAdded.java
index 337982f..8860a42 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerAdded.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerAdded.java
@@ -67,7 +67,7 @@
try {
l.onReviewersAdded(event);
} catch (Exception e) {
- util.logEventListenerError(log, e);
+ util.logEventListenerError(this, l, e);
}
}
} catch (PatchListNotAvailableException | GpgException | IOException
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java
index 00950b7..4bc4764 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java
@@ -71,7 +71,7 @@
try {
listener.onReviewerDeleted(event);
} catch (Exception e) {
- util.logEventListenerError(log, e);
+ util.logEventListenerError(this, listener, e);
}
}
} catch (PatchListNotAvailableException | GpgException | IOException
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/RevisionCreated.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/RevisionCreated.java
index c7d1ef6..7f03c63 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/RevisionCreated.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/RevisionCreated.java
@@ -63,7 +63,7 @@
try {
l.onRevisionCreated(event);
} catch (Exception e) {
- util.logEventListenerError(log, e);
+ util.logEventListenerError(this, l, e);
}
}
} catch ( PatchListNotAvailableException | GpgException | IOException
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/TopicEdited.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/TopicEdited.java
index 837d730..2e583a8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/TopicEdited.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/TopicEdited.java
@@ -58,7 +58,7 @@
try {
l.onTopicEdited(event);
} catch (Exception e) {
- util.logEventListenerError(log, e);
+ util.logEventListenerError(this, l, e);
}
}
} catch (OrmException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/VoteDeleted.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/VoteDeleted.java
index 9d1e7f2..e421ea6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/VoteDeleted.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/VoteDeleted.java
@@ -71,7 +71,7 @@
try {
l.onVoteDeleted(event);
} catch (Exception e) {
- util.logEventListenerError(log, e);
+ util.logEventListenerError(this, l, e);
}
}
} catch (PatchListNotAvailableException | GpgException | IOException
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ChainedReceiveCommands.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChainedReceiveCommands.java
index cfbaa41..a3b30d1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ChainedReceiveCommands.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChainedReceiveCommands.java
@@ -17,8 +17,6 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
-import com.google.common.base.Optional;
-
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
@@ -28,6 +26,7 @@
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Map;
+import java.util.Optional;
/**
* Collection of {@link ReceiveCommand}s that supports multiple updates per ref.
@@ -96,7 +95,7 @@
if (cmd != null) {
return !cmd.getNewId().equals(ObjectId.zeroId())
? Optional.of(cmd.getNewId())
- : Optional.<ObjectId>absent();
+ : Optional.empty();
}
return refCache.get(refName);
}
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 71b29a1..5aaf41a 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
@@ -21,13 +21,11 @@
import com.google.auto.value.AutoValue;
import com.google.common.base.Joiner;
-import com.google.common.base.Optional;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
-import com.google.common.collect.Iterables;
import com.google.common.collect.Multimap;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.Sets;
@@ -83,6 +81,7 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
/**
@@ -256,9 +255,10 @@
private static Optional<SubmitRecord> findOkRecord(
Collection<SubmitRecord> in) {
if (in == null) {
- return Optional.absent();
+ return Optional.empty();
}
- return Iterables.tryFind(in, r -> r.status == SubmitRecord.Status.OK);
+ return in.stream().filter(r -> r.status == SubmitRecord.Status.OK)
+ .findAny();
}
public static void checkSubmitRule(ChangeData cd)
@@ -479,7 +479,9 @@
BatchUpdate.execute(orm.batchUpdates(allProjects),
new SubmitStrategyListener(submitInput, strategies, commits),
submissionId, dryrun);
- } catch (UpdateException | SubmoduleException e) {
+ } catch (SubmoduleException e) {
+ throw new IntegrationException(e);
+ } catch (UpdateException e) {
// BatchUpdate may have inadvertently wrapped an IntegrationException
// thrown by some legacy SubmitStrategyOp code that intended the error
// message to be user-visible. Copy the message from the wrapped
@@ -491,8 +493,7 @@
if (e.getCause() instanceof IntegrationException) {
msg = e.getCause().getMessage();
} else {
- msg = "Error submitting change" + (cs.size() != 1 ? "s" : "") + ": \n"
- + e.getMessage();
+ msg = "Error submitting change" + (cs.size() != 1 ? "s" : "");
}
throw new IntegrationException(msg, e);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java
index 108572f..751933c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java
@@ -18,7 +18,6 @@
import static com.google.common.base.Preconditions.checkState;
import com.google.auto.value.AutoValue;
-import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
@@ -61,6 +60,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
/**
@@ -310,8 +310,8 @@
if (head == null) {
Ref ref = or.repo.getRefDatabase().exactRef(b.get());
head = ref != null
- ? Optional.<RevCommit>of(or.rw.parseCommit(ref.getObjectId()))
- : Optional.<RevCommit>absent();
+ ? Optional.of(or.rw.parseCommit(ref.getObjectId()))
+ : Optional.empty();
heads.put(b, head);
}
if (head.isPresent()) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
index 0667e14..90edfb1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
@@ -184,13 +184,13 @@
public CodeReviewCommit createCherryPickFromCommit(Repository repo,
ObjectInserter inserter, RevCommit mergeTip, RevCommit originalCommit,
PersonIdent cherryPickCommitterIdent, String commitMsg,
- CodeReviewRevWalk rw)
+ CodeReviewRevWalk rw, int parentIndex)
throws MissingObjectException, IncorrectObjectTypeException, IOException,
MergeIdenticalTreeException, MergeConflictException {
final ThreeWayMerger m = newThreeWayMerger(repo, inserter);
- m.setBase(originalCommit.getParent(0));
+ m.setBase(originalCommit.getParent(parentIndex));
if (m.merge(mergeTip, originalCommit)) {
ObjectId tree = m.getResultTreeId();
if (tree.equals(mergeTip.getTree())) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index 4139fc5..a23b78c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -33,7 +33,6 @@
import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_OTHER_REASON;
import com.google.common.base.Function;
-import com.google.common.base.Optional;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.collect.ArrayListMultimap;
@@ -168,6 +167,7 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.regex.Matcher;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsAdvertiseRefsHook.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsAdvertiseRefsHook.java
index 51c2a80..5df6caf 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsAdvertiseRefsHook.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsAdvertiseRefsHook.java
@@ -16,11 +16,15 @@
import static org.eclipse.jgit.lib.RefDatabase.ALL;
+import com.google.auto.value.AutoValue;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.util.MagicBranch;
@@ -46,6 +50,13 @@
private static final Logger log = LoggerFactory
.getLogger(ReceiveCommitsAdvertiseRefsHook.class);
+ @VisibleForTesting
+ @AutoValue
+ public abstract static class Result {
+ public abstract Map<String, Ref> allRefs();
+ public abstract Set<ObjectId> additionalHaves();
+ }
+
private final Provider<InternalChangeQuery> queryProvider;
private final Project.NameKey projectName;
@@ -77,28 +88,46 @@
throw ex;
}
}
+ Result r = advertiseRefs(oldRefs);
+ rp.setAdvertisedRefs(r.allRefs(), r.additionalHaves());
+ }
+
+ @VisibleForTesting
+ public Result advertiseRefs(Map<String, Ref> oldRefs) {
Map<String, Ref> r = Maps.newHashMapWithExpectedSize(oldRefs.size());
+ Set<ObjectId> allPatchSets = Sets.newHashSetWithExpectedSize(oldRefs.size());
for (Map.Entry<String, Ref> e : oldRefs.entrySet()) {
String name = e.getKey();
if (!skip(name)) {
r.put(name, e.getValue());
}
+ if (name.startsWith(RefNames.REFS_CHANGES)) {
+ allPatchSets.add(e.getValue().getObjectId());
+ }
}
- rp.setAdvertisedRefs(r, advertiseOpenChanges());
+ return new AutoValue_ReceiveCommitsAdvertiseRefsHook_Result(
+ r, advertiseOpenChanges(allPatchSets));
}
- private Set<ObjectId> advertiseOpenChanges() {
+ private Set<ObjectId> advertiseOpenChanges(Set<ObjectId> allPatchSets) {
// Advertise some recent open changes, in case a commit is based on one.
int limit = 32;
try {
Set<ObjectId> r = Sets.newHashSetWithExpectedSize(limit);
for (ChangeData cd : queryProvider.get()
+ .setRequestedFields(ImmutableSet.of(ChangeField.PATCH_SET.getName()))
.enforceVisibility(true)
.setLimit(limit)
.byProjectOpen(projectName)) {
PatchSet ps = cd.currentPatchSet();
if (ps != null) {
- r.add(ObjectId.fromString(ps.getRevision().get()));
+ ObjectId id = ObjectId.fromString(ps.getRevision().get());
+ // Ensure we actually observed a patch set ref pointing to this
+ // object, in case the database is out of sync with the repo and the
+ // object doesn't actually exist.
+ if (allPatchSets.contains(id)) {
+ r.add(id);
+ }
}
}
return r;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/RefCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/RefCache.java
index 562db08..96593ac 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/RefCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/RefCache.java
@@ -14,11 +14,10 @@
package com.google.gerrit.server.git;
-import com.google.common.base.Optional;
-
import org.eclipse.jgit.lib.ObjectId;
import java.io.IOException;
+import java.util.Optional;
/**
* Simple short-lived cache of individual refs read from a repo.
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/RepoRefCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/RepoRefCache.java
index 1dfa51e..77f697a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/RepoRefCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/RepoRefCache.java
@@ -14,8 +14,6 @@
package com.google.gerrit.server.git;
-import com.google.common.base.Optional;
-
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
@@ -24,6 +22,7 @@
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
+import java.util.Optional;
/** {@link RefCache} backed directly by a repository. */
public class RepoRefCache implements RefCache {
@@ -42,9 +41,7 @@
return id;
}
Ref ref = refdb.exactRef(refName);
- id = ref != null
- ? Optional.of(ref.getObjectId())
- : Optional.<ObjectId>absent();
+ id = Optional.ofNullable(ref).map(Ref::getObjectId);
ids.put(refName, id);
return id;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/WorkQueue.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/WorkQueue.java
index 00ab31b..a0f729a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/WorkQueue.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/WorkQueue.java
@@ -38,6 +38,7 @@
import java.util.concurrent.Delayed;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
import java.util.concurrent.RunnableScheduledFuture;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.ThreadFactory;
@@ -356,6 +357,15 @@
((CanceledWhileRunning) runnable).setCanceledWhileRunning();
}
}
+ if (runnable instanceof Future<?>) {
+ // Creating new futures eventually passes through
+ // AbstractExecutorService#schedule, which will convert the Guava
+ // Future to a Runnable, thereby making it impossible for the
+ // cancellation to propagate from ScheduledThreadPool's task back to
+ // the Guava future, so kludge it here.
+ ((Future<?>) runnable).cancel(mayInterruptIfRunning);
+ }
+
executor.remove(this);
executor.purge();
return true;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
index 31da05c..c0d96c9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
@@ -107,7 +107,7 @@
try {
newCommit = args.mergeUtil.createCherryPickFromCommit(
args.repo, args.inserter, args.mergeTip.getCurrentTip(), toMerge,
- committer, cherryPickCmtMsg, args.rw);
+ committer, cherryPickCmtMsg, args.rw, 0);
} catch (MergeConflictException mce) {
// Keep going in the case of a single merge failure; the goal is to
// cherry-pick as many commits as possible.
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/Schema.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/Schema.java
index 10f5ecb..faa6934 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/Schema.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/Schema.java
@@ -18,7 +18,6 @@
import com.google.common.base.Function;
import com.google.common.base.MoreObjects;
-import com.google.common.base.Optional;
import com.google.common.base.Predicates;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
@@ -33,6 +32,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
+import java.util.Optional;
/** Specific version of a secondary index schema. */
public class Schema<T> {
@@ -149,15 +149,15 @@
FieldDef<T, ?>... rest) {
FieldDef<T, ?> field = fields.get(first.getName());
if (field != null) {
- return Optional.<FieldDef<T, ?>> of(checkSame(field, first));
+ return Optional.of(checkSame(field, first));
}
for (FieldDef<T, ?> f : rest) {
field = fields.get(f.getName());
if (field != null) {
- return Optional.<FieldDef<T, ?>> of(checkSame(field, f));
+ return Optional.of(checkSame(field, f));
}
}
- return Optional.absent();
+ return Optional.empty();
}
/**
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
index c683513..03f5df8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -19,7 +19,6 @@
import static java.util.stream.Collectors.toSet;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Optional;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
@@ -41,7 +40,6 @@
import com.google.gerrit.server.index.SchemaUtil;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.ChangeData.ChangedLines;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ChangeStatusPredicate;
import com.google.gwtorm.protobuf.CodecFactory;
@@ -632,10 +630,9 @@
@Override
public Integer get(ChangeData input, FillArgs args)
throws OrmException {
- Optional<ChangedLines> changedLines = input.changedLines();
- return changedLines.isPresent()
- ? changedLines.get().insertions + changedLines.get().deletions
- : null;
+ return input.changedLines()
+ .map(c -> c.insertions + c.deletions)
+ .orElse(null);
}
};
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java
index dd8bbaa..70a4888 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.mail;
-import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.Ordering;
import com.google.gerrit.common.errors.EmailException;
@@ -43,6 +42,7 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
+import java.util.Optional;
import java.util.Set;
/** Send comments, after the author of them hit used Publish Comments in the UI.
@@ -252,7 +252,7 @@
} catch (OrmException e) {
log.warn("Could not find the parent of this comment: "
+ child.toString());
- parent = Optional.absent();
+ parent = Optional.empty();
}
if (parent.isPresent()) {
String msg = parent.get().message.trim();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java
index 3322776..61ebfae 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java
@@ -230,7 +230,7 @@
checkColumns(PatchSet.Id.class, 1, 2);
checkColumns(PatchSet.class, 1, 2, 3, 4, 5, 6, 8);
checkColumns(PatchSetApproval.Key.class, 1, 2, 3);
- checkColumns(PatchSetApproval.class, 1, 2, 3, 6, 7);
+ checkColumns(PatchSetApproval.class, 1, 2, 3, 6, 7, 8);
checkColumns(PatchLineComment.Key.class, 1, 2);
checkColumns(PatchLineComment.class, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11);
}
@@ -692,6 +692,11 @@
// ReviewDb allows timestamps before patch set was created, but NoteDb
// truncates this to the patch set creation timestamp.
+ //
+ // ChangeRebuilder ensures all post-submit approvals happen after the
+ // actual submit, so the timestamps may not line up. This shouldn't really
+ // happen, because postSubmit shouldn't be set in ReviewDb until after the
+ // change is submitted in ReviewDb, but you never know.
Timestamp ta = a.getGranted();
Timestamp tb = b.getGranted();
PatchSet psa = checkNotNull(bundleA.patchSets.get(a.getPatchSetId()));
@@ -700,10 +705,12 @@
List<String> exclude = new ArrayList<>(1);
if (bundleA.source == REVIEW_DB && bundleB.source == NOTE_DB) {
excludeGranted =
- ta.before(psa.getCreatedOn()) && tb.equals(psb.getCreatedOn());
+ (ta.before(psa.getCreatedOn()) && tb.equals(psb.getCreatedOn()))
+ || ta.compareTo(tb) < 0;
} else if (bundleA.source == NOTE_DB && bundleB.source == REVIEW_DB) {
excludeGranted =
- tb.before(psb.getCreatedOn()) && ta.equals(psa.getCreatedOn());
+ tb.before(psb.getCreatedOn()) && ta.equals(psa.getCreatedOn())
+ || tb.compareTo(ta) < 0;
}
if (excludeGranted) {
exclude.add("granted");
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
index edb5b4e..68be2c5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -25,12 +25,11 @@
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
-import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.ImmutableSortedSet;
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.Multimaps;
import com.google.common.collect.Ordering;
@@ -349,7 +348,7 @@
// Lazy defensive copies of mutable ReviewDb types, to avoid polluting the
// ChangeNotesCache from handlers.
- private ImmutableMap<PatchSet.Id, PatchSet> patchSets;
+ private ImmutableSortedMap<PatchSet.Id, PatchSet> patchSets;
private ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals;
@VisibleForTesting
@@ -368,18 +367,26 @@
return change;
}
- public ImmutableMap<PatchSet.Id, PatchSet> getPatchSets() {
+ public ImmutableSortedMap<PatchSet.Id, PatchSet> getPatchSets() {
if (patchSets == null) {
- patchSets = ImmutableMap.copyOf(
- Maps.transformValues(state.patchSets(), PatchSet::new));
+ ImmutableSortedMap.Builder<PatchSet.Id, PatchSet> b =
+ ImmutableSortedMap.orderedBy(comparing(PatchSet.Id::get));
+ for (Map.Entry<PatchSet.Id, PatchSet> e : state.patchSets()) {
+ b.put(e.getKey(), new PatchSet(e.getValue()));
+ }
+ patchSets = b.build();
}
return patchSets;
}
public ImmutableListMultimap<PatchSet.Id, PatchSetApproval> getApprovals() {
if (approvals == null) {
- approvals = ImmutableListMultimap.copyOf(
- Multimaps.transformValues(state.approvals(), PatchSetApproval::new));
+ ImmutableListMultimap.Builder<PatchSet.Id, PatchSetApproval> b =
+ ImmutableListMultimap.builder();
+ for (Map.Entry<PatchSet.Id, PatchSetApproval> e : state.approvals()) {
+ b.put(e.getKey(), new PatchSetApproval(e.getValue()));
+ }
+ approvals = b.build();
}
return approvals;
}
@@ -526,7 +533,7 @@
public PatchSet getCurrentPatchSet() {
PatchSet.Id psId = change.currentPatchSetId();
- return checkNotNull(state.patchSets().get(psId),
+ return checkNotNull(getPatchSets().get(psId),
"missing current patch set %s", psId.get());
}
@@ -559,7 +566,7 @@
@Override
protected ObjectId readRef(Repository repo) throws IOException {
return refs != null
- ? refs.get(getRefName()).orNull()
+ ? refs.get(getRefName()).orElse(null)
: super.readRef(repo);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
index 198eb2f..73b7aec 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
@@ -101,7 +101,8 @@
+ P // status
+ P + set(state.pastAssignees(), K)
+ P + set(state.hashtags(), str(10))
- + P + map(state.patchSets(), patchSet())
+ + P + list(state.patchSets(), patchSet())
+ + P + list(state.allPastReviewers(), approval())
+ P + list(state.reviewerUpdates(), 4 * O + K + K + P)
+ P + list(state.submitRecords(), P + list(2, str(4) + P + K) + P)
+ P + list(state.allChangeMessages(), changeMessage())
@@ -172,6 +173,15 @@
+ P; // pushCertificate
}
+ private static int approval() {
+ return O
+ + P + patchSetId() + P + K + P + O + str(10)
+ + 2 // value
+ + P + T // granted
+ + P // tag
+ + P; // realAccountId
+ }
+
private static int changeMessage() {
int key = K + str(20);
return O
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 a3ef2ce..bd20423 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
@@ -35,7 +35,6 @@
import com.google.auto.value.AutoValue;
import com.google.common.base.Enums;
-import com.google.common.base.Optional;
import com.google.common.base.Splitter;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.HashBasedTable;
@@ -90,6 +89,7 @@
import java.util.Map;
import java.util.NavigableSet;
import java.util.Objects;
+import java.util.Optional;
import java.util.Set;
import java.util.TreeMap;
@@ -131,6 +131,7 @@
private final Set<PatchSet.Id> deletedPatchSets;
private final Map<PatchSet.Id, PatchSetState> patchSetStates;
private final Map<ApprovalKey, PatchSetApproval> approvals;
+ private final List<PatchSetApproval> bufferedApprovals;
private final List<ChangeMessage> allChangeMessages;
private final Multimap<PatchSet.Id, ChangeMessage> changeMessagesByPatchSet;
@@ -160,6 +161,7 @@
this.noteUtil = noteUtil;
this.metrics = metrics;
approvals = new LinkedHashMap<>();
+ bufferedApprovals = new ArrayList<>();
reviewers = HashBasedTable.create();
allPastReviewers = new ArrayList<>();
reviewerUpdates = new ArrayList<>();
@@ -213,7 +215,7 @@
topic,
originalSubject,
submissionId,
- assignee != null ? assignee.orNull() : null,
+ assignee != null ? assignee.orElse(null) : null,
status,
Sets.newLinkedHashSet(Lists.reverse(pastAssignees)),
@@ -281,9 +283,6 @@
if (branch == null) {
branch = parseBranch(commit);
}
- if (status == null) {
- status = parseStatus(commit);
- }
PatchSet.Id psId = parsePatchSetId(commit);
if (currentPatchSetId == null || psId.get() > currentPatchSetId.get()) {
@@ -343,6 +342,12 @@
parseSubmitRecords(commit.getFooterLineValues(FOOTER_SUBMITTED_WITH));
}
+ if (status == null) {
+ status = parseStatus(commit);
+ }
+
+ // Parse approvals after status to treat approvals in the same commit as
+ // "Status: merged" as non-post-submit.
for (String line : commit.getFooterLineValues(FOOTER_LABEL)) {
parseApproval(psId, accountId, realAccountId, ts, line);
}
@@ -503,10 +508,10 @@
Optional<Account.Id> parsedAssignee;
if (assigneeValue.equals("")) {
// Empty footer found, assignee deleted
- parsedAssignee = Optional.absent();
+ parsedAssignee = Optional.empty();
} else {
PersonIdent ident = RawParseUtils.parsePersonIdent(assigneeValue);
- parsedAssignee = Optional.fromNullable(noteUtil.parseIdent(ident, id));
+ parsedAssignee = Optional.ofNullable(noteUtil.parseIdent(ident, id));
}
if (assignee == null) {
assignee = parsedAssignee;
@@ -538,12 +543,21 @@
} else if (statusLines.size() > 1) {
throw expectedOneFooter(FOOTER_STATUS, statusLines);
}
- Optional<Change.Status> status = Enums.getIfPresent(
- Change.Status.class, statusLines.get(0).toUpperCase());
- if (!status.isPresent()) {
+ Change.Status status = Enums.getIfPresent(
+ Change.Status.class, statusLines.get(0).toUpperCase()).orNull();
+ if (status == null) {
throw invalidFooter(FOOTER_STATUS, statusLines.get(0));
}
- return status.get();
+ // All approvals after MERGED and before the next status change get the
+ // postSubmit bit. (Currently the state can't change from MERGED to
+ // something else, but just in case.)
+ if (status == Change.Status.MERGED) {
+ for (PatchSetApproval psa : bufferedApprovals) {
+ psa.setPostSubmit(true);
+ }
+ }
+ bufferedApprovals.clear();
+ return status;
}
private PatchSet.Id parsePatchSetId(ChangeNotesCommit commit)
@@ -567,10 +581,11 @@
}
String withParens = psIdLine.substring(s + 1);
if (withParens.startsWith("(") && withParens.endsWith(")")) {
- Optional<PatchSetState> state = Enums.getIfPresent(PatchSetState.class,
- withParens.substring(1, withParens.length() - 1).toUpperCase());
- if (state.isPresent()) {
- return state.get();
+ PatchSetState state = Enums.getIfPresent(PatchSetState.class,
+ withParens.substring(1, withParens.length() - 1).toUpperCase())
+ .orNull();
+ if (state != null) {
+ return state;
}
}
throw invalidFooter(FOOTER_PATCH_SET, psIdLine);
@@ -665,15 +680,18 @@
throw parseException(
"patch set %s requires an identified user as uploader", psId.get());
}
+ PatchSetApproval psa;
if (line.startsWith("-")) {
- parseRemoveApproval(psId, accountId, realAccountId, ts, line);
+ psa = parseRemoveApproval(psId, accountId, realAccountId, ts, line);
} else {
- parseAddApproval(psId, accountId, realAccountId, ts, line);
+ psa = parseAddApproval(psId, accountId, realAccountId, ts, line);
}
+ bufferedApprovals.add(psa);
}
- private void parseAddApproval(PatchSet.Id psId, Account.Id committerId,
- Account.Id realAccountId, Timestamp ts, String line)
+ private PatchSetApproval parseAddApproval(PatchSet.Id psId,
+ Account.Id committerId, Account.Id realAccountId, Timestamp ts,
+ String line)
throws ConfigInvalidException {
// There are potentially 3 accounts involved here:
// 1. The account from the commit, which is the effective IdentifiedUser
@@ -726,11 +744,12 @@
if (!approvals.containsKey(k)) {
approvals.put(k, psa);
}
+ return psa;
}
- private void parseRemoveApproval(PatchSet.Id psId, Account.Id committerId,
- Account.Id realAccountId, Timestamp ts, String line)
- throws ConfigInvalidException {
+ private PatchSetApproval parseRemoveApproval(PatchSet.Id psId,
+ Account.Id committerId, Account.Id realAccountId, Timestamp ts,
+ String line) throws ConfigInvalidException {
// See comments in parseAddApproval about the various users involved.
Account.Id effectiveAccountId;
String label;
@@ -774,6 +793,7 @@
if (!approvals.containsKey(k)) {
approvals.put(k, remove);
}
+ return remove;
}
private void parseSubmitRecords(List<String> lines)
@@ -787,10 +807,9 @@
submitRecords.add(rec);
int s = line.indexOf(' ');
String statusStr = s >= 0 ? line.substring(0, s) : line;
- Optional<SubmitRecord.Status> status =
- Enums.getIfPresent(SubmitRecord.Status.class, statusStr);
- checkFooter(status.isPresent(), FOOTER_SUBMITTED_WITH, line);
- rec.status = status.get();
+ rec.status =
+ Enums.getIfPresent(SubmitRecord.Status.class, statusStr).orNull();
+ checkFooter(rec.status != null, FOOTER_SUBMITTED_WITH, line);
if (s >= 0) {
rec.errorMessage = line.substring(s);
}
@@ -802,10 +821,9 @@
}
rec.labels.add(label);
- Optional<SubmitRecord.Label.Status> status = Enums.getIfPresent(
- SubmitRecord.Label.Status.class, line.substring(0, c));
- checkFooter(status.isPresent(), FOOTER_SUBMITTED_WITH, line);
- label.status = status.get();
+ label.status = Enums.getIfPresent(
+ SubmitRecord.Label.Status.class, line.substring(0, c)).orNull();
+ checkFooter(label.status != null, FOOTER_SUBMITTED_WITH, line);
int c2 = line.indexOf(": ", c + 2);
if (c2 >= 0) {
label.label = line.substring(c + 2, c2);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index 67eb328..e0f7920 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -15,14 +15,12 @@
package com.google.gerrit.server.notedb;
import static com.google.common.base.Preconditions.checkNotNull;
-import static java.util.Comparator.comparing;
import com.google.auto.value.AutoValue;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.Multimap;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.SubmitRecord;
@@ -59,17 +57,17 @@
return new AutoValue_ChangeNotesState(
change.getId(),
null,
- ImmutableSet.<Account.Id>of(),
- ImmutableSet.<String>of(),
- ImmutableSortedMap.<PatchSet.Id, PatchSet>of(),
- ImmutableListMultimap.<PatchSet.Id, PatchSetApproval>of(),
+ ImmutableSet.of(),
+ ImmutableSet.of(),
+ ImmutableList.of(),
+ ImmutableList.of(),
ReviewerSet.empty(),
- ImmutableList.<Account.Id>of(),
- ImmutableList.<ReviewerStatusUpdate>of(),
- ImmutableList.<SubmitRecord>of(),
- ImmutableList.<ChangeMessage>of(),
- ImmutableListMultimap.<PatchSet.Id, ChangeMessage>of(),
- ImmutableListMultimap.<RevId, Comment>of());
+ ImmutableList.of(),
+ ImmutableList.of(),
+ ImmutableList.of(),
+ ImmutableList.of(),
+ ImmutableListMultimap.of(),
+ ImmutableListMultimap.of());
}
static ChangeNotesState create(
@@ -117,8 +115,8 @@
status),
ImmutableSet.copyOf(pastAssignees),
ImmutableSet.copyOf(hashtags),
- ImmutableSortedMap.copyOf(patchSets, comparing(PatchSet.Id::get)),
- ImmutableListMultimap.copyOf(approvals),
+ ImmutableList.copyOf(patchSets.entrySet()),
+ ImmutableList.copyOf(approvals.entries()),
reviewers,
ImmutableList.copyOf(allPastReviewers),
ImmutableList.copyOf(reviewerUpdates),
@@ -161,8 +159,8 @@
// Other related to this Change.
abstract ImmutableSet<Account.Id> pastAssignees();
abstract ImmutableSet<String> hashtags();
- abstract ImmutableSortedMap<PatchSet.Id, PatchSet> patchSets();
- abstract ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals();
+ abstract ImmutableList<Map.Entry<PatchSet.Id, PatchSet>> patchSets();
+ abstract ImmutableList<Map.Entry<PatchSet.Id, PatchSetApproval>> approvals();
abstract ReviewerSet reviewers();
abstract ImmutableList<Account.Id> allPastReviewers();
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 3642dff..1a417e1 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
@@ -39,7 +39,6 @@
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.Table;
@@ -84,6 +83,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.Optional;
import java.util.Set;
/**
@@ -284,7 +284,7 @@
}
public void removeApprovalFor(Account.Id reviewer, String label) {
- approvals.put(label, reviewer, Optional.<Short> absent());
+ approvals.put(label, reviewer, Optional.empty());
}
public void merge(RequestId submissionId,
@@ -414,7 +414,7 @@
}
public void removeAssignee() {
- this.assignee = Optional.absent();
+ this.assignee = Optional.empty();
}
public Map<Account.Id, ReviewerStateInternal> getReviewers() {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbChangeState.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbChangeState.java
index 4aed71e..ad54f02 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbChangeState.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbChangeState.java
@@ -22,7 +22,6 @@
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Optional;
import com.google.common.base.Predicates;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableMap;
@@ -40,6 +39,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
/**
* The state of all relevant NoteDb refs across all repos corresponding to a
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbModule.java
index 72a1f1b..c4d7277 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbModule.java
@@ -16,11 +16,8 @@
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
-import com.google.common.collect.ImmutableMultimap;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Change.Id;
-import com.google.gerrit.reviewdb.client.Project.NameKey;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.notedb.NoteDbUpdateManager.Result;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder;
@@ -29,7 +26,6 @@
import com.google.inject.name.Names;
import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.lib.Repository;
public class NoteDbModule extends FactoryModule {
private final Config cfg;
@@ -79,13 +75,6 @@
}
@Override
- public boolean rebuildProject(ReviewDb db,
- ImmutableMultimap<NameKey, Id> allChanges, NameKey project,
- Repository allUsersRepo) {
- return false;
- }
-
- @Override
public NoteDbUpdateManager stage(ReviewDb db, Change.Id changeId) {
return null;
}
@@ -95,6 +84,12 @@
NoteDbUpdateManager manager) {
return null;
}
+
+ @Override
+ public void buildUpdates(NoteDbUpdateManager manager,
+ ChangeBundle bundle) {
+ // Do nothing.
+ }
});
bind(new TypeLiteral<Cache<ChangeNotesCache.Key, ChangeNotesState>>() {})
.annotatedWith(Names.named(ChangeNotesCache.CACHE_NAME))
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
index 18cac47..1f64bcc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
@@ -21,7 +21,6 @@
import static com.google.gerrit.server.notedb.NoteDbTable.CHANGES;
import com.google.auto.value.AutoValue;
-import com.google.common.base.Optional;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableList;
@@ -58,6 +57,7 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
/**
@@ -362,7 +362,7 @@
StagedResult r = StagedResult.create(
e.getKey(),
NoteDbChangeState.Delta.create(
- e.getKey(), Optional.<ObjectId>absent(), e.getValue()),
+ e.getKey(), Optional.empty(), e.getValue()),
changeRepo, allUsersRepo);
checkState(r.changeCommands().isEmpty(),
"should not have change commands when updating only drafts: %s", r);
@@ -550,7 +550,7 @@
for (Map.Entry<String, Collection<U>> e : all.asMap().entrySet()) {
String refName = e.getKey();
Collection<U> updates = e.getValue();
- ObjectId old = or.cmds.get(refName).or(ObjectId.zeroId());
+ ObjectId old = or.cmds.get(refName).orElse(ObjectId.zeroId());
// Only actually write to the ref if one of the updates explicitly allows
// us to do so, i.e. it is known to represent a new change. This avoids
// writing partial change meta if the change hasn't been backfilled yet.
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/TestChangeRebuilderWrapper.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/TestChangeRebuilderWrapper.java
index 7bfb20c..1c585fc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/TestChangeRebuilderWrapper.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/TestChangeRebuilderWrapper.java
@@ -15,9 +15,7 @@
package com.google.gerrit.server.notedb;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableMultimap;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.notedb.NoteDbUpdateManager.Result;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder;
@@ -29,7 +27,6 @@
import com.google.inject.Singleton;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Repository;
import java.io.IOException;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -83,23 +80,6 @@
}
@Override
- public boolean rebuildProject(ReviewDb db,
- ImmutableMultimap<Project.NameKey, Change.Id> allChanges,
- Project.NameKey project, Repository allUsersRepo)
- throws NoSuchChangeException, IOException, OrmException,
- ConfigInvalidException {
- if (failNextUpdate.getAndSet(false)) {
- throw new IOException("Update failed");
- }
- boolean result =
- delegate.rebuildProject(db, allChanges, project, allUsersRepo);
- if (stealNextUpdate.getAndSet(false)) {
- throw new IOException("Update stolen");
- }
- return result;
- }
-
- @Override
public NoteDbUpdateManager stage(ReviewDb db, Change.Id changeId)
throws NoSuchChangeException, IOException, OrmException {
// Don't inspect stealNextUpdate; that happens in execute() below.
@@ -119,4 +99,11 @@
}
return result;
}
+
+ @Override
+ public void buildUpdates(NoteDbUpdateManager manager, ChangeBundle bundle)
+ throws IOException, OrmException {
+ // Don't check for manual failure; that happens in execute().
+ delegate.buildUpdates(manager, bundle);
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ApprovalEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ApprovalEvent.java
index 3aba7c5..4fed25d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ApprovalEvent.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ApprovalEvent.java
@@ -38,4 +38,9 @@
checkUpdate(update);
update.putApproval(psa.getLabel(), psa.getValue());
}
+
+ @Override
+ protected boolean isPostSubmitApproval() {
+ return psa.isPostSubmit();
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilder.java
index ea96012..a63fbad 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilder.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilder.java
@@ -14,11 +14,9 @@
package com.google.gerrit.server.notedb.rebuild;
-import com.google.common.collect.ImmutableMultimap;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.notedb.ChangeBundle;
import com.google.gerrit.server.notedb.NoteDbUpdateManager;
@@ -28,7 +26,6 @@
import com.google.gwtorm.server.SchemaFactory;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Repository;
import java.io.IOException;
import java.util.concurrent.Callable;
@@ -69,11 +66,8 @@
ChangeBundle bundle) throws NoSuchChangeException, IOException,
OrmException, ConfigInvalidException;
- public abstract boolean rebuildProject(ReviewDb db,
- ImmutableMultimap<Project.NameKey, Change.Id> allChanges,
- Project.NameKey project, Repository allUsersRepo)
- throws NoSuchChangeException, IOException, OrmException,
- ConfigInvalidException;
+ public abstract void buildUpdates(NoteDbUpdateManager manager,
+ ChangeBundle bundle) throws IOException, OrmException;
public abstract NoteDbUpdateManager stage(ReviewDb db, Change.Id changeId)
throws NoSuchChangeException, IOException, OrmException;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
index 1916610..b6e25a0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
@@ -15,7 +15,6 @@
package com.google.gerrit.server.notedb.rebuild;
import static com.google.common.base.MoreObjects.firstNonNull;
-import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_HASHTAGS;
@@ -23,18 +22,16 @@
import static java.util.concurrent.TimeUnit.SECONDS;
import static java.util.stream.Collectors.toList;
-import com.google.common.base.Optional;
import com.google.common.base.Splitter;
import com.google.common.collect.ArrayListMultimap;
-import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.common.collect.Ordering;
import com.google.common.collect.Sets;
import com.google.common.collect.Table;
import com.google.common.primitives.Ints;
-import com.google.gerrit.common.FormatUtil;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
@@ -44,7 +41,6 @@
import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.ReviewDbUtil;
@@ -75,12 +71,8 @@
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.lib.ProgressMonitor;
import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.lib.TextProgressMonitor;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
@@ -88,7 +80,6 @@
import org.slf4j.LoggerFactory;
import java.io.IOException;
-import java.io.PrintWriter;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Collections;
@@ -96,6 +87,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.Optional;
import java.util.Set;
public class ChangeRebuilderImpl extends ChangeRebuilder {
@@ -166,6 +158,8 @@
throws NoSuchChangeException, IOException, OrmException,
ConfigInvalidException {
db = ReviewDbUtil.unwrapDb(db);
+ // Read change just to get project; this instance is then discarded so we
+ // can read a consistent ChangeBundle inside a transaction.
Change change = db.changes().get(changeId);
if (change == null) {
throw new NoSuchChangeException(changeId);
@@ -260,40 +254,7 @@
}
@Override
- public boolean rebuildProject(ReviewDb db,
- ImmutableMultimap<Project.NameKey, Change.Id> allChanges,
- Project.NameKey project, Repository allUsersRepo)
- throws NoSuchChangeException, IOException, OrmException,
- ConfigInvalidException {
- checkArgument(allChanges.containsKey(project));
- boolean ok = true;
- ProgressMonitor pm = new TextProgressMonitor(new PrintWriter(System.out));
- pm.beginTask(
- FormatUtil.elide(project.get(), 50), allChanges.get(project).size());
- try (NoteDbUpdateManager manager = updateManagerFactory.create(project);
- ObjectInserter allUsersInserter = allUsersRepo.newObjectInserter();
- RevWalk allUsersRw = new RevWalk(allUsersInserter.newReader())) {
- manager.setAllUsersRepo(allUsersRepo, allUsersRw, allUsersInserter,
- new ChainedReceiveCommands(allUsersRepo));
- for (Change.Id changeId : allChanges.get(project)) {
- try {
- buildUpdates(manager, bundleReader.fromReviewDb(db, changeId));
- } catch (NoPatchSetsException e) {
- log.warn(e.getMessage());
- } catch (Throwable t) {
- log.error("Failed to rebuild change " + changeId, t);
- ok = false;
- }
- pm.update(1);
- }
- manager.execute();
- } finally {
- pm.endTask();
- }
- return ok;
- }
-
- private void buildUpdates(NoteDbUpdateManager manager, ChangeBundle bundle)
+ public void buildUpdates(NoteDbUpdateManager manager, ChangeBundle bundle)
throws IOException, OrmException {
manager.setCheckExpectedState(false);
Change change = new Change(bundle.getChange());
@@ -427,7 +388,9 @@
private void sortAndFillEvents(Change change, Change noteDbChange,
List<Event> events, Integer minPsNum) {
- events.add(new FinalUpdatesEvent(change, noteDbChange));
+ Event finalUpdates = new FinalUpdatesEvent(change, noteDbChange);
+ events.add(finalUpdates);
+ setPostSubmitDeps(events);
new EventSorter(events).sort();
// Ensure the first event in the list creates the change, setting the author
@@ -476,6 +439,17 @@
}
}
+ private void setPostSubmitDeps(List<Event> events) {
+ Optional<Event> submitEvent = Lists.reverse(events).stream()
+ .filter(Event::isSubmit)
+ .findFirst();
+ if (submitEvent.isPresent()) {
+ events.stream()
+ .filter(Event::isPostSubmitApproval)
+ .forEach(e -> e.addDep(submitEvent.get()));
+ }
+ }
+
private void flushEventsToUpdate(NoteDbUpdateManager manager,
EventList<Event> events, Change change) throws OrmException, IOException {
if (events.isEmpty()) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/Event.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/Event.java
index 78df2d2..4dcdec6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/Event.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/Event.java
@@ -86,6 +86,14 @@
abstract void apply(ChangeUpdate update) throws OrmException, IOException;
+ protected boolean isPostSubmitApproval() {
+ return false;
+ }
+
+ protected boolean isSubmit() {
+ return false;
+ }
+
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/EventList.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/EventList.java
index 4f6e2e8..59ff49e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/EventList.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/EventList.java
@@ -23,21 +23,32 @@
import java.sql.Timestamp;
import java.util.ArrayList;
+import java.util.Iterator;
import java.util.Objects;
-class EventList<E extends Event> extends ArrayList<E> {
- private static final long serialVersionUID = 1L;
+class EventList<E extends Event> implements Iterable<E> {
+ private final ArrayList<E> list = new ArrayList<>();
+ private boolean isSubmit;
- private E getLast() {
- return get(size() - 1);
+ @Override
+ public Iterator<E> iterator() {
+ return list.iterator();
}
- private long getLastTime() {
- return getLast().when.getTime();
+ void add(E e) {
+ list.add(e);
+ if (e.isSubmit()) {
+ isSubmit = true;
+ }
}
- private long getFirstTime() {
- return get(0).when.getTime();
+ void clear() {
+ list.clear();
+ isSubmit = false;
+ }
+
+ boolean isEmpty() {
+ return list.isEmpty();
}
boolean canAdd(E e) {
@@ -55,6 +66,10 @@
|| !Objects.equals(e.tag, last.tag)) {
return false; // Different patch set, author, or tag.
}
+ if (e.isPostSubmitApproval() && isSubmit) {
+ // Post-submit approvals must come after the update that submits.
+ return false;
+ }
long t = e.when.getTime();
long tFirst = getFirstTime();
@@ -114,4 +129,24 @@
String getTag() {
return getLast().tag;
}
+
+ private E get(int i) {
+ return list.get(i);
+ }
+
+ private int size() {
+ return list.size();
+ }
+
+ private E getLast() {
+ return list.get(list.size() - 1);
+ }
+
+ private long getLastTime() {
+ return getLast().when.getTime();
+ }
+
+ private long getFirstTime() {
+ return list.get(0).when.getTime();
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/FinalUpdatesEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/FinalUpdatesEvent.java
index 9babc28..17cf8ba 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/FinalUpdatesEvent.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/FinalUpdatesEvent.java
@@ -58,4 +58,9 @@
update.setSubjectForCommit("Final NoteDb migration updates");
}
}
+
+ @Override
+ protected boolean isSubmit() {
+ return change.getStatus() == Change.Status.MERGED;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/StatusChangeEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/StatusChangeEvent.java
index c7632e8..5bc05d0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/StatusChangeEvent.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/StatusChangeEvent.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.notedb.rebuild;
-import com.google.common.base.Optional;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
@@ -25,6 +24,7 @@
import java.sql.Timestamp;
import java.util.Map;
+import java.util.Optional;
import java.util.regex.Pattern;
class StatusChangeEvent extends Event {
@@ -40,7 +40,7 @@
Change change, Change noteDbChange) {
String msg = message.getMessage();
if (msg == null) {
- return Optional.absent();
+ return Optional.empty();
}
for (Map.Entry<Change.Status, Pattern> e : PATTERNS.entrySet()) {
if (e.getValue().matcher(msg).matches()) {
@@ -48,12 +48,12 @@
message, change, noteDbChange, e.getKey()));
}
}
- return Optional.absent();
+ return Optional.empty();
}
+ private final Change.Status status;
private final Change change;
private final Change noteDbChange;
- private final Change.Status status;
private StatusChangeEvent(ChangeMessage message, Change change,
Change noteDbChange, Change.Status status) {
@@ -87,4 +87,9 @@
noteDbChange.setSubmissionId(change.getSubmissionId());
}
}
+
+ @Override
+ protected boolean isSubmit() {
+ return status == Change.Status.MERGED;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
index 7db206f..42324f5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
@@ -16,7 +16,6 @@
import static com.google.common.base.Preconditions.checkArgument;
-import com.google.common.base.Optional;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.CommentDetail;
import com.google.gerrit.common.data.PatchScript;
@@ -59,6 +58,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.concurrent.Callable;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java
index 73d7b1e..354ccf9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/JarScanner.java
@@ -17,12 +17,9 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.collect.Iterables.transform;
-import com.google.common.base.Optional;
-import com.google.common.base.Predicates;
import com.google.common.base.Strings;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
@@ -49,6 +46,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.jar.Attributes;
import java.util.jar.JarEntry;
@@ -192,9 +190,9 @@
String annotationName;
String annotationValue;
String[] interfaces;
- Iterable<String> exports;
+ Collection<String> exports;
- private ClassData(Iterable<String> exports) {
+ private ClassData(Collection<String> exports) {
super(Opcodes.ASM5);
this.exports = exports;
}
@@ -214,9 +212,12 @@
@Override
public AnnotationVisitor visitAnnotation(String desc, boolean visible) {
+ if (!visible) {
+ return null;
+ }
Optional<String> found =
- Iterables.tryFind(exports, Predicates.equalTo(desc));
- if (visible && found.isPresent()) {
+ exports.stream().filter(x -> x.equals(desc)).findAny();
+ if (found.isPresent()) {
annotationName = desc;
return new AbstractAnnotationVisitor() {
@Override
@@ -287,10 +288,11 @@
}
@Override
- public Optional<PluginEntry> getEntry(String resourcePath) throws IOException {
+ public Optional<PluginEntry> getEntry(String resourcePath)
+ throws IOException {
JarEntry jarEntry = jarFile.getJarEntry(resourcePath);
if (jarEntry == null || jarEntry.getSize() == 0) {
- return Optional.absent();
+ return Optional.empty();
}
return Optional.of(resourceOf(jarEntry));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginContentScanner.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginContentScanner.java
index 15bb92f..c333638 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginContentScanner.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginContentScanner.java
@@ -14,8 +14,6 @@
package com.google.gerrit.server.plugins;
-import com.google.common.base.Optional;
-
import java.io.IOException;
import java.io.InputStream;
import java.lang.annotation.Annotation;
@@ -23,6 +21,7 @@
import java.util.Collections;
import java.util.Enumeration;
import java.util.Map;
+import java.util.Optional;
import java.util.jar.Manifest;
/**
@@ -51,9 +50,8 @@
}
@Override
- public Optional<PluginEntry> getEntry(String resourcePath)
- throws IOException {
- return Optional.absent();
+ public Optional<PluginEntry> getEntry(String resourcePath) {
+ return Optional.empty();
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginEntry.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginEntry.java
index 74ded73..c6077f4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginEntry.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginEntry.java
@@ -11,13 +11,13 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.plugins;
-import com.google.common.base.Optional;
+package com.google.gerrit.server.plugins;
import java.util.Collections;
import java.util.Comparator;
import java.util.Map;
+import java.util.Optional;
/**
* Plugin static resource entry
@@ -38,7 +38,7 @@
};
private static final Map<Object, String> EMPTY_ATTRS = Collections.emptyMap();
- private static final Optional<Long> NO_SIZE = Optional.absent();
+ private static final Optional<Long> NO_SIZE = Optional.empty();
private final String name;
private final long time;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index 407d113..4d55c80 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -20,7 +20,6 @@
import com.google.auto.value.AutoValue;
import com.google.common.base.MoreObjects;
-import com.google.common.base.Optional;
import com.google.common.base.Predicate;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
@@ -92,6 +91,7 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
public class ChangeData {
@@ -611,12 +611,12 @@
Optional<PatchList> r = patchLists.get(psId);
if (r == null) {
if (!lazyLoad) {
- return Optional.absent();
+ return Optional.empty();
}
try {
r = Optional.of(patchListCache.get(c, ps));
} catch (PatchListNotAvailableException e) {
- r = Optional.absent();
+ r = Optional.empty();
}
patchLists.put(psId, r);
}
@@ -631,12 +631,12 @@
Optional<DiffSummary> r = diffSummaries.get(psId);
if (r == null) {
if (!lazyLoad) {
- return Optional.absent();
+ return Optional.empty();
}
try {
r = Optional.of(patchListCache.getDiffSummary(c, ps));
} catch (PatchListNotAvailableException e) {
- r = Optional.absent();
+ r = Optional.empty();
}
diffSummaries.put(psId, r);
}
@@ -646,24 +646,20 @@
private Optional<ChangedLines> computeChangedLines() throws OrmException {
Change c = change();
if (c == null) {
- return Optional.absent();
+ return Optional.empty();
}
PatchSet ps = currentPatchSet();
if (ps == null) {
- return Optional.absent();
+ return Optional.empty();
}
- Optional<PatchList> p = getPatchList(c, ps);
- if (!p.isPresent()) {
- return Optional.absent();
- }
- return Optional.of(
- new ChangedLines(p.get().getInsertions(), p.get().getDeletions()));
+ return getPatchList(c, ps).map(
+ p -> new ChangedLines(p.getInsertions(), p.getDeletions()));
}
public Optional<ChangedLines> changedLines() throws OrmException {
if (changedLines == null) {
if (!lazyLoad) {
- return Optional.absent();
+ return Optional.empty();
}
changedLines = computeChangedLines();
}
@@ -675,7 +671,7 @@
}
public void setNoChangedLines() {
- changedLines = Optional.absent();
+ changedLines = Optional.empty();
}
public Change.Id getId() {
@@ -951,13 +947,10 @@
* @throws OrmException an error occurred reading the database.
*/
public Optional<PatchSetApproval> getSubmitApproval()
- throws OrmException {
- for (PatchSetApproval psa : currentApprovals()) {
- if (psa.isLegacySubmit()) {
- return Optional.fromNullable(psa);
- }
- }
- return Optional.absent();
+ throws OrmException {
+ return currentApprovals().stream()
+ .filter(PatchSetApproval::isLegacySubmit)
+ .findFirst();
}
public ReviewerSet reviewers() throws OrmException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 57f5710..390f8b8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -19,7 +19,6 @@
import static java.util.stream.Collectors.toSet;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Optional;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
@@ -85,6 +84,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.regex.Pattern;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
index a7a8191..a514642 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
@@ -33,7 +33,7 @@
/** A version of the database schema. */
public abstract class SchemaVersion {
/** The current schema version. */
- public static final Class<Schema_133> C = Schema_133.class;
+ public static final Class<Schema_134> C = Schema_134.class;
public static int getBinaryVersion() {
return guessVersion(C);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_134.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_134.java
new file mode 100644
index 0000000..fa01ff3
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_134.java
@@ -0,0 +1,25 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.schema;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+public class Schema_134 extends SchemaVersion {
+ @Inject
+ Schema_134(Provider<Schema_133> prior) {
+ super(prior);
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/account/AuthorizedKeysTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/account/AuthorizedKeysTest.java
index f5849c1..2a86401 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/account/AuthorizedKeysTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/account/AuthorizedKeysTest.java
@@ -16,7 +16,6 @@
import static com.google.common.truth.Truth.assertThat;
-import com.google.common.base.Optional;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountSshKey;
@@ -24,6 +23,7 @@
import java.util.ArrayList;
import java.util.List;
+import java.util.Optional;
public class AuthorizedKeysTest {
private static final String KEY1 =
@@ -168,7 +168,7 @@
* @return the expected line for this key in the authorized_keys file
*/
private static String addDeletedKey(List<Optional<AccountSshKey>> keys) {
- keys.add(Optional.<AccountSshKey> absent());
+ keys.add(Optional.empty());
return AuthorizedKeys.DELETED_KEY_COMMENT + "\n";
}
}
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 ea33e65..d750bc4 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
@@ -399,6 +399,81 @@
}
@Test
+ public void approvalsPostSubmit() throws Exception {
+ Change c = newChange();
+ RequestId submissionId = RequestId.forChange(c);
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.putApproval("Code-Review", (short) 1);
+ update.putApproval("Verified", (short) 1);
+ update.commit();
+
+ update = newUpdate(c, changeOwner);
+ update.merge(submissionId, ImmutableList.of(
+ submitRecord("NOT_READY", null,
+ submitLabel("Verified", "OK", changeOwner.getAccountId()),
+ submitLabel("Code-Review", "NEED", null))));
+ update.commit();
+
+ update = newUpdate(c, changeOwner);
+ update.putApproval("Code-Review", (short) 2);
+ update.commit();
+
+ ChangeNotes notes = newNotes(c);
+ List<PatchSetApproval> approvals =
+ Lists.newArrayList(notes.getApprovals().values());
+ assertThat(approvals).hasSize(2);
+ assertThat(approvals.get(0).getLabel()).isEqualTo("Verified");
+ assertThat(approvals.get(0).getValue()).isEqualTo((short) 1);
+ assertThat(approvals.get(0).isPostSubmit()).isFalse();
+ assertThat(approvals.get(1).getLabel()).isEqualTo("Code-Review");
+ assertThat(approvals.get(1).getValue()).isEqualTo((short) 2);
+ assertThat(approvals.get(1).isPostSubmit()).isTrue();
+ }
+
+ @Test
+ public void approvalsDuringSubmit() throws Exception {
+ Change c = newChange();
+ RequestId submissionId = RequestId.forChange(c);
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.putApproval("Code-Review", (short) 1);
+ update.putApproval("Verified", (short) 1);
+ update.commit();
+
+ Account.Id ownerId = changeOwner.getAccountId();
+ Account.Id otherId = otherUser.getAccountId();
+ update = newUpdate(c, otherUser);
+ update.merge(submissionId, ImmutableList.of(
+ submitRecord("NOT_READY", null,
+ submitLabel("Verified", "OK", ownerId),
+ submitLabel("Code-Review", "NEED", null))));
+ update.putApproval("Other-Label", (short) 1);
+ update.putApprovalFor(ownerId, "Code-Review", (short) 2);
+ update.commit();
+
+ update = newUpdate(c, otherUser);
+ update.putApproval("Other-Label", (short) 2);
+ update.commit();
+
+ ChangeNotes notes = newNotes(c);
+
+ List<PatchSetApproval> approvals =
+ Lists.newArrayList(notes.getApprovals().values());
+ assertThat(approvals).hasSize(3);
+ assertThat(approvals.get(0).getAccountId()).isEqualTo(ownerId);
+ assertThat(approvals.get(0).getLabel()).isEqualTo("Verified");
+ assertThat(approvals.get(0).getValue()).isEqualTo(1);
+ assertThat(approvals.get(0).isPostSubmit()).isFalse();
+ assertThat(approvals.get(1).getAccountId()).isEqualTo(ownerId);
+ assertThat(approvals.get(1).getLabel()).isEqualTo("Code-Review");
+ assertThat(approvals.get(1).getValue()).isEqualTo(2);
+ assertThat(approvals.get(1).isPostSubmit()).isFalse(); // During submit.
+ assertThat(approvals.get(2).getAccountId()).isEqualTo(otherId);
+ assertThat(approvals.get(2).getLabel()).isEqualTo("Other-Label");
+ assertThat(approvals.get(2).getValue()).isEqualTo(2);
+ assertThat(approvals.get(2).isPostSubmit()).isTrue();
+ }
+
+ @Test
public void multipleReviewers() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/NoteDbChangeStateTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/NoteDbChangeStateTest.java
index 216f71b..f2bf2be 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/NoteDbChangeStateTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/NoteDbChangeStateTest.java
@@ -20,7 +20,6 @@
import static com.google.gerrit.server.notedb.NoteDbChangeState.parse;
import static org.eclipse.jgit.lib.ObjectId.zeroId;
-import com.google.common.base.Optional;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
@@ -33,6 +32,8 @@
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
+import java.util.Optional;
+
/** Unit tests for {@link NoteDbChangeState}. */
public class NoteDbChangeStateTest {
static {
@@ -134,7 +135,7 @@
// Static factory methods to avoid type arguments when using as method args.
private static Optional<ObjectId> noMetaId() {
- return Optional.absent();
+ return Optional.empty();
}
private static Optional<ObjectId> metaId(ObjectId id) {
diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbMode.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbMode.java
index 103fee3..23950d4 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbMode.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbMode.java
@@ -14,8 +14,9 @@
package com.google.gerrit.testutil;
+import static com.google.common.base.Preconditions.checkArgument;
+
import com.google.common.base.Enums;
-import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
@@ -47,12 +48,10 @@
return OFF;
}
value = value.toUpperCase().replace("-", "_");
- Optional<NoteDbMode> mode = Enums.getIfPresent(NoteDbMode.class, value);
- if (!mode.isPresent()) {
- throw new IllegalArgumentException(
- "Invalid value for " + VAR + ": " + System.getenv(VAR));
- }
- return mode.get();
+ NoteDbMode mode = Enums.getIfPresent(NoteDbMode.class, value).orNull();
+ checkArgument(mode != null,
+ "Invalid value for %s: %s", VAR, System.getenv(VAR));
+ return mode;
}
public static boolean readWrite() {
diff --git a/lib/asciidoctor/java/AsciiDoctor.java b/lib/asciidoctor/java/AsciiDoctor.java
index c66c4aa..1871b0c 100644
--- a/lib/asciidoctor/java/AsciiDoctor.java
+++ b/lib/asciidoctor/java/AsciiDoctor.java
@@ -24,6 +24,7 @@
import org.kohsuke.args4j.CmdLineException;
import org.kohsuke.args4j.CmdLineParser;
import org.kohsuke.args4j.Option;
+
import java.io.BufferedReader;
import java.io.File;
import java.io.FileInputStream;
diff --git a/lib/prolog/prolog.bzl b/lib/prolog/prolog.bzl
index d4e9e08..995d81c 100644
--- a/lib/prolog/prolog.bzl
+++ b/lib/prolog/prolog.bzl
@@ -18,7 +18,7 @@
name,
srcs,
deps = [],
- visibility = []):
+ **kwargs):
genrule2(
name = name + '__pl2j',
cmd = '$(location //lib/prolog:compiler_bin) ' +
@@ -32,5 +32,5 @@
name = name,
srcs = [':' + name + '__pl2j'],
deps = ['//lib/prolog:runtime'] + deps,
- visibility = visibility,
+ **kwargs
)
diff --git a/plugins/cookbook-plugin b/plugins/cookbook-plugin
index 09981c0..6cb2fcd 160000
--- a/plugins/cookbook-plugin
+++ b/plugins/cookbook-plugin
@@ -1 +1 @@
-Subproject commit 09981c0638f7241a4f435baaa96bd6112a1edaa9
+Subproject commit 6cb2fcdb54d73abf572a5fbb45db644c56f657ce
diff --git a/plugins/reviewnotes b/plugins/reviewnotes
index e9c66c6..45f6975 160000
--- a/plugins/reviewnotes
+++ b/plugins/reviewnotes
@@ -1 +1 @@
-Subproject commit e9c66c6f08edb641d3c935c2fdcaa3fbf3a85d29
+Subproject commit 45f69757be42ac61dfba915b6e2801525416f1af
diff --git a/polygerrit-ui/app/behaviors/gr-path-list-behavior/gr-path-list-behavior_test.html b/polygerrit-ui/app/behaviors/gr-path-list-behavior/gr-path-list-behavior_test.html
index 530b7be..adf0bf1 100644
--- a/polygerrit-ui/app/behaviors/gr-path-list-behavior/gr-path-list-behavior_test.html
+++ b/polygerrit-ui/app/behaviors/gr-path-list-behavior/gr-path-list-behavior_test.html
@@ -13,6 +13,8 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
+<!-- Polymer included for the html import polyfill. -->
+<script src="../../../bower_components/webcomponentsjs/webcomponents.min.js"></script>
<script src="../../bower_components/web-component-tester/browser.js"></script>
<title>gr-path-list-behavior</title>
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index 09d5b84..96c6193 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -66,6 +66,7 @@
<section hidden$="[[!_actionCount(actions.*, _additionalActions.*)]]">
<template is="dom-repeat" items="[[_changeActionValues]]" as="action">
<gr-button title$="[[action.title]]"
+ hidden$="[[_computeActionHidden(action.__key, _hiddenChangeActions.*)]]"
primary$="[[action.__primary]]"
hidden$="[[!action.enabled]]"
data-action-key$="[[action.__key]]"
@@ -78,6 +79,7 @@
<section hidden$="[[!_actionCount(_revisionActions.*, _additionalActions.*)]]">
<template is="dom-repeat" items="[[_revisionActionValues]]" as="action">
<gr-button title$="[[action.title]]"
+ hidden$="[[_computeActionHidden(action.__key, _hiddenRevisionActions.*)]]"
primary$="[[action.__primary]]"
disabled$="[[!action.enabled]]"
data-action-key$="[[action.__key]]"
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 9960f90..e35d3cb 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -102,6 +102,14 @@
type: Array,
value: function() { return []; },
},
+ _hiddenChangeActions: {
+ type: Array,
+ value: function() { return []; },
+ },
+ _hiddenRevisionActions: {
+ type: Array,
+ value: function() { return []; },
+ },
},
ActionType: ActionType,
@@ -169,6 +177,24 @@
], value);
},
+ setActionHidden: function(type, key, hidden) {
+ var path;
+ if (type === ActionType.CHANGE) {
+ path = '_hiddenChangeActions';
+ } else if (type === ActionType.REVISION) {
+ path = '_hiddenRevisionActions';
+ } else {
+ throw Error('Invalid action type given: ' + type);
+ }
+
+ var idx = this.get(path).indexOf(key);
+ if (hidden && idx === -1) {
+ this.push(path, key);
+ } else if (!hidden && idx !== -1) {
+ this.splice(path, idx, 1);
+ }
+ },
+
_indexOfActionButtonWithKey: function(key) {
for (var i = 0; i < this._additionalActions.length; i++) {
if (this._additionalActions[i].__key === key) {
@@ -270,6 +296,12 @@
this._getRevision(this.change, this.patchNum));
},
+ _computeActionHidden: function(key, hiddenActionsChangeRecord) {
+ var hiddenActions =
+ (hiddenActionsChangeRecord && hiddenActionsChangeRecord.base) || [];
+ return hiddenActions.indexOf(key) !== -1;
+ },
+
_getRevision: function(change, patchNum) {
var num = window.parseInt(patchNum, 10);
for (var hash in change.revisions) {
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index a342c87..5fe41ec 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -97,6 +97,64 @@
return element.reload();
});
+ test('hide revision action', function(done) {
+ flush(function() {
+ var buttonEl = element.$$('[data-action-key="submit"]');
+ assert.isOk(buttonEl);
+ assert.isFalse(buttonEl.hasAttribute('hidden'));
+ assert.throws(element.setActionHidden.bind(element, 'invalid type'));
+ element.setActionHidden(element.ActionType.REVISION,
+ element.RevisionActions.SUBMIT, true);
+ assert.lengthOf(element._hiddenRevisionActions, 1);
+ element.setActionHidden(element.ActionType.REVISION,
+ element.RevisionActions.SUBMIT, true);
+ assert.lengthOf(element._hiddenRevisionActions, 1);
+ flush(function() {
+ var buttonEl = element.$$('[data-action-key="submit"]');
+ assert.isOk(buttonEl);
+ assert.isTrue(buttonEl.hasAttribute('hidden'));
+
+ element.setActionHidden(element.ActionType.REVISION,
+ element.RevisionActions.SUBMIT, false);
+ flush(function() {
+ var buttonEl = element.$$('[data-action-key="submit"]');
+ assert.isOk(buttonEl);
+ assert.isFalse(buttonEl.hasAttribute('hidden'));
+ done();
+ });
+ });
+ });
+ });
+
+ test('hide change action', function(done) {
+ flush(function() {
+ var buttonEl = element.$$('[data-action-key="/"]');
+ assert.isOk(buttonEl);
+ assert.isFalse(buttonEl.hasAttribute('hidden'));
+ assert.throws(element.setActionHidden.bind(element, 'invalid type'));
+ element.setActionHidden(element.ActionType.CHANGE,
+ element.ChangeActions.DELETE, true);
+ assert.lengthOf(element._hiddenChangeActions, 1);
+ element.setActionHidden(element.ActionType.CHANGE,
+ element.ChangeActions.DELETE, true);
+ assert.lengthOf(element._hiddenChangeActions, 1);
+ flush(function() {
+ var buttonEl = element.$$('[data-action-key="/"]');
+ assert.isOk(buttonEl);
+ assert.isTrue(buttonEl.hasAttribute('hidden'));
+
+ element.setActionHidden(element.ActionType.CHANGE,
+ element.RevisionActions.DELETE, false);
+ flush(function() {
+ var buttonEl = element.$$('[data-action-key="/"]');
+ assert.isOk(buttonEl);
+ assert.isFalse(buttonEl.hasAttribute('hidden'));
+ done();
+ });
+ });
+ });
+ });
+
test('buttons show', function(done) {
flush(function() {
var buttonEls = Polymer.dom(element.root).querySelectorAll('gr-button');
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
index 108ac92..3ad2d257 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
@@ -59,6 +59,9 @@
.notApproved {
background-color: #ffd4d4;
}
+ .labelStatus {
+ max-width: 9em;
+ }
@media screen and (max-width: 50em), screen and (min-width: 75em) {
:host {
display: table;
@@ -174,6 +177,14 @@
</span>
</section>
</template>
+ <template is="dom-if" if="[[_showLabelStatus]]">
+ <section>
+ <span class="title">Label Status</span>
+ <span class="value labelStatus">
+ [[_computeSubmitStatus(change.labels)]]
+ </span>
+ </section>
+ </template>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
</template>
<script src="gr-change-metadata.js"></script>
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
index 661a296..184215e 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
@@ -37,6 +37,10 @@
type: Boolean,
computed: '_computeShowReviewersByState(serverConfig)',
},
+ _showLabelStatus: {
+ type: Boolean,
+ computed: '_computeShowLabelStatus(change)',
+ },
},
behaviors: [
@@ -143,5 +147,30 @@
}
}.bind(this));
},
+
+ _computeShowLabelStatus: function(change) {
+ var isNewChange = change.status === this.ChangeStatus.NEW;
+ var hasLabels = Object.keys(change.labels).length > 0;
+ return isNewChange && hasLabels;
+ },
+
+ _computeSubmitStatus: function(labels) {
+ var missingLabels = [];
+ var output = '';
+ for (var label in labels) {
+ var obj = labels[label];
+ if (!obj.optional && !obj.approved) {
+ missingLabels.push(label);
+ }
+ }
+ if (missingLabels.length) {
+ output += 'Needs ';
+ output += missingLabels.join(' and ');
+ output += missingLabels.length > 1 ? ' labels' : ' label';
+ } else {
+ output = 'Ready to ubmit';
+ }
+ return output;
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
index 22080e8..281636c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
@@ -78,6 +78,22 @@
assert.isTrue(hasCc());
});
+ test('computes submit status', function() {
+ var labels = {};
+ assert.equal(element._computeSubmitStatus(labels), 'Ready to Submit');
+ labels = { test: {} };
+ assert.equal(element._computeSubmitStatus(labels), 'Needs test Label');
+ labels.test.approved = true;
+ assert.equal(element._computeSubmitStatus(labels), 'Ready to Submit');
+ labels.test.approved = false;
+ labels.test.optional = true;
+ assert.equal(element._computeSubmitStatus(labels), 'Ready to Submit');
+ labels.test.optional = false;
+ labels.test2 = {};
+ assert.equal(element._computeSubmitStatus(labels),
+ 'Needs test and test2 Labels');
+ });
+
suite('remove reviewer votes', function() {
var sandbox;
setup(function() {
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
index e4ab44b..48a3d60 100644
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
+++ b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
@@ -24,6 +24,7 @@
:host {
display: block;
font-family: var(--monospace-font-family);
+ word-wrap: break-word;
}
.file {
border-top: 1px solid #ddd;
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 78e4634..5c950dd 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -40,7 +40,13 @@
margin-bottom: .5em;
}
.rightControls {
+ display: flex;
+ flex-wrap: wrap;
font-weight: normal;
+ justify-content: flex-end;
+ }
+ .separator {
+ margin: 0 .25em;
}
.reviewed,
.status {
@@ -150,9 +156,9 @@
<div>Files</div>
<div class="rightControls">
<gr-button link on-tap="_expandAllDiffs">Show diffs</gr-button>
- /
+ <span class="separator">/</span>
<gr-button link on-tap="_collapseAllDiffs">Hide diffs</gr-button>
- /
+ <span class="separator">/</span>
<select
id="modeSelect"
is="gr-select"
@@ -161,7 +167,7 @@
<option value="SIDE_BY_SIDE">Side By Side</option>
<option value="UNIFIED_DIFF">Unified</option>
</select>
- /
+ <span class="separator">/</span>
<label>
Diff against
<select id="patchChange" on-change="_handlePatchChange">
diff --git a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
index 6e88267..7a7e761 100644
--- a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
+++ b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
@@ -98,13 +98,6 @@
<td><span class="key">u</span></td>
<td>Up to change list</td>
</tr>
- <tr>
- <td>
- <span class="key modifier">Shift</span>
- <span class="key">i</span>
- </td>
- <td>Show/hide inline diffs</td>
- </tr>
</tbody>
<!-- Diff View -->
<tbody hidden$="[[!_computeInView(view, 'gr-diff-view')]]" hidden>
@@ -177,6 +170,13 @@
<td>Show selected file</td>
</tr>
<tr>
+ <td>
+ <span class="key modifier">Shift</span>
+ <span class="key">i</span>
+ </td>
+ <td>Show/hide all inline diffs</td>
+ </tr>
+ <tr>
<td><span class="key">i</span></td>
<td>Show/hide selected inline diff</td>
</tr>
@@ -214,6 +214,17 @@
<td>Go to previous comment thread</td>
</tr>
<tr>
+ <td><span class="key">e</span></td>
+ <td>Expand all comment threads</td>
+ </tr>
+ <tr>
+ <td>
+ <span class="key modifier">Shift</span>
+ <span class="key">e</span>
+ </td>
+ <td>Collapse all comment threads</td>
+ </tr>
+ <tr>
<td>
<span class="key modifier">Shift</span>
<span class="key">←</span>
@@ -277,6 +288,17 @@
<td>Show previous comment thread</td>
</tr>
<tr>
+ <td><span class="key">e</span></td>
+ <td>Expand all comment threads</td>
+ </tr>
+ <tr>
+ <td>
+ <span class="key modifier">Shift</span>
+ <span class="key">e</span>
+ </td>
+ <td>Collapse all comment threads</td>
+ </tr>
+ <tr>
<td>
<span class="key modifier">Shift</span>
<span class="key">←</span>
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
index 32dd687..9c0e902 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
@@ -111,7 +111,7 @@
locationChanged: function() {
var page = '';
var pathname = this._getPathname();
- if (pathname.startsWith('/q/')) {
+ if (pathname.indexOf('/q/') === 0) {
page = '/q/';
} else if (pathname.match(CHANGE_VIEW_REGEX)) { // change view
page = '/c/';
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index 3ad7c74..0a595c7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -394,7 +394,7 @@
},
/**
- * @returns {Boolean} whether any of the lines in _groups are longer
+ * @return {Boolean} whether any of the lines in _groups are longer
* than SYNTAX_MAX_LINE_LENGTH.
*/
_anyLineTooLong: function() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
index 305c36a..963084a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
@@ -29,6 +29,10 @@
type: Array,
value: function() { return []; },
},
+ keyEventTarget: {
+ type: Object,
+ value: function() { return document.body; },
+ },
patchNum: String,
path: String,
projectConfig: Object,
@@ -41,6 +45,10 @@
_orderedComments: Array,
},
+ behaviors: [
+ Gerrit.KeyboardShortcutBehavior,
+ ],
+
listeners: {
'comment-update': '_handleCommentUpdate',
},
@@ -80,6 +88,22 @@
this._orderedComments = this._sortedComments(this.comments);
},
+ _handleKey: function(e) {
+ if (this.shouldSupressKeyboardShortcut(e)) { return; }
+ if (e.keyCode === 69) { // 'e'
+ e.preventDefault();
+ this._expandCollapseComments(e.shiftKey);
+ }
+ },
+
+ _expandCollapseComments: function(actionIsCollapse) {
+ var comments =
+ Polymer.dom(this.root).querySelectorAll('gr-diff-comment');
+ comments.forEach(function(comment) {
+ comment.collapsed = actionIsCollapse;
+ });
+ },
+
_sortedComments: function(comments) {
comments.sort(function(c1, c2) {
var c1Date = c1.__date || util.parseDate(c1.updated);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
index 641dc0f..eb87f56 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
@@ -54,30 +54,25 @@
message: 'i like you, too',
in_reply_to: 'sallys_confession',
updated: '2015-12-25 15:00:20.396000000',
- },
- {
+ }, {
id: 'sallys_confession',
message: 'i like you, jack',
updated: '2015-12-24 15:00:20.396000000',
- },
- {
+ }, {
id: 'sally_to_dr_finklestein',
message: 'i’m running away',
updated: '2015-10-31 09:00:20.396000000',
- },
- {
+ }, {
id: 'sallys_defiance',
in_reply_to: 'sally_to_dr_finklestein',
message: 'i will poison you so i can get away',
updated: '2015-10-31 15:00:20.396000000',
- },
- {
+ }, {
id: 'dr_finklesteins_response',
in_reply_to: 'sally_to_dr_finklestein',
message: 'no i will pull a thread and your arm will fall off',
updated: '2015-10-31 11:00:20.396000000'
- },
- {
+ }, {
id: 'sallys_mission',
message: 'i have to find santa',
updated: '2015-12-24 21:00:20.396000000'
@@ -89,31 +84,26 @@
id: 'sally_to_dr_finklestein',
message: 'i’m running away',
updated: '2015-10-31 09:00:20.396000000',
- },
- {
+ }, {
id: 'dr_finklesteins_response',
in_reply_to: 'sally_to_dr_finklestein',
message: 'no i will pull a thread and your arm will fall off',
updated: '2015-10-31 11:00:20.396000000'
- },
- {
+ }, {
id: 'sallys_defiance',
in_reply_to: 'sally_to_dr_finklestein',
message: 'i will poison you so i can get away',
updated: '2015-10-31 15:00:20.396000000',
- },
- {
+ }, {
id: 'sallys_confession',
message: 'i like you, jack',
updated: '2015-12-24 15:00:20.396000000',
- },
- {
+ }, {
id: 'jacks_reply',
message: 'i like you, too',
in_reply_to: 'sallys_confession',
updated: '2015-12-25 15:00:20.396000000',
- },
- {
+ }, {
id: 'sallys_mission',
message: 'i have to find santa',
updated: '2015-12-24 21:00:20.396000000'
@@ -247,20 +237,17 @@
message: 'i like you, too',
in_reply_to: 'sallys_confession',
updated: '2015-12-25 15:00:20.396000000',
- },
- {
+ }, {
id: 'sallys_confession',
in_reply_to: 'nonexistent_comment',
message: 'i like you, jack',
updated: '2015-12-24 15:00:20.396000000',
- },
- {
+ }, {
id: 'sally_to_dr_finklestein',
in_reply_to: 'nonexistent_comment',
message: 'i’m running away',
updated: '2015-10-31 09:00:20.396000000',
- },
- {
+ }, {
id: 'sallys_defiance',
message: 'i will poison you so i can get away',
updated: '2015-10-31 15:00:20.396000000',
@@ -268,5 +255,37 @@
element.comments = comments;
assert.equal(4, element._orderedComments.length);
});
+
+ test('keyboard shortcuts', function() {
+ var comments = [
+ {
+ id: 'jacks_reply',
+ message: 'i like you, too',
+ in_reply_to: 'sallys_confession',
+ updated: '2015-12-25 15:00:20.396000000',
+ }, {
+ id: 'sallys_confession',
+ in_reply_to: 'nonexistent_comment',
+ message: 'i like you, jack',
+ updated: '2015-12-24 15:00:20.396000000',
+ }, {
+ id: 'sally_to_dr_finklestein',
+ in_reply_to: 'nonexistent_comment',
+ message: 'i’m running away',
+ updated: '2015-10-31 09:00:20.396000000',
+ }, {
+ id: 'sallys_defiance',
+ message: 'i will poison you so i can get away',
+ updated: '2015-10-31 15:00:20.396000000',
+ }];
+ element.comments = comments;
+ var expandCollapseStub = sinon.stub(element, '_expandCollapseComments');
+ MockInteractions.pressAndReleaseKeyOn(element, 69); // 'e'
+ assert.isTrue(expandCollapseStub.lastCall.calledWith(false));
+
+ MockInteractions.pressAndReleaseKeyOn(element, 69, 'shift'); // 'e'
+ assert.isTrue(expandCollapseStub.lastCall.calledWith(true));
+ expandCollapseStub.restore();
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
index 864712d..ebb1f87 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
@@ -170,9 +170,9 @@
<div class="show-hide">
<label class="show-hide">
<input type="checkbox" class="show-hide"
- checked$="[[_commentCollapsed]]"
+ checked$="[[collapsed]]"
on-change="_handleToggleCollapsed">
- [[_computeShowHideText(_commentCollapsed)]]
+ [[_computeShowHideText(collapsed)]]
</label>
</div>
</div>
@@ -186,7 +186,7 @@
<gr-linked-text class="message"
pre
content="[[comment.message]]"
- collapsed="[[_commentCollapsed]]"
+ collapsed="[[collapsed]]"
config="[[projectConfig.commentlinks]]"></gr-linked-text>
<div class="actions" hidden$="[[!showActions]]">
<gr-button class="action reply" on-tap="_handleReply">Reply</gr-button>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
index 791f949..2abd0a8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
@@ -81,7 +81,7 @@
},
patchNum: String,
showActions: Boolean,
- _commentCollapsed: {
+ collapsed: {
type: Boolean,
value: true,
observer: '_toggleCollapseClass',
@@ -103,7 +103,7 @@
attached: function() {
if (this.editing) {
- this._commentCollapsed = false;
+ this.collapsed = false;
}
},
@@ -226,11 +226,11 @@
},
_handleToggleCollapsed: function() {
- this._commentCollapsed = !this._commentCollapsed;
+ this.collapsed = !this.collapsed;
},
- _toggleCollapseClass: function(_commentCollapsed) {
- if (_commentCollapsed) {
+ _toggleCollapseClass: function(collapsed) {
+ if (collapsed) {
this.$.container.classList.add('collapsed');
} else {
this.$.container.classList.remove('collapsed');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
index b05746e..0a591ae 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
@@ -66,6 +66,7 @@
test('collapsible comments', function() {
// When a comment (not draft) is loaded, it should be collapsed
+ assert.isTrue(element.collapsed);
assert.isFalse(isVisible(element.$$('gr-linked-text')),
'gr-linked-text is not visible');
assert.isFalse(isVisible(element.$$('.actions')),
@@ -80,6 +81,7 @@
// When the header row is clicked, the comment should expand
MockInteractions.tap(element.$.header);
+ assert.isFalse(element.collapsed);
assert.isTrue(isVisible(element.$$('gr-linked-text')),
'gr-linked-text is visible');
assert.isTrue(isVisible(element.$$('.actions')),
@@ -124,6 +126,29 @@
'Should navigate to ' + dest + ' without triggering nav');
showStub.restore();
});
+
+ test('comment expand and collapse', function() {
+ element.collapsed = true;
+ assert.isFalse(isVisible(element.$$('gr-linked-text')),
+ 'gr-linked-text is not visible');
+ assert.isFalse(isVisible(element.$$('.actions')),
+ 'actions are not visible');
+ assert.isFalse(isVisible(element.$$('iron-autogrow-textarea')),
+ 'textarea is not visible');
+ assert.isTrue(isVisible(element.$$('.collapsedContent')),
+ 'header middle content is visible');
+
+ element.collapsed = false;
+ assert.isFalse(element.collapsed);
+ assert.isTrue(isVisible(element.$$('gr-linked-text')),
+ 'gr-linked-text is visible');
+ assert.isTrue(isVisible(element.$$('.actions')),
+ 'actions are visible');
+ assert.isFalse(isVisible(element.$$('iron-autogrow-textarea')),
+ 'textarea is not visible');
+ assert.isFalse(isVisible(element.$$('.collapsedContent')),
+ 'header middle content is is not visible');
+ });
});
suite('gr-diff-comment draft tests', function() {
@@ -213,6 +238,7 @@
assert.ok(e.detail.comment);
done();
});
+ assert.isTrue(element.collapsed);
assert.isFalse(isVisible(element.$$('gr-linked-text')),
'gr-linked-text is not visible');
assert.isFalse(isVisible(element.$$('.actions')),
@@ -223,6 +249,7 @@
'header middle content is visible');
MockInteractions.tap(element.$.header);
+ assert.isFalse(element.collapsed);
assert.isTrue(isVisible(element.$$('gr-linked-text')),
'gr-linked-text is visible');
assert.isTrue(isVisible(element.$$('.actions')),
@@ -235,6 +262,7 @@
// When the edit button is pressed, should still see the actions
// and also textarea
MockInteractions.tap(element.$$('.edit'));
+ assert.isFalse(element.collapsed);
assert.isFalse(isVisible(element.$$('gr-linked-text')),
'gr-linked-text is not visible');
assert.isTrue(isVisible(element.$$('.actions')),
@@ -247,6 +275,7 @@
// When toggle again, everything should be hidden except for textarea
// and header middle content should be visible
MockInteractions.tap(element.$.header);
+ assert.isTrue(element.collapsed);
assert.isFalse(isVisible(element.$$('gr-linked-text')),
'gr-linked-text is not visible');
assert.isFalse(isVisible(element.$$('.actions')),
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.html b/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.html
index 57a2e1f..7a263d0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences.html
@@ -101,6 +101,13 @@
bind-value="{{_newPrefs.tab_size}}">
</div>
<div class="pref">
+ <label for="fontSizeInput">Font Size</label>
+ <input is="iron-input" type="number" id="fontSizeInput"
+ prevent-invalid-input
+ allowed-pattern="[0-9]"
+ bind-value="{{_newPrefs.font_size}}">
+ </div>
+ <div class="pref">
<label for="showTabsInput">Show tabs</label>
<input is="iron-input" type="checkbox" id="showTabsInput"
on-tap="_handleShowTabsTap">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences_test.html b/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences_test.html
index 3c4d457..c595663 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-preferences/gr-diff-preferences_test.html
@@ -41,6 +41,7 @@
test('model changes', function() {
element.prefs = {
context: 10,
+ font_size: 12,
line_length: 100,
show_tabs: true,
tab_size: 8,
@@ -51,11 +52,13 @@
element.$.contextSelect.value = '50';
element.fire('change', {}, {node: element.$.contextSelect});
element.$.columnsInput.bindValue = 80;
+ element.$.fontSizeInput.bindValue = 10;
element.$.tabSizeInput.bindValue = 4;
MockInteractions.tap(element.$.showTabsInput);
MockInteractions.tap(element.$.syntaxHighlightInput);
assert.equal(element._newPrefs.context, 50);
+ assert.equal(element._newPrefs.font_size, 10);
assert.equal(element._newPrefs.line_length, 80);
assert.equal(element._newPrefs.tab_size, 4);
assert.isFalse(element._newPrefs.show_tabs);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
index f91c8ef..d3cc461 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
@@ -258,7 +258,7 @@
}
// If there is a range to hide.
- if (context !== WHOLE_FILE && hiddenRange[1] - hiddenRange[0] > 0) {
+ if (context !== WHOLE_FILE && hiddenRange[1] - hiddenRange[0] > 1) {
var linesBeforeCtx = lines.slice(0, hiddenRange[0]);
var hiddenLines = lines.slice(hiddenRange[0], hiddenRange[1]);
var linesAfterCtx = lines.slice(hiddenRange[1]);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.html b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.html
index c89d9b5..f6d0e37 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.html
@@ -511,6 +511,17 @@
assert.equal(result[0].lines.length, rows.length);
});
+ test('_sharedGroupsFromRows no single line collapse', function() {
+ rows = rows.slice(0, 7);
+ var context = 3;
+ var result = element._sharedGroupsFromRows(
+ rows, context, 10, 100);
+
+ // Results in one uncollapsed group with all rows.
+ assert.equal(result.length, 1, 'Results in one group');
+ assert.equal(result[0].lines.length, rows.length);
+ });
+
test('_deltaLinesFromRows', function() {
var startLineNum = 10;
var result = element._deltaLinesFromRows(GrDiffLine.Type.ADD, rows,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js
index e6d3653..c68e925 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js
@@ -112,12 +112,10 @@
_handleCopy: function(e) {
var commentSelected = false;
var target = this._getCopyEventTarget(e);
+ if (target.type === 'textarea') { return; }
+ if (!this._elementDescendedFromClass(target, 'content')) { return; }
if (this.classList.contains(SelectionClass.COMMENT)) {
commentSelected = true;
- } else {
- if (!this._elementDescendedFromClass(target, 'content')) {
- return;
- }
}
var lineEl = this.diffBuilder.getLineElByChild(target);
if (!lineEl) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.html b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.html
index 71fa233..774430a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.html
@@ -79,6 +79,18 @@
<div class="contentText" data-side="right">some other text</div>
</td>
</tr>
+ <tr>
+ <td class="lineNum left" data-value="4">4</td>
+ <td class="content">
+ <div class="contentText" data-side="left">ga ga</div>
+ <div data-side="left">
+ <div class="gr-diff-comment-thread">
+ <textarea data-side="right">test for textarea copying</textarea>
+ </div>
+ </div>
+ </td>
+ <td class="lineNum right" data-value="4">4</td>
+ </tr>
</table>
</gr-diff-selection>
</template>
@@ -87,14 +99,14 @@
<script>
suite('gr-diff-selection', function() {
var element;
- var copyEventStub;
+ var sandbox;
var emulateCopyOn = function(target) {
var fakeEvent = {
target: target,
- preventDefault: sinon.stub(),
+ preventDefault: sandbox.stub(),
clipboardData: {
- setData: sinon.stub(),
+ setData: sandbox.stub(),
},
};
element._getCopyEventTarget.returns(target);
@@ -104,10 +116,11 @@
setup(function() {
element = fixture('basic');
- sinon.stub(element, '_getCopyEventTarget');
+ sandbox = sinon.sandbox.create();
+ sandbox.stub(element, '_getCopyEventTarget');
element._cachedDiffBuilder = {
- getLineElByChild: sinon.stub().returns({}),
- getSideByLineEl: sinon.stub(),
+ getLineElByChild: sandbox.stub().returns({}),
+ getSideByLineEl: sandbox.stub(),
diffElement: element.querySelector('#diffTable'),
};
element.diff = {
@@ -128,6 +141,10 @@
};
});
+ teardown(function() {
+ sandbox.restore();
+ });
+
test('applies selected-left on left side click', function() {
element.classList.add('selected-right');
element._cachedDiffBuilder.getSideByLineEl.returns('left');
@@ -150,26 +167,26 @@
});
test('ignores copy for non-content Element', function() {
- sinon.stub(element, '_getSelectedText');
+ sandbox.stub(element, '_getSelectedText');
emulateCopyOn(element.querySelector('.other'));
assert.isFalse(element._getSelectedText.called);
});
test('asks for text for left side Elements', function() {
element._cachedDiffBuilder.getSideByLineEl.returns('left');
- sinon.stub(element, '_getSelectedText');
+ sandbox.stub(element, '_getSelectedText');
emulateCopyOn(element.querySelector('div.contentText'));
assert.deepEqual(['left', false], element._getSelectedText.lastCall.args);
});
test('reacts to copy for content Elements', function() {
- sinon.stub(element, '_getSelectedText');
+ sandbox.stub(element, '_getSelectedText');
emulateCopyOn(element.querySelector('div.contentText'));
assert.isTrue(element._getSelectedText.called);
});
test('copy event is prevented for content Elements', function() {
- sinon.stub(element, '_getSelectedText');
+ sandbox.stub(element, '_getSelectedText');
element._cachedDiffBuilder.getSideByLineEl.returns('left');
element._getSelectedText.returns('test');
var event = emulateCopyOn(element.querySelector('div.contentText'));
@@ -177,7 +194,7 @@
});
test('inserts text into clipboard on copy', function() {
- sinon.stub(element, '_getSelectedText').returns('the text');
+ sandbox.stub(element, '_getSelectedText').returns('the text');
var event = emulateCopyOn(element.querySelector('div.contentText'));
assert.deepEqual(
['Text', 'the text'], event.clipboardData.setData.lastCall.args);
@@ -219,5 +236,13 @@
element._getSelectedText('left', true));
selection.removeAllRanges();
});
+
+ test('defers to default behavior for textarea', function() {
+ element.classList.add('selected-left');
+ element.classList.remove('selected-right');
+ var selectedTextSpy = sandbox.spy(element, '_getSelectedText');
+ emulateCopyOn(element.querySelector('textarea'));
+ assert.isFalse(selectedTextSpy.called);
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index a2af8c5..7ae5c34 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -129,6 +129,9 @@
display: block;
width: 100%;
}
+ .mobileJumpToFileContainer select {
+ width: 100%;
+ }
}
</style>
<header>
@@ -168,7 +171,7 @@
<option
value$="[[path]]"
selected$="[[_computeFileSelected(path, _path)]]">
- [[_computeFileDisplayName(path)]]
+ [[_computeTruncatedFileDisplayName(path)]]
</option>
</template>
</select>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 03609c0..c5779a4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -16,6 +16,8 @@
var COMMIT_MESSAGE_PATH = '/COMMIT_MSG';
+ var MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX = 900;
+
var DiffViewMode = {
SIDE_BY_SIDE: 'SIDE_BY_SIDE',
UNIFIED: 'UNIFIED_DIFF',
@@ -105,12 +107,17 @@
}
}.bind(this));
if (this.changeViewState.diffMode === null) {
- // Initialize with user's diff mode preference. Default to
- // SIDE_BY_SIDE in the meantime.
- this.set('changeViewState.diffMode', DiffViewMode.SIDE_BY_SIDE);
- this.$.restAPI.getPreferences().then(function(prefs) {
- this.set('changeViewState.diffMode', prefs.diff_view);
- }.bind(this));
+ // If screen size is small, always default to unified view.
+ if (this._getWindowWidth() < MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX) {
+ this.set('changeViewState.diffMode', DiffViewMode.UNIFIED);
+ } else {
+ // Initialize with user's diff mode preference. Default to
+ // SIDE_BY_SIDE in the meantime.
+ this.set('changeViewState.diffMode', DiffViewMode.SIDE_BY_SIDE);
+ this.$.restAPI.getPreferences().then(function(prefs) {
+ this.set('changeViewState.diffMode', prefs.diff_view);
+ }.bind(this));
+ }
}
if (this._path) {
@@ -155,6 +162,10 @@
return this.$.restAPI.getPreferences();
},
+ _getWindowWidth: function() {
+ return window.innerWidth;
+ },
+
_handleReviewedChange: function(e) {
this._setReviewed(Polymer.dom(e).rootTarget.checked);
},
@@ -427,7 +438,22 @@
},
_computeFileDisplayName: function(path) {
- return path == COMMIT_MESSAGE_PATH ? 'Commit message' : path;
+ return path === COMMIT_MESSAGE_PATH ? 'Commit message' : path;
+ },
+
+ _computeTruncatedFileDisplayName: function(path) {
+ return path === COMMIT_MESSAGE_PATH ?
+ 'Commit message' : this._shortenPath(path);
+ },
+
+ _shortenPath: function(path) {
+ var pathPieces = path.split('/');
+
+ if (pathPieces.length < 2) {
+ return path;
+ }
+ // Character is an ellipsis.
+ return '\u2026/' + pathPieces.pop();
},
_computeFileSelected: function(path, currentPath) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
index 566c6a1..ec57726 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
@@ -491,6 +491,39 @@
assert.equal(select.value, 'SIDE_BY_SIDE');
});
+ test('unified view is always default on small screens', function() {
+ var resolvePrefs;
+ var prefsPromise = new Promise(function(resolve) {
+ resolvePrefs = resolve;
+ });
+
+ var getPreferencesStub = sinon.stub(element.$.restAPI, 'getPreferences',
+ function() { return prefsPromise; });
+
+ // Attach a new gr-diff-view so we can intercept the preferences fetch.
+ var view = document.createElement('gr-diff-view');
+
+ view.changeViewState = {diffMode: null};
+
+ sinon.stub(view, '_getWindowWidth', function() {
+ return 800;
+ });
+
+ var select = view.$.modeSelect;
+ fixture('blank').appendChild(view);
+ flushAsynchronousOperations();
+
+ // At this point the diff mode doesn't yet have the user's preference.
+ assert.equal(select.value, 'UNIFIED_DIFF');
+
+ // Receive the overriding preference.
+ resolvePrefs({diff_view: 'SIDE_BY_SIDE'});
+ flushAsynchronousOperations();
+
+ // On small screens, unified should override user perferences
+ assert.equal(select.value, 'UNIFIED_DIFF');
+ });
+
test('_loadHash', function() {
assert.isNotOk(element.$.cursor.initialLineNumber);
@@ -516,5 +549,25 @@
assert.isTrue(element._checkForModifiers({shiftKey: true}));
assert.isFalse(element._checkForModifiers({}));
});
+
+ test('_shortenPath with long path should add ellipsis', function() {
+ var path =
+ 'level1/level2/level3/level4/file.js';
+ var shortenedPath = element._shortenPath(path);
+ // The expected path is truncated with an ellipsis.
+ var expectedPath = '\u2026/file.js';
+ assert.equal(shortenedPath, expectedPath);
+
+ var path = 'level2/file.js';
+ var shortenedPath = element._shortenPath(path);
+ assert.equal(shortenedPath, expectedPath);
+ });
+
+ test('_shortenPath with short path should not add ellipsis', function() {
+ var path = 'file.js';
+ var expectedPath = 'file.js';
+ var shortenedPath = element._shortenPath(path);
+ assert.equal(shortenedPath, expectedPath);
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 4672b31..625d6e8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -113,6 +113,11 @@
max-width: var(--content-width, 80ch);
min-width: var(--content-width, 80ch);
}
+ .content,
+ .lineNum {
+ /* Set font size based the user's diff preference. */
+ font-size: var(--font-size, 12px);
+ }
.content.add .intraline,
.content.add.darkHighlight {
background-color: var(--dark-add-highlight-color);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 1a6759b..8886005 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -359,6 +359,7 @@
_prefsChanged: function(prefs) {
if (!prefs) { return; }
this.customStyle['--content-width'] = prefs.line_length + 'ch';
+ this.customStyle['--font-size'] = prefs.font_size + 'px';
this.updateStyles();
if (this._diff && this._comments) {
@@ -375,6 +376,12 @@
},
_handleGetDiffError: function(response) {
+ // Loading the diff may respond with 409 if the file is too large. In this
+ // case, use a toast error..
+ if (response.status === 409) {
+ this.fire('server-error', {response: response});
+ return;
+ }
this.fire('page-error', {response: response});
},
@@ -386,8 +393,8 @@
this.path,
this._handleGetDiffError.bind(this)).then(function(diff) {
this.filesWeblinks = {
- meta_a: diff.meta_a && diff.meta_a.web_links,
- meta_b: diff.meta_b && diff.meta_b.web_links,
+ meta_a: diff && diff.meta_a && diff.meta_a.web_links,
+ meta_b: diff && diff.meta_b && diff.meta_b.web_links,
};
return diff;
}.bind(this));
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index b9b49c0..20ed61f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -327,6 +327,16 @@
});
content.click();
});
+
+ test('_getDiff handles null diff responses', function(done) {
+ stub('gr-rest-api-interface', {
+ getDiff: function() { return Promise.resolve(null); },
+ });
+ element.changeNum = 123;
+ element.patchRange = {basePatchNum: 1, patchNum: 2};
+ element.path = 'file.txt';
+ element._getDiff().then(done);
+ });
});
suite('logged in', function() {
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.html b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.html
index 9a8ea37..98f6b01 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.html
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.html
@@ -21,26 +21,25 @@
<template>
<style>
:host {
- --gr-arrow-size: .6em;
+ --gr-arrow-size: .65em;
- background-color: #fff;
- border: 1px solid #000;
- border-radius: .5em;
+ background-color: rgba(22, 22, 22, .9);
+ border-radius: 3px;
+ color: #fff;
cursor: pointer;
- padding: .3em;
+ font-family: var(--font-family);
+ padding: .5em .75em;
position: absolute;
white-space: nowrap;
}
.arrow {
- background: #fff;
- border: var(--gr-arrow-size) solid #000;
- border-width: 0 1px 1px 0;
- height: var(--gr-arrow-size);
- left: calc(50% - 1em);
- margin-top: .05em;
+ border: var(--gr-arrow-size) solid transparent;
+ border-top: var(--gr-arrow-size) solid rgba(22, 22, 22, 0.9);
+ height: 0;
+ left: calc(50% - var(--gr-arrow-size));
+ margin-top: .5em;
position: absolute;
- transform: rotate(45deg);
- width: var(--gr-arrow-size);
+ width: 0;
}
</style>
Press <strong>C</strong> to comment.
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
index fbd1ef3..29b3c19 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
@@ -48,7 +48,7 @@
],
listeners: {
- 'tap': '_handleTap',
+ 'mousedown': '_handleMouseDown', // See https://crbug.com/gerrit/4767
},
placeAbove: function(el) {
@@ -87,7 +87,9 @@
}
},
- _handleTap: function() {
+ _handleMouseDown: function(e) {
+ e.preventDefault();
+ e.stopPropagation();
this._fireCreateComment();
},
diff --git a/polygerrit-ui/app/elements/gr-app.js b/polygerrit-ui/app/elements/gr-app.js
index 8ac268e..a188ea1 100644
--- a/polygerrit-ui/app/elements/gr-app.js
+++ b/polygerrit-ui/app/elements/gr-app.js
@@ -166,7 +166,7 @@
_handleLocationChange: function(e) {
var hash = e.detail.hash.substring(1);
var pathname = e.detail.pathname;
- if (pathname.startsWith('/c/') && parseInt(hash, 10) > 0) {
+ if (pathname.indexOf('/c/') === 0 && parseInt(hash, 10) > 0) {
pathname += '@' + hash;
}
this.set('_path', pathname);
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
index 8194c21..45bc6df 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
@@ -232,6 +232,17 @@
</span>
</section>
<section>
+ <span class="title">Font Size</span>
+ <span class="value">
+ <input
+ is="iron-input"
+ type="number"
+ prevent-invalid-input
+ allowed-pattern="[0-9]"
+ bind-value="{{_diffPrefs.font_size}}">
+ </span>
+ </section>
+ <section>
<span class="title">Show Tabs</span>
<span class="value">
<input
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html
index 4e98b43..2862226 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html
@@ -92,6 +92,7 @@
diffPreferences = {
context: 10,
tab_size: 8,
+ font_size: 12,
line_length: 100,
cursor_blink_rate: 0,
intraline_difference: true,
@@ -192,6 +193,8 @@
.firstElementChild.bindValue, diffPreferences.line_length);
assert.equal(valueOf('Tab Width', 'diffPreferences')
.firstElementChild.bindValue, diffPreferences.tab_size);
+ assert.equal(valueOf('Font Size', 'diffPreferences')
+ .firstElementChild.bindValue, diffPreferences.font_size);
assert.equal(valueOf('Show Tabs', 'diffPreferences')
.firstElementChild.checked, diffPreferences.show_tabs);
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
index 31918dd..601a056 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
@@ -118,11 +118,11 @@
},
attached: function() {
- this.listen(document.body, 'click', '_handleBodyClick');
+ this.listen(document.body, 'tap', '_handleBodyTap');
},
detached: function() {
- this.unlisten(document.body, 'click', '_handleBodyClick');
+ this.unlisten(document.body, 'tap', '_handleBodyTap');
},
get focusStart() {
@@ -233,7 +233,7 @@
}
},
- _handleBodyClick: function(e) {
+ _handleBodyTap: function(e) {
var eventPath = Polymer.dom(e).path;
for (var i = 0; i < eventPath.length; i++) {
if (eventPath[i] === this) {
@@ -244,6 +244,7 @@
},
_handleSuggestionTap: function(e) {
+ e.stopPropagation();
this.$.cursor.setCursor(e.target);
this._commit();
this.focus();
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
index 7a26e72..685f568 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
@@ -290,6 +290,7 @@
assert.isTrue(focusSpy.called);
assert.isTrue(commitSpy.called);
assert.isTrue(element.$.suggestions.hasAttribute('hidden'));
+ assert.isTrue(element._focused);
focusSpy.restore();
commitSpy.restore();
});
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.js
index f7c337b..72c7f6e 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.js
@@ -33,6 +33,11 @@
});
};
+ GrChangeActionsInterface.prototype.setActionHidden = function(type, key,
+ hidden) {
+ return this._el.setActionHidden(type, key, hidden);
+ };
+
GrChangeActionsInterface.prototype.add = function(type, label) {
return this._el.addActionButton(type, label);
};
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.html
index 4919a5a..93e676c 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.html
@@ -119,5 +119,22 @@
});
});
});
+
+ test('hide action buttons', function(done) {
+ var key = changeActions.add(changeActions.ActionType.REVISION, 'Bork!');
+ flush(function() {
+ var button = element.$$('[data-action-key="' + key + '"]');
+ assert.isOk(button);
+ assert.isFalse(button.hasAttribute('hidden'));
+ changeActions.setActionHidden(changeActions.ActionType.REVISION, key,
+ true);
+ flush(function() {
+ var button = element.$$('[data-action-key="' + key + '"]');
+ assert.isOk(button);
+ assert.isTrue(button.hasAttribute('hidden'));
+ done();
+ });
+ });
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index 6195272..f46404a 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -191,6 +191,7 @@
auto_hide_diff_table_header: true,
context: 10,
cursor_blink_rate: 0,
+ font_size: 12,
ignore_whitespace: 'IGNORE_NONE',
intraline_difference: true,
line_length: 100,
diff --git a/tools/bzl/pkg_war.bzl b/tools/bzl/pkg_war.bzl
index ef0fe51..aa7d07f 100644
--- a/tools/bzl/pkg_war.bzl
+++ b/tools/bzl/pkg_war.bzl
@@ -124,7 +124,7 @@
outputs = {'war' : '%{name}.war'},
)
-def pkg_war(name, ui = 'ui_optdbg', context = []):
+def pkg_war(name, ui = 'ui_optdbg', context = [], **kwargs):
ui_deps = []
if ui:
ui_deps.append('//gerrit-gwtui:%s' % ui)
@@ -136,4 +136,5 @@
'//gerrit-main:main_bin_deploy.jar',
'//gerrit-war:webapp_assets',
],
+ **kwargs
)
diff --git a/tools/bzl/plugin.bzl b/tools/bzl/plugin.bzl
index bdd1794..32f57e86 100644
--- a/tools/bzl/plugin.bzl
+++ b/tools/bzl/plugin.bzl
@@ -4,7 +4,8 @@
deps = [],
srcs = [],
resources = [],
- manifest_entries = []):
+ manifest_entries = [],
+ **kwargs):
# TODO(davido): Fix stamping: run git describe in plugin directory
# https://github.com/bazelbuild/bazel/issues/1758
manifest_lines = [
@@ -31,4 +32,5 @@
':%s__plugin' % name,
],
visibility = ['//visibility:public'],
+ **kwargs
)
diff --git a/tools/eclipse/BUCK b/tools/eclipse/BUCK
index dfd271d..a8b3f01 100644
--- a/tools/eclipse/BUCK
+++ b/tools/eclipse/BUCK
@@ -15,7 +15,6 @@
'//gerrit-reviewdb:client_tests',
'//gerrit-server:server',
'//gerrit-server:server_tests',
- '//lib:jimfs',
'//lib/asciidoctor:asciidoc_lib',
'//lib/asciidoctor:doc_indexer_lib',
'//lib/auto:auto-value',
