From 837c0ee0d2c2ac369b7acbf8bf14902a9fed2832 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sun, 27 Apr 2014 12:54:20 +0200 Subject: [PATCH] Add REST endpoint to create empty change This change is based on change-fabricator plugin [1] and is a prerequisite for inline edit feature to support 100% Gerrit workflow in browser. [1] https://github.com/davido/gerrit-change-fabricator Bug: Issue 2338 Change-Id: Idf290c14d5a478479fe1cb916ee227aa11435fcd --- Documentation/rest-api-changes.txt | 58 +++- .../rest/change/CreateChangeIT.java | 90 ++++++ .../server/api/changes/ChangeInfoMapper.java | 27 +- .../server/change/ChangesCollection.java | 16 +- .../gerrit/server/change/CreateChange.java | 265 ++++++++++++++++++ .../google/gerrit/server/change/Module.java | 1 + .../google/gerrit/server/git/MergeUtil.java | 10 +- 7 files changed, 449 insertions(+), 18 deletions(-) create mode 100644 gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 74a411f1fe..08721420d2 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -7,6 +7,63 @@ link:rest-api.html[REST API]. [[change-endpoints]] == Change Endpoints +[[create-change]] +=== Create Change +-- +'POST /changes' +-- + +The change info link:#change-info[ChangeInfo] entity must be provided in the +request body. Only the following attributes are honored: `project`, +`branch`, `subject`, `status` and `topic`. The first three attributes are +mandatory. Valid values for status are: `DRAFT` and `NEW`. + +.Request +---- + POST /changes HTTP/1.0 + Content-Type: application/json;charset=UTF-8 + + { + "project" : "myProject", + "subject" : "Let's support 100% Gerrit workflow direct in browser", + "branch" : "master", + "topic" : "create-change-in-browser", + "status" : "DRAFT" + } +---- + +As response a link:#change-info[ChangeInfo] entity is returned that describes +the resulting change. + +.Response +---- + HTTP/1.1 200 OK + Content-Disposition: attachment + Content-Type: application/json;charset=UTF-8 + + )]}' + { + "kind": "gerritcodereview#change", + "id": "myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9941", + "project": "myProject", + "branch": "master", + "topic": "create-change-in-browser", + "change_id": "I8473b95934b5732ac55d26311a706c9c2bde9941", + "subject": "Let's support 100% Gerrit workflow direct in browser", + "status": "DRAFT", + "created": "2014-05-05 07:15:44.639000000", + "updated": "2014-05-05 07:15:44.639000000", + "mergeable": true, + "insertions": 0, + "deletions": 0, + "_sortkey": "002cbc25000004e5", + "_number": 4711, + "owner": { + "name": "John Doe" + } + } +---- + [[list-changes]] === Query Changes -- @@ -3393,7 +3450,6 @@ The `WebLinkInfo` entity describes a link to an external site. |`url` |The link URL. |====================== - GERRIT ------ Part of link:index.html[Gerrit Code Review] diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java new file mode 100644 index 0000000000..6af7adbe6b --- /dev/null +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java @@ -0,0 +1,90 @@ +// 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.acceptance.rest.change; + +import static org.junit.Assert.assertEquals; + +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.RestResponse; +import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.common.ChangeStatus; +import com.google.gerrit.server.change.ChangeJson; + +import org.apache.http.HttpStatus; +import org.junit.Test; + +public class CreateChangeIT extends AbstractDaemonTest { + + @Test + public void createEmptyChange_MissingBranch() throws Exception { + ChangeInfo ci = new ChangeInfo(); + ci.project = project.get(); + RestResponse r = adminSession.post("/changes/", ci); + assertEquals(HttpStatus.SC_BAD_REQUEST, r.getStatusCode()); + r.getEntityContent().contains("branch must be non-empty"); + } + + @Test + public void createEmptyChange_MissingMessage() throws Exception { + ChangeInfo ci = new ChangeInfo(); + ci.project = project.get(); + ci.branch = "master"; + RestResponse r = adminSession.post("/changes/", ci); + assertEquals(HttpStatus.SC_BAD_REQUEST, r.getStatusCode()); + r.getEntityContent().contains("commit message must be non-empty"); + } + + @Test + public void createEmptyChange_InvalidStatus() throws Exception { + ChangeInfo ci = newChangeInfo(ChangeStatus.SUBMITTED); + RestResponse r = adminSession.post("/changes/", ci); + assertEquals(HttpStatus.SC_BAD_REQUEST, r.getStatusCode()); + r.getEntityContent().contains("unsupported change status"); + } + + @Test + public void createNewChange() throws Exception { + assertChange(newChangeInfo(ChangeStatus.NEW)); + } + + @Test + public void createDraftChange() throws Exception { + assertChange(newChangeInfo(ChangeStatus.DRAFT)); + } + + private ChangeInfo newChangeInfo(ChangeStatus status) { + ChangeInfo in = new ChangeInfo(); + in.project = project.get(); + in.branch = "master"; + in.subject = "Empty change"; + in.topic = "support-gerrit-workflow-in-browser"; + in.status = status; + return in; + } + + private void assertChange(ChangeInfo in) throws Exception { + RestResponse r = adminSession.post("/changes/", in); + assertEquals(HttpStatus.SC_CREATED, r.getStatusCode()); + + ChangeJson.ChangeInfo info = newGson().fromJson(r.getReader(), + ChangeJson.ChangeInfo.class); + ChangeInfo out = get(info.changeId); + + assertEquals(in.branch, out.branch); + assertEquals(in.subject, out.subject); + assertEquals(in.topic, out.topic); + assertEquals(in.status, out.status); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java index c2dae0dba4..6aab89cf3d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeInfoMapper.java @@ -21,7 +21,7 @@ import static com.google.gerrit.extensions.common.ListChangesOption.DETAILED_LAB import static com.google.gerrit.extensions.common.ListChangesOption.LABELS; import static com.google.gerrit.extensions.common.ListChangesOption.MESSAGES; -import com.google.common.collect.ImmutableMap; +import com.google.common.collect.EnumBiMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.gerrit.extensions.common.ApprovalInfo; @@ -39,14 +39,16 @@ import java.util.EnumSet; import java.util.List; import java.util.Map; -class ChangeInfoMapper { - private final static ImmutableMap MAP = - Maps.immutableEnumMap(ImmutableMap.of( - Status.DRAFT, ChangeStatus.DRAFT, - Status.NEW, ChangeStatus.NEW, - Status.SUBMITTED, ChangeStatus.SUBMITTED, - Status.MERGED, ChangeStatus.MERGED, - Status.ABANDONED, ChangeStatus.ABANDONED)); +public class ChangeInfoMapper { + private final static EnumBiMap MAP = + EnumBiMap.create(Change.Status.class, ChangeStatus.class); + static { + MAP.put(Status.DRAFT, ChangeStatus.DRAFT); + MAP.put(Status.NEW, ChangeStatus.NEW); + MAP.put(Status.SUBMITTED, ChangeStatus.SUBMITTED); + MAP.put(Status.MERGED, ChangeStatus.MERGED); + MAP.put(Status.ABANDONED, ChangeStatus.ABANDONED); + } private final EnumSet s; @@ -136,6 +138,13 @@ class ChangeInfoMapper { return s.contains(o); } + public static Status changeStatus2Status(ChangeStatus status) { + if (status != null) { + return MAP.inverse().get(status); + } + return Change.Status.NEW; + } + private static ApprovalInfo fromApprovalInfo(ChangeJson.ApprovalInfo ai) { ApprovalInfo ao = new ApprovalInfo(); ao.value = ai.value; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java index f7038549df..73f9b2e383 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java @@ -16,8 +16,10 @@ package com.google.gerrit.server.change; import com.google.common.collect.ImmutableList; import com.google.gerrit.extensions.registration.DynamicMap; +import com.google.gerrit.extensions.restapi.AcceptsPost; import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; +import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestCollection; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.extensions.restapi.TopLevelResource; @@ -35,12 +37,14 @@ import java.util.Collections; import java.util.List; public class ChangesCollection implements - RestCollection { + RestCollection, + AcceptsPost { private final Provider db; private final Provider user; private final ChangeControl.GenericFactory changeControlFactory; private final Provider queryFactory; private final DynamicMap> views; + private final CreateChange.Factory createChangeFactory; @Inject ChangesCollection( @@ -48,12 +52,14 @@ public class ChangesCollection implements Provider user, ChangeControl.GenericFactory changeControlFactory, Provider queryFactory, - DynamicMap> views) { + DynamicMap> views, + CreateChange.Factory createChangeFactory) { this.db = dbProvider; this.user = user; this.changeControlFactory = changeControlFactory; this.queryFactory = queryFactory; this.views = views; + this.createChangeFactory = createChangeFactory; } @Override @@ -125,4 +131,10 @@ public class ChangesCollection implements triplet.getBranchNameKey(), triplet.getChangeKey()).toList(); } + + @SuppressWarnings("unchecked") + @Override + public CreateChange post(TopLevelResource parent) throws RestApiException { + return createChangeFactory.create(); + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java new file mode 100644 index 0000000000..f1e4309784 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java @@ -0,0 +1,265 @@ +// 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.Strings; +import com.google.gerrit.common.data.Capable; +import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.common.ChangeStatus; +import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.BadRequestException; +import com.google.gerrit.extensions.restapi.Response; +import com.google.gerrit.extensions.restapi.RestModifyView; +import com.google.gerrit.extensions.restapi.TopLevelResource; +import com.google.gerrit.extensions.restapi.UnprocessableEntityException; +import com.google.gerrit.reviewdb.client.Branch; +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.CurrentUser; +import com.google.gerrit.server.GerritPersonIdent; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.api.changes.ChangeInfoMapper; +import com.google.gerrit.server.events.CommitReceivedEvent; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.git.MergeUtil; +import com.google.gerrit.server.git.validators.CommitValidationException; +import com.google.gerrit.server.git.validators.CommitValidators; +import com.google.gerrit.server.project.InvalidChangeOperationException; +import com.google.gerrit.server.project.ProjectResource; +import com.google.gerrit.server.project.ProjectsCollection; +import com.google.gerrit.server.project.RefControl; +import com.google.gerrit.server.ssh.NoSshInfo; +import com.google.gerrit.server.util.TimeUtil; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Provider; + +import org.eclipse.jgit.lib.CommitBuilder; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectInserter; +import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.RefUpdate; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; +import org.eclipse.jgit.transport.ReceiveCommand; +import org.eclipse.jgit.util.ChangeIdUtil; + +import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.util.List; + +public class CreateChange implements + RestModifyView { + + public static interface Factory { + CreateChange create(); + } + + private final ReviewDb db; + private final GitRepositoryManager gitManager; + private final PersonIdent myIdent; + private final Provider userProvider; + private final Provider projectsCollection; + private final CommitValidators.Factory commitValidatorsFactory; + private final ChangeInserter.Factory changeInserterFactory; + private final ChangeJson json; + + @Inject + CreateChange(ReviewDb db, + GitRepositoryManager gitManager, + @GerritPersonIdent PersonIdent myIdent, + Provider userProvider, + Provider projectsCollection, + CommitValidators.Factory commitValidatorsFactory, + ChangeInserter.Factory changeInserterFactory, + ChangeJson json) { + this.db = db; + this.gitManager = gitManager; + this.myIdent = myIdent; + this.userProvider = userProvider; + this.projectsCollection = projectsCollection; + this.commitValidatorsFactory = commitValidatorsFactory; + this.changeInserterFactory = changeInserterFactory; + this.json = json; + } + + @Override + public Response apply(TopLevelResource parent, + ChangeInfo input) throws AuthException, OrmException, + BadRequestException, UnprocessableEntityException, IOException, + InvalidChangeOperationException { + + if (Strings.isNullOrEmpty(input.project)) { + throw new BadRequestException("project must be non-empty"); + } + + if (Strings.isNullOrEmpty(input.branch)) { + throw new BadRequestException("branch must be non-empty"); + } + + if (Strings.isNullOrEmpty(input.subject)) { + throw new BadRequestException("commit message must be non-empty"); + } + + if (input.status != null) { + if (input.status != ChangeStatus.NEW + && input.status != ChangeStatus.DRAFT) { + throw new BadRequestException("unsupported change status"); + } + } + + String refName = input.branch; + if (!refName.startsWith(Constants.R_REFS)) { + refName = Constants.R_HEADS + input.branch; + } + + ProjectResource rsrc = projectsCollection.get().parse(input.project); + + Capable r = rsrc.getControl().canPushToAtLeastOneRef(); + if (r != Capable.OK) { + throw new AuthException(r.getMessage()); + } + + RefControl refControl = rsrc.getControl().controlForRef(refName); + if (!refControl.canUpload() || !refControl.canRead()) { + throw new AuthException("cannot upload review"); + } + + Project.NameKey project = rsrc.getNameKey(); + Repository git = gitManager.openRepository(project); + + try { + RevWalk rw = new RevWalk(git); + try { + Ref destRef = git.getRef(refName); + if (destRef == null) { + throw new UnprocessableEntityException(String.format( + "Branch %s does not exist.", refName)); + } + + IdentifiedUser me = (IdentifiedUser)userProvider.get(); + PersonIdent author = + me.newCommitterIdent(myIdent.getWhen(), myIdent.getTimeZone()); + + RevCommit mergeTip = rw.parseCommit(destRef.getObjectId()); + ObjectId id = ChangeIdUtil.computeChangeId(mergeTip.getTree(), + mergeTip, author, author, input.subject); + String commitMessage = ChangeIdUtil.insertId(input.subject, id); + + RevCommit c = newCommit(git, rw, author, mergeTip, commitMessage); + + Change change = new Change( + getChangeId(id, c), + new Change.Id(db.nextChangeId()), + me.getAccountId(), + new Branch.NameKey(project, destRef.getName()), + TimeUtil.nowTs()); + + ChangeInserter ins = + changeInserterFactory.create(refControl, change, c); + + validateCommit(git, refControl, c, me, ins); + updateRef(git, rw, c, change, ins.getPatchSet()); + + change.setTopic(input.topic); + change.setStatus(ChangeInfoMapper.changeStatus2Status(input.status)); + ins.insert(); + + return Response.created(json.format(change.getId())); + } finally { + rw.release(); + } + } finally { + git.close(); + } + } + + private void validateCommit(Repository git, RefControl refControl, + RevCommit c, IdentifiedUser me, ChangeInserter ins) + throws InvalidChangeOperationException { + PatchSet newPatchSet = ins.getPatchSet(); + CommitValidators commitValidators = + commitValidatorsFactory.create(refControl, new NoSshInfo(), git); + CommitReceivedEvent commitReceivedEvent = + new CommitReceivedEvent(new ReceiveCommand( + ObjectId.zeroId(), + c.getId(), + newPatchSet.getRefName()), + refControl.getProjectControl().getProject(), + refControl.getRefName(), + c, + me); + + try { + commitValidators.validateForGerritCommits(commitReceivedEvent); + } catch (CommitValidationException e) { + throw new InvalidChangeOperationException(e.getMessage()); + } + } + + private static void updateRef(Repository git, RevWalk rw, RevCommit c, + Change change, PatchSet newPatchSet) throws IOException { + RefUpdate ru = git.updateRef(newPatchSet.getRefName()); + ru.setExpectedOldObjectId(ObjectId.zeroId()); + ru.setNewObjectId(c); + ru.disableRefLog(); + if (ru.update(rw) != RefUpdate.Result.NEW) { + throw new IOException(String.format( + "Failed to create ref %s in %s: %s", newPatchSet.getRefName(), + change.getDest().getParentKey().get(), ru.getResult())); + } + } + + private static Change.Key getChangeId(ObjectId id, RevCommit emptyCommit) { + List idList = emptyCommit.getFooterLines( + MergeUtil.CHANGE_ID); + Change.Key changeKey = !idList.isEmpty() + ? new Change.Key(idList.get(idList.size() - 1).trim()) + : new Change.Key("I" + id.name()); + return changeKey; + } + + private static RevCommit newCommit(Repository git, RevWalk rw, + PersonIdent authorIdent, RevCommit mergeTip, String commitMessage) + throws IOException { + RevCommit emptyCommit; + ObjectInserter oi = git.newObjectInserter(); + try { + CommitBuilder commit = new CommitBuilder(); + commit.setTreeId(mergeTip.getTree().getId()); + commit.setParentId(mergeTip); + commit.setAuthor(authorIdent); + commit.setCommitter(authorIdent); + commit.setMessage(commitMessage); + emptyCommit = rw.parseCommit(insert(oi, commit)); + } finally { + oi.release(); + } + return emptyCommit; + } + + private static ObjectId insert(ObjectInserter inserter, + CommitBuilder commit) throws IOException, + UnsupportedEncodingException { + ObjectId id = inserter.insert(commit); + inserter.flush(); + return id; + } +} 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 6880ca297e..acb6eeffc6 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 @@ -107,6 +107,7 @@ public class Module extends RestApiModule { factory(EmailReviewComments.Factory.class); factory(ChangeInserter.Factory.class); factory(PatchSetInserter.Factory.class); + factory(CreateChange.Factory.class); } }); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java index 742499bb26..b9b5621e19 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java @@ -78,7 +78,11 @@ import java.util.Set; import java.util.TimeZone; public class MergeUtil { + public static final FooterKey CHANGE_ID = new FooterKey("Change-Id"); + private static final FooterKey REVIEWED_ON = new FooterKey("Reviewed-on"); private static final Logger log = LoggerFactory.getLogger(MergeUtil.class); + private static final String R_HEADS_MASTER = + Constants.R_HEADS + Constants.MASTER; public static boolean useRecursiveMerge(Config cfg) { return cfg.getBoolean("core", null, "useRecursiveMerge", false); @@ -89,12 +93,6 @@ public class MergeUtil { MergeUtil create(ProjectState project, boolean useContentMerge); } - private static final String R_HEADS_MASTER = - Constants.R_HEADS + Constants.MASTER; - - private static final FooterKey REVIEWED_ON = new FooterKey("Reviewed-on"); - private static final FooterKey CHANGE_ID = new FooterKey("Change-Id"); - private final Provider db; private final IdentifiedUser.GenericFactory identifiedUserFactory; private final Provider urlProvider;