Merge "Add ctrl+s as keyboard shortcut to save comment draft"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index ed49276..31dc917 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -3452,6 +3452,15 @@
[[site]]
=== Section site
+[[site.allowOriginRegex]]site.allowOriginRegex::
++
+List of regular expressions matching origins that should be permitted
+to use the Gerrit REST API to read content. These should be trusted
+applications as the sites may be able to use the user's credentials.
+Only applies to GET and HEAD requests.
++
+By default, unset, denying all cross-origin requests.
+
[[site.refreshHeaderFooter]]site.refreshHeaderFooter::
+
If true the server checks the site header, footer and CSS files for
diff --git a/ReleaseNotes/ReleaseNotes-2.11.10.txt b/ReleaseNotes/ReleaseNotes-2.11.10.txt
new file mode 100644
index 0000000..a352aac
--- /dev/null
+++ b/ReleaseNotes/ReleaseNotes-2.11.10.txt
@@ -0,0 +1,28 @@
+= Release notes for Gerrit 2.11.10
+
+Gerrit 2.11.10 is now available:
+
+link:https://gerrit-releases.storage.googleapis.com/gerrit-2.11.10.war[
+https://gerrit-releases.storage.googleapis.com/gerrit-2.11.10.war]
+
+There are no schema changes from link:ReleaseNotes-2.11.9.html[2.11.9].
+
+== Bug Fixes
+
+* Fix synchronization of Myers diff and Histogram diff invocations.
++
+The fix for
+link:https://code.google.com/p/gerrit/issues/detail?id=3361[Issue 3361]
+that was included in Gerrit versions 2.10.7 and 2.11.4 introduced a
+regression that prevented more than one file header diff from being
+computed at the same time across the entire server.
+
+* Fix `sshd.idleTimeout` setting being ignored.
++
+The `sshd.idleTimeout` setting was not being correctly set on the SSHD
+backend, causing idle sessions to not time out.
+
+* Add the correct license for AsciiDoctor.
++
+AsciiDoctor is licensed under the MIT License, not Apache2 as previously
+documented.
diff --git a/ReleaseNotes/ReleaseNotes-2.12.4.txt b/ReleaseNotes/ReleaseNotes-2.12.4.txt
index 21ee41d..411011e 100644
--- a/ReleaseNotes/ReleaseNotes-2.12.4.txt
+++ b/ReleaseNotes/ReleaseNotes-2.12.4.txt
@@ -93,7 +93,7 @@
* Fix `sshd.idleTimeout` setting being ignored.
+
-Ths `sshd.idleTimeout` setting was not being correctly set on the SSHD
+The `sshd.idleTimeout` setting was not being correctly set on the SSHD
backend, causing idle sessions to not time out.
* link:https://bugs.chromium.org/p/gerrit/issues/detail?id=4324[Issue 4324]:
diff --git a/ReleaseNotes/index.txt b/ReleaseNotes/index.txt
index 5534713..3ba24c5 100644
--- a/ReleaseNotes/index.txt
+++ b/ReleaseNotes/index.txt
@@ -14,6 +14,7 @@
[[s2_11]]
== Version 2.11.x
+* link:ReleaseNotes-2.11.10.html[2.11.10]
* link:ReleaseNotes-2.11.9.html[2.11.9]
* link:ReleaseNotes-2.11.8.html[2.11.8]
* link:ReleaseNotes-2.11.7.html[2.11.7]
diff --git a/WORKSPACE b/WORKSPACE
index 4911f44..97fdd05 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -24,24 +24,30 @@
sha1 = '83cd2cd674a217ade95a4bb83a8a14f351f48bd0',
)
-GUICE_VERS = '4.0'
+GUICE_VERS = '4.1.0'
maven_jar(
name = 'guice_library',
artifact = 'com.google.inject:guice:' + GUICE_VERS,
- sha1 = '0f990a43d3725781b6db7cd0acf0a8b62dfd1649',
+ sha1 = 'eeb69005da379a10071aa4948c48d89250febb07',
)
maven_jar(
name = 'guice_assistedinject',
artifact = 'com.google.inject.extensions:guice-assistedinject:' + GUICE_VERS,
- sha1 = '8fa6431da1a2187817e3e52e967535899e2e46ca',
+ sha1 = 'af799dd7e23e6fe8c988da12314582072b07edcb',
)
maven_jar(
name = 'guice_servlet',
artifact = 'com.google.inject.extensions:guice-servlet:' + GUICE_VERS,
- sha1 = '4503da866f4c402b5090579b40c1c4aaefabb164',
+ sha1 = '90ac2db772d9b85e2b05417b74f7464bcc061dcb',
+)
+
+maven_jar(
+ name = 'multibindings',
+ artifact = 'com.google.inject.extensions:guice-multibindings:' + GUICE_VERS,
+ sha1 = '3b27257997ac51b0f8d19676f1ea170427e86d51',
)
maven_jar(
@@ -141,8 +147,8 @@
maven_jar(
name = 'gson',
- artifact = 'com.google.code.gson:gson:2.6.2',
- sha1 = 'f1bc476cc167b18e66c297df599b2377131a8947',
+ artifact = 'com.google.code.gson:gson:2.7',
+ sha1 = '751f548c85fa49f330cecbb1875893f971b33c4e',
)
maven_jar(
@@ -165,14 +171,14 @@
maven_jar(
name = 'joda_time',
- artifact = 'joda-time:joda-time:2.8',
- sha1 = '9f2785d7184b97d005a44241ccaf980f43b9ccdb',
+ artifact = 'joda-time:joda-time:2.9.4',
+ sha1 = '1c295b462f16702ebe720bbb08f62e1ba80da41b',
)
maven_jar(
name = 'joda_convert',
- artifact = 'org.joda:joda-convert:1.2',
- sha1 = '35ec554f0cd00c956cc69051514d9488b1374dec',
+ artifact = 'org.joda:joda-convert:1.8.1',
+ sha1 = '675642ac208e0b741bc9118dcbcae44c271b992a',
)
maven_jar(
@@ -287,8 +293,8 @@
maven_jar(
name = 'commons_net',
- artifact = 'commons-net:commons-net:2.2',
- sha1 = '07993c12f63c78378f8c90de4bc2ee62daa7ca3a',
+ artifact = 'commons-net:commons-net:3.5',
+ sha1 = '342fc284019f590e1308056990fdb24a08f06318',
)
maven_jar(
@@ -327,42 +333,42 @@
sha1 = '2e35862b0435c1b027a21f3d6eecbe50e6e08d54',
)
-OW2_VERS = '5.0.3'
+OW2_VERS = '5.1'
maven_jar(
name = 'ow2_asm',
artifact = 'org.ow2.asm:asm:' + OW2_VERS,
- sha1 = 'dcc2193db20e19e1feca8b1240dbbc4e190824fa',
+ sha1 = '5ef31c4fe953b1fd00b8a88fa1d6820e8785bb45',
)
maven_jar(
name = 'ow2_asm_analysis',
artifact = 'org.ow2.asm:asm-analysis:' + OW2_VERS,
- sha1 = 'c7126aded0e8e13fed5f913559a0dd7b770a10f3',
+ sha1 = '6d1bf8989fc7901f868bee3863c44f21aa63d110',
)
maven_jar(
name = 'ow2_asm_commons',
artifact = 'org.ow2.asm:asm-commons:' + OW2_VERS,
- sha1 = 'a7111830132c7f87d08fe48cb0ca07630f8cb91c',
+ sha1 = '25d8a575034dd9cfcb375a39b5334f0ba9c8474e',
)
maven_jar(
name = 'ow2_asm_tree',
artifact = 'org.ow2.asm:asm-tree:' + OW2_VERS,
- sha1 = '287749b48ba7162fb67c93a026d690b29f410bed',
+ sha1 = '87b38c12a0ea645791ead9d3e74ae5268d1d6c34',
)
maven_jar(
name = 'ow2_asm_util',
artifact = 'org.ow2.asm:asm-util:' + OW2_VERS,
- sha1 = '1512e5571325854b05fb1efce1db75fcced54389',
+ sha1 = 'b60e33a6bd0d71831e0c249816d01e6c1dd90a47',
)
maven_jar(
name = 'auto_value',
- artifact = 'com.google.auto.value:auto-value:1.2',
- sha1 = '6873fed014fe1de1051aae2af68ba266d2934471',
+ artifact = 'com.google.auto.value:auto-value:1.3-rc1',
+ sha1 = 'b764e0fb7e11353fbff493b22fd6e83bf091a179',
)
maven_jar(
@@ -371,36 +377,36 @@
sha1 = '18a9a2ce6abf32ea1b5fd31dae5210ad93f4e5e3',
)
-LUCENE_VERS = '5.4.1'
+LUCENE_VERS = '5.5.0'
maven_jar(
name = 'lucene_core',
artifact = 'org.apache.lucene:lucene-core:' + LUCENE_VERS,
- sha1 = 'c52b2088e2c30dfd95fd296ab6fb9cf8de9855ab',
+ sha1 = 'a74fd869bb5ad7fe6b4cd29df9543a34aea81164',
)
maven_jar(
name = 'lucene_analyzers_common',
artifact = 'org.apache.lucene:lucene-analyzers-common:' + LUCENE_VERS,
- sha1 = 'c2aa2c4e00eb9cdeb5ac00dc0495e70c441f681e',
+ sha1 = '1e0e8243a4410be20c34683034fafa7bb52e55cc',
)
maven_jar(
name = 'backward_codecs',
artifact = 'org.apache.lucene:lucene-backward-codecs:' + LUCENE_VERS,
- sha1 = '5273da96380dfab302ad06c27fe58100db4c4e2f',
+ sha1 = '68480974b2f54f519763632a7c1c5d51cbff3805',
)
maven_jar(
name = 'lucene_misc',
artifact = 'org.apache.lucene:lucene-misc:' + LUCENE_VERS,
- sha1 = '95f433b9d7dd470cc0aa5076e0f233907745674b',
+ sha1 = '504d855a1a38190622fdf990b2298c067e7d60ca',
)
maven_jar(
name = 'lucene_queryparser',
artifact = 'org.apache.lucene:lucene-queryparser:' + LUCENE_VERS,
- sha1 = 'dccd5279bfa656dec21af444a7a66820eb1cd618',
+ sha1 = '0fddc49725b562fd48dff0cff004336ad2a090a4',
)
maven_jar(
@@ -447,8 +453,8 @@
maven_jar(
name = 'jsr305',
- artifact = 'com.google.code.findbugs:jsr305:2.0.2',
- sha1 = '516c03b21d50a644d538de0f0369c620989cd8f0',
+ artifact = 'com.google.code.findbugs:jsr305:3.0.1',
+ sha1 = 'f7be08ec23c21485b9b5a1cf1654c2ec8c58168d',
)
maven_jar(
@@ -458,6 +464,19 @@
sha1 = '51d35e6f8bbc2412265066cea9653dd758c95826',
)
+# Keep this version of Soy synchronized with the version used in Gitiles.
+maven_jar(
+ name = 'soy',
+ artifact = 'com.google.template:soy:2016-08-09',
+ sha1 = '43d33651e95480d515fe26c10a662faafe3ad1e4',
+)
+
+maven_jar(
+ name = 'icu4j',
+ artifact = 'com.ibm.icu:icu4j:57.1',
+ sha1 = '198ea005f41219f038f4291f0b0e9f3259730e92',
+)
+
maven_jar(
name = 'dropwizard_core',
artifact = 'io.dropwizard.metrics:metrics-core:3.1.2',
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 79ff53a..5530be3 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
@@ -64,6 +64,7 @@
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.change.Abandon;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.Revisions;
@@ -238,6 +239,9 @@
@Inject
protected ChangeNotes.Factory notesFactory;
+ @Inject
+ protected Abandon changeAbandoner;
+
@Rule
public ExpectedException exception = ExpectedException.none();
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/HttpResponse.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/HttpResponse.java
index 390cae3..e9c6e96 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/HttpResponse.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/HttpResponse.java
@@ -16,6 +16,7 @@
import com.google.common.base.Preconditions;
+import org.apache.http.Header;
import org.eclipse.jgit.util.IO;
import org.eclipse.jgit.util.RawParseUtils;
@@ -52,7 +53,12 @@
}
public String getContentType() {
- return response.getFirstHeader("X-FYI-Content-Type").getValue();
+ return getHeader("X-FYI-Content-Type");
+ }
+
+ public String getHeader(String name) {
+ Header hdr = response.getFirstHeader(name);
+ return hdr != null ? hdr.getValue() : null;
}
public boolean hasContent() {
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/HttpSession.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/HttpSession.java
index 1e0920e..669b991 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/HttpSession.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/HttpSession.java
@@ -37,7 +37,11 @@
account.username, account.httpPassword);
}
- protected RestResponse execute(Request request) throws IOException {
+ public String url() {
+ return url;
+ }
+
+ public RestResponse execute(Request request) throws IOException {
return new RestResponse(executor.execute(request).returnResponse());
}
}
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-framework/src/test/java/com/google/gerrit/acceptance/RestSession.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/RestSession.java
index 9c59e10..90ece46 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/RestSession.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/RestSession.java
@@ -45,7 +45,7 @@
new BasicHeader(HttpHeaders.ACCEPT, "application/json"));
}
- private RestResponse getWithHeader(String endPoint, Header header)
+ public RestResponse getWithHeader(String endPoint, Header header)
throws IOException {
Request get = Request.Get(url + "/a" + endPoint);
if (header != null) {
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 e4c39a1..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
@@ -16,18 +16,32 @@
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;
+import com.google.gerrit.extensions.api.changes.CherryPickInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.changes.SubmitInput;
+import com.google.gerrit.extensions.api.projects.BranchInfo;
+import com.google.gerrit.extensions.api.projects.BranchInput;
+import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.common.AgreementInfo;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.common.ServerInfo;
+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.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.testutil.ConfigSuite;
+import com.google.gerrit.testutil.TestTimeUtil;
import org.eclipse.jgit.lib.Config;
+import org.junit.AfterClass;
import org.junit.Before;
+import org.junit.BeforeClass;
import org.junit.Test;
import java.util.List;
@@ -43,6 +57,16 @@
return cfg;
}
+ @BeforeClass
+ public static void setTimeForTesting() {
+ TestTimeUtil.resetWithClockStep(1, SECONDS);
+ }
+
+ @AfterClass
+ public static void restoreTime() {
+ TestTimeUtil.useSystemTime();
+ }
+
@Before
public void setUp() throws Exception {
caAutoVerify = configureContributorAgreement(true);
@@ -90,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);
@@ -117,6 +146,83 @@
gApi.accounts().self().listAgreements();
}
+ @Test
+ public void revertChangeWithoutCLA() throws Exception {
+ assume().that(isContributorAgreementsEnabled()).isTrue();
+
+ // Create a change succeeds when agreement is not required
+ setUseContributorAgreements(InheritableBoolean.FALSE);
+ ChangeInfo change = gApi.changes().create(newChangeInput()).get();
+
+ // Approve and submit it
+ setApiUser(admin);
+ gApi.changes().id(change.changeId).current().review(ReviewInput.approve());
+ gApi.changes().id(change.changeId).current().submit(new SubmitInput());
+
+ // Revert is not allowed when CLA is required but not signed
+ setApiUser(user);
+ setUseContributorAgreements(InheritableBoolean.TRUE);
+ exception.expect(AuthException.class);
+ exception.expectMessage("A Contributor Agreement must be completed");
+ gApi.changes().id(change.changeId).revert();
+ }
+
+ @Test
+ public void cherrypickChangeWithoutCLA() throws Exception {
+ assume().that(isContributorAgreementsEnabled()).isTrue();
+
+ // Create a new branch
+ setApiUser(admin);
+ BranchInfo dest = gApi.projects().name(project.get())
+ .branch("cherry-pick-to").create(new BranchInput()).get();
+
+ // Create a change succeeds when agreement is not required
+ setUseContributorAgreements(InheritableBoolean.FALSE);
+ ChangeInfo change = gApi.changes().create(newChangeInput()).get();
+
+ // Approve and submit it
+ gApi.changes().id(change.changeId).current().review(ReviewInput.approve());
+ gApi.changes().id(change.changeId).current().submit(new SubmitInput());
+
+ // Cherry-pick is not allowed when CLA is required but not signed
+ setApiUser(user);
+ setUseContributorAgreements(InheritableBoolean.TRUE);
+ CherryPickInput in = new CherryPickInput();
+ in.destination = dest.ref;
+ in.message = change.subject;
+ exception.expect(AuthException.class);
+ exception.expectMessage("A Contributor Agreement must be completed");
+ gApi.changes().id(change.changeId).current().cherryPick(in);
+ }
+
+ @Test
+ public void createChangeRespectsCLA() throws Exception {
+ assume().that(isContributorAgreementsEnabled()).isTrue();
+
+ // Create a change succeeds when agreement is not required
+ setUseContributorAgreements(InheritableBoolean.FALSE);
+ gApi.changes().create(newChangeInput());
+
+ // Create a change is not allowed when CLA is required but not signed
+ setUseContributorAgreements(InheritableBoolean.TRUE);
+ 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());
+ }
+
private void assertAgreement(AgreementInfo info, ContributorAgreement ca) {
assertThat(info.name).isEqualTo(ca.getName());
assertThat(info.description).isEqualTo(ca.getDescription());
@@ -128,4 +234,12 @@
assertThat(info.autoVerifyGroup).isNull();
}
}
+
+ private ChangeInput newChangeInput() {
+ ChangeInput in = new ChangeInput();
+ in.branch = "master";
+ in.subject = "test";
+ in.project = project.get();
+ return in;
+ }
}
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 db9bb09..52cdc5c 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
@@ -31,6 +31,7 @@
import static org.junit.Assert.fail;
import com.google.common.base.Function;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -73,6 +74,7 @@
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.CurrentUser;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
import com.google.gerrit.server.git.ProjectConfig;
@@ -80,7 +82,6 @@
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.Util;
import com.google.gerrit.testutil.FakeEmailSender.Message;
-import com.google.gerrit.testutil.NoteDbMode;
import com.google.gerrit.testutil.TestTimeUtil;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
@@ -186,6 +187,64 @@
}
@Test
+ public void batchAbandon() throws Exception {
+ CurrentUser user = atrScope.get().getUser();
+ PushOneCommit.Result a = createChange();
+ List<ChangeControl> controlA = changeFinder.find(a.getChangeId(), user);
+ assertThat(controlA).hasSize(1);
+ PushOneCommit.Result b = createChange();
+ List<ChangeControl> controlB = changeFinder.find(b.getChangeId(), user);
+ assertThat(controlB).hasSize(1);
+ List<ChangeControl> list =
+ ImmutableList.of(controlA.get(0), controlB.get(0));
+ changeAbandoner.batchAbandon(
+ controlA.get(0).getProject().getNameKey(), user, list, "deadbeef");
+
+ ChangeInfo info = get(a.getChangeId());
+ assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED);
+ assertThat(Iterables.getLast(info.messages).message.toLowerCase())
+ .contains("abandoned");
+ assertThat(Iterables.getLast(info.messages).message.toLowerCase())
+ .contains("deadbeef");
+
+ info = get(b.getChangeId());
+ assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED);
+ assertThat(Iterables.getLast(info.messages).message.toLowerCase())
+ .contains("abandoned");
+ assertThat(Iterables.getLast(info.messages).message.toLowerCase())
+ .contains("deadbeef");
+ }
+
+ @Test
+ public void batchAbandonChangeProject() throws Exception {
+ String project1Name = name("Project1");
+ String project2Name = name("Project2");
+ gApi.projects().create(project1Name);
+ gApi.projects().create(project2Name);
+ TestRepository<InMemoryRepository> project1 =
+ cloneProject(new Project.NameKey(project1Name));
+ TestRepository<InMemoryRepository> project2 =
+ cloneProject(new Project.NameKey(project2Name));
+
+ CurrentUser user = atrScope.get().getUser();
+ PushOneCommit.Result a =
+ createChange(project1, "master", "x", "x", "x", "");
+ List<ChangeControl> controlA = changeFinder.find(a.getChangeId(), user);
+ assertThat(controlA).hasSize(1);
+ PushOneCommit.Result b =
+ createChange(project2, "master", "x", "x", "x", "");
+ List<ChangeControl> controlB = changeFinder.find(b.getChangeId(), user);
+ assertThat(controlB).hasSize(1);
+ List<ChangeControl> list =
+ ImmutableList.of(controlA.get(0), controlB.get(0));
+ exception.expect(ResourceConflictException.class);
+ exception.expectMessage(String.format(
+ "Project name \"%s\" doesn't match \"%s\"",
+ project2Name, project1Name));
+ changeAbandoner.batchAbandon(new Project.NameKey(project1Name), user, list);
+ }
+
+ @Test
public void abandonDraft() throws Exception {
PushOneCommit.Result r = createDraftChange();
String changeId = r.getChangeId();
@@ -1041,18 +1100,8 @@
.reviewer(user.getId().toString())
.votes();
- if (NoteDbMode.readWrite()) {
- // When NoteDb is enabled each reviewer is explicitly recorded in the
- // NoteDb and this record stays even when all votes of that user have been
- // deleted, hence there is no dummy 0 approval left when a vote is
- // deleted.
- assertThat(m).isEmpty();
- } else {
- // When NoteDb is disabled there is a dummy 0 approval on the change so
- // that the user is still returned as CC when all votes of that user have
- // been deleted.
- assertThat(m).containsEntry("Code-Review", Short.valueOf((short)0));
- }
+ // Dummy 0 approval on the change to block vote copying to this patch set.
+ assertThat(m).containsExactly("Code-Review", Short.valueOf((short)0));
ChangeInfo c = gApi.changes()
.id(r.getChangeId())
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
index 54fe28f..4da22d3 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
@@ -311,6 +311,27 @@
assertVotes(c, user, 0, 0, REWORK);
}
+ @Test
+ public void deleteStickyVote() throws Exception {
+ String label = "Code-Review";
+ ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+ cfg.getLabelSections().get(label)
+ .setCopyMaxScore(true);
+ saveProjectConfig(project, cfg);
+
+
+ // Vote max score on PS1
+ String changeId = createChange(REWORK);
+ vote(admin, changeId, label, 2);
+ assertVotes(detailedChange(changeId), admin, label, 2, null);
+ updateChange(changeId, REWORK);
+ assertVotes(detailedChange(changeId), admin, label, 2, REWORK);
+
+ // Delete vote that was copied via sticky approval
+ deleteVote(admin, changeId, "Code-Review");
+ assertVotes(detailedChange(changeId), admin, label, 0, REWORK);
+ }
+
private ChangeInfo detailedChange(String changeId) throws Exception {
return gApi.changes().id(changeId)
.get(EnumSet.of(ListChangesOption.DETAILED_LABELS,
@@ -495,6 +516,15 @@
return c.revisions.get(c.currentRevision).kind;
}
+ private void vote(TestAccount user, String changeId, String label, int vote)
+ throws Exception {
+ setApiUser(user);
+ gApi.changes()
+ .id(changeId)
+ .current()
+ .review(new ReviewInput().label(label, vote));
+ }
+
private void vote(TestAccount user, String changeId, int codeReviewVote,
int verifiedVote) throws Exception {
setApiUser(user);
@@ -504,6 +534,15 @@
gApi.changes().id(changeId).current().review(in);
}
+ private void deleteVote(TestAccount user, String changeId, String label)
+ throws Exception {
+ setApiUser(user);
+ gApi.changes()
+ .id(changeId)
+ .reviewer(user.getId().toString())
+ .deleteVote(label);
+ }
+
private void assertVotes(ChangeInfo c, TestAccount user, int codeReviewVote,
int verifiedVote) {
assertVotes(c, user, codeReviewVote, verifiedVote, null);
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 3629e29..e8b2924 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
@@ -52,7 +52,6 @@
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.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.server.change.GetRevisionActions;
@@ -567,16 +566,19 @@
}
@Test
- public void diffNonExistingFile() throws Exception {
- PushOneCommit.Result r = createChange();
-
- exception.expect(ResourceNotFoundException.class);
- exception.expectMessage("non-existing");
- gApi.changes()
+ public void diffDeletedFile() throws Exception {
+ pushFactory.create(db, admin.getIdent(), testRepo)
+ .to("refs/heads/master");
+ PushOneCommit.Result r =
+ pushFactory.create(db, admin.getIdent(), testRepo)
+ .rm("refs/for/master");
+ DiffInfo diff = gApi.changes()
.id(r.getChangeId())
.revision(r.getCommit().name())
- .file("non-existing")
+ .file(FILE_NAME)
.diff();
+ assertThat(diff.metaA.lines).isEqualTo(1);
+ assertThat(diff.metaB).isNull();
}
@Test
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-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CorsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CorsIT.java
new file mode 100644
index 0000000..f5ae072
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CorsIT.java
@@ -0,0 +1,160 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.rest.change;
+
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD;
+import static com.google.common.net.HttpHeaders.ORIGIN;
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit.Result;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.testutil.ConfigSuite;
+
+import org.apache.http.Header;
+import org.apache.http.client.fluent.Request;
+import org.apache.http.message.BasicHeader;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Test;
+
+public class CorsIT extends AbstractDaemonTest {
+ @ConfigSuite.Default
+ public static Config allowExampleDotCom() {
+ Config cfg = new Config();
+ cfg.setStringList(
+ "site", null, "allowOriginRegex",
+ ImmutableList.of(
+ "https?://(.+[.])?example[.]com",
+ "http://friend[.]ly"));
+ return cfg;
+ }
+
+ @Test
+ public void origin() throws Exception {
+ Result change = createChange();
+
+ String url = "/changes/" + change.getChangeId() + "/detail";
+ RestResponse r = adminRestSession.get(url);
+ r.assertOK();
+ assertThat(r.getHeader(ACCESS_CONTROL_ALLOW_ORIGIN)).isNull();
+ assertThat(r.getHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS)).isNull();
+
+ check(url, true, "http://example.com");
+ check(url, true, "https://sub.example.com");
+ check(url, true, "http://friend.ly");
+
+ check(url, false, "http://evil.attacker");
+ check(url, false, "http://friendsly");
+ }
+
+ @Test
+ public void putWithOriginRefused() throws Exception {
+ Result change = createChange();
+ String origin = "http://example.com";
+ RestResponse r = adminRestSession.putWithHeader(
+ "/changes/" + change.getChangeId() + "/topic",
+ new BasicHeader(ORIGIN, origin),
+ "A");
+ r.assertOK();
+ checkCors(r, false, origin);
+ }
+
+ @Test
+ public void preflightOk() throws Exception {
+ Result change = createChange();
+
+ String origin = "http://example.com";
+ Request req = Request.Options(adminRestSession.url()
+ + "/a/changes/" + change.getChangeId() + "/detail");
+ req.addHeader(ORIGIN, origin);
+ req.addHeader(ACCESS_CONTROL_REQUEST_METHOD, "GET");
+ req.addHeader(ACCESS_CONTROL_REQUEST_HEADERS, "X-Requested-With");
+
+ RestResponse res = adminRestSession.execute(req);
+ res.assertOK();
+ checkCors(res, true, origin);
+ }
+
+ @Test
+ public void preflightBadOrigin() throws Exception {
+ Result change = createChange();
+
+ Request req = Request.Options(adminRestSession.url()
+ + "/a/changes/" + change.getChangeId() + "/detail");
+ req.addHeader(ORIGIN, "http://evil.attacker");
+ req.addHeader(ACCESS_CONTROL_REQUEST_METHOD, "GET");
+
+ adminRestSession.execute(req).assertBadRequest();
+ }
+
+ @Test
+ public void preflightBadMethod() throws Exception {
+ Result change = createChange();
+
+ for (String method : new String[] {"POST", "PUT", "DELETE", "PATCH"}) {
+ Request req = Request.Options(adminRestSession.url()
+ + "/a/changes/" + change.getChangeId() + "/detail");
+ req.addHeader(ORIGIN, "http://example.com");
+ req.addHeader(ACCESS_CONTROL_REQUEST_METHOD, method);
+ adminRestSession.execute(req).assertBadRequest();
+ }
+ }
+
+ @Test
+ public void preflightBadHeader() throws Exception {
+ Result change = createChange();
+
+ Request req = Request.Options(adminRestSession.url()
+ + "/a/changes/" + change.getChangeId() + "/detail");
+ req.addHeader(ORIGIN, "http://example.com");
+ req.addHeader(ACCESS_CONTROL_REQUEST_METHOD, "GET");
+ req.addHeader(ACCESS_CONTROL_REQUEST_HEADERS, "X-Gerrit-Auth");
+
+ adminRestSession.execute(req).assertBadRequest();
+ }
+
+ private RestResponse check(String url, boolean accept, String origin)
+ throws Exception {
+ Header hdr = new BasicHeader(ORIGIN, origin);
+ RestResponse r = adminRestSession.getWithHeader(url, hdr);
+ r.assertOK();
+ checkCors(r, accept, origin);
+ return r;
+ }
+
+ private void checkCors(RestResponse r, boolean accept, String origin) {
+ String allowOrigin = r.getHeader(ACCESS_CONTROL_ALLOW_ORIGIN);
+ String allowCred = r.getHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS);
+ String allowMethods = r.getHeader(ACCESS_CONTROL_ALLOW_METHODS);
+ String allowHeaders = r.getHeader(ACCESS_CONTROL_ALLOW_HEADERS);
+ if (accept) {
+ assertThat(allowOrigin).isEqualTo(origin);
+ assertThat(allowCred).isEqualTo("true");
+ assertThat(allowMethods).isEqualTo("GET, OPTIONS");
+ assertThat(allowHeaders).isEqualTo("X-Requested-With");
+ } else {
+ assertThat(allowOrigin).isNull();
+ assertThat(allowCred).isNull();
+ assertThat(allowMethods).isNull();
+ assertThat(allowHeaders).isNull();
+ }
+ }
+}
diff --git a/gerrit-extension-api/BUCK b/gerrit-extension-api/BUCK
index 121d236..dda1290 100644
--- a/gerrit-extension-api/BUCK
+++ b/gerrit-extension-api/BUCK
@@ -74,6 +74,7 @@
paths = ['src/main/java'],
srcs = SRCS,
deps = [
+ '//lib:guava',
'//lib/guice:javax-inject',
'//lib/guice:guice_library',
'//lib/guice:guice-assistedinject',
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
index a71ab37..2731476 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
@@ -47,7 +47,7 @@
Map<String, FileInfo> files() throws RestApiException;
Map<String, FileInfo> files(String base) throws RestApiException;
Map<String, FileInfo> files(int parentNum) throws RestApiException;
- FileApi file(String path) throws RestApiException;
+ FileApi file(String path);
MergeableInfo mergeable() throws RestApiException;
MergeableInfo mergeableOtherBranches() throws RestApiException;
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/AuthType.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/AuthType.java
index 004ef1c..2056e25 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/AuthType.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/AuthType.java
@@ -15,10 +15,10 @@
package com.google.gerrit.extensions.client;
public enum AuthType {
- /** Login relies upon the OpenID standard: {@link "http://openid.net/"} */
+ /** Login relies upon the <a href="http://openid.net/">OpenID standard</a> */
OPENID,
- /** Login relies upon the OpenID standard: {@link "http://openid.net/"} in Single Sign On mode */
+ /** Login relies upon the <a href="http://openid.net/">OpenID standard</a> in Single Sign On mode */
OPENID_SSO,
/**
@@ -49,7 +49,7 @@
* Jetty's SSL channel to request client's SSL certificate. For this
* authentication to work a Gerrit administrator has to import the root
* certificate of the trust chain used to issue the client's certificate
- * into the <review-site>/etc/keystore.
+ * into the <review-site>/etc/keystore.
* <p>
* After the authentication is done Gerrit will obtain basic user
* registration (name and email) from LDAP, and some group memberships.
diff --git a/gerrit-gwtui/src/main/java/net/codemirror/mode/ModeInfo.java b/gerrit-gwtui/src/main/java/net/codemirror/mode/ModeInfo.java
index 943be7e..3d99883 100644
--- a/gerrit-gwtui/src/main/java/net/codemirror/mode/ModeInfo.java
+++ b/gerrit-gwtui/src/main/java/net/codemirror/mode/ModeInfo.java
@@ -82,7 +82,6 @@
Modes.I.htmlmixed(),
Modes.I.http(),
Modes.I.idl(),
- Modes.I.jade(),
Modes.I.javascript(),
Modes.I.jinja2(),
Modes.I.jsx(),
@@ -110,6 +109,7 @@
Modes.I.powershell(),
Modes.I.properties(),
Modes.I.protobuf(),
+ Modes.I.pug(),
Modes.I.puppet(),
Modes.I.python(),
Modes.I.q(),
diff --git a/gerrit-gwtui/src/main/java/net/codemirror/mode/Modes.java b/gerrit-gwtui/src/main/java/net/codemirror/mode/Modes.java
index 668a57f..218b96c 100644
--- a/gerrit-gwtui/src/main/java/net/codemirror/mode/Modes.java
+++ b/gerrit-gwtui/src/main/java/net/codemirror/mode/Modes.java
@@ -67,7 +67,6 @@
@Source("htmlmixed.js") @DoNotEmbed DataResource htmlmixed();
@Source("http.js") @DoNotEmbed DataResource http();
@Source("idl.js") @DoNotEmbed DataResource idl();
- @Source("jade.js") @DoNotEmbed DataResource jade();
@Source("javascript.js") @DoNotEmbed DataResource javascript();
@Source("jinja2.js") @DoNotEmbed DataResource jinja2();
@Source("jsx.js") @DoNotEmbed DataResource jsx();
@@ -95,6 +94,7 @@
@Source("powershell.js") @DoNotEmbed DataResource powershell();
@Source("properties.js") @DoNotEmbed DataResource properties();
@Source("protobuf.js") @DoNotEmbed DataResource protobuf();
+ @Source("pug.js") @DoNotEmbed DataResource pug();
@Source("puppet.js") @DoNotEmbed DataResource puppet();
@Source("python.js") @DoNotEmbed DataResource python();
@Source("q.js") @DoNotEmbed DataResource q();
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 943d824..e3f3fb1 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -15,6 +15,14 @@
package com.google.gerrit.httpd.restapi;
import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS;
+import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD;
+import static com.google.common.net.HttpHeaders.ORIGIN;
+import static com.google.common.net.HttpHeaders.VARY;
import static java.math.RoundingMode.CEILING;
import static java.nio.charset.StandardCharsets.ISO_8859_1;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -35,9 +43,12 @@
import com.google.common.base.CharMatcher;
import com.google.common.base.Function;
import com.google.common.base.Joiner;
+import com.google.common.base.Predicates;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMultimap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Lists;
@@ -85,6 +96,7 @@
import com.google.gerrit.server.OptionUtil;
import com.google.gerrit.server.OutputFormat;
import com.google.gerrit.server.account.CapabilityUtils;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.util.http.RequestUtil;
import com.google.gson.ExclusionStrategy;
import com.google.gson.FieldAttributes;
@@ -103,6 +115,7 @@
import com.google.inject.Provider;
import com.google.inject.util.Providers;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.util.TemporaryBuffer;
import org.eclipse.jgit.util.TemporaryBuffer.Heap;
import org.slf4j.Logger;
@@ -131,6 +144,7 @@
import java.util.Set;
import java.util.TreeMap;
import java.util.concurrent.TimeUnit;
+import java.util.regex.Pattern;
import java.util.zip.GZIPOutputStream;
import javax.servlet.ServletException;
@@ -150,6 +164,9 @@
// HTTP 422 Unprocessable Entity.
// TODO: Remove when HttpServletResponse.SC_UNPROCESSABLE_ENTITY is available
private static final int SC_UNPROCESSABLE_ENTITY = 422;
+ private static final String X_REQUESTED_WITH = "X-Requested-With";
+ private static final ImmutableSet<String> ALLOWED_CORS_REQUEST_HEADERS =
+ ImmutableSet.of(X_REQUESTED_WITH);
private static final int HEAP_EST_SIZE = 10 * 8 * 1024; // Presize 10 blocks.
@@ -174,18 +191,29 @@
final Provider<ParameterParser> paramParser;
final AuditService auditService;
final RestApiMetrics metrics;
+ final Pattern allowOrigin;
@Inject
Globals(Provider<CurrentUser> currentUser,
DynamicItem<WebSession> webSession,
Provider<ParameterParser> paramParser,
AuditService auditService,
- RestApiMetrics metrics) {
+ RestApiMetrics metrics,
+ @GerritServerConfig Config cfg) {
this.currentUser = currentUser;
this.webSession = webSession;
this.paramParser = paramParser;
this.auditService = auditService;
this.metrics = metrics;
+ allowOrigin = makeAllowOrigin(cfg);
+ }
+
+ private static Pattern makeAllowOrigin(Config cfg) {
+ String[] allow = cfg.getStringList("site", null, "allowOriginRegex");
+ if (allow.length > 0) {
+ return Pattern.compile(Joiner.on('|').join(allow));
+ }
+ return null;
}
}
@@ -222,6 +250,11 @@
ViewData viewData = null;
try {
+ if (isCorsPreflight(req)) {
+ doCorsPreflight(req, res);
+ return;
+ }
+ checkCors(req, res);
checkUserSession(req);
List<IdString> path = splitPath(req);
@@ -232,7 +265,7 @@
viewData = new ViewData(null, null);
if (path.isEmpty()) {
- if (isGetOrHead(req)) {
+ if (isRead(req)) {
viewData = new ViewData(null, rc.list());
} else if (rc instanceof AcceptsPost && "POST".equals(req.getMethod())) {
@SuppressWarnings("unchecked")
@@ -273,7 +306,7 @@
(RestCollection<RestResource, RestResource>) viewData.view;
if (path.isEmpty()) {
- if (isGetOrHead(req)) {
+ if (isRead(req)) {
viewData = new ViewData(null, c.list());
} else if (c instanceof AcceptsPost && "POST".equals(req.getMethod())) {
@SuppressWarnings("unchecked")
@@ -330,7 +363,7 @@
return;
}
- if (viewData.view instanceof RestReadView<?> && isGetOrHead(req)) {
+ if (viewData.view instanceof RestReadView<?> && isRead(req)) {
result = ((RestReadView<RestResource>) viewData.view).apply(rsrc);
} else if (viewData.view instanceof RestModifyView<?, ?>) {
@SuppressWarnings("unchecked")
@@ -428,6 +461,72 @@
}
}
+ private void checkCors(HttpServletRequest req, HttpServletResponse res) {
+ String origin = req.getHeader(ORIGIN);
+ if (isRead(req)
+ && !Strings.isNullOrEmpty(origin)
+ && isOriginAllowed(origin)) {
+ res.addHeader(VARY, ORIGIN);
+ setCorsHeaders(res, origin);
+ }
+ }
+
+ private static boolean isCorsPreflight(HttpServletRequest req) {
+ return "OPTIONS".equals(req.getMethod())
+ && !Strings.isNullOrEmpty(req.getHeader(ORIGIN))
+ && !Strings.isNullOrEmpty(req.getHeader(ACCESS_CONTROL_REQUEST_METHOD));
+ }
+
+ private void doCorsPreflight(HttpServletRequest req,
+ HttpServletResponse res) throws BadRequestException {
+ CacheHeaders.setNotCacheable(res);
+ res.setHeader(VARY, Joiner.on(", ").join(ImmutableList.of(
+ ORIGIN,
+ ACCESS_CONTROL_REQUEST_METHOD)));
+
+ String origin = req.getHeader(ORIGIN);
+ if (Strings.isNullOrEmpty(origin) || !isOriginAllowed(origin)) {
+ throw new BadRequestException("CORS not allowed");
+ }
+
+ String method = req.getHeader(ACCESS_CONTROL_REQUEST_METHOD);
+ if (!"GET".equals(method) && !"HEAD".equals(method)) {
+ throw new BadRequestException(method + " not allowed in CORS");
+ }
+
+ String headers = req.getHeader(ACCESS_CONTROL_REQUEST_HEADERS);
+ if (headers != null) {
+ res.addHeader(VARY, ACCESS_CONTROL_REQUEST_HEADERS);
+ String badHeader = Iterables.getFirst(
+ Iterables.filter(
+ Splitter.on(',').trimResults().split(headers),
+ Predicates.not(Predicates.in(ALLOWED_CORS_REQUEST_HEADERS))),
+ null);
+ if (badHeader != null) {
+ throw new BadRequestException(badHeader + " not allowed in CORS");
+ }
+ }
+
+ res.setStatus(SC_OK);
+ setCorsHeaders(res, origin);
+ res.setContentType("text/plain");
+ res.setContentLength(0);
+ }
+
+ private void setCorsHeaders(HttpServletResponse res, String origin) {
+ res.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN, origin);
+ res.setHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS, "true");
+ res.setHeader(ACCESS_CONTROL_ALLOW_METHODS, "GET, OPTIONS");
+ res.setHeader(
+ ACCESS_CONTROL_ALLOW_HEADERS,
+ Joiner.on(", ").join(ALLOWED_CORS_REQUEST_HEADERS));
+ }
+
+ private boolean isOriginAllowed(String origin) {
+ return globals.allowOrigin != null
+ && globals.allowOrigin.matcher(origin).matches();
+ }
+
private static String messageOr(Throwable t, String defaultMessage) {
if (!Strings.isNullOrEmpty(t.getMessage())) {
return t.getMessage();
@@ -438,7 +537,7 @@
@SuppressWarnings({"unchecked", "rawtypes"})
private static boolean notModified(HttpServletRequest req, RestResource rsrc,
RestView<RestResource> view) {
- if (!isGetOrHead(req)) {
+ if (!isRead(req)) {
return false;
}
@@ -469,7 +568,7 @@
private static <R extends RestResource> void configureCaching(
HttpServletRequest req, HttpServletResponse res, R rsrc,
RestView<R> view, CacheControl c) {
- if (isGetOrHead(req)) {
+ if (isRead(req)) {
switch (c.getType()) {
case NONE:
default:
@@ -972,25 +1071,20 @@
private void checkUserSession(HttpServletRequest req)
throws AuthException {
CurrentUser user = globals.currentUser.get();
- if (isStateChange(req)) {
- if (user instanceof AnonymousUser) {
- throw new AuthException("Authentication required");
- } else if (!globals.webSession.get().isAccessPathOk(AccessPath.REST_API)) {
- throw new AuthException("Invalid authentication method. In order to authenticate, "
- + "prefix the REST endpoint URL with /a/ (e.g. http://example.com/a/projects/).");
- }
+ if (isRead(req)) {
+ user.setAccessPath(AccessPath.REST_API);
+ } else if (user instanceof AnonymousUser) {
+ throw new AuthException("Authentication required");
+ } else if (!globals.webSession.get().isAccessPathOk(AccessPath.REST_API)) {
+ throw new AuthException("Invalid authentication method. In order to authenticate, "
+ + "prefix the REST endpoint URL with /a/ (e.g. http://example.com/a/projects/).");
}
- user.setAccessPath(AccessPath.REST_API);
}
- private static boolean isGetOrHead(HttpServletRequest req) {
+ private static boolean isRead(HttpServletRequest req) {
return "GET".equals(req.getMethod()) || "HEAD".equals(req.getMethod());
}
- private static boolean isStateChange(HttpServletRequest req) {
- return !isGetOrHead(req);
- }
-
private void checkRequiresCapability(ViewData viewData) throws AuthException {
CapabilityUtils.checkRequiresCapability(globals.currentUser,
viewData.pluginName, viewData.view.getClass());
@@ -1029,7 +1123,7 @@
static long replyText(@Nullable HttpServletRequest req,
HttpServletResponse res, String text) throws IOException {
- if ((req == null || isGetOrHead(req)) && isMaybeHTML(text)) {
+ if ((req == null || isRead(req)) && isMaybeHTML(text)) {
return replyJson(req, res, ImmutableMultimap.of("pp", "0"), new JsonPrimitive(text));
}
if (!text.endsWith("\n")) {
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index c137d1e..77b364c 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -377,7 +377,7 @@
close();
throw new OrmRuntimeException(e);
} catch (ExecutionException e) {
- Throwables.throwIfUnchecked(e.getCause());
+ Throwables.propagateIfPossible(e.getCause());
throw new OrmRuntimeException(e.getCause());
}
}
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/HiddenErrorHandler.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/HiddenErrorHandler.java
index 1b663ae..0c2ec78 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/HiddenErrorHandler.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/HiddenErrorHandler.java
@@ -64,10 +64,15 @@
}
private static byte[] message(HttpConnection conn) {
- String msg = conn.getHttpChannel().getResponse().getReason();
- if (msg == null) {
- msg = HttpStatus.getMessage(conn.getHttpChannel()
- .getResponse().getStatus());
+ String msg;
+ if (conn == null) {
+ msg = "";
+ } else {
+ msg = conn.getHttpChannel().getResponse().getReason();
+ if (msg == null) {
+ msg = HttpStatus.getMessage(conn.getHttpChannel()
+ .getResponse().getStatus());
+ }
}
return msg.getBytes(ISO_8859_1);
}
diff --git a/gerrit-plugin-api/BUILD b/gerrit-plugin-api/BUILD
index 2c18ca6..c761703 100644
--- a/gerrit-plugin-api/BUILD
+++ b/gerrit-plugin-api/BUILD
@@ -28,24 +28,33 @@
'//gerrit-extension-api:api',
'//gerrit-gwtexpui:server',
'//gerrit-reviewdb:server',
- '//lib:args4j',
- '//lib:blame-cache',
- '//lib/dropwizard:dropwizard-core',
- '//lib:guava',
- '//lib:gwtorm',
- '//lib:jsch',
- '//lib:mime-util',
- '//lib:servlet-api-3_1',
- '//lib:velocity',
'//lib/commons:lang',
+ '//lib/dropwizard:dropwizard-core',
'//lib/guice:guice',
'//lib/guice:guice-assistedinject',
'//lib/guice:guice-servlet',
- '//lib/jgit/org.eclipse.jgit:jgit',
+ '//lib/guice:javax-inject',
+ '//lib/guice:multibindings',
'//lib/jgit/org.eclipse.jgit.http.server:jgit-servlet',
+ '//lib/jgit/org.eclipse.jgit:jgit',
'//lib/joda:joda-time',
'//lib/log:api',
'//lib/mina:sshd',
+ '//lib/ow2:ow2-asm',
+ '//lib/ow2:ow2-asm-analysis',
+ '//lib/ow2:ow2-asm-commons',
+ '//lib/ow2:ow2-asm-util',
+ '//lib:args4j',
+ '//lib:blame-cache',
+ '//lib:guava',
+ '//lib:gwtorm',
+ '//lib:icu4j',
+ '//lib:jsch',
+ '//lib:mime-util',
+ '//lib:protobuf',
+ '//lib:servlet-api-3_1',
+ '//lib:soy',
+ '//lib:velocity',
],
visibility = ['//visibility:public'],
)
diff --git a/gerrit-server/BUCK b/gerrit-server/BUCK
index 443d7c4..080b52b 100644
--- a/gerrit-server/BUCK
+++ b/gerrit-server/BUCK
@@ -204,7 +204,6 @@
'//lib:guava',
'//lib:guava-retrying',
'//lib:protobuf',
- '//lib/commons:validator',
'//lib/dropwizard:dropwizard-core',
'//lib/guice:guice-assistedinject',
'//lib/prolog:runtime',
diff --git a/gerrit-server/BUILD b/gerrit-server/BUILD
index 5a6b50f..a591fba 100644
--- a/gerrit-server/BUILD
+++ b/gerrit-server/BUILD
@@ -48,6 +48,7 @@
'//lib:pegdown',
'//lib:protobuf',
'//lib:servlet-api-3_1',
+ '//lib:soy',
'//lib:tukaani-xz',
'//lib:velocity',
'//lib/antlr:java_runtime',
@@ -188,6 +189,7 @@
['src/test/java/**/*.java'],
exclude = TESTUTIL + PROLOG_TESTS + PROLOG_TEST_CASE + QUERY_TESTS
),
+ resources = glob(['src/test/resources/com/google/gerrit/server/mail/*']),
deps = TESTUTIL_DEPS + [
':testutil',
'//gerrit-antlr:query_exception',
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
index 5a5d16c..5c0723a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
@@ -221,7 +221,7 @@
need.add(authorId);
}
- if (committerId != null && canSee(db, update.getNotes(), authorId)) {
+ if (committerId != null && canSee(db, update.getNotes(), committerId)) {
need.add(committerId);
}
need.remove(change.getOwner());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountLoader.java
index 89e9419..f84d399 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountLoader.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountLoader.java
@@ -94,7 +94,7 @@
directory.fillAccountInfo(
Iterables.concat(created.values(), provided), options);
} catch (DirectoryException e) {
- Throwables.throwIfInstanceOf(e.getCause(), OrmException.class);
+ Throwables.propagateIfPossible(e.getCause(), OrmException.class);
throw new OrmException(e);
}
}
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/account/GetDetail.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetDetail.java
index e47ceb3..81c860e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetDetail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetDetail.java
@@ -47,7 +47,7 @@
directory.fillAccountInfo(Collections.singleton(info),
EnumSet.allOf(FillOptions.class));
} catch (DirectoryException e) {
- Throwables.throwIfInstanceOf(e.getCause(), OrmException.class);
+ Throwables.propagateIfPossible(e.getCause(), OrmException.class);
throw new OrmException(e);
}
return info;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
index 213f90d..6b5e83c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
@@ -321,13 +321,9 @@
}
@Override
- public FileApi file(String path) throws RestApiException {
- try {
- return fileApi.create(files.parse(revision,
- IdString.fromDecoded(path)));
- } catch (IOException e) {
- throw new RestApiException("Cannot retrieve file", e);
- }
+ public FileApi file(String path) {
+ return fileApi.create(files.parse(revision,
+ IdString.fromDecoded(path)));
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java
index 354dc62..3567811 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java
@@ -154,8 +154,8 @@
}
});
} catch (PrivilegedActionException e) {
- Throwables.throwIfInstanceOf(e.getException(), NamingException.class);
- Throwables.throwIfInstanceOf(e.getException(), RuntimeException.class);
+ Throwables.propagateIfPossible(e.getException(), NamingException.class);
+ Throwables.propagateIfPossible(e.getException(), RuntimeException.class);
LdapRealm.log.warn("Internal error", e.getException());
return null;
} finally {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
index adbcf22..c4bd68d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
@@ -28,6 +28,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
@@ -50,6 +51,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.util.Collection;
+
@Singleton
public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
UiAction<ChangeResource> {
@@ -91,6 +94,11 @@
return json.create(ChangeJson.NO_OPTIONS).format(change);
}
+ public Change abandon(ChangeControl control)
+ throws RestApiException, UpdateException {
+ return abandon(control, "", NotifyHandling.ALL);
+ }
+
public Change abandon(ChangeControl control, String msgTxt)
throws RestApiException, UpdateException {
return abandon(control, msgTxt, NotifyHandling.ALL);
@@ -98,31 +106,76 @@
public Change abandon(ChangeControl control, String msgTxt,
NotifyHandling notifyHandling) throws RestApiException, UpdateException {
- CurrentUser user = control.getUser();
- Account account = user.isIdentifiedUser()
- ? user.asIdentifiedUser().getAccount()
- : null;
- Op op = new Op(msgTxt, account, notifyHandling);
- try (BatchUpdate u = batchUpdateFactory.create(dbProvider.get(),
- control.getProject().getNameKey(), user, TimeUtil.nowTs())) {
+ Op op = new Op(control.getUser(), msgTxt, notifyHandling);
+ try (BatchUpdate u =
+ batchUpdateFactory.create(
+ dbProvider.get(),
+ control.getProject().getNameKey(),
+ control.getUser(),
+ TimeUtil.nowTs())) {
u.addOp(control.getId(), op).execute();
}
return op.change;
}
+ /**
+ * If an extension has more than one changes to abandon that belong to the
+ * same project, they should use the batch instead of abandoning one by one.
+ * <p>
+ * It's the caller's responsibility to ensure that all jobs inside the same
+ * batch have the matching project from its ChangeControl. Violations will
+ * result in a ResourceConflictException.
+ */
+ public void batchAbandon(Project.NameKey project, CurrentUser user,
+ Collection<ChangeControl> controls, String msgTxt,
+ NotifyHandling notifyHandling) throws RestApiException, UpdateException {
+ if (controls.isEmpty()) {
+ return;
+ }
+ try (BatchUpdate u = batchUpdateFactory.create(
+ dbProvider.get(), project, user, TimeUtil.nowTs())) {
+ for (ChangeControl control : controls) {
+ if (!project.equals(control.getProject().getNameKey())) {
+ throw new ResourceConflictException(
+ String.format(
+ "Project name \"%s\" doesn't match \"%s\"",
+ control.getProject().getNameKey().get(),
+ project.get()));
+ }
+ u.addOp(
+ control.getId(), new Op(control.getUser(), msgTxt, notifyHandling));
+ }
+ u.execute();
+ }
+ }
+
+ public void batchAbandon(Project.NameKey project, CurrentUser user,
+ Collection<ChangeControl> controls, String msgTxt)
+ throws RestApiException, UpdateException {
+ batchAbandon(project, user, controls, msgTxt, NotifyHandling.ALL);
+ }
+
+ public void batchAbandon(Project.NameKey project, CurrentUser user,
+ Collection<ChangeControl> controls)
+ throws RestApiException, UpdateException {
+ batchAbandon(project, user, controls, "", NotifyHandling.ALL);
+ }
+
private class Op extends BatchUpdate.Op {
- private final Account account;
private final String msgTxt;
+ private final NotifyHandling notifyHandling;
+ private final Account account;
private Change change;
private PatchSet patchSet;
private ChangeMessage message;
- private NotifyHandling notifyHandling;
- private Op(String msgTxt, Account account, NotifyHandling notifyHandling) {
- this.account = account;
+ private Op(CurrentUser user, String msgTxt, NotifyHandling notifyHandling) {
this.msgTxt = msgTxt;
this.notifyHandling = notifyHandling;
+ account = user.isIdentifiedUser()
+ ? user.asIdentifiedUser().getAccount()
+ : null;
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/AbandonUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/AbandonUtil.java
index f84599d..205d959 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/AbandonUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/AbandonUtil.java
@@ -14,7 +14,9 @@
package com.google.gerrit.server.change;
-import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.common.collect.ImmutableMultimap;
+import com.google.common.collect.Multimap;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.InternalUser;
import com.google.gerrit.server.config.ChangeCleanupConfig;
import com.google.gerrit.server.project.ChangeControl;
@@ -29,6 +31,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.util.Collection;
import java.util.List;
import java.util.concurrent.TimeUnit;
@@ -37,10 +40,10 @@
private static final Logger log = LoggerFactory.getLogger(AbandonUtil.class);
private final ChangeCleanupConfig cfg;
- private final InternalUser.Factory internalUserFactory;
private final ChangeQueryProcessor queryProcessor;
private final ChangeQueryBuilder queryBuilder;
private final Abandon abandon;
+ private final InternalUser internalUser;
@Inject
AbandonUtil(
@@ -50,10 +53,10 @@
ChangeQueryBuilder queryBuilder,
Abandon abandon) {
this.cfg = cfg;
- this.internalUserFactory = internalUserFactory;
this.queryProcessor = queryProcessor;
this.queryBuilder = queryBuilder;
this.abandon = abandon;
+ internalUser = internalUserFactory.create();
}
public void abandonInactiveOpenChanges() {
@@ -68,29 +71,42 @@
if (!cfg.getAbandonIfMergeable()) {
query += " -is:mergeable";
}
- List<ChangeData> changesToAbandon = queryProcessor.enforceVisibility(false)
- .query(queryBuilder.parse(query)).entities();
- int count = 0;
+
+ List<ChangeData> changesToAbandon =
+ queryProcessor
+ .enforceVisibility(false)
+ .query(queryBuilder.parse(query))
+ .entities();
+ ImmutableMultimap.Builder<Project.NameKey, ChangeControl> builder =
+ ImmutableMultimap.builder();
for (ChangeData cd : changesToAbandon) {
+ ChangeControl control = cd.changeControl(internalUser);
+ builder.put(control.getProject().getNameKey(), control);
+ }
+
+ int count = 0;
+ Multimap<Project.NameKey, ChangeControl> abandons = builder.build();
+ String message = cfg.getAbandonMessage();
+ for (Project.NameKey project : abandons.keySet()) {
+ Collection<ChangeControl> changes = abandons.get(project);
try {
- abandon.abandon(changeControl(cd), cfg.getAbandonMessage());
- count++;
- } catch (ResourceConflictException e) {
- // Change was already merged or abandoned.
+ abandon.batchAbandon(project, internalUser, changes, message);
+ count += changes.size();
} catch (Throwable e) {
- log.error(String.format(
- "Failed to auto-abandon inactive open change %d.",
- cd.getId().get()), e);
+ StringBuilder msg =
+ new StringBuilder("Failed to auto-abandon inactive change(s):");
+ for (ChangeControl change : changes) {
+ msg.append(" ").append(change.getId().get());
+ }
+ msg.append(".");
+ log.error(msg.toString(), e);
}
}
log.info(String.format("Auto-Abandoned %d of %d changes.",
count, changesToAbandon.size()));
} catch (QueryParseException | OrmException e) {
- log.error("Failed to query inactive open changes for auto-abandoning.", e);
+ log.error(
+ "Failed to query inactive open changes for auto-abandoning.", e);
}
}
-
- private ChangeControl changeControl(ChangeData cd) throws OrmException {
- return cd.changeControl(internalUserFactory.create());
- }
}
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 ab92a97..a6c2edf 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
@@ -267,7 +267,7 @@
} catch (PatchListNotAvailableException | GpgException | OrmException
| IOException | RuntimeException e) {
if (!has(CHECK)) {
- Throwables.throwIfInstanceOf(e, OrmException.class);
+ Throwables.propagateIfPossible(e, OrmException.class);
throw new OrmException(e);
}
return checkOnly(cd);
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 545ea17..1a063f4 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
@@ -14,6 +14,7 @@
package com.google.gerrit.server.change;
+import com.google.gerrit.common.data.Capable;
import com.google.gerrit.extensions.api.changes.CherryPickInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -30,6 +31,7 @@
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.RefControl;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -71,8 +73,14 @@
throw new AuthException("Cherry pick not permitted");
}
+ ProjectControl projectControl = control.getProjectControl();
+ Capable capable = projectControl.canPushToAtLeastOneRef();
+ if (capable != Capable.OK) {
+ throw new AuthException(capable.getMessage());
+ }
+
String refName = RefNames.fullName(input.destination);
- RefControl refControl = control.getProjectControl().controlForRef(refName);
+ RefControl refControl = projectControl.controlForRef(refName);
if (!refControl.canUpload()) {
throw new AuthException("Not allowed to cherry pick "
+ revision.getChange().getId().toString() + " to "
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
index f1bdba5..e25e2292 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
@@ -14,8 +14,9 @@
package com.google.gerrit.server.change;
+import static com.google.common.base.Preconditions.checkNotNull;
+
import com.google.gerrit.common.TimeUtil;
-import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.extensions.api.changes.DeleteVoteInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
@@ -28,6 +29,7 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
+import com.google.gerrit.reviewdb.client.LabelId;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -44,6 +46,7 @@
import com.google.gerrit.server.mail.DeleteVoteSender;
import com.google.gerrit.server.mail.ReplyToChangeSender;
import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.util.LabelVote;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -137,64 +140,66 @@
PatchSet.Id psId = change.currentPatchSetId();
ps = psUtil.current(db.get(), ctl.getNotes());
- PatchSetApproval psa = null;
- StringBuilder msg = new StringBuilder();
-
- // get all of the current approvals
+ boolean found = false;
LabelTypes labelTypes = ctx.getControl().getLabelTypes();
- Map<String, Short> currentApprovals = new HashMap<>();
- for (LabelType lt : labelTypes.getLabelTypes()) {
- currentApprovals.put(lt.getName(), (short) 0);
- for (PatchSetApproval a : approvalsUtil.byPatchSetUser(
- ctx.getDb(), ctl, psId, accountId)) {
- if (lt.getLabelId().equals(a.getLabelId())) {
- currentApprovals.put(lt.getName(), a.getValue());
- }
- }
- }
- // removing votes so we need to determine the new set of approval scores
- newApprovals.putAll(currentApprovals);
+
for (PatchSetApproval a : approvalsUtil.byPatchSetUser(
- ctx.getDb(), ctl, psId, accountId)) {
- if (ctl.canRemoveReviewer(a)) {
- if (a.getLabel().equals(label)) {
- // set the approval to 0 if vote is being removed
- newApprovals.put(a.getLabel(), (short) 0);
- // set old value only if the vote changed
- oldApprovals.put(a.getLabel(), a.getValue());
- msg.append("Removed ")
- .append(a.getLabel()).append(formatLabelValue(a.getValue()))
- .append(" by ").append(userFactory.create(a.getAccountId())
- .getNameEmail())
- .append("\n");
- psa = a;
- a.setValue((short)0);
- ctx.getUpdate(psId).removeApprovalFor(a.getAccountId(), label);
- break;
- }
- } else {
+ ctx.getDb(), ctl, psId, accountId)) {
+ if (labelTypes.byLabel(a.getLabelId()) == null) {
+ continue; // Ignore undefined labels.
+ } else if (!a.getLabel().equals(label)) {
+ // Populate map for non-matching labels, needed by VoteDeleted.
+ newApprovals.put(a.getLabel(), a.getValue());
+ continue;
+ } else if (!ctl.canRemoveReviewer(a)) {
throw new AuthException("delete vote not permitted");
}
+ // Set the approval to 0 if vote is being removed.
+ newApprovals.put(a.getLabel(), (short) 0);
+ found = true;
+
+ // Set old value, as required by VoteDeleted.
+ oldApprovals.put(a.getLabel(), a.getValue());
+ break;
}
- if (psa == null) {
+ if (!found) {
throw new ResourceNotFoundException();
}
- ctx.getDb().patchSetApprovals().update(Collections.singleton(psa));
- if (msg.length() > 0) {
- changeMessage =
- new ChangeMessage(new ChangeMessage.Key(change.getId(),
- ChangeUtil.messageUUID(ctx.getDb())),
- ctx.getAccountId(),
- ctx.getWhen(),
- change.currentPatchSetId());
- changeMessage.setMessage(msg.toString());
- cmUtil.addChangeMessage(ctx.getDb(), ctx.getUpdate(psId),
- changeMessage);
- }
+ ctx.getUpdate(psId).removeApprovalFor(accountId, label);
+ ctx.getDb().patchSetApprovals().upsert(
+ Collections.singleton(deletedApproval(ctx)));
+
+ changeMessage =
+ new ChangeMessage(new ChangeMessage.Key(change.getId(),
+ ChangeUtil.messageUUID(ctx.getDb())),
+ ctx.getAccountId(),
+ ctx.getWhen(),
+ change.currentPatchSetId());
+ StringBuilder msg = new StringBuilder();
+ msg.append("Removed ");
+ LabelVote.appendTo(msg, label, checkNotNull(oldApprovals.get(label)));
+ changeMessage.setMessage(
+ msg.append(" by ")
+ .append(userFactory.create(accountId).getNameEmail())
+ .append("\n")
+ .toString());
+ cmUtil.addChangeMessage(ctx.getDb(), ctx.getUpdate(psId),
+ changeMessage);
+
return true;
}
+ private PatchSetApproval deletedApproval(ChangeContext ctx) {
+ return new PatchSetApproval(
+ new PatchSetApproval.Key(
+ ps.getId(),
+ accountId,
+ new LabelId(label)),
+ (short) 0,
+ ctx.getWhen());
+ }
+
@Override
public void postUpdate(Context ctx) {
if (changeMessage == null) {
@@ -220,11 +225,4 @@
user.getAccount(), ctx.getWhen());
}
}
-
- private static String formatLabelValue(short value) {
- if (value > 0) {
- return "+" + value;
- }
- return Short.toString(value);
- }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Files.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Files.java
index 1631d48..35dbec1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Files.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Files.java
@@ -29,7 +29,6 @@
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -49,7 +48,6 @@
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevTree;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.treewalk.TreeWalk;
import org.eclipse.jgit.treewalk.filter.PathFilterGroup;
@@ -69,15 +67,11 @@
public class Files implements ChildCollection<RevisionResource, FileResource> {
private final DynamicMap<RestView<FileResource>> views;
private final Provider<ListFiles> list;
- private final GitRepositoryManager repoManager;
@Inject
- Files(DynamicMap<RestView<FileResource>> views,
- Provider<ListFiles> list,
- GitRepositoryManager repoManager) {
+ Files(DynamicMap<RestView<FileResource>> views, Provider<ListFiles> list) {
this.views = views;
this.list = list;
- this.repoManager = repoManager;
}
@Override
@@ -91,20 +85,8 @@
}
@Override
- public FileResource parse(RevisionResource rev, IdString id)
- throws ResourceNotFoundException, IOException {
- if (Patch.COMMIT_MSG.equals(id.get())) {
- return new FileResource(rev, id.get());
- }
- try (Repository repo = repoManager.openRepository(rev.getProject());
- RevWalk rw = new RevWalk(repo)) {
- RevTree tree = rw.parseTree(
- ObjectId.fromString(rev.getPatchSet().getRevision().get()));
- if (TreeWalk.forPath(repo, id.get(), tree) != null) {
- return new FileResource(rev, id.get());
- }
- }
- throw new ResourceNotFoundException(id);
+ public FileResource parse(RevisionResource rev, IdString id) {
+ return new FileResource(rev, id.get());
}
public static final class ListFiles implements RestReadView<RevisionResource> {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java
index ade46be..3ca496a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java
@@ -16,6 +16,7 @@
import com.google.common.base.Strings;
import com.google.gerrit.common.TimeUtil;
+import com.google.gerrit.common.data.Capable;
import com.google.gerrit.extensions.api.changes.RevertInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -48,6 +49,7 @@
import com.google.gerrit.server.mail.RevertedSender;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.RefControl;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -122,6 +124,13 @@
throws IOException, OrmException, RestApiException,
UpdateException, NoSuchChangeException {
RefControl refControl = req.getControl().getRefControl();
+ ProjectControl projectControl = req.getControl().getProjectControl();
+
+ Capable capable = projectControl.canPushToAtLeastOneRef();
+ if (capable != Capable.OK) {
+ throw new AuthException(capable.getMessage());
+ }
+
Change change = req.getChange();
if (!refControl.canUpload()) {
throw new AuthException("revert not permitted");
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java
index eb01de3..090d99d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java
@@ -464,7 +464,7 @@
throw new ResourceNotFoundException(e.getMessage(), e);
} catch (Exception e) {
- Throwables.throwIfUnchecked(e);
+ Throwables.propagateIfPossible(e);
throw new UpdateException(e);
}
}
@@ -669,7 +669,7 @@
logDebug("No objects to flush");
}
} catch (Exception e) {
- Throwables.throwIfInstanceOf(e, RestApiException.class);
+ Throwables.propagateIfPossible(e, RestApiException.class);
throw new UpdateException(e);
}
}
@@ -744,8 +744,8 @@
maybeLogSlowUpdate(startNanos, "NoteDb");
}
} catch (ExecutionException | InterruptedException e) {
- Throwables.throwIfInstanceOf(e.getCause(), UpdateException.class);
- Throwables.throwIfInstanceOf(e.getCause(), RestApiException.class);
+ Throwables.propagateIfInstanceOf(e.getCause(), UpdateException.class);
+ Throwables.propagateIfInstanceOf(e.getCause(), RestApiException.class);
throw new UpdateException(e);
} catch (OrmException | IOException e) {
throw new UpdateException(e);
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/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 8272aaf..249db3e 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
@@ -29,12 +29,12 @@
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TOPIC;
import static com.google.gerrit.server.notedb.NoteDbTable.CHANGES;
+import com.google.auto.value.AutoValue;
import com.google.common.base.Enums;
import com.google.common.base.Function;
import com.google.common.base.Joiner;
import com.google.common.base.Optional;
import com.google.common.base.Splitter;
-import com.google.common.base.Supplier;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableSet;
@@ -46,6 +46,7 @@
import com.google.common.collect.Table;
import com.google.common.collect.Tables;
import com.google.common.primitives.Ints;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.metrics.Timer1;
@@ -85,7 +86,6 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
-import java.util.Map.Entry;
import java.util.NavigableSet;
import java.util.Objects;
import java.util.Set;
@@ -97,6 +97,20 @@
private static final RevId PARTIAL_PATCH_SET =
new RevId("INVALID PARTIAL PATCH SET");
+ @AutoValue
+ static abstract class ApprovalKey {
+ abstract PatchSet.Id psId();
+ abstract Account.Id accountId();
+ abstract String label();
+ @Nullable abstract String tag();
+
+ private static ApprovalKey create(PatchSet.Id psId, Account.Id accountId,
+ String label, @Nullable String tag) {
+ return new AutoValue_ChangeNotesParser_ApprovalKey(
+ psId, accountId, label, tag);
+ }
+ }
+
// Private final members initialized in the constructor.
private final ChangeNoteUtil noteUtil;
private final NoteDbMetrics metrics;
@@ -114,8 +128,7 @@
private final TreeMap<PatchSet.Id, PatchSet> patchSets;
private final Set<PatchSet.Id> deletedPatchSets;
private final Map<PatchSet.Id, PatchSetState> patchSetStates;
- private final Map<PatchSet.Id,
- Table<Account.Id, Entry<String, String>, Optional<PatchSetApproval>>> approvals;
+ private final Map<ApprovalKey, PatchSetApproval> approvals;
private final List<ChangeMessage> allChangeMessages;
private final Multimap<PatchSet.Id, ChangeMessage> changeMessagesByPatchSet;
@@ -142,7 +155,7 @@
this.walk = walk;
this.noteUtil = noteUtil;
this.metrics = metrics;
- approvals = new HashMap<>();
+ approvals = new LinkedHashMap<>();
reviewers = HashBasedTable.create();
allPastReviewers = new ArrayList<>();
reviewerUpdates = new ArrayList<>();
@@ -210,14 +223,15 @@
}
private Multimap<PatchSet.Id, PatchSetApproval> buildApprovals() {
- Multimap<PatchSet.Id, PatchSetApproval> result =
- ArrayListMultimap.create(approvals.keySet().size(), 3);
- for (Table<?, ?, Optional<PatchSetApproval>> curr : approvals.values()) {
- for (Optional<PatchSetApproval> psa : curr.values()) {
- if (psa.isPresent()) {
- result.put(psa.get().getPatchSetId(), psa.get());
- }
+ Multimap<PatchSet.Id, PatchSetApproval> result = ArrayListMultimap.create();
+ for (PatchSetApproval a : approvals.values()) {
+ if (patchSetStates.get(a.getPatchSetId()) == PatchSetState.DELETED) {
+ continue; // Patch set was explicitly deleted.
+ } else if (allPastReviewers.contains(a.getAccountId())
+ && !reviewers.containsRow(a.getAccountId())) {
+ continue; // Reviewer was explicitly removed.
}
+ result.put(a.getPatchSetId(), a);
}
for (Collection<PatchSetApproval> v : result.asMap().values()) {
Collections.sort((List<PatchSetApproval>) v, ChangeNotes.PSA_BY_TIME);
@@ -606,7 +620,7 @@
"patch set %s requires an identified user as uploader", psId.get());
}
if (line.startsWith("-")) {
- parseRemoveApproval(psId, accountId, line);
+ parseRemoveApproval(psId, accountId, ts, line);
} else {
parseAddApproval(psId, accountId, ts, line);
}
@@ -637,39 +651,37 @@
throw pe;
}
- Entry<String, String> label = Maps.immutableEntry(l.label(), tag);
- Table<Account.Id, Entry<String, String>, Optional<PatchSetApproval>> curr =
- getApprovalsTableIfNoVotePresent(psId, accountId, label);
- if (curr != null) {
- PatchSetApproval psa = new PatchSetApproval(
- new PatchSetApproval.Key(
- psId,
- accountId,
- new LabelId(l.label())),
- l.value(),
- ts);
- psa.setTag(tag);
- curr.put(accountId, label, Optional.of(psa));
+ PatchSetApproval psa = new PatchSetApproval(
+ new PatchSetApproval.Key(
+ psId,
+ accountId,
+ new LabelId(l.label())),
+ l.value(),
+ ts);
+ psa.setTag(tag);
+ ApprovalKey k = ApprovalKey.create(psId, accountId, l.label(), tag);
+ if (!approvals.containsKey(k)) {
+ approvals.put(k, psa);
}
}
private void parseRemoveApproval(PatchSet.Id psId, Account.Id committerId,
- String line) throws ConfigInvalidException {
+ Timestamp ts, String line) throws ConfigInvalidException {
Account.Id accountId;
- Entry<String, String> label;
+ String label;
int s = line.indexOf(' ');
if (s > 0) {
- label = Maps.immutableEntry(line.substring(1, s), tag);
+ label = line.substring(1, s);
PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1));
checkFooter(ident != null, FOOTER_LABEL, line);
accountId = noteUtil.parseIdent(ident, id);
} else {
- label = Maps.immutableEntry(line.substring(1), tag);
+ label = line.substring(1);
accountId = committerId;
}
try {
- LabelType.checkNameInternal(label.getKey());
+ LabelType.checkNameInternal(label);
} catch (IllegalArgumentException e) {
ConfigInvalidException pe =
parseException("invalid %s: %s", FOOTER_LABEL, line);
@@ -677,38 +689,25 @@
throw pe;
}
- Table<Account.Id, Entry<String, String>, Optional<PatchSetApproval>> curr =
- getApprovalsTableIfNoVotePresent(psId, accountId, label);
- if (curr != null) {
- curr.put(accountId, label, Optional.<PatchSetApproval> absent());
+ // Store an actual 0-vote approval in the map for a removed approval, for
+ // several reasons:
+ // - This is closer to the ReviewDb representation, which leads to less
+ // confusion and special-casing of NoteDb.
+ // - More importantly, ApprovalCopier needs an actual approval in order to
+ // block copying an earlier approval over a later delete.
+ PatchSetApproval remove = new PatchSetApproval(
+ new PatchSetApproval.Key(
+ psId,
+ accountId,
+ new LabelId(label)),
+ (short) 0,
+ ts);
+ ApprovalKey k = ApprovalKey.create(psId, accountId, label, tag);
+ if (!approvals.containsKey(k)) {
+ approvals.put(k, remove);
}
}
- private Table<Account.Id, Entry<String, String>, Optional<PatchSetApproval>>
- getApprovalsTableIfNoVotePresent(PatchSet.Id psId, Account.Id accountId,
- Entry<String, String> label) {
-
- Table<Account.Id, Entry<String, String>, Optional<PatchSetApproval>> curr =
- approvals.get(psId);
- if (curr != null) {
- if (curr.contains(accountId, label)) {
- return null;
- }
- } else {
- curr = Tables.newCustomTable(
- Maps.<Account.Id, Map<Entry<String, String>, Optional<PatchSetApproval>>>
- newHashMapWithExpectedSize(2),
- new Supplier<Map<Entry<String, String>, Optional<PatchSetApproval>>>() {
- @Override
- public Map<Entry<String, String>, Optional<PatchSetApproval>> get() {
- return new LinkedHashMap<>();
- }
- });
- approvals.put(psId, curr);
- }
- return curr;
- }
-
private void parseSubmitRecords(List<String> lines)
throws ConfigInvalidException {
SubmitRecord rec = null;
@@ -787,9 +786,6 @@
Table.Cell<Account.Id, ReviewerStateInternal, Timestamp> e = rit.next();
if (e.getColumnKey() == ReviewerStateInternal.REMOVED) {
rit.remove();
- for (Table<Account.Id, ?, ?> curr : approvals.values()) {
- curr.rowKeySet().remove(e.getRowKey());
- }
}
}
}
@@ -832,13 +828,14 @@
// Post-process other collections to remove items corresponding to deleted
// patch sets. This is safer than trying to prevent insertion, as it will
// also filter out items racily added after the patch set was deleted.
+ //
+ // Approvals are filtered in buildApprovals().
NavigableSet<PatchSet.Id> all = patchSets.navigableKeySet();
if (!all.isEmpty()) {
currentPatchSetId = all.last();
} else {
currentPatchSetId = null;
}
- approvals.keySet().retainAll(all);
changeMessagesByPatchSet.keys().retainAll(all);
for (Iterator<ChangeMessage> it = allChangeMessages.iterator();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RepoSequence.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RepoSequence.java
index c47fd4f..071e12c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RepoSequence.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RepoSequence.java
@@ -197,7 +197,7 @@
limit = counter + count;
acquireCount++;
} catch (ExecutionException | RetryException e) {
- Throwables.throwIfInstanceOf(e.getCause(), OrmException.class);
+ Throwables.propagateIfInstanceOf(e.getCause(), OrmException.class);
throw new OrmException(e);
} catch (IOException e) {
throw new OrmException(e);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineLoader.java
index ae37c01..dd15cfc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineLoader.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineLoader.java
@@ -93,7 +93,7 @@
} catch (ExecutionException e) {
// If there was an error computing the result, carry it
// up to the caller so the cache knows this key is invalid.
- Throwables.throwIfInstanceOf(e.getCause(), Exception.class);
+ Throwables.propagateIfInstanceOf(e.getCause(), Exception.class);
throw new Exception(e.getMessage(), e.getCause());
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java
index 1156b91..faeaaf2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java
@@ -260,7 +260,7 @@
} catch (ExecutionException e) {
// If there was an error computing the result, carry it
// up to the caller so the cache knows this key is invalid.
- Throwables.throwIfInstanceOf(e.getCause(), IOException.class);
+ Throwables.propagateIfInstanceOf(e.getCause(), IOException.class);
throw new IOException(e.getMessage(), e.getCause());
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java
index 2097ebd..d27d4f9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java
@@ -148,7 +148,7 @@
} catch (ExecutionException e) {
if (!(e.getCause() instanceof RepositoryNotFoundException)) {
log.warn(String.format("Cannot read project %s", projectName.get()), e);
- Throwables.throwIfInstanceOf(e.getCause(), IOException.class);
+ Throwables.propagateIfInstanceOf(e.getCause(), IOException.class);
throw new IOException(e);
}
return null;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
index 25f80ae..22e5d69 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
@@ -384,7 +384,7 @@
}
final StringBuilder msg = new StringBuilder();
- msg.append(" A Contributor Agreement must be completed before uploading");
+ msg.append("A Contributor Agreement must be completed before uploading");
if (canonicalWebUrl != null) {
msg.append(":\n\n ");
msg.append(canonicalWebUrl);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPattern.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPattern.java
index 8c850fb..ed50a54 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPattern.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPattern.java
@@ -49,7 +49,7 @@
try {
return exampleCache.get(refPattern);
} catch (ExecutionException e) {
- Throwables.throwIfUnchecked(e.getCause());
+ Throwables.propagateIfPossible(e.getCause());
throw new RuntimeException(e);
}
} else if (refPattern.endsWith("/*")) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/AndSource.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/AndSource.java
index c2b8b03..168be5d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/AndSource.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/AndSource.java
@@ -84,7 +84,7 @@
try {
return readImpl();
} catch (OrmRuntimeException err) {
- Throwables.throwIfInstanceOf(err.getCause(), OrmException.class);
+ Throwables.propagateIfInstanceOf(err.getCause(), OrmException.class);
throw new OrmException(err);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryProcessor.java
index 70bdffb..d08f05c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryProcessor.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryProcessor.java
@@ -140,7 +140,7 @@
} catch (OrmRuntimeException e) {
throw new OrmException(e.getMessage(), e);
} catch (OrmException e) {
- Throwables.throwIfInstanceOf(e.getCause(), QueryParseException.class);
+ Throwables.propagateIfInstanceOf(e.getCause(), QueryParseException.class);
throw e;
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/util/LabelVote.java b/gerrit-server/src/main/java/com/google/gerrit/server/util/LabelVote.java
index fab0b34..030383a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/util/LabelVote.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/util/LabelVote.java
@@ -58,6 +58,16 @@
Short.parseShort(text.substring(e + 1), text.length()));
}
+ public static StringBuilder appendTo(StringBuilder sb, String label,
+ short value) {
+ if (value == (short) 0) {
+ return sb.append('-').append(label);
+ } else if (value < 0) {
+ return sb.append(label).append(value);
+ }
+ return sb.append(label).append('+').append(value);
+ }
+
public static LabelVote create(String label, short value) {
return new AutoValue_LabelVote(LabelType.checkNameInternal(label), value);
}
@@ -70,13 +80,9 @@
public abstract short value();
public String format() {
- if (value() == (short) 0) {
- return '-' + label();
- } else if (value() < 0) {
- return label() + value();
- } else {
- return label() + '+' + value();
- }
+ // Max short string length is "-32768".length() == 6.
+ return appendTo(new StringBuilder(label().length() + 6), label(), value())
+ .toString();
}
public String formatWithEquals() {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/util/RequestScopePropagator.java b/gerrit-server/src/main/java/com/google/gerrit/server/util/RequestScopePropagator.java
index bdbb938..382485e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/util/RequestScopePropagator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/util/RequestScopePropagator.java
@@ -129,7 +129,7 @@
try {
wrapped.call();
} catch (Exception e) {
- Throwables.throwIfUnchecked(e);
+ Throwables.propagateIfPossible(e);
throw new RuntimeException(e); // Not possible.
}
}
diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/mime/mime-types.properties b/gerrit-server/src/main/resources/com/google/gerrit/server/mime/mime-types.properties
index d51547c..5a937b6 100644
--- a/gerrit-server/src/main/resources/com/google/gerrit/server/mime/mime-types.properties
+++ b/gerrit-server/src/main/resources/com/google/gerrit/server/mime/mime-types.properties
@@ -97,7 +97,7 @@
in = text/x-properties
ini = text/x-properties
intr = text/x-dylan
-jade = text/x-jade
+jade = text/x-pug
java = text/x-java
jl = text/x-julia
jruby = text/x-ruby
@@ -163,6 +163,7 @@
ps1 = application/x-powershell
psd1 = application/x-powershell
psm1 = application/x-powershell
+pug = text/x-pug
py = text/x-python
pyw = text/x-python
pyx = text/x-cython
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 0173b05..a29d397 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
@@ -322,7 +322,10 @@
update.commit();
notes = newNotes(c);
- assertThat(notes.getApprovals()).isEmpty();
+ assertThat(notes.getApprovals()).containsExactlyEntriesIn(
+ ImmutableMultimap.of(
+ psa.getPatchSetId(),
+ new PatchSetApproval(psa.getKey(), (short) 0, update.getWhen())));
}
@Test
@@ -344,7 +347,10 @@
update.commit();
notes = newNotes(c);
- assertThat(notes.getApprovals()).isEmpty();
+ assertThat(notes.getApprovals()).containsExactlyEntriesIn(
+ ImmutableMultimap.of(
+ psa.getPatchSetId(),
+ new PatchSetApproval(psa.getKey(), (short) 0, update.getWhen())));
// Add back approval on same label.
update = newUpdate(c, otherUser);
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AliasCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AliasCommand.java
index 4ddca0c..fde3a66 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AliasCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AliasCommand.java
@@ -97,7 +97,7 @@
try {
cmd.destroy();
} catch (Exception e) {
- Throwables.throwIfUnchecked(e);
+ Throwables.propagateIfPossible(e);
throw new RuntimeException(e);
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DispatchCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DispatchCommand.java
index f3243c6..f2911dc 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DispatchCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DispatchCommand.java
@@ -136,7 +136,7 @@
try {
cmd.destroy();
} catch (Exception e) {
- Throwables.throwIfUnchecked(e);
+ Throwables.propagateIfPossible(e);
throw new RuntimeException(e);
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java
index c88a02c..24bd8c2 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SuExec.java
@@ -153,7 +153,7 @@
try {
cmd.destroy();
} catch (Exception e) {
- Throwables.throwIfUnchecked(e);
+ Throwables.propagateIfPossible(e);
throw new RuntimeException(e);
}
}
diff --git a/lib/BUCK b/lib/BUCK
index efdf0eb..4534b44 100644
--- a/lib/BUCK
+++ b/lib/BUCK
@@ -67,10 +67,9 @@
maven_jar(
name = 'guava',
- id = 'com.google.guava:guava:20.0:20160818.201422-323',
- sha1 = '13af7470db1026c57aedd0144018e06fe79bba33',
+ id = 'com.google.guava:guava:19.0',
+ sha1 = '6ce200f6b23222af3d8abb6b6459e6c44f4bb0e9',
license = 'Apache2.0',
- repository = MAVEN_SNAPSHOT,
)
maven_jar(
diff --git a/lib/BUILD b/lib/BUILD
index e89e63c..a490038 100644
--- a/lib/BUILD
+++ b/lib/BUILD
@@ -151,6 +151,7 @@
visibility = ['//visibility:public'],
)
+
java_library(
name = 'h2',
exports = ['@h2//jar'],
@@ -202,3 +203,31 @@
exports = ['@derby//jar'],
visibility = ['//visibility:public'],
)
+
+java_library(
+ name = 'soy',
+ exports = ['@soy//jar'],
+ runtime_deps = [
+ ':args4j',
+ ':guava',
+ ':gson',
+ ':icu4j',
+ ':jsr305',
+ ':protobuf',
+ '//lib/guice:guice',
+ '//lib/guice:guice-assistedinject',
+ '//lib/guice:multibindings',
+ '//lib/guice:javax-inject',
+ '//lib/ow2:ow2-asm',
+ '//lib/ow2:ow2-asm-analysis',
+ '//lib/ow2:ow2-asm-commons',
+ '//lib/ow2:ow2-asm-util',
+ ],
+ visibility = ['//visibility:public'],
+)
+
+java_library(
+ name = 'icu4j',
+ exports = [ '@icu4j//jar' ],
+ visibility = ['//visibility:public'],
+)
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/codemirror/BUCK b/lib/codemirror/BUCK
index a0e0e9a..56145ea 100644
--- a/lib/codemirror/BUCK
+++ b/lib/codemirror/BUCK
@@ -1,14 +1,14 @@
include_defs('//lib/maven.defs')
include_defs('//lib/codemirror/cm.defs')
-VERSION = '5.17.0'
+VERSION = '5.18.2'
TOP = 'META-INF/resources/webjars/codemirror/%s' % VERSION
TOP_MINIFIED = 'META-INF/resources/webjars/codemirror-minified/%s' % VERSION
maven_jar(
name = 'codemirror-minified',
id = 'org.webjars.npm:codemirror-minified:' + VERSION,
- sha1 = '05ad901fc9be67eb7ba8997d896488093deb898e',
+ sha1 = '6755af157a7eaf2401468906bef67bbacc3c97f6',
attach_source = False,
license = 'codemirror-minified',
visibility = [],
@@ -17,7 +17,7 @@
maven_jar(
name = 'codemirror-original',
id = 'org.webjars.npm:codemirror:' + VERSION,
- sha1 = 'c025b8d9aca1061e26d1fa482bea0ecea1412e85',
+ sha1 = '18c721ae88eed27cddb458c42f5d221fa3d9713e',
attach_source = False,
license = 'codemirror-original',
visibility = [],
diff --git a/lib/codemirror/cm.defs b/lib/codemirror/cm.defs
index baf2ce5..a1be90f 100644
--- a/lib/codemirror/cm.defs
+++ b/lib/codemirror/cm.defs
@@ -132,7 +132,6 @@
'htmlmixed',
'http',
'idl',
- 'jade',
'javascript',
'jinja2',
'jsx',
@@ -160,6 +159,7 @@
'powershell',
'properties',
'protobuf',
+ 'pug',
'puppet',
'python',
'q',
diff --git a/lib/guice/BUILD b/lib/guice/BUILD
index acade50..5850af2 100644
--- a/lib/guice/BUILD
+++ b/lib/guice/BUILD
@@ -3,6 +3,7 @@
exports = [
':guice_library',
':javax-inject',
+ ':multibindings',
],
visibility = ['//visibility:public'],
)
@@ -36,4 +37,11 @@
java_library(
name = 'javax-inject',
exports = ['@javax_inject//jar'],
+ visibility = ['//visibility:public'],
+)
+
+java_library(
+ name = 'multibindings',
+ exports = [ '@multibindings//jar' ],
+ visibility = ['//visibility:public'],
)
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/plugins/replication b/plugins/replication
index 75af773..5cac325 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 75af77375b34133e85f3ee5f1b19dac19d3f3837
+Subproject commit 5cac325cca171205130c53df8b3ee9ab3b115979
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 1fe09e5..09d5b84 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
@@ -96,13 +96,12 @@
hidden></gr-confirm-rebase-dialog>
<gr-confirm-cherrypick-dialog id="confirmCherrypick"
class="confirmDialog"
- commit-info="[[commitInfo]]"
+ message="[[commitMessage]]"
on-confirm="_handleCherrypickConfirm"
on-cancel="_handleConfirmDialogCancel"
hidden></gr-confirm-cherrypick-dialog>
<gr-confirm-revert-dialog id="confirmRevertDialog"
class="confirmDialog"
- commit-info="[[commitInfo]]"
on-confirm="_handleRevertDialogConfirm"
on-cancel="_handleConfirmDialogCancel"
hidden></gr-confirm-revert-dialog>
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 3445f4e..8deab6b 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
@@ -75,7 +75,10 @@
},
changeNum: String,
patchNum: String,
- commitInfo: Object,
+ commitMessage: {
+ type: String,
+ value: '',
+ },
_loading: {
type: Boolean,
@@ -274,7 +277,7 @@
if (type === ActionType.REVISION) {
this._handleRevisionAction(key);
} else if (key === ChangeActions.REVERT) {
- this.$.confirmRevertDialog.populateRevertMessage();
+ this.$.confirmRevertDialog.populateRevertMessage(this.commitMessage);
this.$.confirmRevertDialog.message = this._modifyRevertMsg();
this._showActionDialog(this.$.confirmRevertDialog);
} else if (key === ChangeActions.ABANDON) {
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 8b51312..1782061 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
@@ -128,18 +128,6 @@
<span class="value">[[change.branch]]</span>
</section>
<section>
- <span class="title">Commit</span>
- <span class="value">
- <template is="dom-if" if="[[_showWebLink]]">
- <a target="_blank"
- href$="[[_webLink]]">[[_computeShortHash(commitInfo)]]</a>
- </template>
- <template is="dom-if" if="[[!_showWebLink]]">
- [[_computeShortHash(commitInfo)]]
- </template>
- </span>
- </section>
- <section>
<span class="title">Topic</span>
<span class="value">
<gr-editable-label
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 af19703..20c117e 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
@@ -27,17 +27,8 @@
properties: {
change: Object,
- commitInfo: Object,
mutable: Boolean,
serverConfig: Object,
- _showWebLink: {
- type: Boolean,
- computed: '_computeShowWebLink(change, commitInfo, serverConfig)',
- },
- _webLink: {
- type: String,
- computed: '_computeWebLink(change, commitInfo, serverConfig)',
- },
_topicReadOnly: {
type: Boolean,
computed: '_computeTopicReadOnly(mutable, change)',
@@ -52,38 +43,6 @@
Gerrit.RESTClientBehavior,
],
- _computeShowWebLink: function(change, commitInfo, serverConfig) {
- var webLink = commitInfo.web_links && commitInfo.web_links.length;
- var gitWeb = serverConfig.gitweb && serverConfig.gitweb.url &&
- serverConfig.gitweb.type && serverConfig.gitweb.type.revision;
- return webLink || gitWeb;
- },
-
- _computeWebLink: function(change, commitInfo, serverConfig) {
- if (!this._computeShowWebLink(change, commitInfo, serverConfig)) {
- return;
- }
-
- if (serverConfig.gitweb && serverConfig.gitweb.url &&
- serverConfig.gitweb.type && serverConfig.gitweb.type.revision) {
- return serverConfig.gitweb.url +
- serverConfig.gitweb.type.revision
- .replace('${project}', change.project)
- .replace('${commit}', commitInfo.commit);
- }
-
- var webLink = commitInfo.web_links[0].url;
- if (!/^https?\:\/\//.test(webLink)) {
- webLink = '../../' + webLink;
- }
-
- return webLink;
- },
-
- _computeShortHash: function(commitInfo) {
- return commitInfo.commit.slice(0, 7);
- },
-
_computeHideStrategy: function(change) {
return !this.changeIsOpen(change.status);
},
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 01f0649..a2d4946 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
@@ -20,11 +20,9 @@
<script src="../../../bower_components/webcomponentsjs/webcomponents.min.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
-<script src="../../../bower_components/page/page.js"></script>
<link rel="import" href="../../../bower_components/iron-test-helpers/iron-test-helpers.html">
<link rel="import" href="gr-change-metadata.html">
-<script src="../../../scripts/util.js"></script>
<test-fixture id="basic">
<template>
@@ -68,79 +66,6 @@
assert.isTrue(element.$$('.strategy').hasAttribute('hidden'));
});
- test('no web link when unavailable', function() {
- element.commitInfo = {};
- element.serverConfig = {};
- element.change = {labels: []};
-
- assert.isNotOk(element._computeShowWebLink(element.change,
- element.commitInfo, element.serverConfig));
- });
-
- test('use web link when available', function() {
- element.commitInfo = {web_links: [{url: 'link-url'}]};
- element.serverConfig = {};
-
- assert.isOk(element._computeShowWebLink(element.change,
- element.commitInfo, element.serverConfig));
- assert.equal(element._computeWebLink(element.change, element.commitInfo,
- element.serverConfig), '../../link-url');
- });
-
- test('does not relativize web links that begin with scheme', function() {
- element.commitInfo = {web_links: [{url: 'https://link-url'}]};
- element.serverConfig = {};
-
- assert.isOk(element._computeShowWebLink(element.change,
- element.commitInfo, element.serverConfig));
- assert.equal(element._computeWebLink(element.change, element.commitInfo,
- element.serverConfig), 'https://link-url');
- });
-
- test('use gitweb when available', function() {
- element.commitInfo = {commit: 'commit-sha'};
- element.serverConfig = {gitweb: {
- url: 'url-base/',
- type: {revision: 'xx ${project} xx ${commit} xx'},
- }};
- element.change = {
- project: 'project-name',
- labels: [],
- current_revision: element.commitInfo.commit
- };
-
- assert.isOk(element._computeShowWebLink(element.change,
- element.commitInfo, element.serverConfig));
-
- assert.equal(element._computeWebLink(element.change, element.commitInfo,
- element.serverConfig), 'url-base/xx project-name xx commit-sha xx');
- });
-
- test('prefer gitweb when both are available', function() {
- element.commitInfo = {
- commit: 'commit-sha',
- web_links: [{url: 'link-url'}]
- };
- element.serverConfig = {gitweb: {
- url: 'url-base/',
- type: {revision: 'xx ${project} xx ${commit} xx'},
- }};
- element.change = {
- project: 'project-name',
- labels: [],
- current_revision: element.commitInfo.commit
- };
-
- assert.isOk(element._computeShowWebLink(element.change,
- element.commitInfo, element.serverConfig));
-
- var link = element._computeWebLink(element.change, element.commitInfo,
- element.serverConfig);
-
- assert.equal(link, 'url-base/xx project-name xx commit-sha xx');
- assert.notEqual(link, '../../link-url');
- });
-
test('show CC section when NoteDb enabled', function() {
function hasCc() {
return element._showReviewersByState;
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index cad4298..61b10e8 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -29,6 +29,7 @@
<link rel="import" href="../gr-change-actions/gr-change-actions.html">
<link rel="import" href="../gr-change-metadata/gr-change-metadata.html">
+<link rel="import" href="../gr-commit-info/gr-commit-info.html">
<link rel="import" href="../gr-download-dialog/gr-download-dialog.html">
<link rel="import" href="../gr-file-list/gr-file-list.html">
<link rel="import" href="../gr-messages-list/gr-messages-list.html">
@@ -136,6 +137,12 @@
border: 1px solid #ddd;
margin: 1em var(--default-horizontal-margin);
}
+ .patchInfo--oldPatchSet .patchInfo-header {
+ background-color: #fff9c4;
+ }
+ .patchInfo--oldPatchSet .latestPatchContainer {
+ display: initial;
+ }
.patchInfo-header,
gr-file-list {
padding: .5em calc(var(--default-horizontal-margin) / 2);
@@ -143,6 +150,11 @@
.patchInfo-header {
background-color: #f6f6f6;
border-bottom: 1px solid #ebebeb;
+ display: flex;
+ justify-content: space-between;
+ }
+ .latestPatchContainer {
+ display: none;
}
@media screen and (max-width: 50em) {
.header {
@@ -210,7 +222,6 @@
<div class="changeInfo-column changeMetadata">
<gr-change-metadata
change="{{_change}}"
- commit-info="[[_commitInfo]]"
server-config="[[serverConfig]]"
mutable="[[_loggedIn]]"
on-show-reply-dialog="_handleShowReplyDialog">
@@ -226,8 +237,8 @@
change="[[_change]]"
actions="[[_change.actions]]"
change-num="[[_changeNum]]"
- patch-num="[[_patchRange.patchNum]]"
- commit-info="[[_commitInfo]]"
+ patch-num="[[_computeLatestPatchNum(_allPatchSets)]]"
+ commit-message="[[_latestCommitMessage]]"
on-reload-change="_handleReloadChange"></gr-change-actions>
</div>
<div class="commitAndRelated">
@@ -252,12 +263,12 @@
</div>
</div>
</section>
- <section class="patchInfo">
+ <section class$="patchInfo [[_computePatchInfoClass(_patchRange.patchNum, _allPatchSets)]]">
<div class="patchInfo-header">
- <span>
+ <div>
<label class="patchSelectLabel" for="patchSetSelect">Patch set</label>
<select id="patchSetSelect" on-change="_handlePatchChange">
- <template is="dom-repeat" items="{{_allPatchSets}}" as="patchNumber">
+ <template is="dom-repeat" items="[[_allPatchSets]]" as="patchNumber">
<option value$="[[patchNumber]]" selected$="[[_computePatchIndexIsSelected(index, _patchRange.patchNum)]]">
<span>[[patchNumber]]</span>
/
@@ -265,13 +276,21 @@
</option>
</template>
</select>
- </span>
- <span class="downloadContainer">
- /
- <gr-button link
- class="download"
- on-tap="_handleDownloadTap">Download</gr-button>
- </span>
+ <span class="downloadContainer">
+ /
+ <gr-button link
+ class="download"
+ on-tap="_handleDownloadTap">Download</gr-button>
+ </span>
+ <span class="latestPatchContainer">
+ /
+ <a href$="/c/[[_change._number]]">Go to latest patch set</a>
+ </span>
+ </div>
+ <gr-commit-info
+ change="[[_change]]"
+ server-config="[[serverConfig]]"
+ commit-info="[[_commitInfo]]"></gr-commit-info>
</div>
<gr-file-list id="fileList"
change="[[_change]]"
@@ -306,8 +325,7 @@
with-backdrop>
<gr-reply-dialog id="replyDialog"
change="[[_change]]"
- patch-num="[[_patchRange.patchNum]]"
- revisions="[[_change.revisions]]"
+ patch-num="[[_computeLatestPatchNum(_allPatchSets)]]"
labels="[[_change.labels]]"
permitted-labels="[[_change.permitted_labels]]"
diff-drafts="[[_diffDrafts]]"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 726e7e5..b6c18e5 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -334,6 +334,12 @@
_resetFileListViewState: function() {
this.set('viewState.selectedFileIndex', 0);
+ if (!!this.viewState.changeNum &&
+ this.viewState.changeNum !== this._changeNum) {
+ // Reset the diff mode to null when navigating from one change to
+ // another, so that the user's preference is restored.
+ this.set('viewState.diffMode', null);
+ }
this.set('viewState.changeNum', this._changeNum);
this.set('viewState.patchRange', this._patchRange);
},
@@ -390,6 +396,14 @@
return allPatchSets[allPatchSets.length - 1];
},
+ _computePatchInfoClass: function(patchNum, allPatchSets) {
+ if (parseInt(patchNum, 10) ===
+ this._computeLatestPatchNum(allPatchSets)) {
+ return '';
+ }
+ return 'patchInfo--oldPatchSet';
+ },
+
_computeAllPatchSets: function(change) {
var patchNums = [];
for (var rev in change.revisions) {
@@ -587,7 +601,6 @@
var reloadPatchNumDependentResources = function() {
return Promise.all([
this._getCommitInfo(),
- this.$.actions.reload(),
this.$.fileList.reload(),
]);
}.bind(this);
@@ -596,6 +609,7 @@
return Promise.all([
this._getLatestCommitMessage(),
+ this.$.actions.reload(),
this.$.relatedChanges.reload(),
this._getProjectConfig(),
]);
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 7f9fb02..2cdeab4 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -165,6 +165,36 @@
assert.deepEqual(element._diffDrafts, {});
});
+ test('change num change', function() {
+ element._changeNum = null;
+ element._patchRange = {
+ basePatchNum: 'PARENT',
+ patchNum: 2,
+ };
+ element._change = {
+ change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
+ labels: {},
+ };
+ element.viewState.changeNum = null;
+ element.viewState.diffMode = 'UNIFIED';
+ flushAsynchronousOperations();
+ assert.equal(element.viewState.diffMode, 'UNIFIED');
+
+ element._changeNum = '1';
+ element.params = {changeNum: '1'};
+ element._change.newProp = '1';
+ flushAsynchronousOperations();
+ assert.equal(element.viewState.diffMode, 'UNIFIED');
+ assert.equal(element.viewState.changeNum, '1');
+
+ element._changeNum = '2';
+ element.params = {changeNum: '2'};
+ element._change.newProp = '2';
+ flushAsynchronousOperations();
+ assert.isNull(element.viewState.diffMode);
+ assert.equal(element.viewState.changeNum, '2');
+ });
+
test('patch num change', function(done) {
element._changeNum = '42';
element._patchRange = {
@@ -183,7 +213,9 @@
status: 'NEW',
labels: {},
};
+ element.viewState.diffMode = 'UNIFIED';
flushAsynchronousOperations();
+
var selectEl = element.$$('.patchInfo-header select');
assert.ok(selectEl);
var optionEls = Polymer.dom(element.root).querySelectorAll(
@@ -201,6 +233,7 @@
var numEvents = 0;
selectEl.addEventListener('change', function(e) {
+ assert.equal(element.viewState.diffMode, 'UNIFIED');
numEvents++;
if (numEvents == 1) {
assert(showStub.lastCall.calledWithExactly('/c/42/1'),
@@ -364,5 +397,14 @@
assert(openSpy.lastCall.calledWithExactly(FocusTarget.CCS),
'_openReplyDialog should have been passed CCS');
});
+
+ test('class is applied to file list on old patch set', function() {
+ var allPatcheSets = [1, 2, 4];
+ assert.equal(element._computePatchInfoClass('1', allPatcheSets),
+ 'patchInfo--oldPatchSet');
+ assert.equal(element._computePatchInfoClass('2', allPatcheSets),
+ 'patchInfo--oldPatchSet');
+ assert.equal(element._computePatchInfoClass('4', allPatcheSets), '');
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.html b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.html
new file mode 100644
index 0000000..5cd65fa
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.html
@@ -0,0 +1,35 @@
+<!--
+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.
+-->
+
+<link rel="import" href="../../../bower_components/polymer/polymer.html">
+
+<dom-module id="gr-commit-info">
+ <template>
+ <style>
+ :host {
+ display: inline-block;
+ }
+ </style>
+ <template is="dom-if" if="[[_showWebLink]]">
+ <a target="_blank"
+ href$="[[_webLink]]">[[_computeShortHash(commitInfo)]]</a>
+ </template>
+ <template is="dom-if" if="[[!_showWebLink]]">
+ [[_computeShortHash(commitInfo)]]
+ </template>
+ </template>
+ <script src="gr-commit-info.js"></script>
+</dom-module>
diff --git a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.js b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.js
new file mode 100644
index 0000000..5aa8601
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.js
@@ -0,0 +1,98 @@
+// 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.
+(function() {
+ 'use strict';
+
+ Polymer({
+ is: 'gr-commit-info',
+
+ properties: {
+ change: Object,
+ commitInfo: Object,
+ serverConfig: Object,
+ _showWebLink: {
+ type: Boolean,
+ computed: '_computeShowWebLink(change, commitInfo, serverConfig)',
+ },
+ _webLink: {
+ type: String,
+ computed: '_computeWebLink(change, commitInfo, serverConfig)',
+ },
+ },
+
+ _isWebLink: function(link) {
+ // This is a whitelist of web link types that provide direct links to
+ // the commit in the url property.
+ return link.name === 'gitiles' || link.name === 'gitweb';
+ },
+
+ _computeShowWebLink: function(change, commitInfo, serverConfig) {
+ if (serverConfig.gitweb && serverConfig.gitweb.url &&
+ serverConfig.gitweb.type && serverConfig.gitweb.type.revision) {
+ return true;
+ }
+
+ if (!commitInfo.web_links) {
+ return false;
+ }
+
+ for (var i = 0; i < commitInfo.web_links.length; i++) {
+ if (this._isWebLink(commitInfo.web_links[i])) {
+ return true;
+ }
+ }
+
+ return false;
+ },
+
+ _computeWebLink: function(change, commitInfo, serverConfig) {
+ if (!this._computeShowWebLink(change, commitInfo, serverConfig)) {
+ return;
+ }
+
+ if (serverConfig.gitweb && serverConfig.gitweb.url &&
+ serverConfig.gitweb.type && serverConfig.gitweb.type.revision) {
+ return serverConfig.gitweb.url +
+ serverConfig.gitweb.type.revision
+ .replace('${project}', change.project)
+ .replace('${commit}', commitInfo.commit);
+ }
+
+ var webLink = null;
+ for (var i = 0; i < commitInfo.web_links.length; i++) {
+ if (this._isWebLink(commitInfo.web_links[i])) {
+ webLink = commitInfo.web_links[i].url;
+ break;
+ }
+ }
+
+ if (!webLink) {
+ return;
+ }
+
+ if (!/^https?\:\/\//.test(webLink)) {
+ webLink = '../../' + webLink;
+ }
+
+ return webLink;
+ },
+
+ _computeShortHash: function(commitInfo) {
+ if (!commitInfo || !commitInfo.commit) {
+ return;
+ }
+ return commitInfo.commit.slice(0, 7);
+ },
+ });
+})();
diff --git a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html
new file mode 100644
index 0000000..36b1628
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html
@@ -0,0 +1,145 @@
+<!DOCTYPE html>
+<!--
+Copyright (C) 2015 The Android Open Source Project
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-commit-info</title>
+
+<script src="../../../bower_components/webcomponentsjs/webcomponents.min.js"></script>
+<script src="../../../bower_components/web-component-tester/browser.js"></script>
+
+<link rel="import" href="../../../bower_components/iron-test-helpers/iron-test-helpers.html">
+<link rel="import" href="gr-commit-info.html">
+
+<test-fixture id="basic">
+ <template>
+ <gr-commit-info></gr-commit-info>
+ </template>
+</test-fixture>
+
+<script>
+ suite('gr-commit-info tests', function() {
+ var element;
+
+ setup(function() {
+ element = fixture('basic');
+ });
+
+ test('no web link when unavailable', function() {
+ element.commitInfo = {};
+ element.serverConfig = {};
+ element.change = {labels: []};
+
+ assert.isNotOk(element._computeShowWebLink(element.change,
+ element.commitInfo, element.serverConfig));
+ });
+
+ test('use web link when available', function() {
+ element.commitInfo = {web_links: [{name: 'gitweb', url: 'link-url'}]};
+ element.serverConfig = {};
+
+ assert.isOk(element._computeShowWebLink(element.change,
+ element.commitInfo, element.serverConfig));
+ assert.equal(element._computeWebLink(element.change, element.commitInfo,
+ element.serverConfig), '../../link-url');
+ });
+
+ test('does not relativize web links that begin with scheme', function() {
+ element.commitInfo = {
+ web_links: [{name: 'gitweb', url: 'https://link-url'}]
+ };
+ element.serverConfig = {};
+
+ assert.isOk(element._computeShowWebLink(element.change,
+ element.commitInfo, element.serverConfig));
+ assert.equal(element._computeWebLink(element.change, element.commitInfo,
+ element.serverConfig), 'https://link-url');
+ });
+
+ test('use gitweb when available', function() {
+ element.commitInfo = {commit: 'commit-sha'};
+ element.serverConfig = {gitweb: {
+ url: 'url-base/',
+ type: {revision: 'xx ${project} xx ${commit} xx'},
+ }};
+ element.change = {
+ project: 'project-name',
+ labels: [],
+ current_revision: element.commitInfo.commit
+ };
+
+ assert.isOk(element._computeShowWebLink(element.change,
+ element.commitInfo, element.serverConfig));
+
+ assert.equal(element._computeWebLink(element.change, element.commitInfo,
+ element.serverConfig), 'url-base/xx project-name xx commit-sha xx');
+ });
+
+ test('prefer gitweb when both are available', function() {
+ element.commitInfo = {
+ commit: 'commit-sha',
+ web_links: [{url: 'link-url'}]
+ };
+ element.serverConfig = {gitweb: {
+ url: 'url-base/',
+ type: {revision: 'xx ${project} xx ${commit} xx'},
+ }};
+ element.change = {
+ project: 'project-name',
+ labels: [],
+ current_revision: element.commitInfo.commit
+ };
+
+ assert.isOk(element._computeShowWebLink(element.change,
+ element.commitInfo, element.serverConfig));
+
+ var link = element._computeWebLink(element.change, element.commitInfo,
+ element.serverConfig);
+
+ assert.equal(link, 'url-base/xx project-name xx commit-sha xx');
+ assert.notEqual(link, '../../link-url');
+ });
+
+ test('ignore web links that are neither gitweb nor gitiles', function() {
+ element.commitInfo = {
+ commit: 'commit-sha',
+ web_links: [
+ {
+ name: 'ignore',
+ url: 'ignore',
+ },
+ {
+ name: 'gitiles',
+ url: 'https://link-url',
+ }
+ ],
+ };
+ element.serverConfig = {};
+
+ assert.isOk(element._computeShowWebLink(element.change,
+ element.commitInfo, element.serverConfig));
+ assert.equal(element._computeWebLink(element.change, element.commitInfo,
+ element.serverConfig), 'https://link-url');
+
+ // Remove gitiles link.
+ element.commitInfo.web_links.splice(1, 1);
+ assert.isNotOk(element._computeShowWebLink(element.change,
+ element.commitInfo, element.serverConfig));
+ assert.isNotOk(element._computeWebLink(element.change, element.commitInfo,
+ element.serverConfig));
+ });
+ });
+</script>
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.js
index 97342d1..f27e4e2 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.js
@@ -32,16 +32,6 @@
properties: {
branch: String,
message: String,
- commitInfo: {
- type: Object,
- readOnly: true,
- observer: '_commitInfoChanged',
- },
- },
-
- _commitInfoChanged: function(commitInfo) {
- // Pre-populate cherry-pick message for editing from commit info.
- this.message = commitInfo.message;
},
_handleConfirmTap: function(e) {
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
index b4baa26..f38ef74 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
@@ -30,14 +30,12 @@
*/
properties: {
- branch: String,
message: String,
- commitInfo: Object,
},
- populateRevertMessage: function() {
+ populateRevertMessage: function(message) {
// Figure out what the revert title should be.
- var originalTitle = this.commitInfo.message.split('\n')[0];
+ var originalTitle = message.split('\n')[0];
var revertTitle = 'Revert of ' + originalTitle;
if (originalTitle.startsWith('Revert of ')) {
revertTitle = 'Reland of ' +
@@ -47,7 +45,7 @@
originalTitle.substring('Reland of '.length);
}
// Add '> ' in front of the original commit text.
- var originalCommitText = this.commitInfo.message.replace(/^/gm, '> ');
+ var originalCommitText = message.replace(/^/gm, '> ');
this.message = revertTitle + '\n\n' +
'Reason for revert: <INSERT REASONING HERE>\n\n' +
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html
index 1d53eef..3b3851a 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html
@@ -40,9 +40,7 @@
test('single line', function() {
assert.isNotOk(element.message);
- element.commitInfo = {message: 'one line commit'};
- assert.isNotOk(element.message);
- element.populateRevertMessage();
+ element.populateRevertMessage('one line commit');
var expected = 'Revert of one line commit\n\n' +
'Reason for revert: <INSERT REASONING HERE>\n\n' +
'Original issue\'s description:\n' +
@@ -52,9 +50,7 @@
test('multi line', function() {
assert.isNotOk(element.message);
- element.commitInfo = {message: 'many lines\ncommit\n\nmessage\n'};
- assert.isNotOk(element.message);
- element.populateRevertMessage();
+ element.populateRevertMessage('many lines\ncommit\n\nmessage\n');
var expected = 'Revert of many lines\n\n' +
'Reason for revert: <INSERT REASONING HERE>\n\n' +
'Original issue\'s description:\n' +
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
index cec1e90..1432d8c 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
@@ -97,9 +97,6 @@
border: none;
width: 100%;
}
- .labelsNotShown {
- color: #666;
- }
.labelContainer:not(:first-of-type) {
margin-top: .5em;
}
@@ -211,28 +208,20 @@
</iron-autogrow-textarea>
</section>
<section class="labelsContainer">
- <template is="dom-if" if="[[_computeShowLabels(patchNum, revisions)]]">
- <template is="dom-repeat"
- items="[[_computeLabelArray(permittedLabels)]]" as="label">
- <div class="labelContainer">
- <span class="labelName">[[label]]</span>
- <iron-selector data-label$="[[label]]"
- selected="[[_computeIndexOfLabelValue(labels, permittedLabels, label, _account)]]">
- <template is="dom-repeat"
- items="[[_computePermittedLabelValues(permittedLabels, label)]]"
- as="value">
- <gr-button has-tooltip data-value$="[[value]]"
- title$="[[_computeLabelValueTitle(labels, label, value)]]">[[value]]</gr-button>
- </template>
- </iron-selector>
- </div>
- </template>
- </template>
- <template is="dom-if" if="[[!_computeShowLabels(patchNum, revisions)]]">
- <span class="labelsNotShown">
- Labels are not shown because this is not the most recent patch set.
- <a href$="/c/[[change._number]]">Go to the latest patch set.</a>
- </span>
+ <template is="dom-repeat"
+ items="[[_computeLabelArray(permittedLabels)]]" as="label">
+ <div class="labelContainer">
+ <span class="labelName">[[label]]</span>
+ <iron-selector data-label$="[[label]]"
+ selected="[[_computeIndexOfLabelValue(labels, permittedLabels, label, _account)]]">
+ <template is="dom-repeat"
+ items="[[_computePermittedLabelValues(permittedLabels, label)]]"
+ as="value">
+ <gr-button has-tooltip data-value$="[[value]]"
+ title$="[[_computeLabelValueTitle(labels, label, value)]]">[[value]]</gr-button>
+ </template>
+ </iron-selector>
+ </div>
</template>
</section>
<section class="draftsContainer" hidden$="[[_computeHideDraftList(diffDrafts)]]">
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index d2b279d..963c8fa 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -48,7 +48,6 @@
properties: {
change: Object,
patchNum: String,
- revisions: Object,
disabled: {
type: Boolean,
value: false,
@@ -152,10 +151,6 @@
if (!this.permittedLabels.hasOwnProperty(label)) { continue; }
var selectorEl = this.$$('iron-selector[data-label="' + label + '"]');
-
- // The selector may not be present if it’s not at the latest patch set.
- if (!selectorEl) { continue; }
-
var selectedVal = selectorEl.selectedItem.getAttribute('data-value');
selectedVal = parseInt(selectedVal, 10);
obj.labels[label] = selectedVal;
@@ -259,16 +254,6 @@
}.bind(this));
},
- _computeShowLabels: function(patchNum, revisions) {
- var num = parseInt(patchNum, 10);
- for (var rev in revisions) {
- if (revisions[rev]._number > num) {
- return false;
- }
- }
- return true;
- },
-
_computeHideDraftList: function(drafts) {
return Object.keys(drafts || {}).length == 0;
},
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
index 8fb4e45..639aeef 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
@@ -107,18 +107,7 @@
MockInteractions.tap(element.$$('.cancel'));
});
- test('show/hide labels', function() {
- var revisions = {
- rev1: {_number: 1},
- rev2: {_number: 2},
- };
- assert.isFalse(element._computeShowLabels('1', revisions));
- assert.isTrue(element._computeShowLabels('2', revisions));
- });
-
test('label picker', function(done) {
- var showLabelsStub = sinon.stub(element, '_computeShowLabels',
- function() { return true; });
element.revisions = {};
element.patchNum = '';
@@ -156,7 +145,6 @@
'Element should be enabled when done sending reply.');
assert.equal(element.draft.length, 0);
saveReviewStub.restore();
- showLabelsStub.restore();
done();
});
@@ -312,7 +300,10 @@
assert.equal(body, 'first error, second error');
});
});
- element.send().then(done);
+
+ // Async tick is needed because iron-selector content is distributed and
+ // distributed content requires an observer to be set up.
+ flush(function() { element.send().then(done); });
});
test('ccs are displayed if NoteDb is enabled', function() {
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.html b/polygerrit-ui/app/elements/core/gr-router/gr-router.html
index 2971ed2..4ad2a37 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.html
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.html
@@ -15,6 +15,7 @@
-->
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
+<link rel="import" href="../gr-reporting/gr-reporting.html">
<script src="../../../bower_components/page/page.js"></script>
<script src="gr-router.js"></script>
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
index 8441372..c69e43f 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -17,10 +17,15 @@
// Polymer makes `app` intrinsically defined on the window by virtue of the
// custom element having the id "app", but it is made explicit here.
var app = document.querySelector('#app');
- var restAPI = document.createElement('gr-rest-api-interface');
- var reporting = document.createElement('gr-reporting');
+ if (!app) {
+ console.log('No gr-app found (running tests)');
+ return;
+ }
window.addEventListener('WebComponentsReady', function() {
+ var restAPI = document.createElement('gr-rest-api-interface');
+ var reporting = document.createElement('gr-reporting');
+
reporting.timeEnd('WebComponentsReady');
reporting.pageLoaded();
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 18c0602..40a90b7 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
@@ -77,6 +77,7 @@
_builder: Object,
_groups: Array,
_layers: Array,
+ _showTabs: Boolean,
},
get diffElement() {
@@ -92,6 +93,7 @@
this._layers = [
this.$.syntaxLayer,
this._createIntralineLayer(),
+ this._createTabIndicatorLayer(),
this.$.rangeLayer,
];
@@ -102,6 +104,7 @@
render: function(comments, prefs) {
this.$.syntaxLayer.enabled = prefs.syntax_highlighting;
+ this._showTabs = !!prefs.show_tabs;
// Stop the processor (if it's running).
this.$.processor.cancel();
@@ -330,6 +333,31 @@
};
},
+ _createTabIndicatorLayer: function() {
+ var show = (function() { return this._showTabs; }).bind(this);
+ return {
+ addListener: function() {},
+ annotate: function(el, line) {
+ // If visible tabs are disabled, do nothing.
+ if (!show()) { return; }
+
+ // Find and annotate the locations of tabs.
+ var split = line.text.split('\t');
+ if (!split) { return; }
+ for (var i = 0, pos = 0; i < split.length - 1; i++) {
+ // Skip forward by the length of the content
+ pos += split[i].length;
+
+ GrAnnotation.annotateElement(el, pos, 1,
+ 'style-scope gr-diff tab-indicator');
+
+ // Skip forward by one tab character.
+ pos++;
+ }
+ },
+ };
+ },
+
/**
* In pages with large diffs, creating the first comment thread can be
* slow because nested Polymer elements (particularly
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index 2090e98..670885a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -493,7 +493,7 @@
for (var i = 0; i < split.length - 1; i++) {
offset += split[i].length;
width = tabSize - (offset % tabSize);
- result += split[i] + this._getTabWrapper(width, this._prefs.show_tabs);
+ result += split[i] + this._getTabWrapper(width);
offset += width;
}
if (split.length) {
@@ -503,7 +503,7 @@
return result;
};
- GrDiffBuilder.prototype._getTabWrapper = function(tabSize, showTabs) {
+ GrDiffBuilder.prototype._getTabWrapper = function(tabSize) {
// Force this to be a number to prevent arbitrary injection.
tabSize = +tabSize;
if (isNaN(tabSize)) {
@@ -511,9 +511,6 @@
}
var str = '<span class="style-scope gr-diff tab ';
- if (showTabs) {
- str += 'withIndicator';
- }
str += '" style="';
// TODO(andybons): CSS tab-size is not supported in IE.
str += 'tab-size:' + tabSize + ';';
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index 187a5cd..af44629 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
@@ -414,6 +414,117 @@
});
});
+ suite('tab indicators', function() {
+ var sandbox;
+ var element;
+ var layer;
+
+ setup(function() {
+ sandbox = sinon.sandbox.create();
+ element = fixture('basic');
+ element._showTabs = true;
+ layer = element._createTabIndicatorLayer();
+ });
+
+ teardown(function() {
+ sandbox.restore();
+ });
+
+ test('does nothing with empty line', function() {
+ var line = {text: ''};
+ var el = document.createElement('div');
+ var annotateElementStub = sandbox.stub(GrAnnotation, 'annotateElement');
+
+ layer.annotate(el, line);
+
+ assert.isFalse(annotateElementStub.called);
+ });
+
+ test('does nothing with no tabs', function() {
+ var str = 'lorem ipsum no tabs';
+ var line = {text: str};
+ var el = document.createElement('div');
+ el.textContent = str;
+ var annotateElementStub = sandbox.stub(GrAnnotation, 'annotateElement');
+
+ layer.annotate(el, line);
+
+ assert.isFalse(annotateElementStub.called);
+ });
+
+ test('annotates tab at beginning', function() {
+ var str = '\tlorem upsum';
+ var line = {text: str};
+ var el = document.createElement('div');
+ el.textContent = str;
+ var annotateElementStub = sandbox.stub(GrAnnotation, 'annotateElement');
+
+ layer.annotate(el, line);
+
+ assert.equal(annotateElementStub.callCount, 1);
+ var args = annotateElementStub.getCalls()[0].args;
+ assert.equal(args[0], el);
+ assert.equal(args[1], 0, 'offset of tab indicator');
+ assert.equal(args[2], 1, 'length of tab indicator');
+ assert.include(args[3], 'tab-indicator');
+ });
+
+ test('does not annotate when disabled', function() {
+ element._showTabs = false;
+
+ var str = '\tlorem upsum';
+ var line = {text: str};
+ var el = document.createElement('div');
+ el.textContent = str;
+ var annotateElementStub = sandbox.stub(GrAnnotation, 'annotateElement');
+
+ layer.annotate(el, line);
+
+ assert.isFalse(annotateElementStub.called);
+ });
+
+ test('annotates multiple in beginning', function() {
+ var str = '\t\tlorem upsum';
+ var line = {text: str};
+ var el = document.createElement('div');
+ el.textContent = str;
+ var annotateElementStub = sandbox.stub(GrAnnotation, 'annotateElement');
+
+ layer.annotate(el, line);
+
+ assert.equal(annotateElementStub.callCount, 2);
+
+ var args = annotateElementStub.getCalls()[0].args;
+ assert.equal(args[0], el);
+ assert.equal(args[1], 0, 'offset of tab indicator');
+ assert.equal(args[2], 1, 'length of tab indicator');
+ assert.include(args[3], 'tab-indicator');
+
+ args = annotateElementStub.getCalls()[1].args;
+ assert.equal(args[0], el);
+ assert.equal(args[1], 1, 'offset of tab indicator');
+ assert.equal(args[2], 1, 'length of tab indicator');
+ assert.include(args[3], 'tab-indicator');
+ });
+
+ test('annotates intermediate tabs', function() {
+ var str = 'lorem\tupsum';
+ var line = {text: str};
+ var el = document.createElement('div');
+ el.textContent = str;
+ var annotateElementStub = sandbox.stub(GrAnnotation, 'annotateElement');
+
+ layer.annotate(el, line);
+
+ assert.equal(annotateElementStub.callCount, 1);
+ var args = annotateElementStub.getCalls()[0].args;
+ assert.equal(args[0], el);
+ assert.equal(args[1], 5, 'offset of tab indicator');
+ assert.equal(args[2], 1, 'length of tab indicator');
+ assert.include(args[3], 'tab-indicator');
+ });
+ });
+
suite('rendering', function() {
var content;
var outputEl;
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 d6a3bc0..7111ee5 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
@@ -105,6 +105,15 @@
}
}.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 (this._path) {
this.fire('title-change',
{title: this._computeFileDisplayName(this._path)});
@@ -113,11 +122,6 @@
this.$.cursor.push('diffs', this.$.diff);
},
- detached: function() {
- // Reset the diff mode to null so that it reverts to the user preference.
- this.changeViewState.diffMode = null;
- },
-
_getLoggedIn: function() {
return this.$.restAPI.getLoggedIn();
},
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 0a4d6b6..c21bd90 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
@@ -32,6 +32,12 @@
</template>
</test-fixture>
+<test-fixture id="blank">
+ <template>
+ <div></div>
+ </template>
+</text-fixture>
+
<script>
suite('gr-diff-view tests', function() {
var element;
@@ -394,6 +400,29 @@
assert.equal(element._getDiffViewMode(), diffDisplay.viewMode);
});
+ test('diff mode selector initializes from preferences', 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');
+ 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, 'SIDE_BY_SIDE');
+
+ // Receive the overriding preference.
+ resolvePrefs({diff_view: 'UNIFIED'});
+ flushAsynchronousOperations();
+ assert.equal(select.value, 'SIDE_BY_SIDE');
+ });
+
test('_loadHash', function() {
assert.isNotOk(element.$.cursor.initialLineNumber);
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 46612a0..2431fb0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -152,11 +152,11 @@
}
.tab {
display: inline-block;
- position: relative;
}
- .tab.withIndicator {
- color: #D68E47;
- text-decoration: line-through;
+ .tab-indicator:before {
+ color: #C62828;
+ /* >> character */
+ content: '\00BB';
}
</style>
<style include="gr-theme-default"></style>
diff --git a/polygerrit-ui/app/elements/gr-app_test.html b/polygerrit-ui/app/elements/gr-app_test.html
index ca26ed0..0c23c4f 100644
--- a/polygerrit-ui/app/elements/gr-app_test.html
+++ b/polygerrit-ui/app/elements/gr-app_test.html
@@ -25,7 +25,7 @@
<test-fixture id="basic">
<template>
- <gr-app></gr-app>
+ <gr-app id="app"></gr-app>
</template>
</test-fixture>
diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button.html b/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
index cc0da66..164bb2d 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
@@ -120,13 +120,14 @@
:host([secondary]:active) {
border-color: #941c0c;
}
- :host([primary][loading]),
- :host([primary][disabled]) {
+ :host([primary][loading]) {
background-color: #7caeff;
border-color: transparent;
color: #fff;
}
-
+ :host([primary][disabled]) {
+ background-color: #888;
+ }
</style>
<content></content>
</template>
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 2714f48..a608cae 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -25,15 +25,15 @@
var basePath = '../elements/';
[
- 'change-list/gr-change-list-item/gr-change-list-item_test.html',
- 'change-list/gr-change-list/gr-change-list_test.html',
'change/gr-account-entry/gr-account-entry_test.html',
'change/gr-account-list/gr-account-list_test.html',
'change/gr-change-actions/gr-change-actions_test.html',
'change/gr-change-metadata/gr-change-metadata_test.html',
'change/gr-change-view/gr-change-view_test.html',
'change/gr-comment-list/gr-comment-list_test.html',
+ 'change/gr-commit-info/gr-commit-info_test.html',
'change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.html',
+ 'change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html',
'change/gr-download-dialog/gr-download-dialog_test.html',
'change/gr-file-list/gr-file-list_test.html',
'change/gr-message/gr-message_test.html',
@@ -41,13 +41,18 @@
'change/gr-related-changes-list/gr-related-changes-list_test.html',
'change/gr-reply-dialog/gr-reply-dialog_test.html',
'change/gr-reviewer-list/gr-reviewer-list_test.html',
+ 'change-list/gr-change-list/gr-change-list_test.html',
+ 'change-list/gr-change-list-item/gr-change-list-item_test.html',
'core/gr-account-dropdown/gr-account-dropdown_test.html',
'core/gr-error-manager/gr-error-manager_test.html',
'core/gr-main-header/gr-main-header_test.html',
+ 'core/gr-reporting/gr-reporting_test.html',
'core/gr-search-bar/gr-search-bar_test.html',
+ 'diff/gr-diff/gr-diff-group_test.html',
+ 'diff/gr-diff/gr-diff_test.html',
'diff/gr-diff-builder/gr-diff-builder_test.html',
- 'diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html',
'diff/gr-diff-comment/gr-diff-comment_test.html',
+ 'diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html',
'diff/gr-diff-cursor/gr-diff-cursor_test.html',
'diff/gr-diff-highlight/gr-annotation_test.html',
'diff/gr-diff-highlight/gr-diff-highlight_test.html',
@@ -55,8 +60,6 @@
'diff/gr-diff-processor/gr-diff-processor_test.html',
'diff/gr-diff-selection/gr-diff-selection_test.html',
'diff/gr-diff-view/gr-diff-view_test.html',
- 'diff/gr-diff/gr-diff-group_test.html',
- 'diff/gr-diff/gr-diff_test.html',
'diff/gr-patch-range-select/gr-patch-range-select_test.html',
'diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.html',
'diff/gr-selection-action-box/gr-selection-action-box_test.html',
