opendev/gerrit
Code Issues Proposed changes

Add Java API for change edits and use it in tests

Browse Source 
For some reason, a Java API for change edits didn't exist previously
even though a detailed REST API has been available. Because of that,
tests needed to use internal classes when they interacted with change
edits. Using internal classes is fragile and impedes refactorings.
That's why we should avoid it.

If possible, the structure and behavior of the tests is kept. Some of
them might and should be improved but that's beyond the scope of this
change. Some of the tests are adapted a bit because the internal
classes allow change edits to be created for previous patch sets
which isn't possible when using the REST API (and hence the Java API).

As modifications to the mentioned internal classes are necessary to
properly implement the 'Apply fix' feature of robot comments, it is
crucial that none of the tests use the internal classes directly.
In addition, the tests which will be added for the 'Apply fix' feature
will also need to modify and query change edits, which will be much
easier with the Java API.

Change-Id: I6b455541d1bc1b7a05b5f0507911181b0451829a
This commit is contained in:
Alice Kober-Sotzek 2017-01-16 12:07:13 +01:00
parent 2a82c08023
commit 5dd17b647e
18 changed files with 1454 additions and 544 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@ -699,9 +699,12 @@ public abstract class AbstractDaemonTest {
return gApi.changes().id(id).get();
}
protected EditInfo getEdit(String id)
protected Optional<EditInfo> getEdit(String id)
throws RestApiException {
return gApi.changes().id(id).getEdit();
return gApi.changes()
.id(id)
.edit()
.get();
}
protected ChangeInfo get(String id, ListChangesOption... options)

41
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/MergeListIT.java
View File

