CreateChange: move permission checks to its own method

This commit groups those permission checks to a dedicated
method. It also renames "rsrc" to "projectResource" to make
it more readable.

Change-Id: I7b9ed0f9aa3d4af5e3fb1a9d0b9de96455d605a8

java/com/google/gerrit/server/restapi/change/CreateChange.java

@@ -164,24 +164,14 @@ public class CreateChange
           UpdateException, PermissionBackendException, ConfigInvalidException {
     checkAndSanitizeChangeInput(input);
 
-    ProjectResource rsrc = projectsCollection.parse(input.project);
+    ProjectResource projectResource = projectsCollection.parse(input.project);
+    ProjectState projectState = projectResource.getProjectState();
+    projectState.checkStatePermitsWrite();
 
-    contributorAgreements.check(rsrc.getNameKey(), rsrc.getUser());
+    Project.NameKey project = projectResource.getNameKey();
+    contributorAgreements.check(project, user.get());
 
-    Project.NameKey project = rsrc.getNameKey();
-    String refName = RefNames.fullName(input.branch);
-    try {
-      permissionBackend.currentUser().project(project).ref(refName).check(RefPermission.READ);
-    } catch (AuthException e) {
-      throw new ResourceNotFoundException(String.format("ref %s not found", refName));
-    }
-
-    permissionBackend
-        .currentUser()
-        .project(project)
-        .ref(refName)
-        .check(RefPermission.CREATE_CHANGE);
-    rsrc.getProjectState().checkStatePermitsWrite();
+    checkRequiredPermissions(project, input.branch);
 
     try (Repository git = gitManager.openRepository(project);
         ObjectInserter oi = git.newObjectInserter();
@@ -189,7 +179,7 @@ public class CreateChange
         RevWalk rw = new RevWalk(reader)) {
       ObjectId parentCommit;
       List<String> groups;
-      Ref destRef = git.getRefDatabase().exactRef(refName);
+      Ref destRef = git.getRefDatabase().exactRef(input.branch);
       if (input.baseChange != null) {
         List<ChangeNotes> notes = changeFinder.find(input.baseChange);
         if (notes.size() != 1) {
@@ -215,14 +205,14 @@ public class CreateChange
         RevCommit destRefRevCommit = rw.parseCommit(destRef.getObjectId());
         if (!rw.isMergedInto(parentRevCommit, destRefRevCommit)) {
           throw new BadRequestException(
-              String.format("Commit %s doesn't exist on ref %s", input.baseCommit, refName));
+              String.format("Commit %s doesn't exist on ref %s", input.baseCommit, input.branch));
         }
         groups = Collections.emptyList();
       } else {
         if (destRef != null) {
           if (Boolean.TRUE.equals(input.newBranch)) {
             throw new ResourceConflictException(
-                String.format("Branch %s already exists.", refName));
+                String.format("Branch %s already exists.", input.branch));
           }
           parentCommit = destRef.getObjectId();
         } else {
@@ -244,7 +234,7 @@ public class CreateChange
 
       boolean isWorkInProgress =
           input.workInProgress == null
-              ? rsrc.getProjectState().is(BooleanProjectConfig.WORK_IN_PROGRESS_BY_DEFAULT)
+              ? projectState.is(BooleanProjectConfig.WORK_IN_PROGRESS_BY_DEFAULT)
                   || MoreObjects.firstNonNull(info.workInProgressByDefault, false)
               : input.workInProgress;
 
@@ -274,16 +264,14 @@ public class CreateChange
             || submitType.equals(SubmitType.MERGE_IF_NECESSARY))) {
           throw new BadRequestException("Submit type: " + submitType + " is not supported");
         }
-        c =
-            newMergeCommit(
-                git, oi, rw, rsrc.getProjectState(), mergeTip, input.merge, author, commitMessage);
+        c = newMergeCommit(git, oi, rw, projectState, mergeTip, input.merge, author, commitMessage);
       } else {
         // create an empty commit
         c = newCommit(oi, rw, author, mergeTip, commitMessage);
       }
 
       Change.Id changeId = new Change.Id(seq.nextChangeId());
