From 1c200b552ba77915ca48b0e8aa3b3364111de186 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Tue, 10 Nov 2015 15:20:16 -0800 Subject: [PATCH] ConsistencyChecker: Update notedb when change status is fixed Also fix the CheckIT#fixReturnsUpdatedValue() test to set the wrong change status in notedb so that the test can attempt to fix it. Add a new fixStatus(...) method to ChangeUpdate, since setStatus(...) doesn't allow to set MERGED as status and we don't want to remove this restriction from this method. Change-Id: Iaf403ae2fa3026afbfd40bba23cbbcdfbc7de6c2 Signed-off-by: Edwin Kempin --- .../gerrit/acceptance/api/change/CheckIT.java | 19 +++++++++++++++++++ .../server/change/ConsistencyChecker.java | 18 ++++++++++++++++-- .../gerrit/server/notedb/ChangeUpdate.java | 4 ++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/CheckIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/CheckIT.java index 07c5c033d0..37a5071f8e 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/CheckIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/CheckIT.java @@ -25,6 +25,10 @@ import com.google.gerrit.extensions.client.ChangeStatus; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ProblemInfo; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.notedb.ChangeUpdate; +import com.google.gerrit.server.project.ChangeControl; +import com.google.inject.Inject; import org.junit.Test; @@ -33,6 +37,15 @@ import java.util.List; @NoHttpd public class CheckIT extends AbstractDaemonTest { + @Inject + private ChangeControl.GenericFactory changeControlFactory; + + @Inject + private IdentifiedUser.GenericFactory userFactory; + + @Inject + private ChangeUpdate.Factory changeUpdateFactory; + // Most types of tests belong in ConsistencyCheckerTest; these mostly just // test paths outside of ConsistencyChecker, like API wiring. @Test @@ -66,6 +79,12 @@ public class CheckIT extends AbstractDaemonTest { Change c = getChange(r); c.setStatus(Change.Status.NEW); db.changes().update(Collections.singleton(c)); + ChangeUpdate changeUpdate = + changeUpdateFactory.create( + changeControlFactory.controlFor( + c, userFactory.create(admin.id))); + changeUpdate.setStatus(Change.Status.NEW); + changeUpdate.commit(); indexer.index(db, c); ChangeInfo info = gApi.changes() diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java index 92b1164c80..349117fbc2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java @@ -46,8 +46,11 @@ import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.UpdateException; import com.google.gerrit.server.git.validators.CommitValidators; import com.google.gerrit.server.index.ChangeIndexer; +import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; +import com.google.gerrit.server.project.ChangeControl; +import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.RefControl; @@ -116,6 +119,8 @@ public class ConsistencyChecker { private final PatchSetInserter.Factory patchSetInserterFactory; private final BatchUpdate.Factory updateFactory; private final ChangeIndexer indexer; + private final ChangeControl.GenericFactory changeControlFactory; + private final ChangeUpdate.Factory changeUpdateFactory; private FixInput fix; private Change change; @@ -138,7 +143,9 @@ public class ConsistencyChecker { PatchSetInfoFactory patchSetInfoFactory, PatchSetInserter.Factory patchSetInserterFactory, BatchUpdate.Factory updateFactory, - ChangeIndexer indexer) { + ChangeIndexer indexer, + ChangeControl.GenericFactory changeControlFactory, + ChangeUpdate.Factory changeUpdateFactory) { this.db = db; this.repoManager = repoManager; this.user = user; @@ -148,6 +155,8 @@ public class ConsistencyChecker { this.patchSetInserterFactory = patchSetInserterFactory; this.updateFactory = updateFactory; this.indexer = indexer; + this.changeControlFactory = changeControlFactory; + this.changeUpdateFactory = changeUpdateFactory; reset(); } @@ -511,10 +520,15 @@ public class ConsistencyChecker { return c; } }); + ChangeUpdate changeUpdate = + changeUpdateFactory.create( + changeControlFactory.controlFor(change, user.get())); + changeUpdate.fixStatus(Change.Status.MERGED); + changeUpdate.commit(); indexer.index(db.get(), change); p.status = Status.FIXED; p.outcome = "Marked change as merged"; - } catch (OrmException | IOException e) { + } catch (OrmException | IOException | NoSuchChangeException e) { log.warn("Error marking " + change.getId() + "as merged", e); p.status = Status.FIX_FAILED; p.outcome = "Error updating status to merged"; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java index cb35619591..065edcd141 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java @@ -169,6 +169,10 @@ public class ChangeUpdate extends AbstractChangeUpdate { this.status = status; } + public void fixStatus(Change.Status status) { + this.status = status; + } + public void putApproval(String label, short value) { approvals.put(label, Optional.of(value)); }