Make config enablement check accept Project.NameKey
A Project.NameKey is expected in the end anyways, and using a
Project.NameKey right from the start spares us from unnecessary
(un-)boxing of values.
Change-Id: I0459eba9b1474f00b8a91b3b18fdd17c73852467
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfig.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfig.java
index 1a803e9..46fc817 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfig.java
@@ -64,36 +64,36 @@
public boolean isEnabled(Event event) {
if (event instanceof PatchSetCreatedEvent) {
PatchSetCreatedEvent e = (PatchSetCreatedEvent) event;
- return isEnabled(e.getProjectNameKey().get(), e.getRefName());
+ return isEnabled(e.getProjectNameKey(), e.getRefName());
} else if (event instanceof CommentAddedEvent) {
CommentAddedEvent e = (CommentAddedEvent) event;
- return isEnabled(e.getProjectNameKey().get(), e.getRefName());
+ return isEnabled(e.getProjectNameKey(), e.getRefName());
} else if (event instanceof ChangeMergedEvent) {
ChangeMergedEvent e = (ChangeMergedEvent) event;
- return isEnabled(e.getProjectNameKey().get(), e.getRefName());
+ return isEnabled(e.getProjectNameKey(), e.getRefName());
} else if (event instanceof ChangeAbandonedEvent) {
ChangeAbandonedEvent e = (ChangeAbandonedEvent) event;
- return isEnabled(e.getProjectNameKey().get(), e.getRefName());
+ return isEnabled(e.getProjectNameKey(), e.getRefName());
} else if (event instanceof ChangeRestoredEvent) {
ChangeRestoredEvent e = (ChangeRestoredEvent) event;
- return isEnabled(e.getProjectNameKey().get(), e.getRefName());
+ return isEnabled(e.getProjectNameKey(), e.getRefName());
} else if (event instanceof DraftPublishedEvent) {
DraftPublishedEvent e = (DraftPublishedEvent) event;
- return isEnabled(e.getProjectNameKey().get(), e.getRefName());
+ return isEnabled(e.getProjectNameKey(), e.getRefName());
} else if (event instanceof RefUpdatedEvent) {
RefUpdatedEvent e = (RefUpdatedEvent) event;
- return isEnabled(e.getProjectNameKey().get(), e.refUpdate.get().refName);
+ return isEnabled(e.getProjectNameKey(), e.getRefName());
} else {
log.debug("Event " + event + " not recognised and ignored");
return false;
}
}
- public boolean isEnabled(String project, String refName) {
- ProjectState projectState = projectCache.get(new Project.NameKey(project));
+ public boolean isEnabled(Project.NameKey projectNK, String refName) {
+ ProjectState projectState = projectCache.get(projectNK);
if (projectState == null) {
log.error("Failed to check if " + pluginName + " is enabled for project "
- + project + ": Project " + project + " not found");
+ + projectNK.get() + ": Project " + projectNK.get() + " not found");
return false;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateComment.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateComment.java
index c05118a..4ce687d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateComment.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateComment.java
@@ -143,7 +143,7 @@
@Override
public List<CommitValidationMessage> onCommitReceived(
CommitReceivedEvent receiveEvent) throws CommitValidationException {
- if (itsConfig.isEnabled(receiveEvent.project.getName(), receiveEvent.refName)) {
+ if (itsConfig.isEnabled(receiveEvent.getProjectNameKey(), receiveEvent.getRefName())) {
return validCommit(receiveEvent.commit);
} else {
return Collections.emptyList();
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfigTest.java
index 678bc76..c0ff02b 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfigTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfigTest.java
@@ -116,7 +116,8 @@
replayMocks();
- assertTrue(itsConfig.isEnabled("testProject", "refs/heads/testBranch"));
+ Project.NameKey projectNK = new Project.NameKey("testProject");
+ assertTrue(itsConfig.isEnabled(projectNK, "refs/heads/testBranch"));
}
public void testIsEnabledRefNoParentNoBranchDisabled() {
@@ -127,7 +128,8 @@
replayMocks();
- assertFalse(itsConfig.isEnabled("testProject", "refs/heads/testBranch"));
+ Project.NameKey projectNK = new Project.NameKey("testProject");
+ assertFalse(itsConfig.isEnabled(projectNK, "refs/heads/testBranch"));
}
public void testIsEnabledRefNoParentNoBranchEnforced() {
@@ -138,7 +140,8 @@
replayMocks();
- assertTrue(itsConfig.isEnabled("testProject", "refs/heads/testBranch"));
+ Project.NameKey projectNK = new Project.NameKey("testProject");
+ assertTrue(itsConfig.isEnabled(projectNK, "refs/heads/testBranch"));
}
public void testIsEnabledRefNoParentMatchingBranchEnabled() {
@@ -149,7 +152,8 @@
replayMocks();
- assertTrue(itsConfig.isEnabled("testProject", "refs/heads/testBranch"));
+ Project.NameKey projectNK = new Project.NameKey("testProject");
+ assertTrue(itsConfig.isEnabled(projectNK, "refs/heads/testBranch"));
}
public void testIsEnabledRefNoParentMatchingBranchDisabled() {
@@ -160,7 +164,8 @@
replayMocks();
- assertFalse(itsConfig.isEnabled("testProject", "refs/heads/testBranch"));
+ Project.NameKey projectNK = new Project.NameKey("testProject");
+ assertFalse(itsConfig.isEnabled(projectNK, "refs/heads/testBranch"));
}
public void testIsEnabledRefNoParentMatchingBranchEnforced() {
@@ -171,7 +176,8 @@
replayMocks();
- assertTrue(itsConfig.isEnabled("testProject", "refs/heads/testBranch"));
+ Project.NameKey projectNK = new Project.NameKey("testProject");
+ assertTrue(itsConfig.isEnabled(projectNK, "refs/heads/testBranch"));
}
public void testIsEnabledRefNoParentNonMatchingBranchEnabled() {
@@ -182,7 +188,8 @@
replayMocks();
- assertFalse(itsConfig.isEnabled("testProject", "refs/heads/testBranch"));
+ Project.NameKey projectNK = new Project.NameKey("testProject");
+ assertFalse(itsConfig.isEnabled(projectNK, "refs/heads/testBranch"));
}
public void testIsEnabledRefNoParentNonMatchingBranchDisabled() {
@@ -193,7 +200,8 @@
replayMocks();
- assertFalse(itsConfig.isEnabled("testProject", "refs/heads/testBranch"));
+ Project.NameKey projectNK = new Project.NameKey("testProject");
+ assertFalse(itsConfig.isEnabled(projectNK, "refs/heads/testBranch"));
}
public void testIsEnabledRefNoParentNonMatchingBranchEnforced() {
@@ -204,7 +212,8 @@
replayMocks();
- assertFalse(itsConfig.isEnabled("testProject", "refs/heads/testBranch"));
+ Project.NameKey projectNK = new Project.NameKey("testProject");
+ assertFalse(itsConfig.isEnabled(projectNK, "refs/heads/testBranch"));
}
public void testIsEnabledRefNoParentMatchingBranchMiddleEnabled() {
@@ -215,7 +224,8 @@
replayMocks();
- assertTrue(itsConfig.isEnabled("testProject", "refs/heads/testBranch"));
+ Project.NameKey projectNK = new Project.NameKey("testProject");
+ assertTrue(itsConfig.isEnabled(projectNK, "refs/heads/testBranch"));
}
public void testIsEnabledRefNoParentMatchingBranchMiddleDisabled() {
@@ -226,7 +236,8 @@
replayMocks();
- assertFalse(itsConfig.isEnabled("testProject", "refs/heads/testBranch"));
+ Project.NameKey projectNK = new Project.NameKey("testProject");
+ assertFalse(itsConfig.isEnabled(projectNK, "refs/heads/testBranch"));
}
public void testIsEnabledRefNoParentMatchingBranchMiddleEnforced() {
@@ -237,7 +248,8 @@
replayMocks();
- assertTrue(itsConfig.isEnabled("testProject", "refs/heads/testBranch"));
+ Project.NameKey projectNK = new Project.NameKey("testProject");
+ assertTrue(itsConfig.isEnabled(projectNK, "refs/heads/testBranch"));
}
public void testIsEnabledRefParentNoBranchEnabled() {
@@ -248,7 +260,8 @@
replayMocks();
- assertFalse(itsConfig.isEnabled("testProject", "refs/heads/testBranch"));
+ Project.NameKey projectNK = new Project.NameKey("testProject");
+ assertFalse(itsConfig.isEnabled(projectNK, "refs/heads/testBranch"));
}
public void testIsEnabledRefParentNoBranchDisabled() {
@@ -259,7 +272,8 @@
replayMocks();
- assertFalse(itsConfig.isEnabled("testProject", "refs/heads/testBranch"));
+ Project.NameKey projectNK = new Project.NameKey("testProject");
+ assertFalse(itsConfig.isEnabled(projectNK, "refs/heads/testBranch"));
}
public void testIsEnabledRefParentNoBranchEnforced() {
@@ -270,7 +284,8 @@
replayMocks();
- assertTrue(itsConfig.isEnabled("testProject", "refs/heads/testBranch"));
+ Project.NameKey projectNK = new Project.NameKey("testProject");
+ assertTrue(itsConfig.isEnabled(projectNK, "refs/heads/testBranch"));
}
public void testIsEnabledEventNoBranches() {
@@ -516,11 +531,10 @@
setupIsEnabled("true", null, branches);
RefUpdatedEvent event = new RefUpdatedEvent();
-
- RefUpdateAttribute refUpdate = new RefUpdateAttribute();
- refUpdate.project = "testProject";
- refUpdate.refName = "refs/heads/testBranch";
- event.refUpdate = Suppliers.ofInstance(refUpdate);
+ RefUpdateAttribute refUpdateAttribute = new RefUpdateAttribute();
+ refUpdateAttribute.project = "testProject";
+ refUpdateAttribute.refName = "refs/heads/testBranch";
+ event.refUpdate = Suppliers.ofInstance(refUpdateAttribute);
ItsConfig itsConfig = createItsConfig();
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateCommentTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateCommentTest.java
index 5ddc514..5a45dd4 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateCommentTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateCommentTest.java
@@ -467,7 +467,8 @@
private void setupCommonMocks() {
expect(itsConfig.getIssuePattern())
.andReturn(Pattern.compile("bug#(\\d+)")).anyTimes();
- expect(itsConfig.isEnabled("myProject", null)).andReturn(true)
+ Project.NameKey projectNK = new Project.NameKey("myProject");
+ expect(itsConfig.isEnabled(projectNK, null)).andReturn(true)
.anyTimes();
}