-      ChangeInserter ins = changeInserterFactory.create(changeId, c, refName);
+      ChangeInserter ins = changeInserterFactory.create(changeId, c, input.branch);
       ins.setMessage(String.format("Uploaded patch set %s.", ins.getPatchSetId().get()));
       ins.setTopic(input.topic);
       ins.setPrivate(input.isPrivate);
@@ -321,6 +309,7 @@ public class CreateChange
     if (Strings.isNullOrEmpty(input.branch)) {
       throw new BadRequestException("branch must be non-empty");
     }
+    input.branch = RefNames.fullName(input.branch);
 
     String subject = Strings.nullToEmpty(input.subject);
     subject = subject.replaceAll("(?m)^#.*$\n?", "").trim();
@@ -352,6 +341,21 @@ public class CreateChange
     input.isPrivate = isPrivate;
   }
 
+  private void checkRequiredPermissions(Project.NameKey project, String refName)
+      throws ResourceNotFoundException, AuthException, PermissionBackendException {
+    try {
+      permissionBackend.currentUser().project(project).ref(refName).check(RefPermission.READ);
+    } catch (AuthException e) {
+      throw new ResourceNotFoundException(String.format("ref %s not found", refName));
+    }
+
+    permissionBackend
+        .currentUser()
+        .project(project)
+        .ref(refName)
+        .check(RefPermission.CREATE_CHANGE);
+  }
+
   private static RevCommit newCommit(
       ObjectInserter oi,
       RevWalk rw,

javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -2451,7 +2451,7 @@ public class ChangeIT extends AbstractDaemonTest {
     in.project = project.get();
     ChangeInfo info = gApi.changes().create(in).get();
     assertThat(info.project).isEqualTo(in.project);
-    assertThat(info.branch).isEqualTo(in.branch);
+    assertThat(RefNames.fullName(info.branch)).isEqualTo(RefNames.fullName(in.branch));
     assertThat(info.subject).isEqualTo(in.subject);
     assertThat(Iterables.getOnlyElement(info.messages).message).isEqualTo("Uploaded patch set 1.");
   }
@@ -2899,7 +2899,7 @@ public class ChangeIT extends AbstractDaemonTest {
     in.newBranch = true;
     ChangeInfo info = gApi.changes().create(in).get();
     assertThat(info.project).isEqualTo(in.project);
-    assertThat(info.branch).isEqualTo(in.branch);
+    assertThat(RefNames.fullName(info.branch)).isEqualTo(RefNames.fullName(in.branch));
     assertThat(info.subject).isEqualTo(in.subject);
     assertThat(Iterables.getOnlyElement(info.messages).message).isEqualTo("Uploaded patch set 1.");
   }

javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java

@@ -48,6 +48,7 @@ import com.google.gerrit.extensions.restapi.RestApiException;
 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.RefNames;
 import com.google.gerrit.server.submit.ChangeAlreadyMergedException;
 import com.google.gerrit.testing.FakeEmailSender.Message;
 import com.google.gerrit.testing.TestTimeUtil;
@@ -483,7 +484,7 @@ public class CreateChangeIT extends AbstractDaemonTest {
   private ChangeInfo assertCreateSucceeds(ChangeInput in) throws Exception {
     ChangeInfo out = gApi.changes().create(in).get();
     assertThat(out.project).isEqualTo(in.project);
-    assertThat(out.branch).isEqualTo(in.branch);
+    assertThat(RefNames.fullName(out.branch)).isEqualTo(RefNames.fullName(in.branch));
     assertThat(out.subject).isEqualTo(in.subject.split("\n")[0]);
     assertThat(out.topic).isEqualTo(in.topic);
     assertThat(out.status).isEqualTo(in.status);