stariy95 commented on code in PR #617:
URL: https://github.com/apache/cayenne/pull/617#discussion_r1617110860


##########
cayenne-project/src/main/java/org/apache/cayenne/project/validation/BaseQueryValidator.java:
##########
@@ -25,36 +25,49 @@
 import org.apache.cayenne.util.Util;
 import org.apache.cayenne.validation.ValidationResult;
 
-
 /**
  * Base validation for all query types
  */
-class BaseQueryValidator extends ConfigurationNodeValidator {
+public abstract class BaseQueryValidator<T extends QueryDescriptor> extends 
ConfigurationNodeValidator<T> {

Review Comment:
   Do we need to make this whole hierarchy public? This will expose all the 
validator classes and we should mark them and support further on.



##########
cayenne-project/src/main/java/org/apache/cayenne/project/validation/ConfigurationNodeValidator.java:
##########
@@ -18,29 +18,101 @@
  ****************************************************************/
 package org.apache.cayenne.project.validation;
 
+import org.apache.cayenne.configuration.ConfigurationNode;
 import org.apache.cayenne.validation.SimpleValidationFailure;
 import org.apache.cayenne.validation.ValidationResult;
 
+import java.util.function.Predicate;
+
 /**
  * A base superclass of various node validators.
  * 
  * @since 3.1
  */
-public abstract class ConfigurationNodeValidator {
+public abstract class ConfigurationNodeValidator<T extends ConfigurationNode> {
+
+    protected final ValidationConfig validationConfig;
 
-    public void addFailure(
-            ValidationResult validationResult,
-            Object source,
-            String messageFormat,
-            Object... messageParameters) {
+    /**
+     * @param validationConfig the config defining the behavior of this 
validator.
+     * @since 5.0
+     */
+    public ConfigurationNodeValidator(ValidationConfig validationConfig) {
+        this.validationConfig = validationConfig;
+    }
 
+    /**
+     * @param node the node that needs to be validated.
+     * @param validationResult the appendable validation result.
+     * @since 5.0
+     */
+    public abstract void validate(T node, ValidationResult validationResult);
+
+    public void addFailure(ValidationResult validationResult, T source, String 
messageFormat,
+                           Object... messageParameters) {
         String message = String.format(messageFormat, messageParameters);
         validationResult.addFailure(new SimpleValidationFailure(source, 
message));
     }
     
-    public void addFailure(
-               ValidationResult validationResult,
-               SimpleValidationFailure failure) {
+    public void addFailure(ValidationResult validationResult, 
SimpleValidationFailure failure) {
        validationResult.addFailure(failure);
     }
+
+    protected Performer<T> on(T node, ValidationResult validationResult) {
+        return new Performer<>(node, validationResult);
+    }
+
+    protected class Performer<N> {
+
+        private final N node;
+        private final ValidationResult validationResult;
+
+        protected Performer(N node, ValidationResult validationResult) {
+            this.node = node;
+            this.validationResult = validationResult;
+        }
+
+        protected Performer<N> performIfEnabled(Inspection inspection, 
ValidationAction<N> validationAction) {
+            if (validationConfig.isEnabled(inspection)) {
+                validationAction.perform(node, validationResult);
+            }
+            return this;
+        }
+
+        protected Performer<N> performIfEnabled(Inspection inspection, 
Runnable validationAction) {
+            if (validationConfig.isEnabled(inspection)) {
+                validationAction.run();
+            }
+            return this;
+        }
+
+        protected Performer<N> performIf(Predicate<N> predicate, 
ValidationAction<N> validationAction) {
+            if (predicate.test(node)) {
+                validationAction.perform(node, validationResult);
+            }
+            return this;
+        }
+
+        protected Performer<N> performIf(Predicate<N> predicate, Runnable 
validationAction) {
+            if (predicate.test(node)) {
+                validationAction.run();
+            }
+            return this;
+        }
+
+        protected Performer<N> perform(ValidationAction<N> validationAction) {
+            validationAction.perform(node, validationResult);
+            return this;
+        }
+
+        protected Performer<N> perform(Runnable validationAction) {
+            validationAction.run();
+            return this;
+        }

Review Comment:
   Do we really need all these methods? What will be the effort to shrink it 
down to 2-3 variants?



##########
cayenne-project/src/main/java/org/apache/cayenne/project/validation/DefaultProjectValidator.java:
##########
@@ -110,7 +112,7 @@ public ValidationResult 
visitDataChannelDescriptor(DataChannelDescriptor channel
         }
 
         public ValidationResult visitDataMap(DataMap dataMap) {
-            mapValidator.validate(dataMap, validationResult);
+            getValidator(DataMap.class).validate(dataMap, validationResult);
             for (Embeddable emb : dataMap.getEmbeddables()) {
                 visitEmbeddable(emb);
             }

Review Comment:
   Don't remember how this visitor works, so this is just a thought. Maybe we 
could skip manual call of visit methods and delegate it back to visitor, this 
would further simplify this code.



##########
cayenne/src/main/resources/org/apache/cayenne/schema/11/validation.xsd:
##########


Review Comment:
   Not sure about naming, `validation` could clash with the runtime model 
validation if we'll implement it someday. But probably it's ok for now.



##########
cayenne-project/src/main/java/org/apache/cayenne/project/validation/ConfigurationNodeValidator.java:
##########
@@ -18,29 +18,101 @@
  ****************************************************************/
 package org.apache.cayenne.project.validation;
 
+import org.apache.cayenne.configuration.ConfigurationNode;
 import org.apache.cayenne.validation.SimpleValidationFailure;
 import org.apache.cayenne.validation.ValidationResult;
 
+import java.util.function.Predicate;
+
 /**
  * A base superclass of various node validators.
  * 
  * @since 3.1
  */
-public abstract class ConfigurationNodeValidator {
+public abstract class ConfigurationNodeValidator<T extends ConfigurationNode> {
+
+    protected final ValidationConfig validationConfig;
 
-    public void addFailure(
-            ValidationResult validationResult,
-            Object source,
-            String messageFormat,
-            Object... messageParameters) {
+    /**
+     * @param validationConfig the config defining the behavior of this 
validator.
+     * @since 5.0
+     */
+    public ConfigurationNodeValidator(ValidationConfig validationConfig) {
+        this.validationConfig = validationConfig;
+    }
 
+    /**
+     * @param node the node that needs to be validated.
+     * @param validationResult the appendable validation result.
+     * @since 5.0
+     */
+    public abstract void validate(T node, ValidationResult validationResult);
+
+    public void addFailure(ValidationResult validationResult, T source, String 
messageFormat,
+                           Object... messageParameters) {
         String message = String.format(messageFormat, messageParameters);
         validationResult.addFailure(new SimpleValidationFailure(source, 
message));
     }
     
-    public void addFailure(
-               ValidationResult validationResult,
-               SimpleValidationFailure failure) {
+    public void addFailure(ValidationResult validationResult, 
SimpleValidationFailure failure) {
        validationResult.addFailure(failure);
     }
+
+    protected Performer<T> on(T node, ValidationResult validationResult) {
+        return new Performer<>(node, validationResult);
+    }
+
+    protected class Performer<N> {
+
+        private final N node;
+        private final ValidationResult validationResult;
+
+        protected Performer(N node, ValidationResult validationResult) {
+            this.node = node;
+            this.validationResult = validationResult;
+        }
+
+        protected Performer<N> performIfEnabled(Inspection inspection, 
ValidationAction<N> validationAction) {
+            if (validationConfig.isEnabled(inspection)) {
+                validationAction.perform(node, validationResult);
+            }
+            return this;
+        }
+
+        protected Performer<N> performIfEnabled(Inspection inspection, 
Runnable validationAction) {
+            if (validationConfig.isEnabled(inspection)) {
+                validationAction.run();
+            }
+            return this;
+        }
+
+        protected Performer<N> performIf(Predicate<N> predicate, 
ValidationAction<N> validationAction) {
+            if (predicate.test(node)) {
+                validationAction.perform(node, validationResult);
+            }
+            return this;
+        }
+
+        protected Performer<N> performIf(Predicate<N> predicate, Runnable 
validationAction) {
+            if (predicate.test(node)) {
+                validationAction.run();
+            }
+            return this;
+        }
+
+        protected Performer<N> perform(ValidationAction<N> validationAction) {
+            validationAction.perform(node, validationResult);
+            return this;
+        }
+
+        protected Performer<N> perform(Runnable validationAction) {
+            validationAction.run();
+            return this;
+        }
+    }
+
+    protected interface ValidationAction<N> {

Review Comment:
   Just a note, not sure we need to change this interface. This could be just 
`extends BiConsumer<N, ValidationResult>`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@cayenne.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to