Merge changes I6f698f3e,I48d8d181
* changes:
Extract status from ChangeAttribute
Allow to negate conditions by setting their first value to !
diff --git a/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/PropertyAttributeExtractor.java b/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/PropertyAttributeExtractor.java
index 133c995..b1a47be 100644
--- a/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/PropertyAttributeExtractor.java
+++ b/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/util/PropertyAttributeExtractor.java
@@ -61,6 +61,11 @@
properties.add(propertyFactory.create("change-id", changeAttribute.id));
properties.add(propertyFactory.create("change-number", changeAttribute.number));
properties.add(propertyFactory.create("change-url", changeAttribute.url));
+ String status = null;
+ if (changeAttribute.status != null) {
+ status = changeAttribute.status.toString();
+ }
+ properties.add(propertyFactory.create("status", status));
properties.addAll(extractFrom(changeAttribute.owner, "owner"));
return properties;
}
diff --git a/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/workflow/Condition.java b/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/workflow/Condition.java
index 47aae91..61aa365 100644
--- a/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/workflow/Condition.java
+++ b/hooks-its/src/main/java/com/googlesource/gerrit/plugins/hooks/workflow/Condition.java
@@ -14,27 +14,34 @@
package com.googlesource.gerrit.plugins.hooks.workflow;
-import java.util.Arrays;
import java.util.Collections;
+import java.util.List;
import java.util.Set;
import javax.annotation.Nullable;
+import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
/**
* A condition as used in {@link Rule}, as precondition to {@link Action}s.
- *
- * A condition consists of a key and an associated set of values. A condition
- * is said to match a set of properties, is this set contains at least one
- * property that matches the rules key and whose value matches at least one
- * of the rule's value.
+ * <p>
+ * A condition consists of a key and an associated set of values.
+ * <p>
+ * For positive conditions (see constructor), the condition is said to match a
+ * set of properties, if this set contains at least one property that matches
+ * the rules key and whose value matches at least one of the rule's value.
+ * <p>
+ * For negated conditions (see constructor), the condition is said to match a
+ * set of properties, if this set does not contain any property that matches
+ * the rules key and whose value matches at least one of the rule's value.
*/
public class Condition {
private final String key;
private final Set<String> values;
+ private final boolean negated;
public interface Factory {
Condition create(@Assisted("key") String key,
@@ -44,19 +51,29 @@
/**
* Constructs a condition.
* @param key The key to use for values.
- * @param values A comma separated list of values to associate to the key.
+ * @param values A comma separated list of values to associate to the key. If
+ * the first value is not "!", it's a positive condition. If the first
+ * value is "!", the "!" is removed from the values and the condition is
+ * considered a negated condition.
*/
@Inject
public Condition(@Assisted("key") String key,
@Nullable @Assisted("values") String values) {
this.key = key;
Set<String> modifyableValues;
+ boolean modifyableNegated = false;
if (values == null) {
modifyableValues = Collections.emptySet();
} else {
- modifyableValues = Sets.newHashSet(Arrays.asList(values.split(",")));
+ List<String> valueList = Lists.newArrayList(values.split(","));
+ if (!valueList.isEmpty() && "!".equals(valueList.get(0))) {
+ modifyableNegated = true;
+ valueList.remove(0);
+ }
+ modifyableValues = Sets.newHashSet(valueList);
}
this.values = Collections.unmodifiableSet(modifyableValues);
+ this.negated = modifyableNegated;
}
public String getKey() {
@@ -67,8 +84,11 @@
* Checks whether or not the Condition matches the given set of properties
*
* @param properties The set of properties to match against.
- * @return True iff properties contains at least one property that matches
- * the rules key and whose value matches at least one of the rule's value.
+ * @return For positive conditions, true iff properties contains at least
+ * one property that matches the rules key and whose value matches at
+ * least one of the rule's value. For negated conditions, true iff
+ * properties does not contain any property that matches the rules key
+ * and whose value matches at least one of the rule's value.
*/
public boolean isMetBy(Iterable<Property> properties) {
for (Property property : properties) {
@@ -76,11 +96,11 @@
if ((key == null && propertyKey == null)
|| (key != null && key.equals(propertyKey))) {
if (values.contains(property.getValue())) {
- return true;
+ return !negated;
}
}
}
- return false;
+ return negated;
}
@Override
diff --git a/hooks-its/src/main/resources/Documentation/config.md b/hooks-its/src/main/resources/Documentation/config.md
index 2593171..afc74f9 100644
--- a/hooks-its/src/main/resources/Documentation/config.md
+++ b/hooks-its/src/main/resources/Documentation/config.md
@@ -94,9 +94,22 @@
----
name = value1, value2, ..., valueN
----
-and match if the event comes with a property 'name' having 'value1',
-or 'value2', or ..., or 'valueN'.
+and (if 'value1' is not +!+) match if the event comes with a property
+'name' having 'value1', or 'value2', or ..., or 'valueN'. So for
+example to match events that come with an 'association' property
+having 'subject', or 'footer-Bug', the following condition can be
+used:
+----
+association = subject,footer-Bug
+----
+If 'value1' is +!+, the conditon matches if the event does not come
+with a property 'name' having 'value2', or ..., or 'valueN'. So for
+example to match events that do not come with a 'status' property
+having 'DRAFT', the following condition can be used:
+----
+status = !,DRAFT
+----
[[event-properties]]
Event Properties
@@ -343,6 +356,14 @@
full name of the project the change belongs to.
'subject'::
first line of the change's most recent patch set's commit message.
+'status'::
+ status of the change ('null', 'NEW', 'SUBMITTED', 'DRAFT', 'MERGED',
+ or 'ABANDONED' )
+ +
+ This property will typically be 'null' unless the used Gerrit
+ incorporates
+ https://gerrit-review.googlesource.com/#/c/47042/[upstream change
+ 47042].
'topic'::
name of the topic the change belongs to.
diff --git a/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/PropertyAttributeExtractorTest.java b/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/PropertyAttributeExtractorTest.java
index a106d67..80f90e3 100644
--- a/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/PropertyAttributeExtractorTest.java
+++ b/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/util/PropertyAttributeExtractorTest.java
@@ -19,6 +19,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
+import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.server.config.FactoryModule;
import com.google.gerrit.server.data.AccountAttribute;
import com.google.gerrit.server.data.ApprovalAttribute;
@@ -124,6 +125,10 @@
expect(propertyFactory.create("change-url", "http://www.example.org/test"))
.andReturn(propertyUrl);
+ Property propertyStatus = createMock(Property.class);
+ expect(propertyFactory.create("status", null))
+ .andReturn(propertyStatus);
+
Property propertyEmail= createMock(Property.class);
expect(propertyFactory.create("owner-email", "testEmail"))
.andReturn(propertyEmail);
@@ -152,6 +157,91 @@
expected.add(propertyId);
expected.add(propertyNumber);
expected.add(propertyUrl);
+ expected.add(propertyStatus);
+ expected.add(propertyEmail);
+ expected.add(propertyName);
+ expected.add(propertyUsername);
+ assertEquals("Properties do not match", expected, actual);
+ }
+
+ public void testChangeAttributeFull() {
+ AccountAttribute owner = new AccountAttribute();
+ owner.email = "testEmail";
+ owner.name = "testName";
+ owner.username = "testUsername";
+
+ ChangeAttribute changeAttribute = new ChangeAttribute();
+ changeAttribute.project = "testProject";
+ changeAttribute.branch = "testBranch";
+ changeAttribute.topic = "testTopic";
+ changeAttribute.subject = "testSubject";
+ changeAttribute.id = "testId";
+ changeAttribute.number = "4711";
+ changeAttribute.url = "http://www.example.org/test";
+ changeAttribute.status = Status.ABANDONED;
+ changeAttribute.owner = owner;
+
+ Property propertyProject = createMock(Property.class);
+ expect(propertyFactory.create("project", "testProject"))
+ .andReturn(propertyProject);
+
+ Property propertyBranch = createMock(Property.class);
+ expect(propertyFactory.create("branch", "testBranch"))
+ .andReturn(propertyBranch);
+
+ Property propertyTopic = createMock(Property.class);
+ expect(propertyFactory.create("topic", "testTopic"))
+ .andReturn(propertyTopic);
+
+ Property propertySubject = createMock(Property.class);
+ expect(propertyFactory.create("subject", "testSubject"))
+ .andReturn(propertySubject);
+
+ Property propertyId = createMock(Property.class);
+ expect(propertyFactory.create("change-id", "testId"))
+ .andReturn(propertyId);
+
+ Property propertyNumber = createMock(Property.class);
+ expect(propertyFactory.create("change-number", "4711"))
+ .andReturn(propertyNumber);
+
+ Property propertyUrl = createMock(Property.class);
+ expect(propertyFactory.create("change-url", "http://www.example.org/test"))
+ .andReturn(propertyUrl);
+
+ Property propertyStatus = createMock(Property.class);
+ expect(propertyFactory.create("status", "ABANDONED"))
+ .andReturn(propertyStatus);
+
+ Property propertyEmail= createMock(Property.class);
+ expect(propertyFactory.create("owner-email", "testEmail"))
+ .andReturn(propertyEmail);
+
+ Property propertyName = createMock(Property.class);
+ expect(propertyFactory.create("owner-name", "testName"))
+ .andReturn(propertyName);
+
+ Property propertyUsername = createMock(Property.class);
+ expect(propertyFactory.create("owner-username", "testUsername"))
+ .andReturn(propertyUsername);
+
+
+ replayMocks();
+
+ PropertyAttributeExtractor extractor =
+ injector.getInstance(PropertyAttributeExtractor.class);
+
+ Set<Property> actual = extractor.extractFrom(changeAttribute);
+
+ Set<Property> expected = Sets.newHashSet();
+ expected.add(propertyProject);
+ expected.add(propertyBranch);
+ expected.add(propertyTopic);
+ expected.add(propertySubject);
+ expected.add(propertyId);
+ expected.add(propertyNumber);
+ expected.add(propertyUrl);
+ expected.add(propertyStatus);
expected.add(propertyEmail);
expected.add(propertyName);
expected.add(propertyUsername);
diff --git a/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/workflow/ConditionTest.java b/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/workflow/ConditionTest.java
index 857f9e8..886ab40 100644
--- a/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/workflow/ConditionTest.java
+++ b/hooks-its/src/test/java/com/googlesource/gerrit/plugins/hooks/workflow/ConditionTest.java
@@ -172,6 +172,146 @@
assertTrue("isMetBy gave false", condition.isMetBy(properties));
}
+ public void testNegatedIsMetByEmpty() {
+ Condition condition = createCondition("testKey", "!,testValue");
+
+ Collection<Property> properties = Collections.emptySet();
+
+ replayMocks();
+
+ assertTrue("isMetBy gave false", condition.isMetBy(properties));
+ }
+
+ public void testNegatedIsMetByMismatchedKey() {
+ Condition condition = createCondition("testKey", "!,testValue");
+
+ Property property1 = createMock(Property.class);
+ expect(property1.getKey()).andReturn("otherKey").anyTimes();
+ expect(property1.getValue()).andReturn("testValue").anyTimes();
+
+ Collection<Property> properties = Lists.newArrayListWithCapacity(1);
+ properties.add(property1);
+
+ replayMocks();
+
+ assertTrue("isMetBy gave false", condition.isMetBy(properties));
+ }
+
+ public void testNegatedIsMetByMaMismatchedValue() {
+ Condition condition = createCondition("testKey", "!,testValue");
+
+ Property property1 = createMock(Property.class);
+ expect(property1.getKey()).andReturn("testKey").anyTimes();
+ expect(property1.getValue()).andReturn("otherValue").anyTimes();
+
+ Collection<Property> properties = Lists.newArrayListWithCapacity(1);
+ properties.add(property1);
+
+ replayMocks();
+
+ assertTrue("isMetBy gave false", condition.isMetBy(properties));
+ }
+
+ public void testNegatedIsMetByOredNoMatch() {
+ Condition condition = createCondition("testKey", "!,value1,value2,value3");
+
+ Property property1 = createMock(Property.class);
+ expect(property1.getKey()).andReturn("testKey").anyTimes();
+ expect(property1.getValue()).andReturn("otherValue").anyTimes();
+
+ Collection<Property> properties = Lists.newArrayListWithCapacity(1);
+ properties.add(property1);
+
+ replayMocks();
+
+ assertTrue("isMetBy gave false", condition.isMetBy(properties));
+ }
+
+ public void testNegatedIsMetByOredSingleMatch() {
+ Condition condition = createCondition("testKey", "!,value1,value2,value3");
+
+ Property property1 = createMock(Property.class);
+ expect(property1.getKey()).andReturn("testKey").anyTimes();
+ expect(property1.getValue()).andReturn("value1").anyTimes();
+
+ Collection<Property> properties = Lists.newArrayListWithCapacity(1);
+ properties.add(property1);
+
+ replayMocks();
+
+ assertFalse("isMetBy gave true", condition.isMetBy(properties));
+ }
+
+ public void testNegatedIsMetByOredMultiple() {
+ Condition condition = createCondition("testKey", "!,value1,value2,value3");
+
+ Property property1 = createMock(Property.class);
+ expect(property1.getKey()).andReturn("testKey").anyTimes();
+ expect(property1.getValue()).andReturn("value1").anyTimes();
+
+ Property property2 = createMock(Property.class);
+ expect(property2.getKey()).andReturn("testKey").anyTimes();
+ expect(property2.getValue()).andReturn("value3").anyTimes();
+
+ Collection<Property> properties = Lists.newArrayListWithCapacity(2);
+ properties.add(property1);
+ properties.add(property2);
+
+ replayMocks();
+
+ assertFalse("isMetBy gave true", condition.isMetBy(properties));
+ }
+
+ public void testNegatedIsMetByOredAll() {
+ Condition condition = createCondition("testKey", "!,value1,value2,value3");
+
+ Property property1 = createMock(Property.class);
+ expect(property1.getKey()).andReturn("testKey").anyTimes();
+ expect(property1.getValue()).andReturn("value1").anyTimes();
+
+ Property property2 = createMock(Property.class);
+ expect(property2.getKey()).andReturn("testKey").anyTimes();
+ expect(property2.getValue()).andReturn("value2").anyTimes();
+
+ Property property3 = createMock(Property.class);
+ expect(property3.getKey()).andReturn("testKey").anyTimes();
+ expect(property3.getValue()).andReturn("value3").anyTimes();
+
+ Collection<Property> properties = Lists.newArrayListWithCapacity(1);
+ properties.add(property1);
+ properties.add(property2);
+ properties.add(property3);
+
+ replayMocks();
+
+ assertFalse("isMetBy gave true", condition.isMetBy(properties));
+ }
+
+ public void testNegatedIsMetByOredOvershoot() {
+ Condition condition = createCondition("testKey", "!,value1,value2,value3");
+
+ Property property1 = createMock(Property.class);
+ expect(property1.getKey()).andReturn("testKey").anyTimes();
+ expect(property1.getValue()).andReturn("otherValue1").anyTimes();
+
+ Property property2 = createMock(Property.class);
+ expect(property2.getKey()).andReturn("testKey").anyTimes();
+ expect(property2.getValue()).andReturn("value2").anyTimes();
+
+ Property property3 = createMock(Property.class);
+ expect(property3.getKey()).andReturn("testKey").anyTimes();
+ expect(property3.getValue()).andReturn("otherValue3").anyTimes();
+
+ Collection<Property> properties = Lists.newArrayListWithCapacity(3);
+ properties.add(property1);
+ properties.add(property2);
+ properties.add(property3);
+
+ replayMocks();
+
+ assertFalse("isMetBy gave true", condition.isMetBy(properties));
+ }
+
private Condition createCondition(String key, String value) {
Condition.Factory factory = injector.getInstance(Condition.Factory.class);
return factory.create(key, value);
