Add a basic consistency checker for changes
In the past, Gerrit bugs, lack of transactions, and unreliable NoSQL
backends have at various times produced a bewildering variety of
corrupt states. Similarly, we are not immune from bugs being
introduced in the future.
Add a tool to detect and explain some of these possible states.
Change-Id: Ia91b35b140bf05254877f413003d12cf779b775c
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 584c661..bb9bb3a 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -1134,6 +1134,52 @@
HTTP/1.1 204 No Content
----
+[[check-change]]
+=== Check change
+--
+'GET /changes/link:#change-id[\{change-id\}]/check'
+--
+
+Performs consistency checks on the change, and returns a
+link:#check-result[CheckResult] entity.
+
+.Request
+----
+ GET /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/check HTTP/1.0
+----
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json;charset=UTF-8
+
+ )]}'
+ {
+ "change": {
+ "id": "myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940",
+ "project": "myProject",
+ "branch": "master",
+ "change_id": "I8473b95934b5732ac55d26311a706c9c2bde9940",
+ "subject": "Implementing Feature X",
+ "status": "NEW",
+ "created": "2013-02-01 09:59:32.126000000",
+ "updated": "2013-02-21 11:16:36.775000000",
+ "mergeable": true,
+ "insertions": 34,
+ "deletions": 101,
+ "_sortkey": "0023412400000f7d",
+ "_number": 3965,
+ "owner": {
+ "name": "John Doe"
+ }
+ },
+ "messages": [
+ "Current patch set 1 not found"
+ ]
+ }
+----
+
[[edit-endpoints]]
== Change Edit Endpoints
@@ -3861,6 +3907,24 @@
|`restore_path`|optional|Path to file to restore.
|===========================
+[[check-result]]
+=== CheckResult
+The `CheckResult` entity contains the results of a consistency check on
+a change.
+
+[options="header",cols="1,6"]
+|===========================
+|Field Name|Description
+|`change`|
+link:#change-info[ChangeInfo] entity containing information about the change,
+as in link:#get-change[Get Change] with no options. Some fields not marked
+optional may be missing if a consistency check failed, but at least
+`id`, `project`, `branch`, and `_number` will be present.
+|`messages`|
+List of messages describing potential problems with the change. May be
+empty if no problems were found.
+|===========================
+
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/gerrit-server/BUCK b/gerrit-server/BUCK
index 11f054c..b9ca3e3 100644
--- a/gerrit-server/BUCK
+++ b/gerrit-server/BUCK
@@ -204,6 +204,7 @@
'//lib:guava',
'//lib:gwtorm',
'//lib:junit',
+ '//lib:truth',
'//lib/guice:guice',
'//lib/guice:guice-assistedinject',
'//lib/jgit:jgit',
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Check.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Check.java
new file mode 100644
index 0000000..8d613fb
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Check.java
@@ -0,0 +1,76 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.change;
+
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.server.account.AccountInfo;
+import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Singleton
+public class Check implements RestReadView<ChangeResource> {
+ private static final Logger log = LoggerFactory.getLogger(Check.class);
+
+ private final Provider<ConsistencyChecker> checkerProvider;
+ private final ChangeJson json;
+
+ @Inject
+ Check(Provider<ConsistencyChecker> checkerProvider,
+ ChangeJson json) {
+ this.checkerProvider = checkerProvider;
+ this.json = json;
+ }
+
+ @Override
+ public CheckResult apply(ChangeResource rsrc) {
+ CheckResult result = new CheckResult();
+ result.messages = checkerProvider.get().check(rsrc.getChange());
+ try {
+ result.change = json.format(rsrc);
+ } catch (OrmException e) {
+ // Even with no options there are a surprising number of dependencies in
+ // ChangeJson. Fall back to a very basic implementation with no
+ // dependencies if this fails.
+ String msg = "Error rendering final ChangeInfo";
+ log.warn(msg, e);
+ result.messages.add(msg);
+ result.change = basicChangeInfo(rsrc.getChange());
+ }
+ return result;
+ }
+
+ private static ChangeInfo basicChangeInfo(Change c) {
+ ChangeInfo info = new ChangeInfo();
+ info.project = c.getProject().get();
+ info.branch = c.getDest().getShortName();
+ info.topic = c.getTopic();
+ info.changeId = c.getKey().get();
+ info.subject = c.getSubject();
+ info.status = c.getStatus();
+ info.owner = new AccountInfo(c.getOwner());
+ info.created = c.getCreatedOn();
+ info.updated = c.getLastUpdatedOn();
+ info._number = c.getId().get();
+ info.finish();
+ return info;
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CheckResult.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CheckResult.java
new file mode 100644
index 0000000..673d010
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CheckResult.java
@@ -0,0 +1,24 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.change;
+
+import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
+
+import java.util.List;
+
+public class CheckResult {
+ public ChangeInfo change;
+ public List<String> messages;
+}
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
new file mode 100644
index 0000000..e5c2ad8
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java
@@ -0,0 +1,260 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.change;
+
+import com.google.common.base.Function;
+import com.google.common.collect.Collections2;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.MultimapBuilder;
+import com.google.common.collect.Ordering;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+import org.eclipse.jgit.errors.IncorrectObjectTypeException;
+import org.eclipse.jgit.errors.MissingObjectException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Checks changes for various kinds of inconsistency and corruption.
+ * <p>
+ * A single instance may be reused for checking multiple changes, but not
+ * concurrently.
+ */
+public class ConsistencyChecker {
+ private static final Logger log =
+ LoggerFactory.getLogger(ConsistencyChecker.class);
+
+ private final Provider<ReviewDb> db;
+ private final GitRepositoryManager repoManager;
+
+ private Change change;
+ private Repository repo;
+ private RevWalk rw;
+
+ private PatchSet currPs;
+ private RevCommit currPsCommit;
+
+ private List<String> messages;
+
+ @Inject
+ ConsistencyChecker(Provider<ReviewDb> db,
+ GitRepositoryManager repoManager) {
+ this.db = db;
+ this.repoManager = repoManager;
+ reset();
+ }
+
+ private void reset() {
+ change = null;
+ repo = null;
+ rw = null;
+ messages = new ArrayList<>();
+ }
+
+ public List<String> check(Change c) {
+ reset();
+ change = c;
+ try {
+ checkImpl();
+ return messages;
+ } finally {
+ if (rw != null) {
+ rw.release();
+ }
+ if (repo != null) {
+ repo.close();
+ }
+ }
+ }
+
+ private void checkImpl() {
+ checkOwner();
+ checkCurrentPatchSetEntity();
+
+ // All checks that require the repo.
+ if (!openRepo()) {
+ return;
+ }
+ if (!checkPatchSets()) {
+ return;
+ }
+ checkMerged();
+ }
+
+ private void checkOwner() {
+ try {
+ if (db.get().accounts().get(change.getOwner()) == null) {
+ messages.add("Missing change owner: " + change.getOwner());
+ }
+ } catch (OrmException e) {
+ error("Failed to look up owner", e);
+ }
+ }
+
+ private void checkCurrentPatchSetEntity() {
+ try {
+ PatchSet.Id psId = change.currentPatchSetId();
+ currPs = db.get().patchSets().get(psId);
+ if (currPs == null) {
+ messages.add(String.format("Current patch set %d not found", psId.get()));
+ }
+ } catch (OrmException e) {
+ error("Failed to look up current patch set", e);
+ }
+ }
+
+ private boolean openRepo() {
+ Project.NameKey project = change.getDest().getParentKey();
+ try {
+ repo = repoManager.openRepository(project);
+ rw = new RevWalk(repo);
+ return true;
+ } catch (RepositoryNotFoundException e) {
+ return error("Destination repository not found: " + project, e);
+ } catch (IOException e) {
+ return error("Failed to open repository: " + project, e);
+ }
+ }
+
+ private boolean checkPatchSets() {
+ List<PatchSet> all;
+ try {
+ all = db.get().patchSets().byChange(change.getId()).toList();
+ } catch (OrmException e) {
+ return error("Failed to look up patch sets", e);
+ }
+ Function<PatchSet, Integer> toPsId = new Function<PatchSet, Integer>() {
+ @Override
+ public Integer apply(PatchSet in) {
+ return in.getId().get();
+ }
+ };
+ Multimap<ObjectId, PatchSet> bySha = MultimapBuilder.hashKeys(all.size())
+ .treeSetValues(Ordering.natural().onResultOf(toPsId))
+ .build();
+ for (PatchSet ps : all) {
+ ObjectId objId;
+ String rev = ps.getRevision().get();
+ int psNum = ps.getId().get();
+ try {
+ objId = ObjectId.fromString(rev);
+ } catch (IllegalArgumentException e) {
+ error(String.format("Invalid revision on patch set %d: %s", psNum, rev),
+ e);
+ continue;
+ }
+ bySha.put(objId, ps);
+
+ RevCommit psCommit = parseCommit(
+ objId, String.format("patch set %d", psNum));
+ if (psCommit == null) {
+ continue;
+ }
+ if (ps.getId().equals(change.currentPatchSetId())) {
+ currPsCommit = psCommit;
+ }
+ }
+
+ for (Map.Entry<ObjectId, Collection<PatchSet>> e
+ : bySha.asMap().entrySet()) {
+ if (e.getValue().size() > 1) {
+ messages.add(String.format("Multiple patch sets pointing to %s: %s",
+ e.getKey().name(),
+ Collections2.transform(e.getValue(), toPsId)));
+ }
+ }
+
+ return currPs != null && currPsCommit != null;
+ }
+
+ private void checkMerged() {
+ String refName = change.getDest().get();
+ Ref dest;
+ try {
+ dest = repo.getRef(refName);
+ } catch (IOException e) {
+ messages.add("Failed to look up destination ref: " + refName);
+ return;
+ }
+ if (dest == null) {
+ messages.add("Destination ref not found (may be new branch): "
+ + change.getDest().get());
+ return;
+ }
+ RevCommit tip = parseCommit(dest.getObjectId(),
+ "destination ref " + refName);
+ if (tip == null) {
+ return;
+ }
+ boolean merged;
+ try {
+ merged = rw.isMergedInto(currPsCommit, tip);
+ } catch (IOException e) {
+ messages.add("Error checking whether patch set " + currPs.getId().get()
+ + " is merged");
+ return;
+ }
+ if (merged && change.getStatus() != Change.Status.MERGED) {
+ messages.add(String.format("Patch set %d (%s) is merged into destination"
+ + " ref %s (%s), but change status is %s", currPs.getId().get(),
+ currPsCommit.name(), refName, tip.name(), change.getStatus()));
+ // TODO(dborowitz): Just fix it.
+ } else if (!merged && change.getStatus() == Change.Status.MERGED) {
+ messages.add(String.format("Patch set %d (%s) is not merged into"
+ + " destination ref %s (%s), but change status is %s",
+ currPs.getId().get(), currPsCommit.name(), refName, tip.name(),
+ change.getStatus()));
+ }
+ }
+
+ private RevCommit parseCommit(ObjectId objId, String desc) {
+ try {
+ return rw.parseCommit(objId);
+ } catch (MissingObjectException e) {
+ messages.add(String.format("Object missing: %s: %s", desc, objId.name()));
+ } catch (IncorrectObjectTypeException e) {
+ messages.add(String.format("Not a commit: %s: %s", desc, objId.name()));
+ } catch (IOException e) {
+ messages.add(String.format("Failed to look up: %s: %s", desc, objId.name()));
+ }
+ return null;
+ }
+
+ private boolean error(String msg, Throwable t) {
+ messages.add(msg);
+ // TODO(dborowitz): Expose stack trace to administrators.
+ log.warn("Error in consistency check of change " + change.getId(), t);
+ return false;
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
index fc13786..634e19d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
@@ -52,6 +52,7 @@
get(CHANGE_KIND, "topic").to(GetTopic.class);
get(CHANGE_KIND, "in").to(IncludedIn.class);
get(CHANGE_KIND, "hashtags").to(GetHashtags.class);
+ get(CHANGE_KIND, "check").to(Check.class);
put(CHANGE_KIND, "topic").to(PutTopic.class);
delete(CHANGE_KIND, "topic").to(PutTopic.class);
delete(CHANGE_KIND).to(DeleteDraftChange.class);
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/change/ConsistencyCheckerTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/change/ConsistencyCheckerTest.java
new file mode 100644
index 0000000..3104b69
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/change/ConsistencyCheckerTest.java
@@ -0,0 +1,242 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.change;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testutil.TestChanges.incrementPatchSet;
+import static com.google.gerrit.testutil.TestChanges.newChange;
+import static com.google.gerrit.testutil.TestChanges.newPatchSet;
+import static java.util.Collections.singleton;
+
+import com.google.gerrit.common.TimeUtil;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.testutil.InMemoryDatabase;
+import com.google.gerrit.testutil.InMemoryRepositoryManager;
+import com.google.inject.util.Providers;
+
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ConsistencyCheckerTest {
+ private InMemoryDatabase schemaFactory;
+ private ReviewDb db;
+ private InMemoryRepositoryManager repoManager;
+ private ConsistencyChecker checker;
+
+ private TestRepository<InMemoryRepository> repo;
+ private Project.NameKey project;
+ private Account.Id userId;
+ private RevCommit tip;
+
+ @Before
+ public void setUp() throws Exception {
+ schemaFactory = InMemoryDatabase.newDatabase();
+ schemaFactory.create();
+ db = schemaFactory.open();
+ repoManager = new InMemoryRepositoryManager();
+ checker = new ConsistencyChecker(Providers.<ReviewDb> of(db), repoManager);
+ project = new Project.NameKey("repo");
+ repo = new TestRepository<>(repoManager.createRepository(project));
+ userId = new Account.Id(1);
+ db.accounts().insert(singleton(new Account(userId, TimeUtil.nowTs())));
+ tip = repo.branch("master").commit().create();
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ if (db != null) {
+ db.close();
+ }
+ if (schemaFactory != null) {
+ InMemoryDatabase.drop(schemaFactory);
+ }
+ }
+
+ @Test
+ public void validNewChange() throws Exception {
+ Change c = newChange(project, userId);
+ db.changes().insert(singleton(c));
+ RevCommit commit1 = repo.branch(c.currentPatchSetId().toRefName()).commit()
+ .parent(tip).create();
+ PatchSet ps1 = newPatchSet(c.currentPatchSetId(), commit1, userId);
+ db.patchSets().insert(singleton(ps1));
+
+ incrementPatchSet(c);
+ RevCommit commit2 = repo.branch(c.currentPatchSetId().toRefName()).commit()
+ .parent(tip).create();
+ PatchSet ps2 = newPatchSet(c.currentPatchSetId(), commit2, userId);
+ db.patchSets().insert(singleton(ps2));
+
+ assertThat(checker.check(c)).isEmpty();
+ }
+
+ @Test
+ public void validMergedChange() throws Exception {
+ Change c = newChange(project, userId);
+ c.setStatus(Change.Status.MERGED);
+ db.changes().insert(singleton(c));
+ RevCommit commit1 = repo.branch(c.currentPatchSetId().toRefName()).commit()
+ .parent(tip).create();
+ PatchSet ps1 = newPatchSet(c.currentPatchSetId(), commit1, userId);
+ db.patchSets().insert(singleton(ps1));
+
+ incrementPatchSet(c);
+ RevCommit commit2 = repo.branch(c.currentPatchSetId().toRefName()).commit()
+ .parent(tip).create();
+ PatchSet ps2 = newPatchSet(c.currentPatchSetId(), commit2, userId);
+ db.patchSets().insert(singleton(ps2));
+
+ repo.branch(c.getDest().get()).update(commit2);
+ assertThat(checker.check(c)).isEmpty();
+ }
+
+ @Test
+ public void missingOwner() throws Exception {
+ Change c = newChange(project, new Account.Id(2));
+ db.changes().insert(singleton(c));
+ RevCommit commit = repo.branch(c.currentPatchSetId().toRefName()).commit()
+ .parent(tip).create();
+ PatchSet ps = newPatchSet(c.currentPatchSetId(), commit, userId);
+ db.patchSets().insert(singleton(ps));
+
+ assertThat(checker.check(c)).containsExactly("Missing change owner: 2");
+ }
+
+ @Test
+ public void missingRepo() throws Exception {
+ Change c = newChange(new Project.NameKey("otherproject"), userId);
+ db.changes().insert(singleton(c));
+ PatchSet ps = newPatchSet(c.currentPatchSetId(),
+ ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"), userId);
+ db.patchSets().insert(singleton(ps));
+ assertThat(checker.check(c))
+ .containsExactly("Destination repository not found: otherproject");
+ }
+
+ @Test
+ public void invalidRevision() throws Exception {
+ Change c = newChange(project, userId);
+ db.changes().insert(singleton(c));
+
+ PatchSet ps = new PatchSet(c.currentPatchSetId());
+ ps.setRevision(new RevId("fooooooooooooooooooooooooooooooooooooooo"));
+ ps.setUploader(userId);
+ ps.setCreatedOn(TimeUtil.nowTs());
+ db.patchSets().insert(singleton(ps));
+
+ incrementPatchSet(c);
+ RevCommit commit2 = repo.branch(c.currentPatchSetId().toRefName()).commit()
+ .parent(tip).create();
+ PatchSet ps2 = newPatchSet(c.currentPatchSetId(), commit2, userId);
+ db.patchSets().insert(singleton(ps2));
+
+ assertThat(checker.check(c)).containsExactly(
+ "Invalid revision on patch set 1:"
+ + " fooooooooooooooooooooooooooooooooooooooo");
+ }
+
+ @Test
+ public void patchSetObjectMissing() throws Exception {
+ Change c = newChange(project, userId);
+ db.changes().insert(singleton(c));
+ PatchSet ps = newPatchSet(c.currentPatchSetId(),
+ ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"), userId);
+ db.patchSets().insert(singleton(ps));
+
+ assertThat(checker.check(c)).containsExactly(
+ "Object missing: patch set 1: deadbeefdeadbeefdeadbeefdeadbeefdeadbeef");
+ }
+
+ @Test
+ public void currentPatchSetMissing() throws Exception {
+ Change c = newChange(project, userId);
+ db.changes().insert(singleton(c));
+ assertThat(checker.check(c))
+ .containsExactly("Current patch set 1 not found");
+ }
+
+ @Test
+ public void duplicatePatchSetRevisions() throws Exception {
+ Change c = newChange(project, userId);
+ db.changes().insert(singleton(c));
+ RevCommit commit1 = repo.branch(c.currentPatchSetId().toRefName()).commit()
+ .parent(tip).create();
+ PatchSet ps1 = newPatchSet(c.currentPatchSetId(), commit1, userId);
+ db.patchSets().insert(singleton(ps1));
+
+ incrementPatchSet(c);
+ PatchSet ps2 = newPatchSet(c.currentPatchSetId(), commit1, userId);
+ db.patchSets().insert(singleton(ps2));
+
+ assertThat(checker.check(c)).containsExactly("Multiple patch sets pointing to "
+ + commit1.name() + ": [1, 2]");
+ }
+
+ @Test
+ public void missingDestRef() throws Exception {
+ RefUpdate ru = repo.getRepository().updateRef("refs/heads/master");
+ ru.setForceUpdate(true);
+ assertThat(ru.delete()).isEqualTo(RefUpdate.Result.FORCED);
+ Change c = newChange(project, userId);
+ db.changes().insert(singleton(c));
+ RevCommit commit = repo.commit().create();
+ PatchSet ps = newPatchSet(c.currentPatchSetId(), commit, userId);
+ db.patchSets().insert(singleton(ps));
+
+ assertThat(checker.check(c)).containsExactly(
+ "Destination ref not found (may be new branch): master");
+ }
+
+ @Test
+ public void mergedChangeIsNotMerged() throws Exception {
+ Change c = newChange(project, userId);
+ c.setStatus(Change.Status.MERGED);
+ db.changes().insert(singleton(c));
+ RevCommit commit = repo.branch(c.currentPatchSetId().toRefName()).commit()
+ .parent(tip).create();
+ PatchSet ps = newPatchSet(c.currentPatchSetId(), commit, userId);
+ db.patchSets().insert(singleton(ps));
+
+ assertThat(checker.check(c)).containsExactly(
+ "Patch set 1 (" + commit.name() + ") is not merged into destination ref"
+ + " master (" + tip.name() + "), but change status is MERGED");
+ }
+
+ @Test
+ public void newChangeIsMerged() throws Exception {
+ Change c = newChange(project, userId);
+ db.changes().insert(singleton(c));
+ RevCommit commit = repo.branch(c.currentPatchSetId().toRefName()).commit()
+ .parent(tip).create();
+ PatchSet ps = newPatchSet(c.currentPatchSetId(), commit, userId);
+ db.patchSets().insert(singleton(ps));
+ repo.branch(c.getDest().get()).update(commit);
+
+ assertThat(checker.check(c)).containsExactly(
+ "Patch set 1 (" + commit.name() + ") is merged into destination ref"
+ + " master (" + commit.name() + "), but change status is NEW");
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java
index cfcbc7a..fa7bc259 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java
@@ -24,6 +24,7 @@
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.AllUsersName;
@@ -39,6 +40,7 @@
import com.google.inject.Injector;
import org.easymock.EasyMock;
+import org.eclipse.jgit.lib.ObjectId;
import java.util.concurrent.atomic.AtomicInteger;
@@ -66,6 +68,15 @@
return c;
}
+ public static PatchSet newPatchSet(PatchSet.Id id, ObjectId revision,
+ Account.Id userId) {
+ PatchSet ps = new PatchSet(id);
+ ps.setRevision(new RevId(revision.name()));
+ ps.setUploader(userId);
+ ps.setCreatedOn(TimeUtil.nowTs());
+ return ps;
+ }
+
public static ChangeUpdate newUpdate(Injector injector,
GitRepositoryManager repoManager, NotesMigration migration, Change c,
final AllUsersNameProvider allUsers, final IdentifiedUser user)
