Change kind cache: Don't fail if prior ps has no parent, but next ps has 1 parent
This case failed with:
[2016-11-14 18:44:53,805] [ReceiveCommits-1] ERROR com.google.gerrit.server.git.ReceiveCommits : [com.google.gerrit.acceptance.api.change.ChangeIT_changeNoParentToOneParent_project-1479177893785-ed68f500]Can't insert change/patch set for com.google.gerrit.acceptance.api.change.ChangeIT_changeNoParentToOneParent_project
com.google.gerrit.extensions.restapi.RestApiException: Error inserting change/patchset
at com.google.gerrit.server.git.ReceiveCommits$1.apply(ReceiveCommits.java:274)
at com.google.gerrit.server.git.ReceiveCommits$1.apply(ReceiveCommits.java:1)
at com.google.gerrit.server.git.ReceiveCommits.insertChangesAndPatchSets(ReceiveCommits.java:851)
at com.google.gerrit.server.git.ReceiveCommits.processCommands(ReceiveCommits.java:605)
at com.google.gerrit.server.git.AsyncReceiveCommits$Worker.run(AsyncReceiveCommits.java:89)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
at com.google.gerrit.server.util.RequestScopePropagator$5.call(RequestScopePropagator.java:221)
at com.google.gerrit.server.util.RequestScopePropagator$4.call(RequestScopePropagator.java:200)
at com.google.gerrit.server.util.ThreadLocalRequestScopePropagator$1.call(ThreadLocalRequestScopePropagator.java:55)
at com.google.gerrit.server.util.RequestScopePropagator$1.call(RequestScopePropagator.java:99)
at com.google.gerrit.server.util.RequestScopePropagator$2.run(RequestScopePropagator.java:131)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:180)
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
at com.google.gerrit.server.git.WorkQueue$Task.run(WorkQueue.java:417)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:745)
Caused by: com.google.gerrit.server.git.UpdateException: com.google.common.util.concurrent.UncheckedExecutionException: java.lang.ArrayIndexOutOfBoundsException: 0
at com.google.gerrit.server.git.BatchUpdate.executeUpdateRepo(BatchUpdate.java:697)
at com.google.gerrit.server.git.BatchUpdate.execute(BatchUpdate.java:421)
at com.google.gerrit.server.git.BatchUpdate.execute(BatchUpdate.java:673)
at com.google.gerrit.server.git.BatchUpdate.execute(BatchUpdate.java:668)
at com.google.gerrit.server.git.ReceiveCommits.insertChangesAndPatchSets(ReceiveCommits.java:849)
... 16 more
Caused by: com.google.common.util.concurrent.UncheckedExecutionException: java.lang.ArrayIndexOutOfBoundsException: 0
at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2214)
at com.google.common.cache.LocalCache.get(LocalCache.java:4053)
at com.google.common.cache.LocalCache$LocalManualCache.get(LocalCache.java:4899)
at com.google.gerrit.server.change.ChangeKindCacheImpl.getChangeKind(ChangeKindCacheImpl.java:338)
at com.google.gerrit.server.git.ReplaceOp.updateRepo(ReplaceOp.java:194)
at com.google.gerrit.server.git.BatchUpdate.executeUpdateRepo(BatchUpdate.java:681)
... 20 more
Caused by: java.lang.ArrayIndexOutOfBoundsException: 0
at org.eclipse.jgit.revwalk.RevCommit.getParent(RevCommit.java:353)
at com.google.gerrit.server.change.ChangeKindCacheImpl$Loader.call(ChangeKindCacheImpl.java:245)
at com.google.gerrit.server.change.ChangeKindCacheImpl$Loader.call(ChangeKindCacheImpl.java:1)
at com.google.common.cache.LocalCache$LocalManualCache$1.load(LocalCache.java:4904)
at com.google.common.cache.LocalCache$LoadingValueReference.loadFuture(LocalCache.java:3628)
at com.google.common.cache.LocalCache$Segment.loadSync(LocalCache.java:2336)
at com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(LocalCache.java:2295)
at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2208)
... 25 more
Change-Id: I689b703d4912d54bfc659def577b689fc1cda887
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 006d8a4..7bea801 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -16,6 +16,8 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
+import static com.google.gerrit.acceptance.GitUtil.assertPushOk;
+import static com.google.gerrit.acceptance.GitUtil.pushHead;
import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
import static com.google.gerrit.extensions.client.ReviewerState.CC;
@@ -50,6 +52,7 @@
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.api.projects.BranchInput;
+import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.SubmitType;
@@ -88,6 +91,7 @@
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.PushResult;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -538,6 +542,48 @@
}
@Test
+ @TestProjectInput(createEmptyCommit = false)
+ public void changeNoParentToOneParent() throws Exception {
+ // create initial commit with no parent and push it as change, so that patch
+ // set 1 has no parent
+ RevCommit c =
+ testRepo.commit().message("Initial commit").insertChangeId().create();
+ String id = GitUtil.getChangeId(testRepo, c).get();
+ testRepo.reset(c);
+
+ PushResult pr = pushHead(testRepo, "refs/for/master", false);
+ assertPushOk(pr, "refs/for/master");
+
+ ChangeInfo change = gApi.changes().id(id).get();
+ assertThat(change.revisions.get(change.currentRevision).commit.parents)
+ .isEmpty();
+
+ // create another initial commit with no parent and push it directly into
+ // the remote repository
+ c = testRepo.amend(c.getId()).message("Initial Empty Commit").create();
+ testRepo.reset(c);
+ pr = pushHead(testRepo, "refs/heads/master", false);
+ assertPushOk(pr, "refs/heads/master");
+
+ // create a successor commit and push it as second patch set to the change,
+ // so that patch set 2 has 1 parent
+ RevCommit c2 = testRepo.commit().message("Initial commit").parent(c)
+ .insertChangeId(id.substring(1)).create();
+ testRepo.reset(c2);
+
+ pr = pushHead(testRepo, "refs/for/master", false);
+ assertPushOk(pr, "refs/for/master");
+
+ change = gApi.changes().id(id).get();
+ RevisionInfo rev = change.revisions.get(change.currentRevision);
+ assertThat(rev.commit.parents).hasSize(1);
+ assertThat(rev.commit.parents.get(0).commit).isEqualTo(c.name());
+
+ // check that change kind is correctly detected as REWORK
+ assertThat(rev.kind).isEqualTo(ChangeKind.REWORK);
+ }
+
+ @Test
public void addReviewerThatCannotSeeChange() throws Exception {
// create hidden project that is only visible to administrators
Project.NameKey p = createProject("p");
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
index edc1b12..1d1b27b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
@@ -222,7 +222,8 @@
}
if ((prior.getParentCount() != 1 || next.getParentCount() != 1)
- && !onlyFirstParentChanged(prior, next)) {
+ && (!onlyFirstParentChanged(prior, next)
+ || prior.getParentCount() == 0)) {
// Trivial rebases done by machine only work well on 1 parent.
return ChangeKind.REWORK;
}
