opendev/gerrit
Code Issues Proposed changes

Revert "restapi: add an alternative CreateChange endpoint"

Browse Source 
This reverts commit 44c724e185.

Reason for revert: this did not fix our slowness problem, which
was slow CreateChange calls on Android's platform/frameworks/base
when specifying a merge. On closer inspection, generating the merge
reads too much data.

Change-Id: I9fb9a074bc7ae9da13ef262a5a9a7a2596ec0387
This commit is contained in:
Han-Wen Nienhuys
2019-10-30 17:15:39 +00:00
committed by David Pursehouse
parent e2be01cde7
commit fccb35cf19
6 changed files with 8 additions and 185 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
Documentation/rest-api-projects.txt
View File

@@ -1252,57 +1252,6 @@ entity is returned.
}
----
[[create-change]]
=== Create Change for review.
This endpoint is functionally equivalent to
link:rest-api-changes.html#create-change[create change in the change
API], but it has the project name in the URL, which is easier to route
in sharded deployments.
.Request
----
POST /projects/myProject/create.change HTTP/1.0
Content-Type: application/json; charset=UTF-8
{
"subject" : "Let's support 100% Gerrit workflow direct in browser",
"branch" : "master",
"topic" : "create-change-in-browser",
"status" : "NEW"
}
----
As response a link:#change-info[ChangeInfo] entity is returned that describes
the resulting change.
.Response
----
HTTP/1.1 201 OK
Content-Disposition: attachment
Content-Type: application/json; charset=UTF-8
)]}'
{
"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": "NEW",
"created": "2014-05-05 07:15:44.639000000",
"updated": "2014-05-05 07:15:44.639000000",
"mergeable": true,
"insertions": 0,
"deletions": 0,
"_number": 4711,
"owner": {
"name": "John Doe"
}
}
----
[[create-access-change]]
=== Create Access Rights Change for review.
--

24
java/com/google/gerrit/server/restapi/change/CreateChange.java
View File

@@ -170,28 +170,16 @@ public class CreateChange
BatchUpdate.Factory updateFactory, TopLevelResource parent, ChangeInput input)
throws IOException, InvalidChangeOperationException, RestApiException, UpdateException,
PermissionBackendException, ConfigInvalidException {
if (Strings.isNullOrEmpty(input.project)) {
throw new BadRequestException("project must be non-empty");
}
return execute(updateFactory, input, projectsCollection.parse(input.project));
}
/** Creates the changes in the given project. This is public for reuse in the project API. */
public Response<ChangeInfo> execute(
BatchUpdate.Factory updateFactory, ChangeInput input, ProjectResource projectResource)
throws IOException, RestApiException, UpdateException, PermissionBackendException,
ConfigInvalidException {
if (!user.get().isIdentifiedUser()) {
throw new AuthException("Authentication required");
}
ProjectState projectState = projectResource.getProjectState();
projectState.checkStatePermitsWrite();
IdentifiedUser me = user.get().asIdentifiedUser();
checkAndSanitizeChangeInput(input, me);
ProjectResource projectResource = projectsCollection.parse(input.project);
ProjectState projectState = projectResource.getProjectState();
projectState.checkStatePermitsWrite();
Project.NameKey project = projectResource.getNameKey();
contributorAgreements.check(project, user.get());
@@ -214,6 +202,10 @@ public class CreateChange
*/
private void checkAndSanitizeChangeInput(ChangeInput input, IdentifiedUser me)
throws RestApiException, PermissionBackendException, IOException {
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");
}

70
java/com/google/gerrit/server/restapi/project/CreateChange.java
View File

@@ -1,70 +0,0 @@
// Copyright (C) 2019 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.restapi.project;
import com.google.common.base.Strings;
import com.google.gerrit.exceptions.InvalidNameException;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInput;
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.RestApiException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView;
import com.google.gerrit.server.update.UpdateException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
public class CreateChange extends RetryingRestModifyView<ProjectResource, ChangeInput, ChangeInfo> {
private final com.google.gerrit.server.restapi.change.CreateChange changeCreateChange;
private final Provider<CurrentUser> user;
@Inject
public CreateChange(
RetryHelper retryHelper,
Provider<CurrentUser> user,
com.google.gerrit.server.restapi.change.CreateChange changeCreateChange) {
super(retryHelper);
this.changeCreateChange = changeCreateChange;
this.user = user;
}
@Override
public Response<ChangeInfo> applyImpl(
BatchUpdate.Factory updateFactory, ProjectResource rsrc, ChangeInput input)
throws PermissionBackendException, IOException, ConfigInvalidException,
InvalidChangeOperationException, InvalidNameException, UpdateException, RestApiException {
if (!user.get().isIdentifiedUser()) {
throw new AuthException("Authentication required");
}
if (!Strings.isNullOrEmpty(input.project)) {
throw new BadRequestException("may not specify project");
}
input.project = rsrc.getName();
return changeCreateChange.execute(updateFactory, input, rsrc);
}
}

1
java/com/google/gerrit/server/restapi/project/Module.java
View File

@@ -77,7 +77,6 @@ public class Module extends RestApiModule {
child(PROJECT_KIND, "branches").to(BranchesCollection.class);
create(BRANCH_KIND).to(CreateBranch.class);
post(PROJECT_KIND, "create.change").to(CreateChange.class);
put(BRANCH_KIND).to(PutBranch.class);
get(BRANCH_KIND).to(GetBranch.class);
delete(BRANCH_KIND).to(DeleteBranch.class);

1
javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java
View File

@@ -69,7 +69,6 @@ public class ProjectsRestApiBindingsIT extends AbstractDaemonTest {
RestCall.get("/projects/%s/statistics.git"),
RestCall.post("/projects/%s/index"),
RestCall.post("/projects/%s/gc"),
RestCall.post("/projects/%s/create.change"),
RestCall.get("/projects/%s/children"),
RestCall.get("/projects/%s/branches"),
RestCall.post("/projects/%s/branches:delete"),

46
javatests/com/google/gerrit/acceptance/rest/project/CreateChangeIT.java
View File

@@ -1,46 +0,0 @@
// Copyright (C) 2019 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.project;
import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.entities.RefNames.REFS_HEADS;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.common.ChangeInput;
import org.junit.Test;
public class CreateChangeIT extends AbstractDaemonTest {
// Just a basic test. The real functionality is tested under the restapi.change acceptance tests.
@Test
public void basic() throws Exception {
BranchInput branchInput = new BranchInput();
branchInput.ref = "foo";
assertThat(gApi.projects().name(project.get()).branches().get().stream().map(i -> i.ref))
.doesNotContain(REFS_HEADS + branchInput.ref);
RestResponse r =
adminRestSession.put(
"/projects/" + project.get() + "/branches/" + branchInput.ref, branchInput);
r.assertCreated();
ChangeInput input = new ChangeInput();
input.branch = "foo";
input.subject = "subject";
RestResponse cr = adminRestSession.post("/projects/" + project.get() + "/create.change", input);
cr.assertCreated();
}
}