From 6d82df9bbbfb6efb0d3705460c45dacf1a81ea14 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Thu, 13 Dec 2018 11:34:30 +0100 Subject: [PATCH] Remove all unneeded checks of primary storage of a change There is a remaining usage in ChangeUpdate which requires more work. It already has a TODO. Change-Id: I259f9740d1ca8c8af531b8ac61d392af98261971 Signed-off-by: Edwin Kempin --- .../google/gerrit/server/ApprovalCopier.java | 45 ---------------- .../server/change/PatchSetInserter.java | 17 ------ .../gerrit/server/change/RebaseChangeOp.java | 7 --- .../gerrit/server/git/receive/ReplaceOp.java | 21 +------- .../server/index/change/ChangeField.java | 33 +++++------- .../server/index/change/ChangeIndexer.java | 3 -- .../server/index/change/StalenessChecker.java | 54 ++----------------- .../server/notedb/NoteDbChangeState.java | 38 +------------ .../server/submit/RebaseSubmitStrategy.java | 1 - .../server/change/ConsistencyCheckerIT.java | 7 --- .../index/change/StalenessCheckerTest.java | 29 ---------- 11 files changed, 21 insertions(+), 234 deletions(-) diff --git a/java/com/google/gerrit/server/ApprovalCopier.java b/java/com/google/gerrit/server/ApprovalCopier.java index 708380442b..2a87d1f10d 100644 --- a/java/com/google/gerrit/server/ApprovalCopier.java +++ b/java/com/google/gerrit/server/ApprovalCopier.java @@ -30,7 +30,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.change.ChangeKindCache; import com.google.gerrit.server.change.LabelNormalizer; import com.google.gerrit.server.notedb.ChangeNotes; -import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.query.change.ChangeData; @@ -74,50 +73,6 @@ public class ApprovalCopier { this.psUtil = psUtil; } - /** - * Apply approval copy settings from prior PatchSets to a new PatchSet. - * - * @param db review database. - * @param notes change notes for user uploading PatchSet - * @param ps new PatchSet - * @param rw open walk that can read the patch set commit; null to open the repo on demand. - * @param repoConfig repo config used for change kind detection; null to read from repo on demand. - * @throws OrmException - */ - public void copyInReviewDb( - ReviewDb db, - ChangeNotes notes, - PatchSet ps, - @Nullable RevWalk rw, - @Nullable Config repoConfig) - throws OrmException { - copyInReviewDb(db, notes, ps, rw, repoConfig, Collections.emptyList()); - } - - /** - * Apply approval copy settings from prior PatchSets to a new PatchSet. - * - * @param db review database. - * @param notes change notes for user uploading PatchSet - * @param ps new PatchSet - * @param rw open walk that can read the patch set commit; null to open the repo on demand. - * @param repoConfig repo config used for change kind detection; null to read from repo on demand. - * @param dontCopy PatchSetApprovals indicating which (account, label) pairs should not be copied - * @throws OrmException - */ - public void copyInReviewDb( - ReviewDb db, - ChangeNotes notes, - PatchSet ps, - @Nullable RevWalk rw, - @Nullable Config repoConfig, - Iterable dontCopy) - throws OrmException { - if (PrimaryStorage.of(notes.getChange()) == PrimaryStorage.REVIEW_DB) { - db.patchSetApprovals().insert(getForPatchSet(db, notes, ps, rw, repoConfig, dontCopy)); - } - } - Iterable getForPatchSet( ReviewDb db, ChangeNotes notes, diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java index ca2f6a23b3..fb32c50aba 100644 --- a/java/com/google/gerrit/server/change/PatchSetInserter.java +++ b/java/com/google/gerrit/server/change/PatchSetInserter.java @@ -31,8 +31,6 @@ 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.reviewdb.client.PatchSetInfo; -import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.ApprovalCopier; import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.ChangeUtil; @@ -79,7 +77,6 @@ public class PatchSetInserter implements BatchUpdateOp { private final ProjectCache projectCache; private final RevisionCreated revisionCreated; private final ApprovalsUtil approvalsUtil; - private final ApprovalCopier approvalCopier; private final ChangeMessagesUtil cmUtil; private final PatchSetUtil psUtil; @@ -101,7 +98,6 @@ public class PatchSetInserter implements BatchUpdateOp { private NotifyHandling notify = NotifyHandling.ALL; private ListMultimap accountsToNotify = ImmutableListMultimap.of(); private boolean allowClosed; - private boolean copyApprovals = true; // Fields set during some phase of BatchUpdate.Op. private Change change; @@ -114,7 +110,6 @@ public class PatchSetInserter implements BatchUpdateOp { public PatchSetInserter( PermissionBackend permissionBackend, ApprovalsUtil approvalsUtil, - ApprovalCopier approvalCopier, ChangeMessagesUtil cmUtil, PatchSetInfoFactory patchSetInfoFactory, CommitValidators.Factory commitValidatorsFactory, @@ -127,7 +122,6 @@ public class PatchSetInserter implements BatchUpdateOp { @Assisted ObjectId commitId) { this.permissionBackend = permissionBackend; this.approvalsUtil = approvalsUtil; - this.approvalCopier = approvalCopier; this.cmUtil = cmUtil; this.patchSetInfoFactory = patchSetInfoFactory; this.commitValidatorsFactory = commitValidatorsFactory; @@ -192,11 +186,6 @@ public class PatchSetInserter implements BatchUpdateOp { return this; } - public PatchSetInserter setCopyApprovals(boolean copyApprovals) { - this.copyApprovals = copyApprovals; - return this; - } - public Change getChange() { checkState(change != null, "getChange() only valid after executing update"); return change; @@ -218,8 +207,6 @@ public class PatchSetInserter implements BatchUpdateOp { @Override public boolean updateChange(ChangeContext ctx) throws ResourceConflictException, OrmException, IOException { - ReviewDb db = ctx.getDb(); - change = ctx.getChange(); ChangeUpdate update = ctx.getUpdate(psId); update.setSubjectForCommit("Create patch set " + psId.get()); @@ -263,10 +250,6 @@ public class PatchSetInserter implements BatchUpdateOp { change.setStatus(Change.Status.NEW); } change.setCurrentPatchSet(patchSetInfo); - if (copyApprovals) { - approvalCopier.copyInReviewDb( - db, ctx.getNotes(), patchSet, ctx.getRevWalk(), ctx.getRepoView().getConfig()); - } if (changeMessage != null) { cmUtil.addChangeMessage(update, changeMessage); } diff --git a/java/com/google/gerrit/server/change/RebaseChangeOp.java b/java/com/google/gerrit/server/change/RebaseChangeOp.java index dac8fec488..ea266a08a5 100644 --- a/java/com/google/gerrit/server/change/RebaseChangeOp.java +++ b/java/com/google/gerrit/server/change/RebaseChangeOp.java @@ -71,7 +71,6 @@ public class RebaseChangeOp implements BatchUpdateOp { private boolean validate = true; private boolean checkAddPatchSetPermission = true; private boolean forceContentMerge; - private boolean copyApprovals = true; private boolean detailedCommitMessage; private boolean postMessage = true; private boolean matchAuthorToCommitterDate = false; @@ -128,11 +127,6 @@ public class RebaseChangeOp implements BatchUpdateOp { return this; } - public RebaseChangeOp setCopyApprovals(boolean copyApprovals) { - this.copyApprovals = copyApprovals; - return this; - } - public RebaseChangeOp setDetailedCommitMessage(boolean detailedCommitMessage) { this.detailedCommitMessage = detailedCommitMessage; return this; @@ -189,7 +183,6 @@ public class RebaseChangeOp implements BatchUpdateOp { .setDescription("Rebase") .setNotify(NotifyHandling.NONE) .setFireRevisionCreated(fireRevisionCreated) - .setCopyApprovals(copyApprovals) .setCheckAddPatchSetPermission(checkAddPatchSetPermission) .setValidate(validate); if (postMessage) { diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java index d2ef12c4b9..8c03fdb252 100644 --- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java +++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java @@ -43,7 +43,6 @@ import com.google.gerrit.reviewdb.client.Comment; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetInfo; -import com.google.gerrit.server.ApprovalCopier; import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.CommentsUtil; @@ -116,7 +115,6 @@ public class ReplaceOp implements BatchUpdateOp { private static final String CHANGE_IS_CLOSED = "change is closed"; private final AccountResolver accountResolver; - private final ApprovalCopier approvalCopier; private final ApprovalsUtil approvalsUtil; private final ChangeData.Factory changeDataFactory; private final ChangeKindCache changeKindCache; @@ -162,7 +160,6 @@ public class ReplaceOp implements BatchUpdateOp { @Inject ReplaceOp( AccountResolver accountResolver, - ApprovalCopier approvalCopier, ApprovalsUtil approvalsUtil, ChangeData.Factory changeDataFactory, ChangeKindCache changeKindCache, @@ -190,7 +187,6 @@ public class ReplaceOp implements BatchUpdateOp { @Assisted @Nullable MagicBranchInput magicBranch, @Assisted @Nullable PushCertificate pushCertificate) { this.accountResolver = accountResolver; - this.approvalCopier = approvalCopier; this.approvalsUtil = approvalsUtil; this.changeDataFactory = changeDataFactory; this.changeKindCache = changeKindCache; @@ -315,21 +311,8 @@ public class ReplaceOp implements BatchUpdateOp { update.setPsDescription(psDescription); MailRecipients fromFooters = getRecipientsFromFooters(accountResolver, commit.getFooterLines()); - Iterable newApprovals = - approvalsUtil.addApprovalsForNewPatchSet( - ctx.getDb(), - update, - projectState.getLabelTypes(), - newPatchSet, - ctx.getUser(), - approvals); - approvalCopier.copyInReviewDb( - ctx.getDb(), - ctx.getNotes(), - newPatchSet, - ctx.getRevWalk(), - ctx.getRepoView().getConfig(), - newApprovals); + approvalsUtil.addApprovalsForNewPatchSet( + ctx.getDb(), update, projectState.getLabelTypes(), newPatchSet, ctx.getUser(), approvals); reviewerAdditions = reviewerAdder.prepare( diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java index bae33778a5..af44ea7a97 100644 --- a/java/com/google/gerrit/server/index/change/ChangeField.java +++ b/java/com/google/gerrit/server/index/change/ChangeField.java @@ -63,7 +63,6 @@ import com.google.gerrit.server.StarredChangesUtil; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.index.change.StalenessChecker.RefStatePattern; import com.google.gerrit.server.notedb.ChangeNotes; -import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage; import com.google.gerrit.server.notedb.ReviewerStateInternal; import com.google.gerrit.server.notedb.RobotCommentNotes; import com.google.gerrit.server.project.SubmitRuleOptions; @@ -805,19 +804,17 @@ public class ChangeField { .values() .forEach(r -> result.add(RefState.of(r.ref()).toByteArray(allUsers(cd)))); - if (PrimaryStorage.of(cd.change()) == PrimaryStorage.NOTE_DB) { - ChangeNotes notes = cd.notes(); - result.add( - RefState.create(notes.getRefName(), notes.getMetaId()).toByteArray(project)); - notes.getRobotComments(); // Force loading robot comments. - RobotCommentNotes robotNotes = notes.getRobotCommentNotes(); - result.add( - RefState.create(robotNotes.getRefName(), robotNotes.getMetaId()) - .toByteArray(project)); - cd.draftRefs() - .values() - .forEach(r -> result.add(RefState.of(r).toByteArray(allUsers(cd)))); - } + ChangeNotes notes = cd.notes(); + result.add( + RefState.create(notes.getRefName(), notes.getMetaId()).toByteArray(project)); + notes.getRobotComments(); // Force loading robot comments. + RobotCommentNotes robotNotes = notes.getRobotCommentNotes(); + result.add( + RefState.create(robotNotes.getRefName(), robotNotes.getMetaId()) + .toByteArray(project)); + cd.draftRefs() + .values() + .forEach(r -> result.add(RefState.of(r).toByteArray(allUsers(cd)))); return result; }); @@ -842,11 +839,9 @@ public class ChangeField { result.add( RefStatePattern.create(RefNames.refsStarredChangesPrefix(id) + "*") .toByteArray(allUsers(cd))); - if (PrimaryStorage.of(cd.change()) == PrimaryStorage.NOTE_DB) { - result.add( - RefStatePattern.create(RefNames.refsDraftCommentsPrefix(id) + "*") - .toByteArray(allUsers(cd))); - } + result.add( + RefStatePattern.create(RefNames.refsDraftCommentsPrefix(id) + "*") + .toByteArray(allUsers(cd))); return result; }); diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexer.java b/java/com/google/gerrit/server/index/change/ChangeIndexer.java index af13cace3f..0c81bfa048 100644 --- a/java/com/google/gerrit/server/index/change/ChangeIndexer.java +++ b/java/com/google/gerrit/server/index/change/ChangeIndexer.java @@ -34,7 +34,6 @@ import com.google.gerrit.server.index.IndexUtils; import com.google.gerrit.server.logging.TraceContext; import com.google.gerrit.server.logging.TraceContext.TraceTimer; import com.google.gerrit.server.plugincontext.PluginSetContext; -import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.util.RequestContext; import com.google.gerrit.server.util.ThreadLocalRequestContext; @@ -419,8 +418,6 @@ public class ChangeIndexer { indexImpl(changeDataFactory.create(db.get(), project, id)); return true; } - } catch (NoSuchChangeException nsce) { - logger.atFine().log("Change %s was deleted, aborting reindexing the change.", id.get()); } catch (Exception e) { if (!isCausedByRepositoryNotFoundException(e)) { throw e; diff --git a/java/com/google/gerrit/server/index/change/StalenessChecker.java b/java/com/google/gerrit/server/index/change/StalenessChecker.java index d5d6b05302..f573a847b1 100644 --- a/java/com/google/gerrit/server/index/change/StalenessChecker.java +++ b/java/com/google/gerrit/server/index/change/StalenessChecker.java @@ -16,7 +16,6 @@ package com.google.gerrit.server.index.change; import static com.google.common.base.Preconditions.checkArgument; import static java.nio.charset.StandardCharsets.UTF_8; -import static java.util.Objects.requireNonNull; import static java.util.stream.Collectors.joining; import com.google.auto.value.AutoValue; @@ -29,21 +28,15 @@ import com.google.common.collect.SetMultimap; import com.google.common.collect.Sets; import com.google.common.collect.Streams; import com.google.common.flogger.FluentLogger; -import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.index.IndexConfig; import com.google.gerrit.index.RefState; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; -import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.reviewdb.server.ReviewDbUtil; import com.google.gerrit.server.UsedAt; import com.google.gerrit.server.git.GitRepositoryManager; -import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage; import com.google.gerrit.server.query.change.ChangeData; -import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.Singleton; import java.io.IOException; import java.util.List; @@ -66,21 +59,16 @@ public class StalenessChecker { private final ChangeIndexCollection indexes; private final GitRepositoryManager repoManager; private final IndexConfig indexConfig; - private final Provider db; @Inject StalenessChecker( - ChangeIndexCollection indexes, - GitRepositoryManager repoManager, - IndexConfig indexConfig, - Provider db) { + ChangeIndexCollection indexes, GitRepositoryManager repoManager, IndexConfig indexConfig) { this.indexes = indexes; this.repoManager = repoManager; this.indexConfig = indexConfig; - this.db = db; } - public boolean isStale(Change.Id id) throws IOException, OrmException { + public boolean isStale(Change.Id id) throws IOException { ChangeIndex i = indexes.getSearchIndex(); if (i == null) { return false; // No index; caller couldn't do anything if it is stale. @@ -96,25 +84,16 @@ public class StalenessChecker { return true; // Not in index, but caller wants it to be. } ChangeData cd = result.get(); - return isStale( - repoManager, - id, - cd.change(), - ReviewDbUtil.unwrapDb(db.get()).changes().get(id), - parseStates(cd), - parsePatterns(cd)); + return isStale(repoManager, id, parseStates(cd), parsePatterns(cd)); } @UsedAt(UsedAt.Project.GOOGLE) public static boolean isStale( GitRepositoryManager repoManager, Change.Id id, - Change indexChange, - @Nullable Change reviewDbChange, SetMultimap states, ListMultimap patterns) { - return reviewDbChangeIsStale(indexChange, reviewDbChange) - || refsAreStale(repoManager, id, states, patterns); + return refsAreStale(repoManager, id, states, patterns); } @VisibleForTesting @@ -134,31 +113,6 @@ public class StalenessChecker { return false; } - @VisibleForTesting - static boolean reviewDbChangeIsStale(Change indexChange, @Nullable Change reviewDbChange) { - requireNonNull(indexChange); - PrimaryStorage storageFromIndex = PrimaryStorage.of(indexChange); - PrimaryStorage storageFromReviewDb = PrimaryStorage.of(reviewDbChange); - if (reviewDbChange == null) { - if (storageFromIndex == PrimaryStorage.REVIEW_DB) { - return true; // Index says it should have been in ReviewDb, but it wasn't. - } - return false; // Not in ReviewDb, but that's ok. - } - checkArgument( - indexChange.getId().equals(reviewDbChange.getId()), - "mismatched change ID: %s != %s", - indexChange.getId(), - reviewDbChange.getId()); - if (storageFromIndex != storageFromReviewDb) { - return true; // Primary storage differs, definitely stale. - } - if (storageFromReviewDb != PrimaryStorage.REVIEW_DB) { - return false; // Not a ReviewDb change, don't check rowVersion. - } - return reviewDbChange.getRowVersion() != indexChange.getRowVersion(); - } - private SetMultimap parseStates(ChangeData cd) { return RefState.parseStates(cd.getRefStates()); } diff --git a/java/com/google/gerrit/server/notedb/NoteDbChangeState.java b/java/com/google/gerrit/server/notedb/NoteDbChangeState.java index e6b82e6772..d63994eb0b 100644 --- a/java/com/google/gerrit/server/notedb/NoteDbChangeState.java +++ b/java/com/google/gerrit/server/notedb/NoteDbChangeState.java @@ -79,10 +79,7 @@ public class NoteDbChangeState { } public static PrimaryStorage of(@Nullable Change c) { - return of(NoteDbChangeState.parse(c)); - } - - public static PrimaryStorage of(@Nullable NoteDbChangeState s) { + NoteDbChangeState s = NoteDbChangeState.parse(c); return s != null ? s.getPrimaryStorage() : REVIEW_DB; } } @@ -270,39 +267,6 @@ public class NoteDbChangeState { return state; } - // TODO(dborowitz): Ugly. Refactor these static methods into a Checker class - // or something. They do not belong in NoteDbChangeState itself because: - // - need to inject Config but don't want a whole Factory - // - can't be methods on NoteDbChangeState because state is nullable (though - // we could also solve this by inventing an empty-but-non-null state) - // Also we should clean up duplicated code between static/non-static methods. - public static boolean isChangeUpToDate( - @Nullable NoteDbChangeState state, RefCache changeRepoRefs, Change.Id changeId) - throws IOException { - if (PrimaryStorage.of(state) == NOTE_DB) { - return true; // Primary storage is NoteDb, up to date by definition. - } - if (state == null) { - return !changeRepoRefs.get(changeMetaRef(changeId)).isPresent(); - } - return state.isChangeUpToDate(changeRepoRefs); - } - - public static boolean areDraftsUpToDate( - @Nullable NoteDbChangeState state, - RefCache draftsRepoRefs, - Change.Id changeId, - Account.Id accountId) - throws IOException { - if (PrimaryStorage.of(state) == NOTE_DB) { - return true; // Primary storage is NoteDb, up to date by definition. - } - if (state == null) { - return !draftsRepoRefs.get(refsDraftComments(changeId, accountId)).isPresent(); - } - return state.areDraftsUpToDate(draftsRepoRefs, accountId); - } - public static long getReadOnlySkew(Config cfg) { return cfg.getTimeUnit("notedb", null, "maxTimestampSkew", 1000, TimeUnit.MILLISECONDS); } diff --git a/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java b/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java index 144b5b1292..026a717478 100644 --- a/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java +++ b/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java @@ -177,7 +177,6 @@ public class RebaseSubmitStrategy extends SubmitStrategy { .setFireRevisionCreated(false) // Bypass approval copier since SubmitStrategyOp copy all approvals // later anyway. - .setCopyApprovals(false) .setValidate(false) .setCheckAddPatchSetPermission(false) // RebaseAlways should set always modify commit message like diff --git a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java index 3beb2b4b29..a9ac3c187b 100644 --- a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java +++ b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java @@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.extensions.common.ProblemInfo.Status.FIXED; import static com.google.gerrit.extensions.common.ProblemInfo.Status.FIX_FAILED; import static com.google.gerrit.testing.TestChanges.newPatchSet; -import static java.util.Collections.singleton; import static java.util.Objects.requireNonNull; import com.google.gerrit.acceptance.AbstractDaemonTest; @@ -43,7 +42,6 @@ import com.google.gerrit.server.change.ConsistencyChecker; import com.google.gerrit.server.change.PatchSetInserter; import com.google.gerrit.server.notedb.ChangeNoteUtil; import com.google.gerrit.server.notedb.ChangeNotes; -import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage; import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.BatchUpdateOp; import com.google.gerrit.server.update.ChangeContext; @@ -804,11 +802,6 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { c.setCurrentPatchSet(psId, subject, c.getOriginalSubject()); PatchSet ps = newPatchSet(psId, rev, adminId); - if (PrimaryStorage.of(c) == PrimaryStorage.REVIEW_DB) { - db.patchSets().insert(singleton(ps)); - db.changes().update(singleton(c)); - } - addNoteDbCommit( c.getId(), "Update patch set " diff --git a/javatests/com/google/gerrit/server/index/change/StalenessCheckerTest.java b/javatests/com/google/gerrit/server/index/change/StalenessCheckerTest.java index 51bda661e5..a38eabe036 100644 --- a/javatests/com/google/gerrit/server/index/change/StalenessCheckerTest.java +++ b/javatests/com/google/gerrit/server/index/change/StalenessCheckerTest.java @@ -17,7 +17,6 @@ package com.google.gerrit.server.index.change; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assert_; import static com.google.gerrit.server.index.change.StalenessChecker.refsAreStale; -import static com.google.gerrit.testing.TestChanges.newChange; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.stream.Collectors.toList; @@ -25,16 +24,12 @@ import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.ListMultimap; import com.google.gerrit.index.RefState; -import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.index.change.StalenessChecker.RefStatePattern; -import com.google.gerrit.server.notedb.NoteDbChangeState; import com.google.gerrit.testing.GerritBaseTests; import com.google.gerrit.testing.InMemoryRepositoryManager; -import com.google.gwtorm.protobuf.CodecFactory; -import com.google.gwtorm.protobuf.ProtobufCodec; import java.util.stream.Stream; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.ObjectId; @@ -51,8 +46,6 @@ public class StalenessCheckerTest extends GerritBaseTests { private static final Change.Id C = new Change.Id(1234); - private static final ProtobufCodec CHANGE_CODEC = CodecFactory.encoder(Change.class); - private GitRepositoryManager repoManager; private Repository r1; private Repository r2; @@ -316,29 +309,7 @@ public class StalenessCheckerTest extends GerritBaseTests { .isFalse(); } - @Test - public void reviewDbChangeIsStale() throws Exception { - Change indexChange = newChange(P1, new Account.Id(1)); - indexChange.setNoteDbState(SHA1); - - // Change is missing from ReviewDb but present in index. - assertThat(StalenessChecker.reviewDbChangeIsStale(indexChange, null)).isTrue(); - - // Change differs only in primary storage. - Change noteDbPrimary = clone(indexChange); - noteDbPrimary.setNoteDbState(NoteDbChangeState.NOTE_DB_PRIMARY_STATE); - assertThat(StalenessChecker.reviewDbChangeIsStale(indexChange, noteDbPrimary)).isTrue(); - - assertThat(StalenessChecker.reviewDbChangeIsStale(indexChange, clone(indexChange))).isFalse(); - - // Can't easily change row version to check true case. - } - private static Iterable byteArrays(String... strs) { return Stream.of(strs).map(s -> s != null ? s.getBytes(UTF_8) : null).collect(toList()); } - - private static Change clone(Change change) { - return CHANGE_CODEC.decode(CHANGE_CODEC.encodeToByteArray(change)); - } }