Merge "Add check before requesting conflicts in related change view"
diff --git a/.buckversion b/.buckversion
index af38772..560aff2 100644
--- a/.buckversion
+++ b/.buckversion
@@ -1 +1 @@
-7b7817c48f30687781040b2b82ac9218d5c4eaa4
+d6949e1440ef2048d697c637a4adae1b509bf72d
diff --git a/BUILD b/BUILD
index a6d5067..c914fb7 100644
--- a/BUILD
+++ b/BUILD
@@ -1,3 +1,4 @@
+package(default_visibility = ['//visibility:public'])
load('//tools/bzl:pkg_war.bzl', 'pkg_war')
genrule(
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 462c226..564d298 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -3419,6 +3419,67 @@
+
Default is 1.
+[[receiveemail]]
+=== Section receiveemail
+
+[[receiveemail.protocol]]receiveemail.protocol::
++
+Specifies the protocol used for receiving emails. Valid options are
+'POP3', 'IMAP' and 'NONE'. Note that Gerrit will automatically switch between
+POP3 and POP3s as well as IMAP and IMAPS depending on the specified
+link:#receiveemail.encryption[encryption].
++
+Defaults to 'NONE' which means that receiving emails is disabled.
+
+[[receiveemail.host]]receiveemail.host::
++
+The hostname of the mailserver. Example: 'imap.gmail.com'.
++
+Defaults to an empty string which means that receiving emails is disabled.
+
+[[receiveemail.port]]receiveemail.port::
++
+The port the email server exposes for receving emails.
++
+Defaults to the industry standard for a given protocol and encryption:
+POP3: 110; POP3S: 995; IMAP: 143; IMAPS: 995.
+
+[[receiveemail.username]]receiveemail.username::
++
+Username used for authenticating with the email server.
++
+Defaults to an empty string.
+
+[[receiveemail.password]]receiveemail.password::
++
+Password used for authenticating with the email server.
++
+Defaults to an empty string.
+
+[[receiveemail.encryption]]receiveemail.encryption::
++
+Encryption standard used for transport layer security between Gerrit and the
+email server. Possible values include 'NONE', 'SSL' and 'TLS'.
++
+Defaults to 'NONE'.
+
+[[receiveemail.fetchInterval]]receiveemail.fetchInterval::
++
+Time between two consecutive fetches from the email server. Communication with
+the email server is not kept alive. Examples: 60s, 10m, 1h.
++
+Defaults to 60 seconds.
+
+[[receiveemail.enableImapIdle]]receiveemail.enableImapIdle::
++
+If the IMAP protocol is used for retrieving emails, IMAPv4 IDLE can be used to
+keep the connection with the email server alive and receive a push when a new
+email is delivered to the inbox. In this case, Gerrit will process the email
+immediately and will not have a fetch delay.
+
++
+Defaults to false.
+
[[sendemail]]
=== Section sendemail
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt
index 5ca6353..77370df 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -2,12 +2,10 @@
Bazel build is experimental. Major missing parts:
-* PolyGerrit
* License tracking
* Version stamping
* Custom plugins
* Eclipse project generation.
-* Publishing to maven.
* Test suites for SSH, acceptance, etc.
* tag tests as slow, flaky, etc.
@@ -81,7 +79,17 @@
bazel-bin/gerrit-extension-api/extension-api_deploy.jar
----
-TODO - fix and document deployment to maven
+Install {extension,plugin,gwt}-api to the local maven repository:
+
+----
+ tools/maven/api.sh install bazel
+----
+
+Install gerrit.war to the local maven repository:
+
+----
+ tools/maven/api.sh war_install bazel
+----
=== Plugins
diff --git a/Documentation/dev-release.txt b/Documentation/dev-release.txt
index 8741d80..023841c 100644
--- a/Documentation/dev-release.txt
+++ b/Documentation/dev-release.txt
@@ -154,7 +154,7 @@
----
buck clean
buck build --no-cache release docs
- ./tools/maven/api.sh install
+ ./tools/maven/api.sh install <buck|bazel>
----
* Sanity check WAR
@@ -186,13 +186,13 @@
* Push the WAR to Maven Central:
+
----
- ./tools/maven/api.sh war_deploy
+ ./tools/maven/api.sh war_deploy <buck|bazel>
----
* Push the plugin artifacts to Maven Central:
+
----
- ./tools/maven/api.sh deploy
+ ./tools/maven/api.sh deploy <buck|bazel>
----
+
If no artifacts are uploaded, clean the `buck-out` folder and retry:
diff --git a/WORKSPACE b/WORKSPACE
index 3c8d851..cd19c74 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -156,7 +156,7 @@
)
maven_jar(
- name = 'ewah',
+ name = 'javaewah',
artifact = 'com.googlecode.javaewah:JavaEWAH:0.7.9',
sha1 = 'eceaf316a8faf0e794296ebe158ae110c7d72a5a',
)
@@ -381,6 +381,36 @@
sha1 = '2e35862b0435c1b027a21f3d6eecbe50e6e08d54',
)
+GREENMAIL_VERS = '1.5.2'
+
+maven_jar(
+ name = 'greenmail',
+ artifact = 'com.icegreen:greenmail:' + GREENMAIL_VERS,
+ sha1 = '6b4862a09f8642da58c109117b24ccc19a4a6d39',
+)
+
+MAIL_VERS = '1.5.6'
+
+maven_jar(
+ name = 'mail',
+ artifact = 'com.sun.mail:javax.mail:' + MAIL_VERS,
+ sha1 = 'ab5daef2f881c42c8e280cbe918ec4d7fdfd7efe',
+)
+
+MIME4J_VERS = '0.8.0'
+
+maven_jar(
+ name = 'mime4j_core',
+ artifact = 'org.apache.james:apache-mime4j-core:' + MIME4J_VERS,
+ sha1 = 'd54f45fca44a2f210569656b4ca3574b42911c95',
+)
+
+maven_jar(
+ name = 'mime4j_dom',
+ artifact = 'org.apache.james:apache-mime4j-dom:' + MIME4J_VERS,
+ sha1 = '6720c93d14225c3e12c4a69768a0370c80e376a3',
+)
+
OW2_VERS = '5.1'
maven_jar(
@@ -415,8 +445,8 @@
maven_jar(
name = 'auto_value',
- artifact = 'com.google.auto.value:auto-value:1.3-rc1',
- sha1 = 'b764e0fb7e11353fbff493b22fd6e83bf091a179',
+ artifact = 'com.google.auto.value:auto-value:1.4-rc1',
+ sha1 = '9347939002003a7a3c3af48271fc2c18734528a4',
)
maven_jar(
diff --git a/gerrit-acceptance-framework/BUCK b/gerrit-acceptance-framework/BUCK
index 89f8ea3..dca71e6 100644
--- a/gerrit-acceptance-framework/BUCK
+++ b/gerrit-acceptance-framework/BUCK
@@ -56,10 +56,12 @@
'//lib:truth',
],
provided_deps = PROVIDED + [
+ '//lib/greenmail:greenmail',
'//lib:gwtorm',
'//lib/guice:guice',
'//lib/guice:guice-assistedinject',
'//lib/guice:guice-servlet',
+ '//lib/mail:mail',
],
visibility = ['PUBLIC'],
)
diff --git a/gerrit-acceptance-framework/BUILD b/gerrit-acceptance-framework/BUILD
index ed04efa..d01534a 100644
--- a/gerrit-acceptance-framework/BUILD
+++ b/gerrit-acceptance-framework/BUILD
@@ -49,10 +49,12 @@
'//lib:truth',
],
deps = PROVIDED + [ # We want these deps to be exported_deps
+ '//lib/greenmail:greenmail',
'//lib:gwtorm',
'//lib/guice:guice',
'//lib/guice:guice-assistedinject',
'//lib/guice:guice-servlet',
+ '//lib/mail:mail',
],
visibility = ['//visibility:public'],
)
@@ -64,4 +66,5 @@
title = 'Gerrit Acceptance Test Framework Documentation',
libs = [':lib'],
pkgs = ['com.google.gerrit.acceptance'],
+ visibility = ['//visibility:public'],
)
diff --git a/gerrit-acceptance-tests/BUCK b/gerrit-acceptance-tests/BUCK
index d5d0b0d..3f7e04f 100644
--- a/gerrit-acceptance-tests/BUCK
+++ b/gerrit-acceptance-tests/BUCK
@@ -29,11 +29,13 @@
'//lib/bouncycastle:bcpg',
'//lib/bouncycastle:bcprov',
+ '//lib/greenmail:greenmail',
'//lib/guice:guice',
'//lib/guice:guice-assistedinject',
'//lib/guice:guice-servlet',
'//lib/log:api',
'//lib/jgit/org.eclipse.jgit:jgit',
+ '//lib/mail:mail',
'//lib/mina:sshd',
],
visibility = [
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 479e34b..de907ca 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
@@ -28,6 +28,7 @@
import static com.google.gerrit.server.project.Util.category;
import static com.google.gerrit.server.project.Util.value;
import static java.util.concurrent.TimeUnit.SECONDS;
+import static java.util.stream.Collectors.toList;
import static org.junit.Assert.fail;
import com.google.common.collect.ImmutableList;
@@ -54,6 +55,7 @@
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ApprovalInfo;
@@ -1002,6 +1004,65 @@
}
@Test
+ public void implicitlyCcOnNonVotingReview() throws Exception {
+ PushOneCommit.Result r = createChange();
+ setApiUser(user);
+ gApi.changes()
+ .id(r.getChangeId())
+ .revision(r.getCommit().name())
+ .review(new ReviewInput());
+
+ ChangeInfo c = gApi.changes()
+ .id(r.getChangeId())
+ .get();
+ // If we're not reading from NoteDb, then the CCed user will be returned
+ // in the REVIEWER state.
+ ReviewerState state = notesMigration.readChanges() ? CC : REVIEWER;
+ assertThat(c.reviewers.get(state).stream().map(ai -> ai._accountId)
+ .collect(toList())).containsExactly(user.id.get());
+ }
+
+ @Test
+ public void implicitlyAddReviewerOnVotingReview() throws Exception {
+ PushOneCommit.Result r = createChange();
+ setApiUser(user);
+ gApi.changes()
+ .id(r.getChangeId())
+ .revision(r.getCommit().name())
+ .review(ReviewInput.recommend().message("LGTM"));
+
+ ChangeInfo c = gApi.changes()
+ .id(r.getChangeId())
+ .get();
+ assertThat(c.reviewers.get(REVIEWER).stream().map(ai -> ai._accountId)
+ .collect(toList())).containsExactly(user.id.get());
+
+ // Further test: remove the vote, then comment again. The user should be
+ // implicitly re-added to the ReviewerSet, as a CC if we're using NoteDb.
+ setApiUser(admin);
+ gApi.changes()
+ .id(r.getChangeId())
+ .reviewer(user.getId().toString())
+ .remove();
+ c = gApi.changes()
+ .id(r.getChangeId())
+ .get();
+ assertThat(c.reviewers.values()).isEmpty();
+
+ setApiUser(user);
+ gApi.changes()
+ .id(r.getChangeId())
+ .revision(r.getCommit().name())
+ .review(new ReviewInput().message("hi"));
+ c = gApi.changes()
+ .id(r.getChangeId())
+ .get();
+ ReviewerState state = notesMigration.readChanges() ? CC : REVIEWER;
+ assertThat(c.reviewers.get(state).stream().map(ai -> ai._accountId)
+ .collect(toList())).containsExactly(user.id.get());
+ }
+
+ @Test
public void addReviewerToClosedChange() throws Exception {
PushOneCommit.Result r = createChange();
gApi.changes()
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
index 1ef6337..c9cb06f 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
@@ -277,12 +277,19 @@
assertThat(result.labels).isNull();
assertThat(result.reviewers).isNull();
- // Verify user is not added as reviewer.
+ // Verify user is added to CC list.
ChangeInfo c = gApi.changes()
.id(r.getChangeId())
.get();
- assertReviewers(c, REVIEWER);
- assertReviewers(c, CC);
+ if (notesMigration.readChanges()) {
+ assertReviewers(c, REVIEWER);
+ assertReviewers(c, CC, user);
+ } else {
+ // If we aren't reading from NoteDb, the user will appear as a
+ // reviewer.
+ assertReviewers(c, REVIEWER, user);
+ assertReviewers(c, CC);
+ }
}
@Test
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/BUCK b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/BUCK
new file mode 100644
index 0000000..5c3cd07
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/BUCK
@@ -0,0 +1,7 @@
+include_defs('//gerrit-acceptance-tests/tests.defs')
+
+acceptance_tests(
+ group = 'server_mail',
+ srcs = glob(['*IT.java']),
+ labels = ['server'],
+)
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/BUILD b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/BUILD
new file mode 100644
index 0000000..77e6a0f
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/BUILD
@@ -0,0 +1,11 @@
+load("//gerrit-acceptance-tests:tests.bzl", "acceptance_tests")
+
+acceptance_tests(
+ srcs = glob(["*IT.java"]),
+ group = "server_mail",
+ labels = ["server"],
+ deps = [
+ "//lib/greenmail",
+ "//lib/mail",
+ ],
+)
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailIT.java
new file mode 100644
index 0000000..66a7f15
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailIT.java
@@ -0,0 +1,104 @@
+// 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.server.mail;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.server.mail.receive.MailReceiver;
+import com.google.gerrit.testutil.ConfigSuite;
+import com.google.inject.Inject;
+
+import com.icegreen.greenmail.junit.GreenMailRule;
+import com.icegreen.greenmail.user.GreenMailUser;
+import com.icegreen.greenmail.util.GreenMail;
+import com.icegreen.greenmail.util.GreenMailUtil;
+import com.icegreen.greenmail.util.ServerSetupTest;
+
+import org.eclipse.jgit.lib.Config;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import javax.mail.internet.MimeMessage;
+
+@NoHttpd
+@RunWith(ConfigSuite.class)
+public class MailIT extends AbstractDaemonTest {
+ private final static String RECEIVEEMAIL = "receiveemail";
+ private final static String HOST = "localhost";
+ private final static String USERNAME = "user@domain.com";
+ private final static String PASSWORD = "password";
+
+ @Inject
+ private MailReceiver mailReceiver;
+
+ @Inject
+ private GreenMail greenMail;
+
+ @Rule
+ public final GreenMailRule mockPop3Server = new GreenMailRule(
+ ServerSetupTest.SMTP_POP3_IMAP);
+
+ @ConfigSuite.Default
+ public static Config pop3Config() {
+ Config cfg = new Config();
+ cfg.setString(RECEIVEEMAIL, null, "host", HOST);
+ cfg.setString(RECEIVEEMAIL, null, "port", "3110");
+ cfg.setString(RECEIVEEMAIL, null, "username", USERNAME);
+ cfg.setString(RECEIVEEMAIL, null, "password", PASSWORD);
+ cfg.setString(RECEIVEEMAIL, null, "protocol", "POP3");
+ cfg.setString(RECEIVEEMAIL, null, "fetchInterval", "99");
+ return cfg;
+ }
+
+ @ConfigSuite.Config
+ public static Config imapConfig() {
+ Config cfg = new Config();
+ cfg.setString(RECEIVEEMAIL, null, "host", HOST);
+ cfg.setString(RECEIVEEMAIL, null, "port", "3143");
+ cfg.setString(RECEIVEEMAIL, null, "username", USERNAME);
+ cfg.setString(RECEIVEEMAIL, null, "password", PASSWORD);
+ cfg.setString(RECEIVEEMAIL, null, "protocol", "IMAP");
+ cfg.setString(RECEIVEEMAIL, null, "fetchInterval", "99");
+ return cfg;
+ }
+
+ @Test
+ public void testDelete() throws Exception {
+ GreenMailUser user = mockPop3Server.setUser(USERNAME, USERNAME, PASSWORD);
+ user.deliver(createSimpleMessage());
+ assertThat(mockPop3Server.getReceivedMessages().length).isEqualTo(1);
+ // Let Gerrit handle emails
+ mailReceiver.handleEmails();
+ // Check that the message is still present
+ assertThat(mockPop3Server.getReceivedMessages().length).isEqualTo(1);
+ // Mark the message for deletion
+ mailReceiver.requestDeletion(
+ mockPop3Server.getReceivedMessages()[0].getMessageID());
+ // Let Gerrit handle emails
+ mailReceiver.handleEmails();
+ // Check that the message was deleted
+ assertThat(mockPop3Server.getReceivedMessages().length).isEqualTo(0);
+ }
+
+ private MimeMessage createSimpleMessage() {
+ return GreenMailUtil
+ .createTextEmail(USERNAME, "from@localhost.com", "subject",
+ "body",
+ greenMail.getImap().getServerSetup());
+ }
+}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
index 29375fe..e1771ce 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
@@ -20,6 +20,7 @@
import static com.google.gerrit.reviewdb.client.RefNames.refsDraftComments;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.stream.Collectors.toList;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
import static org.junit.Assert.fail;
@@ -68,6 +69,7 @@
import com.google.gerrit.server.notedb.ChangeBundleReader;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NoteDbChangeState;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NoteDbUpdateManager;
import com.google.gerrit.server.notedb.TestChangeRebuilderWrapper;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder.NoPatchSetsException;
@@ -99,6 +101,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.concurrent.TimeUnit;
public class ChangeRebuilderIT extends AbstractDaemonTest {
@@ -592,11 +595,15 @@
ReviewDb db = getUnwrappedDb();
Change c = db.changes().get(id);
// Leave change meta ID alone so DraftCommentNotes does the rebuild.
+ ObjectId badSha =
+ ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef");
NoteDbChangeState bogusState = new NoteDbChangeState(
- id, NoteDbChangeState.parse(c).getChangeMetaId(),
- ImmutableMap.<Account.Id, ObjectId>of(
- user.getId(),
- ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef")));
+ id,
+ PrimaryStorage.REVIEW_DB,
+ Optional.of(
+ NoteDbChangeState.RefState.create(
+ NoteDbChangeState.parse(c).getChangeMetaId(),
+ ImmutableMap.of(user.getId(), badSha))));
c.setNoteDbState(bogusState.toString());
db.changes().update(Collections.singleton(c));
@@ -1088,6 +1095,32 @@
checker.rebuildAndCheckChanges(id);
}
+ @Test
+ public void ignoreChangeMessageBeyondCurrentPatchSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+ PatchSet.Id psId1 = r.getPatchSetId();
+ Change.Id id = psId1.getParentKey();
+ gApi.changes().id(id.get()).current().review(ReviewInput.recommend());
+
+ r = amendChange(r.getChangeId());
+ PatchSet.Id psId2 = r.getPatchSetId();
+
+ assertThat(db.patchSets().byChange(id)).hasSize(2);
+ assertThat(db.changeMessages().byPatchSet(psId2)).hasSize(1);
+ db.patchSets().deleteKeys(Collections.singleton(psId2));
+
+ checker.rebuildAndCheckChanges(psId2.getParentKey());
+ setNotesMigration(true, true);
+
+ ChangeData cd = changeDataFactory.create(db, project, id);
+ assertThat(cd.change().currentPatchSetId()).isEqualTo(psId1);
+ assertThat(cd.patchSets().stream().map(ps -> ps.getId()).collect(toList()))
+ .containsExactly(psId1);
+ PatchSet ps = cd.currentPatchSet();
+ assertThat(ps).isNotNull();
+ assertThat(ps.getId()).isEqualTo(psId1);
+ }
+
private void assertChangesReadOnly(RestApiException e) throws Exception {
Throwable cause = e.getCause();
assertThat(cause).isInstanceOf(UpdateException.class);
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/NoteDbPrimaryIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/NoteDbPrimaryIT.java
new file mode 100644
index 0000000..ba04366
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/NoteDbPrimaryIT.java
@@ -0,0 +1,193 @@
+// 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.server.notedb;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.TruthJUnit.assume;
+import static com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage.REVIEW_DB;
+import static java.util.stream.Collectors.toList;
+
+import com.google.common.collect.Iterables;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.extensions.api.changes.DraftInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.client.ChangeStatus;
+import com.google.gerrit.extensions.common.ApprovalInfo;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.server.ReviewDbUtil;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.RepoRefCache;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.NoteDbChangeState;
+import com.google.gerrit.testutil.NoteDbMode;
+import com.google.inject.Inject;
+
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+
+public class NoteDbPrimaryIT extends AbstractDaemonTest {
+ @Inject
+ private AllUsersName allUsers;
+
+ @Before
+ public void setUp() throws Exception {
+ assume().that(NoteDbMode.get()).isEqualTo(NoteDbMode.READ_WRITE);
+ db = ReviewDbUtil.unwrapDb(db);
+ }
+
+ @Test
+ public void updateChange() throws Exception {
+ PushOneCommit.Result r = createChange();
+ Change.Id id = r.getChange().getId();
+ setNoteDbPrimary(id);
+
+ gApi.changes().id(id.get()).current().review(ReviewInput.approve());
+ gApi.changes().id(id.get()).current().submit();
+
+ ChangeInfo info = gApi.changes().id(id.get()).get();
+ assertThat(info.status).isEqualTo(ChangeStatus.MERGED);
+ ApprovalInfo approval =
+ Iterables.getOnlyElement(info.labels.get("Code-Review").all);
+ assertThat(approval._accountId).isEqualTo(admin.id.get());
+ assertThat(approval.value).isEqualTo(2);
+ assertThat(info.messages).hasSize(3);
+ assertThat(Iterables.getLast(info.messages).message)
+ .isEqualTo("Change has been successfully merged by " + admin.fullName);
+
+ ChangeNotes notes = notesFactory.create(db, project, id);
+ assertThat(notes.getChange().getStatus()).isEqualTo(Change.Status.MERGED);
+ assertThat(notes.getChange().getNoteDbState())
+ .isEqualTo(NoteDbChangeState.NOTE_DB_PRIMARY_STATE);
+
+ // Writes weren't reflected in ReviewDb.
+ assertThat(db.changes().get(id).getStatus()).isEqualTo(Change.Status.NEW);
+ assertThat(db.patchSetApprovals().byChange(id)).isEmpty();
+ assertThat(db.changeMessages().byChange(id)).hasSize(1);
+ }
+
+ @Test
+ public void deleteDraftComment() throws Exception {
+ PushOneCommit.Result r = createChange();
+ Change.Id id = r.getChange().getId();
+ setNoteDbPrimary(id);
+
+ DraftInput din = new DraftInput();
+ din.path = PushOneCommit.FILE_NAME;
+ din.line = 1;
+ din.message = "A comment";
+ gApi.changes().id(id.get()).current().createDraft(din);
+
+ CommentInfo di = Iterables.getOnlyElement(
+ gApi.changes().id(id.get()).current().drafts()
+ .get(PushOneCommit.FILE_NAME));
+ assertThat(di.message).isEqualTo(din.message);
+
+ assertThat(
+ db.patchComments().draftByChangeFileAuthor(id, din.path, admin.id))
+ .isEmpty();
+
+ gApi.changes().id(id.get()).current().draft(di.id).delete();
+ assertThat(gApi.changes().id(id.get()).current().drafts()).isEmpty();
+ }
+
+ @Test
+ public void deleteVote() throws Exception {
+ PushOneCommit.Result r = createChange();
+ Change.Id id = r.getChange().getId();
+ setNoteDbPrimary(id);
+
+ gApi.changes().id(id.get()).current().review(ReviewInput.approve());
+ List<ApprovalInfo> approvals =
+ gApi.changes().id(id.get()).get().labels.get("Code-Review").all;
+ assertThat(approvals).hasSize(1);
+ assertThat(approvals.get(0).value).isEqualTo(2);
+
+ gApi.changes().id(id.get()).reviewer(admin.id.toString())
+ .deleteVote("Code-Review");
+
+ approvals = gApi.changes().id(id.get()).get().labels.get("Code-Review").all;
+ assertThat(approvals).hasSize(1);
+ assertThat(approvals.get(0).value).isEqualTo(0);
+ }
+
+ @Test
+ public void deleteVoteViaReview() throws Exception {
+ PushOneCommit.Result r = createChange();
+ Change.Id id = r.getChange().getId();
+ setNoteDbPrimary(id);
+
+ gApi.changes().id(id.get()).current().review(ReviewInput.approve());
+ List<ApprovalInfo> approvals =
+ gApi.changes().id(id.get()).get().labels.get("Code-Review").all;
+ assertThat(approvals).hasSize(1);
+ assertThat(approvals.get(0).value).isEqualTo(2);
+
+ gApi.changes().id(id.get()).current().review(ReviewInput.noScore());
+
+ approvals = gApi.changes().id(id.get()).get().labels.get("Code-Review").all;
+ assertThat(approvals).hasSize(1);
+ assertThat(approvals.get(0).value).isEqualTo(0);
+ }
+
+ @Test
+ public void deleteReviewer() throws Exception {
+ PushOneCommit.Result r = createChange();
+ Change.Id id = r.getChange().getId();
+ setNoteDbPrimary(id);
+
+ gApi.changes().id(id.get()).addReviewer(user.id.toString());
+ assertThat(getReviewers(id)).containsExactly(user.id);
+ gApi.changes().id(id.get()).reviewer(user.id.toString()).remove();
+ assertThat(getReviewers(id)).isEmpty();
+ }
+
+ private void setNoteDbPrimary(Change.Id id) throws Exception {
+ Change c = db.changes().get(id);
+ assertThat(c).named("change " + id).isNotNull();
+ NoteDbChangeState state = NoteDbChangeState.parse(c);
+ assertThat(state.getPrimaryStorage())
+ .named("storage of " + id)
+ .isEqualTo(REVIEW_DB);
+
+ try (Repository changeRepo = repoManager.openRepository(c.getProject());
+ Repository allUsersRepo = repoManager.openRepository(allUsers)) {
+ assertThat(
+ state.isUpToDate(
+ new RepoRefCache(changeRepo), new RepoRefCache(allUsersRepo)))
+ .named("change " + id + " up to date")
+ .isTrue();
+ }
+
+ c.setNoteDbState(NoteDbChangeState.NOTE_DB_PRIMARY_STATE);
+ db.changes().update(Collections.singleton(c));
+ }
+
+ private List<Account.Id> getReviewers(Change.Id id) throws Exception {
+ return gApi.changes().id(id.get()).get()
+ .reviewers.values().stream()
+ .flatMap(Collection::stream)
+ .map(a -> new Account.Id(a._accountId))
+ .collect(toList());
+ }
+}
diff --git a/gerrit-acceptance-tests/tests.bzl b/gerrit-acceptance-tests/tests.bzl
index 160234f..cd1e7ec 100644
--- a/gerrit-acceptance-tests/tests.bzl
+++ b/gerrit-acceptance-tests/tests.bzl
@@ -7,16 +7,12 @@
def acceptance_tests(
group,
- srcs,
- flaky = 0,
deps = [],
labels = [],
vm_args = ['-Xmx256m'],
**kwargs):
junit_tests(
name = group,
- srcs = srcs,
- flaky = flaky,
deps = deps + BOUNCYCASTLE + [
'//gerrit-acceptance-tests:lib',
],
@@ -24,6 +20,7 @@
'acceptance',
'slow',
],
+ size = "medium",
jvm_flags = vm_args,
**kwargs
)
diff --git a/gerrit-common/BUILD b/gerrit-common/BUILD
index 9add6e7..72bd71b 100644
--- a/gerrit-common/BUILD
+++ b/gerrit-common/BUILD
@@ -68,11 +68,13 @@
],
)
-junit_tests(
- name = 'auto_value_tests',
- srcs = AUTO_VALUE_TEST_SRCS,
- deps = [
- '//lib:truth',
- '//lib/auto:auto-value',
- ],
-)
+# TODO(davido): Enable this test again when this bazel bug is fixed:
+# https://github.com/bazelbuild/bazel/issues/2044
+#junit_tests(
+# name = 'auto_value_tests',
+# srcs = AUTO_VALUE_TEST_SRCS,
+# deps = [
+# '//lib:truth',
+# '//lib/auto:auto-value',
+# ],
+#)
diff --git a/gerrit-elasticsearch/BUILD b/gerrit-elasticsearch/BUILD
index bb1f0e2..b8cfaa1 100644
--- a/gerrit-elasticsearch/BUILD
+++ b/gerrit-elasticsearch/BUILD
@@ -35,6 +35,7 @@
name = 'elasticsearch_tests',
tags = ['elastic', 'flaky'],
srcs = glob(['src/test/java/**/*.java']),
+ size = "medium",
deps = [
':elasticsearch',
'//gerrit-extension-api:api',
diff --git a/gerrit-extension-api/BUILD b/gerrit-extension-api/BUILD
index cbe0e26..6f4df01 100644
--- a/gerrit-extension-api/BUILD
+++ b/gerrit-extension-api/BUILD
@@ -55,4 +55,5 @@
libs = [':api'],
pkgs = ['com.google.gerrit.extensions'],
external_docs = [JGIT_DOC_URL, GUAVA_DOC_URL],
+ visibility = ['//visibility:public'],
)
diff --git a/gerrit-gpg/BUILD b/gerrit-gpg/BUILD
index 79f50b1..8cab45c 100644
--- a/gerrit-gpg/BUILD
+++ b/gerrit-gpg/BUILD
@@ -35,6 +35,7 @@
'//lib/bouncycastle:bcprov-without-neverlink',
],
visibility = ['//visibility:public'],
+ testonly = 1,
)
junit_tests(
diff --git a/gerrit-gwtui/BUILD b/gerrit-gwtui/BUILD
index 833ffab..7e692e8 100644
--- a/gerrit-gwtui/BUILD
+++ b/gerrit-gwtui/BUILD
@@ -1,6 +1,6 @@
-load('//tools/bzl:gwt.bzl', 'gwt_module')
+load('//tools/bzl:gwt.bzl', 'gwt_genrule', 'gen_ui_module',
+ 'gwt_user_agent_permutations')
load('//tools/bzl:license.bzl', 'license_test')
-load(':gwt.bzl', 'gwt_binary', 'gwt_genrule', 'gen_ui_module')
gwt_genrule()
gwt_genrule('_r')
@@ -8,6 +8,8 @@
gen_ui_module(name = 'ui_module')
gen_ui_module(name = 'ui_module', suffix = '_r')
+gwt_user_agent_permutations()
+
license_test(
name = "ui_module_license_test",
target = ":ui_module",
diff --git a/gerrit-gwtui/gwt.bzl b/gerrit-gwtui/gwt.bzl
deleted file mode 100644
index 3e83789..0000000
--- a/gerrit-gwtui/gwt.bzl
+++ /dev/null
@@ -1,197 +0,0 @@
-# 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.
-
-# Port of Buck native gwt_binary() rule. See discussion in context of
-# https://github.com/facebook/buck/issues/109
-load('//tools/bzl:genrule2.bzl', 'genrule2')
-load('//tools/bzl:gwt.bzl', 'gwt_module')
-
-jar_filetype = FileType(['.jar'])
-
-MODULE = 'com.google.gerrit.GerritGwtUI'
-
-GWT_COMPILER = "com.google.gwt.dev.Compiler"
-
-GWT_JVM_ARGS = ['-Xmx512m']
-
-GWT_COMPILER_ARGS = [
- '-XdisableClassMetadata',
-]
-
-GWT_COMPILER_ARGS_RELEASE_MODE = GWT_COMPILER_ARGS + [
- '-XdisableCastChecking',
-]
-
-GWT_TRANSITIVE_DEPS = [
- '//lib/gwt:ant',
- '//lib/gwt:colt',
- '//lib/gwt:javax-validation',
- '//lib/gwt:javax-validation_src',
- '//lib/gwt:jsinterop-annotations',
- '//lib/gwt:jsinterop-annotations_src',
- '//lib/gwt:tapestry',
- '//lib/gwt:w3c-css-sac',
- '//lib/ow2:ow2-asm',
- '//lib/ow2:ow2-asm-analysis',
- '//lib/ow2:ow2-asm-commons',
- '//lib/ow2:ow2-asm-tree',
- '//lib/ow2:ow2-asm-util',
-]
-
-DEPS = GWT_TRANSITIVE_DEPS + [
- '//gerrit-gwtexpui:CSS',
- '//lib:gwtjsonrpc',
- '//lib/gwt:dev',
- '@jgit_src//file',
-]
-
-def _impl(ctx):
- output_zip = ctx.outputs.output
- output_dir = output_zip.path + '.gwt_output'
- deploy_dir = output_zip.path + '.gwt_deploy'
-
- deps = _get_transitive_closure(ctx)
-
- paths = []
- for dep in deps:
- paths.append(dep.path)
-
- cmd = "external/local_jdk/bin/java %s -Dgwt.normalizeTimestamps=true -cp %s %s -war %s -deploy %s " % (
- " ".join(ctx.attr.jvm_args),
- ":".join(paths),
- GWT_COMPILER,
- output_dir,
- deploy_dir,
- )
- cmd += " ".join([
- "-style %s" % ctx.attr.style,
- "-optimize %s" % ctx.attr.optimize,
- "-strict",
- " ".join(ctx.attr.compiler_args),
- " ".join(ctx.attr.modules) + "\n",
- "rm -rf %s/gwt-unitCache\n" % output_dir,
- "root=`pwd`\n",
- "cd %s; $root/%s Cc ../%s $(find .)\n" % (
- output_dir,
- ctx.executable._zip.path,
- output_zip.basename,
- )
- ])
-
- ctx.action(
- inputs = list(deps) + ctx.files._jdk + ctx.files._zip,
- outputs = [output_zip],
- mnemonic = "GwtBinary",
- progress_message = "GWT compiling " + output_zip.short_path,
- command = "set -e\n" + cmd,
- )
-
-def _get_transitive_closure(ctx):
- deps = set()
- for dep in ctx.attr.module_deps:
- deps += dep.java.transitive_runtime_deps
- deps += dep.java.transitive_source_jars
- for dep in ctx.attr.deps:
- if hasattr(dep, 'java'):
- deps += dep.java.transitive_runtime_deps
- elif hasattr(dep, 'files'):
- deps += dep.files
-
- return deps
-
-gwt_binary = rule(
- implementation = _impl,
- attrs = {
- "style": attr.string(default = "OBF"),
- "optimize": attr.string(default = "9"),
- "deps": attr.label_list(allow_files=jar_filetype),
- "modules": attr.string_list(mandatory=True),
- "module_deps": attr.label_list(allow_files=jar_filetype),
- "compiler_args": attr.string_list(),
- "jvm_args": attr.string_list(),
- "_jdk": attr.label(
- default=Label("//tools/defaults:jdk")),
- "_zip": attr.label(
- default=Label("@bazel_tools//tools/zip:zipper"),
- cfg = "host",
- executable=True,
- single_file=True),
- },
- outputs = {
- "output": "%{name}.zip",
- },
-)
-
-def gwt_genrule(suffix = ""):
- dbg = 'ui_dbg' + suffix
- opt = 'ui_opt' + suffix
- module_dep = ':ui_module' + suffix
- args = GWT_COMPILER_ARGS_RELEASE_MODE if suffix == "_r" else GWT_COMPILER_ARGS
-
- genrule2(
- name = 'ui_optdbg' + suffix,
- srcs = [
- ':' + dbg,
- ':' + opt,
- ],
- cmd = 'cd $$TMP;' +
- 'unzip -q $$ROOT/$(location :%s);' % dbg +
- 'mv' +
- ' gerrit_ui/gerrit_ui.nocache.js' +
- ' gerrit_ui/dbg_gerrit_ui.nocache.js;' +
- 'unzip -qo $$ROOT/$(location :%s);' % opt +
- 'mkdir -p $$(dirname $@);' +
- 'zip -qr $$ROOT/$@ .',
- out = 'ui_optdbg' + suffix + '.zip',
- visibility = ['//visibility:public'],
- )
-
- gwt_binary(
- name = opt,
- modules = [MODULE],
- module_deps = [module_dep],
- deps = DEPS,
- compiler_args = args,
- jvm_args = GWT_JVM_ARGS,
- )
-
- gwt_binary(
- name = dbg,
- modules = [MODULE],
- style = 'PRETTY',
- optimize = "0",
- module_deps = [module_dep],
- deps = DEPS,
- compiler_args = GWT_COMPILER_ARGS,
- jvm_args = GWT_JVM_ARGS,
- )
-
-def gen_ui_module(name, suffix = ""):
- gwt_module(
- name = name + suffix,
- srcs = native.glob(['src/main/java/**/*.java']),
- gwt_xml = 'src/main/java/%s.gwt.xml' % MODULE.replace('.', '/'),
- resources = native.glob(
- ['src/main/java/**/*'],
- exclude = ['src/main/java/**/*.java'] +
- ['src/main/java/%s.gwt.xml' % MODULE.replace('.', '/')]),
- deps = [
- '//gerrit-gwtui-common:diffy_logo',
- '//gerrit-gwtui-common:client',
- '//gerrit-gwtexpui:CSS',
- '//lib/codemirror:codemirror' + suffix,
- '//lib/gwt:user',
- ],
- visibility = ['//visibility:public'],
- )
diff --git a/gerrit-gwtui/gwt.defs b/gerrit-gwtui/gwt.defs
index cd8fa74..85553f2 100644
--- a/gerrit-gwtui/gwt.defs
+++ b/gerrit-gwtui/gwt.defs
@@ -18,14 +18,14 @@
'firefox',
'gecko1_8',
'safari',
- 'msie', 'ie8', 'ie9', 'ie10', 'ie11',
+ 'msie', 'ie8', 'ie9', 'ie10',
'edge',
]
ALIASES = {
'chrome': 'safari',
'firefox': 'gecko1_8',
- 'msie': 'ie11',
- 'edge': 'edge',
+ 'msie': 'ie10',
+ 'edge': 'gecko1_8',
}
MODULE = 'com.google.gerrit.GerritGwtUI'
CPU_COUNT = cpu_count()
@@ -124,7 +124,6 @@
prebuilt_jar(
name = '%s_gwtxml_lib' % gwt_name,
binary_jar = ':%s_gwtxml_gen' % gwt_name,
- gwt_jar = ':%s_gwtxml_gen' % gwt_name,
)
gwt_binary(
name = gwt_name,
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java
index 000e5fd..066464a 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java
@@ -27,11 +27,11 @@
public class SearchSuggestOracle extends HighlightSuggestOracle {
private static final List<ParamSuggester> paramSuggester = Arrays.asList(
- new ParamSuggester(Arrays.asList("project:", "parentproject:"),
+ new ParamSuggester(Arrays.asList("project:", "p:", "parentproject:"),
new ProjectNameSuggestOracle()),
new ParamSuggester(Arrays.asList(
- "owner:", "reviewer:", "commentby:", "reviewedby:", "author:",
- "committer:", "from:", "assignee:"),
+ "owner:", "o:", "reviewer:", "r:", "commentby:", "reviewedby:",
+ "author:", "committer:", "from:", "assignee:"),
new AccountSuggestOracle() {
@Override
public void onRequestSuggestions(final Request request, final Callback done) {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Assignee.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Assignee.java
index 2956ffc..f1489bb 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Assignee.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Assignee.java
@@ -98,7 +98,6 @@
this.changeId = info.legacyId();
this.canEdit = info.hasActions() && info.actions().containsKey("assignee");
setAssignee(info.assignee());
- assigneeSuggestOracle.setChange(changeId);
editAssigneeIcon.setVisible(canEdit);
if (!canEdit) {
show.setTitle(null);
@@ -111,8 +110,10 @@
UIObject.setVisible(error, false);
editAssigneeIcon.setVisible(false);
suggestBox.setFocus(true);
- suggestBox.setText(FormatUtil.nameEmail(currentAssignee));
- suggestBox.selectAll();
+ if (currentAssignee != null) {
+ suggestBox.setText(FormatUtil.nameEmail(currentAssignee));
+ suggestBox.selectAll();
+ }
}
void onCloseForm() {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/AssigneeSuggestOracle.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/AssigneeSuggestOracle.java
index 8dc5574..47d7541 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/AssigneeSuggestOracle.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/AssigneeSuggestOracle.java
@@ -14,13 +14,12 @@
package com.google.gerrit.client.change;
-import com.google.gerrit.client.change.ReviewerSuggestOracle.RestReviewerSuggestion;
-import com.google.gerrit.client.change.ReviewerSuggestOracle.SuggestReviewerInfo;
-import com.google.gerrit.client.changes.ChangeApi;
+import com.google.gerrit.client.account.AccountApi;
+import com.google.gerrit.client.info.AccountInfo;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.rpc.Natives;
+import com.google.gerrit.client.ui.AccountSuggestOracle.AccountSuggestion;
import com.google.gerrit.client.ui.SuggestAfterTypingNCharsOracle;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gwt.core.client.JsArray;
import java.util.ArrayList;
@@ -29,31 +28,24 @@
/** REST API based suggestion Oracle for assignee */
public class AssigneeSuggestOracle extends SuggestAfterTypingNCharsOracle {
- private Change.Id changeId;
-
- public void setChange(Change.Id changeId) {
- this.changeId = changeId;
- }
-
@Override
protected void _onRequestSuggestions(Request req, Callback cb) {
- ChangeApi
- .suggestReviewers(changeId.get(), req.getQuery(), req.getLimit(), true)
- .get(new GerritCallback<JsArray<SuggestReviewerInfo>>() {
- @Override
- public void onSuccess(JsArray<SuggestReviewerInfo> result) {
- List<RestReviewerSuggestion> r = new ArrayList<>(result.length());
- for (SuggestReviewerInfo reviewer : Natives.asList(result)) {
- r.add(new RestReviewerSuggestion(reviewer, req.getQuery()));
- }
- cb.onSuggestionsReady(req, new Response(r));
- }
+ AccountApi.suggest(req.getQuery(), req.getLimit(),
+ new GerritCallback<JsArray<AccountInfo>>() {
+ @Override
+ public void onSuccess(JsArray<AccountInfo> result) {
+ List<AccountSuggestion> r = new ArrayList<>(result.length());
+ for (AccountInfo reviewer : Natives.asList(result)) {
+ r.add(new AccountSuggestion(reviewer, req.getQuery()));
+ }
+ cb.onSuggestionsReady(req, new Response(r));
+ }
- @Override
- public void onFailure(Throwable err) {
- List<Suggestion> r = Collections.emptyList();
- cb.onSuggestionsReady(req, new Response(r));
- }
- });
+ @Override
+ public void onFailure(Throwable err) {
+ List<Suggestion> r = Collections.emptyList();
+ cb.onSuggestionsReady(req, new Response(r));
+ }
+ });
}
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AccountSuggestOracle.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AccountSuggestOracle.java
index bfeeaec..e43a24e 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AccountSuggestOracle.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AccountSuggestOracle.java
@@ -45,7 +45,7 @@
public static class AccountSuggestion implements SuggestOracle.Suggestion {
private final String suggestion;
- AccountSuggestion(AccountInfo info, String query) {
+ public AccountSuggestion(AccountInfo info, String query) {
this.suggestion = format(info, query);
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/SuggestAfterTypingNCharsOracle.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/SuggestAfterTypingNCharsOracle.java
index 2d7736b..cab29da 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/SuggestAfterTypingNCharsOracle.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/SuggestAfterTypingNCharsOracle.java
@@ -29,7 +29,8 @@
@Override
protected void onRequestSuggestions(Request req, Callback cb) {
- if (req.getQuery().length() >= Gerrit.info().suggest().from()) {
+ if (req.getQuery() != null
+ && req.getQuery().length() >= Gerrit.info().suggest().from()) {
_onRequestSuggestions(req, cb);
} else {
List<Suggestion> none = Collections.emptyList();
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
index 7d62dd7..a0ab714 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
@@ -68,6 +68,7 @@
import com.google.gerrit.server.index.IndexModule;
import com.google.gerrit.server.index.IndexModule.IndexType;
import com.google.gerrit.server.mail.SignedTokenEmailTokenVerifier;
+import com.google.gerrit.server.mail.receive.MailReceiver;
import com.google.gerrit.server.mail.send.SmtpEmailSender;
import com.google.gerrit.server.mime.MimeUtil2Module;
import com.google.gerrit.server.patch.DiffExecutorModule;
@@ -362,6 +363,7 @@
modules.add(new SearchingChangeCacheImpl.Module(slave));
modules.add(new InternalAccountDirectory.Module());
modules.add(new DefaultCacheFactory.Module());
+ modules.add(cfgInjector.getInstance(MailReceiver.Module.class));
if (emailModule != null) {
modules.add(emailModule);
} else {
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitSendEmail.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitSendEmail.java
index afac097..b140ab1 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitSendEmail.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitSendEmail.java
@@ -21,7 +21,7 @@
import com.google.gerrit.pgm.init.api.InitStep;
import com.google.gerrit.pgm.init.api.Section;
import com.google.gerrit.server.config.SitePaths;
-import com.google.gerrit.server.mail.send.SmtpEmailSender.Encryption;
+import com.google.gerrit.server.mail.Encryption;
import com.google.inject.Inject;
import com.google.inject.Singleton;
diff --git a/gerrit-plugin-api/BUILD b/gerrit-plugin-api/BUILD
index e2d8372..e231a02 100644
--- a/gerrit-plugin-api/BUILD
+++ b/gerrit-plugin-api/BUILD
@@ -103,4 +103,5 @@
'//gerrit-gwtexpui:server',
'//gerrit-reviewdb:server',
],
+ visibility = ['//visibility:public'],
)
diff --git a/gerrit-server/BUCK b/gerrit-server/BUCK
index a50df82..1e71009 100644
--- a/gerrit-server/BUCK
+++ b/gerrit-server/BUCK
@@ -70,6 +70,8 @@
'//lib/lucene:lucene-analyzers-common',
'//lib/lucene:lucene-core-and-backward-codecs',
'//lib/lucene:lucene-queryparser',
+ '//lib/mime4j:core',
+ '//lib/mime4j:dom',
'//lib/ow2:ow2-asm',
'//lib/ow2:ow2-asm-tree',
'//lib/ow2:ow2-asm-util',
diff --git a/gerrit-server/BUILD b/gerrit-server/BUILD
index 3874fc9..d410865 100644
--- a/gerrit-server/BUILD
+++ b/gerrit-server/BUILD
@@ -72,6 +72,8 @@
'//lib/lucene:lucene-analyzers-common',
'//lib/lucene:lucene-core-and-backward-codecs',
'//lib/lucene:lucene-queryparser',
+ '//lib/mime4j:core',
+ '//lib/mime4j:dom',
'//lib/ow2:ow2-asm',
'//lib/ow2:ow2-asm-tree',
'//lib/ow2:ow2-asm-util',
@@ -127,6 +129,7 @@
'//lib/powermock:powermock-module-junit4-common',
],
visibility = ['//visibility:public'],
+ testonly = 1,
)
PROLOG_TEST_CASE = [
@@ -151,6 +154,7 @@
'//lib/guice:guice',
'//lib/prolog:runtime',
],
+ testonly = 1,
)
junit_tests(
@@ -181,11 +185,13 @@
'//lib/antlr:java_runtime',
],
visibility = ['//visibility:public'],
+ testonly = 1,
)
junit_tests(
name = 'query_tests',
srcs = QUERY_TESTS,
+ size = "medium",
deps = TESTUTIL_DEPS + [
':testutil',
'//gerrit-antlr:query_exception',
@@ -204,6 +210,7 @@
exclude = TESTUTIL + PROLOG_TESTS + PROLOG_TEST_CASE + QUERY_TESTS
),
resources = glob(['src/test/resources/com/google/gerrit/server/mail/*']),
+ size = "medium",
deps = TESTUTIL_DEPS + [
':testutil',
'//gerrit-antlr:query_exception',
diff --git a/gerrit-server/src/main/java/com/google/gerrit/metrics/Timer0.java b/gerrit-server/src/main/java/com/google/gerrit/metrics/Timer0.java
index d8ff413..d2c9a52 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/metrics/Timer0.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/metrics/Timer0.java
@@ -26,7 +26,7 @@
* Typical usage in a try-with-resources block:
*
* <pre>
- * try (Timer.Context ctx = timer.start()) {
+ * try (Timer0.Context ctx = timer.start()) {
* }
* </pre>
*/
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java
index d1e0ae5..e65879b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java
@@ -16,6 +16,7 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage.REVIEW_DB;
import static java.util.stream.Collectors.toList;
import com.google.common.collect.ComparisonChain;
@@ -42,6 +43,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
@@ -357,8 +359,11 @@
for (Comment c : comments) {
update.deleteComment(c);
}
- db.patchComments().delete(toPatchLineComments(update.getId(),
- PatchLineComment.Status.DRAFT, comments));
+ if (PrimaryStorage.of(update.getChange()) == REVIEW_DB) {
+ // Avoid OrmConcurrencyException trying to delete non-existent entities.
+ db.patchComments().delete(toPatchLineComments(update.getId(),
+ PatchLineComment.Status.DRAFT, comments));
+ }
}
public void deleteAllDraftsFromAllUsers(Change.Id changeId)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/PatchSetUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/PatchSetUtil.java
index 0dcf3bf..3ca11d5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/PatchSetUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/PatchSetUtil.java
@@ -16,6 +16,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage.REVIEW_DB;
import static com.google.gerrit.server.notedb.PatchSetState.DRAFT;
import static com.google.gerrit.server.notedb.PatchSetState.PUBLISHED;
@@ -27,6 +28,7 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.notedb.PatchSetState;
import com.google.gwtorm.server.OrmException;
@@ -126,7 +128,10 @@
checkArgument(ps.isDraft(),
"cannot delete non-draft patch set %s", ps.getId());
update.setPatchSetState(PatchSetState.DELETED);
- db.patchSets().delete(Collections.singleton(ps));
+ if (PrimaryStorage.of(update.getChange()) == REVIEW_DB) {
+ // Avoid OrmConcurrencyException trying to delete non-existent entities.
+ db.patchSets().delete(Collections.singleton(ps));
+ }
}
private void ensurePatchSetMatches(PatchSet.Id psId, ChangeUpdate update) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ReviewersUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ReviewersUtil.java
index 1781f1a..a09537a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ReviewersUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ReviewersUtil.java
@@ -14,16 +14,21 @@
package com.google.gerrit.server;
+import static java.util.stream.Collectors.toList;
+
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.errors.NoSuchGroupException;
-import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.GroupBaseInfo;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
import com.google.gerrit.extensions.restapi.Url;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Description.Units;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.client.Project;
@@ -49,6 +54,7 @@
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import com.google.inject.Singleton;
import java.io.IOException;
import java.util.ArrayList;
@@ -56,9 +62,46 @@
import java.util.EnumSet;
import java.util.HashSet;
import java.util.List;
+import java.util.Objects;
import java.util.Set;
public class ReviewersUtil {
+ @Singleton
+ private static class Metrics {
+ final Timer0 queryAccountsLatency;
+ final Timer0 recommendAccountsLatency;
+ final Timer0 loadAccountsLatency;
+ final Timer0 queryGroupsLatency;
+
+ @Inject
+ Metrics(MetricMaker metricMaker) {
+ queryAccountsLatency = metricMaker.newTimer(
+ "reviewer_suggestion/query_accounts",
+ new Description(
+ "Latency for querying accounts for reviewer suggestion")
+ .setCumulative()
+ .setUnit(Units.MILLISECONDS));
+ recommendAccountsLatency = metricMaker.newTimer(
+ "reviewer_suggestion/recommend_accounts",
+ new Description(
+ "Latency for recommending accounts for reviewer suggestion")
+ .setCumulative()
+ .setUnit(Units.MILLISECONDS));
+ loadAccountsLatency = metricMaker.newTimer(
+ "reviewer_suggestion/load_accounts",
+ new Description(
+ "Latency for loading accounts for reviewer suggestion")
+ .setCumulative()
+ .setUnit(Units.MILLISECONDS));
+ queryGroupsLatency = metricMaker.newTimer(
+ "reviewer_suggestion/query_groups",
+ new Description(
+ "Latency for querying groups for reviewer suggestion")
+ .setCumulative()
+ .setUnit(Units.MILLISECONDS));
+ }
+ }
+
private static final String MAX_SUFFIX = "\u9fa5";
// Generate a candidate list at 3x the size of what the user wants to see to
// give the ranking algorithm a good set of candidates it can work with
@@ -75,6 +118,7 @@
private final Provider<CurrentUser> currentUser;
private final Provider<ReviewDb> dbProvider;
private final ReviewerRecommender reviewerRecommender;
+ private final Metrics metrics;
@Inject
ReviewersUtil(AccountCache accountCache,
@@ -87,7 +131,8 @@
GroupMembers.Factory groupMembersFactory,
Provider<CurrentUser> currentUser,
Provider<ReviewDb> dbProvider,
- ReviewerRecommender reviewerRecommender) {
+ ReviewerRecommender reviewerRecommender,
+ Metrics metrics) {
Set<FillOptions> fillOptions = EnumSet.of(FillOptions.SECONDARY_EMAILS);
fillOptions.addAll(AccountLoader.DETAILED_OPTIONS);
this.accountCache = accountCache;
@@ -101,6 +146,7 @@
this.groupBackend = groupBackend;
this.groupMembersFactory = groupMembersFactory;
this.reviewerRecommender = reviewerRecommender;
+ this.metrics = metrics;
}
public interface VisibilityControl {
@@ -123,58 +169,34 @@
candidateList = suggestAccounts(suggestReviewers, visibilityControl);
}
- List<Account.Id> sortedRecommendations = reviewerRecommender
- .suggestReviewers(changeNotes, suggestReviewers, projectControl,
- candidateList);
-
- // Populate AccountInfo
- List<SuggestedReviewerInfo> reviewer = new ArrayList<>();
- for (Account.Id id : sortedRecommendations) {
- AccountInfo account = accountLoader.get(id);
- if (account != null) {
- SuggestedReviewerInfo info = new SuggestedReviewerInfo();
- info.account = account;
- info.count = 1;
- reviewer.add(info);
- }
- }
- accountLoader.fill();
+ List<Account.Id> sortedRecommendations = recommendAccounts(changeNotes,
+ suggestReviewers, projectControl, candidateList);
+ List<SuggestedReviewerInfo> suggestedReviewer =
+ loadAccounts(sortedRecommendations);
if (!excludeGroups && !Strings.isNullOrEmpty(query)) {
- for (GroupReference g : suggestAccountGroup(suggestReviewers, projectControl)) {
- GroupAsReviewer result = suggestGroupAsReviewer(
- suggestReviewers, projectControl.getProject(), g, visibilityControl);
- if (result.allowed || result.allowedWithConfirmation) {
- GroupBaseInfo info = new GroupBaseInfo();
- info.id = Url.encode(g.getUUID().get());
- info.name = g.getName();
- SuggestedReviewerInfo suggestedReviewerInfo = new SuggestedReviewerInfo();
- suggestedReviewerInfo.group = info;
- suggestedReviewerInfo.count = result.size;
- if (result.allowedWithConfirmation) {
- suggestedReviewerInfo.confirm = true;
- }
- // Always add groups at the end as individual accounts are usually
- // more important
- reviewer.add(suggestedReviewerInfo);
- }
- }
+ // Add groups at the end as individual accounts are usually more
+ // important.
+ suggestedReviewer.addAll(suggestAccountGroups(
+ suggestReviewers, projectControl, visibilityControl));
}
- if (reviewer.size() <= limit) {
- return reviewer;
+ if (suggestedReviewer.size() <= limit) {
+ return suggestedReviewer;
}
- return reviewer.subList(0, limit);
+ return suggestedReviewer.subList(0, limit);
}
private List<Account.Id> suggestAccounts(SuggestReviewers suggestReviewers,
VisibilityControl visibilityControl)
throws OrmException {
- AccountIndex searchIndex = accountIndexes.getSearchIndex();
- if (searchIndex != null) {
- return suggestAccountsFromIndex(suggestReviewers);
+ try (Timer0.Context ctx = metrics.queryAccountsLatency.start()) {
+ AccountIndex searchIndex = accountIndexes.getSearchIndex();
+ if (searchIndex != null) {
+ return suggestAccountsFromIndex(suggestReviewers);
+ }
+ return suggestAccountsFromDb(suggestReviewers, visibilityControl);
}
- return suggestAccountsFromDb(suggestReviewers, visibilityControl);
}
private List<Account.Id> suggestAccountsFromIndex(
@@ -249,7 +271,60 @@
return false;
}
- private List<GroupReference> suggestAccountGroup(
+ private List<Account.Id> recommendAccounts(ChangeNotes changeNotes,
+ SuggestReviewers suggestReviewers, ProjectControl projectControl,
+ List<Account.Id> candidateList) throws OrmException {
+ try (Timer0.Context ctx = metrics.recommendAccountsLatency.start()) {
+ return reviewerRecommender.suggestReviewers(changeNotes, suggestReviewers,
+ projectControl, candidateList);
+ }
+ }
+
+ private List<SuggestedReviewerInfo> loadAccounts(List<Account.Id> accountIds)
+ throws OrmException {
+ try (Timer0.Context ctx = metrics.loadAccountsLatency.start()) {
+ List<SuggestedReviewerInfo> reviewer = accountIds.stream()
+ .map(accountLoader::get)
+ .filter(Objects::nonNull)
+ .map(a -> {
+ SuggestedReviewerInfo info = new SuggestedReviewerInfo();
+ info.account = a;
+ info.count = 1;
+ return info;
+ }).collect(toList());
+ accountLoader.fill();
+ return reviewer;
+ }
+ }
+
+ private List<SuggestedReviewerInfo> suggestAccountGroups(
+ SuggestReviewers suggestReviewers, ProjectControl projectControl,
+ VisibilityControl visibilityControl) throws OrmException, IOException {
+ try (Timer0.Context ctx = metrics.queryGroupsLatency.start()) {
+ List<SuggestedReviewerInfo> groups = new ArrayList<>();
+ for (GroupReference g : suggestAccountGroups(suggestReviewers,
+ projectControl)) {
+ GroupAsReviewer result = suggestGroupAsReviewer(suggestReviewers,
+ projectControl.getProject(), g, visibilityControl);
+ if (result.allowed || result.allowedWithConfirmation) {
+ GroupBaseInfo info = new GroupBaseInfo();
+ info.id = Url.encode(g.getUUID().get());
+ info.name = g.getName();
+ SuggestedReviewerInfo suggestedReviewerInfo =
+ new SuggestedReviewerInfo();
+ suggestedReviewerInfo.group = info;
+ suggestedReviewerInfo.count = result.size;
+ if (result.allowedWithConfirmation) {
+ suggestedReviewerInfo.confirm = true;
+ }
+ groups.add(suggestedReviewerInfo);
+ }
+ }
+ return groups;
+ }
+ }
+
+ private List<GroupReference> suggestAccountGroups(
SuggestReviewers suggestReviewers, ProjectControl ctl) {
return Lists.newArrayList(
Iterables.limit(groupBackend.suggest(suggestReviewers.getQuery(), ctl),
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeOp.java
index d1f7cac..afec66a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeOp.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.change;
import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage.REVIEW_DB;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -32,6 +33,7 @@
import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
import com.google.gerrit.server.git.BatchUpdate.RepoContext;
import com.google.gerrit.server.git.BatchUpdateReviewDb;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -153,6 +155,10 @@
private void deleteChangeElementsFromDb(ChangeContext ctx, Change.Id id)
throws OrmException {
+ if (PrimaryStorage.of(ctx.getChange()) != REVIEW_DB) {
+ return;
+ }
+ // Avoid OrmConcurrencyException trying to delete non-existent entities.
// Only delete from ReviewDb here; deletion from NoteDb is handled in
// BatchUpdate.
ReviewDb db = unwrap(ctx.getDb());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java
index e473e39..79cc35b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.change;
+import static com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage.REVIEW_DB;
+
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -34,6 +36,7 @@
import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
import com.google.gerrit.server.git.BatchUpdate.RepoContext;
import com.google.gerrit.server.git.UpdateException;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.NoSuchChangeException;
@@ -146,11 +149,14 @@
psUtil.delete(ctx.getDb(), ctx.getUpdate(patchSet.getId()), patchSet);
accountPatchReviewStore.get().clearReviewed(psId);
- // Use the unwrap from DeleteChangeOp to handle BatchUpdateReviewDb.
- ReviewDb db = DeleteChangeOp.unwrap(ctx.getDb());
- db.changeMessages().delete(db.changeMessages().byPatchSet(psId));
- db.patchComments().delete(db.patchComments().byPatchSet(psId));
- db.patchSetApprovals().delete(db.patchSetApprovals().byPatchSet(psId));
+ if (PrimaryStorage.of(ctx.getChange()) == REVIEW_DB) {
+ // Avoid OrmConcurrencyException trying to delete non-existent entities.
+ // Use the unwrap from DeleteChangeOp to handle BatchUpdateReviewDb.
+ ReviewDb db = DeleteChangeOp.unwrap(ctx.getDb());
+ db.changeMessages().delete(db.changeMessages().byPatchSet(psId));
+ db.patchComments().delete(db.patchComments().byPatchSet(psId));
+ db.patchSetApprovals().delete(db.patchSetApprovals().byPatchSet(psId));
+ }
}
private void deleteOrUpdateDraftChange(ChangeContext ctx)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java
index 2882a29..14d9ae3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.change;
+import static com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage.REVIEW_DB;
+
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.common.TimeUtil;
@@ -46,6 +48,7 @@
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.mail.send.DeleteReviewerSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -179,8 +182,10 @@
} else {
msg.append(".");
}
-
- ctx.getDb().patchSetApprovals().delete(del);
+ if (PrimaryStorage.of(ctx.getChange()) == REVIEW_DB) {
+ // Avoid OrmConcurrencyException trying to update non-existent entities.
+ ctx.getDb().patchSetApprovals().delete(del);
+ }
ChangeUpdate update = ctx.getUpdate(currPs.getId());
update.removeReviewer(reviewerId);
@@ -208,8 +213,10 @@
Account.Id accountId) throws OrmException {
Change.Id changeId = ctx.getNotes().getChangeId();
Iterable<PatchSetApproval> approvals;
+ PrimaryStorage r = PrimaryStorage.of(ctx.getChange());
- if (migration.readChanges()) {
+ if (migration.readChanges()
+ && r == PrimaryStorage.REVIEW_DB) {
// Because NoteDb and ReviewDb have different semantics for zero-value
// approvals, we must fall back to ReviewDb as the source of truth here.
ReviewDb db = ctx.getDb();
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 aab40e0..e1d1caa 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
@@ -15,6 +15,7 @@
package com.google.gerrit.server.change;
import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage.REVIEW_DB;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.LabelTypes;
@@ -44,6 +45,7 @@
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.mail.send.DeleteVoteSender;
import com.google.gerrit.server.mail.send.ReplyToChangeSender;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.util.LabelVote;
import com.google.gwtorm.server.OrmException;
@@ -166,8 +168,11 @@
}
ctx.getUpdate(psId).removeApprovalFor(accountId, label);
- ctx.getDb().patchSetApprovals().upsert(
- Collections.singleton(deletedApproval(ctx)));
+ if (PrimaryStorage.of(ctx.getChange()) == REVIEW_DB) {
+ // Avoid OrmConcurrencyException trying to update non-existent entities.
+ ctx.getDb().patchSetApprovals().upsert(
+ Collections.singleton(deletedApproval(ctx)));
+ }
StringBuilder msg = new StringBuilder();
msg.append("Removed ");
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index c930c82..2decd12 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -17,6 +17,7 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.server.CommentsUtil.setCommentRevId;
+import static com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage.REVIEW_DB;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.joining;
@@ -47,8 +48,10 @@
import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
import com.google.gerrit.extensions.api.changes.ReviewInput.RobotCommentInput;
import com.google.gerrit.extensions.api.changes.ReviewResult;
+import com.google.gerrit.extensions.api.changes.ReviewerInfo;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.client.Side;
+import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
@@ -83,6 +86,7 @@
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.project.ChangeControl;
@@ -227,11 +231,48 @@
try (BatchUpdate bu = batchUpdateFactory.create(db.get(),
revision.getChange().getProject(), revision.getUser(), ts)) {
+ Account.Id id = bu.getUser().getAccountId();
+ boolean ccOrReviewer = input.labels != null;
+
+ if (!ccOrReviewer) {
+ // Check if user was already CCed or reviewing prior to this review.
+ ReviewerSet currentReviewers = approvalsUtil.getReviewers(
+ db.get(), revision.getChangeResource().getNotes());
+ ccOrReviewer = currentReviewers.all().contains(id);
+ }
+
// Apply reviewer changes first. Revision emails should be sent to the
- // updated set of reviewers.
+ // updated set of reviewers. Also keep track of whether the user added
+ // themselves as a reviewer or to the CC list.
for (PostReviewers.Addition reviewerResult : reviewerResults) {
bu.addOp(revision.getChange().getId(), reviewerResult.op);
+ if (!ccOrReviewer && reviewerResult.result.reviewers != null) {
+ for (ReviewerInfo reviewerInfo : reviewerResult.result.reviewers) {
+ if (id.equals(reviewerInfo._accountId)) {
+ ccOrReviewer = true;
+ break;
+ }
+ }
+ }
+ if (!ccOrReviewer && reviewerResult.result.ccs != null) {
+ for (AccountInfo accountInfo : reviewerResult.result.ccs) {
+ if (id.equals(accountInfo._accountId)) {
+ ccOrReviewer = true;
+ break;
+ }
+ }
+ }
}
+
+ if (!ccOrReviewer) {
+ // User posting this review isn't currently in the reviewer or CC list,
+ // isn't being explicitly added, and isn't voting on any label.
+ // Automatically CC them on this change so they receive replies.
+ PostReviewers.Addition selfAddition =
+ postReviewers.ccCurrentUser(bu.getUser(), revision);
+ bu.addOp(revision.getChange().getId(), selfAddition.op);
+ }
+
bu.addOp(
revision.getChange().getId(),
new Op(revision.getPatchSet().getId(), input, reviewerResults));
@@ -833,8 +874,11 @@
}
forceCallerAsReviewer(ctx, current, ups, del);
- ctx.getDb().patchSetApprovals().delete(del);
- ctx.getDb().patchSetApprovals().upsert(ups);
+ if (PrimaryStorage.of(update.getChange()) == REVIEW_DB) {
+ // Avoid OrmConcurrencyException trying to delete non-existent entities.
+ ctx.getDb().patchSetApprovals().delete(del);
+ ctx.getDb().patchSetApprovals().upsert(ups);
+ }
return !del.isEmpty() || !ups.isEmpty();
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
index 420c05c..f0af5da 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
@@ -39,6 +39,7 @@
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountCache;
@@ -177,6 +178,13 @@
input.state(), input.notify);
}
+ Addition ccCurrentUser(CurrentUser user, RevisionResource revision) {
+ return new Addition(
+ user.getUserName(), revision.getChangeResource(),
+ ImmutableMap.of(user.getAccountId(), revision.getControl()),
+ CC, NotifyHandling.NONE);
+ }
+
private Addition putAccount(String reviewer, ReviewerResource rsrc,
ReviewerState state, NotifyHandling notify)
throws UnprocessableEntityException {
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 61f6557..3953f27 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
@@ -44,7 +44,6 @@
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.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.reviewdb.server.ReviewDbWrapper;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
@@ -54,7 +53,9 @@
import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NoteDbUpdateManager;
+import com.google.gerrit.server.notedb.NoteDbUpdateManager.MismatchedStateException;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
@@ -62,7 +63,6 @@
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.NoSuchRefException;
import com.google.gerrit.server.util.RequestId;
-import com.google.gwtorm.server.OrmConcurrencyException;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
@@ -938,10 +938,17 @@
@SuppressWarnings("resource") // Not always opened.
NoteDbUpdateManager updateManager = null;
try {
- ChangeContext ctx;
+ PrimaryStorage storage;
db.changes().beginTransaction(id);
try {
- ctx = newChangeContext(db, repo, rw, id);
+ ChangeContext ctx = newChangeContext(db, repo, rw, id);
+ storage = PrimaryStorage.of(ctx.getChange());
+ if (storage == PrimaryStorage.NOTE_DB
+ && !notesMigration.readChanges()) {
+ throw new OrmException(
+ "must have NoteDb enabled to update change " + id);
+ }
+
// Call updateChange on each op.
logDebug("Calling updateChange on {} ops", changeOps.size());
for (Op op : changeOps) {
@@ -961,34 +968,45 @@
updateManager = stageNoteDbUpdate(ctx, deleted);
}
- // Bump lastUpdatedOn or rowVersion and commit.
- Iterable<Change> cs = changesToUpdate(ctx);
- if (newChanges.containsKey(id)) {
- // Insert rather than upsert in case of a race on change IDs.
- logDebug("Inserting change");
- db.changes().insert(cs);
- } else if (deleted) {
- logDebug("Deleting change");
- db.changes().delete(cs);
+ if (storage == PrimaryStorage.REVIEW_DB) {
+ // If primary storage of this change is in ReviewDb, bump
+ // lastUpdatedOn or rowVersion and commit. Otherwise, don't waste
+ // time updating ReviewDb at all.
+ Iterable<Change> cs = changesToUpdate(ctx);
+ if (isNewChange(id)) {
+ // Insert rather than upsert in case of a race on change IDs.
+ logDebug("Inserting change");
+ db.changes().insert(cs);
+ } else if (deleted) {
+ logDebug("Deleting change");
+ db.changes().delete(cs);
+ } else {
+ logDebug("Updating change");
+ db.changes().update(cs);
+ }
+ if (!dryrun) {
+ db.commit();
+ }
} else {
- logDebug("Updating change");
- db.changes().update(cs);
- }
- if (!dryrun) {
- db.commit();
+ logDebug(
+ "Skipping ReviewDb write since primary storage is {}", storage);
}
} finally {
db.rollback();
}
- if (notesMigration.commitChangeWrites()) {
+ // Do not execute the NoteDbUpdateManager, as we don't want too much
+ // contention on the underlying repo, and we would rather use a single
+ // ObjectInserter/BatchRefUpdate later.
+ //
+ // TODO(dborowitz): May or may not be worth trying to batch together
+ // flushed inserters as well.
+ if (storage == PrimaryStorage.NOTE_DB) {
+ // Should have failed above if NoteDb is disabled.
+ checkState(notesMigration.commitChangeWrites());
+ noteDbResult = updateManager.stage().get(id);
+ } else if (notesMigration.commitChangeWrites()) {
try {
- // Do not execute the NoteDbUpdateManager, as we don't want too much
- // contention on the underlying repo, and we would rather use a
- // single ObjectInserter/BatchRefUpdate later.
- //
- // TODO(dborowitz): May or may not be worth trying to batch
- // together flushed inserters as well.
noteDbResult = updateManager.stage().get(id);
} catch (IOException ex) {
// Ignore all errors trying to update NoteDb at this point. We've
@@ -1014,7 +1032,7 @@
RevWalk rw, Change.Id id) throws Exception {
Change c = newChanges.get(id);
if (c == null) {
- c = ReviewDbUtil.unwrapDb(db).changes().get(id);
+ c = ChangeNotes.readOneReviewDbChange(db, id);
}
// Pass in preloaded change to controlFor, to avoid:
// - reading from a db that does not belong to this update
@@ -1034,20 +1052,29 @@
for (ChangeUpdate u : ctx.updates.values()) {
updateManager.add(u);
}
+
+ Change c = ctx.getChange();
if (deleted) {
- updateManager.deleteChange(ctx.getChange().getId());
+ updateManager.deleteChange(c.getId());
}
try {
- updateManager.stageAndApplyDelta(ctx.getChange());
- } catch (OrmConcurrencyException ex) {
- // Refused to apply update because NoteDb was out of sync. Go ahead with
- // this ReviewDb update; it's still out of sync, but this is no worse
- // than before, and it will eventually get rebuilt.
- logDebug("Ignoring OrmConcurrencyException while staging");
+ updateManager.stageAndApplyDelta(c);
+ } catch (MismatchedStateException ex) {
+ // Refused to apply update because NoteDb was out of sync, which can
+ // only happen if ReviewDb is the primary storage for this change.
+ //
+ // Go ahead with this ReviewDb update; it's still out of sync, but this
+ // is no worse than before, and it will eventually get rebuilt.
+ logDebug("Ignoring MismatchedStateException while staging");
}
+
return updateManager;
}
+ private boolean isNewChange(Change.Id id) {
+ return newChanges.containsKey(id);
+ }
+
private void logDebug(String msg, Throwable t) {
if (log.isDebugEnabled()) {
BatchUpdate.this.logDebug("[" + taskId + "]" + msg, t);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java
index cb1d1d4..a60b86f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java
@@ -69,7 +69,6 @@
import org.slf4j.LoggerFactory;
import java.io.IOException;
-import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -289,7 +288,7 @@
cmUtil.addChangeMessage(ctx.getDb(), update, msg);
if (mergedByPushOp == null) {
- resetChange(ctx, msg);
+ resetChange(ctx);
} else {
mergedByPushOp.setPatchSetProvider(Providers.of(newPatchSet))
.updateChange(ctx);
@@ -333,16 +332,8 @@
return current;
}
- private void resetChange(ChangeContext ctx, ChangeMessage msg)
- throws OrmException {
+ private void resetChange(ChangeContext ctx) {
Change change = ctx.getChange();
- if (change.getStatus().isClosed()) {
- ctx.getDb().patchSets().delete(Collections.singleton(newPatchSet));
- ctx.getDb().changeMessages().delete(Collections.singleton(msg));
- rejectMessage = CHANGE_IS_CLOSED;
- return;
- }
-
if (!change.currentPatchSetId().equals(priorPatchSetId)) {
return;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexer.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexer.java
index 8ce5d5e..5ef548c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexer.java
@@ -337,6 +337,7 @@
for (ChangeIndex i : getWriteIndexes()) {
i.delete(id);
}
+ log.info("Deleted change {} from index.", id.get());
fireChangeDeletedFromIndexEvent(id.get());
return null;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/Address.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/Address.java
index d4655f5..2088261 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/Address.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/Address.java
@@ -24,7 +24,16 @@
if (0 <= lt && lt < gt && lt + 1 < at && at + 1 < gt) {
final String email = in.substring(lt + 1, gt).trim();
final String name = in.substring(0, lt).trim();
- return new Address(name.length() > 0 ? name : null, email);
+ int nameStart = 0;
+ int nameEnd = name.length();
+ if (name.startsWith("\"")) {
+ nameStart++;
+ }
+ if (name.endsWith("\"")) {
+ nameEnd--;
+ }
+ return new Address(name.length() > 0 ?
+ name.substring(nameStart, nameEnd): null, email);
}
if (lt < 0 && gt < 0 && 0 < at && at < in.length() - 1) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailSettings.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailSettings.java
index 2f2fd8c5..b719193 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailSettings.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailSettings.java
@@ -15,21 +15,47 @@
package com.google.gerrit.server.mail;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.mail.receive.Protocol;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import org.eclipse.jgit.lib.Config;
+import java.util.concurrent.TimeUnit;
+
@Singleton
public class EmailSettings {
+ private static final String SEND_EMAL = "sendemail";
+ private static final String RECEIVE_EMAL = "receiveemail";
+ // Send
public final boolean html;
public final boolean includeDiff;
public final int maximumDiffSize;
+ // Receive
+ public final Protocol protocol;
+ public final String host;
+ public final int port;
+ public final String username;
+ public final String password;
+ public final Encryption encryption;
+ public final long fetchInterval; // in milliseconds
@Inject
EmailSettings(@GerritServerConfig Config cfg) {
- html = cfg.getBoolean("sendemail", "html", true);
- includeDiff = cfg.getBoolean("sendemail", "includeDiff", false);
- maximumDiffSize = cfg.getInt("sendemail", "maximumDiffSize", 256 << 10);
+ // Send
+ html = cfg.getBoolean(SEND_EMAL, "html", true);
+ includeDiff = cfg.getBoolean(SEND_EMAL, "includeDiff", false);
+ maximumDiffSize = cfg.getInt(SEND_EMAL, "maximumDiffSize", 256 << 10);
+ // Receive
+ protocol = cfg.getEnum(RECEIVE_EMAL, null, "protocol", Protocol.NONE);
+ host = cfg.getString(RECEIVE_EMAL, null, "host");
+ port = cfg.getInt(RECEIVE_EMAL, "port", 0);
+ username = cfg.getString(RECEIVE_EMAL, null, "username");
+ password = cfg.getString(RECEIVE_EMAL, null, "password");
+ encryption =
+ cfg.getEnum(RECEIVE_EMAL, null, "encryption", Encryption.NONE);
+ fetchInterval = cfg.getTimeUnit(RECEIVE_EMAL, null, "fetchInterval",
+ TimeUnit.MILLISECONDS.convert(60, TimeUnit.SECONDS),
+ TimeUnit.MILLISECONDS);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/Encryption.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/Encryption.java
new file mode 100644
index 0000000..a557532
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/Encryption.java
@@ -0,0 +1,19 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail;
+
+public enum Encryption {
+ NONE, SSL, TLS
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/ImapMailReceiver.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/ImapMailReceiver.java
new file mode 100644
index 0000000..32a26b1
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/ImapMailReceiver.java
@@ -0,0 +1,143 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive;
+
+import com.google.gerrit.server.mail.EmailSettings;
+import com.google.gerrit.server.mail.Encryption;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+import org.apache.commons.net.imap.IMAPClient;
+import org.apache.commons.net.imap.IMAPSClient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+@Singleton
+public class ImapMailReceiver extends MailReceiver {
+ private static final Logger log =
+ LoggerFactory.getLogger(ImapMailReceiver.class);
+ private static final String inboxFolder = "INBOX";
+
+ @Inject
+ public ImapMailReceiver(EmailSettings mailSettings) {
+ super(mailSettings);
+ }
+
+ /**
+ * handleEmails will open a connection to the mail server, remove emails
+ * where deletion is pending, read new email and close the connection.
+ */
+ @Override
+ public synchronized void handleEmails() {
+ IMAPClient imap;
+ if (mailSettings.encryption != Encryption.NONE) {
+ imap = new IMAPSClient(mailSettings.encryption.name(), false);
+ } else {
+ imap = new IMAPClient();
+ }
+ if (mailSettings.port > 0) {
+ imap.setDefaultPort(mailSettings.port);
+ }
+ // Set a 30s timeout for each operation
+ imap.setDefaultTimeout(30 * 1000);
+ try {
+ imap.connect(mailSettings.host);
+ try {
+ if (!imap.login(mailSettings.username, mailSettings.password)) {
+ log.error("Could not login to IMAP server");
+ return;
+ }
+ try {
+ if (!imap.select(inboxFolder)){
+ log.error("Could not select IMAP folder " + inboxFolder);
+ return;
+ }
+ // Fetch just the internal dates first to know how many messages we
+ // should fetch.
+ if (!imap.fetch("1:*", "(INTERNALDATE)")) {
+ log.error("IMAP fetch failed. Will retry in next fetch cycle.");
+ return;
+ }
+ // Format of reply is one line per email and one line to indicate
+ // that the fetch was successful.
+ // Example:
+ // * 1 FETCH (INTERNALDATE "Mon, 24 Oct 2016 16:53:22 +0200 (CEST)")
+ // * 2 FETCH (INTERNALDATE "Mon, 24 Oct 2016 16:53:22 +0200 (CEST)")
+ // AAAC OK FETCH completed.
+ int numMessages = imap.getReplyStrings().length - 1;
+ log.info("Fetched " + numMessages + " messages via IMAP");
+ if (numMessages == 0) {
+ return;
+ }
+ // Fetch the full version of all emails
+ List<MailMessage> mailMessages = new ArrayList<>(numMessages);
+ for (int i = 1; i <= numMessages; i++) {
+ if (imap.fetch(i + ":" + i, "(BODY.PEEK[])")) {
+ // Obtain full reply
+ String[] rawMessage = imap.getReplyStrings();
+ if (rawMessage.length < 2) {
+ continue;
+ }
+ // First and last line are IMAP status codes. We have already
+ // checked, that the fetch returned true (OK), so we safely ignore
+ // those two lines.
+ StringBuilder b = new StringBuilder(2 * (rawMessage.length - 2));
+ for(int j = 1; j < rawMessage.length - 1; j++) {
+ if (j > 1) {
+ b.append("\n");
+ }
+ b.append(rawMessage[j]);
+ }
+ try {
+ MailMessage mailMessage = RawMailParser.parse(b.toString());
+ if (pendingDeletion.contains(mailMessage.id())) {
+ // Mark message as deleted
+ if (imap.store(i + ":" + i, "+FLAGS", "(\\Deleted)")) {
+ pendingDeletion.remove(mailMessage.id());
+ } else {
+ log.error("Could not mark mail message as deleted: " +
+ mailMessage.id());
+ }
+ } else {
+ mailMessages.add(mailMessage);
+ }
+ } catch (MailParsingException e) {
+ log.error("Exception while parsing email after IMAP fetch", e);
+ }
+ } else {
+ log.error("IMAP fetch failed. Will retry in next fetch cycle.");
+ }
+ }
+ // Permanently delete emails marked for deletion
+ if (!imap.expunge()) {
+ log.error("Could not expunge IMAP emails");
+ }
+ // TODO(hiesel) Call email handling logic with mailMessages
+ } finally {
+ imap.logout();
+ }
+ } finally {
+ imap.disconnect();
+ }
+ } catch (IOException e) {
+ log.error("Error while talking to IMAP server", e);
+ return;
+ }
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailMessage.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailMessage.java
new file mode 100644
index 0000000..966f3e6
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailMessage.java
@@ -0,0 +1,89 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.server.mail.Address;
+
+import org.joda.time.DateTime;
+
+/**
+ * MailMessage is a simplified representation of an RFC 2045-2047 mime email
+ * message used for representing received emails inside Gerrit. It is populated
+ * by the MailParser after MailReceiver has received a message. Transformations
+ * done by the parser include stitching mime parts together, transforming all
+ * content to UTF-16 and removing attachments.
+ *
+ * A valid MailMessage contains at least the following fields: id, from, to,
+ * subject and dateReceived.
+ */
+@AutoValue
+public abstract class MailMessage {
+ // Unique Identifier
+ public abstract String id();
+ // Envelop Information
+ public abstract Address from();
+ public abstract ImmutableList<Address> to();
+ @Nullable
+ public abstract ImmutableList<Address> cc();
+ // Metadata
+ public abstract DateTime dateReceived();
+ public abstract ImmutableList<String> additionalHeaders();
+ // Content
+ public abstract String subject();
+ @Nullable
+ public abstract String textContent();
+ @Nullable
+ public abstract String htmlContent();
+
+ public static Builder builder() {
+ return new AutoValue_MailMessage.Builder();
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public abstract Builder id(String val);
+ public abstract Builder from(Address val);
+ public abstract ImmutableList.Builder<Address> toBuilder();
+
+ public Builder addTo(Address val) {
+ toBuilder().add(val);
+ return this;
+ }
+
+ public abstract ImmutableList.Builder<Address> ccBuilder();
+
+ public Builder addCc(Address val) {
+ ccBuilder().add(val);
+ return this;
+ }
+
+ public abstract Builder dateReceived(DateTime val);
+ public abstract ImmutableList.Builder<String> additionalHeadersBuilder();
+
+ public Builder addAdditionalHeader(String val) {
+ additionalHeadersBuilder().add(val);
+ return this;
+ }
+
+ public abstract Builder subject(String val);
+ public abstract Builder textContent(String val);
+ public abstract Builder htmlContent(String val);
+
+ public abstract MailMessage build();
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailParsingException.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailParsingException.java
new file mode 100644
index 0000000..edadef8
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailParsingException.java
@@ -0,0 +1,28 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive;
+
+/** MailParsingException indicates that an email could not be parsed. */
+public class MailParsingException extends Exception {
+ private static final long serialVersionUID = 1L;
+
+ public MailParsingException(String msg) {
+ super(msg);
+ }
+
+ public MailParsingException(String msg, Throwable cause) {
+ super(msg, cause);
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailReceiver.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailReceiver.java
new file mode 100644
index 0000000..4a8f7fb
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailReceiver.java
@@ -0,0 +1,106 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.gerrit.extensions.events.LifecycleListener;
+import com.google.gerrit.lifecycle.LifecycleModule;
+import com.google.gerrit.server.mail.EmailSettings;
+import com.google.inject.Inject;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.Timer;
+import java.util.TimerTask;
+
+/** MailReceiver implements base functionality for receiving emails. */
+public abstract class MailReceiver implements LifecycleListener {
+ protected EmailSettings mailSettings;
+ protected Set<String> pendingDeletion;
+ private Timer timer;
+
+ public static class Module extends LifecycleModule {
+ private final EmailSettings mailSettings;
+
+ @Inject
+ Module(EmailSettings mailSettings) {
+ this.mailSettings = mailSettings;
+ }
+
+ @Override
+ protected void configure() {
+ if (mailSettings.protocol == Protocol.NONE) {
+ return;
+ }
+ listener().to(MailReceiver.class);
+ switch (mailSettings.protocol) {
+ case IMAP:
+ bind(MailReceiver.class).to(ImapMailReceiver.class);
+ break;
+ case POP3:
+ bind(MailReceiver.class).to(Pop3MailReceiver.class);
+ break;
+ case NONE:
+ default:
+ }
+ }
+ }
+
+ @Inject
+ public MailReceiver(EmailSettings mailSettings) {
+ this.mailSettings = mailSettings;
+ pendingDeletion = Collections.synchronizedSet(new HashSet<>());
+ }
+
+ @Override
+ public void start() {
+ if (timer == null) {
+ timer = new Timer();
+ } else {
+ timer.cancel();
+ }
+ timer.scheduleAtFixedRate(new TimerTask() {
+ @Override
+ public void run() {
+ MailReceiver.this.handleEmails();
+ }
+ }, 0l, mailSettings.fetchInterval);
+ }
+
+ @Override
+ public void stop() {
+ if (timer != null) {
+ timer.cancel();
+ }
+ }
+
+ /**
+ * requestDeletion will enqueue an email for deletion and delete it the
+ * next time we connect to the email server. This does not guarantee deletion
+ * as the Gerrit instance might fail before we connect to the email server.
+ * @param messageId
+ */
+ public void requestDeletion(String messageId) {
+ pendingDeletion.add(messageId);
+ }
+
+ /**
+ * handleEmails will open a connection to the mail server, remove emails
+ * where deletion is pending, read new email and close the connection.
+ */
+ @VisibleForTesting
+ public abstract void handleEmails();
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/Pop3MailReceiver.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/Pop3MailReceiver.java
new file mode 100644
index 0000000..6c81011
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/Pop3MailReceiver.java
@@ -0,0 +1,134 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive;
+
+import com.google.common.primitives.Ints;
+import com.google.gerrit.server.mail.EmailSettings;
+import com.google.gerrit.server.mail.Encryption;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+import org.apache.commons.net.pop3.POP3Client;
+import org.apache.commons.net.pop3.POP3MessageInfo;
+import org.apache.commons.net.pop3.POP3SClient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+@Singleton
+public class Pop3MailReceiver extends MailReceiver {
+ private static final Logger log =
+ LoggerFactory.getLogger(Pop3MailReceiver.class);
+
+ @Inject
+ public Pop3MailReceiver(EmailSettings mailSettings) {
+ super(mailSettings);
+ }
+
+ /**
+ * handleEmails will open a connection to the mail server, remove emails
+ * where deletion is pending, read new email and close the connection.
+ */
+ @Override
+ public synchronized void handleEmails() {
+ POP3Client pop3;
+ if (mailSettings.encryption != Encryption.NONE) {
+ pop3 = new POP3SClient(mailSettings.encryption.name());
+ } else {
+ pop3 = new POP3Client();
+ }
+ if (mailSettings.port > 0) {
+ pop3.setDefaultPort(mailSettings.port);
+ }
+ try {
+ pop3.connect(mailSettings.host);
+ } catch (IOException e) {
+ log.error("Could not connect to POP3 email server", e);
+ return;
+ }
+ try {
+ try {
+ if (!pop3.login(mailSettings.username, mailSettings.password)) {
+ log.error("Could not login to POP3 email server."
+ + " Check username and password");
+ return;
+ }
+ try {
+ POP3MessageInfo[] messages = pop3.listMessages();
+ if (messages == null) {
+ log.error("Could not retrieve message list via POP3");
+ return;
+ }
+ log.info("Received " + messages.length + " messages via POP3");
+ // Fetch messages
+ List<MailMessage> mailMessages = new ArrayList<>();
+ for (POP3MessageInfo msginfo : messages) {
+ if (msginfo == null) {
+ // Message was deleted
+ continue;
+ }
+ try (BufferedReader reader =
+ (BufferedReader) pop3.retrieveMessage(msginfo.number)) {
+ if (reader == null) {
+ log.error(
+ "Could not retrieve POP3 message header for message {}",
+ msginfo.identifier);
+ return;
+ }
+ int[] message = fetchMessage(reader);
+ MailMessage mailMessage = RawMailParser.parse(message);
+ // Delete messages where deletion is pending. This requires
+ // knowing the integer message ID of the email. We therefore parse
+ // the message first and extract the Message-ID specified in RFC
+ // 822 and delete the message if deletion is pending.
+ if (pendingDeletion.contains(mailMessage.id())) {
+ if (pop3.deleteMessage(msginfo.number)) {
+ pendingDeletion.remove(mailMessage.id());
+ } else {
+ log.error("Could not delete message " + msginfo.number);
+ }
+ } else {
+ // Process message further
+ mailMessages.add(mailMessage);
+ }
+ } catch (MailParsingException e) {
+ log.error("Could not parse message " + msginfo.number);
+ }
+ }
+ // TODO(hiesel) Call processing logic with mailMessages
+ } finally {
+ pop3.logout();
+ }
+ } finally {
+ pop3.disconnect();
+ }
+ } catch (IOException e) {
+ log.error("Error while issuing POP3 command", e);
+ }
+ }
+
+ public final int[] fetchMessage(BufferedReader reader) throws IOException {
+ List<Integer> character = new ArrayList<>();
+ int ch;
+ while ((ch = reader.read()) != -1) {
+ character.add(ch);
+ }
+ return Ints.toArray(character);
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/Protocol.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/Protocol.java
new file mode 100644
index 0000000..e8311f1
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/Protocol.java
@@ -0,0 +1,19 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive;
+
+public enum Protocol {
+ NONE, POP3, IMAP
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/RawMailParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/RawMailParser.java
new file mode 100644
index 0000000..ed35c9b
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/RawMailParser.java
@@ -0,0 +1,170 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive;
+
+import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.io.CharStreams;
+import com.google.gerrit.server.mail.Address;
+
+import org.apache.james.mime4j.MimeException;
+import org.apache.james.mime4j.dom.Entity;
+import org.apache.james.mime4j.dom.Message;
+import org.apache.james.mime4j.dom.MessageBuilder;
+import org.apache.james.mime4j.dom.Multipart;
+import org.apache.james.mime4j.dom.TextBody;
+import org.apache.james.mime4j.dom.address.Mailbox;
+import org.apache.james.mime4j.message.DefaultMessageBuilder;
+import org.joda.time.DateTime;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+
+/**
+ * RawMailParser parses raw email content received through POP3 or IMAP into
+ * an internal {@link MailMessage}.
+ */
+public class RawMailParser {
+ private static final ImmutableSet<String> MAIN_HEADERS =
+ ImmutableSet.of("to", "from", "cc", "date", "message-id",
+ "subject", "content-type");
+
+ /**
+ * Parses a MailMessage from a string.
+ * @param raw String as received over the wire
+ * @return Parsed MailMessage
+ * @throws MailParsingException
+ */
+ public static MailMessage parse(String raw) throws MailParsingException {
+ MailMessage.Builder messageBuilder = MailMessage.builder();
+ Message mimeMessage;
+ try {
+ MessageBuilder builder = new DefaultMessageBuilder();
+ mimeMessage =
+ builder.parseMessage(new ByteArrayInputStream(raw.getBytes()));
+ } catch (IOException | MimeException e) {
+ throw new MailParsingException("Can't parse email", e);
+ }
+ // Add general headers
+ messageBuilder.id(mimeMessage.getMessageId());
+ messageBuilder.subject(mimeMessage.getSubject());
+ messageBuilder.dateReceived(new DateTime(mimeMessage.getDate()));
+
+ // Add From, To and Cc
+ if (mimeMessage.getFrom() != null && mimeMessage.getFrom().size() > 0) {
+ Mailbox from = mimeMessage.getFrom().get(0);
+ messageBuilder.from(new Address(from.getName(), from.getAddress()));
+ }
+ if (mimeMessage.getTo() != null) {
+ for (Mailbox m : mimeMessage.getTo().flatten()) {
+ messageBuilder.addTo(new Address(m.getName(), m.getAddress()));
+ }
+ }
+ if (mimeMessage.getCc() != null) {
+ for (Mailbox m : mimeMessage.getCc().flatten()) {
+ messageBuilder.addCc(new Address(m.getName(), m.getAddress()));
+ }
+ }
+
+ // Add additional headers
+ mimeMessage.getHeader().getFields().stream()
+ .filter(f -> !MAIN_HEADERS.contains(f.getName().toLowerCase()))
+ .forEach(f -> messageBuilder.addAdditionalHeader(
+ f.getName() + ": " + f.getBody()));
+
+ // Add text and html body parts
+ StringBuilder textBuilder = new StringBuilder();
+ StringBuilder htmlBuilder = new StringBuilder();
+ try {
+ handleMimePart(mimeMessage, textBuilder, htmlBuilder);
+ } catch (IOException e) {
+ throw new MailParsingException("Can't parse email", e);
+ }
+ messageBuilder.textContent(Strings.emptyToNull(textBuilder.toString()));
+ messageBuilder.htmlContent(Strings.emptyToNull(htmlBuilder.toString()));
+
+ try {
+ // build() will only succeed if all required attributes were set. We wrap
+ // the IllegalStateException in a MailParsingException indicating that
+ // required attributes are missing, so that the caller doesn't fall over.
+ return messageBuilder.build();
+ } catch (IllegalStateException e) {
+ throw new MailParsingException(
+ "Missing required attributes after email was parsed", e);
+ }
+ }
+
+ /**
+ * Parses a MailMessage from an array of characters. Note that the character
+ * array is int-typed. This method is only used by POP3, which specifies that
+ * all transferred characters are US-ASCII (RFC 6856). When reading the input
+ * in Java, io.Reader yields ints. These can be safely converted to chars
+ * as all US-ASCII characters fit in a char. If emails contain non-ASCII
+ * characters, such as UTF runes, these will be encoded in ASCII using either
+ * Base64 or quoted-printable encoding.
+ * @param chars Array as received over the wire
+ * @return Parsed MailMessage
+ * @throws MailParsingException
+ */
+ public static MailMessage parse(int[] chars) throws MailParsingException {
+ StringBuilder b = new StringBuilder(chars.length);
+ for (int c : chars) {
+ b.append((char) c);
+ }
+ return parse(b.toString());
+ }
+
+ /**
+ * Traverses a mime tree and parses out text and html parts. All other parts
+ * will be dropped.
+ * @param part MimePart to parse
+ * @param textBuilder StringBuilder to append all plaintext parts
+ * @param htmlBuilder StringBuilder to append all html parts
+ * @throws IOException
+ */
+ private static void handleMimePart(Entity part, StringBuilder textBuilder,
+ StringBuilder htmlBuilder) throws IOException {
+ if (isPlainOrHtml(part.getMimeType()) &&
+ !isAttachment(part.getDispositionType())) {
+ TextBody tb = (TextBody) part.getBody();
+ String result = CharStreams.toString(new InputStreamReader(
+ tb.getInputStream(), tb.getMimeCharset()));
+ if (part.getMimeType().equals("text/plain")) {
+ textBuilder.append(result);
+ } else if (part.getMimeType().equals("text/html")) {
+ htmlBuilder.append(result);
+ }
+ } else if (isMixedOrAlternative(part.getMimeType())) {
+ Multipart multipart = (Multipart) part.getBody();
+ for (Entity e : multipart.getBodyParts()) {
+ handleMimePart(e, textBuilder, htmlBuilder);
+ }
+ }
+ }
+
+ private static boolean isPlainOrHtml(String mimeType) {
+ return (mimeType.equals("text/plain") || mimeType.equals("text/html"));
+ }
+
+ private static boolean isMixedOrAlternative(String mimeType) {
+ return mimeType.equals("multipart/alternative") ||
+ mimeType.equals("multipart/mixed");
+ }
+
+ private static boolean isAttachment(String dispositionType) {
+ return dispositionType != null && dispositionType.equals("attachment");
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java
index f27c45f..d5622bf 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java
@@ -25,6 +25,7 @@
import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.mail.Address;
+import com.google.gerrit.server.mail.Encryption;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -61,10 +62,6 @@
}
}
- public enum Encryption {
- NONE, SSL, TLS
- }
-
private final boolean enabled;
private final int connectTimeout;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
index 3c669f0..c00035c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
@@ -27,6 +27,7 @@
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -109,16 +110,20 @@
}
protected final Args args;
+ protected final PrimaryStorage primaryStorage;
protected final boolean autoRebuild;
private final Change.Id changeId;
private ObjectId revision;
private boolean loaded;
- AbstractChangeNotes(Args args, Change.Id changeId, boolean autoRebuild) {
+ AbstractChangeNotes(Args args, Change.Id changeId,
+ @Nullable PrimaryStorage primaryStorage, boolean autoRebuild) {
this.args = checkNotNull(args);
this.changeId = checkNotNull(changeId);
- this.autoRebuild = autoRebuild;
+ this.primaryStorage = primaryStorage;
+ this.autoRebuild = primaryStorage == PrimaryStorage.REVIEW_DB
+ && autoRebuild;
}
public Change.Id getChangeId() {
@@ -135,6 +140,9 @@
return self();
}
boolean read = args.migration.readChanges();
+ if (!read && primaryStorage == PrimaryStorage.NOTE_DB) {
+ throw new OrmException("NoteDb is required to read change " + changeId);
+ }
boolean readOrWrite = read || args.migration.writeChanges();
if (!readOrWrite && !autoRebuild) {
loadDefaults();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java
index 61ebfae..3c7277a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java
@@ -327,16 +327,16 @@
private Timestamp getLatestTimestamp() {
Ordering<Timestamp> o = Ordering.natural().nullsFirst();
Timestamp ts = null;
- for (ChangeMessage cm : getChangeMessages()) {
+ for (ChangeMessage cm : filterChangeMessages()) {
ts = o.max(ts, cm.getWrittenOn());
}
for (PatchSet ps : getPatchSets()) {
ts = o.max(ts, ps.getCreatedOn());
}
- for (PatchSetApproval psa : getPatchSetApprovals()) {
+ for (PatchSetApproval psa : filterPatchSetApprovals().values()) {
ts = o.max(ts, psa.getGranted());
}
- for (PatchLineComment plc : getPatchLineComments()) {
+ for (PatchLineComment plc : filterPatchLineComments().values()) {
// Ignore draft comments, as they do not show up in the change meta graph.
if (plc.getStatus() != PatchLineComment.Status.DRAFT) {
ts = o.max(ts, plc.getWrittenOn());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
index 68be2c5..d0b0c54 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -52,6 +52,7 @@
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.git.RefCache;
import com.google.gerrit.server.git.RepoRefCache;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache;
@@ -95,6 +96,11 @@
+ String.format(fmt, args));
}
+ public static Change readOneReviewDbChange(ReviewDb db, Change.Id id)
+ throws OrmException {
+ return ReviewDbUtil.unwrapDb(db).changes().get(id);
+ }
+
@Singleton
public static class Factory {
private final Args args;
@@ -118,7 +124,7 @@
public ChangeNotes createChecked(ReviewDb db, Project.NameKey project,
Change.Id changeId) throws OrmException, NoSuchChangeException {
- Change change = ReviewDbUtil.unwrapDb(db).changes().get(changeId);
+ Change change = readOneReviewDbChange(db, changeId);
if (change == null || !change.getProject().equals(project)) {
throw new NoSuchChangeException(changeId);
}
@@ -142,7 +148,7 @@
private Change loadChangeFromDb(ReviewDb db, Project.NameKey project,
Change.Id changeId) throws OrmException {
- Change change = ReviewDbUtil.unwrapDb(db).changes().get(changeId);
+ Change change = readOneReviewDbChange(db, changeId);
checkArgument(project != null, "project is required");
checkNotNull(change,
"change %s not found in ReviewDb", changeId);
@@ -297,9 +303,8 @@
Project.NameKey project) throws OrmException, IOException {
Set<Change.Id> ids = scan(repo);
List<ChangeNotes> changeNotes = new ArrayList<>(ids.size());
- db = ReviewDbUtil.unwrapDb(db);
for (Change.Id id : ids) {
- Change change = db.changes().get(id);
+ Change change = readOneReviewDbChange(db, id);
if (change == null) {
log.warn("skipping change {} found in project {} " +
"but not in ReviewDb",
@@ -358,7 +363,7 @@
private ChangeNotes(Args args, Change change, boolean autoRebuild,
@Nullable RefCache refs) {
- super(args, change.getId(), autoRebuild);
+ super(args, change.getId(), PrimaryStorage.of(change), autoRebuild);
this.change = new Change(change);
this.refs = refs;
}
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 430103c..37fc9f9 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
@@ -60,6 +60,7 @@
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
@@ -73,6 +74,8 @@
import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.FooterKey;
import org.eclipse.jgit.util.RawParseUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.nio.charset.Charset;
@@ -91,8 +94,13 @@
import java.util.Optional;
import java.util.Set;
import java.util.TreeMap;
+import java.util.TreeSet;
+import java.util.function.Function;
class ChangeNotesParser {
+ private static final Logger log =
+ LoggerFactory.getLogger(ChangeNotesParser.class);
+
// Sentinel RevId indicating a mutable field on a patch set was parsed, but
// the parser does not yet know its commit SHA-1.
private static final RevId PARTIAL_PATCH_SET =
@@ -232,8 +240,8 @@
private Multimap<PatchSet.Id, PatchSetApproval> buildApprovals() {
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.
+ if (!patchSets.containsKey(a.getPatchSetId())) {
+ continue; // Patch set deleted or missing.
} else if (allPastReviewers.contains(a.getAccountId())
&& !reviewers.containsRow(a.getAccountId())) {
continue; // Reviewer was explicitly removed.
@@ -283,10 +291,6 @@
}
PatchSet.Id psId = parsePatchSetId(commit);
- if (currentPatchSetId == null || psId.get() > currentPatchSetId.get()) {
- currentPatchSetId = psId;
- }
-
PatchSetState psState = parsePatchSetState(commit);
if (psState != null) {
if (!patchSetStates.containsKey(psId)) {
@@ -874,18 +878,16 @@
}
}
- private void updatePatchSetStates() throws ConfigInvalidException {
- for (PatchSet ps : patchSets.values()) {
+ private void updatePatchSetStates() {
+ Set<PatchSet.Id> missing = new TreeSet<>(ReviewDbUtil.intKeyOrdering());
+ for (Iterator<PatchSet> it = patchSets.values().iterator();
+ it.hasNext();) {
+ PatchSet ps = it.next();
if (ps.getRevision().equals(PARTIAL_PATCH_SET)) {
- throw parseException("No %s found for patch set %s",
- FOOTER_COMMIT, ps.getPatchSetId());
+ missing.add(ps.getId());
+ it.remove();
}
}
- if (patchSetStates.isEmpty()) {
- return;
- }
-
- boolean deleted = false;
for (Map.Entry<PatchSet.Id, PatchSetState> e : patchSetStates.entrySet()) {
switch (e.getValue()) {
case PUBLISHED:
@@ -893,7 +895,6 @@
break;
case DELETED:
- deleted = true;
patchSets.remove(e.getKey());
break;
@@ -905,15 +906,11 @@
break;
}
}
- if (!deleted) {
- return;
- }
// 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().
+ // (or otherwise missing) 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.
NavigableSet<PatchSet.Id> all = patchSets.navigableKeySet();
if (!all.isEmpty()) {
currentPatchSetId = all.last();
@@ -922,19 +919,35 @@
}
changeMessagesByPatchSet.keys().retainAll(all);
- for (Iterator<ChangeMessage> it = allChangeMessages.iterator();
- it.hasNext();) {
- if (!all.contains(it.next().getPatchSetId())) {
+ int pruned = pruneEntitiesForMissingPatchSets(
+ allChangeMessages, ChangeMessage::getPatchSetId, missing);
+ pruned += pruneEntitiesForMissingPatchSets(
+ comments.values(), c -> new PatchSet.Id(id, c.key.patchSetId), missing);
+ pruned += pruneEntitiesForMissingPatchSets(
+ approvals.values(), PatchSetApproval::getPatchSetId, missing);
+
+ if (!missing.isEmpty()) {
+ log.warn(
+ "ignoring {} additional entities due to missing patch sets: {}",
+ pruned, missing);
+ }
+ }
+
+ private <T> int pruneEntitiesForMissingPatchSets(
+ Iterable<T> ents, Function<T, PatchSet.Id> psIdFunc,
+ Set<PatchSet.Id> missing) {
+ int pruned = 0;
+ for (Iterator<T> it = ents.iterator(); it.hasNext();) {
+ PatchSet.Id psId = psIdFunc.apply(it.next());
+ if (!patchSets.containsKey(psId)) {
+ pruned++;
+ missing.add(psId);
it.remove();
+ } else if (deletedPatchSets.contains(psId)) {
+ it.remove(); // Not an error we need to report, don't increment pruned.
}
}
- for (Iterator<Comment> it = comments.values().iterator();
- it.hasNext();) {
- PatchSet.Id psId = new PatchSet.Id(id, it.next().key.patchSetId);
- if (!all.contains(psId)) {
- it.remove();
- }
- }
+ return pruned;
}
private void checkMandatoryFooters() throws ConfigInvalidException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index e0f7920..d5abdee 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -31,6 +31,7 @@
import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
@@ -141,7 +142,10 @@
abstract Timestamp createdOn();
abstract Timestamp lastUpdatedOn();
abstract Account.Id owner();
- abstract String branch(); // Project not included.
+
+ // Project not included, as it's not stored anywhere in the meta ref.
+ abstract String branch();
+
@Nullable abstract PatchSet.Id currentPatchSetId();
abstract String subject();
@Nullable abstract String topic();
@@ -172,17 +176,35 @@
changeMessagesByPatchSet();
abstract ImmutableListMultimap<RevId, Comment> publishedComments();
+ Change newChange(Project.NameKey project) {
+ ChangeColumns c = checkNotNull(columns(), "columns are required");
+ Change change = new Change(
+ c.changeKey(),
+ changeId(),
+ c.owner(),
+ new Branch.NameKey(project, c.branch()),
+ c.createdOn());
+ copyNonConstructorColumnsTo(change);
+ change.setNoteDbState(NoteDbChangeState.NOTE_DB_PRIMARY_STATE);
+ return change;
+ }
+
void copyColumnsTo(Change change) {
- ChangeColumns c = checkNotNull(columns());
+ ChangeColumns c = checkNotNull(columns(), "columns are required");
+ change.setKey(c.changeKey());
+ change.setOwner(c.owner());
+ change.setDest(new Branch.NameKey(change.getProject(), c.branch()));
+ change.setCreatedOn(c.createdOn());
+ copyNonConstructorColumnsTo(change);
+ }
+
+ private void copyNonConstructorColumnsTo(Change change) {
+ ChangeColumns c = checkNotNull(columns(), "columns are required");
if (c.status() != null) {
change.setStatus(c.status());
}
- change.setKey(c.changeKey());
- change.setDest(new Branch.NameKey(change.getProject(), c.branch()));
change.setTopic(Strings.emptyToNull(c.topic()));
- change.setCreatedOn(c.createdOn());
change.setLastUpdatedOn(c.lastUpdatedOn());
- change.setOwner(c.owner());
change.setSubmissionId(c.submissionId());
change.setAssignee(c.assignee());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java
index 661112e..7ba90a2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java
@@ -31,6 +31,7 @@
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.RepoRefCache;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NoteDbUpdateManager.StagedResult;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder;
import com.google.gerrit.server.project.NoSuchChangeException;
@@ -85,7 +86,9 @@
Args args,
@Assisted Change.Id changeId,
@Assisted Account.Id author) {
- super(args, changeId, true);
+ // PrimaryStorage is unknown; this should only called by
+ // PatchLineCommentsUtil#draftByAuthor, which can live with this.
+ super(args, changeId, null, false);
this.change = null;
this.author = author;
this.rebuildResult = null;
@@ -97,7 +100,7 @@
Account.Id author,
boolean autoRebuild,
NoteDbUpdateManager.Result rebuildResult) {
- super(args, change.getId(), autoRebuild);
+ super(args, change.getId(), PrimaryStorage.of(change), autoRebuild);
this.change = change;
this.author = author;
this.rebuildResult = rebuildResult;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbChangeState.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbChangeState.java
index ad54f02..80f9eaf 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbChangeState.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbChangeState.java
@@ -16,26 +16,28 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
import static com.google.gerrit.reviewdb.client.RefNames.refsDraftComments;
-import static java.util.Comparator.comparing;
+import static com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage.NOTE_DB;
+import static com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage.REVIEW_DB;
+import static org.eclipse.jgit.lib.ObjectId.zeroId;
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Predicates;
import com.google.common.base.Splitter;
+import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.git.RefCache;
import org.eclipse.jgit.lib.ObjectId;
import java.io.IOException;
-import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -48,13 +50,37 @@
* Stored serialized in the {@code Change#noteDbState} field, and used to
* determine whether the state in NoteDb is out of date.
* <p>
- * Serialized in the form:
- * <pre>
- * [meta-sha],[account1]=[drafts-sha],[account2]=[drafts-sha]...
- * </pre>
+ * Serialized in one of the forms:
+ * <ul>
+ * <li>[meta-sha],[account1]=[drafts-sha],[account2]=[drafts-sha]...
+ * <li>R[meta-sha],[account1]=[drafts-sha],[account2]=[drafts-sha]...
+ * <li>N
+ * </ul>
+ *
* in numeric account ID order, with hex SHA-1s for human readability.
*/
public class NoteDbChangeState {
+ public static final String NOTE_DB_PRIMARY_STATE = "N";
+
+ public enum PrimaryStorage {
+ REVIEW_DB('R'),
+ NOTE_DB('N');
+
+ private final char code;
+
+ private PrimaryStorage(char code) {
+ this.code = code;
+ }
+
+ public static PrimaryStorage of(Change c) {
+ return of(NoteDbChangeState.parse(c));
+ }
+
+ public static PrimaryStorage of(NoteDbChangeState s) {
+ return s != null ? s.getPrimaryStorage() : REVIEW_DB;
+ }
+ }
+
@AutoValue
public abstract static class Delta {
static Delta create(Change.Id changeId, Optional<ObjectId> newChangeMetaId,
@@ -73,31 +99,89 @@
abstract ImmutableMap<Account.Id, ObjectId> newDraftIds();
}
+ @AutoValue
+ public abstract static class RefState {
+ @VisibleForTesting
+ public static RefState create(ObjectId changeMetaId,
+ Map<Account.Id, ObjectId> draftIds) {
+ return new AutoValue_NoteDbChangeState_RefState(
+ changeMetaId.copy(),
+ ImmutableMap.copyOf(
+ Maps.filterValues(draftIds, id -> !zeroId().equals(id))));
+ }
+
+ private static Optional<RefState> parse(Change.Id changeId,
+ List<String> parts) {
+ checkArgument(!parts.isEmpty(),
+ "missing state string for change %s", changeId);
+ ObjectId changeMetaId = ObjectId.fromString(parts.get(0));
+ Map<Account.Id, ObjectId> draftIds =
+ Maps.newHashMapWithExpectedSize(parts.size() - 1);
+ Splitter s = Splitter.on('=');
+ for (int i = 1; i < parts.size(); i++) {
+ String p = parts.get(i);
+ List<String> draftParts = s.splitToList(p);
+ checkArgument(draftParts.size() == 2,
+ "invalid draft state part for change %s: %s", changeId, p);
+ draftIds.put(Account.Id.parse(draftParts.get(0)),
+ ObjectId.fromString(draftParts.get(1)));
+ }
+ return Optional.of(create(changeMetaId, draftIds));
+ }
+
+ abstract ObjectId changeMetaId();
+ abstract ImmutableMap<Account.Id, ObjectId> draftIds();
+
+ @Override
+ public String toString() {
+ return appendTo(new StringBuilder()).toString();
+ }
+
+ StringBuilder appendTo(StringBuilder sb) {
+ sb.append(changeMetaId().name());
+ for (Account.Id id : ReviewDbUtil.intKeyOrdering()
+ .sortedCopy(draftIds().keySet())) {
+ sb.append(',')
+ .append(id.get())
+ .append('=')
+ .append(draftIds().get(id).name());
+ }
+ return sb;
+ }
+ }
+
public static NoteDbChangeState parse(Change c) {
- return parse(c.getId(), c.getNoteDbState());
+ return c != null ? parse(c.getId(), c.getNoteDbState()) : null;
}
@VisibleForTesting
public static NoteDbChangeState parse(Change.Id id, String str) {
- if (str == null) {
+ if (Strings.isNullOrEmpty(str)) {
+ // Return null rather than Optional as this is what goes in the field in
+ // ReviewDb.
return null;
}
List<String> parts = Splitter.on(',').splitToList(str);
- checkArgument(!parts.isEmpty(),
- "invalid state string for change %s: %s", id, str);
- ObjectId changeMetaId = ObjectId.fromString(parts.get(0));
- Map<Account.Id, ObjectId> draftIds =
- Maps.newHashMapWithExpectedSize(parts.size() - 1);
- Splitter s = Splitter.on('=');
- for (int i = 1; i < parts.size(); i++) {
- String p = parts.get(i);
- List<String> draftParts = s.splitToList(p);
- checkArgument(draftParts.size() == 2,
- "invalid draft state part for change %s: %s", id, p);
- draftIds.put(Account.Id.parse(draftParts.get(0)),
- ObjectId.fromString(draftParts.get(1)));
+
+ // Only valid NOTE_DB state is "N".
+ String first = parts.get(0);
+ if (parts.size() == 1 && first.charAt(0) == NOTE_DB.code) {
+ return new NoteDbChangeState(id, NOTE_DB, Optional.empty());
}
- return new NoteDbChangeState(id, changeMetaId, draftIds);
+
+ // Otherwise it must be REVIEW_DB, either "R,<RefState>" or just
+ // "<RefState>". Allow length > 0 for forward compatibility.
+ if (first.length() > 0) {
+ Optional<RefState> refState;
+ if (first.charAt(0) == REVIEW_DB.code) {
+ refState = RefState.parse(id, parts.subList(1, parts.size()));
+ } else {
+ refState = RefState.parse(id, parts);
+ }
+ return new NoteDbChangeState(id, REVIEW_DB, refState);
+ }
+ throw new IllegalArgumentException(
+ "invalid state string for change " + id + ": " + str);
}
public static NoteDbChangeState applyDelta(Change change, Delta delta) {
@@ -112,6 +196,10 @@
return null;
}
NoteDbChangeState oldState = parse(change.getId(), oldStr);
+ if (oldState != null && oldState.getPrimaryStorage() == NOTE_DB) {
+ // NOTE_DB state doesn't include RefState, so applying a delta is a no-op.
+ return oldState;
+ }
ObjectId changeMetaId;
if (delta.newChangeMetaId().isPresent()) {
@@ -121,12 +209,12 @@
return null;
}
} else {
- changeMetaId = oldState.changeMetaId;
+ changeMetaId = oldState.getChangeMetaId();
}
Map<Account.Id, ObjectId> draftIds = new HashMap<>();
if (oldState != null) {
- draftIds.putAll(oldState.draftIds);
+ draftIds.putAll(oldState.getDraftIds());
}
for (Map.Entry<Account.Id, ObjectId> e : delta.newDraftIds().entrySet()) {
if (e.getValue().equals(ObjectId.zeroId())) {
@@ -137,13 +225,20 @@
}
NoteDbChangeState state = new NoteDbChangeState(
- change.getId(), changeMetaId, draftIds);
+ change.getId(),
+ oldState != null
+ ? oldState.getPrimaryStorage()
+ : REVIEW_DB,
+ Optional.of(RefState.create(changeMetaId, draftIds)));
change.setNoteDbState(state.toString());
return state;
}
public static boolean isChangeUpToDate(@Nullable NoteDbChangeState state,
RefCache changeRepoRefs, Change.Id changeId) throws IOException {
+ if (PrimaryStorage.of(state) == NOTE_DB) {
+ return true; // Primary storage is NoteDb, up to date by definition.
+ }
if (state == null) {
return !changeRepoRefs.get(changeMetaRef(changeId)).isPresent();
}
@@ -153,6 +248,9 @@
public static boolean areDraftsUpToDate(@Nullable NoteDbChangeState state,
RefCache draftsRepoRefs, Change.Id changeId, Account.Id accountId)
throws IOException {
+ if (PrimaryStorage.of(state) == NOTE_DB) {
+ return true; // Primary storage is NoteDb, up to date by definition.
+ }
if (state == null) {
return !draftsRepoRefs.get(refsDraftComments(changeId, accountId))
.isPresent();
@@ -160,56 +258,74 @@
return state.areDraftsUpToDate(draftsRepoRefs, accountId);
}
- public static String toString(ObjectId changeMetaId,
- Map<Account.Id, ObjectId> draftIds) {
- List<Account.Id> accountIds = Lists.newArrayList(draftIds.keySet());
- Collections.sort(accountIds, comparing(Account.Id::get));
- StringBuilder sb = new StringBuilder(changeMetaId.name());
- for (Account.Id id : accountIds) {
- sb.append(',')
- .append(id.get())
- .append('=')
- .append(draftIds.get(id).name());
+ private final Change.Id changeId;
+ private final PrimaryStorage primaryStorage;
+ private final Optional<RefState> refState;
+
+ public NoteDbChangeState(
+ Change.Id changeId,
+ PrimaryStorage primaryStorage,
+ Optional<RefState> refState) {
+ this.changeId = checkNotNull(changeId);
+ this.primaryStorage = checkNotNull(primaryStorage);
+ this.refState = refState;
+
+ switch (primaryStorage) {
+ case REVIEW_DB:
+ checkArgument(
+ refState.isPresent(),
+ "expected RefState for change %s with primary storage %s",
+ changeId, primaryStorage);
+ break;
+ case NOTE_DB:
+ checkArgument(
+ !refState.isPresent(),
+ "expected no RefState for change %s with primary storage %s",
+ changeId, primaryStorage);
+ break;
+ default:
+ throw new IllegalStateException(
+ "invalid PrimaryStorage: " + primaryStorage);
}
- return sb.toString();
}
- private final Change.Id changeId;
- private final ObjectId changeMetaId;
- private final ImmutableMap<Account.Id, ObjectId> draftIds;
-
- public NoteDbChangeState(Change.Id changeId, ObjectId changeMetaId,
- Map<Account.Id, ObjectId> draftIds) {
- this.changeId = checkNotNull(changeId);
- this.changeMetaId = checkNotNull(changeMetaId);
- this.draftIds = ImmutableMap.copyOf(Maps.filterValues(
- draftIds, Predicates.not(Predicates.equalTo(ObjectId.zeroId()))));
+ public PrimaryStorage getPrimaryStorage() {
+ return primaryStorage;
}
public boolean isChangeUpToDate(RefCache changeRepoRefs) throws IOException {
+ if (primaryStorage == NOTE_DB) {
+ return true; // Primary storage is NoteDb, up to date by definition.
+ }
Optional<ObjectId> id = changeRepoRefs.get(changeMetaRef(changeId));
if (!id.isPresent()) {
- return changeMetaId.equals(ObjectId.zeroId());
+ return getChangeMetaId().equals(ObjectId.zeroId());
}
- return id.get().equals(changeMetaId);
+ return id.get().equals(getChangeMetaId());
}
public boolean areDraftsUpToDate(RefCache draftsRepoRefs, Account.Id accountId)
throws IOException {
+ if (primaryStorage == NOTE_DB) {
+ return true; // Primary storage is NoteDb, up to date by definition.
+ }
Optional<ObjectId> id =
draftsRepoRefs.get(refsDraftComments(changeId, accountId));
if (!id.isPresent()) {
- return !draftIds.containsKey(accountId);
+ return !getDraftIds().containsKey(accountId);
}
- return id.get().equals(draftIds.get(accountId));
+ return id.get().equals(getDraftIds().get(accountId));
}
public boolean isUpToDate(RefCache changeRepoRefs, RefCache draftsRepoRefs)
throws IOException {
+ if (primaryStorage == NOTE_DB) {
+ return true; // Primary storage is NoteDb, up to date by definition.
+ }
if (!isChangeUpToDate(changeRepoRefs)) {
return false;
}
- for (Account.Id accountId : draftIds.keySet()) {
+ for (Account.Id accountId : getDraftIds().keySet()) {
if (!areDraftsUpToDate(draftsRepoRefs, accountId)) {
return false;
}
@@ -224,16 +340,36 @@
@VisibleForTesting
public ObjectId getChangeMetaId() {
- return changeMetaId;
+ return refState().changeMetaId();
}
@VisibleForTesting
ImmutableMap<Account.Id, ObjectId> getDraftIds() {
- return draftIds;
+ return refState().draftIds();
+ }
+
+ @VisibleForTesting
+ Optional<RefState> getRefState() {
+ return refState;
+ }
+
+ private RefState refState() {
+ checkState(refState.isPresent(),
+ "state for %s has no RefState: %s", changeId, this);
+ return refState.get();
}
@Override
public String toString() {
- return toString(changeMetaId, draftIds);
+ switch (primaryStorage) {
+ case REVIEW_DB:
+ // Don't include enum field, just IDs (though parse would accept it).
+ return refState().toString();
+ case NOTE_DB:
+ return NOTE_DB_PRIMARY_STATE;
+ default:
+ throw new IllegalArgumentException(
+ "Unsupported PrimaryStorage: " + primaryStorage);
+ }
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
index 1f64bcc..1e5cb1e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
@@ -37,6 +37,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.InMemoryInserter;
import com.google.gerrit.server.git.InsertedObject;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gwtorm.server.OrmConcurrencyException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.assistedinject.Assisted;
@@ -489,6 +490,17 @@
}
}
+ public static class MismatchedStateException extends OrmException {
+ private static final long serialVersionUID = 1L;
+
+ private MismatchedStateException(Change.Id id, NoteDbChangeState expectedState) {
+ super(String.format(
+ "cannot apply NoteDb updates for change %s;"
+ + " change meta ref does not match %s",
+ id, expectedState.getChangeMetaId().name()));
+ }
+ }
+
private void checkExpectedState() throws OrmException, IOException {
if (!checkExpectedState) {
return;
@@ -513,15 +525,17 @@
// - We short-circuited before adding any commands that update this
// ref, and we won't stage a delta for this change either.
// Either way, it is safe to proceed here rather than throwing
- // OrmConcurrencyException.
+ // MismatchedStateException.
+ continue;
+ }
+
+ if (expectedState.getPrimaryStorage() == PrimaryStorage.NOTE_DB) {
+ // NoteDb is primary, no need to compare state to ReviewDb.
continue;
}
if (!expectedState.isChangeUpToDate(changeRepo.cmds.getRepoRefCache())) {
- throw new OrmConcurrencyException(String.format(
- "cannot apply NoteDb updates for change %s;"
- + " change meta ref does not match %s",
- u.getId(), expectedState.getChangeMetaId().name()));
+ throw new MismatchedStateException(u.getId(), expectedState);
}
}
@@ -529,7 +543,8 @@
ChangeDraftUpdate u = us.iterator().next();
NoteDbChangeState expectedState = NoteDbChangeState.parse(u.getChange());
- if (expectedState == null) {
+ if (expectedState == null
+ || expectedState.getPrimaryStorage() == PrimaryStorage.NOTE_DB) {
continue; // See above.
}
@@ -539,7 +554,8 @@
throw new OrmConcurrencyException(String.format(
"cannot apply NoteDb updates for change %s;"
+ " draft ref for account %s does not match %s",
- u.getId(), accountId, expectedState.getChangeMetaId().name()));
+ u.getId(), accountId,
+ expectedState.getChangeMetaId().name()));
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RobotCommentNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RobotCommentNotes.java
index 4a26d7e..f60fd2d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RobotCommentNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/RobotCommentNotes.java
@@ -22,6 +22,7 @@
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.client.RobotComment;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -47,7 +48,7 @@
RobotCommentNotes(
Args args,
@Assisted Change change) {
- super(args, change.getId(), false);
+ super(args, change.getId(), PrimaryStorage.of(change), false);
this.change = change;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
index b6e25a0..20524cf 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
@@ -54,8 +54,10 @@
import com.google.gerrit.server.notedb.ChangeBundleReader;
import com.google.gerrit.server.notedb.ChangeDraftUpdate;
import com.google.gerrit.server.notedb.ChangeNoteUtil;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NoteDbChangeState;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NoteDbUpdateManager;
import com.google.gerrit.server.notedb.NoteDbUpdateManager.OpenRepo;
import com.google.gerrit.server.notedb.NoteDbUpdateManager.Result;
@@ -184,7 +186,8 @@
public NoteDbUpdateManager stage(ReviewDb db, Change.Id changeId)
throws NoSuchChangeException, IOException, OrmException {
db = ReviewDbUtil.unwrapDb(db);
- Change change = db.changes().get(changeId);
+ Change change =
+ checkNoteDbState(ChangeNotes.readOneReviewDbChange(db, changeId));
if (change == null) {
throw new NoSuchChangeException(changeId);
}
@@ -200,7 +203,8 @@
NoteDbUpdateManager manager) throws NoSuchChangeException, OrmException,
IOException {
db = ReviewDbUtil.unwrapDb(db);
- Change change = db.changes().get(changeId);
+ Change change =
+ checkNoteDbState(ChangeNotes.readOneReviewDbChange(db, changeId));
if (change == null) {
throw new NoSuchChangeException(changeId);
}
@@ -253,6 +257,16 @@
return r;
}
+ private static Change checkNoteDbState(Change c) throws OrmException {
+ // Can only rebuild a change if its primary storage is ReviewDb.
+ NoteDbChangeState s = NoteDbChangeState.parse(c);
+ if (s != null && s.getPrimaryStorage() != PrimaryStorage.REVIEW_DB) {
+ throw new OrmException(String.format(
+ "cannot rebuild change " + c.getId() + " with state " + s));
+ }
+ return c;
+ }
+
@Override
public void buildUpdates(NoteDbUpdateManager manager, ChangeBundle bundle)
throws IOException, OrmException {
@@ -319,10 +333,11 @@
List<Event> msgEvents = parseChangeMessage(msg, change, noteDbChange);
if (msg.getPatchSetId() != null) {
PatchSetEvent pse = patchSetEvents.get(msg.getPatchSetId());
- if (pse != null) {
- for (Event e : msgEvents) {
- e.addDep(pse);
- }
+ if (pse == null) {
+ continue; // Ignore events for missing patch sets.
+ }
+ for (Event e : msgEvents) {
+ e.addDep(pse);
}
}
events.addAll(msgEvents);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
index 2da2031..893b2d4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.schema;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Stopwatch;
import com.google.common.collect.Lists;
import com.google.gerrit.reviewdb.client.CurrentSchemaVersion;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -30,6 +31,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.concurrent.TimeUnit;
/** A version of the database schema. */
public abstract class SchemaVersion {
@@ -142,11 +144,14 @@
private void migrateData(List<SchemaVersion> pending, UpdateUI ui,
CurrentSchemaVersion curr, ReviewDb db) throws OrmException, SQLException {
for (SchemaVersion v : pending) {
+ Stopwatch sw = Stopwatch.createStarted();
ui.message(String.format(
"Migrating data to schema %d ...",
v.getVersionNbr()));
v.migrateData(db, ui);
v.finish(curr, db);
+ ui.message(String.format("\t> Done (%.3f s)",
+ sw.elapsed(TimeUnit.MILLISECONDS) / 1000d));
}
}
diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/mail/CommentHtml.soy b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/CommentHtml.soy
index 3042287..6dc37b8 100644
--- a/gerrit-server/src/main/resources/com/google/gerrit/server/mail/CommentHtml.soy
+++ b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/CommentHtml.soy
@@ -54,7 +54,7 @@
{/if}
{if $coverLetter}
- {call .Pre}{param content: $coverLetter /}{/call}
+ <div style="white-space:pre-wrap">{$coverLetter}</div>
{/if}
<ul style="{$ulStyle}">
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/RawMailParserTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/RawMailParserTest.java
new file mode 100644
index 0000000..5ef077a
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/RawMailParserTest.java
@@ -0,0 +1,76 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.server.mail.receive.data.AttachmentMessage;
+import com.google.gerrit.server.mail.receive.data.Base64HeaderMessage;
+import com.google.gerrit.server.mail.receive.data.HtmlMimeMessage;
+import com.google.gerrit.server.mail.receive.data.NonUTF8Message;
+import com.google.gerrit.server.mail.receive.data.QuotedPrintableHeaderMessage;
+import com.google.gerrit.server.mail.receive.data.RawMailMessage;
+import com.google.gerrit.server.mail.receive.data.SimpleTextMessage;
+import com.google.gerrit.testutil.GerritBaseTests;
+
+import org.junit.Test;
+
+public class RawMailParserTest extends GerritBaseTests {
+ @Test
+ public void testParseEmail() throws Exception {
+ RawMailMessage[] messages = new RawMailMessage[] {
+ new SimpleTextMessage(),
+ new Base64HeaderMessage(),
+ new QuotedPrintableHeaderMessage(),
+ new HtmlMimeMessage(),
+ new AttachmentMessage(),
+ new NonUTF8Message(),
+ };
+ for (RawMailMessage rawMailMessage : messages) {
+ if (rawMailMessage.rawChars() != null) {
+ // Assert Character to Mail Parser
+ MailMessage parsedMailMessage =
+ RawMailParser.parse(rawMailMessage.rawChars());
+ assertMail(parsedMailMessage, rawMailMessage.expectedMailMessage());
+ }
+ if (rawMailMessage.raw() != null) {
+ // Assert String to Mail Parser
+ MailMessage parsedMailMessage = RawMailParser
+ .parse(rawMailMessage.raw());
+ assertMail(parsedMailMessage, rawMailMessage.expectedMailMessage());
+ }
+ }
+ }
+
+ /**
+ * This method makes it easier to debug failing tests by checking each
+ * property individual instead of calling equals as it will immediately
+ * reveal the property that diverges between the two objects.
+ * @param have MailMessage retrieved from the parser
+ * @param want MailMessage that would be expected
+ */
+ private void assertMail(MailMessage have, MailMessage want) {
+ assertThat(have.id()).isEqualTo(want.id());
+ assertThat(have.to()).isEqualTo(want.to());
+ assertThat(have.from()).isEqualTo(want.from());
+ assertThat(have.cc()).isEqualTo(want.cc());
+ assertThat(have.dateReceived().getMillis())
+ .isEqualTo(want.dateReceived().getMillis());
+ assertThat(have.additionalHeaders()).isEqualTo(want.additionalHeaders());
+ assertThat(have.subject()).isEqualTo(want.subject());
+ assertThat(have.textContent()).isEqualTo(want.textContent());
+ assertThat(have.htmlContent()).isEqualTo(want.htmlContent());
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/AttachmentMessage.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/AttachmentMessage.java
new file mode 100644
index 0000000..390209a
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/AttachmentMessage.java
@@ -0,0 +1,91 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive.data;
+
+
+import com.google.gerrit.server.mail.Address;
+import com.google.gerrit.server.mail.receive.MailMessage;
+
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+import org.junit.Ignore;
+
+/**
+ * Tests that all mime parts that are neither text/plain, nor text/html are
+ * dropped.
+ */
+@Ignore
+public class AttachmentMessage extends RawMailMessage {
+ private static String raw = "MIME-Version: 1.0\n" +
+ "Date: Tue, 25 Oct 2016 02:11:35 -0700\n" +
+ "Message-ID: <CAM7sg=3meaAVUxW3KXeJEVs8sv_ADw1BnvpcHHiYVR2TQQi__w" +
+ "@mail.gmail.com>\n" +
+ "Subject: Test Subject\n" +
+ "From: Patrick Hiesel <hiesel@google.com>\n" +
+ "To: Patrick Hiesel <hiesel@google.com>\n" +
+ "Content-Type: multipart/mixed; boundary=001a114e019a56962d054062708f\n" +
+ "\n" +
+ "--001a114e019a56962d054062708f\n" +
+ "Content-Type: multipart/alternative; boundary=001a114e019a5696250540" +
+ "62708d\n" +
+ "\n" +
+ "--001a114e019a569625054062708d\n" +
+ "Content-Type: text/plain; charset=UTF-8\n" +
+ "\n" +
+ "Contains unwanted attachment" +
+ "\n" +
+ "--001a114e019a569625054062708d\n" +
+ "Content-Type: text/html; charset=UTF-8\n" +
+ "\n" +
+ "<div dir=\"ltr\">Contains unwanted attachment</div>" +
+ "\n" +
+ "--001a114e019a569625054062708d--\n" +
+ "--001a114e019a56962d054062708f\n" +
+ "Content-Type: text/plain; charset=US-ASCII; name=\"test.txt\"\n" +
+ "Content-Disposition: attachment; filename=\"test.txt\"\n" +
+ "Content-Transfer-Encoding: base64\n" +
+ "X-Attachment-Id: f_iv264bt50\n" +
+ "\n" +
+ "VEVTVAo=\n" +
+ "--001a114e019a56962d054062708f--";
+
+ @Override
+ public String raw() {
+ return raw;
+ }
+
+ @Override
+ public int[] rawChars() {
+ return null;
+ }
+
+ @Override
+ public MailMessage expectedMailMessage() {
+ System.out.println("\uD83D\uDE1B test");
+ MailMessage.Builder expect = MailMessage.builder();
+ expect
+ .id("<CAM7sg=3meaAVUxW3KXeJEVs8sv_ADw1BnvpcHHiYVR2TQQi__w" +
+ "@mail.gmail.com>")
+ .from(new Address("Patrick Hiesel", "hiesel@google.com"))
+ .addTo(new Address("Patrick Hiesel", "hiesel@google.com"))
+ .textContent("Contains unwanted attachment")
+ .htmlContent("<div dir=\"ltr\">Contains unwanted attachment</div>")
+ .subject("Test Subject")
+ .addAdditionalHeader("MIME-Version: 1.0")
+ .dateReceived(
+ new DateTime(2016, 10, 25, 9, 11, 35, 0, DateTimeZone.UTC));
+ return expect.build();
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/Base64HeaderMessage.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/Base64HeaderMessage.java
new file mode 100644
index 0000000..5511e75
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/Base64HeaderMessage.java
@@ -0,0 +1,64 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive.data;
+
+import com.google.gerrit.server.mail.Address;
+import com.google.gerrit.server.mail.receive.MailMessage;
+
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+import org.junit.Ignore;
+
+/**
+ * Tests parsing a Base64 encoded subject.
+ */
+@Ignore
+public class Base64HeaderMessage extends RawMailMessage {
+ private static String textContent = "Some Text";
+ private static String raw = "" +
+ "Date: Tue, 25 Oct 2016 02:11:35 -0700\n" +
+ "Message-ID: <001a114da7ae26e2eb053fe0c29c@google.com>\n" +
+ "Subject: =?UTF-8?B?8J+YmyB0ZXN0?=\n" +
+ "From: \"Jonathan Nieder (Gerrit)\" <noreply-gerritcodereview-" +
+ "CtTy0igsBrnvL7dKoWEIEg@google.com>\n" +
+ "To: ekempin <ekempin@google.com>\n" +
+ "Content-Type: text/plain; charset=UTF-8; format=flowed; delsp=yes\n" +
+ "\n" + textContent;
+
+ @Override
+ public String raw() {
+ return raw;
+ }
+
+ @Override
+ public int[] rawChars() {
+ return null;
+ }
+
+ @Override
+ public MailMessage expectedMailMessage() {
+ MailMessage.Builder expect = MailMessage.builder();
+ expect
+ .id("<001a114da7ae26e2eb053fe0c29c@google.com>")
+ .from(new Address("Jonathan Nieder (Gerrit)",
+ "noreply-gerritcodereview-CtTy0igsBrnvL7dKoWEIEg@google.com"))
+ .addTo(new Address("ekempin","ekempin@google.com"))
+ .textContent(textContent)
+ .subject("\uD83D\uDE1B test")
+ .dateReceived(
+ new DateTime(2016, 10, 25, 9, 11, 35, 0, DateTimeZone.UTC));
+ return expect.build();
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/HtmlMimeMessage.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/HtmlMimeMessage.java
new file mode 100644
index 0000000..2ed096e
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/HtmlMimeMessage.java
@@ -0,0 +1,105 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive.data;
+
+import com.google.gerrit.server.mail.Address;
+import com.google.gerrit.server.mail.receive.MailMessage;
+
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+import org.junit.Ignore;
+
+/**
+ * Tests a message containing mime/alternative (text + html) content.
+ */
+@Ignore
+public class HtmlMimeMessage extends RawMailMessage {
+ private static String textContent = "Simple test";
+
+ // htmlContent is encoded in quoted-printable
+ private static String htmlContent = "<div dir=3D\"ltr\">Test <span style" +
+ "=3D\"background-color:rgb(255,255,0)\">Messa=\n" +
+ "ge</span> in <u>HTML=C2=A0</u><a href=3D\"https://en.wikipedia.org/" +
+ "wiki/%C3%=\n9Cmlaut_(band)\" class=3D\"gmail-mw-redirect\" title=3D\"" +
+ "=C3=9Cmlaut (band)\" st=\nyle=3D\"text-decoration:none;color:rgb(11," +
+ "0,128);background-image:none;backg=\nround-position:initial;background" +
+ "-size:initial;background-repeat:initial;ba=\nckground-origin:initial;" +
+ "background-clip:initial;font-family:sans-serif;font=\n" +
+ "-size:14px\">=C3=9C</a></div>";
+
+ private static String unencodedHtmlContent = "" +
+ "<div dir=\"ltr\">Test <span style=\"background-color:rgb(255,255,0)\">" +
+ "Message</span> in <u>HTML </u><a href=\"https://en.wikipedia.org/wiki/" +
+ "%C3%9Cmlaut_(band)\" class=\"gmail-mw-redirect\" title=\"Ümlaut " +
+ "(band)\" style=\"text-decoration:none;color:rgb(11,0,128);" +
+ "background-image:none;background-position:initial;background-size:" +
+ "initial;background-repeat:initial;background-origin:initial;background" +
+ "-clip:initial;font-family:sans-serif;font-size:14px\">Ü</a></div>";
+
+ private static String raw = "" +
+ "MIME-Version: 1.0\n" +
+ "Date: Tue, 25 Oct 2016 02:11:35 -0700\n" +
+ "Message-ID: <001a114cd8be55b4ab053face5cd@google.com>\n" +
+ "Subject: Change in gerrit[master]: Implement receiver class structure " +
+ "and bindings\n" +
+ "From: \"ekempin (Gerrit)\" <noreply-gerritcodereview-qUgXfQecoDLHwp0Ml" +
+ "dAzig@google.com>\n" +
+ "To: Patrick Hiesel <hiesel@google.com>\n" +
+ "Cc: ekempin <ekempin@google.com>\n" +
+ "Content-Type: multipart/alternative; boundary=001a114cd8b" +
+ "e55b486053face5ca\n" +
+ "\n" +
+ "--001a114cd8be55b486053face5ca\n" +
+ "Content-Type: text/plain; charset=UTF-8; format=flowed; delsp=yes\n" +
+ "\n" +
+ textContent +
+ "\n" +
+ "--001a114cd8be55b486053face5ca\n" +
+ "Content-Type: text/html; charset=UTF-8\n" +
+ "Content-Transfer-Encoding: quoted-printable\n" +
+ "\n" +
+ htmlContent +
+ "\n" +
+ "--001a114cd8be55b486053face5ca--";
+
+ @Override
+ public String raw() {
+ return raw;
+ }
+
+ @Override
+ public int[] rawChars() {
+ return null;
+ }
+
+ @Override
+ public MailMessage expectedMailMessage() {
+ MailMessage.Builder expect = MailMessage.builder();
+ expect
+ .id("<001a114cd8be55b4ab053face5cd@google.com>")
+ .from(new Address("ekempin (Gerrit)",
+ "noreply-gerritcodereview-qUgXfQecoDLHwp0MldAzig@google.com"))
+ .addCc(new Address("ekempin","ekempin@google.com"))
+ .addTo(new Address("Patrick Hiesel","hiesel@google.com"))
+ .textContent(textContent)
+ .htmlContent(unencodedHtmlContent)
+ .subject("Change in gerrit[master]: Implement " +
+ "receiver class structure and bindings")
+ .addAdditionalHeader("MIME-Version: 1.0")
+ .dateReceived(
+ new DateTime(2016, 10, 25, 9, 11, 35, 0, DateTimeZone.UTC));
+ return expect.build();
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/NonUTF8Message.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/NonUTF8Message.java
new file mode 100644
index 0000000..1472049
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/NonUTF8Message.java
@@ -0,0 +1,68 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.gerrit.server.mail.receive.data;
+
+import com.google.gerrit.server.mail.Address;
+import com.google.gerrit.server.mail.receive.MailMessage;
+
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+import org.junit.Ignore;
+
+/**
+ * Tests that non-UTF8 encodings are handled correctly.
+ */
+@Ignore
+public class NonUTF8Message extends RawMailMessage {
+ private static String textContent = "Some Text";
+ private static String raw = "" +
+ "Date: Tue, 25 Oct 2016 02:11:35 -0700\n" +
+ "Message-ID: <001a114da7ae26e2eb053fe0c29c@google.com>\n" +
+ "Subject: =?UTF-8?B?8J+YmyB0ZXN0?=\n" +
+ "From: \"Jonathan Nieder (Gerrit)\" <noreply-gerritcodereview-" +
+ "CtTy0igsBrnvL7dKoWEIEg@google.com>\n" +
+ "To: ekempin <ekempin@google.com>\n" +
+ "Content-Type: text/plain; charset=UTF-8; format=flowed; delsp=yes\n" +
+ "\n" + textContent;
+
+ @Override
+ public String raw() {
+ return null;
+ }
+
+ @Override
+ public int[] rawChars() {
+ int[] arr = new int[raw.length()];
+ int i = 0;
+ for (char c : raw.toCharArray()) {
+ arr[i++] = c;
+ }
+ return arr;
+ }
+
+ @Override
+ public MailMessage expectedMailMessage() {
+ MailMessage.Builder expect = MailMessage.builder();
+ expect
+ .id("<001a114da7ae26e2eb053fe0c29c@google.com>")
+ .from(new Address("Jonathan Nieder (Gerrit)",
+ "noreply-gerritcodereview-CtTy0igsBrnvL7dKoWEIEg@google.com"))
+ .addTo(new Address("ekempin","ekempin@google.com"))
+ .textContent(textContent)
+ .subject("\uD83D\uDE1B test")
+ .dateReceived(
+ new DateTime(2016, 10, 25, 9, 11, 35, 0, DateTimeZone.UTC));
+ return expect.build();
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/QuotedPrintableHeaderMessage.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/QuotedPrintableHeaderMessage.java
new file mode 100644
index 0000000..f694447
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/QuotedPrintableHeaderMessage.java
@@ -0,0 +1,65 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive.data;
+
+import com.google.gerrit.server.mail.Address;
+import com.google.gerrit.server.mail.receive.MailMessage;
+
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+import org.junit.Ignore;
+
+/**
+ * Tests parsing a quoted printable encoded subject
+ */
+@Ignore
+public class QuotedPrintableHeaderMessage extends RawMailMessage {
+ private static String textContent = "Some Text";
+ private static String raw = "" +
+ "Date: Tue, 25 Oct 2016 02:11:35 -0700\n" +
+ "Message-ID: <001a114da7ae26e2eb053fe0c29c@google.com>\n" +
+ "Subject: =?UTF-8?Q?=C3=A2me vulgaire?=\n" +
+ "From: \"Jonathan Nieder (Gerrit)\" <noreply-gerritcodereview-" +
+ "CtTy0igsBrnvL7dKoWEIEg@google.com>\n" +
+ "To: ekempin <ekempin@google.com>\n" +
+ "Content-Type: text/plain; charset=UTF-8; format=flowed; delsp=yes\n" +
+ "\n" + textContent;
+
+ @Override
+ public String raw() {
+ return raw;
+ }
+
+ @Override
+ public int[] rawChars() {
+ return null;
+ }
+
+ @Override
+ public MailMessage expectedMailMessage() {
+ System.out.println("\uD83D\uDE1B test");
+ MailMessage.Builder expect = MailMessage.builder();
+ expect
+ .id("<001a114da7ae26e2eb053fe0c29c@google.com>")
+ .from(new Address("Jonathan Nieder (Gerrit)",
+ "noreply-gerritcodereview-CtTy0igsBrnvL7dKoWEIEg@google.com"))
+ .addTo(new Address("ekempin","ekempin@google.com"))
+ .textContent(textContent)
+ .subject("âme vulgaire")
+ .dateReceived(
+ new DateTime(2016, 10, 25, 9, 11, 35, 0, DateTimeZone.UTC));
+ return expect.build();
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/RawMailMessage.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/RawMailMessage.java
new file mode 100644
index 0000000..8afa8cc
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/RawMailMessage.java
@@ -0,0 +1,31 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive.data;
+
+import com.google.gerrit.server.mail.receive.MailMessage;
+
+import org.junit.Ignore;
+
+/**
+ * Base class for all email parsing tests.
+ */
+@Ignore
+public abstract class RawMailMessage {
+ // Raw content to feed the parser
+ public abstract String raw();
+ public abstract int[] rawChars();
+ // Parsed representation for asserting the expected parser output
+ public abstract MailMessage expectedMailMessage();
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/SimpleTextMessage.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/SimpleTextMessage.java
new file mode 100644
index 0000000..179c514
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/data/SimpleTextMessage.java
@@ -0,0 +1,136 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.mail.receive.data;
+
+import com.google.gerrit.server.mail.Address;
+import com.google.gerrit.server.mail.receive.MailMessage;
+
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+import org.junit.Ignore;
+
+/**
+ * Tests parsing a simple text message with different headers.
+ */
+@Ignore
+public class SimpleTextMessage extends RawMailMessage {
+ private static String textContent = "" +
+ "Jonathan Nieder has posted comments on this change. ( \n" +
+ "https://gerrit-review.googlesource.com/90018 )\n" +
+ "\n" +
+ "Change subject: (Re)enable voting buttons for merged changes\n" +
+ "...........................................................\n" +
+ "\n" +
+ "\n" +
+ "Patch Set 2:\n" +
+ "\n" +
+ "This is producing NPEs server-side and 500s for the client. \n" +
+ "when I try to load this change:\n" +
+ "\n" +
+ " Error in GET /changes/90018/detail?O=10004\n" +
+ " com.google.gwtorm.OrmException: java.lang.NullPointerException\n" +
+ "\tat com.google.gerrit.change.ChangeJson.format(ChangeJson.java:303)\n" +
+ "\tat com.google.gerrit.change.ChangeJson.format(ChangeJson.java:285)\n" +
+ "\tat com.google.gerrit.change.ChangeJson.format(ChangeJson.java:263)\n" +
+ "\tat com.google.gerrit.change.GetChange.apply(GetChange.java:50)\n" +
+ "\tat com.google.gerrit.change.GetDetail.apply(GetDetail.java:51)\n" +
+ "\tat com.google.gerrit.change.GetDetail.apply(GetDetail.java:26)\n" +
+ "\tat \n" +
+ "com.google.gerrit.RestApiServlet.service(RestApiServlet.java:367)\n" +
+ "\tat javax.servlet.http.HttpServlet.service(HttpServlet.java:717)\n" +
+ "[...]\n" +
+ " Caused by: java.lang.NullPointerException\n" +
+ "\tat \n" +
+ "com.google.gerrit.ChangeJson.setLabelScores(ChangeJson.java:670)\n" +
+ "\tat \n" +
+ "com.google.gerrit.ChangeJson.labelsFor(ChangeJson.java:845)\n" +
+ "\tat \n" +
+ "com.google.gerrit.change.ChangeJson.labelsFor(ChangeJson.java:598)\n" +
+ "\tat \n" +
+ "com.google.gerrit.change.ChangeJson.toChange(ChangeJson.java:499)\n" +
+ "\tat com.google.gerrit.change.ChangeJson.format(ChangeJson.java:294)\n" +
+ "\t... 105 more\n" +
+ "-- \n" +
+ "To view, visit https://gerrit-review.googlesource.com/90018\n" +
+ "To unsubscribe, visit https://gerrit-review.googlesource.com\n" +
+ "\n" +
+ "Gerrit-MessageType: comment\n" +
+ "Gerrit-Change-Id: Iba501e00bee77be3bd0ced72f88fd04ba0accaed\n" +
+ "Gerrit-PatchSet: 2\n" +
+ "Gerrit-Project: gerrit\n" +
+ "Gerrit-Branch: master\n" +
+ "Gerrit-Owner: ekempin <ekempin@google.com>\n" +
+ "Gerrit-Reviewer: Dave Borowitz <dborowitz@google.com>\n" +
+ "Gerrit-Reviewer: Edwin Kempin <ekempin@google.com>\n" +
+ "Gerrit-Reviewer: GerritForge CI <gerritforge@gmail.com>\n" +
+ "Gerrit-Reviewer: Jonathan Nieder <jrn@google.com>\n" +
+ "Gerrit-Reviewer: Patrick Hiesel <hiesel@google.com>\n" +
+ "Gerrit-Reviewer: ekempin <ekempin@google.com>\n" +
+ "Gerrit-HasComments: No";
+
+ private static String raw = "" +
+ "Authentication-Results: mx.google.com; dkim=pass header.i=" +
+ "@google.com;\n" +
+ "Date: Tue, 25 Oct 2016 02:11:35 -0700\n" +
+ "In-Reply-To: <gerrit.1477487889000.Iba501e00bee77be3bd0ced" +
+ "72f88fd04ba0accaed@gerrit-review.googlesource.com>\n" +
+ "References: <gerrit.1477487889000.Iba501e00bee77be3bd0ced72f8" +
+ "8fd04ba0accaed@gerrit-review.googlesource.com>\n" +
+ "Message-ID: <001a114da7ae26e2eb053fe0c29c@google.com>\n" +
+ "Subject: Change in gerrit[master]: (Re)enable voting buttons for " +
+ "merged changes\n" +
+ "From: \"Jonathan Nieder (Gerrit)\" <noreply-gerritcodereview-CtTy0" +
+ "igsBrnvL7dKoWEIEg@google.com>\n" +
+ "To: ekempin <ekempin@google.com>\n" +
+ "Cc: Dave Borowitz <dborowitz@google.com>, Jonathan Nieder " +
+ "<jrn@google.com>, Patrick Hiesel <hiesel@google.com>\n" +
+ "Content-Type: text/plain; charset=UTF-8; format=flowed; delsp=yes\n" +
+ "\n" + textContent;
+
+ @Override
+ public String raw() {
+ return raw;
+ }
+
+ @Override
+ public int[] rawChars() {
+ return null;
+ }
+
+ @Override
+ public MailMessage expectedMailMessage() {
+ MailMessage.Builder expect = MailMessage.builder();
+ expect
+ .id("<001a114da7ae26e2eb053fe0c29c@google.com>")
+ .from(new Address("Jonathan Nieder (Gerrit)",
+ "noreply-gerritcodereview-CtTy0igsBrnvL7dKoWEIEg@google.com"))
+ .addTo(new Address("ekempin","ekempin@google.com"))
+ .addCc(new Address("Dave Borowitz", "dborowitz@google.com"))
+ .addCc(new Address("Jonathan Nieder", "jrn@google.com"))
+ .addCc(new Address("Patrick Hiesel", "hiesel@google.com"))
+ .textContent(textContent)
+ .subject("Change in gerrit[master]: (Re)enable voting"
+ + " buttons for merged changes")
+ .dateReceived(
+ new DateTime(2016, 10, 25, 9, 11, 35, 0, DateTimeZone.UTC))
+ .addAdditionalHeader("Authentication-Results: mx.google.com; " +
+ "dkim=pass header.i=@google.com;")
+ .addAdditionalHeader("In-Reply-To: <gerrit.1477487889000.Iba501e00bee" +
+ "77be3bd0ced72f88fd04ba0accaed@gerrit-review.googlesource.com>")
+ .addAdditionalHeader("References: <gerrit.1477487889000.Iba501e00bee" +
+ "77be3bd0ced72f88fd04ba0accaed@gerrit-review.googlesource.com>");
+ return expect.build();
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java
index 97bf864..faa3105 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java
@@ -328,6 +328,10 @@
throws Exception {
Change c1 = TestChanges.newChange(
new Project.NameKey("project"), new Account.Id(100));
+ PatchSet ps = new PatchSet(c1.currentPatchSetId());
+ ps.setRevision(new RevId("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"));
+ ps.setUploader(accountId);
+ ps.setCreatedOn(TimeUtil.nowTs());
PatchSetApproval a = new PatchSetApproval(
new PatchSetApproval.Key(
c1.currentPatchSetId(), accountId, new LabelId("Code-Review")),
@@ -338,16 +342,16 @@
c2.setLastUpdatedOn(a.getGranted());
// Both ReviewDb, exact match required.
- ChangeBundle b1 = new ChangeBundle(c1, messages(), patchSets(),
+ ChangeBundle b1 = new ChangeBundle(c1, messages(), patchSets(ps),
approvals(a), comments(), reviewers(), REVIEW_DB);
- ChangeBundle b2 = new ChangeBundle(c2, messages(), patchSets(),
+ ChangeBundle b2 = new ChangeBundle(c2, messages(), patchSets(ps),
approvals(a), comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"effective last updated time differs for Change.Id " + c1.getId() + ":"
- + " {2009-09-30 17:00:00.0} != {2009-09-30 17:00:06.0}");
+ + " {2009-09-30 17:00:00.0} != {2009-09-30 17:00:12.0}");
// NoteDb allows latest timestamp from all entities in bundle.
- b2 = new ChangeBundle(c2, messages(), patchSets(),
+ b2 = new ChangeBundle(c2, messages(), patchSets(ps),
approvals(a), comments(), reviewers(), NOTE_DB);
assertNoDiffs(b1, b2);
}
@@ -356,6 +360,10 @@
public void diffChangesIgnoresChangeTimestampIfAnyOtherEntitiesExist() {
Change c1 = TestChanges.newChange(
new Project.NameKey("project"), new Account.Id(100));
+ PatchSet ps = new PatchSet(c1.currentPatchSetId());
+ ps.setRevision(new RevId("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"));
+ ps.setUploader(accountId);
+ ps.setCreatedOn(TimeUtil.nowTs());
PatchSetApproval a = new PatchSetApproval(
new PatchSetApproval.Key(
c1.currentPatchSetId(), accountId, new LabelId("Code-Review")),
@@ -368,9 +376,9 @@
// ReviewDb has later lastUpdatedOn timestamp than NoteDb, allowed since
// NoteDb matches the latest timestamp of a non-Change entity.
- ChangeBundle b1 = new ChangeBundle(c2, messages(), patchSets(),
+ ChangeBundle b1 = new ChangeBundle(c2, messages(), patchSets(ps),
approvals(a), comments(), reviewers(), REVIEW_DB);
- ChangeBundle b2 = new ChangeBundle(c1, messages(), patchSets(),
+ ChangeBundle b2 = new ChangeBundle(c1, messages(), patchSets(ps),
approvals(a), comments(), reviewers(), NOTE_DB);
assertThat(b1.getChange().getLastUpdatedOn())
.isGreaterThan(b2.getChange().getLastUpdatedOn());
@@ -384,7 +392,7 @@
assertDiffs(b1, b2,
"effective last updated time differs for Change.Id " + c1.getId()
+ " in NoteDb vs. ReviewDb:"
- + " {2009-09-30 17:00:06.0} != {2009-09-30 17:00:12.0}");
+ + " {2009-09-30 17:00:12.0} != {2009-09-30 17:00:18.0}");
}
@Test
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 92ba426..29c11ff 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
@@ -19,7 +19,6 @@
import static com.google.gerrit.reviewdb.client.RefNames.refsDraftComments;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
-import static com.google.gerrit.testutil.TestChanges.incrementPatchSet;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
import static org.junit.Assert.fail;
@@ -251,7 +250,7 @@
assertThat(psa2.getAccountId().get()).isEqualTo(1);
assertThat(psa2.getLabel()).isEqualTo("Code-Review");
assertThat(psa2.getValue()).isEqualTo((short) +1);
- assertThat(psa2.getGranted()).isEqualTo(truncate(after(c, 3000)));
+ assertThat(psa2.getGranted()).isEqualTo(truncate(after(c, 4000)));
}
@Test
@@ -873,10 +872,6 @@
assertThat(ts4).isGreaterThan(ts3);
incrementPatchSet(c);
- RevCommit commit = tr.commit().message("PS2").create();
- update = newUpdate(c, changeOwner);
- update.setCommit(rw, commit);
- update.commit();
Timestamp ts5 = newNotes(c).getChange().getLastUpdatedOn();
assertThat(ts5).isGreaterThan(ts4);
@@ -983,11 +978,7 @@
assertThat(ps1.getUploader()).isEqualTo(changeOwner.getAccountId());
// ps2 by other user
- incrementPatchSet(c);
- RevCommit commit = tr.commit().message("PS2").create();
- ChangeUpdate update = newUpdate(c, otherUser);
- update.setCommit(rw, commit);
- update.commit();
+ RevCommit commit = incrementPatchSet(c, otherUser);
notes = newNotes(c);
PatchSet ps2 = notes.getCurrentPatchSet();
assertThat(ps2.getId()).isEqualTo(new PatchSet.Id(c.getId(), 2));
@@ -998,10 +989,11 @@
assertThat(ps2.getRevision().get()).isNotEqualTo(ps1.getRevision());
assertThat(ps2.getRevision().get()).isEqualTo(commit.name());
assertThat(ps2.getUploader()).isEqualTo(otherUser.getAccountId());
- assertThat(ps2.getCreatedOn()).isEqualTo(update.getWhen());
+ assertThat(ps2.getCreatedOn())
+ .isEqualTo(notes.getChange().getLastUpdatedOn());
// comment on ps1, current patch set is still ps2
- update = newUpdate(c, changeOwner);
+ ChangeUpdate update = newUpdate(c, changeOwner);
update.setPatchSetId(ps1.getId());
update.setChangeMessage("Comment on old patch set.");
update.commit();
@@ -1014,8 +1006,7 @@
Change c = newChange();
PatchSet.Id psId1 = c.currentPatchSetId();
- // ps2
- incrementPatchSet(c);
+ incrementCurrentPatchSetFieldOnly(c);
PatchSet.Id psId2 = c.currentPatchSetId();
RevCommit commit = tr.commit().message("PS2").create();
ChangeUpdate update = newUpdate(c, changeOwner);
@@ -1074,8 +1065,7 @@
assertThat(notes.getPatchSets().get(psId1).getGroups())
.containsExactly("a", "b").inOrder();
- // ps2
- incrementPatchSet(c);
+ incrementCurrentPatchSetFieldOnly(c);
PatchSet.Id psId2 = c.currentPatchSetId();
update = newUpdate(c, changeOwner);
update.setCommit(rw, tr.commit().message("PS2").create());
@@ -1101,7 +1091,7 @@
// ps2 with push cert
Change c = newChange();
PatchSet.Id psId1 = c.currentPatchSetId();
- incrementPatchSet(c);
+ incrementCurrentPatchSetFieldOnly(c);
PatchSet.Id psId2 = c.currentPatchSetId();
ChangeUpdate update = newUpdate(c, changeOwner);
update.setPatchSetId(psId2);
@@ -1656,6 +1646,9 @@
public void patchLineCommentNotesFormatMultiplePatchSetsSameRevId()
throws Exception {
Change c = newChange();
+ PatchSet.Id psId1 = c.currentPatchSetId();
+ incrementPatchSet(c);
+ PatchSet.Id psId2 = c.currentPatchSetId();
String uuid1 = "uuid1";
String uuid2 = "uuid2";
String uuid3 = "uuid3";
@@ -1667,9 +1660,6 @@
Timestamp time = TimeUtil.nowTs();
RevId revId = new RevId("abcd1234abcd1234abcd1234abcd1234abcd1234");
- PatchSet.Id psId1 = c.currentPatchSetId();
- PatchSet.Id psId2 = new PatchSet.Id(c.getId(), psId1.get() + 1);
-
Comment comment1 =
newComment(psId1, "file1", uuid1, range1, range1.getEndLine(),
otherUser, null, time, message1, (short) 0, revId.get());
@@ -2497,6 +2487,35 @@
assertThat(msg.getRealAuthor()).isEqualTo(changeOwner.getAccountId());
}
+ @Test
+ public void ignoreEntitiesBeyondCurrentPatchSet() throws Exception {
+ Change c = newChange();
+ ChangeNotes notes = newNotes(c);
+ int numMessages = notes.getChangeMessages().size();
+ int numPatchSets = notes.getPatchSets().size();
+ int numApprovals = notes.getApprovals().size();
+ int numComments = notes.getComments().size();
+
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.setPatchSetId(
+ new PatchSet.Id(c.getId(), c.currentPatchSetId().get() + 1));
+ update.setChangeMessage("Should be ignored");
+ update.putApproval("Code-Review", (short) 2);
+ CommentRange range = new CommentRange(1, 1, 2, 1);
+ Comment comment = newComment(update.getPatchSetId(), "filename",
+ "uuid", range, range.getEndLine(), changeOwner, null,
+ new Timestamp(update.getWhen().getTime()), "comment", (short) 1,
+ "abcd1234abcd1234abcd1234abcd1234abcd1234");
+ update.putComment(Status.PUBLISHED, comment);
+ update.commit();
+
+ notes = newNotes(c);
+ assertThat(notes.getChangeMessages()).hasSize(numMessages);
+ assertThat(notes.getPatchSets()).hasSize(numPatchSets);
+ assertThat(notes.getApprovals()).hasSize(numApprovals);
+ assertThat(notes.getComments()).hasSize(numComments);
+ }
+
private boolean testJson() {
return noteUtil.getWriteJson();
}
@@ -2529,4 +2548,24 @@
.isNotNull();
assertThat(cause.getMessage()).isEqualTo(expectedMsg);
}
+
+ private void incrementCurrentPatchSetFieldOnly(Change c) {
+ TestChanges.incrementPatchSet(c);
+ }
+
+ private RevCommit incrementPatchSet(Change c) throws Exception {
+ return incrementPatchSet(c, userFactory.create(c.getOwner()));
+ }
+
+ private RevCommit incrementPatchSet(Change c, IdentifiedUser user)
+ throws Exception {
+ incrementCurrentPatchSetFieldOnly(c);
+ RevCommit commit = tr.commit()
+ .message("PS" + c.currentPatchSetId().get())
+ .create();
+ ChangeUpdate update = newUpdate(c, user);
+ update.setCommit(rw, commit);
+ update.commit();
+ return tr.parseBody(commit);
+ }
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/NoteDbChangeStateTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/NoteDbChangeStateTest.java
index f2bf2be..e3613e3 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/NoteDbChangeStateTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/NoteDbChangeStateTest.java
@@ -18,6 +18,8 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.server.notedb.NoteDbChangeState.applyDelta;
import static com.google.gerrit.server.notedb.NoteDbChangeState.parse;
+import static com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage.NOTE_DB;
+import static com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage.REVIEW_DB;
import static org.eclipse.jgit.lib.ObjectId.zeroId;
import com.google.common.collect.ImmutableMap;
@@ -48,30 +50,44 @@
ObjectId.fromString("badc0feebadc0feebadc0feebadc0feebadc0fee");
@Test
- public void parseWithoutDrafts() {
+ public void parseReviewDbWithoutDrafts() {
NoteDbChangeState state = parse(new Change.Id(1), SHA1.name());
-
+ assertThat(state.getPrimaryStorage()).isEqualTo(REVIEW_DB);
assertThat(state.getChangeId()).isEqualTo(new Change.Id(1));
assertThat(state.getChangeMetaId()).isEqualTo(SHA1);
assertThat(state.getDraftIds()).isEmpty();
+ assertThat(state.toString()).isEqualTo(SHA1.name());
+ state = parse(new Change.Id(1), "R," + SHA1.name());
+ assertThat(state.getPrimaryStorage()).isEqualTo(REVIEW_DB);
+ assertThat(state.getChangeId()).isEqualTo(new Change.Id(1));
+ assertThat(state.getChangeMetaId()).isEqualTo(SHA1);
+ assertThat(state.getDraftIds()).isEmpty();
assertThat(state.toString()).isEqualTo(SHA1.name());
}
@Test
- public void parseWithDrafts() {
- NoteDbChangeState state = parse(
- new Change.Id(1),
- SHA1.name() + ",2003=" + SHA2.name() + ",1001=" + SHA3.name());
-
+ public void parseReviewDbWithDrafts() {
+ String str = SHA1.name() + ",2003=" + SHA2.name() + ",1001=" + SHA3.name();
+ String expected =
+ SHA1.name() + ",1001=" + SHA3.name() + ",2003=" + SHA2.name();
+ NoteDbChangeState state = parse(new Change.Id(1), str);
+ assertThat(state.getPrimaryStorage()).isEqualTo(REVIEW_DB);
assertThat(state.getChangeId()).isEqualTo(new Change.Id(1));
assertThat(state.getChangeMetaId()).isEqualTo(SHA1);
assertThat(state.getDraftIds()).containsExactly(
new Account.Id(1001), SHA3,
new Account.Id(2003), SHA2);
+ assertThat(state.toString()).isEqualTo(expected);
- assertThat(state.toString()).isEqualTo(
- SHA1.name() + ",1001=" + SHA3.name() + ",2003=" + SHA2.name());
+ state = parse(new Change.Id(1), "R," + str);
+ assertThat(state.getPrimaryStorage()).isEqualTo(REVIEW_DB);
+ assertThat(state.getChangeId()).isEqualTo(new Change.Id(1));
+ assertThat(state.getChangeMetaId()).isEqualTo(SHA1);
+ assertThat(state.getDraftIds()).containsExactly(
+ new Account.Id(1001), SHA3,
+ new Account.Id(2003), SHA2);
+ assertThat(state.toString()).isEqualTo(expected);
}
@Test
@@ -127,6 +143,27 @@
SHA3.name() + ",1001=" + SHA2.name());
}
+ @Test
+ public void parseNoteDbPrimary() {
+ NoteDbChangeState state = parse(new Change.Id(1), "N");
+ assertThat(state.getPrimaryStorage()).isEqualTo(NOTE_DB);
+ assertThat(state.getRefState().isPresent()).isFalse();
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void parseInvalidPrimaryStorage() {
+ parse(new Change.Id(1), "X");
+ }
+
+ @Test
+ public void applyDeltaToNoteDbPrimaryIsNoOp() {
+ Change c = newChange();
+ c.setNoteDbState("N");
+ applyDelta(c, Delta.create(c.getId(), metaId(SHA1),
+ drafts(new Account.Id(1001), SHA2)));
+ assertThat(c.getNoteDbState()).isEqualTo("N");
+ }
+
private static Change newChange() {
return TestChanges.newChange(
new Project.NameKey("project"), new Account.Id(12345));
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 1941fd4..3e3ec13 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -94,6 +94,7 @@
import org.junit.Ignore;
import org.junit.Test;
+import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
@@ -970,15 +971,22 @@
long thirtyHoursInMs = MILLISECONDS.convert(30, HOURS);
resetTimeWithClockStep(thirtyHoursInMs, MILLISECONDS);
TestRepository<Repo> repo = createProject("repo");
- Change change1 = insert(repo, newChange(repo));
- Change change2 = insert(repo, newChange(repo));
- // Queried by AgePredicate constructor.
+ long startMs = TestTimeUtil.START.getMillis();
+ Change change1 =
+ insert(repo, newChange(repo), null, new Timestamp(startMs));
+ Change change2 = insert(
+ repo, newChange(repo), null,
+ new Timestamp(startMs + thirtyHoursInMs));
+
+ // Stop time so age queries use the same endpoint.
TestTimeUtil.setClockStep(0, MILLISECONDS);
- long now = TimeUtil.nowMs();
+ TestTimeUtil.setClock(new Timestamp(startMs + 2 * thirtyHoursInMs));
+ long nowMs = TimeUtil.nowMs();
+
assertThat(lastUpdatedMs(change2) - lastUpdatedMs(change1))
.isEqualTo(thirtyHoursInMs);
- assertThat(now - lastUpdatedMs(change2)).isEqualTo(thirtyHoursInMs);
- assertThat(TimeUtil.nowMs()).isEqualTo(now);
+ assertThat(nowMs - lastUpdatedMs(change2)).isEqualTo(thirtyHoursInMs);
+ assertThat(TimeUtil.nowMs()).isEqualTo(nowMs);
assertQuery("-age:1d");
assertQuery("-age:" + (30 * 60 - 1) + "m");
@@ -991,10 +999,14 @@
@Test
public void byBefore() throws Exception {
- resetTimeWithClockStep(30, HOURS);
+ long thirtyHoursInMs = MILLISECONDS.convert(30, HOURS);
+ resetTimeWithClockStep(thirtyHoursInMs, MILLISECONDS);
TestRepository<Repo> repo = createProject("repo");
- Change change1 = insert(repo, newChange(repo));
- Change change2 = insert(repo, newChange(repo));
+ long startMs = TestTimeUtil.START.getMillis();
+ Change change1 =
+ insert(repo, newChange(repo), null, new Timestamp(startMs));
+ Change change2 = insert(
+ repo, newChange(repo), null, new Timestamp(startMs + thirtyHoursInMs));
TestTimeUtil.setClockStep(0, MILLISECONDS);
assertQuery("before:2009-09-29");
@@ -1011,10 +1023,14 @@
@Test
public void byAfter() throws Exception {
- resetTimeWithClockStep(30, HOURS);
+ long thirtyHoursInMs = MILLISECONDS.convert(30, HOURS);
+ resetTimeWithClockStep(thirtyHoursInMs, MILLISECONDS);
TestRepository<Repo> repo = createProject("repo");
- Change change1 = insert(repo, newChange(repo));
- Change change2 = insert(repo, newChange(repo));
+ long startMs = TestTimeUtil.START.getMillis();
+ Change change1 =
+ insert(repo, newChange(repo), null, new Timestamp(startMs));
+ Change change2 = insert(
+ repo, newChange(repo), null, new Timestamp(startMs + thirtyHoursInMs));
TestTimeUtil.setClockStep(0, MILLISECONDS);
assertQuery("after:2009-10-03");
@@ -1607,17 +1623,21 @@
}
protected Change insert(TestRepository<Repo> repo, ChangeInserter ins) throws Exception {
- return insert(repo, ins, null);
+ return insert(repo, ins, null, TimeUtil.nowTs());
}
protected Change insert(TestRepository<Repo> repo, ChangeInserter ins,
@Nullable Account.Id owner) throws Exception {
+ return insert(repo, ins, owner, TimeUtil.nowTs());
+ }
+
+ protected Change insert(TestRepository<Repo> repo, ChangeInserter ins,
+ @Nullable Account.Id owner, Timestamp createdOn) throws Exception {
Project.NameKey project = new Project.NameKey(
repo.getRepository().getDescription().getRepositoryName());
Account.Id ownerId = owner != null ? owner : userId;
IdentifiedUser user = userFactory.create(ownerId);
- try (BatchUpdate bu =
- updateFactory.create(db, project, user, TimeUtil.nowTs())) {
+ try (BatchUpdate bu = updateFactory.create(db, project, user, createdOn)) {
bu.insertChange(ins);
bu.execute();
return ins.getChange();
diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestTimeUtil.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestTimeUtil.java
index efb2b19..32f0af8 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestTimeUtil.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestTimeUtil.java
@@ -28,6 +28,9 @@
/** Static utility methods for dealing with dates and times in tests. */
public class TestTimeUtil {
+ public static final DateTime START =
+ new DateTime(2009, 9, 30, 17, 0, 0, DateTimeZone.forOffsetHours(-4));
+
private static Long clockStepMs;
private static AtomicLong clockMs;
@@ -42,9 +45,7 @@
public static synchronized void resetWithClockStep(
long clockStep, TimeUnit clockStepUnit) {
// Set an arbitrary start point so tests are more repeatable.
- clockMs = new AtomicLong(
- new DateTime(2009, 9, 30, 17, 0, 0, DateTimeZone.forOffsetHours(-4))
- .getMillis());
+ clockMs = new AtomicLong(START.getMillis());
setClockStep(clockStep, clockStepUnit);
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetMembersCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetMembersCommand.java
index f654f6a..893c8f2 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetMembersCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetMembersCommand.java
@@ -19,6 +19,7 @@
import com.google.common.base.MoreObjects;
import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -83,26 +84,30 @@
@Override
protected void run() throws UnloggedFailure, Failure, Exception {
- for (AccountGroup.UUID groupUuid : groups) {
- GroupResource resource =
- groupsCollection.parse(TopLevelResource.INSTANCE,
- IdString.fromUrl(groupUuid.get()));
- if (!accountsToRemove.isEmpty()) {
- deleteMembers.apply(resource, fromMembers(accountsToRemove));
- reportMembersAction("removed from", resource, accountsToRemove);
+ try {
+ for (AccountGroup.UUID groupUuid : groups) {
+ GroupResource resource =
+ groupsCollection.parse(TopLevelResource.INSTANCE,
+ IdString.fromUrl(groupUuid.get()));
+ if (!accountsToRemove.isEmpty()) {
+ deleteMembers.apply(resource, fromMembers(accountsToRemove));
+ reportMembersAction("removed from", resource, accountsToRemove);
+ }
+ if (!groupsToRemove.isEmpty()) {
+ deleteIncludedGroups.apply(resource, fromGroups(groupsToRemove));
+ reportGroupsAction("excluded from", resource, groupsToRemove);
+ }
+ if (!accountsToAdd.isEmpty()) {
+ addMembers.apply(resource, fromMembers(accountsToAdd));
+ reportMembersAction("added to", resource, accountsToAdd);
+ }
+ if (!groupsToInclude.isEmpty()) {
+ addIncludedGroups.apply(resource, fromGroups(groupsToInclude));
+ reportGroupsAction("included to", resource, groupsToInclude);
+ }
}
- if (!groupsToRemove.isEmpty()) {
- deleteIncludedGroups.apply(resource, fromGroups(groupsToRemove));
- reportGroupsAction("excluded from", resource, groupsToRemove);
- }
- if (!accountsToAdd.isEmpty()) {
- addMembers.apply(resource, fromMembers(accountsToAdd));
- reportMembersAction("added to", resource, accountsToAdd);
- }
- if (!groupsToInclude.isEmpty()) {
- addIncludedGroups.apply(resource, fromGroups(groupsToInclude));
- reportGroupsAction("included to", resource, groupsToInclude);
- }
+ } catch (RestApiException e) {
+ throw die(e.getMessage());
}
}
diff --git a/gerrit-util-http/BUILD b/gerrit-util-http/BUILD
index 0e3ac0e..c5d72ba 100644
--- a/gerrit-util-http/BUILD
+++ b/gerrit-util-http/BUILD
@@ -20,6 +20,7 @@
'//lib/jgit/org.eclipse.jgit:jgit',
],
visibility = ['//visibility:public'],
+ testonly = 1,
)
junit_tests(
diff --git a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
index 25aa5bc..6c46acd 100644
--- a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
+++ b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
@@ -53,6 +53,7 @@
import com.google.gerrit.server.index.IndexModule;
import com.google.gerrit.server.index.IndexModule.IndexType;
import com.google.gerrit.server.mail.SignedTokenEmailTokenVerifier;
+import com.google.gerrit.server.mail.receive.MailReceiver;
import com.google.gerrit.server.mail.send.SmtpEmailSender;
import com.google.gerrit.server.mime.MimeUtil2Module;
import com.google.gerrit.server.notedb.ConfigNotesMigration;
@@ -310,6 +311,7 @@
modules.add(new SearchingChangeCacheImpl.Module());
modules.add(new InternalAccountDirectory.Module());
modules.add(new DefaultCacheFactory.Module());
+ modules.add(cfgInjector.getInstance(MailReceiver.Module.class));
modules.add(new SmtpEmailSender.Module());
modules.add(new SignedTokenEmailTokenVerifier.Module());
modules.add(new PluginRestApiModule());
diff --git a/lib/auto/BUCK b/lib/auto/BUCK
index 6197e34..c186f87 100644
--- a/lib/auto/BUCK
+++ b/lib/auto/BUCK
@@ -2,8 +2,8 @@
maven_jar(
name = 'auto-value',
- id = 'com.google.auto.value:auto-value:1.3-rc1',
- sha1 = 'b764e0fb7e11353fbff493b22fd6e83bf091a179',
+ id = 'com.google.auto.value:auto-value:1.4-rc1',
+ sha1 = '9347939002003a7a3c3af48271fc2c18734528a4',
license = 'Apache2.0',
visibility = ['PUBLIC'],
)
diff --git a/lib/codemirror/cm.bzl b/lib/codemirror/cm.bzl
index 581bb568..6400725 100644
--- a/lib/codemirror/cm.bzl
+++ b/lib/codemirror/cm.bzl
@@ -243,7 +243,7 @@
'@codemirror_original//jar',
'@codemirror_minified//jar',
],
- out = 'cm%s.js' % suffix,
+ outs = ['cm%s.js' % suffix],
)
# Main CSS
@@ -261,7 +261,7 @@
'@codemirror_original//jar',
'@codemirror_minified//jar',
],
- out = 'cm%s.css' % suffix,
+ outs = ['cm%s.css' % suffix],
)
# Modes
@@ -279,7 +279,7 @@
'@codemirror_original//jar',
'@codemirror_minified//jar',
],
- out = 'mode_%s%s.js' % (n, suffix),
+ outs = ['mode_%s%s.js' % (n, suffix)],
)
# Themes
@@ -297,7 +297,7 @@
'@codemirror_original//jar',
'@codemirror_minified//jar',
],
- out = 'theme_%s%s.css' % (n, suffix),
+ outs = ['theme_%s%s.css' % (n, suffix)],
)
# Merge Addon bundled with diff-match-patch
@@ -321,7 +321,7 @@
'@codemirror_original//jar',
'@codemirror_minified//jar',
],
- out = 'addon_merge%s.js' % suffix,
+ outs = ['addon_merge%s.js' % suffix],
)
# Jar packaging
@@ -347,7 +347,7 @@
] + [
':theme_%s%s' % (n, suffix) for n in CM_THEMES
],
- outs = [ 'codemirror%s.jar' % suffix ],
+ outs = ['codemirror%s.jar' % suffix],
)
native.java_import(
diff --git a/lib/greenmail/BUCK b/lib/greenmail/BUCK
new file mode 100644
index 0000000..9a49c4d
--- /dev/null
+++ b/lib/greenmail/BUCK
@@ -0,0 +1,21 @@
+include_defs('//lib/maven.defs')
+
+VERSION = '1.5.2'
+
+java_library(
+ name = 'greenmail',
+ exported_deps = [
+ ':greenmail_library',
+ ],
+ visibility = ['PUBLIC'],
+)
+
+maven_jar(
+ name = 'greenmail_library',
+ id = 'com.icegreen:greenmail:' + VERSION,
+ sha1 = '6b4862a09f8642da58c109117b24ccc19a4a6d39',
+ license = 'Apache2.0',
+ exclude_java_sources = True,
+ visibility = ['PUBLIC'],
+)
+
diff --git a/lib/greenmail/BUILD b/lib/greenmail/BUILD
new file mode 100644
index 0000000..9dd45bc
--- /dev/null
+++ b/lib/greenmail/BUILD
@@ -0,0 +1,7 @@
+package(default_visibility = ['//visibility:public'])
+java_library(
+ name = 'greenmail',
+ exports = ['@greenmail//jar'],
+ visibility = ['//visibility:public'],
+ data = ['//lib:LICENSE-Apache2.0'],
+)
diff --git a/lib/jgit/org.eclipse.jgit/BUCK b/lib/jgit/org.eclipse.jgit/BUCK
index 74338de..3f463df 100644
--- a/lib/jgit/org.eclipse.jgit/BUCK
+++ b/lib/jgit/org.eclipse.jgit/BUCK
@@ -9,7 +9,7 @@
license = 'jgit',
repository = REPO,
unsign = True,
- deps = [':ewah'],
+ deps = [':javaewah'],
exclude = [
'META-INF/eclipse.inf',
'about.html',
@@ -18,7 +18,7 @@
)
maven_jar(
- name = 'ewah',
+ name = 'javaewah',
id = 'com.googlecode.javaewah:JavaEWAH:0.7.9',
sha1 = 'eceaf316a8faf0e794296ebe158ae110c7d72a5a',
license = 'Apache2.0',
diff --git a/lib/jgit/org.eclipse.jgit/BUILD b/lib/jgit/org.eclipse.jgit/BUILD
index bfebb7e..ac8db97 100644
--- a/lib/jgit/org.eclipse.jgit/BUILD
+++ b/lib/jgit/org.eclipse.jgit/BUILD
@@ -3,14 +3,14 @@
java_library(
name = 'jgit-signed',
exports = ['@jgit//jar'],
- runtime_deps = [':ewah'],
+ runtime_deps = [':javaewah'],
visibility = ['//visibility:public'],
data = ['//lib:LICENSE-jgit'],
)
java_library(
- name = 'ewah',
- exports = ['@ewah//jar'],
+ name = 'javaewah',
+ exports = ['@javaewah//jar'],
visibility = ['//visibility:public'],
data = ['//lib:LICENSE-Apache2.0'],
)
diff --git a/lib/js/BUILD b/lib/js/BUILD
index 6758ca1..71fa94f 100644
--- a/lib/js/BUILD
+++ b/lib/js/BUILD
@@ -32,18 +32,3 @@
srcs = [ "//lib/highlightjs:highlight.min.js" ],
data = ['//lib:LICENSE-highlightjs',],
)
-
-bower_component(
- name = 'iron-test-helpers',
- seed = True,
-)
-
-bower_component(
- name = 'test-fixture',
- seed = True,
-)
-
-bower_component(
- name = 'web-component-tester',
- seed = True,
-)
diff --git a/lib/js/bower_archives.bzl b/lib/js/bower_archives.bzl
index d90a7cc..9c9a5a9 100644
--- a/lib/js/bower_archives.bzl
+++ b/lib/js/bower_archives.bzl
@@ -7,6 +7,21 @@
load("//tools/bzl:js.bzl", "bower_archive")
def load_bower_archives():
bower_archive(
+ name = "accessibility-developer-tools",
+ package = "accessibility-developer-tools",
+ version = "2.11.0",
+ sha1 = "792cb24b649dafb316e7e536f8ae65d0d7b52bab")
+ bower_archive(
+ name = "async",
+ package = "async",
+ version = "1.5.2",
+ sha1 = "1ec975d3b3834646a7e3d4b7e68118b90ed72508")
+ bower_archive(
+ name = "chai",
+ package = "chai",
+ version = "3.5.0",
+ sha1 = "849ad3ee7c77506548b7b5db603a4e150b9431aa")
+ bower_archive(
name = "iron-a11y-announcer",
package = "iron-a11y-announcer",
version = "1.0.5",
@@ -52,11 +67,36 @@
version = "1.1.1",
sha1 = "480423380be0536f948735d91bc472f6e7ced5b4")
bower_archive(
+ name = "lodash",
+ package = "lodash",
+ version = "3.10.1",
+ sha1 = "2f207a8293c4c554bf6cf071241f7a00dc513d3a")
+ bower_archive(
+ name = "mocha",
+ package = "mocha",
+ version = "2.5.3",
+ sha1 = "22ef0d1f43ba5e2241369c501ac648f00c0440c0")
+ bower_archive(
name = "neon-animation",
package = "neon-animation",
version = "1.2.4",
sha1 = "e8ccbb930c4b7ff470b1450baa901618888a7fd3")
bower_archive(
+ name = "sinon-chai",
+ package = "sinon-chai",
+ version = "2.8.0",
+ sha1 = "0464b5d944fdf8116bb23e0b02ecfbac945b3517")
+ bower_archive(
+ name = "sinonjs",
+ package = "sinonjs",
+ version = "1.17.1",
+ sha1 = "a26a6aab7358807de52ba738770f6ac709afd240")
+ bower_archive(
+ name = "stacky",
+ package = "stacky",
+ version = "1.3.2",
+ sha1 = "d6c07a0112ab2e9677fe085933744466a89232fb")
+ bower_archive(
name = "web-animations-js",
package = "web-animations-js",
version = "2.2.2",
diff --git a/lib/js/bower_components.bzl b/lib/js/bower_components.bzl
index 480d6ce..74515e1 100644
--- a/lib/js/bower_components.bzl
+++ b/lib/js/bower_components.bzl
@@ -7,6 +7,18 @@
load("//tools/bzl:js.bzl", "bower_component")
def define_bower_components():
bower_component(
+ name = "accessibility-developer-tools",
+ license = "//lib:LICENSE-Apache2.0",
+ )
+ bower_component(
+ name = "async",
+ license = "//lib:LICENSE-polymer",
+ )
+ bower_component(
+ name = "chai",
+ license = "//lib:LICENSE-polymer",
+ )
+ bower_component(
name = "es6-promise",
license = "//lib:LICENSE-polymer",
seed = True,
@@ -112,6 +124,12 @@
seed = True,
)
bower_component(
+ name = "iron-test-helpers",
+ license = "//lib:LICENSE-polymer",
+ deps = [ ":polymer" ],
+ seed = True,
+ )
+ bower_component(
name = "iron-validatable-behavior",
license = "//lib:LICENSE-polymer",
deps = [
@@ -120,6 +138,14 @@
],
)
bower_component(
+ name = "lodash",
+ license = "//lib:LICENSE-polymer",
+ )
+ bower_component(
+ name = "mocha",
+ license = "//lib:LICENSE-polymer",
+ )
+ bower_component(
name = "moment",
license = "//lib:LICENSE-moment",
seed = True,
@@ -153,10 +179,43 @@
seed = True,
)
bower_component(
+ name = "sinon-chai",
+ license = "//lib:LICENSE-polymer",
+ )
+ bower_component(
+ name = "sinonjs",
+ license = "//lib:LICENSE-polymer",
+ )
+ bower_component(
+ name = "stacky",
+ license = "//lib:LICENSE-polymer",
+ )
+ bower_component(
+ name = "test-fixture",
+ license = "//lib:LICENSE-polymer",
+ seed = True,
+ )
+ bower_component(
name = "web-animations-js",
license = "//lib:LICENSE-Apache2.0",
)
bower_component(
+ name = "web-component-tester",
+ license = "//lib:LICENSE-polymer",
+ deps = [
+ ":accessibility-developer-tools",
+ ":async",
+ ":chai",
+ ":lodash",
+ ":mocha",
+ ":sinon-chai",
+ ":sinonjs",
+ ":stacky",
+ ":test-fixture",
+ ],
+ seed = True,
+ )
+ bower_component(
name = "webcomponentsjs",
license = "//lib:LICENSE-polymer",
)
diff --git a/lib/mail/BUCK b/lib/mail/BUCK
new file mode 100644
index 0000000..07c78d8
--- /dev/null
+++ b/lib/mail/BUCK
@@ -0,0 +1,21 @@
+include_defs('//lib/maven.defs')
+
+VERSION = '1.5.6'
+
+java_library(
+ name = 'mail',
+ exported_deps = [
+ ':mail_library',
+ ],
+ visibility = ['PUBLIC'],
+)
+
+maven_jar(
+ name = 'mail_library',
+ id = 'com.sun.mail:javax.mail:' + VERSION,
+ sha1 = 'ab5daef2f881c42c8e280cbe918ec4d7fdfd7efe',
+ license = 'DO_NOT_DISTRIBUTE',
+ exclude_java_sources = True,
+ visibility = ['PUBLIC'],
+)
+
diff --git a/lib/mail/BUILD b/lib/mail/BUILD
new file mode 100644
index 0000000..40dd302
--- /dev/null
+++ b/lib/mail/BUILD
@@ -0,0 +1,6 @@
+java_library(
+ name = 'mail',
+ exports = ['@mail//jar'],
+ visibility = ['//visibility:public'],
+ data = ['//lib:LICENSE-DO_NOT_DISTRIBUTE'],
+)
diff --git a/lib/mime4j/BUCK b/lib/mime4j/BUCK
new file mode 100644
index 0000000..13fc42a
--- /dev/null
+++ b/lib/mime4j/BUCK
@@ -0,0 +1,35 @@
+include_defs('//lib/maven.defs')
+
+VERSION = '0.8.0'
+
+java_library(
+ name = 'core',
+ exported_deps = [
+ ':core_library',
+ ],
+ visibility = ['PUBLIC'],
+)
+
+maven_jar(
+ name = 'core_library',
+ id = 'org.apache.james:apache-mime4j-core:' + VERSION,
+ sha1 = 'd54f45fca44a2f210569656b4ca3574b42911c95',
+ license = 'Apache2.0',
+ visibility = ['PUBLIC'],
+)
+
+java_library(
+ name = 'dom',
+ exported_deps = [
+ ':dom_library',
+ ],
+ visibility = ['PUBLIC'],
+)
+
+maven_jar(
+ name = 'dom_library',
+ id = 'org.apache.james:apache-mime4j-dom:' + VERSION,
+ sha1 = '6720c93d14225c3e12c4a69768a0370c80e376a3',
+ license = 'Apache2.0',
+ visibility = ['PUBLIC'],
+)
diff --git a/lib/mime4j/BUILD b/lib/mime4j/BUILD
new file mode 100644
index 0000000..e7b85ef
--- /dev/null
+++ b/lib/mime4j/BUILD
@@ -0,0 +1,13 @@
+java_library(
+ name = "core",
+ data = ["//lib:LICENSE-Apache2.0"],
+ visibility = ["//visibility:public"],
+ exports = ["@mime4j_core//jar"],
+)
+
+java_library(
+ name = "dom",
+ data = ["//lib:LICENSE-Apache2.0"],
+ visibility = ["//visibility:public"],
+ exports = ["@mime4j_dom//jar"],
+)
diff --git a/plugins/replication b/plugins/replication
index 3212bcd..e1beaa8 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 3212bcd4f2c0dc791a99af97ee98df70746f2306
+Subproject commit e1beaa8e16c05af5538d6459f9e4d3e4af500bca
diff --git a/polygerrit-ui/README.md b/polygerrit-ui/README.md
index 1e548d5..28d028b 100644
--- a/polygerrit-ui/README.md
+++ b/polygerrit-ui/README.md
@@ -111,6 +111,14 @@
Then visit http://localhost:8081/elements/foo/bar_test.html
+## Running tests (bazel)
+
+Run
+
+```sh
+WCT_ARGS='--some-flag' sh polygerrit-ui/app/run_test.sh
+```
+
## Style guide
We follow the [Google JavaScript Style Guide](https://google.github.io/styleguide/javascriptguide.xml)
diff --git a/polygerrit-ui/app/BUILD b/polygerrit-ui/app/BUILD
index b6b74b7..d390b58 100644
--- a/polygerrit-ui/app/BUILD
+++ b/polygerrit-ui/app/BUILD
@@ -2,17 +2,10 @@
default_visibility = ["//visibility:public"])
load('//tools/bzl:genrule2.bzl', 'genrule2')
-load("//tools/bzl:js.bzl", "bower_component_bundle", "vulcanize")
-
-bower_component_bundle(
- name = 'test_components',
- deps = [
- '//polygerrit-ui:polygerrit_components',
- '//lib/js:iron-test-helpers',
- '//lib/js:test-fixture',
- '//lib/js:web-component-tester',
- ],
-)
+load(
+ "//tools/bzl:js.bzl",
+ "bower_component_bundle", "vulcanize",
+ "bower_component", "js_component")
vulcanize(
name = "gr-app",
@@ -30,10 +23,10 @@
filegroup(
name = "top_sources",
- srcs = glob([
+ srcs = [
'favicon.ico',
'index.html',
- ]),
+ ],
)
filegroup(
@@ -67,3 +60,46 @@
],
outs = [ "polygerrit_ui.zip" ],
)
+
+bower_component_bundle(
+ name = 'test_components',
+ deps = [
+ '//polygerrit-ui:polygerrit_components',
+ '//lib/js:iron-test-helpers',
+ '//lib/js:test-fixture',
+ '//lib/js:web-component-tester',
+ ],
+)
+
+filegroup(
+ name = "pg_code",
+ srcs = glob([
+ '**/*.html',
+ '**/*.js',
+ ], exclude = [
+ 'bower_components/**',
+ ])
+)
+
+genrule2(
+ name = "pg_code_zip",
+ outs = [ "pg_code.zip", ],
+ srcs = [ ":pg_code" ],
+ cmd = " && ".join([
+ ("tar -cf- --mtime=1980-01-01\\ 00:00 $(locations :pg_code) |"
+ + " tar --strip-components=2 -C $$TMP/ -xf-"),
+ "cd $$TMP",
+ "zip -rq $$ROOT/$@ *"])
+)
+
+sh_test(
+ name = "wct_test",
+ srcs = [ "wct_test.sh" ],
+ data = [
+ ":pg_code.zip",
+ ":test_components.zip",
+ "test/index.html",
+ ],
+ # Should not run sandboxed.
+ tags = ["local", "manual"],
+)
diff --git a/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.html b/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.html
index 98f2b18..588fff5 100644
--- a/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.html
+++ b/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.html
@@ -52,7 +52,8 @@
change="[[change]]"
filter="[[filter]]"
placeholder="[[placeholder]]"
- on-add="_handleAdd">
+ on-add="_handleAdd"
+ on-input-keydown="_handleInputKeydown">
</gr-account-entry>
</template>
<script src="gr-account-list.js"></script>
diff --git a/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.js b/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.js
index 4e0ff94..2e3200f 100644
--- a/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.js
+++ b/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.js
@@ -37,6 +37,10 @@
'remove': '_handleRemove',
},
+ get accountChips() {
+ return Polymer.dom(this.root).querySelectorAll('gr-account-chip');
+ },
+
get focusStart() {
return this.$.entry.focusStart;
},
@@ -104,6 +108,19 @@
e.detail.account);
},
+ _handleInputKeydown: function(e) {
+ var input = e.detail.input;
+ if (input.selectionStart !== input.selectionEnd ||
+ input.selectionStart !== 0) {
+ return;
+ }
+ switch (e.detail.keyCode) {
+ case 8: // Backspace
+ this.splice('accounts', this.accounts.length - 1, 1);
+ break;
+ }
+ },
+
additions: function() {
return this.accounts.filter(function(account) {
return account._pendingAdd;
diff --git a/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list_test.html b/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list_test.html
index 7092c4e..cbfe1ff 100644
--- a/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list_test.html
@@ -228,5 +228,34 @@
},
]);
});
+
+ suite('keyboard interactions', function() {
+ var sandbox;
+ setup(function() {
+ sandbox = sinon.sandbox.create();
+ });
+
+ teardown(function() {
+ sandbox.restore();
+ });
+
+ test('backspace from input removes account iff cursor is in start pos',
+ function(done) {
+ var input = element.$.entry.$.input;
+ sandbox.stub(element.$.entry, '_getReviewerSuggestions');
+ sandbox.stub(input, '_updateSuggestions');
+ input.text = 'test';
+ MockInteractions.focus(input.$.input);
+ flush(function() {
+ assert.equal(element.accounts.length, 2);
+ MockInteractions.pressAndReleaseKeyOn(input.$.input, 8); // Backspace
+ assert.equal(element.accounts.length, 2);
+ input.text = '';
+ MockInteractions.pressAndReleaseKeyOn(input.$.input, 8); // Backspace
+ assert.equal(element.accounts.length, 1);
+ done();
+ });
+ });
+ });
});
</script>
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 96c6193..30e9e86 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
@@ -112,6 +112,19 @@
on-confirm="_handleAbandonDialogConfirm"
on-cancel="_handleConfirmDialogCancel"
hidden></gr-confirm-abandon-dialog>
+ <gr-confirm-dialog
+ id="confirmDeleteDialog"
+ class="confirmDialog"
+ confirm-label="Delete"
+ on-cancel="_handleConfirmDialogCancel"
+ on-confirm="_handleDeleteConfirm">
+ <div class="header">
+ Delete Change
+ </div>
+ <div class="main">
+ Do you really want to delete the change?
+ </div>
+ </gr-confirm-dialog>
</gr-overlay>
<gr-js-api-interface id="jsAPI"></gr-js-api-interface>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
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 9fdc2a9..ef2a3b4 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
@@ -336,6 +336,8 @@
var type = el.getAttribute('data-action-type');
if (type === ActionType.REVISION) {
this._handleRevisionAction(key);
+ } else if (key === ChangeActions.DELETE) {
+ this._showActionDialog(this.$.confirmDeleteDialog);
} else if (key === ChangeActions.REVERT) {
this.showRevertDialog();
} else if (key === ChangeActions.ABANDON) {
@@ -441,6 +443,10 @@
{message: el.message});
},
+ _handleDeleteConfirm: function() {
+ this._fireAction('/', this.actions[ChangeActions.DELETE], false);
+ },
+
_setLoadingOnButtonWithKey: function(key) {
var buttonEl = this.$$('[data-action-key="' + key + '"]');
buttonEl.setAttribute('loading', true);
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index c180f46..00e61d9 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -434,5 +434,59 @@
populateRevertMsgStub.restore();
});
});
+
+ suite('delete change', function() {
+ var fireActionStub;
+ var deleteAction;
+
+ var tapDeleteAction = function() {
+ var deleteButton = element.$$('gr-button[data-action-key=\'/\']');
+ MockInteractions.tap(deleteButton);
+ flushAsynchronousOperations();
+ };
+
+ setup(function() {
+ fireActionStub = sinon.stub(element, '_fireAction');
+ element.change = {
+ current_revision: 'abc1234',
+ };
+ deleteAction = {
+ method: 'DELETE',
+ label: 'Delete Change',
+ title: 'Delete change X_X',
+ enabled: true,
+ };
+ element.actions = {
+ '/': deleteAction,
+ };
+ });
+
+ teardown(function() {
+ fireActionStub.restore();
+ });
+
+ test('does not delete on action', function() {
+ tapDeleteAction();
+ assert.isFalse(fireActionStub.called);
+ });
+
+ test('shows confirm dialog', function() {
+ tapDeleteAction();
+ assert.isFalse(element.$$('#confirmDeleteDialog').hidden);
+ MockInteractions.tap(
+ element.$$('#confirmDeleteDialog').$$('gr-button[primary]'));
+ flushAsynchronousOperations();
+ assert.isTrue(fireActionStub.calledWith('/', deleteAction, false));
+ });
+
+ test('hides delete confirm on cancel', function() {
+ tapDeleteAction();
+ MockInteractions.tap(
+ element.$$('#confirmDeleteDialog').$$('gr-button:not([primary])'));
+ flushAsynchronousOperations();
+ assert.isTrue(element.$$('#confirmDeleteDialog').hidden);
+ assert.isFalse(fireActionStub.called);
+ });
+ });
});
</script>
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 ecd26ad..1c703fd 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
@@ -18,6 +18,7 @@
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/rest-client-behavior.html">
<link rel="import" href="../../shared/gr-account-link/gr-account-link.html">
+<link rel="import" href="../../shared/gr-select/gr-select.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../../shared/gr-change-star/gr-change-star.html">
<link rel="import" href="../../shared/gr-date-formatter/gr-date-formatter.html">
@@ -92,9 +93,13 @@
.changeMetadata {
font-size: .95em;
}
+ /* Prevent plugin text from overflowing. */
+ #change_plugins {
+ word-break: break-all;
+ }
.commitMessage {
font-family: var(--monospace-font-family);
- flex: 0 0 72ch;
+ flex: 1 0 72ch;
margin-right: 2em;
margin-bottom: 1em;
}
@@ -206,7 +211,7 @@
}
}
</style>
- <div class="container loading" hidden$="{{!_loading}}">Loading...</div>
+ <div class="container loading" hidden$="[[!_loading]]">Loading...</div>
<div class="container" hidden$="{{_loading}}">
<div class="header">
<span class="header-title">
@@ -231,7 +236,7 @@
<div id="change_plugins"></div>
</div>
<div class="changeInfo-column mainChangeInfo">
- <div class="commitActions" hidden$="[[!_loggedIn]]"">
+ <div class="commitActions" hidden$="[[!_loggedIn]]">
<gr-button
class="reply"
secondary
@@ -261,18 +266,24 @@
<div class="relatedChanges">
<gr-related-changes-list id="relatedChanges"
change="[[_change]]"
- patch-num="[[_computeLatestPatchNum(_allPatchSets)]]"></gr-related-changes-list>
+ patch-num="[[_computeLatestPatchNum(_allPatchSets)]]">
+ </gr-related-changes-list>
</div>
</div>
</div>
</section>
- <section class$="patchInfo [[_computePatchInfoClass(_patchRange.patchNum, _allPatchSets)]]">
+ <section class$="patchInfo [[_computePatchInfoClass(_patchRange.patchNum,
+ _allPatchSets)]]">
<div class="patchInfo-header">
<div>
- <label class="patchSelectLabel" for="patchSetSelect">Patch set</label>
- <select id="patchSetSelect" on-change="_handlePatchChange">
- <template is="dom-repeat" items="[[_allPatchSets]]" as="patchNumber">
- <option value$="[[patchNumber]]" selected$="[[_computePatchIndexIsSelected(index, _patchRange.patchNum)]]">
+ <label class="patchSelectLabel" for="patchSetSelect">
+ Patch set
+ </label>
+ <select id="patchSetSelect" bind-value="{{_selectedPatchSet}}"
+ is="gr-select" on-change="_handlePatchChange">
+ <template is="dom-repeat" items="[[_allPatchSets]]"
+ as="patchNumber">
+ <option value$="[[patchNumber]]">
<span>[[patchNumber]]</span>
/
<span>[[_computeLatestPatchNum(_allPatchSets)]]</span>
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 f6ecc9e..bac25be 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
@@ -73,7 +73,10 @@
type: String,
value: '',
},
- _patchRange: Object,
+ _patchRange: {
+ type: Object,
+ observer: '_updateSelected',
+ },
_allPatchSets: {
type: Array,
computed: '_computeAllPatchSets(_change)',
@@ -89,6 +92,7 @@
value: 'Reply',
computed: '_computeReplyButtonLabel(_diffDrafts.*)',
},
+ _selectedPatchSet: String,
_initialLoadComplete: {
type: Boolean,
value: false,
@@ -458,6 +462,8 @@
this._patchRange.patchNum ||
this._computeLatestPatchNum(this._allPatchSets));
+ this._updateSelected();
+
var title = change.subject + ' (' + change.change_id.substr(0, 9) + ')';
this.fire('title-change', {title: title});
},
@@ -528,10 +534,6 @@
}
},
- _computePatchIndexIsSelected: function(index, patchNum) {
- return this._allPatchSets[index] == patchNum;
- },
-
_computeLabelNames: function(labels) {
return Object.keys(labels).sort();
},
@@ -776,5 +778,9 @@
this.$.fileList.reload(),
]);
},
+
+ _updateSelected: function() {
+ this._selectedPatchSet = this._patchRange.patchNum;
+ },
});
})();
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 8358e40..e61d6b0 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
@@ -277,12 +277,10 @@
var optionEls = Polymer.dom(element.root).querySelectorAll(
'.patchInfo-header option');
assert.equal(optionEls.length, 4);
- assert.isFalse(element.$$('.patchInfo-header option[value="1"]')
- .hasAttribute('selected'));
- assert.isTrue(element.$$('.patchInfo-header option[value="2"]')
- .hasAttribute('selected'));
- assert.isFalse(element.$$('.patchInfo-header option[value="3"]')
- .hasAttribute('selected'));
+ var select = element.$$('.patchInfo-header #patchSetSelect').bindValue;
+ assert.notEqual(select, 1);
+ assert.equal(select, 2);
+ assert.notEqual(select, 3);
assert.equal(optionEls[3].value, 13);
var showStub = sandbox.stub(page, 'show');
@@ -329,12 +327,12 @@
var optionEls = Polymer.dom(element.root).querySelectorAll(
'.patchInfo-header option');
assert.equal(optionEls.length, 4);
- assert.isFalse(element.$$('.patchInfo-header option[value="1"]')
- .hasAttribute('selected'));
- assert.isTrue(element.$$('.patchInfo-header option[value="2"]')
- .hasAttribute('selected'));
- assert.isFalse(element.$$('.patchInfo-header option[value="3"]')
- .hasAttribute('selected'));
+ assert.notEqual(
+ element.$$('.patchInfo-header #patchSetSelect').bindValue, 1);
+ assert.equal(
+ element.$$('.patchInfo-header #patchSetSelect').bindValue, 2);
+ assert.notEqual(
+ element.$$('.patchInfo-header #patchSetSelect').bindValue, 3);
assert.equal(optionEls[3].value, 13);
var showStub = sandbox.stub(page, 'show');
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 007c0fc..f11cca2 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -22,6 +22,7 @@
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../../shared/gr-linked-text/gr-linked-text.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
+<link rel="import" href="../../shared/gr-select/gr-select.html">
<dom-module id="gr-file-list">
<template>
@@ -182,13 +183,17 @@
<span class="separator">/</span>
<label>
Diff against
- <select id="patchChange" on-change="_handlePatchChange">
+ <select id="patchChange" bind-value="{{_diffAgainst}}" is="gr-select"
+ on-change="_handlePatchChange">
<option value="PARENT">Base</option>
- <template is="dom-repeat" items="[[_computePatchSets(revisions, patchRange.*)]]" as="patchNum">
- <option
- value$="[[patchNum]]"
- selected$="[[_computePatchSetSelected(patchNum, patchRange.basePatchNum)]]"
- disabled$="[[_computePatchSetDisabled(patchNum, patchRange.patchNum)]]">[[patchNum]]</option>
+ <template
+ is="dom-repeat"
+ items="[[_computePatchSets(revisions, patchRange.*)]]"
+ as="patchNum">
+ <option value$="[[patchNum]]" disabled$=
+ "[[_computePatchSetDisabled(patchNum, patchRange.patchNum)]]">
+ [[patchNum]]
+ </option>
</template>
</select>
</label>
@@ -207,7 +212,8 @@
<div class$="[[_computeClass('status', file.__path)]]">
[[_computeFileStatus(file.status)]]
</div>
- <a class="path" href$="[[_computeDiffURL(changeNum, patchRange, file.__path)]]">
+ <a class="path"
+ href$="[[_computeDiffURL(changeNum, patchRange, file.__path)]]">
<div title$="[[_computeFileDisplayName(file.__path)]]">
[[_computeFileDisplayName(file.__path)]]
</div>
@@ -217,7 +223,9 @@
</div>
</a>
<div class="comments">
- <span class="drafts">[[_computeDraftsString(drafts, patchRange.patchNum, file.__path)]]</span>
+ <span class="drafts">
+ [[_computeDraftsString(drafts, patchRange.patchNum, file.__path)]]
+ </span>
[[_computeCommentsString(comments, patchRange.patchNum, file.__path)]]
</div>
<div class$="[[_computeClass('stats', file.__path)]]">
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 340cb63..bf6dcf3 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -25,7 +25,10 @@
is: 'gr-file-list',
properties: {
- patchRange: Object,
+ patchRange: {
+ type: Object,
+ observer: '_updateSelected',
+ },
patchNum: String,
changeNum: String,
comments: Object,
@@ -58,6 +61,7 @@
type: Array,
value: function() { return []; },
},
+ _diffAgainst: String,
_diffPrefs: Object,
_userPrefs: Object,
_localPrefs: Object,
@@ -181,10 +185,6 @@
return parseInt(patchNum, 10) >= parseInt(currentPatchNum, 10);
},
- _computePatchSetSelected: function(patchNum, basePatchNum) {
- return parseInt(patchNum, 10) === parseInt(basePatchNum, 10);
- },
-
_handleHiddenChange: function(e) {
var model = e.model;
model.set('file.__expanded', !model.file.__expanded);
@@ -527,6 +527,10 @@
this._numFilesShown = this._files.length;
},
+ _updateSelected: function(patchRange) {
+ this._diffAgainst = patchRange.basePatchNum;
+ },
+
/**
* _getDiffViewMode: Get the diff view (side-by-side or unified) based on
* the current state.
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index 0caa6be..b530bae 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -436,7 +436,7 @@
document.getElementById('blank').restore();
});
- test('show/hide diffs disabled for large amounds of files', function(done) {
+ test('show/hide diffs disabled for large amounts of files', function(done) {
element._files = [];
element.changeNum = '42';
element.patchRange = {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
index 2abd0a8..410a813 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
@@ -215,7 +215,9 @@
_handleTextareaKeydown: function(e) {
switch (e.keyCode) {
case 27: // 'esc'
- this._handleCancel(e);
+ if (this._messageText.length === 0) {
+ this._handleCancel(e);
+ }
break;
case 83: // 's'
if (e.ctrlKey) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
index 11de0ba..0ad3e11 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
@@ -47,6 +47,7 @@
suite('gr-diff-comment tests', function() {
var element;
+ var sandbox;
setup(function() {
stub('gr-rest-api-interface', {
getAccount: function() { return Promise.resolve(null); },
@@ -62,6 +63,11 @@
message: 'is this a crossover episode!?',
updated: '2015-12-08 19:48:33.843000000',
};
+ sandbox = sinon.sandbox.create();
+ });
+
+ teardown(function() {
+ sandbox.restore();
});
test('collapsible comments', function() {
@@ -149,6 +155,22 @@
assert.isFalse(isVisible(element.$$('.collapsedContent')),
'header middle content is is not visible');
});
+
+ test('esc does not close comment unless text is empty', function(done) {
+ element.editing = true;
+ element._messageText = 'test';
+ var textarea = element.$.editTextarea;
+ var closeSpy = sandbox.spy(element, '_handleCancel');
+
+ flush(function() {
+ MockInteractions.pressAndReleaseKeyOn(textarea, 27); // esc
+ assert.isFalse(closeSpy.called);
+ element._messageText = '';
+ MockInteractions.pressAndReleaseKeyOn(textarea, 27); // esc
+ assert.isTrue(closeSpy.called);
+ done();
+ });
+ });
});
suite('gr-diff-comment draft tests', function() {
@@ -321,6 +343,7 @@
MockInteractions.tap(element.$$('.cancel'));
MockInteractions.tap(element.$$('.discard'));
element.flushDebouncer('fire-update');
+ element._messageText = '';
MockInteractions.pressAndReleaseKeyOn(element.$.editTextarea, 27); // esc
});
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 8262658..2b89a7a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -90,6 +90,10 @@
.full-width {
width: 100%;
}
+ .full-width .contentText {
+ white-space: pre-wrap;
+ word-wrap: break-word;
+ }
.lineNum,
.content {
/* Set font size based the user's diff preference. */
@@ -138,10 +142,6 @@
background-color: #fef;
color: #849;
}
- .contentText {
- white-space: pre-wrap;
- word-wrap: break-word;
- }
.contextControl gr-button {
display: inline-block;
font-family: var(--monospace-font-family);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index 8db05a3..306f283 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -50,8 +50,7 @@
assert.isFalse(element.classList.contains('no-left'));
});
- test('view does not start with displayLine classList',
- function() {
+ test('view does not start with displayLine classList', function() {
assert.isFalse(
element.$$('.diffContainer').classList.contains('displayLine'));
});
@@ -63,6 +62,7 @@
assert.isTrue(spy.called);
assert.isTrue(
element.$$('.diffContainer').classList.contains('displayLine'));
+ spy.restore();
});
test('get drafts', function(done) {
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.html b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.html
index c496703..54bccb3 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.html
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.html
@@ -13,7 +13,7 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
-
+<link rel="import" href="../../shared/gr-select/gr-select.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<dom-module id="gr-patch-range-select">
@@ -28,12 +28,11 @@
</style>
Patch set:
<span class="patchRange">
- <select id="leftPatchSelect" on-change="_handlePatchChange">
- <option value="PARENT"
- selected$="[[_computeLeftSelected('PARENT', patchRange)]]">Base</option>
+ <select id="leftPatchSelect" bind-value="{{_leftSelected}}"
+ on-change="_handlePatchChange" is="gr-select">
+ <option value="PARENT">Base</option>
<template is="dom-repeat" items="{{availablePatches}}" as="patchNum">
<option value$="[[patchNum]]"
- selected$="[[_computeLeftSelected(patchNum, patchRange)]]"
disabled$="[[_computeLeftDisabled(patchNum, patchRange)]]">[[patchNum]]</option>
</template>
</select>
@@ -46,10 +45,10 @@
</span>
→
<span class="patchRange">
- <select id="rightPatchSelect" on-change="_handlePatchChange">
+ <select id="rightPatchSelect" bind-value="{{_rightSelected}}"
+ on-change="_handlePatchChange" is="gr-select">
<template is="dom-repeat" items="{{availablePatches}}" as="patchNum">
<option value$="[[patchNum]]"
- selected$="[[_computeRightSelected(patchNum, patchRange)]]"
disabled$="[[_computeRightDisabled(patchNum, patchRange)]]">[[patchNum]]</option>
</template>
</select>
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
index b50043e..350429f 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
@@ -21,13 +21,23 @@
availablePatches: Array,
changeNum: String,
filesWeblinks: Object,
- patchRange: Object,
path: String,
+ patchRange: {
+ type: Object,
+ observer: '_updateSelected'
+ },
+ _rightSelected: String,
+ _leftSelected: String,
+ },
+
+ _updateSelected: function() {
+ this._rightSelected = this.patchRange.patchNum;
+ this._leftSelected = this.patchRange.basePatchNum;
},
_handlePatchChange: function(e) {
- var leftPatch = this.$.leftPatchSelect.value;
- var rightPatch = this.$.rightPatchSelect.value;
+ var leftPatch = this._leftSelected;
+ var rightPatch = this._rightSelected;
var rangeStr = rightPatch;
if (leftPatch != 'PARENT') {
rangeStr = leftPatch + '..' + rangeStr;
@@ -36,14 +46,6 @@
e.target.blur();
},
- _computeLeftSelected: function(patchNum, patchRange) {
- return patchNum == patchRange.basePatchNum;
- },
-
- _computeRightSelected: function(patchNum, patchRange) {
- return patchNum == patchRange.patchNum;
- },
-
_computeLeftDisabled: function(patchNum, patchRange) {
return parseInt(patchNum, 10) >= parseInt(patchRange.patchNum, 10);
},
@@ -52,5 +54,18 @@
if (patchRange.basePatchNum == 'PARENT') { return false; }
return parseInt(patchNum, 10) <= parseInt(patchRange.basePatchNum, 10);
},
+
+ // On page load, the dom-if for options getting added occurs after
+ // the value was set in the select. This ensures that after they
+ // are loaded, the correct value will get selected. I attempted to
+ // debounce these, but because they are detecting two different
+ // events, sometimes the timing was off and one ended up missing.
+ _synchronizeSelectionRight: function() {
+ this.$.rightPatchSelect.value = this._rightSelected;
+ },
+
+ _synchronizeSelectionLeft: function() {
+ this.$.leftPatchSelect.value = this._leftSelected;
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html
index 95789bc..68eeaa9 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html
@@ -67,6 +67,10 @@
element.changeNum = '42';
element.path = 'path/to/file.txt';
element.availablePatches = ['1', '2', '3'];
+ element.patchRange = {
+ basePatchNum: 'PARENT',
+ patchNum: '3',
+ };
flushAsynchronousOperations();
var numEvents = 0;
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
index 601a056..1c9e010 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
@@ -31,6 +31,13 @@
* @event cancel
*/
+ /**
+ * Fired on keydown to allow for custom hooks into autocomplete textbox
+ * behavior.
+ *
+ * @event input-keydown
+ */
+
properties: {
/**
@@ -212,6 +219,7 @@
this._commit();
break;
}
+ this.fire('input-keydown', {keyCode: e.keyCode, input: this.$.input});
},
_cancel: function() {
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
index 431c795..03fa8e43 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
@@ -297,5 +297,13 @@
focusSpy.restore();
commitSpy.restore();
});
+
+ test('input-keydown event fired', function() {
+ var listener = sinon.spy();
+ element.addEventListener('input-keydown', listener);
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 9); // tab
+ flushAsynchronousOperations();
+ assert.isTrue(listener.called);
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.js b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.js
index cb852fd..a3eccb8 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.js
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.js
@@ -50,27 +50,19 @@
_contentOrConfigChanged: function(content, config) {
var output = Polymer.dom(this.$.output);
output.textContent = '';
- var parser = new GrLinkTextParser(config, function(text, href, html) {
+ var parser = new GrLinkTextParser(
+ config, function(text, href, fragment) {
if (href) {
var a = document.createElement('a');
a.href = href;
a.textContent = text;
a.target = '_blank';
output.appendChild(a);
- } else if (html) {
- var fragment = document.createDocumentFragment();
- // Create temporary div to hold the nodes in.
- var div = document.createElement('div');
- div.innerHTML = html;
- while (div.firstChild) {
- fragment.appendChild(div.firstChild);
- }
+ } else if (fragment) {
output.appendChild(fragment);
- } else {
- output.appendChild(document.createTextNode(text));
}
});
parser.parse(content);
- }
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
index 5203520..f530331 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
@@ -123,6 +123,35 @@
assert.equal(linkEl2.textContent, 'Issue 3450');
});
+ test('Change-Id pattern parsed before bug pattern', function() {
+ // "Change-Id:" pattern.
+ var changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
+ var prefix = 'Change-Id: ';
+
+ // "Issue/Bug" pattern.
+ var bug = 'Issue 3650';
+
+ var changeUrl = '/q/' + changeID;
+ var bugUrl = 'https://code.google.com/p/gerrit/issues/detail?id=3650';
+
+ element.content = prefix + changeID + bug;
+
+ var textNode = element.$.output.childNodes[0];
+ var changeLinkEl = element.$.output.childNodes[1];
+ var bugLinkEl = element.$.output.childNodes[2];
+
+ assert.equal(textNode.textContent, prefix);
+
+ assert.equal(changeLinkEl.target, '_blank');
+ assert.isTrue(changeLinkEl.href.endsWith(changeUrl));
+ assert.equal(changeLinkEl.textContent, changeID);
+
+ assert.equal(bugLinkEl.target, '_blank');
+ assert.equal(bugLinkEl.href, bugUrl);
+ assert.equal(bugLinkEl.textContent, 'Issue 3650');
+ });
+
+
test('html field in link config', function() {
element.content = 'google:do a barrel roll';
var linkEl = element.$.output.childNodes[0];
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js
index b4b1678..5e1ff62 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js
@@ -27,13 +27,83 @@
this.callback(text, href);
};
-GrLinkTextParser.prototype.addHTML = function(html) {
- this.callback(null, null, html);
+GrLinkTextParser.prototype.processLinks = function(text, outputArray) {
+ this.sortArrayReverse(outputArray);
+ var fragment = document.createDocumentFragment();
+ var cursor = text.length;
+
+ // Start inserting linkified URLs from the end of the String. That way, the
+ // string positions of the items don't change as we iterate through.
+ outputArray.forEach(function(item) {
+ // Add any text between the current linkified item and the item added before
+ // if it exists.
+ if (item.position + item.length !== cursor) {
+ fragment.insertBefore(
+ document.createTextNode(
+ text.slice(item.position + item.length, cursor)),
+ fragment.firstChild);
+ }
+ fragment.insertBefore(item.html, fragment.firstChild);
+ cursor = item.position;
+ });
+
+ // Add the beginning portion at the end.
+ if (cursor !== 0) {
+ fragment.insertBefore(
+ document.createTextNode(text.slice(0, cursor)), fragment.firstChild);
+ }
+
+ this.callback(null, null, fragment);
+};
+
+GrLinkTextParser.prototype.sortArrayReverse = function(outputArray) {
+ outputArray.sort(function(a, b) {return b.position - a.position});
+};
+
+GrLinkTextParser.prototype.addItem =
+ function(text, href, html, position, length, outputArray) {
+ var htmlOutput = '';
+
+ if (href) {
+ var a = document.createElement('a');
+ a.href = href;
+ a.textContent = text;
+ a.target = '_blank';
+ htmlOutput = a;
+ } else if (html) {
+ var fragment = document.createDocumentFragment();
+ // Create temporary div to hold the nodes in.
+ var div = document.createElement('div');
+ div.innerHTML = html;
+ while (div.firstChild) {
+ fragment.appendChild(div.firstChild);
+ }
+ htmlOutput = fragment;
+ }
+
+ outputArray.push({
+ html: htmlOutput,
+ position: position,
+ length: length,
+ });
+};
+
+GrLinkTextParser.prototype.addLink =
+ function(text, href, position, length, outputArray) {
+ if (!text) {
+ return;
+ }
+ this.addItem(text, href, null, position, length, outputArray);
+};
+
+GrLinkTextParser.prototype.addHTML =
+ function(html, position, length, outputArray) {
+ this.addItem(null, null, html, position, length, outputArray);
};
GrLinkTextParser.prototype.parse = function(text) {
linkify(text, {
- callback: this.parseChunk.bind(this)
+ callback: this.parseChunk.bind(this),
});
};
@@ -46,6 +116,8 @@
};
GrLinkTextParser.prototype.parseLinks = function(text, patterns) {
+ // The outputArray is used to store all of the matches found for all patterns.
+ var outputArray = [];
for (var p in patterns) {
if (patterns[p].enabled != null && patterns[p].enabled == false) {
continue;
@@ -66,22 +138,29 @@
var pattern = new RegExp(patterns[p].match, 'g');
var match;
- while ((match = pattern.exec(text)) != null) {
- var before = text.substr(0, match.index);
- this.addText(before);
- text = text.substr(match.index + match[0].length);
+ var textToCheck = text;
+ var susbtrIndex = 0;
+
+ while ((match = pattern.exec(textToCheck)) != null) {
+ textToCheck = textToCheck.substr(match.index + match[0].length);
var result = match[0].replace(pattern,
patterns[p].html || patterns[p].link);
if (patterns[p].html) {
- this.addHTML(result);
+ this.addHTML(
+ result, susbtrIndex + match.index, match[0].length, outputArray);
} else if (patterns[p].link) {
- this.addText(match[0], result);
+ this.addLink(match[0], result,
+ susbtrIndex + match.index, match[0].length, outputArray);
} else {
throw Error('linkconfig entry ' + p +
' doesn’t contain a link or html attribute.');
}
+
+ // Update the substring location so we know where we are in relation to
+ // the initial full text string.
+ susbtrIndex = susbtrIndex + match.index + match[0].length;
}
}
- this.addText(text);
+ this.processLinks(text, outputArray);
};
diff --git a/polygerrit-ui/app/elements/shared/gr-select/gr-select.js b/polygerrit-ui/app/elements/shared/gr-select/gr-select.js
index 9e14f08..bef260e9 100644
--- a/polygerrit-ui/app/elements/shared/gr-select/gr-select.js
+++ b/polygerrit-ui/app/elements/shared/gr-select/gr-select.js
@@ -16,40 +16,33 @@
Polymer({
is: 'gr-select',
-
extends: 'select',
-
properties: {
bindValue: {
type: String,
notify: true,
+ observer: '_updateValue',
},
},
- observers: [
- '_valueChanged(bindValue)',
- ],
+ listeners: {
+ change: '_valueChanged',
+ 'dom-change': '_updateValue',
+ },
- attached: function() {
- this.addEventListener('change', function() {
- this.bindValue = this.value;
- });
+ _updateValue: function() {
+ if (this.bindValue) {
+ this.value = this.bindValue;
+ }
+ },
+
+ _valueChanged: function() {
+ this.bindValue = this.value;
},
ready: function() {
// If not set via the property, set bind-value to the element value.
if (!this.bindValue) { this.bindValue = this.value; }
},
-
- _valueChanged: function(bindValue) {
- var options = Polymer.dom(this.root).querySelectorAll('option');
- for (var i = 0; i < options.length; i++) {
- if (options[i].getAttribute('value') === bindValue + '') {
- options[i].setAttribute('selected', true);
- this.value = bindValue;
- break;
- }
- }
- },
});
})();
diff --git a/polygerrit-ui/app/index.html b/polygerrit-ui/app/index.html
index 0e00f77..a5cddad 100644
--- a/polygerrit-ui/app/index.html
+++ b/polygerrit-ui/app/index.html
@@ -23,6 +23,7 @@
<link rel="stylesheet" href="/styles/fonts.css">
<link rel="stylesheet" href="/styles/main.css">
<script src="/bower_components/webcomponentsjs/webcomponents-lite.js"></script>
+<link rel="preload" href="/elements/gr-app.js">
<link rel="import" href="/elements/gr-app.html">
<body unresolved>
diff --git a/polygerrit-ui/app/run_test.sh b/polygerrit-ui/app/run_test.sh
new file mode 100644
index 0000000..b557db8
--- /dev/null
+++ b/polygerrit-ui/app/run_test.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+wct_bin=$(which wct)
+if [[ -z "$wct_bin" ]]; then
+ echo "WCT must be on the path."
+ exit 1
+fi
+
+npm_bin=$(which npm)
+if [[ -z "$npm_bin" ]]; then
+ echo "NPM must be on the path."
+ exit 1
+fi
+
+# WCT tests are not hermetic, and need extra environment variables.
+# TODO(hanwen): does $DISPLAY even work on OSX?
+bazel test \
+ --test_env="HOME=$HOME" \
+ --test_env="WCT=${wct_bin}" \
+ --test_env="WCT_ARGS=${WCT_ARGS}" \
+ --test_env="NPM=${npm_bin}" \
+ --test_env="DISPLAY=${DISPLAY}" \
+ "$@" \
+ //polygerrit-ui/app:wct_test
diff --git a/polygerrit-ui/app/wct_test.sh b/polygerrit-ui/app/wct_test.sh
new file mode 100755
index 0000000..10de424
--- /dev/null
+++ b/polygerrit-ui/app/wct_test.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+
+set -ex
+
+t=$(mktemp -d || mktemp -d -t wct-XXXXXXXXXX)
+components=$TEST_SRCDIR/gerrit/polygerrit-ui/app/test_components.zip
+code=$TEST_SRCDIR/gerrit/polygerrit-ui/app/pg_code.zip
+
+echo $t
+unzip -qd $t $components
+unzip -qd $t $code
+mkdir -p $t/test
+cp $TEST_SRCDIR/gerrit/polygerrit-ui/app/test/index.html $t/test/
+
+# For some reason wct tries to install selenium into its node_modules
+# directory on first run. If you've installed into /usr/local and
+# aren't running wct as root, you're screwed. Turning this option off
+# through skipSeleniumInstall seems to still work, so there's that.
+
+# Sauce tests are disabled by default in order to run local tests
+# only. Run it with (saucelabs.com account required; free for open
+# source): WCT_ARGS='--plugin sauce' buck test --no-results-cache
+# --include web
+
+cat <<EOF > $t/wct.conf.js
+module.exports = {
+ 'suites': ['test'],
+ 'webserver': {
+ 'pathMappings': [
+ {'/components/bower_components': 'bower_components'}
+ ]
+ },
+ 'plugins': {
+ 'local': {
+ 'skipSeleniumInstall': true
+ },
+ 'sauce': {
+ 'disabled': true,
+ 'browsers': [
+ 'OS X 10.11/chrome',
+ 'Windows 10/chrome',
+ 'Linux/firefox',
+ 'OS X 10.11/safari',
+ 'Windows 10/microsoftedge'
+ ]
+ }
+ }
+ };
+EOF
+
+export PATH="$(dirname $WCT):$(dirname $NPM):$PATH"
+
+cd $t
+test -n "${WCT}"
+
+$(basename ${WCT}) ${WCT_ARGS}
diff --git a/tools/bzl/gwt.bzl b/tools/bzl/gwt.bzl
index c27e28f..7eff506 100644
--- a/tools/bzl/gwt.bzl
+++ b/tools/bzl/gwt.bzl
@@ -12,10 +12,72 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-# GWT Rules Skylark rules for building [GWT](http://www.gwtproject.org/)
-# modules using Bazel.
+# Port of Buck native gwt_binary() rule. See discussion in context of
+# https://github.com/facebook/buck/issues/109
+load('//tools/bzl:genrule2.bzl', 'genrule2')
load('//tools/bzl:java.bzl', 'java_library2')
+jar_filetype = FileType(['.jar'])
+
+BROWSERS = [
+ 'chrome',
+ 'firefox',
+ 'gecko1_8',
+ 'safari',
+ 'msie', 'ie8', 'ie9', 'ie10',
+ 'edge',
+]
+ALIASES = {
+ 'chrome': 'safari',
+ 'firefox': 'gecko1_8',
+ 'msie': 'ie10',
+ 'edge': 'gecko1_8',
+}
+
+MODULE = 'com.google.gerrit.GerritGwtUI'
+
+GWT_COMPILER = "com.google.gwt.dev.Compiler"
+
+GWT_JVM_ARGS = ['-Xmx512m']
+
+GWT_COMPILER_ARGS = [
+ '-XdisableClassMetadata',
+]
+
+GWT_COMPILER_ARGS_RELEASE_MODE = GWT_COMPILER_ARGS + [
+ '-XdisableCastChecking',
+]
+
+GWT_TRANSITIVE_DEPS = [
+ '//lib/gwt:ant',
+ '//lib/gwt:colt',
+ '//lib/gwt:javax-validation',
+ '//lib/gwt:javax-validation_src',
+ '//lib/gwt:jsinterop-annotations',
+ '//lib/gwt:jsinterop-annotations_src',
+ '//lib/gwt:tapestry',
+ '//lib/gwt:w3c-css-sac',
+ '//lib/ow2:ow2-asm',
+ '//lib/ow2:ow2-asm-analysis',
+ '//lib/ow2:ow2-asm-commons',
+ '//lib/ow2:ow2-asm-tree',
+ '//lib/ow2:ow2-asm-util',
+]
+
+DEPS = GWT_TRANSITIVE_DEPS + [
+ '//gerrit-gwtexpui:CSS',
+ '//lib:gwtjsonrpc',
+ '//lib/gwt:dev',
+ '@jgit_src//file',
+]
+
+USER_AGENT_XML = """<module rename-to='gerrit_ui'>
+<inherits name='%s'/>
+<set-property name='user.agent' value='%s'/>
+<set-property name='locale' value='default'/>
+</module>
+"""
+
def gwt_module(gwt_xml=None, resources=[], srcs=[], **kwargs):
if gwt_xml:
resources += [gwt_xml]
@@ -24,3 +86,201 @@
srcs = srcs,
resources = resources,
**kwargs)
+
+def _gwt_user_agent_module(ctx):
+ """Generate user agent specific GWT module."""
+ if not ctx.attr.user_agent:
+ return None
+
+ ua = ctx.attr.user_agent
+ impl = ua
+ if ua in ALIASES:
+ impl = ALIASES[ua]
+
+ # intermediate artifact: user agent speific GWT xml file
+ gwt_user_agent_xml = ctx.new_file(ctx.label.name + "_gwt.xml")
+ ctx.file_action(output = gwt_user_agent_xml,
+ content=USER_AGENT_XML % (MODULE, impl))
+
+ # intermediate artifact: user agent specific zip with GWT module
+ gwt_user_agent_zip = ctx.new_file(ctx.label.name + "_gwt.zip")
+ gwt = '%s_%s.gwt.xml' % (MODULE.replace('.', '/'), ua)
+ dir = gwt_user_agent_zip.path + ".dir"
+ cmd = " && ".join([
+ "p=$PWD",
+ "mkdir -p %s" % dir,
+ "cd %s" % dir,
+ "mkdir -p $(dirname %s)" % gwt,
+ "cp $p/%s %s" % (gwt_user_agent_xml.path, gwt),
+ "$p/%s cC $p/%s $(find . | sed 's|^./||')" % (ctx.executable._zip.path, gwt_user_agent_zip.path)
+ ])
+ ctx.action(
+ inputs = [gwt_user_agent_xml] + ctx.files._zip,
+ outputs = [gwt_user_agent_zip],
+ command = cmd,
+ mnemonic = "GenerateUserAgentGWTModule")
+
+ return struct(
+ zip=gwt_user_agent_zip,
+ module=MODULE + '_' + ua
+ )
+
+def _gwt_binary_impl(ctx):
+ module = MODULE
+ output_zip = ctx.outputs.output
+ output_dir = output_zip.path + '.gwt_output'
+ deploy_dir = output_zip.path + '.gwt_deploy'
+
+ deps = _get_transitive_closure(ctx)
+
+ paths = []
+ for dep in deps:
+ paths.append(dep.path)
+
+ gwt_user_agent_modules = []
+ ua = _gwt_user_agent_module(ctx)
+ if ua:
+ paths.append(ua.zip.path)
+ gwt_user_agent_modules.append(ua.zip)
+ module = ua.module
+
+ cmd = "external/local_jdk/bin/java %s -Dgwt.normalizeTimestamps=true -cp %s %s -war %s -deploy %s " % (
+ " ".join(ctx.attr.jvm_args),
+ ":".join(paths),
+ GWT_COMPILER,
+ output_dir,
+ deploy_dir,
+ )
+ # TODO(davido): clean up command concatenation
+ cmd += " ".join([
+ "-style %s" % ctx.attr.style,
+ "-optimize %s" % ctx.attr.optimize,
+ "-strict",
+ " ".join(ctx.attr.compiler_args),
+ module + "\n",
+ "rm -rf %s/gwt-unitCache\n" % output_dir,
+ "root=`pwd`\n",
+ "cd %s; $root/%s Cc ../%s $(find .)\n" % (
+ output_dir,
+ ctx.executable._zip.path,
+ output_zip.basename,
+ )
+ ])
+
+ ctx.action(
+ inputs = list(deps) + ctx.files._jdk + ctx.files._zip + gwt_user_agent_modules,
+ outputs = [output_zip],
+ mnemonic = "GwtBinary",
+ progress_message = "GWT compiling " + output_zip.short_path,
+ command = "set -e\n" + cmd,
+ )
+
+def _get_transitive_closure(ctx):
+ deps = set()
+ for dep in ctx.attr.module_deps:
+ deps += dep.java.transitive_runtime_deps
+ deps += dep.java.transitive_source_jars
+ for dep in ctx.attr.deps:
+ if hasattr(dep, 'java'):
+ deps += dep.java.transitive_runtime_deps
+ elif hasattr(dep, 'files'):
+ deps += dep.files
+
+ return deps
+
+gwt_binary = rule(
+ implementation = _gwt_binary_impl,
+ attrs = {
+ "user_agent": attr.string(),
+ "style": attr.string(default = "OBF"),
+ "optimize": attr.string(default = "9"),
+ "deps": attr.label_list(allow_files=jar_filetype),
+ "module_deps": attr.label_list(allow_files=jar_filetype),
+ "compiler_args": attr.string_list(),
+ "jvm_args": attr.string_list(),
+ "_jdk": attr.label(
+ default=Label("//tools/defaults:jdk")),
+ "_zip": attr.label(
+ default=Label("@bazel_tools//tools/zip:zipper"),
+ cfg = "host",
+ executable=True,
+ single_file=True),
+ },
+ outputs = {
+ "output": "%{name}.zip",
+ },
+)
+
+def gwt_genrule(suffix = ""):
+ dbg = 'ui_dbg' + suffix
+ opt = 'ui_opt' + suffix
+ module_dep = ':ui_module' + suffix
+ args = GWT_COMPILER_ARGS_RELEASE_MODE if suffix == "_r" else GWT_COMPILER_ARGS
+
+ genrule2(
+ name = 'ui_optdbg' + suffix,
+ srcs = [
+ ':' + dbg,
+ ':' + opt,
+ ],
+ cmd = 'cd $$TMP;' +
+ 'unzip -q $$ROOT/$(location :%s);' % dbg +
+ 'mv' +
+ ' gerrit_ui/gerrit_ui.nocache.js' +
+ ' gerrit_ui/dbg_gerrit_ui.nocache.js;' +
+ 'unzip -qo $$ROOT/$(location :%s);' % opt +
+ 'mkdir -p $$(dirname $@);' +
+ 'zip -qr $$ROOT/$@ .',
+ outs = ['ui_optdbg' + suffix + '.zip'],
+ visibility = ['//visibility:public'],
+ )
+
+ gwt_binary(
+ name = opt,
+ module_deps = [module_dep],
+ deps = DEPS,
+ compiler_args = args,
+ jvm_args = GWT_JVM_ARGS,
+ )
+
+ gwt_binary(
+ name = dbg,
+ style = 'PRETTY',
+ optimize = "0",
+ module_deps = [module_dep],
+ deps = DEPS,
+ compiler_args = GWT_COMPILER_ARGS,
+ jvm_args = GWT_JVM_ARGS,
+ )
+
+def gen_ui_module(name, suffix = ""):
+ gwt_module(
+ name = name + suffix,
+ srcs = native.glob(['src/main/java/**/*.java']),
+ gwt_xml = 'src/main/java/%s.gwt.xml' % MODULE.replace('.', '/'),
+ resources = native.glob(
+ ['src/main/java/**/*'],
+ exclude = ['src/main/java/**/*.java'] +
+ ['src/main/java/%s.gwt.xml' % MODULE.replace('.', '/')]),
+ deps = [
+ '//gerrit-gwtui-common:diffy_logo',
+ '//gerrit-gwtui-common:client',
+ '//gerrit-gwtexpui:CSS',
+ '//lib/codemirror:codemirror' + suffix,
+ '//lib/gwt:user',
+ ],
+ visibility = ['//visibility:public'],
+ )
+
+def gwt_user_agent_permutations():
+ for ua in BROWSERS:
+ gwt_binary(
+ name = "ui_%s" % ua,
+ user_agent = ua,
+ style = 'PRETTY',
+ optimize = "0",
+ module_deps = [':ui_module'],
+ deps = DEPS,
+ compiler_args = GWT_COMPILER_ARGS,
+ jvm_args = GWT_JVM_ARGS,
+ )
diff --git a/tools/js/bower2bazel.py b/tools/js/bower2bazel.py
index 33f7320..08a997f 100755
--- a/tools/js/bower2bazel.py
+++ b/tools/js/bower2bazel.py
@@ -39,6 +39,7 @@
# TODO(hanwen): remove these, and add appropriate license files under //lib
"BSD": "polymer",
"MIT": "polymer",
+ "BSD-3-Clause": "polymer",
}
# list of licenses for packages that don't specify one in their bower.json file.
@@ -47,8 +48,11 @@
"fetch": "fetch",
"moment": "moment",
"page": "page.js",
+ "lodash": "polymer", # MIT, actually.
"promise-polyfill": "promise-polyfill",
"webcomponentsjs": "polymer", # self-identifies as BSD.
+ "sinon-chai": "polymer", # WTFPL & BSD.
+ "sinonjs": "polymer", # BSD.
}
@@ -201,7 +205,7 @@
license = license_map.get(license, license)
else:
if pkg_name not in package_licenses:
- msg = "package %s does not specify license." % pkg_name
+ msg = "package %s does not specify license: %s" % (pkg_name, pkg)
sys.stderr.write(msg)
raise Exception(msg)
license = package_licenses[pkg_name]
diff --git a/tools/maven/BUCK b/tools/maven/BUCK
index 322b5a2..0541fc0 100644
--- a/tools/maven/BUCK
+++ b/tools/maven/BUCK
@@ -1,6 +1,6 @@
-include_defs('//VERSION')
include_defs('//tools/maven/package.defs')
include_defs('//tools/maven/repository.defs')
+include_defs('//version.bzl')
if GERRIT_VERSION.endswith('-SNAPSHOT'):
URL = MAVEN_SNAPSHOT_URL
diff --git a/tools/maven/BUILD b/tools/maven/BUILD
new file mode 100644
index 0000000..14eb2be
--- /dev/null
+++ b/tools/maven/BUILD
@@ -0,0 +1,31 @@
+load('//:version.bzl', 'GERRIT_VERSION')
+load('//tools/maven:package.bzl', 'maven_package')
+
+MAVEN_REPOSITORY = 'sonatype-nexus-staging'
+# TODO(davido): support snapshot repositories
+MAVEN_RELEASE_URL = 'https://oss.sonatype.org/service/local/staging/deploy/maven2'
+
+maven_package(
+ repository = MAVEN_REPOSITORY,
+ url = MAVEN_RELEASE_URL,
+ version = GERRIT_VERSION,
+ jar = {
+ 'gerrit-acceptance-framework': '//gerrit-acceptance-framework:acceptance-framework_deploy.jar',
+ 'gerrit-extension-api': '//gerrit-extension-api:extension-api_deploy.jar',
+ 'gerrit-plugin-api': '//gerrit-plugin-api:plugin-api_deploy.jar',
+ 'gerrit-plugin-gwtui': '//gerrit-plugin-gwtui:gwtui-api_deploy.jar',
+ },
+ src = {
+ 'gerrit-acceptance-framework': '//gerrit-acceptance-framework:liblib-src.jar',
+ 'gerrit-extension-api': '//gerrit-extension-api:libapi-src.jar',
+ 'gerrit-plugin-api': '//gerrit-plugin-api:plugin-api-sources_deploy.jar',
+ 'gerrit-plugin-gwtui': '//gerrit-plugin-gwtui:gwtui-api-source_deploy.jar',
+ },
+ doc = {
+ 'gerrit-acceptance-framework': '//gerrit-acceptance-framework:acceptance-framework-javadoc',
+ 'gerrit-extension-api': '//gerrit-extension-api:extension-api-javadoc',
+ 'gerrit-plugin-api': '//gerrit-plugin-api:plugin-api-javadoc',
+ 'gerrit-plugin-gwtui': '//gerrit-plugin-gwtui:gwtui-api-javadoc',
+ },
+ war = {'gerrit-war': '//:release'},
+)
diff --git a/tools/maven/api.sh b/tools/maven/api.sh
index c7ce65e..93b5f2e 100755
--- a/tools/maven/api.sh
+++ b/tools/maven/api.sh
@@ -14,16 +14,20 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-if [[ "$#" == "0" ]] ; then
+if [[ "$#" != "2" ]] ; then
cat <<EOF
-Usage: run "$0 COMMAND" from the top of your workspace, where
-COMMAND is one of
+Usage: run "$0 COMMAND BUILD-TOOL" from the top of your workspace,
+where COMMAND is one of
install
deploy
war_install
war_deploy
+and BUILD-TOOL is one of
+
+ buck
+ bazel
Set VERBOSE in the environment to get more information.
EOF
@@ -54,16 +58,33 @@
;;
esac
+case "$2" in
+bazel)
+ buildProc=bazel
+ ;;
+buck)
+ buildProc=buck
+ ;;
+*)
+ echo "unknown build-tool $2. Should be buck or bazel."
+ exit 1
+ ;;
+esac
+
if [[ "${VERBOSE:-x}" != "x" ]]; then
set -o xtrace
fi
-buck build //tools/maven:gen_${command} || \
- { echo "buck failed to build gen_${command}. Use VERBOSE=1 for more info" ; exit 1 ; }
+$buildProc build //tools/maven:gen_${command} || \
+ { echo "$buildProc failed to build gen_${command}. Use VERBOSE=1 for more info" ; exit 1 ; }
-script="./buck-out/gen/tools/maven/gen_${command}/${command}.sh"
-
-# The PEX wrapper does some funky exit handling, so even if the script
-# does "exit(0)", the return status is '1'. So we can't tell if the
-# following invocation was successful.
-${script}
+if [[ "$buildProc" = "bazel" ]]; then
+ script="./bazel-genfiles/tools/maven/${command}.sh"
+ ${script}
+else
+ script="./buck-out/gen/tools/maven/gen_${command}/${command}.sh"
+ # The PEX wrapper does some funky exit handling, so even if the script
+ # does "exit(0)", the return status is '1'. So we can't tell if the
+ # following invocation was successful.
+ ${script}
+fi
diff --git a/tools/maven/package.bzl b/tools/maven/package.bzl
new file mode 100644
index 0000000..fbd08c6
--- /dev/null
+++ b/tools/maven/package.bzl
@@ -0,0 +1,93 @@
+# 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.
+
+sh_bang_template = (' && '.join([
+ "echo '#!/bin/bash -e' > $@",
+ 'echo "# this script should run from the root of your workspace." >> $@',
+ 'echo "" >> $@',
+ "echo 'if [[ \"$$VERBOSE\" ]]; then set -x ; fi' >> $@",
+ 'echo "" >> $@',
+ 'echo %s >> $@',
+ 'echo "" >> $@',
+ 'echo %s >> $@']))
+
+def maven_package(
+ version,
+ repository = None,
+ url = None,
+ jar = {},
+ src = {},
+ doc = {},
+ war = {}):
+
+ build_cmd = ['bazel', 'build']
+ mvn_cmd = ['python', 'tools/maven/mvn.py', '-v', version]
+ api_cmd = mvn_cmd[:]
+ api_targets = []
+ for type,d in [('jar', jar), ('java-source', src), ('javadoc', doc)]:
+ for a,t in sorted(d.items()):
+ api_cmd.append('-s %s:%s:$(location %s)' % (a,type,t))
+ api_targets.append(t)
+
+ native.genrule(
+ name = 'gen_api_install',
+ cmd = sh_bang_template % (
+ ' '.join(build_cmd + api_targets),
+ ' '.join(api_cmd + ['-a', 'install'])),
+ srcs = api_targets,
+ outs = ['api_install.sh'],
+ executable = True,
+ )
+
+ if repository and url:
+ native.genrule(
+ name = 'gen_api_deploy',
+ cmd = sh_bang_template % (
+ ' '.join(build_cmd + api_targets),
+ ' '.join(api_cmd + ['-a', 'deploy',
+ '--repository', repository,
+ '--url', url])),
+ srcs = api_targets,
+ outs = ['api_deploy.sh'],
+ executable = True,
+ )
+
+ war_cmd = mvn_cmd[:]
+ war_targets = []
+ for a,t in sorted(war.items()):
+ war_cmd.append('-s %s:war:$(location %s)' % (a,t))
+ war_targets.append(t)
+
+ native.genrule(
+ name = 'gen_war_install',
+ cmd = sh_bang_template % (' '.join(build_cmd + war_targets),
+ ' '.join(war_cmd + ['-a', 'install'])),
+ srcs = war_targets,
+ outs = ['war_install.sh'],
+ executable = True,
+ )
+
+ if repository and url:
+ native.genrule(
+ name = 'gen_war_deploy',
+ cmd = sh_bang_template % (
+ ' '.join(build_cmd + war_targets),
+ ' '.join(war_cmd + [
+ '-a', 'deploy',
+ '--repository', repository,
+ '--url', url])),
+ srcs = war_targets,
+ outs = ['war_deploy.sh'],
+ executable = True,
+ )
diff --git a/tools/maven/package.defs b/tools/maven/package.defs
index c412ebd..a557170 100644
--- a/tools/maven/package.defs
+++ b/tools/maven/package.defs
@@ -13,10 +13,10 @@
# limitations under the License.
sh_bang_template = (' && '.join([
- "echo '#!/bin/bash -eu' > $OUT",
+ "echo '#!/bin/bash -e' > $OUT",
'echo "# this script should run from the root of your workspace." >> $OUT',
'echo "" >> $OUT',
- "echo 'if [[ -n \"$${VERBOSE:-}\" ]]; then set -x ; fi' >> $OUT",
+ "echo 'if [[ \"${VERBOSE}\" ]]; then set -x ; fi' >> $OUT",
'echo "" >> $OUT',
'echo %s >> $OUT',
'echo "" >> $OUT',
diff --git a/tools/version.py b/tools/version.py
index 9f03a59..eac2700 100755
--- a/tools/version.py
+++ b/tools/version.py
@@ -53,7 +53,7 @@
replace_in_file(pom, src_pattern)
src_pattern = re.compile(r"^(GERRIT_VERSION = ')([-.\w]+)(')$", re.MULTILINE)
-replace_in_file('VERSION', src_pattern)
+replace_in_file('version.bzl', src_pattern)
src_pattern = re.compile(r'^(\s*-DarchetypeVersion=)([-.\w]+)(\s*\\)$',
re.MULTILINE)
diff --git a/tools/workspace-status.sh b/tools/workspace-status.sh
index 6f17754..cbc263e 100755
--- a/tools/workspace-status.sh
+++ b/tools/workspace-status.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
# This script will be run by bazel when the build process starts to
# generate key-value information that represents the status of the
diff --git a/VERSION b/version.bzl
similarity index 100%
rename from VERSION
rename to version.bzl
