Merge changes I69cf3fc4,Ib6e9e942

* changes:
  Add support for suggested fixes of robot comments
  Split long method into smaller ones
This commit is contained in:
ekempin 2016-12-07 15:01:27 +00:00 committed by Gerrit Code Review
commit 6b1cd81f90
19 changed files with 944 additions and 33 deletions

View File

@ -4743,6 +4743,10 @@ The name of the label.
=== \{file-id\}
The path of the file.
[[fix-id]]
=== \{fix-id\}
UUID of a suggested fix.
[[revision-id]]
=== \{revision-id\}
Identifier that uniquely identifies one revision of a change.
@ -5445,6 +5449,36 @@ merged into the destination branch as this exact SHA-1. If not, insert
a new patch set referring to this commit.
|==========================
[[fix-suggestion-info]]
=== FixSuggestionInfo
The `FixSuggestionInfo` entity represents a suggested fix.
[options="header",cols="1,^1,5"]
|==========================
|Field Name ||Description
|`fix_id` |generated, don't set|The <<fix-id,UUID>> of the suggested
fix. It will be generated automatically and hence will be ignored if it's set
for input objects.
|`description` ||A description of the suggested fix.
|`replacements` ||A list of <<fix-replacement-info,FixReplacementInfo>>
entities indicating how the content of the file on which the comment was placed
should be modified. They should refer to non-overlapping regions.
|==========================
[[fix-replacement-info]]
=== FixReplacementInfo
The `FixReplacementInfo` entity describes how the content of a file (which is
further specified by the surrounding context) should be replaced by another
content.
[options="header",cols="1,6"]
|==========================
|Field Name |Description
|`range` |A <<comment-range,CommentRange>> indicating which content
of the file should be replaced.
|`replacement` |The content which should be used instead of the current one.
|==========================
[[git-person-info]]
=== GitPersonInfo
The `GitPersonInfo` entity contains information about the
@ -5967,12 +6001,14 @@ In addition `RobotCommentInfo` has the following fields:
[options="header",cols="1,^1,5"]
|===========================
|Field Name ||Description
|`robot_id` ||The ID of the robot that generated this comment.
|`robot_run_id` ||An ID of the run of the robot.
|`url` |optional|URL to more information.
|`properties` |optional|
Robot specific properties as map that maps arbitrary keys to values.
|Field Name ||Description
|`robot_id` ||The ID of the robot that generated this comment.
|`robot_run_id` ||An ID of the run of the robot.
|`url` |optional|URL to more information.
|`properties` |optional|Robot specific properties as map that maps arbitrary
keys to values.
|`fix_suggestions`|optional|Suggested fixes for this robot comment as a list of
<<fix-suggestion-info,FixSuggestionInfo>> entities.
|===========================
[[robot-comment-input]]

View File

@ -0,0 +1,93 @@
// Copyright (C) 2016 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;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.FailureStrategy;
import com.google.common.truth.IterableSubject;
import com.google.common.truth.Subject;
import com.google.common.truth.SubjectFactory;
import java.util.List;
import java.util.function.Function;
public class ListSubject<S extends Subject<S, E>, E> extends IterableSubject {
private final Function<E, S> elementAssertThatFunction;
@SuppressWarnings("unchecked")
public static <S extends Subject<S, E>, E> ListSubject<S, E> assertThat(
List<E> list, Function<E, S> elementAssertThatFunction) {
// The ListSubjectFactory always returns ListSubjects.
// -> Casting is appropriate.
return (ListSubject<S, E>) assertAbout(
new ListSubjectFactory<>(elementAssertThatFunction)).that(list);
}
private ListSubject(FailureStrategy failureStrategy, List<E> list,
Function<E, S> elementAssertThatFunction) {
super(failureStrategy, list);
this.elementAssertThatFunction = elementAssertThatFunction;
}
public S element(int index) {
checkArgument(index >= 0, "index(%s) must be >= 0", index);
// The constructor only accepts lists.
// -> Casting is appropriate.
@SuppressWarnings("unchecked")
List<E> list = (List<E>) actual();
isNotNull();
if (index >= list.size()) {
fail("has an element at index " + index);
}
return elementAssertThatFunction.apply(list.get(index));
}
public S onlyElement() {
isNotNull();
hasSize(1);
return element(0);
}
@SuppressWarnings("unchecked")
@Override
public ListSubject<S, E> named(String s, Object... objects) {
// This object is returned which is of type ListSubject.
// -> Casting is appropriate.
return (ListSubject<S, E>) super.named(s, objects);
}
private static class ListSubjectFactory<S extends Subject<S, T>, T>
extends SubjectFactory<IterableSubject, Iterable<?>> {
private Function<T, S> elementAssertThatFunction;
ListSubjectFactory(Function<T, S> elementAssertThatFunction) {
this.elementAssertThatFunction = elementAssertThatFunction;
}
@SuppressWarnings("unchecked")
@Override
public ListSubject<S, T> getSubject(FailureStrategy failureStrategy,
Iterable<?> objects) {
// The constructor of ListSubject only accepts lists.
// -> Casting is appropriate.
return new ListSubject<>(failureStrategy, (List<T>) objects,
elementAssertThatFunction);
}
}
}

