diff --git a/component-runtime-manager/src/main/java/org/talend/sdk/component/runtime/manager/reflect/ReflectionService.java b/component-runtime-manager/src/main/java/org/talend/sdk/component/runtime/manager/reflect/ReflectionService.java index d2d3cbd8d9471..e26b005860194 100644 --- a/component-runtime-manager/src/main/java/org/talend/sdk/component/runtime/manager/reflect/ReflectionService.java +++ b/component-runtime-manager/src/main/java/org/talend/sdk/component/runtime/manager/reflect/ReflectionService.java @@ -913,95 +913,123 @@ public void onParameter(final ParameterMeta meta, final JsonValue value) { return; } - if (Boolean.parseBoolean(meta.getMetadata().get("tcomp::validation::required")) - && value == JsonValue.NULL) { + if (isRequiredAndMissingWithoutDefault(meta, value)) { errors.add(MESSAGES.required(meta.getPath())); } final Map metadata = meta.getMetadata(); - { - final String min = metadata.get("tcomp::validation::min"); - if (min != null) { - final double bound = Double.parseDouble(min); - if (value.getValueType() == JsonValue.ValueType.NUMBER - && ((JsonNumber) value).doubleValue() < bound) { - errors.add(MESSAGES.min(meta.getPath(), bound, ((JsonNumber) value).doubleValue())); - } + validateRuleMin(meta, value, metadata); + validateRuleMax(meta, value, metadata); + validateRuleMinLength(meta, value, metadata); + validateRuleMaxLength(meta, value, metadata); + validateRuleMinItems(meta, value, metadata); + validateRuleMaxItems(meta, value, metadata); + validateRuleUniqueItems(meta, value, metadata); + validateRulePattern(meta, value, metadata); + } + + private void validateRulePattern(final ParameterMeta meta, final JsonValue value, + final Map metadata) { + final String pattern = metadata.get("tcomp::validation::pattern"); + if (pattern != null && value.getValueType() == JsonValue.ValueType.STRING) { + final String val = ((JsonString) value).getString(); + if (!new JavascriptRegex(pattern).test(val)) { + errors.add(MESSAGES.pattern(meta.getPath(), pattern)); } } - { - final String max = metadata.get("tcomp::validation::max"); - if (max != null) { - final double bound = Double.parseDouble(max); - if (value.getValueType() == JsonValue.ValueType.NUMBER - && ((JsonNumber) value).doubleValue() > bound) { - errors.add(MESSAGES.max(meta.getPath(), bound, ((JsonNumber) value).doubleValue())); - } + } + + private void validateRuleUniqueItems(final ParameterMeta meta, final JsonValue value, + final Map metadata) { + final String unique = metadata.get("tcomp::validation::uniqueItems"); + if (unique != null && value.getValueType() == JsonValue.ValueType.ARRAY) { + final JsonArray array = value.asJsonArray(); + if (new HashSet<>(array).size() != array.size()) { + errors.add(MESSAGES.uniqueItems(meta.getPath())); } } - { - final String min = metadata.get("tcomp::validation::minLength"); - if (min != null) { - final double bound = Double.parseDouble(min); - if (value.getValueType() == JsonValue.ValueType.STRING) { - final String val = ((JsonString) value).getString(); - if (val.length() < bound) { - errors.add(MESSAGES.minLength(meta.getPath(), bound, val.length())); - } - } + } + + private void validateRuleMaxItems(final ParameterMeta meta, final JsonValue value, + final Map metadata) { + final String max = metadata.get("tcomp::validation::maxItems"); + if (max != null) { + final double bound = Double.parseDouble(max); + if (value.getValueType() == JsonValue.ValueType.ARRAY && value.asJsonArray().size() > bound) { + errors.add(MESSAGES.maxItems(meta.getPath(), bound, value.asJsonArray().size())); } } - { - final String max = metadata.get("tcomp::validation::maxLength"); - if (max != null) { - final double bound = Double.parseDouble(max); - if (value.getValueType() == JsonValue.ValueType.STRING) { - final String val = ((JsonString) value).getString(); - if (val.length() > bound) { - errors.add(MESSAGES.maxLength(meta.getPath(), bound, val.length())); - } - } + } + + private void validateRuleMinItems(final ParameterMeta meta, final JsonValue value, + final Map metadata) { + final String min = metadata.get("tcomp::validation::minItems"); + if (min != null) { + final double bound = Double.parseDouble(min); + if (value.getValueType() == JsonValue.ValueType.ARRAY && value.asJsonArray().size() < bound) { + errors.add(MESSAGES.minItems(meta.getPath(), bound, value.asJsonArray().size())); } } - { - final String min = metadata.get("tcomp::validation::minItems"); - if (min != null) { - final double bound = Double.parseDouble(min); - if (value.getValueType() == JsonValue.ValueType.ARRAY && value.asJsonArray().size() < bound) { - errors.add(MESSAGES.minItems(meta.getPath(), bound, value.asJsonArray().size())); + } + + private void validateRuleMaxLength(final ParameterMeta meta, final JsonValue value, + final Map metadata) { + final String max = metadata.get("tcomp::validation::maxLength"); + if (max != null) { + final double bound = Double.parseDouble(max); + if (value.getValueType() == JsonValue.ValueType.STRING) { + final String val = ((JsonString) value).getString(); + if (val.length() > bound) { + errors.add(MESSAGES.maxLength(meta.getPath(), bound, val.length())); } } } - { - final String max = metadata.get("tcomp::validation::maxItems"); - if (max != null) { - final double bound = Double.parseDouble(max); - if (value.getValueType() == JsonValue.ValueType.ARRAY && value.asJsonArray().size() > bound) { - errors.add(MESSAGES.maxItems(meta.getPath(), bound, value.asJsonArray().size())); + } + + private void validateRuleMinLength(final ParameterMeta meta, final JsonValue value, + final Map metadata) { + final String min = metadata.get("tcomp::validation::minLength"); + if (min != null) { + final double bound = Double.parseDouble(min); + if (value.getValueType() == JsonValue.ValueType.STRING) { + final String val = ((JsonString) value).getString(); + if (val.length() < bound) { + errors.add(MESSAGES.minLength(meta.getPath(), bound, val.length())); } } } - { - final String unique = metadata.get("tcomp::validation::uniqueItems"); - if (unique != null) { - if (value.getValueType() == JsonValue.ValueType.ARRAY) { - final JsonArray array = value.asJsonArray(); - if (new HashSet<>(array).size() != array.size()) { - errors.add(MESSAGES.uniqueItems(meta.getPath())); - } - } + } + + private void validateRuleMax(final ParameterMeta meta, final JsonValue value, + final Map metadata) { + final String max = metadata.get("tcomp::validation::max"); + if (max != null) { + final double bound = Double.parseDouble(max); + if (value.getValueType() == JsonValue.ValueType.NUMBER + && ((JsonNumber) value).doubleValue() > bound) { + errors.add(MESSAGES.max(meta.getPath(), bound, ((JsonNumber) value).doubleValue())); } } - { - final String pattern = metadata.get("tcomp::validation::pattern"); - if (pattern != null && value.getValueType() == JsonValue.ValueType.STRING) { - final String val = ((JsonString) value).getString(); - if (!new JavascriptRegex(pattern).test((CharSequence) val)) { - errors.add(MESSAGES.pattern(meta.getPath(), pattern)); - } + } + + private void validateRuleMin(final ParameterMeta meta, final JsonValue value, + final Map metadata) { + final String min = metadata.get("tcomp::validation::min"); + if (min != null) { + final double bound = Double.parseDouble(min); + if (value.getValueType() == JsonValue.ValueType.NUMBER + && ((JsonNumber) value).doubleValue() < bound) { + errors.add(MESSAGES.min(meta.getPath(), bound, ((JsonNumber) value).doubleValue())); } } } + private static boolean isRequiredAndMissingWithoutDefault(final ParameterMeta meta, + final JsonValue value) { + return Boolean.parseBoolean(meta.getMetadata().get("tcomp::validation::required")) + && value == JsonValue.NULL + && meta.getMetadata().get("tcomp::ui::defaultvalue::value") == null; + } + private void throwIfFailed() { if (!errors.isEmpty()) { throw new IllegalArgumentException("- " + String.join("\n- ", errors)); diff --git a/component-runtime-manager/src/test/java/org/talend/sdk/component/runtime/manager/ReflectionServiceTest.java b/component-runtime-manager/src/test/java/org/talend/sdk/component/runtime/manager/ReflectionServiceTest.java index 31aa7a8885aeb..9a440bd213df3 100644 --- a/component-runtime-manager/src/test/java/org/talend/sdk/component/runtime/manager/ReflectionServiceTest.java +++ b/component-runtime-manager/src/test/java/org/talend/sdk/component/runtime/manager/ReflectionServiceTest.java @@ -22,6 +22,7 @@ import static java.util.Collections.singletonMap; import static java.util.stream.Collectors.toList; import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -56,14 +57,19 @@ import org.apache.xbean.propertyeditor.AbstractConverter; import org.apache.xbean.propertyeditor.PropertyEditorRegistry; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import org.talend.sdk.component.api.configuration.Option; import org.talend.sdk.component.api.configuration.condition.ActiveIf; import org.talend.sdk.component.api.configuration.constraint.Max; import org.talend.sdk.component.api.configuration.constraint.Min; import org.talend.sdk.component.api.configuration.constraint.Pattern; import org.talend.sdk.component.api.configuration.constraint.Required; +import org.talend.sdk.component.api.configuration.constraint.Uniques; import org.talend.sdk.component.api.configuration.type.DataSet; import org.talend.sdk.component.api.configuration.type.DataStore; +import org.talend.sdk.component.api.configuration.ui.DefaultValue; import org.talend.sdk.component.api.configuration.ui.widget.DateTime; import org.talend.sdk.component.api.service.cache.LocalCache; import org.talend.sdk.component.api.service.configuration.Configuration; @@ -307,6 +313,45 @@ void validationRequiredList() throws NoSuchMethodException { assertThrows(IllegalArgumentException.class, () -> factory.apply(emptyMap())); } + @ParameterizedTest + @MethodSource("absentDefaultValueCases") + void validationRequiredWithAbsentDefaultValuePasses(final Map config) + throws NoSuchMethodException { + // @Required + @DefaultValue field absent from config must not throw — QTDI-2489 + final Function, Object[]> factory = getComponentFactory(RequiredWithDefaultConfig.class); + assertDoesNotThrow((Executable) () -> factory.apply(config)); + } + + static Stream> absentDefaultValueCases() { + return Stream.of( + // all @DefaultValue fields absent + Map.of("root.noDefault", "provided"), + // region provided, emptyDefault (@DefaultValue("")) and anotherField absent + Map.of("root.noDefault", "provided", "root.region", "us-east-1"), + // emptyDefault explicitly provided, region and anotherField absent + Map.of("root.noDefault", "provided", "root.emptyDefault", "custom")); + } + + @Test + void validationRequiredWithDefaultValueResolvedValue() throws NoSuchMethodException { + // Given a config missing the 'region' key, the resolved field value equals the declared default + final Function, Object[]> factory = getComponentFactory(RequiredWithDefaultConfig.class); + final RequiredWithDefaultConfig result = + (RequiredWithDefaultConfig) factory.apply(Map.of("root.noDefault", "provided"))[0]; + assertEquals("DEFAULT", result.region); + } + + @Test + void validationRequiredNoDefaultValueMissingKeyFails() throws NoSuchMethodException { + // Given a @Required field with no @DefaultValue and a missing key, validation must throw + final Function, Object[]> factory = getComponentFactory(RequiredWithDefaultConfig.class); + final Map config = Map.of("root.region", "us-east-1"); + final IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, + () -> factory.apply(config)); + assertTrue(ex.getMessage().contains("root.noDefault"), + "Error message should mention the missing required field"); + } + @Test void validationRequiredListObject() throws NoSuchMethodException { final Function, Object[]> factory = getComponentFactory(RequiredListObject.class); @@ -407,6 +452,67 @@ void validationNestedListKo() throws NoSuchMethodException { () -> factory.apply(singletonMap("root.nesteds[0].value", "short"))); } + static Stream> validationConstraintsKoCases() { + return Stream.of( + Map.of("root.numMin", "3", "root.numMax", "5"), + Map.of("root.numMin", "7", "root.numMax", "15"), + Map.of("root.strMin", "ab", "root.strMax", "hi"), + Map.of("root.strMin", "hello", "root.strMax", "toolong"), + Map.of("root.listMin[0]", "a"), + Map.of("root.listMax[0]", "a", "root.listMax[1]", "b", "root.listMax[2]", "c", + "root.listMax[3]", "d")); + } + + @ParameterizedTest + @MethodSource("validationConstraintsKoCases") + void validationConstraintsKo(final Map params) throws NoSuchMethodException { + final Function, Object[]> factory = + getComponentFactory(SomeValidationConstraintsConfig.class); + assertThrows(IllegalArgumentException.class, () -> factory.apply(params)); + } + + static Stream> validationConstraintsOkCases() { + return Stream.of( + Map.of("root.numMin", "7", "root.numMax", "5"), + Map.of("root.strMin", "hello", "root.strMax", "hi"), + Map.of("root.listMin[0]", "a", "root.listMin[1]", "b"), + Map.of("root.listMax[0]", "a", "root.listMax[1]", "b")); + } + + @ParameterizedTest + @MethodSource("validationConstraintsOkCases") + void validationConstraintsOk(final Map params) throws NoSuchMethodException { + final Function, Object[]> factory = + getComponentFactory(SomeValidationConstraintsConfig.class); + assertDoesNotThrow(() -> factory.apply(params)); + } + + @Test + void validationUniqueItemsKo() throws NoSuchMethodException { + final Function, Object[]> factory = getComponentFactory(SomeUniquesConfig.class); + final Map map = Map.of("root.tags[0]", "dup", "root.tags[1]", "dup"); + assertThrows(IllegalArgumentException.class, () -> factory.apply(map)); + } + + @Test + void validationUniqueItemsOk() throws NoSuchMethodException { + final Function, Object[]> factory = getComponentFactory(SomeUniquesConfig.class); + assertDoesNotThrow(() -> factory.apply(Map.of("root.tags[0]", "a", "root.tags[1]", "b"))); + } + + @Test + void validationPatternKo() throws NoSuchMethodException { + final Function, Object[]> factory = getComponentFactory(SomeConfig5.class); + final Map map = Map.of("root.regex", "UPPERCASE"); + assertThrows(IllegalArgumentException.class, () -> factory.apply(map)); + } + + @Test + void validationPatternOk() throws NoSuchMethodException { + final Function, Object[]> factory = getComponentFactory(SomeConfig5.class); + assertDoesNotThrow(() -> factory.apply(Map.of("root.regex", "lowercase"))); + } + @Test void validationIntegerConstraints() throws NoSuchMethodException { final Function, Object[]> factory = getComponentFactory(SomeIntegerConfig.class); @@ -1013,6 +1119,62 @@ public static class RequiredListObject { private List list; } + public static class RequiredWithDefaultConfig { + + @Option + @Required + @DefaultValue("DEFAULT") + private String region = "DEFAULT"; + + @Option + @Required + @DefaultValue("") + private String emptyDefault = ""; + + @Option + @Required + private String noDefault; + + @Option + @Required + @DefaultValue("X") + private String anotherField = "X"; + } + + public static class SomeValidationConstraintsConfig { + + @Option + @Min(5) + private int numMin; + + @Option + @Max(10) + private int numMax; + + @Option + @Min(3) + private String strMin; + + @Option + @Max(5) + private String strMax; + + @Option + @Min(2) + private List listMin; + + @Option + @Max(3) + private List listMax; + } + + public static class SomeUniquesConfig { + + @Option + @Uniques + private List tags; + } + @EqualsAndHashCode public static class SomeIntegerConfig { @@ -1082,6 +1244,10 @@ public FakeComponent(@Option("root") final RequiredListObject root) { // no-op } + public FakeComponent(@Option("root") final RequiredWithDefaultConfig root) { + // no-op + } + public FakeComponent(@Option("root") final ConfigWithDate root) { // no-op } @@ -1093,6 +1259,14 @@ public FakeComponent(@Option("root") final JsonObject root) { public FakeComponent(@Option("root") final SomeIntegerConfig root) { // mo-op } + + public FakeComponent(@Option("root") final SomeValidationConstraintsConfig root) { + // no-op + } + + public FakeComponent(@Option("root") final SomeUniquesConfig root) { + // no-op + } } public static class ConfigWithDate {