diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java index 9c45aaf007..3322b683f8 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java @@ -363,6 +363,15 @@ class ChangeNotesParser { submissionId = parseSubmissionId(commit); } + if (lastUpdatedOn == null || ts.after(lastUpdatedOn)) { + lastUpdatedOn = ts; + } + + if (deletedPatchSets.contains(psId)) { + // Do not update PS details as PS was deleted and this meta data is of no relevance. + return; + } + // Parse mutable patch set fields first so they can be recorded in the PendingPatchSetFields. parseDescription(psId, commit); parseGroups(psId, commit); @@ -410,10 +419,6 @@ class ChangeNotesParser { previousWorkInProgressFooter = null; parseWorkInProgress(commit); - - if (lastUpdatedOn == null || ts.after(lastUpdatedOn)) { - lastUpdatedOn = ts; - } } private String parseSubmissionId(ChangeNotesCommit commit) throws ConfigInvalidException { @@ -487,10 +492,6 @@ class ChangeNotesParser { throw parseException("patch set %s requires an identified user as uploader", psId.get()); } if (patchSetCommitParsed(psId)) { - if (deletedPatchSets.contains(psId)) { - // Do not update PS details as PS was deleted and this meta data is of no relevance. - return; - } ObjectId commitId = patchSets.get(psId).commitId().orElseThrow(IllegalStateException::new); throw new ConfigInvalidException( String.format( diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java index 5993206eba..145e914e7e 100644 --- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java +++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java @@ -3062,6 +3062,38 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { assertThat(newNotes(c).getUpdateCount()).isEqualTo(3); } + @Test + public void createPatchSetAfterPatchSetDeletion() throws Exception { + Change c = newChange(); + assertThat(newNotes(c).getChange().currentPatchSetId().get()).isEqualTo(1); + + // Create PS2. + incrementCurrentPatchSetFieldOnly(c); + RevCommit commit = tr.commit().message("PS" + c.currentPatchSetId().get()).create(); + ChangeUpdate update = newUpdate(c, changeOwner); + update.setCommit(rw, commit); + update.setGroups(ImmutableList.of(commit.name())); + update.commit(); + assertThat(newNotes(c).getChange().currentPatchSetId().get()).isEqualTo(2); + + // Delete PS2. + update = newUpdate(c, changeOwner); + update.setPatchSetState(PatchSetState.DELETED); + update.commit(); + c = newNotes(c).getChange(); + assertThat(c.currentPatchSetId().get()).isEqualTo(1); + + // Create another PS2 + incrementCurrentPatchSetFieldOnly(c); + commit = tr.commit().message("PS" + c.currentPatchSetId().get()).create(); + update = newUpdate(c, changeOwner); + update.setPatchSetState(PatchSetState.PUBLISHED); + update.setCommit(rw, commit); + update.setGroups(ImmutableList.of(commit.name())); + update.commit(); + assertThat(newNotes(c).getChange().currentPatchSetId().get()).isEqualTo(2); + } + private String readNote(ChangeNotes notes, ObjectId noteId) throws Exception { ObjectId dataId = notes.revisionNoteMap.noteMap.getNote(noteId).getData(); return new String(rw.getObjectReader().open(dataId, OBJ_BLOB).getCachedBytes(), UTF_8);