View File

@ -0,0 +1,62 @@
// Copyright (C) 2016 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;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.FailureStrategy;
import com.google.common.truth.IntegerSubject;
import com.google.common.truth.Subject;
import com.google.common.truth.SubjectFactory;
import com.google.common.truth.Truth;
import com.google.gerrit.extensions.client.Comment;
public class RangeSubject extends Subject<RangeSubject, Comment.Range> {
private static final SubjectFactory<RangeSubject, Comment.Range>
RANGE_SUBJECT_FACTORY =
new SubjectFactory<RangeSubject, Comment.Range>() {
@Override
public RangeSubject getSubject(FailureStrategy failureStrategy,
Comment.Range range) {
return new RangeSubject(failureStrategy, range);
}
};
public static RangeSubject assertThat(Comment.Range range) {
return assertAbout(RANGE_SUBJECT_FACTORY)
.that(range);
}
private RangeSubject(FailureStrategy failureStrategy, Comment.Range range) {
super(failureStrategy, range);
}
public IntegerSubject startLine() {
return Truth.assertThat(actual().startLine).named("startLine");
}
public IntegerSubject startCharacter() {
return Truth.assertThat(actual().startCharacter).named("startCharacter");
}
public IntegerSubject endLine() {
return Truth.assertThat(actual().endLine).named("endLine");
}
public IntegerSubject endCharacter() {
return Truth.assertThat(actual().endCharacter).named("endCharacter");
}
}

View File

@ -4,4 +4,11 @@ acceptance_tests(
group = 'api_revision',
srcs = glob(['*IT.java']),
labels = ['api'],
deps = [':custom_truth_subjects'],
)
java_library(
name = 'custom_truth_subjects',
srcs = glob(['*Subject.java']),
deps = ['//gerrit-acceptance-tests:lib'],
)

View File

@ -4,4 +4,12 @@ acceptance_tests(
srcs = glob(["*IT.java"]),
group = "api_revision",
labels = ["api"],
deps = [":custom_truth_subjects"],
)
java_library(
name = "custom_truth_subjects",
testonly = 1,
srcs = glob(["*Subject.java"]),
deps = ["//gerrit-acceptance-tests:lib"],
)

View File

@ -0,0 +1,60 @@
// Copyright (C) 2016 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.api.revision;
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.acceptance.RangeSubject;
import com.google.gerrit.extensions.common.FixReplacementInfo;
public class FixReplacementInfoSubject
extends Subject<FixReplacementInfoSubject, FixReplacementInfo> {
private static final SubjectFactory<FixReplacementInfoSubject,
FixReplacementInfo> FIX_REPLACEMENT_INFO_SUBJECT_FACTORY =
new SubjectFactory<FixReplacementInfoSubject, FixReplacementInfo>() {
@Override
public FixReplacementInfoSubject getSubject(
FailureStrategy failureStrategy,
FixReplacementInfo fixReplacementInfo) {
return new FixReplacementInfoSubject(failureStrategy,
fixReplacementInfo);
}
};
public static FixReplacementInfoSubject assertThat(
FixReplacementInfo fixReplacementInfo) {
return assertAbout(FIX_REPLACEMENT_INFO_SUBJECT_FACTORY)
.that(fixReplacementInfo);
}
private FixReplacementInfoSubject(FailureStrategy failureStrategy,
FixReplacementInfo fixReplacementInfo) {
super(failureStrategy, fixReplacementInfo);
}
public RangeSubject range() {
return RangeSubject.assertThat(actual().range).named("range");
}
public StringSubject replacement() {
return Truth.assertThat(actual().replacement).named("replacement");
}
}

View File

