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