ChangeNotes: Remove support for readOnlyUntil
This was used only by the primary storage migration code, which is now
gone. No additional data migration is needed:
* Unknown footers in NoteDb commits are silently ignored.
* Deserializing protos persisted in the ChangeNotesCache will ignore
the unknown fields.
Change-Id: I45b5736efa3d43584560ea93aeaefe3ea4fd4e80
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
index 046f2f7..626340f 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
@@ -29,7 +29,6 @@
import com.google.gerrit.server.InternalUser;
import com.google.gwtorm.server.OrmException;
import java.io.IOException;
-import java.sql.Timestamp;
import java.util.Date;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
@@ -49,7 +48,6 @@
protected final Account.Id realAccountId;
protected final PersonIdent authorIdent;
protected final Date when;
- private final long readOnlySkewMs;
@Nullable private final ChangeNotes notes;
private final Change change;
@@ -75,7 +73,6 @@
this.realAccountId = realAccountId != null ? realAccountId : accountId;
this.authorIdent = ident(noteUtil, serverIdent, user, when);
this.when = when;
- this.readOnlySkewMs = NoteDbChangeState.getReadOnlySkew(cfg);
}
protected AbstractChangeUpdate(
@@ -99,7 +96,6 @@
this.realAccountId = realAccountId;
this.authorIdent = authorIdent;
this.when = when;
- this.readOnlySkewMs = NoteDbChangeState.getReadOnlySkew(cfg);
}
private static void checkUserType(CurrentUser user) {
@@ -211,7 +207,6 @@
}
checkArgument(rw.getObjectReader().getCreatedFromInserter() == ins);
- checkNotReadOnly();
logger.atFinest().log(
"%s for change %s of project %s in %s (NoteDb)",
@@ -244,18 +239,6 @@
return result;
}
- protected void checkNotReadOnly() throws OrmException {
- ChangeNotes notes = getNotes();
- if (notes == null) {
- // Can only happen during ChangeRebuilder, which will never include a read-only lease.
- return;
- }
- Timestamp until = notes.getReadOnlyUntil();
- if (until != null && NoteDbChangeState.timeForReadOnlyCheck(readOnlySkewMs).before(until)) {
- throw new OrmException("change " + notes.getChangeId() + " is read-only until " + until);
- }
- }
-
/**
* Create a commit containing the contents of this update.
*
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
index 8a0cabe..628dfd2 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
@@ -39,7 +39,6 @@
public static final FooterKey FOOTER_PATCH_SET_DESCRIPTION =
new FooterKey("Patch-set-description");
public static final FooterKey FOOTER_PRIVATE = new FooterKey("Private");
- public static final FooterKey FOOTER_READ_ONLY_UNTIL = new FooterKey("Read-only-until");
public static final FooterKey FOOTER_REAL_USER = new FooterKey("Real-user");
public static final FooterKey FOOTER_STATUS = new FooterKey("Status");
public static final FooterKey FOOTER_SUBJECT = new FooterKey("Subject");
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index acb6572..2717488 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -60,7 +60,6 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
-import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@@ -501,11 +500,6 @@
getPatchSets().get(psId), () -> String.format("missing current patch set %s", psId.get()));
}
- @VisibleForTesting
- public Timestamp getReadOnlyUntil() {
- return state.readOnlyUntil();
- }
-
@Override
protected void onLoad(LoadHandle handle) throws NoSuchChangeException, IOException {
ObjectId rev = handle.id();
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
index add5803..58f1166 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
@@ -170,7 +170,6 @@
+ list(state.changeMessages(), changeMessage())
+ P
+ map(state.publishedComments().asMap(), comment())
- + T // readOnlyUntil
+ 1 // isPrivate
+ 1 // workInProgress
+ 1; // reviewStarted
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index cbb7020..890febb 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -26,7 +26,6 @@
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET_DESCRIPTION;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PRIVATE;
-import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_READ_ONLY_UNTIL;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_REAL_USER;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_REVERT_OF;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_STATUS;
@@ -77,7 +76,6 @@
import java.io.IOException;
import java.nio.charset.Charset;
import java.sql.Timestamp;
-import java.text.ParseException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@@ -85,7 +83,6 @@
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
-import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
@@ -99,7 +96,6 @@
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.FooterKey;
-import org.eclipse.jgit.util.GitDateParser;
import org.eclipse.jgit.util.RawParseUtils;
class ChangeNotesParser {
@@ -163,7 +159,6 @@
private String submissionId;
private String tag;
private RevisionNoteMap<ChangeRevisionNote> revisionNoteMap;
- private Timestamp readOnlyUntil;
private Boolean isPrivate;
private Boolean workInProgress;
private Boolean previousWorkInProgressFooter;
@@ -266,7 +261,6 @@
submitRecords,
buildAllMessages(),
comments,
- readOnlyUntil,
firstNonNull(isPrivate, false),
firstNonNull(workInProgress, false),
firstNonNull(hasReviewStarted, true),
@@ -401,10 +395,6 @@
// behavior.
}
- if (readOnlyUntil == null) {
- parseReadOnlyUntil(commit);
- }
-
if (isPrivate == null) {
parseIsPrivate(commit);
}
@@ -933,20 +923,6 @@
}
}
- private void parseReadOnlyUntil(ChangeNotesCommit commit) throws ConfigInvalidException {
- String raw = parseOneFooter(commit, FOOTER_READ_ONLY_UNTIL);
- if (raw == null) {
- return;
- }
- try {
- readOnlyUntil = new Timestamp(GitDateParser.parse(raw, null, Locale.US).getTime());
- } catch (ParseException e) {
- ConfigInvalidException cie = invalidFooter(FOOTER_READ_ONLY_UNTIL, raw);
- cie.initCause(e);
- throw cie;
- }
- }
-
private void parseIsPrivate(ChangeNotesCommit commit) throws ConfigInvalidException {
String raw = parseOneFooter(commit, FOOTER_PRIVATE);
if (raw == null) {
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index 7ce7e66..52cd99f 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -120,7 +120,6 @@
List<SubmitRecord> submitRecords,
List<ChangeMessage> changeMessages,
ListMultimap<RevId, Comment> publishedComments,
- @Nullable Timestamp readOnlyUntil,
boolean isPrivate,
boolean workInProgress,
boolean reviewStarted,
@@ -168,7 +167,6 @@
.submitRecords(submitRecords)
.changeMessages(changeMessages)
.publishedComments(publishedComments)
- .readOnlyUntil(readOnlyUntil)
.build();
}
@@ -304,9 +302,6 @@
abstract ImmutableListMultimap<RevId, Comment> publishedComments();
- @Nullable
- abstract Timestamp readOnlyUntil();
-
Change newChange(Project.NameKey project) {
ChangeColumns c = requireNonNull(columns(), "columns are required");
Change change =
@@ -430,8 +425,6 @@
abstract Builder publishedComments(ListMultimap<RevId, Comment> publishedComments);
- abstract Builder readOnlyUntil(@Nullable Timestamp readOnlyUntil);
-
abstract ChangeNotesState build();
}
@@ -494,10 +487,6 @@
.forEach(m -> b.addChangeMessage(toByteString(m, ChangeMessageProtoConverter.INSTANCE)));
object.publishedComments().values().forEach(c -> b.addPublishedComment(GSON.toJson(c)));
- if (object.readOnlyUntil() != null) {
- b.setReadOnlyUntil(object.readOnlyUntil().getTime()).setHasReadOnlyUntil(true);
- }
-
return Protos.toByteArray(b.build());
}
@@ -630,9 +619,6 @@
.stream()
.map(r -> GSON.fromJson(r, Comment.class))
.collect(toImmutableListMultimap(c -> new RevId(c.revId), c -> c)));
- if (proto.getHasReadOnlyUntil()) {
- b.readOnlyUntil(new Timestamp(proto.getReadOnlyUntil()));
- }
return b.build();
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 837e093..3528f23 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -29,7 +29,6 @@
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET_DESCRIPTION;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PRIVATE;
-import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_READ_ONLY_UNTIL;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_REAL_USER;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_REVERT_OF;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_STATUS;
@@ -71,7 +70,6 @@
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import java.io.IOException;
-import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.Date;
@@ -152,7 +150,6 @@
private boolean isAllowWriteToNewtRef;
private String psDescription;
private boolean currentPatchSet;
- private Timestamp readOnlyUntil;
private Boolean isPrivate;
private Boolean workInProgress;
private Integer revertOf;
@@ -730,10 +727,6 @@
addIdent(msg, realAccountId).append('\n');
}
- if (readOnlyUntil != null) {
- addFooter(msg, FOOTER_READ_ONLY_UNTIL, NoteDbUtil.formatTime(serverIdent, readOnlyUntil));
- }
-
if (isPrivate != null) {
addFooter(msg, FOOTER_PRIVATE, isPrivate);
}
@@ -793,7 +786,6 @@
&& tag == null
&& psDescription == null
&& !currentPatchSet
- && readOnlyUntil == null
&& isPrivate == null
&& workInProgress == null
&& revertOf == null;
@@ -832,10 +824,6 @@
this.workInProgress = workInProgress;
}
- void setReadOnlyUntil(Timestamp readOnlyUntil) {
- this.readOnlyUntil = readOnlyUntil;
- }
-
private static StringBuilder addFooter(StringBuilder sb, FooterKey footer) {
return sb.append(footer.getName()).append(": ");
}
@@ -857,13 +845,4 @@
sb.append('>');
return sb;
}
-
- @Override
- protected void checkNotReadOnly() throws OrmException {
- // Allow setting Read-only-until to 0 to release an existing lease.
- if (readOnlyUntil != null && readOnlyUntil.getTime() == 0) {
- return;
- }
- super.checkNotReadOnly();
- }
}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
index 0ea1bea..2931b17 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
@@ -704,19 +704,6 @@
}
@Test
- public void serializeReadOnlyUntil() throws Exception {
- assertRoundTrip(
- newBuilder().readOnlyUntil(new Timestamp(1212L)).build(),
- ChangeNotesStateProto.newBuilder()
- .setMetaId(SHA_BYTES)
- .setChangeId(ID.get())
- .setColumns(colsProto)
- .setReadOnlyUntil(1212L)
- .setHasReadOnlyUntil(true)
- .build());
- }
-
- @Test
public void changeNotesStateMethods() throws Exception {
assertThatSerializedClass(ChangeNotesState.class)
.hasAutoValueMethods(
@@ -746,7 +733,6 @@
.put(
"publishedComments",
new TypeLiteral<ImmutableListMultimap<RevId, Comment>>() {}.getType())
- .put("readOnlyUntil", Timestamp.class)
.build());
}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 033fb12..21443dc 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -15,7 +15,6 @@
package com.google.gerrit.server.notedb;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assert_;
import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
import static com.google.gerrit.reviewdb.client.RefNames.refsDraftComments;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
@@ -54,14 +53,12 @@
import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.testing.TestChanges;
-import com.google.gerrit.testing.TestTimeUtil;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.sql.Timestamp;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
-import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
@@ -2760,76 +2757,6 @@
}
@Test
- public void readOnlyUntilExpires() throws Exception {
- Change c = newChange();
- ChangeUpdate update = newUpdate(c, changeOwner);
- Timestamp until = new Timestamp(TimeUtil.nowMs() + 10000);
- update.setReadOnlyUntil(until);
- update.commit();
-
- update = newUpdate(c, changeOwner);
- update.setTopic("failing-topic");
- try {
- update.commit();
- assert_().fail("expected OrmException");
- } catch (OrmException e) {
- assertThat(e.getMessage()).contains("read-only until");
- }
-
- ChangeNotes notes = newNotes(c);
- assertThat(notes.getChange().getTopic()).isNotEqualTo("failing-topic");
- assertThat(notes.getReadOnlyUntil()).isEqualTo(until);
-
- TestTimeUtil.incrementClock(30, TimeUnit.SECONDS);
- update = newUpdate(c, changeOwner);
- update.setTopic("succeeding-topic");
- update.commit();
-
- // Write succeeded; lease still exists, even though it's expired.
- notes = newNotes(c);
- assertThat(notes.getChange().getTopic()).isEqualTo("succeeding-topic");
- assertThat(notes.getReadOnlyUntil()).isEqualTo(until);
-
- // New lease takes precedence.
- update = newUpdate(c, changeOwner);
- until = new Timestamp(TimeUtil.nowMs() + 10000);
- update.setReadOnlyUntil(until);
- update.commit();
- assertThat(newNotes(c).getReadOnlyUntil()).isEqualTo(until);
- }
-
- @Test
- public void readOnlyUntilCleared() throws Exception {
- Change c = newChange();
- ChangeUpdate update = newUpdate(c, changeOwner);
- Timestamp until = new Timestamp(TimeUtil.nowMs() + TimeUnit.DAYS.toMillis(30));
- update.setReadOnlyUntil(until);
- update.commit();
-
- update = newUpdate(c, changeOwner);
- update.setTopic("failing-topic");
- try {
- update.commit();
- assert_().fail("expected OrmException");
- } catch (OrmException e) {
- assertThat(e.getMessage()).contains("read-only until");
- }
-
- // Sentinel timestamp of 0 can be written to clear lease.
- update = newUpdate(c, changeOwner);
- update.setReadOnlyUntil(new Timestamp(0));
- update.commit();
-
- update = newUpdate(c, changeOwner);
- update.setTopic("succeeding-topic");
- update.commit();
-
- ChangeNotes notes = newNotes(c);
- assertThat(notes.getChange().getTopic()).isEqualTo("succeeding-topic");
- assertThat(notes.getReadOnlyUntil()).isEqualTo(new Timestamp(0));
- }
-
- @Test
public void privateDefault() throws Exception {
Change c = newChange();
ChangeNotes notes = newNotes(c);
diff --git a/proto/cache.proto b/proto/cache.proto
index c2ac0d9..52f5e1c 100644
--- a/proto/cache.proto
+++ b/proto/cache.proto
@@ -181,8 +181,8 @@
// JSON produced from com.google.gerrit.reviewdb.client.Comment.
repeated string published_comment = 16;
- int64 read_only_until = 17;
- bool has_read_only_until = 18;
+ reserved 17; // read_only_until
+ reserved 18; // has_read_only_until
}
