[MENFORCER-458] Move Require Property,Environment to new API

enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractPropertyEnforcerRule.java → enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/property/AbstractPropertyEnforcerRule.java

@@ -16,18 +16,18 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.maven.plugins.enforcer;
+package org.apache.maven.enforcer.rules.property;
 
 import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
-import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.apache.maven.enforcer.rules.AbstractStandardEnforcerRule;
 
 /**
  * Abstract enforcer rule that give a foundation to validate properties from multiple sources.
  *
  * @author Paul Gier
  * @author <a href='mailto:marvin[at]marvinformatics[dot]com'>Marvin Froeder</a>
  */
-public abstract class AbstractPropertyEnforcerRule extends AbstractNonCacheableEnforcerRule {
+abstract class AbstractPropertyEnforcerRule extends AbstractStandardEnforcerRule {
 
     /**
      * Match the property value to a given regular expression. Defaults to <code>null</code> (any value is ok).
@@ -45,10 +45,6 @@ public abstract class AbstractPropertyEnforcerRule extends AbstractNonCacheableE
      */
     private String regexMessage = null;
 
-    public AbstractPropertyEnforcerRule() {
-        super();
-    }
-
     /**
      * Set the property value to a given regular expression. Defaults to <code>null</code> (any value is ok).
      *
@@ -86,8 +82,8 @@ public final String getRegexMessage() {
     }
 
     @Override
-    public void execute(EnforcerRuleHelper helper) throws EnforcerRuleException {
-        Object propValue = resolveValue(helper);
+    public void execute() throws EnforcerRuleException {
+        Object propValue = resolveValue();
 
         // Check that the property is not null or empty string
         if (propValue == null) {
@@ -123,8 +119,7 @@ public void execute(EnforcerRuleHelper helper) throws EnforcerRuleException {
     /**
      * Resolves the property value
      *
-     * @param helper
      * @throws EnforcerRuleException
      */
-    protected abstract Object resolveValue(EnforcerRuleHelper helper) throws EnforcerRuleException;
+    protected abstract Object resolveValue() throws EnforcerRuleException;
 }

enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireEnvironmentVariable.java → enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/property/RequireEnvironmentVariable.java

@@ -16,17 +16,17 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.maven.plugins.enforcer;
+package org.apache.maven.enforcer.rules.property;
 
-import org.apache.maven.enforcer.rule.api.EnforcerRule;
-import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import javax.inject.Named;
 
 /**
  * This rule checks that certain environment variable is set.
  *
  * @author <a href='mailto:marvin[at]marvinformatics[dot]com'>Marvin Froeder</a>
  */
-public class RequireEnvironmentVariable extends AbstractPropertyEnforcerRule {
+@Named("requireEnvironmentVariable")
+public final class RequireEnvironmentVariable extends AbstractPropertyEnforcerRule {
 
     /**
      * Specify the required variable.
@@ -39,36 +39,22 @@ public class RequireEnvironmentVariable extends AbstractPropertyEnforcerRule {
      * @see #setVariableName(String)
      * @see #getVariableName()
      */
-    public final void setVariableName(String variableName) {
+    public void setVariableName(String variableName) {
         this.variableName = variableName;
     }
 
-    public final String getVariableName() {
+    public String getVariableName() {
         return variableName;
     }
 
     @Override
-    public String resolveValue(EnforcerRuleHelper helper) {
-        String envValue = System.getenv(variableName);
-        return envValue;
-    }
-
-    @Override
-    public boolean isCacheable() {
-        // environment variables won't change while maven is on the run
-        return true;
-    }
-
-    @Override
-    public boolean isResultValid(EnforcerRule cachedRule) {
-        // this rule shall always have the same result, since environment
-        // variables are set before maven is launched
-        return true;
+    public String resolveValue() {
+        return System.getenv(variableName);
     }
 
     @Override
     public String getCacheId() {
-        return variableName;
+        return String.valueOf(toString().hashCode());
     }
 
     @Override
@@ -80,4 +66,11 @@ public String getPropertyName() {
     public String getName() {
         return "Environment variable";
     }
+
+    @Override
+    public String toString() {
+        return String.format(
+                "RequireEnvironmentVariable[variableName=%s, message=%s, regex=%s, regexMessage=%s]",
+                variableName, getMessage(), getRegex(), getRegexMessage());
+    }
 }

enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequireProperty.java → enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/property/RequireProperty.java

@@ -16,18 +16,24 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.maven.plugins.enforcer;
+package org.apache.maven.enforcer.rules.property;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+
+import java.util.Objects;
 
 import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
-import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.apache.maven.enforcer.rules.utils.ExpressionEvaluator;
 import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
 
 /**
  * This rule checks that certain properties are set.
  *
  * @author Paul Gier
  */
-public class RequireProperty extends AbstractPropertyEnforcerRule {
+@Named("requireProperty")
+public final class RequireProperty extends AbstractPropertyEnforcerRule {
 
     /**
      * Specify the required property.
@@ -37,23 +43,24 @@ public class RequireProperty extends AbstractPropertyEnforcerRule {
      */
     private String property = null;
 
-    public final void setProperty(String property) {
+    private final ExpressionEvaluator evaluator;
+
+    @Inject
+    public RequireProperty(ExpressionEvaluator evaluator) {
+        this.evaluator = Objects.requireNonNull(evaluator);
+    }
+
+    public void setProperty(String property) {
         this.property = property;
     }
 
     @Override
-    public Object resolveValue(EnforcerRuleHelper helper) throws EnforcerRuleException {
-        Object propValue = null;
+    public Object resolveValue() throws EnforcerRuleException {
         try {
-            propValue = helper.evaluate("${" + property + "}");
-        } catch (ExpressionEvaluationException eee) {
-            throw new EnforcerRuleException("Unable to evaluate property: " + property, eee);
+            return evaluator.evaluate("${" + property + "}");
+        } catch (ExpressionEvaluationException e) {
+            throw new EnforcerRuleException(e);
         }
-        return propValue;
-    }
-
-    protected String resolveValue() {
-        return null;
     }
 
     @Override
@@ -65,4 +72,11 @@ public String getPropertyName() {
     public String getName() {
         return "Property";
     }
+
+    @Override
+    public String toString() {
+        return String.format(
+                "RequireProperty[property=%s, message=%s, regex=%s, regexMessage=%s]",
+                property, getMessage(), getRegex(), getRegexMessage());
+    }
 }

enforcer-rules/src/site/apt/requireEnvironmentVariable.apt.vm

@@ -27,16 +27,21 @@ Require Environment Variable
 
   This rule checks that a specified environment variable is set.
 
-
-   The following parameters are supported by this rule:
-
-   * message - an optional message to the user if the rule fails.
+  The following parameters are supported by this rule:
 
-   * variableName - The name of the environment variable to be checked for.
+  * <<message>> - an optional message to the user if the rule fails.
 
-   []
+  * <<variableName>> - The name of the environment variable to be checked for.
+
+  * <<regex>> - an optional regular expression used to check the value of the property.
+
+  * <<regexMessage>> - an optional message to the user if the regex check fails.
+
+  []
+
+  The regex is applied to the entire value of the environment variable (i.e. using the regex "match" method),
+  and not just a substring of the environment variable value.
 
-
   Sample Plugin Configuration:
 
 +---+

enforcer-rules/src/site/apt/requireProperty.apt.vm

@@ -29,13 +29,13 @@ Require Property
 
   The following parameters are supported by this rule:
 
-  * property - the property to evaluate.
+  * <<property>> - the property to evaluate.
 
-  * message - an optional message to the user if the rule fails. Default is: "Property 'xxx' is required for this build".	
+  * <<message>> - an optional message to the user if the rule fails. Default is: "Property 'xxx' is required for this build".
 
-  * regex - a regular expression used to check the value of the property.
+  * <<regex>> - an optional regular expression used to check the value of the property.
 
-  * regexMessage - an optional message to the user if the regex check fails.
+  * <<regexMessage>> - an optional message to the user if the regex check fails.
 
   []
 

enforcer-rules/src/test/java/org/apache/maven/plugins/enforcer/TestRequireEnvironmentVariable.java → enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/property/TestRequireEnvironmentVariable.java

@@ -16,10 +16,10 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.maven.plugins.enforcer;
+package org.apache.maven.enforcer.rules.property;
 
 import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
-import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.fail;
@@ -29,24 +29,20 @@
  *
  * @author <a href='mailto:marvin[at]marvinformatics[dot]com'>Marvin Froeder</a>
  */
-public class TestRequireEnvironmentVariable {
+class TestRequireEnvironmentVariable {
 
+    private RequireEnvironmentVariable rule = new RequireEnvironmentVariable();
     /**
      * Test rule.
      *
      */
     @Test
-    public void testRule() {
-        MockProject project = new MockProject();
-        project.setProperty("testProp", "This is a test.");
-        EnforcerRuleHelper helper = EnforcerTestUtils.getHelper(project);
-
-        RequireEnvironmentVariable rule = new RequireEnvironmentVariable();
+    void testRule() {
         // this env variable should not be set
         rule.setVariableName("JUNK");
 
         try {
-            rule.execute(helper);
+            rule.execute();
             fail("Expected an exception.");
         } catch (EnforcerRuleException e) {
             // expected to catch this.
@@ -55,7 +51,7 @@ public void testRule() {
         // PATH shall be common to windows and linux
         rule.setVariableName("PATH");
         try {
-            rule.execute(helper);
+            rule.execute();
         } catch (EnforcerRuleException e) {
             fail("This should not throw an exception");
         }
@@ -66,17 +62,15 @@ public void testRule() {
      *
      */
     @Test
-    public void testRuleWithRegex() {
-        EnforcerRuleHelper helper = EnforcerTestUtils.getHelper();
+    void testRuleWithRegex() {
 
-        RequireEnvironmentVariable rule = new RequireEnvironmentVariable();
         rule.setVariableName("PATH");
         // This expression should not match the property
         // value
         rule.setRegex("[^abc]");
 
         try {
-            rule.execute(helper);
+            rule.execute();
             fail("Expected an exception.");
         } catch (EnforcerRuleException e) {
             // expected to catch this.
@@ -85,9 +79,15 @@ public void testRuleWithRegex() {
         // can't really predict what a PATH will looks like, just enforce it ain't empty
         rule.setRegex(".{1,}");
         try {
-            rule.execute(helper);
+            rule.execute();
         } catch (EnforcerRuleException e) {
             fail("This should not throw an exception");
         }
     }
+
+    @Test
+    void ruleShouldBeCached() {
+        rule.setVariableName("TEST");
+        Assertions.assertThat(rule.getCacheId()).isNotEmpty();
+    }
 }