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 94dcedec12..3d5de080d6 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 @@ -97,14 +97,16 @@ public abstract class AbstractChangeNotes { } protected final Args args; + protected final boolean autoRebuild; private final Change.Id changeId; private ObjectId revision; private boolean loaded; - AbstractChangeNotes(Args args, Change.Id changeId) { + AbstractChangeNotes(Args args, Change.Id changeId, boolean autoRebuild) { this.args = checkNotNull(args); this.changeId = checkNotNull(changeId); + this.autoRebuild = autoRebuild; } public Change.Id getChangeId() { @@ -120,7 +122,9 @@ public abstract class AbstractChangeNotes { if (loaded) { return self(); } - if (!args.migration.readChanges()) { + boolean read = args.migration.readChanges(); + boolean readOrWrite = read || args.migration.writeChanges(); + if (!readOrWrite || !autoRebuild) { loadDefaults(); return self(); } @@ -129,9 +133,15 @@ public abstract class AbstractChangeNotes { } try (Timer1.Context timer = args.metrics.readLatency.start(CHANGES); Repository repo = args.repoManager.openRepository(getProjectName()); + // Call openHandle even if reading is disabled, to trigger + // auto-rebuilding before this object may get passed to a ChangeUpdate. LoadHandle handle = openHandle(repo)) { - revision = handle.id(); - onLoad(handle); + if (read) { + revision = handle.id(); + onLoad(handle); + } else { + loadDefaults(); + } loaded = true; } catch (ConfigInvalidException | IOException e) { throw new OrmException(e); @@ -145,7 +155,11 @@ public abstract class AbstractChangeNotes { } protected LoadHandle openHandle(Repository repo) throws IOException { - return LoadHandle.create(ChangeNotesCommit.newRevWalk(repo), readRef(repo)); + return openHandle(repo, readRef(repo)); + } + + protected LoadHandle openHandle(Repository repo, ObjectId id) { + return LoadHandle.create(ChangeNotesCommit.newRevWalk(repo), id); } public T reload() throws OrmException { 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 ddf09ccbbc..a6cd8fa38f 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 @@ -377,7 +377,6 @@ public class ChangeNotes extends AbstractChangeNotes { private final Project.NameKey project; private final Change change; - private final boolean autoRebuild; private final RefCache refs; private ImmutableSortedMap patchSets; @@ -403,10 +402,9 @@ public class ChangeNotes extends AbstractChangeNotes { private ChangeNotes(Args args, Project.NameKey project, Change change, boolean autoRebuild, @Nullable RefCache refs) { - super(args, change.getId()); + super(args, change.getId(), autoRebuild); this.project = project; this.change = new Change(change); - this.autoRebuild = autoRebuild; this.refs = refs; } @@ -626,26 +624,31 @@ public class ChangeNotes extends AbstractChangeNotes { protected LoadHandle openHandle(Repository repo) throws IOException { if (autoRebuild) { NoteDbChangeState state = NoteDbChangeState.parse(change); + ObjectId id = readRef(repo); + if (state == null && id == null) { + return super.openHandle(repo, id); + } RefCache refs = this.refs != null ? this.refs : new RepoRefCache(repo); if (!NoteDbChangeState.isChangeUpToDate(state, refs, getChangeId())) { - return rebuildAndOpen(repo); + return rebuildAndOpen(repo, id); } } return super.openHandle(repo); } - private LoadHandle rebuildAndOpen(Repository repo) throws IOException { + private LoadHandle rebuildAndOpen(Repository repo, ObjectId oldId) + throws IOException { try { NoteDbChangeState newState = args.rebuilder.get().rebuild(args.db.get(), getChangeId()); if (newState == null) { - return super.openHandle(repo); // May be null in tests. + return super.openHandle(repo, oldId); // May be null in tests. } repo.scanForRepoChanges(); return LoadHandle.create( ChangeNotesCommit.newRevWalk(repo), newState.getChangeMetaId()); } catch (NoSuchChangeException e) { - return super.openHandle(repo); + return super.openHandle(repo, oldId); } catch (OrmException | ConfigInvalidException e) { throw new IOException(e); } 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 068c008e5c..74c27bde40 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 @@ -52,7 +52,6 @@ public class DraftCommentNotes extends AbstractChangeNotes { private final Change change; private final Account.Id author; - private final boolean autoRebuild; private ImmutableListMultimap comments; private RevisionNoteMap revisionNoteMap; @@ -70,10 +69,9 @@ public class DraftCommentNotes extends AbstractChangeNotes { Args args, @Assisted Change.Id changeId, @Assisted Account.Id author) { - super(args, changeId); + super(args, changeId, true); this.change = null; this.author = author; - this.autoRebuild = true; } DraftCommentNotes( @@ -81,10 +79,9 @@ public class DraftCommentNotes extends AbstractChangeNotes { Change change, Account.Id author, boolean autoRebuild) { - super(args, change.getId()); + super(args, change.getId(), autoRebuild); this.change = change; this.author = author; - this.autoRebuild = autoRebuild; } RevisionNoteMap getRevisionNoteMap() {