Merge "Report location change via CustomEvents"
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 0196d1f..73e8ba5 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
@@ -151,8 +151,15 @@
public static PushResult pushHead(TestRepository<?> testRepo, String ref,
boolean pushTags, boolean force) throws GitAPIException {
+ return pushHead(testRepo, ref, pushTags, force, null);
+ }
+
+ public static PushResult pushHead(TestRepository<?> testRepo, String ref,
+ boolean pushTags, boolean force, List<String> pushOptions)
+ throws GitAPIException {
PushCommand pushCmd = testRepo.git().push();
pushCmd.setForce(force);
+ pushCmd.setPushOptions(pushOptions);
pushCmd.setRefSpecs(new RefSpec("HEAD:" + ref));
if (pushTags) {
pushCmd.setPushTags();
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
index c892877..e46176e 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.GitUtil.pushHead;
+import static org.junit.Assert.assertEquals;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
@@ -136,6 +137,7 @@
private String changeId;
private Tag tag;
private boolean force;
+ private List<String> pushOptions;
private final TestRepository<?>.CommitBuilder commitBuilder;
@@ -275,8 +277,8 @@
}
tagCommand.call();
}
- return new Result(ref, pushHead(testRepo, ref, tag != null, force), c,
- subject);
+ return new Result(ref,
+ pushHead(testRepo, ref, tag != null, force, pushOptions), c, subject);
}
public void setTag(final Tag tag) {
@@ -287,6 +289,14 @@
this.force = force;
}
+ public List<String> getPushOptions() {
+ return pushOptions;
+ }
+
+ public void setPushOptions(List<String> pushOptions) {
+ this.pushOptions = pushOptions;
+ }
+
public void noParents() {
commitBuilder.noParents();
}
@@ -326,6 +336,10 @@
return commit;
}
+ public void assertPushOptions(List<String> pushOptions) {
+ assertEquals(pushOptions, getPushOptions());
+ }
+
public void assertChange(Change.Status expectedStatus,
String expectedTopic, TestAccount... expectedReviewers)
throws OrmException, NoSuchChangeException {
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
index e935065..00b48b4 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.junit.Assert.fail;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.common.data.ContributorAgreement;
@@ -113,6 +114,11 @@
// Sign the agreement
gApi.accounts().self().signAgreement(caAutoVerify.getName());
+
+ // Explicitly reset the user to force a new request context
+ setApiUser(user);
+
+ // Verify that the agreement was signed
result = gApi.accounts().self().listAgreements();
assertThat(result).hasSize(1);
AgreementInfo info = result.get(0);
@@ -190,7 +196,7 @@
}
@Test
- public void createChangeWithoutCLA() throws Exception {
+ public void createChangeRespectsCLA() throws Exception {
assume().that(isContributorAgreementsEnabled()).isTrue();
// Create a change succeeds when agreement is not required
@@ -199,8 +205,21 @@
// Create a change is not allowed when CLA is required but not signed
setUseContributorAgreements(InheritableBoolean.TRUE);
- exception.expect(AuthException.class);
- exception.expectMessage("A Contributor Agreement must be completed");
+ try {
+ gApi.changes().create(newChangeInput());
+ fail("Expected AuthException");
+ } catch (AuthException e) {
+ assertThat(e.getMessage()).contains(
+ "A Contributor Agreement must be completed");
+ }
+
+ // Sign the agreement
+ gApi.accounts().self().signAgreement(caAutoVerify.getName());
+
+ // Explicitly reset the user to force a new request context
+ setApiUser(user);
+
+ // Create a change succeeds after signing the agreement
gApi.changes().create(newChangeInput());
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 7dee60f..c673e7b 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -176,6 +176,21 @@
}
@Test
+ public void pushForMasterWithTopicOption() throws Exception {
+ String topicOption = "topic=myTopic";
+ List<String> pushOptions = new ArrayList<>();
+ pushOptions.add(topicOption);
+
+ PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
+ push.setPushOptions(pushOptions);
+ PushOneCommit.Result r = push.to("refs/for/master");
+
+ r.assertOkStatus();
+ r.assertChange(Change.Status.NEW, "myTopic");
+ r.assertPushOptions(pushOptions);
+ }
+
+ @Test
public void pushForMasterWithNotify() throws Exception {
TestAccount user2 = accounts.user2();
String pushSpec = "refs/for/master"
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetAgreements.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetAgreements.java
index 2924f97..46d6f11 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetAgreements.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetAgreements.java
@@ -23,6 +23,7 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.AgreementJson;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -44,21 +45,18 @@
private static final Logger log =
LoggerFactory.getLogger(GetAgreements.class);
- private final Provider<IdentifiedUser> self;
+ private final Provider<CurrentUser> self;
private final ProjectCache projectCache;
- private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final AgreementJson agreementJson;
private final boolean agreementsEnabled;
@Inject
- GetAgreements(Provider<IdentifiedUser> self,
+ GetAgreements(Provider<CurrentUser> self,
ProjectCache projectCache,
- IdentifiedUser.GenericFactory identifiedUserFactory,
AgreementJson agreementJson,
@GerritServerConfig Config config) {
this.self = self;
this.projectCache = projectCache;
- this.identifiedUserFactory = identifiedUserFactory;
this.agreementJson = agreementJson;
this.agreementsEnabled =
config.getBoolean("auth", "contributorAgreements", false);
@@ -71,12 +69,14 @@
throw new MethodNotAllowedException("contributor agreements disabled");
}
- if (self.get() != resource.getUser()) {
+ if (!self.get().isIdentifiedUser()) {
throw new AuthException("not allowed to get contributor agreements");
}
- IdentifiedUser user =
- identifiedUserFactory.create(self.get().getAccountId());
+ IdentifiedUser user = self.get().asIdentifiedUser();
+ if (user != resource.getUser()) {
+ throw new AuthException("not allowed to get contributor agreements");
+ }
List<AgreementInfo> results = new ArrayList<>();
Collection<ContributorAgreement> cas =
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 2f73360..00cab38 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
@@ -42,6 +42,7 @@
import com.google.common.collect.HashBiMap;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedListMultimap;
import com.google.common.collect.ListMultimap;
@@ -315,6 +316,8 @@
private final RequestId receiveId;
private MagicBranchInput magicBranch;
private boolean newChangeForAllNotInTarget;
+ private final ListMultimap<String, String> pushOptions =
+ LinkedListMultimap.create();
private List<CreateRequest> newChanges = Collections.emptyList();
private final Map<Change.Id, ReplaceRequest> replaceByChange =
@@ -490,6 +493,7 @@
advHooks.add(new HackPushNegotiateHook());
rp.setAdvertiseRefsHook(AdvertiseRefsHookChain.newChain(advHooks));
rp.setPostReceiveHook(lazyPostReceive.get());
+ rp.setAllowPushOptions(true);
}
public void init() {
@@ -915,6 +919,18 @@
}
private void parseCommands(Collection<ReceiveCommand> commands) {
+ List<String> optionList = rp.getPushOptions();
+ if (optionList != null) {
+ for (String option : optionList) {
+ int e = option.indexOf('=');
+ if (e > 0) {
+ pushOptions.put(option.substring(0, e), option.substring(e + 1));
+ } else {
+ pushOptions.put(option, "");
+ }
+ }
+ }
+
logDebug("Parsing {} commands", commands.size());
for (ReceiveCommand cmd : commands) {
if (cmd.getResult() != NOT_ATTEMPTED) {
@@ -1305,14 +1321,14 @@
return new MailRecipients(reviewer, cc);
}
- String parse(CmdLineParser clp, Repository repo, Set<String> refs)
- throws CmdLineException {
+ String parse(CmdLineParser clp, Repository repo, Set<String> refs,
+ ListMultimap<String, String> pushOptions) throws CmdLineException {
String ref = RefNames.fullName(
MagicBranch.getDestBranchName(cmd.getRefName()));
+ ListMultimap<String, String> options = LinkedListMultimap.create(pushOptions);
int optionStart = ref.indexOf('%');
if (0 < optionStart) {
- ListMultimap<String, String> options = LinkedListMultimap.create();
for (String s : COMMAS.split(ref.substring(optionStart + 1))) {
int e = s.indexOf('=');
if (0 < e) {
@@ -1321,10 +1337,13 @@
options.put(s, "");
}
}
- clp.parseOptionMap(options);
ref = ref.substring(0, optionStart);
}
+ if (!options.isEmpty()) {
+ clp.parseOptionMap(options);
+ }
+
// Split the destination branch by branch and topic. The topic
// suffix is entirely optional, so it might not even exist.
String head = readHEAD(repo);
@@ -1347,6 +1366,19 @@
}
}
+ /**
+ * Gets an unmodifiable view of the pushOptions.
+ * <p>
+ * The collection is empty if the client does not support push options, or if
+ * the client did not send any options.
+ *
+ * @return an unmodifiable view of pushOptions.
+ */
+ @Nullable
+ public ListMultimap<String, String> getPushOptions() {
+ return ImmutableListMultimap.copyOf(pushOptions);
+ }
+
private void parseMagicBranch(ReceiveCommand cmd) {
// Permit exactly one new change request per push.
if (magicBranch != null) {
@@ -1362,8 +1394,10 @@
String ref;
CmdLineParser clp = optionParserFactory.create(magicBranch);
magicBranch.clp = clp;
+
try {
- ref = magicBranch.parse(clp, repo, rp.getAdvertisedRefs().keySet());
+ ref = magicBranch.parse(
+ clp, repo, rp.getAdvertisedRefs().keySet(), pushOptions);
} catch (CmdLineException e) {
if (!clp.wasHelpRequestedByOption()) {
logDebug("Invalid branch syntax");
diff --git a/lib/JGIT_VERSION b/lib/JGIT_VERSION
index 6457323..5964a28 100644
--- a/lib/JGIT_VERSION
+++ b/lib/JGIT_VERSION
@@ -1,4 +1,4 @@
include_defs('//lib/maven.defs')
REPO = GERRIT # Leave here even if set to MAVEN_CENTRAL.
-VERS = '4.4.1.201607150455-r.118-g1096652'
+VERS = '4.4.1.201607150455-r.137-gdd2a5a7'
diff --git a/lib/jgit/org.eclipse.jgit.archive/BUCK b/lib/jgit/org.eclipse.jgit.archive/BUCK
index 384a5e0..29ba7ce 100644
--- a/lib/jgit/org.eclipse.jgit.archive/BUCK
+++ b/lib/jgit/org.eclipse.jgit.archive/BUCK
@@ -4,7 +4,7 @@
maven_jar(
name = 'jgit-archive',
id = 'org.eclipse.jgit:org.eclipse.jgit.archive:' + VERS,
- sha1 = '3f45cd199e40a7c68ee07a1743c06d1c3d07308a',
+ sha1 = '15cbe6b7e2b10ab94ffd7fa2091a3ed1b56f8c3f',
license = 'jgit',
repository = REPO,
deps = ['//lib/jgit/org.eclipse.jgit:jgit'],
diff --git a/lib/jgit/org.eclipse.jgit.http.server/BUCK b/lib/jgit/org.eclipse.jgit.http.server/BUCK
index 2ade9ff..cbbb0e2 100644
--- a/lib/jgit/org.eclipse.jgit.http.server/BUCK
+++ b/lib/jgit/org.eclipse.jgit.http.server/BUCK
@@ -4,7 +4,7 @@
maven_jar(
name = 'jgit-servlet',
id = 'org.eclipse.jgit:org.eclipse.jgit.http.server:' + VERS,
- sha1 = 'fa67bf925001cfc663bf98772f37d5c5c1abd756',
+ sha1 = 'e6930052a609e7c61782bd46754765e7845fc3ee',
license = 'jgit',
repository = REPO,
deps = ['//lib/jgit/org.eclipse.jgit:jgit'],
diff --git a/lib/jgit/org.eclipse.jgit.junit/BUCK b/lib/jgit/org.eclipse.jgit.junit/BUCK
index a31ee6f..ed77543 100644
--- a/lib/jgit/org.eclipse.jgit.junit/BUCK
+++ b/lib/jgit/org.eclipse.jgit.junit/BUCK
@@ -4,7 +4,7 @@
maven_jar(
name = 'junit',
id = 'org.eclipse.jgit:org.eclipse.jgit.junit:' + VERS,
- sha1 = 'dc7edb9c3060655c7fb93ab9b9349e815bab266f',
+ sha1 = 'fc8e7ec3b61f8bde33d554c43beffbe47953b2c2',
license = 'DO_NOT_DISTRIBUTE',
repository = REPO,
unsign = True,
diff --git a/lib/jgit/org.eclipse.jgit/BUCK b/lib/jgit/org.eclipse.jgit/BUCK
index 7c06726..386ea04 100644
--- a/lib/jgit/org.eclipse.jgit/BUCK
+++ b/lib/jgit/org.eclipse.jgit/BUCK
@@ -4,8 +4,8 @@
maven_jar(
name = 'jgit',
id = 'org.eclipse.jgit:org.eclipse.jgit:' + VERS,
- bin_sha1 = 'cd142b9030910babd119702f1c4eeae13ee90018',
- src_sha1 = '3e65e476bfb4a529e18752ffcd27b566e7ee7241',
+ bin_sha1 = '2c4482429f2c5064375cd1634023d0a7d65961a9',
+ src_sha1 = '3f1a513a2d8a17cc2ef7fe7105cd6c040ab06a8e',
license = 'jgit',
repository = REPO,
unsign = True,
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 1b30bde..07badbf 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
@@ -198,8 +198,15 @@
},
_handleTextareaKeydown: function(e) {
- if (e.keyCode == 27) { // 'esc'
- this._handleCancel(e);
+ switch (e.keyCode) {
+ case 27: // 'esc'
+ this._handleCancel(e);
+ break;
+ case 83: // 's'
+ if (e.ctrlKey) {
+ this._handleSave(e);
+ }
+ break;
}
},
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 fcf8b41..bddc3ab 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
@@ -211,6 +211,18 @@
MockInteractions.pressAndReleaseKeyOn(element.$.editTextarea, 27); // esc
});
+ test('ctrl+s saves comment', function(done) {
+ var stub = sinon.stub(element, 'save', function() {
+ assert.isTrue(stub.called);
+ stub.restore();
+ done();
+ });
+ element._messageText = 'is that the horse from horsing around??';
+ MockInteractions.pressAndReleaseKeyOn(
+ element.$.editTextarea.textarea,
+ 83, 'ctrl'); // 'ctrl + s'
+ });
+
test('draft saving/editing', function(done) {
var fireStub = sinon.stub(element, 'fire');
