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);