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 <ekempin@google.com>
This commit is contained in:
		@@ -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()
 | 
			
		||||
 
 | 
			
		||||
@@ -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";
 | 
			
		||||
 
 | 
			
		||||
@@ -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));
 | 
			
		||||
  }
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user