@ -14,7 +14,6 @@
package com.google.gerrit.acceptance.api.change;
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.reviewdb.client.Patch.MERGE_LIST;
import static java.nio.charset.StandardCharsets.UTF_8;
@ -30,13 +29,8 @@ import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.DiffInfo;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.server.edit.ChangeEditModifier;
import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import org.eclipse.jgit.dircache.InvalidPathException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Before;
@ -56,12 +50,6 @@ public class MergeListIT extends AbstractDaemonTest {
private RevCommit parent2;
private RevCommit grandParent2;
@Inject
private ChangeEditModifier modifier;
@Inject
private ChangeEditUtil editUtil;
@Before
public void setup() throws Exception {
ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
@ -153,23 +141,32 @@ public class MergeListIT extends AbstractDaemonTest {
@Test
public void editMergeList() throws Exception {
ChangeData cd = getOnlyElement(queryProvider.get().byKeyPrefix(changeId));
modifier.createEdit(cd.change(), cd.currentPatchSet());
gApi.changes()
.id(changeId)
.edit()
.create();
exception.expect(InvalidPathException.class);
exception.expect(ResourceConflictException.class);
exception.expectMessage("Invalid path: " + MERGE_LIST);
modifier.modifyFile(editUtil.byChange(cd.change()).get(), MERGE_LIST,
RawInputUtil.create("new content"));
gApi.changes()
.id(changeId)
.edit()
.modifyFile(MERGE_LIST, RawInputUtil.create("new content"));
}
@Test
public void deleteMergeList() throws Exception {
ChangeData cd = getOnlyElement(queryProvider.get().byKeyPrefix(changeId));
modifier.createEdit(cd.change(), cd.currentPatchSet());
gApi.changes()
.id(changeId)
.edit()
.create();
exception.expect(InvalidChangeOperationException.class);
exception.expect(ResourceConflictException.class);
exception.expectMessage("no changes were made");
modifier.deleteFile(editUtil.byChange(cd.change()).get(), MERGE_LIST);
gApi.changes()
.id(changeId)
.edit()
.deleteFile(MERGE_LIST);
}
private String getMergeListContent(RevCommit... commits) {

948
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
View File

File diff suppressed because it is too large Load Diff

12
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
View File

@ -20,6 +20,7 @@ import static com.google.gerrit.acceptance.GitUtil.assertPushOk;
import static com.google.gerrit.acceptance.GitUtil.assertPushRejected;
import static com.google.gerrit.acceptance.GitUtil.pushHead;
import static com.google.gerrit.common.FooterConstants.CHANGE_ID;
import static com.google.gerrit.extensions.common.EditInfoSubject.assertThat;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.project.Util.category;
import static com.google.gerrit.server.project.Util.value;
@ -77,6 +78,7 @@ import java.util.EnumSet;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
@ -376,18 +378,20 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
public void pushForMasterAsEdit() throws Exception {
PushOneCommit.Result r = pushTo("refs/for/master");
r.assertOkStatus();
EditInfo edit = getEdit(r.getChangeId());
assertThat(edit).isNull();
Optional<EditInfo> edit = getEdit(r.getChangeId());
assertThat(edit).isAbsent();
// specify edit as option
r = amendChange(r.getChangeId(), "refs/for/master%edit");
r.assertOkStatus();
edit = getEdit(r.getChangeId());
assertThat(edit).isNotNull();
assertThat(edit).isPresent();
@SuppressWarnings("OptionalGetWithoutIsPresent")
EditInfo editInfo = edit.get();
r.assertMessage("Updated Changes:\n "
+ canonicalWebUrl.get()
+ r.getChange().getId()
+ " " + edit.commit.subject + " [EDIT]\n");
+ " " + editInfo.commit.subject + " [EDIT]\n");
}
@Test

23
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
View File

@ -36,7 +36,6 @@ import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.edit.ChangeEditModifier;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.ReceiveCommitsAdvertiseRefsHook;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
@ -68,9 +67,6 @@ import java.util.Map;
@NoHttpd
public class RefAdvertisementIT extends AbstractDaemonTest {
@Inject
private ChangeEditModifier editModifier;
@Inject
private ProjectControl.GenericFactory projectControlFactory;
@ -260,15 +256,21 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
deny(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
Change c = notesFactory.createChecked(db, project, c1.getId()).getChange();
PatchSet ps1 = getPatchSet(new PatchSet.Id(c1.getId(), 1));
String changeId = c.getKey().get();
// Admin's edit is not visible.
setApiUser(admin);
editModifier.createEdit(c, ps1);
gApi.changes()
.id(changeId)
.edit()
.create();
// User's edit is visible.
setApiUser(user);
editModifier.createEdit(c, ps1);
gApi.changes()
.id(changeId)
.edit()
.create();
assertUploadPackRefs(
"HEAD",
@ -288,9 +290,12 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
deny(Permission.READ, REGISTERED_USERS, "refs/heads/master");
allow(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
PatchSet ps1 = getPatchSet(new PatchSet.Id(c1.getId(), 1));
String changeId = c1.change().getKey().get();
setApiUser(admin);
editModifier.createEdit(c1.change(), ps1);
gApi.changes()
.id(changeId)
.edit()
.create();
setApiUser(user);
assertUploadPackRefs(

29
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
View File

@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.server.change;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.GitUtil.pushHead;
import static com.google.gerrit.extensions.common.EditInfoSubject.assertThat;
import static java.util.concurrent.TimeUnit.SECONDS;
import com.google.common.collect.ImmutableList;
@ -27,13 +28,12 @@ import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.common.RawInputUtil;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.GetRelated.ChangeAndCommit;
import com.google.gerrit.server.change.GetRelated.RelatedInfo;
import com.google.gerrit.server.edit.ChangeEditModifier;
import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
import com.google.gerrit.server.query.change.ChangeData;
@ -48,6 +48,7 @@ import org.junit.Before;
import org.junit.Test;
import java.util.List;
import java.util.Optional;
public class GetRelatedIT extends AbstractDaemonTest {
private String systemTimeZone;
@ -64,12 +65,6 @@ public class GetRelatedIT extends AbstractDaemonTest {
System.setProperty("user.timezone", systemTimeZone);
}
@Inject
private ChangeEditUtil editUtil;
@Inject
private ChangeEditModifier editModifier;
@Inject
private BatchUpdate.Factory updateFactory;
@ -579,11 +574,19 @@ public class GetRelatedIT extends AbstractDaemonTest {
pushHead(testRepo, "refs/for/master", false);
Change ch2 = getChange(c2_1).change();
editModifier.createEdit(ch2, getPatchSet(ch2.currentPatchSetId()));
editModifier.modifyFile(editUtil.byChange(ch2).get(), "a.txt",
RawInputUtil.create(new byte[] {'a'}));
ObjectId editRev =
ObjectId.fromString(editUtil.byChange(ch2).get().getRevision().get());
String changeId2 = ch2.getKey().get();
gApi.changes()
.id(changeId2)
.edit()
.create();
gApi.changes()
.id(changeId2)
.edit()
.modifyFile("a.txt", RawInputUtil.create(new byte[] {'a'}));
Optional<EditInfo> edit = getEdit(changeId2);
assertThat(edit).isPresent();
@SuppressWarnings("OptionalGetWithoutIsPresent")
ObjectId editRev = ObjectId.fromString(edit.get().commit.commit);
PatchSet.Id ps1_1 = getPatchSetId(c1_1);
PatchSet.Id ps2_1 = getPatchSetId(c2_1);

22
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
View File

@ -132,9 +132,24 @@ public interface ChangeApi {
ChangeInfo get() throws RestApiException;
/** {@code get} with {@link ListChangesOption} set to none. */
ChangeInfo info() throws RestApiException;
/** Retrieve change edit when exists. */
/**
* Retrieve change edit when exists.
*
* @deprecated Replaced by {@link ChangeApi#edit()} in combination with
* {@link ChangeEditApi#get()}.
*/
@Deprecated
EditInfo getEdit() throws RestApiException;
/**
* Provides access to an API regarding the change edit of this change.
*
* @return a {@code ChangeEditApi} for the change edit of this change
* @throws RestApiException if the API isn't accessible
*/
ChangeEditApi edit() throws RestApiException;
/**
* Set hashtags on a change
**/
@ -356,6 +371,11 @@ public interface ChangeApi {
throw new NotImplementedException();
}
@Override
public ChangeEditApi edit() {
throw new NotImplementedException();
}
@Override
public void setHashtags(HashtagsInput input) {
throw new NotImplementedException();

237
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java Normal file
View File

@ -0,0 +1,237 @@
// Copyright (C) 2017 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.extensions.api.changes;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.NotImplementedException;
import com.google.gerrit.extensions.restapi.RawInput;
import com.google.gerrit.extensions.restapi.RestApiException;
import java.util.Optional;
/**
* An API for the change edit of a change. A change edit is similar to a patch
* set and will become one if it is published
* (by {@link #publish(PublishChangeEditInput)}). Whenever the descriptions
* below refer to files of a change edit, they actually refer to the files of
* the Git tree which is represented by the change edit. A change can have at
* most one change edit at each point in time.
*/
public interface ChangeEditApi {
/**
* Retrieves details regarding the change edit.
*
* @return an {@code Optional} containing details about the change edit if it
* exists, or {@code Optional.empty()}
* @throws RestApiException if the change edit couldn't be retrieved
*/
Optional<EditInfo> get() throws RestApiException;
/**
* Creates a new change edit. It has exactly the same Git tree as the current
* patch set of the change.
*
* @throws RestApiException if the change edit couldn't be created or a change
* edit already exists
*/
void create() throws RestApiException;
/**
* Deletes the change edit.
*
* @throws RestApiException if the change edit couldn't be deleted or a change
* edit wasn't present
*/
void delete() throws RestApiException;
/**
* Rebases the change edit on top of the latest patch set of this change.
*
* @throws RestApiException if the change edit couldn't be rebased or a change
* edit wasn't present
*/
void rebase() throws RestApiException;
/**
* Publishes the change edit using default settings. See
* {@link #publish(PublishChangeEditInput)} for more details.
*
* @throws RestApiException if the change edit couldn't be published or a
* change edit wasn't present
*/
void publish() throws RestApiException;
/**
* Publishes the change edit. Publishing means that the change edit is turned
* into a regular patch set of the change.
*
* @param publishChangeEditInput a {@code PublishChangeEditInput} specifying
* the options which should be applied
* @throws RestApiException if the change edit couldn't be published or a
* change edit wasn't present
*/
void publish(PublishChangeEditInput publishChangeEditInput)
throws RestApiException;
/**
* Retrieves the contents of the specified file from the change edit.
*
* @param filePath the path of the file
* @return an {@code Optional} containing the contents of the file as a
* {@code BinaryResult} if the file exists within the change edit, or
* {@code Optional.empty()}
* @throws RestApiException if the contents of the file couldn't be retrieved
* or a change edit wasn't present
*/
Optional<BinaryResult> getFile(String filePath) throws RestApiException;
/**
* Renames a file of the change edit or moves the file to another directory.
* If the change edit doesn't exist, it will be created based on the current
* patch set of the change.
*
* @param oldFilePath the current file path
* @param newFilePath the desired file path
* @throws RestApiException if the file couldn't be renamed
*/
void renameFile(String oldFilePath, String newFilePath)
throws RestApiException;
/**
* Restores a file of the change edit to the state in which it was before the
* patch set on which the change edit is based. This includes the file content
* as well as the existence or non-existence of the file. If the change edit
* doesn't exist, it will be created based on the current patch set of the
* change.
*
* @param filePath the path of the file
* @throws RestApiException if the file couldn't be restored to its previous
* state
*/
void restoreFile(String filePath) throws RestApiException;
/**
* Modify the contents of the specified file of the change edit. If no content
* is provided, the content of the file is erased but the file isn't deleted.
* If the change edit doesn't exist, it will be created based on the current
* patch set of the change.
*
* @param filePath the path of the file which should be modified
* @param newContent the desired content of the file
* @throws RestApiException if the content of the file couldn't be modified
*/
void modifyFile(String filePath, RawInput newContent) throws RestApiException;
/**
* Deletes the specified file from the change edit. If the change edit doesn't
* exist, it will be created based on the current patch set of the change.
*
* @param filePath the path fo the file which should be deleted
* @throws RestApiException if the file couldn't be deleted
*/
void deleteFile(String filePath) throws RestApiException;
/**
* Retrieves the commit message of the change edit.
*
* @return the commit message of the change edit
* @throws RestApiException if the commit message couldn't be retrieved or a
* change edit wasn't present
*/
String getCommitMessage() throws RestApiException;
/**
* Modifies the commit message of the change edit. If the change edit doesn't
* exist, it will be created based on the current patch set of the change.
*
* @param newCommitMessage the desired commit message
* @throws RestApiException if the commit message couldn't be modified
*/
void modifyCommitMessage(String newCommitMessage) throws RestApiException;
/**
* A default implementation which allows source compatibility
* when adding new methods to the interface.
**/
class NotImplemented implements ChangeEditApi {
@Override
public Optional<EditInfo> get() {
throw new NotImplementedException();
}
@Override
public void create() {
throw new NotImplementedException();
}
@Override
public void delete() {
throw new NotImplementedException();
}
@Override
public void rebase() {
throw new NotImplementedException();
}
@Override
public void publish() {
throw new NotImplementedException();
}
@Override
public void publish(PublishChangeEditInput publishChangeEditInput) {
throw new NotImplementedException();
}
@Override
public Optional<BinaryResult> getFile(String filePath) {
throw new NotImplementedException();
}
@Override
public void renameFile(String oldFilePath, String newFilePath) {
throw new NotImplementedException();
}
@Override
public void restoreFile(String filePath) {
throw new NotImplementedException();
}
@Override
public void modifyFile(String filePath, RawInput newContent) {
throw new NotImplementedException();
}
@Override
public void deleteFile(String filePath) {
throw new NotImplementedException();
}
@Override
public String getCommitMessage() {
throw new NotImplementedException();
}
@Override
public void modifyCommitMessage(String newCommitMessage) {
throw new NotImplementedException();
}
}
}

20
gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
View File

@ -18,6 +18,7 @@ import com.google.gerrit.extensions.api.changes.AbandonInput;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AssigneeInput;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.ChangeEditApi;
import com.google.gerrit.extensions.api.changes.Changes;
import com.google.gerrit.extensions.api.changes.FixInput;
import com.google.gerrit.extensions.api.changes.HashtagsInput;
@ -40,7 +41,6 @@ import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.change.Abandon;
import com.google.gerrit.server.change.ChangeEdits;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.Check;
@ -113,9 +113,9 @@ class ChangeApiImpl implements ChangeApi {
private final ListChangeComments listComments;
private final ListChangeRobotComments listChangeRobotComments;
private final ListChangeDrafts listDrafts;
private final ChangeEditApiImpl.Factory changeEditApi;
private final Check check;
private final Index index;
private final ChangeEdits.Detail editDetail;
private final Move move;
@Inject
@ -145,9 +145,9 @@ class ChangeApiImpl implements ChangeApi {
ListChangeComments listComments,
ListChangeRobotComments listChangeRobotComments,
ListChangeDrafts listDrafts,
ChangeEditApiImpl.Factory changeEditApi,
Check check,
Index index,
ChangeEdits.Detail editDetail,
Move move,
@Assisted ChangeResource change) {
this.changeApi = changeApi;
@ -176,9 +176,9 @@ class ChangeApiImpl implements ChangeApi {
this.listComments = listComments;
this.listChangeRobotComments = listChangeRobotComments;
this.listDrafts = listDrafts;
this.changeEditApi = changeEditApi;
this.check = check;
this.index = index;
this.editDetail = editDetail;
this.move = move;
this.change = change;
}
@ -409,12 +409,12 @@ class ChangeApiImpl implements ChangeApi {
@Override
public EditInfo getEdit() throws RestApiException {
try {
Response<EditInfo> edit = editDetail.apply(change);
return edit.isNone() ? null : edit.value();
} catch (IOException | OrmException e) {
throw new RestApiException("Cannot retrieve change edit", e);
}
return edit().get().orElse(null);
}
@Override
public ChangeEditApi edit() throws RestApiException {
return changeEditApi.create(change);
}
@Override

246
gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java Normal file
View File

@ -0,0 +1,246 @@
// Copyright (C) 2017 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.api.changes;
import com.google.gerrit.extensions.api.changes.ChangeEditApi;
import com.google.gerrit.extensions.api.changes.PublishChangeEditInput;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RawInput;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.change.ChangeEditResource;
import com.google.gerrit.server.change.ChangeEdits;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.DeleteChangeEdit;
import com.google.gerrit.server.change.PublishChangeEdit;
import com.google.gerrit.server.change.RebaseChangeEdit;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
import java.util.Optional;
public class ChangeEditApiImpl implements ChangeEditApi {
interface Factory {
ChangeEditApiImpl create(ChangeResource changeResource);
}
private final ChangeEdits.Detail editDetail;
private final ChangeEdits.Post changeEditsPost;
private final DeleteChangeEdit deleteChangeEdit;
private final RebaseChangeEdit.Rebase rebaseChangeEdit;
private final PublishChangeEdit.Publish publishChangeEdit;
private final ChangeEdits.Get changeEditsGet;
private final ChangeEdits.Put changeEditsPut;
private final ChangeEdits.DeleteContent changeEditDeleteContent;
private final ChangeEdits.GetMessage getChangeEditCommitMessage;
private final ChangeEdits.EditMessage modifyChangeEditCommitMessage;
private final ChangeEdits changeEdits;
private final ChangeResource changeResource;
@Inject
public ChangeEditApiImpl(ChangeEdits.Detail editDetail,
ChangeEdits.Post changeEditsPost,
DeleteChangeEdit deleteChangeEdit,
RebaseChangeEdit.Rebase rebaseChangeEdit,
PublishChangeEdit.Publish publishChangeEdit,
ChangeEdits.Get changeEditsGet,
ChangeEdits.Put changeEditsPut,
ChangeEdits.DeleteContent changeEditDeleteContent,
ChangeEdits.GetMessage getChangeEditCommitMessage,
ChangeEdits.EditMessage modifyChangeEditCommitMessage,
ChangeEdits changeEdits,
@Assisted ChangeResource changeResource) {
this.editDetail = editDetail;
this.changeEditsPost = changeEditsPost;
this.deleteChangeEdit = deleteChangeEdit;
this.rebaseChangeEdit = rebaseChangeEdit;
this.publishChangeEdit = publishChangeEdit;
this.changeEditsGet = changeEditsGet;
this.changeEditsPut = changeEditsPut;
this.changeEditDeleteContent = changeEditDeleteContent;
this.getChangeEditCommitMessage = getChangeEditCommitMessage;
this.modifyChangeEditCommitMessage = modifyChangeEditCommitMessage;
this.changeEdits = changeEdits;
this.changeResource = changeResource;
}
@Override
public Optional<EditInfo> get() throws RestApiException {
try {
Response<EditInfo> edit = editDetail.apply(changeResource);
return edit.isNone() ? Optional.empty() : Optional.of(edit.value());
} catch (IOException | OrmException e) {
throw new RestApiException("Cannot retrieve change edit", e);
}
}
@Override
public void create() throws RestApiException {
try {
changeEditsPost.apply(changeResource, null);
} catch (InvalidChangeOperationException | IOException | OrmException e) {
throw new RestApiException("Cannot create change edit", e);
}
}
@Override
public void delete() throws RestApiException {
try {
deleteChangeEdit.apply(changeResource, new DeleteChangeEdit.Input());
} catch (IOException | OrmException e) {
throw new RestApiException("Cannot delete change edit", e);
}
}
@Override
public void rebase() throws RestApiException {
try {
rebaseChangeEdit.apply(changeResource, null);
} catch (IOException | InvalidChangeOperationException | OrmException e) {
throw new RestApiException("Cannot rebase change edit", e);
}
}
@Override
public void publish() throws RestApiException {
publish(null);
}
@Override
public void publish(PublishChangeEditInput publishChangeEditInput)
throws RestApiException {
try {
publishChangeEdit.apply(changeResource, publishChangeEditInput);
} catch (IOException | OrmException | UpdateException e) {
throw new RestApiException("Cannot publish change edit", e);
}
}
@Override
public Optional<BinaryResult> getFile(String filePath)
throws RestApiException {
try {
ChangeEditResource changeEditResource = getChangeEditResource(filePath);
Response<BinaryResult> fileResponse =
changeEditsGet.apply(changeEditResource);
return fileResponse.isNone()
? Optional.empty()
: Optional.of(fileResponse.value());
} catch (IOException | InvalidChangeOperationException | OrmException e) {
throw new RestApiException("Cannot retrieve file of change edit", e);
}
}
@Override
public void renameFile(String oldFilePath, String newFilePath)
throws RestApiException {
try {
ChangeEdits.Post.Input renameInput = new ChangeEdits.Post.Input();
renameInput.oldPath = oldFilePath;
renameInput.newPath = newFilePath;
changeEditsPost.apply(changeResource, renameInput);
} catch (InvalidChangeOperationException | IOException | OrmException e) {
throw new RestApiException("Cannot rename file of change edit", e);
}
}
@Override
public void restoreFile(String filePath) throws RestApiException {
try {
ChangeEdits.Post.Input restoreInput = new ChangeEdits.Post.Input();
restoreInput.restorePath = filePath;
changeEditsPost.apply(changeResource, restoreInput);
} catch (InvalidChangeOperationException | IOException | OrmException e) {
throw new RestApiException("Cannot restore file of change edit", e);
}
}
@Override
public void modifyFile(String filePath, RawInput newContent)
throws RestApiException {
try {
ChangeEditResource changeEditResource =
getOrCreateChangeEditResource(filePath);
ChangeEdits.Put.Input input = new ChangeEdits.Put.Input();
input.content = newContent;
changeEditsPut.apply(changeEditResource, input);
} catch (IOException | InvalidChangeOperationException | OrmException e) {
throw new RestApiException("Cannot modify file of change edit", e);
}
}
@Override
public void deleteFile(String filePath) throws RestApiException {
try {
ChangeEditResource changeEditResource =
getOrCreateChangeEditResource(filePath);
changeEditDeleteContent.apply(changeEditResource, null);
} catch (IOException | InvalidChangeOperationException | OrmException e) {
throw new RestApiException("Cannot delete file of change edit", e);
}
}
@Override
public String getCommitMessage() throws RestApiException {
try {
try (BinaryResult binaryResult =
getChangeEditCommitMessage.apply(changeResource)) {
return binaryResult.asString();
}
} catch (IOException | OrmException e) {
throw new RestApiException("Cannot get commit message of change edit", e);
}
}
@Override
public void modifyCommitMessage(String newCommitMessage)
throws RestApiException {
ChangeEdits.EditMessage.Input input = new ChangeEdits.EditMessage.Input();
input.message = newCommitMessage;
try {
modifyChangeEditCommitMessage.apply(changeResource, input);
} catch (IOException | InvalidChangeOperationException | OrmException e) {
throw new RestApiException("Cannot modify commit message of change edit",
e);
}
}
private ChangeEditResource getOrCreateChangeEditResource(String filePath)
throws InvalidChangeOperationException, RestApiException, OrmException,
IOException {
// For some cases, the REST API automatically creates a change edit resource
// if it doesn't exist. We mimic this behavior here.
try {
return getChangeEditResource(filePath);
} catch (ResourceNotFoundException e) {
create();
return getChangeEditResource(filePath);
}
}
private ChangeEditResource getChangeEditResource(String filePath)
throws ResourceNotFoundException, AuthException, IOException,
InvalidChangeOperationException, OrmException {
return changeEdits.parse(changeResource, IdString.fromDecoded(filePath));
}
}

1
gerrit-server/src/main/java/com/google/gerrit/server/api/changes/Module.java
View File

@ -30,5 +30,6 @@ public class Module extends FactoryModule {
factory(FileApiImpl.Factory.class);
factory(ReviewerApiImpl.Factory.class);
factory(RevisionReviewerApiImpl.Factory.class);
factory(ChangeEditApiImpl.Factory.class);
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java
View File

@ -458,7 +458,7 @@ public class ChangeEdits implements
}
@Override
public Response<?> apply(ChangeEditResource rsrc)
public Response<BinaryResult> apply(ChangeEditResource rsrc)
throws IOException {
try {
ChangeEdit edit = rsrc.getChangeEdit();

45
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@ -69,7 +69,6 @@ import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeTriplet;
import com.google.gerrit.server.change.PatchSetInserter;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.edit.ChangeEditModifier;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.index.IndexConfig;
@ -138,7 +137,6 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Inject protected ChangeQueryBuilder queryBuilder;
@Inject protected GerritApi gApi;
@Inject protected IdentifiedUser.GenericFactory userFactory;
@Inject protected ChangeEditModifier changeEditModifier;
@Inject protected ChangeIndexCollection indexes;
@Inject protected ChangeIndexer indexer;
@Inject protected IndexConfig indexConfig;
@ -1581,25 +1579,27 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
Account.Id user2 = createAccount("user2");
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(repo, newChange(repo));
ChangeNotes notes1 =
notesFactory.create(db, change1.getProject(), change1.getId());
PatchSet ps1 = psUtil.get(db, notes1, change1.currentPatchSetId());
String changeId1 = change1.getKey().get();
Change change2 = insert(repo, newChange(repo));
ChangeNotes notes2 =
notesFactory.create(db, change2.getProject(), change2.getId());
PatchSet ps2 = psUtil.get(db, notes2, change2.currentPatchSetId());
String changeId2 = change2.getKey().get();
requestContext.setContext(newRequestContext(user1));
assertQuery("has:edit");
assertThat(changeEditModifier.createEdit(change1, ps1))
.isEqualTo(RefUpdate.Result.NEW);
assertThat(changeEditModifier.createEdit(change2, ps2))
.isEqualTo(RefUpdate.Result.NEW);
gApi.changes()
.id(changeId1)
.edit()
.create();
gApi.changes()
.id(changeId2)
.edit()
.create();
requestContext.setContext(newRequestContext(user2));
assertQuery("has:edit");
assertThat(changeEditModifier.createEdit(change2, ps2))
.isEqualTo(RefUpdate.Result.NEW);
gApi.changes()
.id(changeId2)
.edit()
.create();
requestContext.setContext(newRequestContext(user1));
assertQuery("has:edit", change2, change1);
@ -1698,13 +1698,16 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
Project.NameKey project = new Project.NameKey("repo");
TestRepository<Repo> repo = createProject(project.get());
Change change = insert(repo, newChange(repo));
String changeId = change.getKey().get();
ChangeNotes notes =
notesFactory.create(db, change.getProject(), change.getId());
PatchSet ps = psUtil.get(db, notes, change.currentPatchSetId());
requestContext.setContext(newRequestContext(user));
assertThat(changeEditModifier.createEdit(change, ps))
.isEqualTo(RefUpdate.Result.NEW);
gApi.changes()
.id(changeId)
.edit()
.create();
assertQuery("has:edit", change);
assertThat(indexer.reindexIfStale(project, change.getId()).get()).isFalse();
@ -1734,17 +1737,17 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
Change change = insert(repo, newChangeForCommit(repo, commit));
Change.Id id = change.getId();
int c = id.get();
ChangeNotes notes =
notesFactory.create(db, change.getProject(), change.getId());
PatchSet ps = psUtil.get(db, notes, change.currentPatchSetId());
String changeId = change.getKey().get();
requestContext.setContext(newRequestContext(user));
// Ensure one of each type of supported ref is present for the change. If
// any more refs are added, update this test to reflect them.
// Edit
assertThat(changeEditModifier.createEdit(change, ps))
.isEqualTo(RefUpdate.Result.NEW);
gApi.changes()
.id(changeId)
.edit()
.create();
// Star
gApi.accounts()

66
gerrit-test-util/src/main/java/com/google/gerrit/extensions/common/CommitInfoSubject.java Normal file
View File

@ -0,0 +1,66 @@
// Copyright (C) 2017 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.extensions.common;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.FailureStrategy;
import com.google.common.truth.StringSubject;
import com.google.common.truth.Subject;
import com.google.common.truth.SubjectFactory;
import com.google.common.truth.Truth;
import com.google.gerrit.truth.ListSubject;
public class CommitInfoSubject extends Subject<CommitInfoSubject, CommitInfo> {
private static final SubjectFactory<CommitInfoSubject, CommitInfo>
COMMIT_INFO_SUBJECT_FACTORY =
new SubjectFactory<CommitInfoSubject, CommitInfo>() {
@Override
public CommitInfoSubject getSubject(FailureStrategy failureStrategy,
CommitInfo commitInfo) {
return new CommitInfoSubject(failureStrategy, commitInfo);
}
};
public static CommitInfoSubject assertThat(CommitInfo commitInfo) {
return assertAbout(COMMIT_INFO_SUBJECT_FACTORY)
.that(commitInfo);
}
private CommitInfoSubject(FailureStrategy failureStrategy,
CommitInfo commitInfo) {
super(failureStrategy, commitInfo);
}
public StringSubject commit() {
isNotNull();
CommitInfo commitInfo = actual();
return Truth.assertThat(commitInfo.commit).named("commit");
}
public ListSubject<CommitInfoSubject, CommitInfo> parents() {
isNotNull();
CommitInfo commitInfo = actual();
return ListSubject.assertThat(commitInfo.parents,
CommitInfoSubject::assertThat).named("parents");
}
public GitPersonSubject committer() {
isNotNull();
CommitInfo commitInfo = actual();
return GitPersonSubject.assertThat(commitInfo.committer).named("committer");
}
}

67
gerrit-test-util/src/main/java/com/google/gerrit/extensions/common/EditInfoSubject.java Normal file
View File

@ -0,0 +1,67 @@
// Copyright (C) 2017 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.extensions.common;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.FailureStrategy;
import com.google.common.truth.StringSubject;
import com.google.common.truth.Subject;
import com.google.common.truth.SubjectFactory;
import com.google.common.truth.Truth;
import com.google.gerrit.truth.OptionalSubject;
import java.util.Optional;
public class EditInfoSubject extends Subject<EditInfoSubject, EditInfo> {
private static final SubjectFactory<EditInfoSubject, EditInfo>
EDIT_INFO_SUBJECT_FACTORY =
new SubjectFactory<EditInfoSubject, EditInfo>() {
@Override
public EditInfoSubject getSubject(FailureStrategy failureStrategy,
EditInfo editInfo) {
return new EditInfoSubject(failureStrategy, editInfo);
}
};
public static EditInfoSubject assertThat(EditInfo editInfo) {
return assertAbout(EDIT_INFO_SUBJECT_FACTORY)
.that(editInfo);
}
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
public static OptionalSubject<EditInfoSubject, EditInfo> assertThat(
Optional<EditInfo> editInfoOptional) {
return OptionalSubject.assertThat(editInfoOptional,
EditInfoSubject::assertThat);
}
private EditInfoSubject(FailureStrategy failureStrategy, EditInfo editInfo) {
super(failureStrategy, editInfo);
}
public CommitInfoSubject commit() {
isNotNull();
EditInfo editInfo = actual();
return CommitInfoSubject.assertThat(editInfo.commit).named("commit");
}
public StringSubject baseRevision() {
isNotNull();
EditInfo editInfo = actual();
return Truth.assertThat(editInfo.baseRevision).named("baseRevision");
}
}

54
gerrit-test-util/src/main/java/com/google/gerrit/extensions/common/GitPersonSubject.java Normal file
View File

@ -0,0 +1,54 @@
// Copyright (C) 2017 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.extensions.common;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.ComparableSubject;
import com.google.common.truth.FailureStrategy;
import com.google.common.truth.Subject;
import com.google.common.truth.SubjectFactory;
import com.google.common.truth.Truth;
import java.sql.Timestamp;
public class GitPersonSubject extends Subject<GitPersonSubject, GitPerson> {
private static final SubjectFactory<GitPersonSubject, GitPerson>
GIT_PERSON_SUBJECT_FACTORY =
new SubjectFactory<GitPersonSubject, GitPerson>() {
@Override
public GitPersonSubject getSubject(FailureStrategy failureStrategy,
GitPerson gitPerson) {
return new GitPersonSubject(failureStrategy, gitPerson);
}
};
public static GitPersonSubject assertThat(GitPerson gitPerson) {
return assertAbout(GIT_PERSON_SUBJECT_FACTORY)
.that(gitPerson);
}
private GitPersonSubject(FailureStrategy failureStrategy,
GitPerson gitPerson) {
super(failureStrategy, gitPerson);
}
public ComparableSubject<?, Timestamp> creationDate() {
isNotNull();
GitPerson gitPerson = actual();
return Truth.assertThat(gitPerson.date).named("creationDate");
}
}

73
gerrit-test-util/src/main/java/com/google/gerrit/extensions/restapi/BinaryResultSubject.java Normal file
View File

@ -0,0 +1,73 @@
// Copyright (C) 2017 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.extensions.restapi;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.FailureStrategy;
import com.google.common.truth.PrimitiveByteArraySubject;
import com.google.common.truth.Subject;
import com.google.common.truth.SubjectFactory;
import com.google.common.truth.Truth;
import com.google.gerrit.truth.OptionalSubject;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.Optional;
public class BinaryResultSubject
extends Subject<BinaryResultSubject, BinaryResult> {
private static final SubjectFactory<BinaryResultSubject, BinaryResult>
BINARY_RESULT_SUBJECT_FACTORY =
new SubjectFactory<BinaryResultSubject, BinaryResult>() {
@Override
public BinaryResultSubject getSubject(FailureStrategy failureStrategy,
BinaryResult binaryResult) {
return new BinaryResultSubject(failureStrategy,
binaryResult);
}
};
public static BinaryResultSubject assertThat(BinaryResult binaryResult) {
return assertAbout(BINARY_RESULT_SUBJECT_FACTORY)
.that(binaryResult);
}
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
public static OptionalSubject<BinaryResultSubject, BinaryResult> assertThat(
Optional<BinaryResult> binaryResultOptional) {
return OptionalSubject.assertThat(binaryResultOptional,
BinaryResultSubject::assertThat);
}
private BinaryResultSubject(FailureStrategy failureStrategy,
BinaryResult binaryResult) {
super(failureStrategy, binaryResult);
}
public PrimitiveByteArraySubject bytes() throws IOException {
isNotNull();
// We shouldn't close the BinaryResult within this method as it might still
// be used afterwards. Besides, closing it doesn't have an effect for most
// implementations of a BinaryResult.
@SuppressWarnings("resource")
BinaryResult binaryResult = actual();
ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
binaryResult.writeTo(byteArrayOutputStream);
byte[] bytes = byteArrayOutputStream.toByteArray();
return Truth.assertThat(bytes);
}
}

105
gerrit-test-util/src/main/java/com/google/gerrit/truth/OptionalSubject.java Normal file
View File

@ -0,0 +1,105 @@
// Copyright (C) 2017 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.truth;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.DefaultSubject;
import com.google.common.truth.FailureStrategy;
import com.google.common.truth.Subject;
import com.google.common.truth.SubjectFactory;
import com.google.common.truth.Truth;
import java.util.Optional;
import java.util.function.Function;
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
public class OptionalSubject<S extends Subject<S, ? super T>, T>
extends Subject<OptionalSubject<S, T>, Optional<T>> {
private final Function<? super T, ? extends S> valueAssertThatFunction;
public static <S extends Subject<S, ? super T>, T> OptionalSubject<S, T>
assertThat(Optional<T> optional,
Function<? super T, ? extends S> elementAssertThatFunction) {
OptionalSubjectFactory<S, T> optionalSubjectFactory =
new OptionalSubjectFactory<>(elementAssertThatFunction);
return assertAbout(optionalSubjectFactory).that(optional);
}
public static OptionalSubject<DefaultSubject, ?> assertThat(
Optional<?> optional) {
// Unfortunately, we need to cast to DefaultSubject as Truth.assertThat()
// only returns Subject<DefaultSubject, Object>. There shouldn't be a way
// for that method not to return a DefaultSubject because the generic type
// definitions of a Subject are quite strict.
Function<Object, DefaultSubject> valueAssertThatFunction =
value -> (DefaultSubject) Truth.assertThat(value);
return assertThat(optional, valueAssertThatFunction);
}
private OptionalSubject(FailureStrategy failureStrategy, Optional<T> optional,
Function<? super T, ? extends S> valueAssertThatFunction) {
super(failureStrategy, optional);
this.valueAssertThatFunction = valueAssertThatFunction;
}
public void isPresent() {
isNotNull();
Optional<T> optional = actual();
if (!optional.isPresent()) {
fail("has a value");
}
}
public void isAbsent() {
isNotNull();
Optional<T> optional = actual();
if (optional.isPresent()) {
fail("does not have a value");
}
}
public void isEmpty() {
isAbsent();
}
@SuppressWarnings("OptionalGetWithoutIsPresent")
public S value() {
isNotNull();
isPresent();
Optional<T> optional = actual();
return valueAssertThatFunction.apply(optional.get());
}
private static class OptionalSubjectFactory<S extends Subject<S, ? super T>,
T> extends SubjectFactory<OptionalSubject<S, T>, Optional<T>> {
private Function<? super T, ? extends S> valueAssertThatFunction;
OptionalSubjectFactory(
Function<? super T, ? extends S> valueAssertThatFunction) {
this.valueAssertThatFunction = valueAssertThatFunction;
}
@Override
public OptionalSubject<S, T> getSubject(FailureStrategy failureStrategy,
Optional<T> optional) {
return new OptionalSubject<>(failureStrategy, optional,
valueAssertThatFunction);
}
}
}