Revert "Revert new ChangeRebuilder event changes"
This reverts commit 2249c68450bf0a099e2b8bfbc9b5250c4da79903.
GERRIT_NOTE_DB=check failures in CommentsIT are flaky and present
even before this revert.
Change-Id: I2255be6852fb30bb1697a8a0d3b490a3f1d28916
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeMessageEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeMessageEvent.java
index a990e19..bc8af82 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeMessageEvent.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeMessageEvent.java
@@ -31,11 +31,6 @@
private static final Pattern TOPIC_REMOVED_REGEXP =
Pattern.compile("^Topic (.+) removed$");
- private static final Pattern STATUS_ABANDONED_REGEXP =
- Pattern.compile("^Abandoned(\n.*)*$");
- private static final Pattern STATUS_RESTORED_REGEXP =
- Pattern.compile("^Restored(\n.*)*$");
-
private final ChangeMessage message;
private final Change noteDbChange;
@@ -57,7 +52,6 @@
checkUpdate(update);
update.setChangeMessage(message.getMessage());
setTopic(update);
- setStatus(update);
}
private void setTopic(ChangeUpdate update) {
@@ -86,21 +80,4 @@
noteDbChange.setTopic(null);
}
}
-
- private void setStatus(ChangeUpdate update) {
- String msg = message.getMessage();
- if (msg == null) {
- return;
- }
- if (STATUS_ABANDONED_REGEXP.matcher(msg).matches()) {
- update.setStatus(Change.Status.ABANDONED);
- noteDbChange.setStatus(Change.Status.ABANDONED);
- return;
- }
-
- if (STATUS_RESTORED_REGEXP.matcher(msg).matches()) {
- update.setStatus(Change.Status.NEW);
- noteDbChange.setStatus(Change.Status.NEW);
- }
- }
}
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 d4caa6c..cdd895a 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
@@ -355,19 +355,16 @@
Change noteDbChange = new Change(null, null, null, null, null);
for (ChangeMessage msg : bundle.getChangeMessages()) {
- if (msg.getPatchSetId() == null) {
- // No dependency necessary; will get assigned to most recent patch set
- // in sortAndFillEvents.
- events.add(
- new ChangeMessageEvent(msg, noteDbChange, change.getCreatedOn()));
- continue;
+ 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);
+ }
+ }
}
- PatchSetEvent pse = patchSetEvents.get(msg.getPatchSetId());
- if (pse != null) {
- events.add(
- new ChangeMessageEvent(msg, noteDbChange, change.getCreatedOn())
- .addDep(pse));
- }
+ events.addAll(msgEvents);
}
sortAndFillEvents(change, noteDbChange, events, minPsNum);
@@ -396,6 +393,18 @@
}
}
+ private List<Event> parseChangeMessage(ChangeMessage msg, Change change,
+ Change noteDbChange) {
+ List<Event> events = new ArrayList<>(2);
+ events.add(new ChangeMessageEvent(msg, noteDbChange, change.getCreatedOn()));
+ Optional<StatusChangeEvent> sce =
+ StatusChangeEvent.parseFromMessage(msg, change, noteDbChange);
+ if (sce.isPresent()) {
+ events.add(sce.get());
+ }
+ return events;
+ }
+
private static Integer getMinPatchSetNum(ChangeBundle bundle) {
Integer minPsNum = null;
for (PatchSet ps : bundle.getPatchSets()) {
@@ -418,8 +427,8 @@
private void sortAndFillEvents(Change change, Change noteDbChange,
List<Event> events, Integer minPsNum) {
- new EventSorter(events).sort();
events.add(new FinalUpdatesEvent(change, noteDbChange));
+ new EventSorter(events).sort();
// Ensure the first event in the list creates the change, setting the author
// and any required footers.
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/Event.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/Event.java
index d04d1b5..b7b08b0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/Event.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/Event.java
@@ -79,10 +79,6 @@
abstract void apply(ChangeUpdate update) throws OrmException, IOException;
- protected boolean isPatchSet() {
- return false;
- }
-
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
@@ -95,6 +91,7 @@
@Override
public int compareTo(Event other) {
return ComparisonChain.start()
+ .compareFalseFirst(this.isFinalUpdates(), other.isFinalUpdates())
.compare(this.when, other.when)
.compareTrueFirst(isPatchSet(), isPatchSet())
.compareTrueFirst(this.predatesChange, other.predatesChange)
@@ -103,4 +100,12 @@
ReviewDbUtil.intKeyOrdering().nullsLast())
.result();
}
+
+ private boolean isPatchSet() {
+ return this instanceof PatchSetEvent;
+ }
+
+ private boolean isFinalUpdates() {
+ return this instanceof FinalUpdatesEvent;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/FinalUpdatesEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/FinalUpdatesEvent.java
index a700f18..26f6b1f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/FinalUpdatesEvent.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/FinalUpdatesEvent.java
@@ -46,7 +46,8 @@
// TODO(dborowitz): Stamp approximate approvals at this time.
update.fixStatus(change.getStatus());
}
- if (change.getSubmissionId() != null) {
+ if (change.getSubmissionId() != null
+ && noteDbChange.getSubmissionId() == null) {
update.setSubmissionId(change.getSubmissionId());
}
if (!Objects.equals(change.getAssignee(), noteDbChange.getAssignee())) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/PatchSetEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/PatchSetEvent.java
index 5baddd3..c3fb267 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/PatchSetEvent.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/PatchSetEvent.java
@@ -66,11 +66,6 @@
}
}
- @Override
- protected boolean isPatchSet() {
- return true;
- }
-
private void setRevision(ChangeUpdate update, PatchSet ps)
throws IOException {
String rev = ps.getRevision().get();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/StatusChangeEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/StatusChangeEvent.java
new file mode 100644
index 0000000..29e0868
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/StatusChangeEvent.java
@@ -0,0 +1,90 @@
+// 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.notedb.rebuild;
+
+import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.ChangeMessage;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gwtorm.server.OrmException;
+
+import java.sql.Timestamp;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+class StatusChangeEvent extends Event {
+ private static final ImmutableMap<Change.Status, Pattern> PATTERNS =
+ ImmutableMap.of(
+ Change.Status.ABANDONED, Pattern.compile("^Abandoned(\n.*)*$"),
+ Change.Status.MERGED, Pattern.compile(
+ "^Change has been successfully"
+ + " (merged|cherry-picked|rebased|pushed).*$"),
+ Change.Status.NEW, Pattern.compile("^Restored(\n.*)*$"));
+
+ static Optional<StatusChangeEvent> parseFromMessage(ChangeMessage message,
+ Change change, Change noteDbChange) {
+ String msg = message.getMessage();
+ if (msg == null) {
+ return Optional.absent();
+ }
+ for (Map.Entry<Change.Status, Pattern> e : PATTERNS.entrySet()) {
+ if (e.getValue().matcher(msg).matches()) {
+ return Optional.of(new StatusChangeEvent(
+ message, change, noteDbChange, e.getKey()));
+ }
+ }
+ return Optional.absent();
+ }
+
+ private final Change change;
+ private final Change noteDbChange;
+ private final Change.Status status;
+
+ private StatusChangeEvent(ChangeMessage message, Change change,
+ Change noteDbChange, Change.Status status) {
+ this(message.getPatchSetId(), message.getAuthor(),
+ message.getWrittenOn(), change, noteDbChange, message.getTag(),
+ status);
+ }
+
+ private StatusChangeEvent(PatchSet.Id psId, Account.Id author,
+ Timestamp when, Change change, Change noteDbChange,
+ String tag, Change.Status status) {
+ super(psId, author, when, change.getCreatedOn(), tag);
+ this.change = change;
+ this.noteDbChange = noteDbChange;
+ this.status = status;
+ }
+
+ @Override
+ boolean uniquePerUpdate() {
+ return true;
+ }
+
+ @SuppressWarnings("deprecation")
+ @Override
+ void apply(ChangeUpdate update) throws OrmException {
+ checkUpdate(update);
+ update.fixStatus(status);
+ noteDbChange.setStatus(status);
+ if (status == Change.Status.MERGED) {
+ update.setSubmissionId(change.getSubmissionId());
+ noteDbChange.setSubmissionId(change.getSubmissionId());
+ }
+ }
+}