@ -0,0 +1,71 @@
// Copyright (C) 2016 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.api.revision;
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.acceptance.ListSubject;
import com.google.gerrit.extensions.common.FixReplacementInfo;
import com.google.gerrit.extensions.common.FixSuggestionInfo;
public class FixSuggestionInfoSubject
extends Subject<FixSuggestionInfoSubject, FixSuggestionInfo> {
private static final SubjectFactory<FixSuggestionInfoSubject,
FixSuggestionInfo> FIX_SUGGESTION_INFO_SUBJECT_FACTORY =
new SubjectFactory<FixSuggestionInfoSubject, FixSuggestionInfo>() {
@Override
public FixSuggestionInfoSubject getSubject(
FailureStrategy failureStrategy,
FixSuggestionInfo fixSuggestionInfo) {
return new FixSuggestionInfoSubject(failureStrategy,
fixSuggestionInfo);
}
};
public static FixSuggestionInfoSubject assertThat(
FixSuggestionInfo fixSuggestionInfo) {
return assertAbout(FIX_SUGGESTION_INFO_SUBJECT_FACTORY)
.that(fixSuggestionInfo);
}
private FixSuggestionInfoSubject(FailureStrategy failureStrategy,
FixSuggestionInfo fixSuggestionInfo) {
super(failureStrategy, fixSuggestionInfo);
}
public StringSubject fixId() {
return Truth.assertThat(actual().fixId).named("fixId");
}
public ListSubject<FixReplacementInfoSubject,
FixReplacementInfo> replacements() {
return ListSubject.assertThat(actual().replacements,
FixReplacementInfoSubject::assertThat).named("replacements");
}
public FixReplacementInfoSubject onlyReplacement() {
return replacements().onlyElement();
}
public StringSubject description() {
return Truth.assertThat(actual().description).named("description");
}
}

View File

@ -0,0 +1,69 @@
// Copyright (C) 2016 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.api.revision;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.FailureStrategy;
import com.google.common.truth.Subject;
import com.google.common.truth.SubjectFactory;
import com.google.gerrit.acceptance.ListSubject;
import com.google.gerrit.extensions.common.FixSuggestionInfo;
import com.google.gerrit.extensions.common.RobotCommentInfo;
import java.util.List;
class RobotCommentInfoSubject
extends Subject<RobotCommentInfoSubject, RobotCommentInfo> {
private static final SubjectFactory<RobotCommentInfoSubject, RobotCommentInfo>
ROBOT_COMMENT_INFO_SUBJECT_FACTORY =
new SubjectFactory<RobotCommentInfoSubject, RobotCommentInfo>() {
@Override
public RobotCommentInfoSubject getSubject(
FailureStrategy failureStrategy,
RobotCommentInfo robotCommentInfo) {
return new RobotCommentInfoSubject(failureStrategy, robotCommentInfo);
}
};
public static ListSubject<RobotCommentInfoSubject,
RobotCommentInfo> assertThatList(
List<RobotCommentInfo> robotCommentInfos) {
return ListSubject.assertThat(robotCommentInfos,
RobotCommentInfoSubject::assertThat).named("robotCommentInfos");
}
public static RobotCommentInfoSubject assertThat(
RobotCommentInfo robotCommentInfo) {
return assertAbout(ROBOT_COMMENT_INFO_SUBJECT_FACTORY)
.that(robotCommentInfo);
}
private RobotCommentInfoSubject(FailureStrategy failureStrategy,
RobotCommentInfo robotCommentInfo) {
super(failureStrategy, robotCommentInfo);
}
public ListSubject<FixSuggestionInfoSubject,
FixSuggestionInfo> fixSuggestions() {
return ListSubject.assertThat(actual().fixSuggestions,
FixSuggestionInfoSubject::assertThat).named("fixSuggestions");
}
public FixSuggestionInfoSubject onlyFixSuggestion() {
return fixSuggestions().onlyElement();
}
}

View File

