From b2848d8fcd2c31169b10a89a5630c9a980953087 Mon Sep 17 00:00:00 2001 From: Sonja Skiba Date: Tue, 27 Aug 2024 13:21:55 +0200 Subject: [PATCH 1/7] Add first implementation of Validator and exemplary usage --- .../ide/commandlet/InstallCommandlet.java | 10 ++++++- .../ide/property/CommandletProperty.java | 7 +++-- .../tools/ide/property/EditionProperty.java | 7 +++-- .../tools/ide/property/FileProperty.java | 7 ++--- .../tools/ide/property/FolderProperty.java | 7 ++--- .../tools/ide/property/LocaleProperty.java | 6 ++--- .../tools/ide/property/NumberProperty.java | 7 +++-- .../tools/ide/property/PathProperty.java | 12 ++++----- .../tools/ide/property/PluginProperty.java | 7 +++-- .../devonfw/tools/ide/property/Property.java | 20 ++++++++------ .../ide/property/RepositoryProperty.java | 6 ++--- .../tools/ide/property/StringProperty.java | 7 +++-- .../tools/ide/property/ToolProperty.java | 7 +++-- .../tools/ide/property/VersionProperty.java | 6 ++--- .../ide/validation/PropertyValidator.java | 8 ++++++ .../ide/validation/ValidationResult.java | 9 +++++++ .../tools/ide/validation/ValidationState.java | 27 +++++++++++++++++++ 17 files changed, 106 insertions(+), 54 deletions(-) create mode 100644 cli/src/main/java/com/devonfw/tools/ide/validation/PropertyValidator.java create mode 100644 cli/src/main/java/com/devonfw/tools/ide/validation/ValidationResult.java create mode 100644 cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java diff --git a/cli/src/main/java/com/devonfw/tools/ide/commandlet/InstallCommandlet.java b/cli/src/main/java/com/devonfw/tools/ide/commandlet/InstallCommandlet.java index 79d4fad41..c83f53a9c 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/commandlet/InstallCommandlet.java +++ b/cli/src/main/java/com/devonfw/tools/ide/commandlet/InstallCommandlet.java @@ -4,6 +4,8 @@ import com.devonfw.tools.ide.property.ToolProperty; import com.devonfw.tools.ide.property.VersionProperty; import com.devonfw.tools.ide.tool.ToolCommandlet; +import com.devonfw.tools.ide.validation.PropertyValidator; +import com.devonfw.tools.ide.validation.ValidationState; import com.devonfw.tools.ide.version.VersionIdentifier; /** @@ -29,7 +31,13 @@ public InstallCommandlet(IdeContext context) { super(context); addKeyword(getName()); this.tool = add(new ToolProperty("", true, "tool")); - this.version = add(new VersionProperty("", false, "version")); + + PropertyValidator p = (VersionIdentifier value, ValidationState validationState) -> { + if (!value.isValid()) { + validationState.addErrorMessage("Version is not valid"); + } + }; + this.version = add(new VersionProperty("", false, "version", p)); } @Override diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/CommandletProperty.java b/cli/src/main/java/com/devonfw/tools/ide/property/CommandletProperty.java index f6af09c6e..76b42d0aa 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/CommandletProperty.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/CommandletProperty.java @@ -1,10 +1,9 @@ package com.devonfw.tools.ide.property; -import java.util.function.Consumer; - import com.devonfw.tools.ide.commandlet.Commandlet; import com.devonfw.tools.ide.completion.CompletionCandidateCollector; import com.devonfw.tools.ide.context.IdeContext; +import com.devonfw.tools.ide.validation.PropertyValidator; /** * {@link Property} with {@link #getValueType() value type} {@link Commandlet}. @@ -29,9 +28,9 @@ public CommandletProperty(String name, boolean required, String alias) { * @param name the {@link #getName() property name}. * @param required the {@link #isRequired() required flag}. * @param alias the {@link #getAlias() property alias}. - * @param validator the {@link Consumer} used to {@link #validate() validate} the {@link #getValue() value}. + * @param validator the {@link PropertyValidator} used to {@link #validate() validate} the {@link #getValue() value}. */ - public CommandletProperty(String name, boolean required, String alias, Consumer validator) { + public CommandletProperty(String name, boolean required, String alias, PropertyValidator validator) { super(name, required, alias, false, validator); } diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/EditionProperty.java b/cli/src/main/java/com/devonfw/tools/ide/property/EditionProperty.java index 36d10dd41..7cc2d782b 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/EditionProperty.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/EditionProperty.java @@ -1,8 +1,7 @@ package com.devonfw.tools.ide.property; -import java.util.function.Consumer; - import com.devonfw.tools.ide.context.IdeContext; +import com.devonfw.tools.ide.validation.PropertyValidator; public class EditionProperty extends Property { @@ -24,9 +23,9 @@ public EditionProperty(String name, boolean required, String alias) { * @param name the {@link #getName() property name}. * @param required the {@link #isRequired() required flag}. * @param alias the {@link #getAlias() property alias}. - * @param validator the {@link Consumer} used to {@link #validate() validate} the {@link #getValue() value}. + * @param validator the {@link PropertyValidator} used to {@link #validate() validate} the {@link #getValue() value}. */ - public EditionProperty(String name, boolean required, String alias, Consumer validator) { + public EditionProperty(String name, boolean required, String alias, PropertyValidator validator) { super(name, required, alias, false, validator); } diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/FileProperty.java b/cli/src/main/java/com/devonfw/tools/ide/property/FileProperty.java index 15229c10a..2eea9d925 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/FileProperty.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/FileProperty.java @@ -1,7 +1,8 @@ package com.devonfw.tools.ide.property; import java.nio.file.Path; -import java.util.function.Consumer; + +import com.devonfw.tools.ide.validation.PropertyValidator; /** * {@link PathProperty} for a file. @@ -28,9 +29,9 @@ public FileProperty(String name, boolean required, String alias, boolean mustExi * @param required the {@link #isRequired() required flag}. * @param alias the {@link #getAlias() property alias}. * @param mustExist the {@link #isPathRequiredToExist() required to exist flag}. - * @param validator the {@link Consumer} used to {@link #validate() validate} the {@link #getValue() value}. + * @param validator the {@link PropertyValidator} used to {@link #validate() validate} the {@link #getValue() value}. */ - public FileProperty(String name, boolean required, String alias, boolean mustExist, Consumer validator) { + public FileProperty(String name, boolean required, String alias, boolean mustExist, PropertyValidator validator) { super(name, required, alias, mustExist, validator); } diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/FolderProperty.java b/cli/src/main/java/com/devonfw/tools/ide/property/FolderProperty.java index da30bea9e..ff776533f 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/FolderProperty.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/FolderProperty.java @@ -1,7 +1,8 @@ package com.devonfw.tools.ide.property; import java.nio.file.Path; -import java.util.function.Consumer; + +import com.devonfw.tools.ide.validation.PropertyValidator; /** * {@link PathProperty} for a folder (directory). @@ -28,9 +29,9 @@ public FolderProperty(String name, boolean required, String alias, boolean mustE * @param required the {@link #isRequired() required flag}. * @param alias the {@link #getAlias() property alias}. * @param mustExist the {@link #isPathRequiredToExist() required to exist flag}. - * @param validator the {@link Consumer} used to {@link #validate() validate} the {@link #getValue() value}. + * @param validator the {@link PropertyValidator} used to {@link #validate() validate} the {@link #getValue() value}. */ - public FolderProperty(String name, boolean required, String alias, boolean mustExist, Consumer validator) { + public FolderProperty(String name, boolean required, String alias, boolean mustExist, PropertyValidator validator) { super(name, required, alias, mustExist, validator); } diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/LocaleProperty.java b/cli/src/main/java/com/devonfw/tools/ide/property/LocaleProperty.java index 31fee10ce..ac90578dd 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/LocaleProperty.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/LocaleProperty.java @@ -2,11 +2,11 @@ import java.util.Arrays; import java.util.Locale; -import java.util.function.Consumer; import com.devonfw.tools.ide.commandlet.Commandlet; import com.devonfw.tools.ide.completion.CompletionCandidateCollector; import com.devonfw.tools.ide.context.IdeContext; +import com.devonfw.tools.ide.validation.PropertyValidator; /** * {@link Property} with {@link Locale} as {@link #getValueType() value type}. @@ -33,9 +33,9 @@ public LocaleProperty(String name, boolean required, String alias) { * @param name the {@link #getName() property name}. * @param required the {@link #isRequired() required flag}. * @param alias the {@link #getAlias() property alias}. - * @param validator the {@link Consumer} used to {@link #validate() validate} the {@link #getValue() value}. + * @param validator the {@link PropertyValidator} used to {@link #validate() validate} the {@link #getValue() value}. */ - public LocaleProperty(String name, boolean required, String alias, Consumer validator) { + public LocaleProperty(String name, boolean required, String alias, PropertyValidator validator) { super(name, required, alias, false, validator); } diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/NumberProperty.java b/cli/src/main/java/com/devonfw/tools/ide/property/NumberProperty.java index ca0bf8277..f13c59bb3 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/NumberProperty.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/NumberProperty.java @@ -1,8 +1,7 @@ package com.devonfw.tools.ide.property; -import java.util.function.Consumer; - import com.devonfw.tools.ide.context.IdeContext; +import com.devonfw.tools.ide.validation.PropertyValidator; /** * {@link Property} with {@link #getValueType() value type} {@link Long}. @@ -27,9 +26,9 @@ public NumberProperty(String name, boolean required, String alias) { * @param name the {@link #getName() property name}. * @param required the {@link #isRequired() required flag}. * @param alias the {@link #getAlias() property alias}. - * @param validator the {@link Consumer} used to {@link #validate() validate} the {@link #getValue() value}. + * @param validator the {@link PropertyValidator} used to {@link #validate() validate} the {@link #getValue() value}. */ - public NumberProperty(String name, boolean required, String alias, Consumer validator) { + public NumberProperty(String name, boolean required, String alias, PropertyValidator validator) { super(name, required, alias, false, validator); } diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java b/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java index 04c9a8344..4ff353756 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java @@ -3,13 +3,13 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.function.Consumer; import java.util.stream.Stream; import com.devonfw.tools.ide.cli.CliException; import com.devonfw.tools.ide.commandlet.Commandlet; import com.devonfw.tools.ide.completion.CompletionCandidateCollector; import com.devonfw.tools.ide.context.IdeContext; +import com.devonfw.tools.ide.validation.PropertyValidator; /** * {@link Property} with {@link Path} as {@link #getValueType() value type}. @@ -38,9 +38,9 @@ public PathProperty(String name, boolean required, String alias, boolean mustExi * @param required the {@link #isRequired() required flag}. * @param alias the {@link #getAlias() property alias}. * @param mustExist the {@link #isPathRequiredToExist() required to exist flag}. - * @param validator the {@link Consumer} used to {@link #validate() validate} the {@link #getValue() value}. + * @param validator the {@link PropertyValidator} used to {@link #validate() validate} the {@link #getValue() value}. */ - public PathProperty(String name, boolean required, String alias, boolean mustExist, Consumer validator) { + public PathProperty(String name, boolean required, String alias, boolean mustExist, PropertyValidator validator) { super(name, required, alias, false, validator); this.mustExist = mustExist; @@ -78,7 +78,7 @@ public boolean validate() { /** * @return {@code true} if the {@link Path} {@link #getValue() value} must {@link Files#exists(Path, java.nio.file.LinkOption...) exist} if set, {@code false} - * otherwise. + * otherwise. */ protected boolean isPathRequiredToExist() { @@ -87,7 +87,7 @@ protected boolean isPathRequiredToExist() { /** * @return {@code true} if the {@link Path} {@link #getValue() value} must be a {@link Files#isDirectory(Path, java.nio.file.LinkOption...) folder} if it - * exists, {@code false} otherwise. + * exists, {@code false} otherwise. */ protected boolean isPathRequiredToBeFolder() { @@ -96,7 +96,7 @@ protected boolean isPathRequiredToBeFolder() { /** * @return {@code true} if the {@link Path} {@link #getValue() value} must be a {@link Files#isRegularFile(Path, java.nio.file.LinkOption...) file} if it - * exists, {@code false} otherwise. + * exists, {@code false} otherwise. */ protected boolean isPathRequiredToBeFile() { diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/PluginProperty.java b/cli/src/main/java/com/devonfw/tools/ide/property/PluginProperty.java index 38d138a04..e022f5445 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/PluginProperty.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/PluginProperty.java @@ -1,7 +1,5 @@ package com.devonfw.tools.ide.property; -import java.util.function.Consumer; - import com.devonfw.tools.ide.commandlet.Commandlet; import com.devonfw.tools.ide.completion.CompletionCandidateCollector; import com.devonfw.tools.ide.context.IdeContext; @@ -9,6 +7,7 @@ import com.devonfw.tools.ide.tool.plugin.PluginBasedCommandlet; import com.devonfw.tools.ide.tool.plugin.PluginDescriptor; import com.devonfw.tools.ide.tool.plugin.PluginMaps; +import com.devonfw.tools.ide.validation.PropertyValidator; /** * {@link Property} representing the plugin of a {@link PluginBasedCommandlet}. @@ -34,9 +33,9 @@ public PluginProperty(String name, boolean required, String alias) { * @param required the {@link #isRequired() required flag}. * @param alias the {@link #getAlias() property alias}. * @param multivalued the boolean flag about multiple arguments - * @param validator the {@link Consumer} used to {@link #validate() validate} the {@link #getValue() value}. + * @param validator the {@link PropertyValidator} used to {@link #validate() validate} the {@link #getValue() value}. */ - public PluginProperty(String name, boolean required, String alias, boolean multivalued, Consumer validator) { + public PluginProperty(String name, boolean required, String alias, boolean multivalued, PropertyValidator validator) { super(name, required, alias, multivalued, validator); } diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/Property.java b/cli/src/main/java/com/devonfw/tools/ide/property/Property.java index 193854a31..d25b65dad 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/Property.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/Property.java @@ -11,6 +11,8 @@ import com.devonfw.tools.ide.completion.CompletionCandidateCollector; import com.devonfw.tools.ide.completion.CompletionCandidateCollectorAdapter; import com.devonfw.tools.ide.context.IdeContext; +import com.devonfw.tools.ide.validation.PropertyValidator; +import com.devonfw.tools.ide.validation.ValidationState; /** * A {@link Property} is a simple container for a {@link #getValue() value} with a fixed {@link #getName() name} and {@link #getValueType() type}. Further we @@ -37,7 +39,9 @@ public abstract class Property { /** @see #isRequired() */ protected final boolean required; - private final Consumer validator; + private final PropertyValidator validator; + + private ValidationState state = new ValidationState(); /** @see #isMultiValued() */ private final boolean multivalued; @@ -71,7 +75,7 @@ public Property(String name, boolean required, String alias) { * @param multivalued the boolean flag about multiple arguments * @param validator the {@link Consumer} used to {@link #validate() validate} the {@link #getValue() value}. */ - public Property(String name, boolean required, String alias, boolean multivalued, Consumer validator) { + public Property(String name, boolean required, String alias, boolean multivalued, PropertyValidator validator) { super(); this.name = name; @@ -111,7 +115,7 @@ public String getNameOrAlias() { /** * @return {@code true} if this property is required (if argument is not present the {@link Commandlet} cannot be invoked), {@code false} otherwise (if - * optional). + * optional). */ public boolean isRequired() { @@ -139,7 +143,7 @@ public boolean isOption() { /** * @return {@code true} if this {@link Property} forces an implicit {@link CliArgument#isEndOptions() end-options} as if "--" was provided before its first - * {@link CliArgument argument}. + * {@link CliArgument argument}. */ public boolean isEndOptions() { @@ -449,9 +453,9 @@ public boolean matches(String nameOrAlias) { /** * @return {@code true} if this {@link Property} is valid, {@code false} if it is {@link #isRequired() required} but no {@link #getValue() value} has been - * set. - * @throws RuntimeException if the {@link #getValue() value} is violating given constraints. This is checked by the optional {@link Consumer} function given - * at construction time. + * set. + * @throws RuntimeException if the {@link #getValue() value} is violating given constraints. This is checked by the optional {@link Consumer} function + * given at construction time. */ public boolean validate() { @@ -460,7 +464,7 @@ public boolean validate() { } if (this.validator != null) { for (V value : this.value) { - this.validator.accept(value); + validator.validate(value, state); } } return true; diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/RepositoryProperty.java b/cli/src/main/java/com/devonfw/tools/ide/property/RepositoryProperty.java index 908d33dbb..bb1c68a12 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/RepositoryProperty.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/RepositoryProperty.java @@ -3,9 +3,9 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; -import java.util.function.Consumer; import com.devonfw.tools.ide.context.IdeContext; +import com.devonfw.tools.ide.validation.PropertyValidator; /** * Extends {@link FileProperty} for repository properties config file with auto-completion. @@ -30,9 +30,9 @@ public RepositoryProperty(String name, boolean required, String alias) { * @param name the {@link #getName() property name}. * @param required the {@link #isRequired() required flag}. * @param alias the {@link #getAlias() property alias}. - * @param validator the {@link Consumer} used to {@link #validate() validate} the {@link #getValue() value}. + * @param validator the {@link PropertyValidator} used to {@link #validate() validate} the {@link #getValue() value}. */ - public RepositoryProperty(String name, boolean required, String alias, Consumer validator) { + public RepositoryProperty(String name, boolean required, String alias, PropertyValidator validator) { super(name, required, alias, true, validator); } diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/StringProperty.java b/cli/src/main/java/com/devonfw/tools/ide/property/StringProperty.java index 35e9c5bfb..b9ff9c479 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/StringProperty.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/StringProperty.java @@ -1,8 +1,7 @@ package com.devonfw.tools.ide.property; -import java.util.function.Consumer; - import com.devonfw.tools.ide.context.IdeContext; +import com.devonfw.tools.ide.validation.PropertyValidator; /** * {@link Property} with {@link #getValueType() value type} {@link String}. @@ -40,10 +39,10 @@ public StringProperty(String name, boolean required, boolean multivalued, String * @param name the {@link #getName() property name}. * @param required the {@link #isRequired() required flag}. * @param alias the {@link #getAlias() property alias}. - * @param validator the {@link Consumer} used to {@link #validate() validate} the {@link #getValue() value}. + * @param validator the {@link PropertyValidator} used to {@link #validate() validate} the {@link #getValue() value}. * @param multivalued the boolean flag about multiple arguments */ - public StringProperty(String name, boolean required, String alias, boolean multivalued, Consumer validator) { + public StringProperty(String name, boolean required, String alias, boolean multivalued, PropertyValidator validator) { super(name, required, alias, multivalued, validator); } diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/ToolProperty.java b/cli/src/main/java/com/devonfw/tools/ide/property/ToolProperty.java index 2bca2e85e..8df35c8c1 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/ToolProperty.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/ToolProperty.java @@ -1,11 +1,10 @@ package com.devonfw.tools.ide.property; -import java.util.function.Consumer; - import com.devonfw.tools.ide.commandlet.Commandlet; import com.devonfw.tools.ide.completion.CompletionCandidateCollector; import com.devonfw.tools.ide.context.IdeContext; import com.devonfw.tools.ide.tool.ToolCommandlet; +import com.devonfw.tools.ide.validation.PropertyValidator; /** * {@link Property} with {@link #getValueType() value type} {@link ToolCommandlet}. @@ -44,9 +43,9 @@ public ToolProperty(String name, boolean required, boolean multivalued, String a * @param required the {@link #isRequired() required flag}. * @param alias the {@link #getAlias() property alias}. * @param multivalued the boolean flag about multiple arguments - * @param validator the {@link Consumer} used to {@link #validate() validate} the {@link #getValue() value}. + * @param validator the {@link PropertyValidator} used to {@link #validate() validate} the {@link #getValue() value}. */ - public ToolProperty(String name, boolean required, String alias, boolean multivalued, Consumer validator) { + public ToolProperty(String name, boolean required, String alias, boolean multivalued, PropertyValidator validator) { super(name, required, alias, multivalued, validator); } diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/VersionProperty.java b/cli/src/main/java/com/devonfw/tools/ide/property/VersionProperty.java index 4db38498e..0bff2636f 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/VersionProperty.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/VersionProperty.java @@ -2,7 +2,6 @@ import java.util.Collections; import java.util.List; -import java.util.function.Consumer; import java.util.stream.IntStream; import com.devonfw.tools.ide.commandlet.Commandlet; @@ -10,6 +9,7 @@ import com.devonfw.tools.ide.completion.CompletionCandidateCollector; import com.devonfw.tools.ide.context.IdeContext; import com.devonfw.tools.ide.tool.ToolCommandlet; +import com.devonfw.tools.ide.validation.PropertyValidator; import com.devonfw.tools.ide.version.VersionIdentifier; import com.devonfw.tools.ide.version.VersionSegment; @@ -36,9 +36,9 @@ public VersionProperty(String name, boolean required, String alias) { * @param name the {@link #getName() property name}. * @param required the {@link #isRequired() required flag}. * @param alias the {@link #getAlias() property alias}. - * @param validator the {@link Consumer} used to {@link #validate() validate} the {@link #getValue() value}. + * @param validator the {@link PropertyValidator} used to {@link #validate() validate} the {@link #getValue() value}. */ - public VersionProperty(String name, boolean required, String alias, Consumer validator) { + public VersionProperty(String name, boolean required, String alias, PropertyValidator validator) { super(name, required, alias, false, validator); } diff --git a/cli/src/main/java/com/devonfw/tools/ide/validation/PropertyValidator.java b/cli/src/main/java/com/devonfw/tools/ide/validation/PropertyValidator.java new file mode 100644 index 000000000..aafaefd03 --- /dev/null +++ b/cli/src/main/java/com/devonfw/tools/ide/validation/PropertyValidator.java @@ -0,0 +1,8 @@ +package com.devonfw.tools.ide.validation; + +@FunctionalInterface +public interface PropertyValidator { + + void validate(V value, ValidationState validationState); + +} diff --git a/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationResult.java b/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationResult.java new file mode 100644 index 000000000..c9fe76643 --- /dev/null +++ b/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationResult.java @@ -0,0 +1,9 @@ +package com.devonfw.tools.ide.validation; + +public interface ValidationResult { + + boolean isValid(); + + String getErrorMessage(); + +} diff --git a/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java b/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java new file mode 100644 index 000000000..37298a50a --- /dev/null +++ b/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java @@ -0,0 +1,27 @@ +package com.devonfw.tools.ide.validation; + +public class ValidationState implements ValidationResult { + + private StringBuilder errorMessage; + + public boolean isValid() { + return (this.errorMessage == null); + } + + public String getErrorMessage() { + if (this.errorMessage == null) { + return null; + } + return this.errorMessage.toString(); + } + + public void addErrorMessage(String error) { + if (this.errorMessage == null) { + this.errorMessage = new StringBuilder(error.length()); + } else { + this.errorMessage.append('\n'); + } + this.errorMessage.append(error); + } + +} From 83eb75aa57e44df829ba30820a40da58f8e925e7 Mon Sep 17 00:00:00 2001 From: Sonja Skiba Date: Fri, 30 Aug 2024 08:58:48 +0200 Subject: [PATCH 2/7] Fix return of validate --- cli/src/main/java/com/devonfw/tools/ide/property/Property.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/Property.java b/cli/src/main/java/com/devonfw/tools/ide/property/Property.java index d25b65dad..2b4812cae 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/Property.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/Property.java @@ -467,7 +467,7 @@ public boolean validate() { validator.validate(value, state); } } - return true; + return state.isValid(); } @Override From ecac345caf5a433dbd4f672028c736ea45bcdecf Mon Sep 17 00:00:00 2001 From: Sonja Skiba Date: Fri, 30 Aug 2024 12:22:38 +0200 Subject: [PATCH 3/7] make Property.validate return ValidationState instead of boolean as discuessed in the daily --- .../devonfw/tools/ide/commandlet/Commandlet.java | 5 ++++- .../tools/ide/commandlet/InstallCommandlet.java | 14 ++++++-------- .../devonfw/tools/ide/property/PathProperty.java | 14 +++++++------- .../com/devonfw/tools/ide/property/Property.java | 11 ++++++----- 4 files changed, 23 insertions(+), 21 deletions(-) diff --git a/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java b/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java index bed28e78e..080770f8f 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java +++ b/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java @@ -13,6 +13,7 @@ import com.devonfw.tools.ide.property.Property; import com.devonfw.tools.ide.tool.ToolCommandlet; import com.devonfw.tools.ide.tool.plugin.PluginDescriptor; +import com.devonfw.tools.ide.validation.ValidationState; import com.devonfw.tools.ide.version.VersionIdentifier; /** @@ -228,7 +229,9 @@ public boolean validate() { } } for (Property property : this.propertiesList) { - if (!property.validate()) { + ValidationState state = property.validate(); + if (!state.isValid()) { + context.debug(state.getErrorMessage()); return false; } } diff --git a/cli/src/main/java/com/devonfw/tools/ide/commandlet/InstallCommandlet.java b/cli/src/main/java/com/devonfw/tools/ide/commandlet/InstallCommandlet.java index c83f53a9c..a11277c2a 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/commandlet/InstallCommandlet.java +++ b/cli/src/main/java/com/devonfw/tools/ide/commandlet/InstallCommandlet.java @@ -4,7 +4,6 @@ import com.devonfw.tools.ide.property.ToolProperty; import com.devonfw.tools.ide.property.VersionProperty; import com.devonfw.tools.ide.tool.ToolCommandlet; -import com.devonfw.tools.ide.validation.PropertyValidator; import com.devonfw.tools.ide.validation.ValidationState; import com.devonfw.tools.ide.version.VersionIdentifier; @@ -31,13 +30,12 @@ public InstallCommandlet(IdeContext context) { super(context); addKeyword(getName()); this.tool = add(new ToolProperty("", true, "tool")); - - PropertyValidator p = (VersionIdentifier value, ValidationState validationState) -> { - if (!value.isValid()) { - validationState.addErrorMessage("Version is not valid"); - } - }; - this.version = add(new VersionProperty("", false, "version", p)); + this.version = add(new VersionProperty("", false, "version", + (VersionIdentifier v, ValidationState state) -> { + if (!v.isValid()) { + state.addErrorMessage("Given version " + v + " is not a valid version"); + } + })); } @Override diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java b/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java index 4ff353756..2273fba17 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java @@ -5,11 +5,11 @@ import java.nio.file.Path; import java.util.stream.Stream; -import com.devonfw.tools.ide.cli.CliException; import com.devonfw.tools.ide.commandlet.Commandlet; import com.devonfw.tools.ide.completion.CompletionCandidateCollector; import com.devonfw.tools.ide.context.IdeContext; import com.devonfw.tools.ide.validation.PropertyValidator; +import com.devonfw.tools.ide.validation.ValidationState; /** * {@link Property} with {@link Path} as {@link #getValueType() value type}. @@ -59,20 +59,20 @@ public Path parse(String valueAsString, IdeContext context) { } @Override - public boolean validate() { - + public ValidationState validate() { + ValidationState state = super.validate(); for (Path path : this.value) { if (path != null && Files.exists(path)) { if (isPathRequiredToBeFile() && !Files.isRegularFile(path)) { - throw new CliException("Path " + path + " is not a file."); + state.addErrorMessage("Path " + path + " is not a file."); } else if (isPathRequiredToBeFolder() && !Files.isDirectory(path)) { - throw new CliException("Path " + path + " is not a folder."); + state.addErrorMessage("Path " + path + " is not a folder."); } } else if (isPathRequiredToExist()) { - throw new CliException("Path " + path + " does not exist."); + state.addErrorMessage("Path " + path + " does not exist."); } } - return super.validate(); + return state; } diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/Property.java b/cli/src/main/java/com/devonfw/tools/ide/property/Property.java index 2b4812cae..0e296325b 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/Property.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/Property.java @@ -41,8 +41,6 @@ public abstract class Property { private final PropertyValidator validator; - private ValidationState state = new ValidationState(); - /** @see #isMultiValued() */ private final boolean multivalued; @@ -457,17 +455,20 @@ public boolean matches(String nameOrAlias) { * @throws RuntimeException if the {@link #getValue() value} is violating given constraints. This is checked by the optional {@link Consumer} function * given at construction time. */ - public boolean validate() { + public ValidationState validate() { + + ValidationState state = new ValidationState(); if (this.required && (getValue() == null)) { - return false; + state.addErrorMessage("Property " + this.name + " is required but no value has been set"); + return state; } if (this.validator != null) { for (V value : this.value) { validator.validate(value, state); } } - return state.isValid(); + return state; } @Override From ad67b942e5d2456d5b49fe8ed36a7e68e749eafb Mon Sep 17 00:00:00 2001 From: Sonja Skiba Date: Thu, 5 Sep 2024 09:59:56 +0200 Subject: [PATCH 4/7] Changes according to review add joining of ValidationResults --- .../tools/ide/commandlet/Commandlet.java | 3 +- .../ide/commandlet/InstallCommandlet.java | 2 +- .../tools/ide/property/PathProperty.java | 7 ++- .../devonfw/tools/ide/property/Property.java | 3 +- .../tools/ide/validation/ValidationState.java | 5 ++ .../ide/validation/ValidationStateTest.java | 61 +++++++++++++++++++ 6 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 cli/src/test/java/com/devonfw/tools/ide/validation/ValidationStateTest.java diff --git a/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java b/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java index 080770f8f..27e5b0c28 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java +++ b/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java @@ -13,6 +13,7 @@ import com.devonfw.tools.ide.property.Property; import com.devonfw.tools.ide.tool.ToolCommandlet; import com.devonfw.tools.ide.tool.plugin.PluginDescriptor; +import com.devonfw.tools.ide.validation.ValidationResult; import com.devonfw.tools.ide.validation.ValidationState; import com.devonfw.tools.ide.version.VersionIdentifier; @@ -229,7 +230,7 @@ public boolean validate() { } } for (Property property : this.propertiesList) { - ValidationState state = property.validate(); + ValidationResult state = property.validate(); if (!state.isValid()) { context.debug(state.getErrorMessage()); return false; diff --git a/cli/src/main/java/com/devonfw/tools/ide/commandlet/InstallCommandlet.java b/cli/src/main/java/com/devonfw/tools/ide/commandlet/InstallCommandlet.java index a11277c2a..4ea01b581 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/commandlet/InstallCommandlet.java +++ b/cli/src/main/java/com/devonfw/tools/ide/commandlet/InstallCommandlet.java @@ -31,7 +31,7 @@ public InstallCommandlet(IdeContext context) { addKeyword(getName()); this.tool = add(new ToolProperty("", true, "tool")); this.version = add(new VersionProperty("", false, "version", - (VersionIdentifier v, ValidationState state) -> { + (v, state) -> { if (!v.isValid()) { state.addErrorMessage("Given version " + v + " is not a valid version"); } diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java b/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java index 2273fba17..537e89fa9 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java @@ -9,6 +9,7 @@ import com.devonfw.tools.ide.completion.CompletionCandidateCollector; import com.devonfw.tools.ide.context.IdeContext; import com.devonfw.tools.ide.validation.PropertyValidator; +import com.devonfw.tools.ide.validation.ValidationResult; import com.devonfw.tools.ide.validation.ValidationState; /** @@ -59,8 +60,8 @@ public Path parse(String valueAsString, IdeContext context) { } @Override - public ValidationState validate() { - ValidationState state = super.validate(); + public ValidationResult validate() { + ValidationState state = new ValidationState(); for (Path path : this.value) { if (path != null && Files.exists(path)) { if (isPathRequiredToBeFile() && !Files.isRegularFile(path)) { @@ -72,8 +73,8 @@ public ValidationState validate() { state.addErrorMessage("Path " + path + " does not exist."); } } + state.add(super.validate()); return state; - } /** diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/Property.java b/cli/src/main/java/com/devonfw/tools/ide/property/Property.java index 0e296325b..c8b1f895f 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/Property.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/Property.java @@ -12,6 +12,7 @@ import com.devonfw.tools.ide.completion.CompletionCandidateCollectorAdapter; import com.devonfw.tools.ide.context.IdeContext; import com.devonfw.tools.ide.validation.PropertyValidator; +import com.devonfw.tools.ide.validation.ValidationResult; import com.devonfw.tools.ide.validation.ValidationState; /** @@ -455,7 +456,7 @@ public boolean matches(String nameOrAlias) { * @throws RuntimeException if the {@link #getValue() value} is violating given constraints. This is checked by the optional {@link Consumer} function * given at construction time. */ - public ValidationState validate() { + public ValidationResult validate() { ValidationState state = new ValidationState(); diff --git a/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java b/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java index 37298a50a..d53a6b118 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java +++ b/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java @@ -24,4 +24,9 @@ public void addErrorMessage(String error) { this.errorMessage.append(error); } + public void add(ValidationResult result) { + if (!result.isValid()) { + addErrorMessage(result.getErrorMessage()); + } + } } diff --git a/cli/src/test/java/com/devonfw/tools/ide/validation/ValidationStateTest.java b/cli/src/test/java/com/devonfw/tools/ide/validation/ValidationStateTest.java new file mode 100644 index 000000000..e4a82b1f8 --- /dev/null +++ b/cli/src/test/java/com/devonfw/tools/ide/validation/ValidationStateTest.java @@ -0,0 +1,61 @@ +package com.devonfw.tools.ide.validation; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +class ValidationStateTest { + + @Test + void testAddTwoResults() { + + ValidationState stateOne = new ValidationState(); + stateOne.addErrorMessage("state one: first error message"); + stateOne.addErrorMessage("state one: second error message"); + ValidationState stateTwo = new ValidationState(); + stateTwo.addErrorMessage("state two: first error message"); + stateTwo.addErrorMessage("state two: second error message"); + + stateOne.add(stateTwo); + + assertThat(stateOne.getErrorMessage()).isEqualTo("state one: first error message" + "\n" + + "state one: second error message" + "\n" + + "state two: first error message" + "\n" + + "state two: second error message"); + } + + @Test + void testAddEmptyResult() { + + ValidationState stateOne = new ValidationState(); + stateOne.addErrorMessage("state one: first error message"); + stateOne.addErrorMessage("state one: second error message"); + ValidationState stateTwo = new ValidationState(); + + stateOne.add(stateTwo); + + assertThat(stateOne.getErrorMessage()).isEqualTo("state one: first error message" + "\n" + "state one: second error message"); + } + + @Test + void testAddToEmptyResult() { + ValidationState stateOne = new ValidationState(); + ValidationState stateTwo = new ValidationState(); + stateTwo.addErrorMessage("state two: first error message"); + stateTwo.addErrorMessage("state two: second error message"); + + stateOne.add(stateTwo); + + assertThat(stateOne.getErrorMessage()).isEqualTo("state two: first error message" + "\n" + "state two: second error message"); + } + + @Test + void testAddBothEmptyResult() { + ValidationState stateOne = new ValidationState(); + ValidationState stateTwo = new ValidationState(); + + stateOne.add(stateTwo); + + assertThat(stateOne.getErrorMessage()).isNull(); + } +} From 1efcb0b7045f3997102e009380e348349f286717 Mon Sep 17 00:00:00 2001 From: Sonja Skiba Date: Thu, 5 Sep 2024 14:14:27 +0200 Subject: [PATCH 5/7] Add property name to validation state --- .../tools/ide/commandlet/Commandlet.java | 12 +++--- .../tools/ide/commandlet/ShellCommandlet.java | 2 +- .../tools/ide/context/AbstractIdeContext.java | 2 +- .../tools/ide/property/PathProperty.java | 2 +- .../devonfw/tools/ide/property/Property.java | 2 +- .../tools/ide/validation/ValidationState.java | 17 +++++++- .../ide/validation/ValidationStateTest.java | 41 +++++++++++-------- 7 files changed, 51 insertions(+), 27 deletions(-) diff --git a/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java b/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java index 27e5b0c28..4c3bd131c 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java +++ b/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java @@ -221,22 +221,24 @@ public boolean isProcessableOutput() { * @return {@code true} if this {@link Commandlet} is the valid candidate to be {@link #run()}, {@code false} otherwise. * @see Property#validate() */ - public boolean validate() { + public ValidationResult validate() { // avoid validation exception if not a candidate to be run. for (Property property : this.propertiesList) { if (property.isRequired() && (property.getValue() == null)) { - return false; + ValidationState state = new ValidationState(property.getNameOrAlias()); + state.addErrorMessage("Required property has no value"); + return state; } } for (Property property : this.propertiesList) { ValidationResult state = property.validate(); if (!state.isValid()) { - context.debug(state.getErrorMessage()); - return false; + context.error(state.getErrorMessage()); + return state; } } - return true; + return new ValidationState(null); } /** diff --git a/cli/src/main/java/com/devonfw/tools/ide/commandlet/ShellCommandlet.java b/cli/src/main/java/com/devonfw/tools/ide/commandlet/ShellCommandlet.java index a20a08743..b2d14a6da 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/commandlet/ShellCommandlet.java +++ b/cli/src/main/java/com/devonfw/tools/ide/commandlet/ShellCommandlet.java @@ -199,6 +199,6 @@ private boolean apply(CliArgument argument, Commandlet commandlet) { } currentArgument = currentArgument.getNext(!endOpts); } - return commandlet.validate(); + return commandlet.validate().isValid(); } } diff --git a/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java b/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java index 6118a3ad3..40560182e 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java +++ b/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java @@ -824,7 +824,7 @@ private boolean applyAndRun(CliArguments arguments, Commandlet cmd) { boolean matches = apply(arguments, cmd, null); if (matches) { - matches = cmd.validate(); + matches = cmd.validate().isValid(); } if (matches) { debug("Running commandlet {}", cmd); diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java b/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java index 537e89fa9..f87c4e2d3 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/PathProperty.java @@ -61,7 +61,7 @@ public Path parse(String valueAsString, IdeContext context) { @Override public ValidationResult validate() { - ValidationState state = new ValidationState(); + ValidationState state = new ValidationState(this.getNameOrAlias()); for (Path path : this.value) { if (path != null && Files.exists(path)) { if (isPathRequiredToBeFile() && !Files.isRegularFile(path)) { diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/Property.java b/cli/src/main/java/com/devonfw/tools/ide/property/Property.java index c8b1f895f..4fed98ef3 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/Property.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/Property.java @@ -458,7 +458,7 @@ public boolean matches(String nameOrAlias) { */ public ValidationResult validate() { - ValidationState state = new ValidationState(); + ValidationState state = new ValidationState(this.getNameOrAlias()); if (this.required && (getValue() == null)) { state.addErrorMessage("Property " + this.name + " is required but no value has been set"); diff --git a/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java b/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java index d53a6b118..0d077697e 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java +++ b/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java @@ -4,6 +4,12 @@ public class ValidationState implements ValidationResult { private StringBuilder errorMessage; + private String propertyName; + + public ValidationState(String propertyName) { + this.propertyName = propertyName; + } + public boolean isValid() { return (this.errorMessage == null); } @@ -17,7 +23,9 @@ public String getErrorMessage() { public void addErrorMessage(String error) { if (this.errorMessage == null) { - this.errorMessage = new StringBuilder(error.length()); + this.errorMessage = new StringBuilder(propertyName.length()); + this.errorMessage.append(String.format("Error in property %s:", propertyName)); + this.errorMessage.append('\n'); } else { this.errorMessage.append('\n'); } @@ -26,7 +34,12 @@ public void addErrorMessage(String error) { public void add(ValidationResult result) { if (!result.isValid()) { - addErrorMessage(result.getErrorMessage()); + if (this.errorMessage == null) { + this.errorMessage = new StringBuilder(result.getErrorMessage().length()); + this.errorMessage.append(result.getErrorMessage()); + } else { + addErrorMessage(result.getErrorMessage()); + } } } } diff --git a/cli/src/test/java/com/devonfw/tools/ide/validation/ValidationStateTest.java b/cli/src/test/java/com/devonfw/tools/ide/validation/ValidationStateTest.java index e4a82b1f8..1c6263ff6 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/validation/ValidationStateTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/validation/ValidationStateTest.java @@ -1,58 +1,67 @@ package com.devonfw.tools.ide.validation; -import org.junit.jupiter.api.Test; - import static org.assertj.core.api.Assertions.assertThat; +import org.junit.jupiter.api.Test; + class ValidationStateTest { @Test void testAddTwoResults() { - ValidationState stateOne = new ValidationState(); + ValidationState stateOne = new ValidationState("testPropertyOne"); stateOne.addErrorMessage("state one: first error message"); stateOne.addErrorMessage("state one: second error message"); - ValidationState stateTwo = new ValidationState(); + ValidationState stateTwo = new ValidationState("testPropertyTwo"); stateTwo.addErrorMessage("state two: first error message"); stateTwo.addErrorMessage("state two: second error message"); stateOne.add(stateTwo); - assertThat(stateOne.getErrorMessage()).isEqualTo("state one: first error message" + "\n" - + "state one: second error message" + "\n" - + "state two: first error message" + "\n" - + "state two: second error message"); + assertThat(stateOne.getErrorMessage()).isEqualTo( + "Error in property testPropertyOne:" + "\n" + + "state one: first error message" + "\n" + + "state one: second error message" + "\n" + + "Error in property testPropertyTwo:" + "\n" + + "state two: first error message" + "\n" + + "state two: second error message"); } @Test void testAddEmptyResult() { - ValidationState stateOne = new ValidationState(); + ValidationState stateOne = new ValidationState("testPropertyOne"); stateOne.addErrorMessage("state one: first error message"); stateOne.addErrorMessage("state one: second error message"); - ValidationState stateTwo = new ValidationState(); + ValidationState stateTwo = new ValidationState("testPropertyTwo"); stateOne.add(stateTwo); - assertThat(stateOne.getErrorMessage()).isEqualTo("state one: first error message" + "\n" + "state one: second error message"); + assertThat(stateOne.getErrorMessage()).isEqualTo( + "Error in property testPropertyOne:" + "\n" + + "state one: first error message" + "\n" + + "state one: second error message"); } @Test void testAddToEmptyResult() { - ValidationState stateOne = new ValidationState(); - ValidationState stateTwo = new ValidationState(); + ValidationState stateOne = new ValidationState("testPropertyOne"); + ValidationState stateTwo = new ValidationState("testPropertyTwo"); stateTwo.addErrorMessage("state two: first error message"); stateTwo.addErrorMessage("state two: second error message"); stateOne.add(stateTwo); - assertThat(stateOne.getErrorMessage()).isEqualTo("state two: first error message" + "\n" + "state two: second error message"); + assertThat(stateOne.getErrorMessage()).isEqualTo( + "Error in property testPropertyTwo:" + "\n" + + "state two: first error message" + "\n" + + "state two: second error message"); } @Test void testAddBothEmptyResult() { - ValidationState stateOne = new ValidationState(); - ValidationState stateTwo = new ValidationState(); + ValidationState stateOne = new ValidationState("testPropertyOne"); + ValidationState stateTwo = new ValidationState("testPropertyTwo"); stateOne.add(stateTwo); From 64315395d5ab4d226178b9f8ee537fe4c6c04a4c Mon Sep 17 00:00:00 2001 From: Sonja Skiba Date: Fri, 6 Sep 2024 10:16:53 +0200 Subject: [PATCH 6/7] Use ValidationResult as return for apply and applyAndRun --- .../tools/ide/commandlet/Commandlet.java | 17 ++------ .../tools/ide/context/AbstractIdeContext.java | 41 +++++++++++-------- .../tools/ide/validation/ValidationState.java | 11 +++-- 3 files changed, 36 insertions(+), 33 deletions(-) diff --git a/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java b/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java index 4c3bd131c..8e20efc8c 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java +++ b/cli/src/main/java/com/devonfw/tools/ide/commandlet/Commandlet.java @@ -222,23 +222,12 @@ public boolean isProcessableOutput() { * @see Property#validate() */ public ValidationResult validate() { - + ValidationState state = new ValidationState(null); // avoid validation exception if not a candidate to be run. for (Property property : this.propertiesList) { - if (property.isRequired() && (property.getValue() == null)) { - ValidationState state = new ValidationState(property.getNameOrAlias()); - state.addErrorMessage("Required property has no value"); - return state; - } - } - for (Property property : this.propertiesList) { - ValidationResult state = property.validate(); - if (!state.isValid()) { - context.error(state.getErrorMessage()); - return state; - } + state.add(property.validate()); } - return new ValidationState(null); + return state; } /** diff --git a/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java b/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java index 40560182e..a50e467f7 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java +++ b/cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java @@ -50,6 +50,8 @@ import com.devonfw.tools.ide.step.Step; import com.devonfw.tools.ide.step.StepImpl; import com.devonfw.tools.ide.url.model.UrlMetadata; +import com.devonfw.tools.ide.validation.ValidationResult; +import com.devonfw.tools.ide.validation.ValidationState; import com.devonfw.tools.ide.variable.IdeVariables; /** @@ -774,10 +776,10 @@ public int run(CliArguments arguments) { if (!current.isEnd()) { String keyword = current.get(); firstCandidate = this.commandletManager.getCommandletByFirstKeyword(keyword); - boolean matches; + ValidationResult firstResult = null; if (firstCandidate != null) { - matches = applyAndRun(arguments.copy(), firstCandidate); - if (matches) { + firstResult = applyAndRun(arguments.copy(), firstCandidate); + if (firstResult.isValid()) { supressStepSuccess = firstCandidate.isSuppressStepSuccess(); step.success(); return ProcessResult.SUCCESS; @@ -785,14 +787,17 @@ public int run(CliArguments arguments) { } for (Commandlet cmd : this.commandletManager.getCommandlets()) { if (cmd != firstCandidate) { - matches = applyAndRun(arguments.copy(), cmd); - if (matches) { + ValidationResult result = applyAndRun(arguments.copy(), cmd); + if (result.isValid()) { supressStepSuccess = cmd.isSuppressStepSuccess(); step.success(); return ProcessResult.SUCCESS; } } } + if (firstResult != null) { + throw new CliException(firstResult.getErrorMessage()); + } step.error("Invalid arguments: {}", current.getArgs()); } @@ -818,15 +823,15 @@ public int run(CliArguments arguments) { * @return {@code true} if the given {@link Commandlet} matched and did {@link Commandlet#run() run} successfully, {@code false} otherwise (the * {@link Commandlet} did not match and we have to try a different candidate). */ - private boolean applyAndRun(CliArguments arguments, Commandlet cmd) { + private ValidationResult applyAndRun(CliArguments arguments, Commandlet cmd) { cmd.clearProperties(); - boolean matches = apply(arguments, cmd, null); - if (matches) { - matches = cmd.validate().isValid(); + ValidationResult result = apply(arguments, cmd, null); + if (result.isValid()) { + result = cmd.validate(); } - if (matches) { + if (result.isValid()) { debug("Running commandlet {}", cmd); if (cmd.isIdeHomeRequired() && (this.ideHome == null)) { throw new CliException(getMessageIdeHomeNotFound(), ProcessResult.NO_IDE_HOME); @@ -868,7 +873,7 @@ private boolean applyAndRun(CliArguments arguments, Commandlet cmd) { } else { trace("Commandlet did not match"); } - return matches; + return result; } /** @@ -914,7 +919,7 @@ private boolean completeCommandlet(CliArguments arguments, Commandlet cmd, Compl if (cmd.isIdeHomeRequired() && (this.ideHome == null)) { return false; } else { - return apply(arguments.copy(), cmd, collector); + return apply(arguments.copy(), cmd, collector).isValid(); } } @@ -927,7 +932,7 @@ private boolean completeCommandlet(CliArguments arguments, Commandlet cmd, Compl * @return {@code true} if the given {@link Commandlet} matches to the given {@link CliArgument}(s) and those have been applied (set in the {@link Commandlet} * and {@link Commandlet#validate() validated}), {@code false} otherwise (the {@link Commandlet} did not match and we have to try a different candidate). */ - public boolean apply(CliArguments arguments, Commandlet cmd, CompletionCandidateCollector collector) { + public ValidationResult apply(CliArguments arguments, Commandlet cmd, CompletionCandidateCollector collector) { trace("Trying to match arguments to commandlet {}", cmd.getName()); CliArgument currentArgument = arguments.current(); @@ -952,7 +957,9 @@ public boolean apply(CliArguments arguments, Commandlet cmd, CompletionCandidate } if (currentProperty == null) { trace("No option or next value found"); - return false; + ValidationState state = new ValidationState(null); + state.addErrorMessage("No matching property found"); + return state; } trace("Next property candidate to match argument is {}", currentProperty); if (currentProperty == property) { @@ -969,11 +976,13 @@ public boolean apply(CliArguments arguments, Commandlet cmd, CompletionCandidate } boolean matches = currentProperty.apply(arguments, this, cmd, collector); if (!matches || currentArgument.isCompletion()) { - return false; + ValidationState state = new ValidationState(null); + state.addErrorMessage("No matching property found"); + return state; } currentArgument = arguments.current(); } - return true; + return new ValidationState(null); } @Override diff --git a/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java b/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java index 0d077697e..ed165da20 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java +++ b/cli/src/main/java/com/devonfw/tools/ide/validation/ValidationState.java @@ -23,9 +23,14 @@ public String getErrorMessage() { public void addErrorMessage(String error) { if (this.errorMessage == null) { - this.errorMessage = new StringBuilder(propertyName.length()); - this.errorMessage.append(String.format("Error in property %s:", propertyName)); - this.errorMessage.append('\n'); + if (this.propertyName == null) { + this.errorMessage = new StringBuilder(error.length() + 1); + this.errorMessage.append('\n'); + } else { + this.errorMessage = new StringBuilder(error.length() + propertyName.length() + 21); // 21 for the static text below + this.errorMessage.append(String.format("Error in property %s:", propertyName)); + this.errorMessage.append('\n'); + } } else { this.errorMessage.append('\n'); } From 32bd8b63b9a94f30d316234aae796107288872b2 Mon Sep 17 00:00:00 2001 From: Sonja Skiba Date: Mon, 9 Sep 2024 09:53:52 +0200 Subject: [PATCH 7/7] Change error message when required property is empty --- cli/src/main/java/com/devonfw/tools/ide/property/Property.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/src/main/java/com/devonfw/tools/ide/property/Property.java b/cli/src/main/java/com/devonfw/tools/ide/property/Property.java index 4fed98ef3..01740e322 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/property/Property.java +++ b/cli/src/main/java/com/devonfw/tools/ide/property/Property.java @@ -461,7 +461,7 @@ public ValidationResult validate() { ValidationState state = new ValidationState(this.getNameOrAlias()); if (this.required && (getValue() == null)) { - state.addErrorMessage("Property " + this.name + " is required but no value has been set"); + state.addErrorMessage("Value is required and cannot be empty."); return state; } if (this.validator != null) {