From cfc9e6da54e08900139841e1f89fe3945d8d39c6 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Thu, 23 Feb 2017 16:20:05 -0500 Subject: [PATCH] Rebase: Fix CurrentRevision in NoteDb This caller was missed long ago when we refactored everything to use PatchSetUtil to look up PatchSets. Probably not coincidentally because it was never tested, nor indeed plumbed into ChangeApi. Change-Id: Ib77304286df14f22370375911bb59bd8bd3e6356 --- .../acceptance/api/change/ChangeIT.java | 19 ++++++++++++++++-- .../extensions/api/changes/ChangeApi.java | 16 +++++++++++++++ .../server/api/changes/ChangeApiImpl.java | 20 +++++++++++++++++++ .../google/gerrit/server/change/Rebase.java | 10 ++++++---- 4 files changed, 59 insertions(+), 6 deletions(-) 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 0559cb0f36..8fa80dd007 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 @@ -88,6 +88,7 @@ import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.MethodNotAllowedException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; +import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.UnprocessableEntityException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; @@ -328,8 +329,22 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().id(r.getChangeId()).revert(); } + @FunctionalInterface + private interface Rebase { + void call(String id) throws RestApiException; + } + @Test - public void rebase() throws Exception { + public void rebaseViaRevisionApi() throws Exception { + testRebase(id -> gApi.changes().id(id).current().rebase()); + } + + @Test + public void rebaseViaChangeApi() throws Exception { + testRebase(id -> gApi.changes().id(id).rebase()); + } + + private void testRebase(Rebase rebase) throws Exception { // Create two changes both with the same parent PushOneCommit.Result r = createChange(); testRepo.reset("HEAD~1"); @@ -342,7 +357,7 @@ public class ChangeIT extends AbstractDaemonTest { String changeId = r2.getChangeId(); // Rebase the second change - gApi.changes().id(changeId).current().rebase(); + rebase.call(changeId); // Second change should have 2 patch sets ChangeInfo c2 = gApi.changes().id(changeId).get(); diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java index 8d143267c4..27fdc180ad 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java @@ -114,6 +114,12 @@ public interface ChangeApi { /** Publishes a draft change. */ void publish() throws RestApiException; + /** Rebase the current revision of a change using default options. */ + void rebase() throws RestApiException; + + /** Rebase the current revision of a change. */ + void rebase(RebaseInput in) throws RestApiException; + /** Deletes a change. */ void delete() throws RestApiException; @@ -315,6 +321,16 @@ public interface ChangeApi { throw new NotImplementedException(); } + @Override + public void rebase() { + throw new NotImplementedException(); + } + + @Override + public void rebase(RebaseInput in) { + throw new NotImplementedException(); + } + @Override public void delete() { throw new NotImplementedException(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java index 52353ca696..5c398eaafe 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java @@ -14,6 +14,7 @@ package com.google.gerrit.server.api.changes; +import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.extensions.api.changes.AbandonInput; import com.google.gerrit.extensions.api.changes.AddReviewerInput; import com.google.gerrit.extensions.api.changes.AssigneeInput; @@ -24,6 +25,7 @@ import com.google.gerrit.extensions.api.changes.FixInput; import com.google.gerrit.extensions.api.changes.HashtagsInput; import com.google.gerrit.extensions.api.changes.IncludedInInfo; import com.google.gerrit.extensions.api.changes.MoveInput; +import com.google.gerrit.extensions.api.changes.RebaseInput; import com.google.gerrit.extensions.api.changes.RestoreInput; import com.google.gerrit.extensions.api.changes.RevertInput; import com.google.gerrit.extensions.api.changes.ReviewerApi; @@ -63,6 +65,7 @@ import com.google.gerrit.server.change.PostReviewers; import com.google.gerrit.server.change.PublishDraftPatchSet; import com.google.gerrit.server.change.PutAssignee; import com.google.gerrit.server.change.PutTopic; +import com.google.gerrit.server.change.Rebase; import com.google.gerrit.server.change.Restore; import com.google.gerrit.server.change.Revert; import com.google.gerrit.server.change.Reviewers; @@ -99,6 +102,7 @@ class ChangeApiImpl implements ChangeApi { private final CreateMergePatchSet updateByMerge; private final Provider submittedTogether; private final PublishDraftPatchSet.CurrentRevision publishDraftChange; + private final Rebase.CurrentRevision rebase; private final DeleteChange deleteChange; private final GetTopic getTopic; private final PutTopic putTopic; @@ -133,6 +137,7 @@ class ChangeApiImpl implements ChangeApi { CreateMergePatchSet updateByMerge, Provider submittedTogether, PublishDraftPatchSet.CurrentRevision publishDraftChange, + Rebase.CurrentRevision rebase, DeleteChange deleteChange, GetTopic getTopic, PutTopic putTopic, @@ -165,6 +170,7 @@ class ChangeApiImpl implements ChangeApi { this.updateByMerge = updateByMerge; this.submittedTogether = submittedTogether; this.publishDraftChange = publishDraftChange; + this.rebase = rebase; this.deleteChange = deleteChange; this.getTopic = getTopic; this.putTopic = putTopic; @@ -325,6 +331,20 @@ class ChangeApiImpl implements ChangeApi { } } + @Override + public void rebase() throws RestApiException { + rebase(new RebaseInput()); + } + + @Override + public void rebase(RebaseInput in) throws RestApiException { + try { + rebase.apply(change, in); + } catch (EmailException | OrmException | UpdateException | RestApiException | IOException e) { + throw new RestApiException("Cannot rebase change", e); + } + } + @Override public void delete() throws RestApiException { try { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java index 046712db30..608a56f6f3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java @@ -29,6 +29,7 @@ import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change.Status; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.change.RebaseUtil.Base; import com.google.gerrit.server.git.BatchUpdate; import com.google.gerrit.server.git.GitRepositoryManager; @@ -204,18 +205,19 @@ public class Rebase } public static class CurrentRevision implements RestModifyView { + private final PatchSetUtil psUtil; private final Rebase rebase; @Inject - CurrentRevision(Rebase rebase) { + CurrentRevision(PatchSetUtil psUtil, Rebase rebase) { + this.psUtil = psUtil; this.rebase = rebase; } @Override public ChangeInfo apply(ChangeResource rsrc, RebaseInput input) - throws EmailException, OrmException, UpdateException, RestApiException, IOException, - NoSuchChangeException { - PatchSet ps = rebase.dbProvider.get().patchSets().get(rsrc.getChange().currentPatchSetId()); + throws EmailException, OrmException, UpdateException, RestApiException, IOException { + PatchSet ps = psUtil.current(rebase.dbProvider.get(), rsrc.getNotes()); if (ps == null) { throw new ResourceConflictException("current revision is missing"); } else if (!rsrc.getControl().isPatchVisible(ps, rebase.dbProvider.get())) {