Fix smaller API issues and add tests
This commit fixes two smaller issues:
1) When sending a PUT request to change the config we make sure we
return a fresh result from the cache to include all changes.
2) We do not initialize an emtpy Set for copyScores as this would
overwrite existing copyScores if no input is provided.
Tests are added to cover these cases.
Change-Id: I33ad2bc9c6993d2ee6cf380cbee25b81329c36bf
diff --git a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/api/LabelDefinition.java b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/api/LabelDefinition.java
index 3ded3c2..4b9d6ea 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/api/LabelDefinition.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/api/LabelDefinition.java
@@ -16,7 +16,6 @@
import com.google.common.base.MoreObjects;
import com.google.gerrit.common.data.LabelFunction;
-import java.util.HashSet;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
@@ -26,9 +25,7 @@
public Boolean ignoreSelfApproval;
public Set<String> copyScores;
- public LabelDefinition() {
- copyScores = new HashSet<>();
- }
+ public LabelDefinition() {}
public LabelDefinition(String function, Boolean ignoreSelfApproval, Set<String> copyScores) {
this.function = function;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServlet.java b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServlet.java
index eae8d61..87c7b08 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServlet.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServlet.java
@@ -77,6 +77,6 @@
projectCache.evict(resource.getNameKey());
}
- return configTranslator.convertFrom(resource.getProjectState());
+ return configTranslator.convertFrom(projectCache.get(resource.getNameKey()));
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/PluginIT.java b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/PluginIT.java
index 2ddbba7..0bee0cb 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/PluginIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/PluginIT.java
@@ -141,18 +141,53 @@
SubmitConfig config = new SubmitConfig(ImmutableMap.of("Code-Review", codeReview), null);
postConfig(project, config);
- LabelType myLabel = projectCache.get(project).getConfig().getLabelSections().get("Code-Review");
- assertThat(myLabel.getFunction()).isEqualTo(LabelFunction.MAX_NO_BLOCK);
+ LabelType localCR = projectCache.get(project).getConfig().getLabelSections().get("Code-Review");
+ assertThat(localCR.getFunction()).isEqualTo(LabelFunction.MAX_NO_BLOCK);
+
+ // Check that the label has the same configs besides the function, which we changed
+ LabelType allProjectsCR = projectCache.getAllProjects().getLabelTypes().byLabel("Code-Review");
+ localCR.setFunction(allProjectsCR.getFunction());
+ assertLabelTypeEquals(localCR, allProjectsCR);
}
- private void postConfig(Project.NameKey project, SubmitConfig config) throws Exception {
+ @Test
+ public void replyContainsUpdatedConfig() throws Exception {
+ LabelDefinition codeReviewNoSelfApproval = new LabelDefinition(null, true, null);
+ SubmitConfig config =
+ new SubmitConfig(ImmutableMap.of("Code-Review", codeReviewNoSelfApproval), null);
+ SubmitConfig response = postConfig(project, config);
+ assertThat(response.labels.keySet()).containsExactly("Code-Review");
+ assertThat(response.labels.get("Code-Review").ignoreSelfApproval).isTrue();
+ }
+
+ private SubmitConfig postConfig(Project.NameKey project, SubmitConfig config) throws Exception {
RawInput rawInput =
RawInputUtil.create(newGson().toJson(config).getBytes(Charsets.UTF_8), JSON_TYPE);
RestResponse configResult = adminRestSession.putRaw(endpointUrl(project), rawInput);
configResult.assertOK();
+ return newGson().fromJson(configResult.getEntityContent(), SubmitConfig.class);
}
private static String endpointUrl(Project.NameKey project) {
return "/projects/" + project.get() + "/simple-submit-rules";
}
+
+ private static void assertLabelTypeEquals(LabelType l1, LabelType l2) {
+ assertThat(l1.allowPostSubmit()).isEqualTo(l2.allowPostSubmit());
+ assertThat(l1.canOverride()).isEqualTo(l2.canOverride());
+ assertThat(l1.getDefaultValue()).isEqualTo(l2.getDefaultValue());
+ assertThat(l1.getLabelId()).isEqualTo(l2.getLabelId());
+ assertThat(l1.getMax()).isEqualTo(l2.getMax());
+ assertThat(l1.getMin()).isEqualTo(l2.getMin());
+ assertThat(l1.getName()).isEqualTo(l2.getName());
+ assertThat(l1.getRefPatterns()).isEqualTo(l2.getRefPatterns());
+ assertThat(l1.ignoreSelfApproval()).isEqualTo(l2.ignoreSelfApproval());
+ assertThat(l1.isCopyAllScoresIfNoChange()).isEqualTo(l2.isCopyAllScoresIfNoChange());
+ assertThat(l1.isCopyAllScoresIfNoCodeChange()).isEqualTo(l2.isCopyAllScoresIfNoCodeChange());
+ assertThat(l1.isCopyAllScoresOnMergeFirstParentUpdate())
+ .isEqualTo(l2.isCopyAllScoresOnMergeFirstParentUpdate());
+ assertThat(l1.isCopyAllScoresOnTrivialRebase()).isEqualTo(l2.isCopyAllScoresOnTrivialRebase());
+ assertThat(l1.isCopyMaxScore()).isEqualTo(l2.isCopyMaxScore());
+ assertThat(l1.isCopyMinScore()).isEqualTo(l2.isCopyMinScore());
+ }
}
