Store topic in notedb when notedb is rebuilt
In the database only the current topic is stored explictly. This is why we must parse the change messages to rebuild the topic history. Rebuilding the topic history based on parsing the change messages is a heuristic, which may not work in all cases. To ensure that the current topic is correctly set in notedb a final FixChangeEvent is added as last event to the event list that is used to build up the notedb for one change. The FinalUpdatesEvent will set the current topic in notedb if it wasn't set before. To know whether the topic needs to be set we keep track of the change state in notedb while the events are processed. All fixes done by the FinalUpdatesEvent (at the moment only an update of the topic, but more in future) will be done in a seperate commit on the notes branch. This means these updates will not be squashed with the updates from other events. This way there is a clear seperation between normal updates and the fix update. Also the FixChangeEvent will only lead to an update if there is anything to fix. If not, it will not create a commit on the notes branch. Change-Id: I489ddcbc1b71bbec786141f92f2f51d4644bf085 Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
@@ -180,6 +180,9 @@ public abstract class AbstractChangeUpdate extends VersionedMetaData {
|
||||
public abstract void writeCommit(BatchMetaDataUpdate batch)
|
||||
throws OrmException, IOException;
|
||||
|
||||
/** Whether no updates have been done. */
|
||||
public abstract boolean isEmpty();
|
||||
|
||||
/**
|
||||
* @return the NameKey for the project where the update will be stored,
|
||||
* which is not necessarily the same as the change's project.
|
||||
|
||||
@@ -288,7 +288,8 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
|
||||
return true;
|
||||
}
|
||||
|
||||
private boolean isEmpty() {
|
||||
@Override
|
||||
public boolean isEmpty() {
|
||||
return deleteComments.isEmpty()
|
||||
&& upsertComments.isEmpty();
|
||||
}
|
||||
|
||||
@@ -60,6 +60,8 @@ import java.util.List;
|
||||
import java.util.Objects;
|
||||
import java.util.concurrent.Callable;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import java.util.regex.Matcher;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
public class ChangeRebuilder {
|
||||
private static final long TS_WINDOW_MS =
|
||||
@@ -132,18 +134,18 @@ public class ChangeRebuilder {
|
||||
events.add(new ApprovalEvent(psa));
|
||||
}
|
||||
|
||||
Change notedbChange = new Change(null, null, null, null, null);
|
||||
for (ChangeMessage msg : db.changeMessages().byChange(changeId)) {
|
||||
events.add(new ChangeMessageEvent(msg));
|
||||
events.add(new ChangeMessageEvent(msg, notedbChange));
|
||||
}
|
||||
|
||||
Collections.sort(events);
|
||||
events.add(new FinalUpdatesEvent(change, notedbChange));
|
||||
BatchMetaDataUpdate batch = null;
|
||||
ChangeUpdate update = null;
|
||||
for (Event e : events) {
|
||||
if (!sameUpdate(e, update)) {
|
||||
if (update != null) {
|
||||
writeToBatch(batch, update, changeRepo);
|
||||
}
|
||||
IdentifiedUser user = userFactory.create(dbProvider, e.who);
|
||||
update = updateFactory.create(
|
||||
controlFactory.controlFor(change, user), e.when);
|
||||
@@ -155,9 +157,7 @@ public class ChangeRebuilder {
|
||||
e.apply(update);
|
||||
}
|
||||
if (batch != null) {
|
||||
if (update != null) {
|
||||
writeToBatch(batch, update, changeRepo);
|
||||
}
|
||||
|
||||
// Since the BatchMetaDataUpdates generated by all ChangeRebuilders on a
|
||||
// given project are backed by the same BatchRefUpdate, we need to
|
||||
@@ -237,6 +237,10 @@ public class ChangeRebuilder {
|
||||
private void writeToBatch(BatchMetaDataUpdate batch,
|
||||
AbstractChangeUpdate update, Repository repo) throws IOException,
|
||||
OrmException {
|
||||
if (update == null || update.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
|
||||
try (ObjectInserter inserter = repo.newObjectInserter()) {
|
||||
update.setInserter(inserter);
|
||||
update.writeCommit(batch);
|
||||
@@ -251,7 +255,8 @@ public class ChangeRebuilder {
|
||||
return update != null
|
||||
&& round(event.when) == round(update.getWhen())
|
||||
&& event.who.equals(update.getUser().getAccountId())
|
||||
&& event.psId.equals(update.getPatchSetId());
|
||||
&& event.psId.equals(update.getPatchSetId())
|
||||
&& !(event instanceof FinalUpdatesEvent);
|
||||
}
|
||||
|
||||
private abstract static class Event implements Comparable<Event> {
|
||||
@@ -372,18 +377,74 @@ public class ChangeRebuilder {
|
||||
}
|
||||
|
||||
private static class ChangeMessageEvent extends Event {
|
||||
private final ChangeMessage message;
|
||||
private static final Pattern TOPIC_SET_REGEXP =
|
||||
Pattern.compile("^Topic set to (.+)$");
|
||||
private static final Pattern TOPIC_CHANGED_REGEXP =
|
||||
Pattern.compile("^Topic changed from (.+) to (.+)$");
|
||||
private static final Pattern TOPIC_REMOVED_REGEXP =
|
||||
Pattern.compile("^Topic (.+) removed$");
|
||||
|
||||
ChangeMessageEvent(ChangeMessage message) {
|
||||
private final ChangeMessage message;
|
||||
private final Change notedbChange;
|
||||
|
||||
ChangeMessageEvent(ChangeMessage message, Change notedbChange) {
|
||||
super(message.getPatchSetId(), message.getAuthor(),
|
||||
message.getWrittenOn());
|
||||
this.message = message;
|
||||
this.notedbChange = notedbChange;
|
||||
}
|
||||
|
||||
@Override
|
||||
void apply(ChangeUpdate update) throws OrmException {
|
||||
checkUpdate(update);
|
||||
update.setChangeMessage(message.getMessage());
|
||||
setTopic(update);
|
||||
}
|
||||
|
||||
private void setTopic(ChangeUpdate update) {
|
||||
String msg = message.getMessage();
|
||||
Matcher m = TOPIC_SET_REGEXP.matcher(msg);
|
||||
if (m.matches()) {
|
||||
String topic = m.group(1);
|
||||
update.setTopic(topic);
|
||||
notedbChange.setTopic(topic);
|
||||
return;
|
||||
}
|
||||
|
||||
m = TOPIC_CHANGED_REGEXP.matcher(msg);
|
||||
if (m.matches()) {
|
||||
String topic = m.group(2);
|
||||
update.setTopic(topic);
|
||||
notedbChange.setTopic(topic);
|
||||
return;
|
||||
}
|
||||
|
||||
if (TOPIC_REMOVED_REGEXP.matcher(msg).matches()) {
|
||||
update.setTopic(null);
|
||||
notedbChange.setTopic(null);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static class FinalUpdatesEvent extends Event {
|
||||
private final Change change;
|
||||
private final Change notedbChange;
|
||||
|
||||
FinalUpdatesEvent(Change change, Change notedbChange) {
|
||||
super(change.currentPatchSetId(), change.getOwner(),
|
||||
change.getLastUpdatedOn());
|
||||
this.change = change;
|
||||
this.notedbChange = notedbChange;
|
||||
}
|
||||
|
||||
@Override
|
||||
void apply(ChangeUpdate update) throws OrmException {
|
||||
if (!Objects.equals(change.getTopic(), notedbChange.getTopic())) {
|
||||
update.setTopic(change.getTopic());
|
||||
}
|
||||
if (!update.isEmpty()) {
|
||||
update.setSubjectForCommit("Final notedb migration updates");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -535,7 +535,8 @@ public class ChangeUpdate extends AbstractChangeUpdate {
|
||||
return getProjectName(ctl);
|
||||
}
|
||||
|
||||
private boolean isEmpty() {
|
||||
@Override
|
||||
public boolean isEmpty() {
|
||||
return commitSubject == null
|
||||
&& approvals.isEmpty()
|
||||
&& changeMessage == null
|
||||
|
||||
Reference in New Issue
Block a user