@ -17,18 +17,25 @@ package com.google.gerrit.acceptance.api.revision;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
import static com.google.gerrit.acceptance.api.revision.RobotCommentInfoSubject.assertThatList;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.extensions.client.Comment;
import com.google.gerrit.extensions.common.FixReplacementInfo;
import com.google.gerrit.extensions.common.FixSuggestionInfo;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.RobotCommentInput;
import com.google.gerrit.extensions.common.RobotCommentInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.RestApiException;
import org.junit.Before;
import org.junit.Test;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@ -36,11 +43,18 @@ import java.util.Map;
public class RobotCommentsIT extends AbstractDaemonTest {
private String changeId;
private FixReplacementInfo fixReplacementInfo;
private FixSuggestionInfo fixSuggestionInfo;
private RobotCommentInput withFixRobotCommentInput;
@Before
public void setUp() throws Exception {
PushOneCommit.Result changeResult = createChange();
changeId = changeResult.getChangeId();
fixReplacementInfo = createFixReplacementInfo();
fixSuggestionInfo = createFixSuggestionInfo(fixReplacementInfo);
withFixRobotCommentInput = createRobotCommentInput(fixSuggestionInfo);
}
@Test
@ -99,10 +113,7 @@ public class RobotCommentsIT extends AbstractDaemonTest {
RobotCommentInput robotCommentInput = createRobotCommentInput();
addRobotComment(changeId, robotCommentInput);
List<RobotCommentInfo> robotCommentInfos = gApi.changes()
.id(changeId)
.current()
.robotCommentsAsList();
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
RobotCommentInfo robotCommentInfo =
Iterables.getOnlyElement(robotCommentInfos);
@ -130,6 +141,145 @@ public class RobotCommentsIT extends AbstractDaemonTest {
assertRobotComment(comment, in, false);
}
@Test
public void addedFixSuggestionCanBeRetrieved() throws Exception {
assume().that(notesMigration.enabled()).isTrue();
addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThatList(robotCommentInfos).onlyElement()
.onlyFixSuggestion().isNotNull();
}
@Test
public void fixIdIsGeneratedForFixSuggestion()
throws Exception {
assume().that(notesMigration.enabled()).isTrue();
addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThatList(robotCommentInfos).onlyElement()
.onlyFixSuggestion().fixId().isNotEmpty();
assertThatList(robotCommentInfos).onlyElement()
.onlyFixSuggestion().fixId().isNotEqualTo(fixSuggestionInfo.fixId);
}
@Test
public void descriptionOfFixSuggestionIsAcceptedAsIs() throws Exception {
assume().that(notesMigration.enabled()).isTrue();
addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThatList(robotCommentInfos).onlyElement().onlyFixSuggestion()
.description().isEqualTo(fixSuggestionInfo.description);
}
@Test
public void descriptionOfFixSuggestionIsMandatory() throws Exception {
assume().that(notesMigration.enabled()).isTrue();
fixSuggestionInfo.description = null;
exception.expect(BadRequestException.class);
exception.expectMessage(String.format("A description is required for the "
+ "suggested fix of the robot comment on %s",
withFixRobotCommentInput.path));
addRobotComment(changeId, withFixRobotCommentInput);
}
@Test
public void addedFixReplacementCanBeRetrieved() throws Exception {
assume().that(notesMigration.enabled()).isTrue();
addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThatList(robotCommentInfos).onlyElement().onlyFixSuggestion()
.onlyReplacement().isNotNull();
}
@Test
public void fixReplacementsAreMandatory() throws Exception {
assume().that(notesMigration.enabled()).isTrue();
fixSuggestionInfo.replacements = Collections.emptyList();
exception.expect(BadRequestException.class);
exception.expectMessage(String.format("At least one replacement is required"
+ " for the suggested fix of the robot comment on %s",
withFixRobotCommentInput.path));
addRobotComment(changeId, withFixRobotCommentInput);
}
@Test
public void rangeOfFixReplacementIsAcceptedAsIs() throws Exception {
assume().that(notesMigration.enabled()).isTrue();
addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThatList(robotCommentInfos).onlyElement().onlyFixSuggestion()
.onlyReplacement().range().isEqualTo(fixReplacementInfo.range);
}
@Test
public void rangeOfFixReplacementIsMandatory() throws Exception {
assume().that(notesMigration.enabled()).isTrue();
fixReplacementInfo.range = null;
exception.expect(BadRequestException.class);
exception.expectMessage(String.format("A range must be given for the "
+ "replacement of the robot comment on %s",
withFixRobotCommentInput.path));
addRobotComment(changeId, withFixRobotCommentInput);
}
@Test
public void rangeOfFixReplacementNeedsToBeValid() throws Exception {
assume().that(notesMigration.enabled()).isTrue();
fixReplacementInfo.range = createRange(13, 9, 5, 10);
exception.expect(BadRequestException.class);
exception.expectMessage(String.format("Range (13:9 - 5:10) is not "
+ "valid for the replacement of the robot comment on %s",
withFixRobotCommentInput.path));
addRobotComment(changeId, withFixRobotCommentInput);
}
@Test
public void replacementStringOfFixReplacementIsAcceptedAsIs()
throws Exception {
assume().that(notesMigration.enabled()).isTrue();
addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThatList(robotCommentInfos).onlyElement()
.onlyFixSuggestion().onlyReplacement()
.replacement().isEqualTo(fixReplacementInfo.replacement);
}
@Test
public void replacementStringOfFixReplacementIsMandatory()
throws Exception {
assume().that(notesMigration.enabled()).isTrue();
fixReplacementInfo.replacement = null;
exception.expect(BadRequestException.class);
exception.expectMessage(String.format("A content for replacement must be "
+ "indicated for the replacement of the robot comment on %s",
withFixRobotCommentInput.path));
addRobotComment(changeId, withFixRobotCommentInput);
}
@Test
public void robotCommentsNotSupportedWithoutNoteDb() throws Exception {
assume().that(notesMigration.enabled()).isFalse();
@ -159,15 +309,43 @@ public class RobotCommentsIT extends AbstractDaemonTest {
return in;
}
private RobotCommentInput createRobotCommentInput() {
private RobotCommentInput createRobotCommentInput(
FixSuggestionInfo... fixSuggestionInfos) {
RobotCommentInput in = createRobotCommentInputWithMandatoryFields();
in.url = "http://www.happy-robot.com";
in.properties = new HashMap<>();
in.properties.put("key1", "value1");
in.properties.put("key2", "value2");
in.fixSuggestions = Arrays.asList(fixSuggestionInfos);
return in;
}
private FixSuggestionInfo createFixSuggestionInfo(
FixReplacementInfo... fixReplacementInfos) {
FixSuggestionInfo newFixSuggestionInfo = new FixSuggestionInfo();
newFixSuggestionInfo.fixId = "An ID which must be overwritten.";
newFixSuggestionInfo.description = "A description for a suggested fix.";
newFixSuggestionInfo.replacements = Arrays.asList(fixReplacementInfos);
return newFixSuggestionInfo;
}
private FixReplacementInfo createFixReplacementInfo() {
FixReplacementInfo newFixReplacementInfo = new FixReplacementInfo();
newFixReplacementInfo.replacement = "some replacement code";
newFixReplacementInfo.range = createRange(3, 12, 15, 4);
return newFixReplacementInfo;
}
private Comment.Range createRange(int startLine, int startCharacter,
int endLine, int endCharacter) {
Comment.Range range = new Comment.Range();
range.startLine = startLine;
range.startCharacter = startCharacter;
range.endLine = endLine;
range.endCharacter = endCharacter;
return range;
}
private void addRobotComment(String targetChangeId,
RobotCommentInput robotCommentInput) throws Exception {
ReviewInput reviewInput = new ReviewInput();
@ -180,6 +358,13 @@ public class RobotCommentsIT extends AbstractDaemonTest {
.review(reviewInput);
}
private List<RobotCommentInfo> getRobotComments() throws RestApiException {
return gApi.changes()
.id(changeId)
.current()
.robotCommentsAsList();
}
private void assertRobotComment(RobotCommentInfo c,
RobotCommentInput expected) {
assertRobotComment(c, expected, true);

View File

@ -18,6 +18,7 @@ import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
import com.google.gerrit.extensions.client.Comment;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.FixSuggestionInfo;
import com.google.gerrit.extensions.restapi.DefaultInput;
import java.util.ArrayList;
@ -103,6 +104,7 @@ public class ReviewInput {
public String robotRunId;
public String url;
public Map<String, String> properties;
public List<FixSuggestionInfo> fixSuggestions;
}
public ReviewInput message(String msg) {

View File

@ -41,6 +41,15 @@ public abstract class Comment {
public int endLine;
public int endCharacter;
public boolean isValid() {
return startLine >= 0
&& startCharacter >= 0
&& endLine >= 0
&& endCharacter >= 0
&& startLine <= endLine
&& (startLine != endLine || startCharacter <= endCharacter);
}
@Override
public boolean equals(Object o) {
if (o instanceof Range) {
@ -57,6 +66,16 @@ public abstract class Comment {
public int hashCode() {
return Objects.hash(startLine, startCharacter, endLine, endCharacter);
}
@Override
public String toString() {
return "Range{" +
"startLine=" + startLine +
", startCharacter=" + startCharacter +
", endLine=" + endLine +
", endCharacter=" + endCharacter +
'}';
}
}
public short side() {

View File

@ -0,0 +1,22 @@
// Copyright (C) 2016 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 com.google.gerrit.extensions.client.Comment;
public class FixReplacementInfo {
public Comment.Range range;
public String replacement;
}

View File

@ -0,0 +1,23 @@
// Copyright (C) 2016 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 java.util.List;
public class FixSuggestionInfo {
public String fixId;
public String description;
public List<FixReplacementInfo> replacements;
}

View File

@ -14,6 +14,7 @@
package com.google.gerrit.extensions.common;
import java.util.List;
import java.util.Map;
public class RobotCommentInfo extends CommentInfo {
@ -21,4 +22,5 @@ public class RobotCommentInfo extends CommentInfo {
public String robotRunId;
public String url;
public Map<String, String> properties;
public List<FixSuggestionInfo> fixSuggestions;
}

View File

@ -0,0 +1,33 @@
// Copyright (C) 2016 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.reviewdb.client;
public class FixReplacement {
public Comment.Range range;
public String replacement;
public FixReplacement(Comment.Range range, String replacement) {
this.range = range;
this.replacement = replacement;
}
@Override
public String toString() {
return "FixReplacement{" +
"range=" + range +
", replacement='" + replacement + '\'' +
'}';
}
}

View File

@ -0,0 +1,39 @@
// Copyright (C) 2016 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.reviewdb.client;
import java.util.List;
public class FixSuggestion {
public String fixId;
public String description;
public List<FixReplacement> replacements;
public FixSuggestion(String fixId, String description,
List<FixReplacement> replacements) {
this.fixId = fixId;
this.description = description;
this.replacements = replacements;
}
@Override
public String toString() {
return "FixSuggestion{" +
"fixId='" + fixId + '\'' +
", description='" + description + '\'' +
", replacements=" + replacements +
'}';
}
}

View File

@ -15,6 +15,7 @@
package com.google.gerrit.reviewdb.client;
import java.sql.Timestamp;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@ -23,6 +24,7 @@ public class RobotComment extends Comment {
public String robotRunId;
public String url;
public Map<String, String> properties;
public List<FixSuggestion> fixSuggestions;
public RobotComment(Key key, Account.Id author, Timestamp writtenOn,
short side, String message, String serverId, String robotId,
@ -54,6 +56,8 @@ public class RobotComment extends Comment {
.append("tag=").append(Objects.toString(tag, "")).append(',')
.append("url=").append(url).append(',')
.append("properties=").append(properties != null ? properties : "")
.append("fixSuggestions=")
.append(fixSuggestions != null ? fixSuggestions : "")
.append('}')
.toString();
}

View File

@ -21,9 +21,13 @@ import com.google.common.collect.FluentIterable;
import com.google.gerrit.extensions.client.Comment.Range;
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.FixReplacementInfo;
import com.google.gerrit.extensions.common.FixSuggestionInfo;
import com.google.gerrit.extensions.common.RobotCommentInfo;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.FixReplacement;
import com.google.gerrit.reviewdb.client.FixSuggestion;
import com.google.gerrit.reviewdb.client.RobotComment;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gwtorm.server.OrmException;
@ -34,6 +38,7 @@ import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import java.util.stream.Collectors;
class CommentJson {
@ -147,7 +152,7 @@ class CommentJson {
}
}
private Range toRange(Comment.Range commentRange) {
protected Range toRange(Comment.Range commentRange) {
Range range = null;
if (commentRange != null) {
range = new Range();
@ -181,10 +186,40 @@ class CommentJson {
rci.robotRunId = c.robotRunId;
rci.url = c.url;
rci.properties = c.properties;
rci.fixSuggestions = toFixSuggestionInfos(c.fixSuggestions);
fillCommentInfo(c, rci, loader);
return rci;
}
private List<FixSuggestionInfo> toFixSuggestionInfos(
List<FixSuggestion> fixSuggestions) {
if (fixSuggestions.isEmpty()) {
return null;
}
return fixSuggestions.stream()
.map(this::toFixSuggestionInfo)
.collect(Collectors.toList());
}
private FixSuggestionInfo toFixSuggestionInfo(FixSuggestion fixSuggestion) {
FixSuggestionInfo fixSuggestionInfo = new FixSuggestionInfo();
fixSuggestionInfo.fixId = fixSuggestion.fixId;
fixSuggestionInfo.description = fixSuggestion.description;
fixSuggestionInfo.replacements = fixSuggestion.replacements.stream()
.map(this::toFixReplacementInfo)
.collect(Collectors.toList());
return fixSuggestionInfo;
}
private FixReplacementInfo toFixReplacementInfo(
FixReplacement fixReplacement) {
FixReplacementInfo fixReplacementInfo = new FixReplacementInfo();
fixReplacementInfo.range = toRange(fixReplacement.range);
fixReplacementInfo.replacement = fixReplacement.replacement;
return fixReplacementInfo;
}
private RobotCommentFormatter() {
}
}

View File

@ -51,6 +51,8 @@ import com.google.gerrit.extensions.api.changes.ReviewerInfo;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.FixReplacementInfo;
import com.google.gerrit.extensions.common.FixSuggestionInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
@ -64,6 +66,8 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.FixReplacement;
import com.google.gerrit.reviewdb.client.FixSuggestion;
import com.google.gerrit.reviewdb.client.LabelId;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
@ -73,6 +77,7 @@ import com.google.gerrit.reviewdb.client.RobotComment;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
@ -111,6 +116,7 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
@Singleton
public class PostReview implements RestModifyView<RevisionResource, ReviewInput> {
@ -507,19 +513,102 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
for (Map.Entry<String, List<RobotCommentInput>> e : in.entrySet()) {
String path = e.getKey();
for (RobotCommentInput c : e.getValue()) {
if (c.robotId == null) {
throw new BadRequestException(String
.format("robotId is missing for robot comment on %s", path));
}
if (c.robotRunId == null) {
throw new BadRequestException(String
.format("robotRunId is missing for robot comment on %s", path));
}
ensureRobotIdIsSet(c.robotId, path);
ensureRobotRunIdIsSet(c.robotRunId, path);
ensureFixSuggestionsAreAddable(c.fixSuggestions, path);
}
}
checkComments(revision, in);
}
private void ensureRobotIdIsSet(String robotId, String path)
throws BadRequestException {
if (robotId == null) {
throw new BadRequestException(String
.format("robotId is missing for robot comment on %s", path));
}
}
private void ensureRobotRunIdIsSet(String robotRunId, String path)
throws BadRequestException {
if (robotRunId == null) {
throw new BadRequestException(String
.format("robotRunId is missing for robot comment on %s", path));
}
}
private void ensureFixSuggestionsAreAddable(
List<FixSuggestionInfo> fixSuggestionInfos, String path)
throws BadRequestException {
if (fixSuggestionInfos == null) {
return;
}
for (FixSuggestionInfo fixSuggestionInfo : fixSuggestionInfos) {
ensureDescriptionIsSet(path, fixSuggestionInfo.description);
ensureFixReplacementsAreAddable(path, fixSuggestionInfo.replacements);
}
}
private void ensureDescriptionIsSet(String path, String description)
throws BadRequestException {
if (description == null) {
throw new BadRequestException(String.format("A description is required "
+ "for the suggested fix of the robot comment on %s", path));
}
}
private void ensureFixReplacementsAreAddable(String path,
List<FixReplacementInfo> fixReplacementInfos) throws BadRequestException {
ensureReplacementsArePresent(path, fixReplacementInfos);
for (FixReplacementInfo fixReplacementInfo : fixReplacementInfos) {
ensureRangeIsSet(path, fixReplacementInfo.range);
ensureRangeIsValid(path, fixReplacementInfo.range);
ensureReplacementStringIsSet(path, fixReplacementInfo.replacement);
}
}
private void ensureReplacementsArePresent(String path,
List<FixReplacementInfo> fixReplacementInfos) throws BadRequestException {
if (fixReplacementInfos == null || fixReplacementInfos.isEmpty()) {
throw new BadRequestException(String.format("At least one replacement is "
+ "required for the suggested fix of the robot comment on %s", path));
}
}
private void ensureRangeIsSet(String path,
com.google.gerrit.extensions.client.Comment.Range range)
throws BadRequestException {
if (range == null) {
throw new BadRequestException(String.format("A range must be given "
+ "for the replacement of the robot comment on %s", path));
}
}
private void ensureRangeIsValid(String path,
com.google.gerrit.extensions.client.Comment.Range range)
throws BadRequestException {
if (range == null) {
return;
}
if (!range.isValid()) {
throw new BadRequestException(String.format("Range (%s:%s - %s:%s) is not"
+ " valid for the replacement of the robot comment on %s",
range.startLine, range.startCharacter, range.endLine,
range.endCharacter, path));
}
}
private void ensureReplacementStringIsSet(String path, String replacement)
throws BadRequestException {
if (replacement == null) {
throw new BadRequestException(String.format("A content for replacement "
+ "must be indicated for the replacement of the robot comment on %s",
path));
}
}
/**
* Used to compare Comments with CommentInput comments.
*/
@ -679,34 +768,86 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
return false;
}
List<RobotComment> newRobotComments = getNewRobotComments(ctx);
commentsUtil.putRobotComments(ctx.getUpdate(psId), newRobotComments);
comments.addAll(newRobotComments);
return !newRobotComments.isEmpty();
}
private List<RobotComment> getNewRobotComments(ChangeContext ctx)
throws OrmException {
List<RobotComment> toAdd = new ArrayList<>(in.robotComments.size());
Set<CommentSetEntry> existingIds = in.omitDuplicateComments
? readExistingRobotComments(ctx)
: Collections.emptySet();
for (Map.Entry<String, List<RobotCommentInput>> ent : in.robotComments.entrySet()) {
for (Map.Entry<String, List<RobotCommentInput>> ent :
in.robotComments.entrySet()) {
String path = ent.getKey();
for (RobotCommentInput c : ent.getValue()) {
RobotComment e = commentsUtil.newRobotComment(
ctx, path, psId, c.side(), c.message, c.robotId, c.robotRunId);
e.parentUuid = Url.decode(c.inReplyTo);
e.url = c.url;
e.properties = c.properties;
e.setLineNbrAndRange(c.line, c.range);
e.tag = in.tag;
setCommentRevId(e, patchListCache, ctx.getChange(), ps);
RobotComment e = createRobotCommentFromInput(ctx, path, c);
if (existingIds.contains(CommentSetEntry.create(e))) {
continue;
}
toAdd.add(e);
}
}
return toAdd;
}
commentsUtil.putRobotComments(ctx.getUpdate(psId), toAdd);
comments.addAll(toAdd);
return !toAdd.isEmpty();
private RobotComment createRobotCommentFromInput(ChangeContext ctx,
String path, RobotCommentInput robotCommentInput) throws OrmException {
RobotComment robotComment = commentsUtil.newRobotComment(
ctx, path, psId, robotCommentInput.side(), robotCommentInput.message,
robotCommentInput.robotId, robotCommentInput.robotRunId);
robotComment.parentUuid = Url.decode(robotCommentInput.inReplyTo);
robotComment.url = robotCommentInput.url;
robotComment.properties = robotCommentInput.properties;
robotComment.setLineNbrAndRange(robotCommentInput.line,
robotCommentInput.range);
robotComment.tag = in.tag;
setCommentRevId(robotComment, patchListCache, ctx.getChange(), ps);
robotComment.fixSuggestions = createFixSuggestionsFromInput(ctx,
robotCommentInput.fixSuggestions);
return robotComment;
}
private List<FixSuggestion> createFixSuggestionsFromInput(ChangeContext ctx,
List<FixSuggestionInfo> fixSuggestionInfos) throws OrmException {
if (fixSuggestionInfos == null) {
return Collections.emptyList();
}
List<FixSuggestion> fixSuggestions =
new ArrayList<>(fixSuggestionInfos.size());
for (FixSuggestionInfo fixSuggestionInfo : fixSuggestionInfos) {
fixSuggestions.add(
createFixSuggestionFromInput(ctx, fixSuggestionInfo));
}
return fixSuggestions;
}
private FixSuggestion createFixSuggestionFromInput(ChangeContext ctx,
FixSuggestionInfo fixSuggestionInfo) throws OrmException {
List<FixReplacement> fixReplacements =
toFixReplacements(fixSuggestionInfo.replacements);
String fixId = ChangeUtil.messageUUID(ctx.getDb());
return new FixSuggestion(fixId, fixSuggestionInfo.description,
fixReplacements);
}
private List<FixReplacement> toFixReplacements(
List<FixReplacementInfo> fixReplacementInfos) {
return fixReplacementInfos.stream()
.map(this::toFixReplacement)
.collect(Collectors.toList());
}
private FixReplacement toFixReplacement(
FixReplacementInfo fixReplacementInfo) {
Comment.Range range = new Comment.Range(fixReplacementInfo.range);
return new FixReplacement(range, fixReplacementInfo.replacement);
}
private Set<CommentSetEntry> readExistingComments(ChangeContext ctx)