From 32bc958a97b2db7f22f6abaa1d6bab175a607596 Mon Sep 17 00:00:00 2001 From: Thierry Boileau Date: Wed, 15 Jul 2026 10:43:15 +0200 Subject: [PATCH 1/8] fix(QTDI-2489): Apply default value before required validation for absent keys When a new @Required + @DefaultValue field is added to an existing connector, the framework was throwing IllegalArgumentException because @Required validation fired on the raw config map before @DefaultValue was applied to absent keys. Add a guard in PayloadValidator.onParameter to skip the required-field error when a default value is declared (tcomp::ui::defaultvalue::value metadata present). Existing @Required-without-default behaviour is unchanged. Co-Authored-By: GitHub Copilot --- .../manager/reflect/ReflectionService.java | 3 +- .../manager/ReflectionServiceTest.java | 97 +++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) 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..032143c64dcd6 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 @@ -914,7 +914,8 @@ public void onParameter(final ParameterMeta meta, final JsonValue value) { } if (Boolean.parseBoolean(meta.getMetadata().get("tcomp::validation::required")) - && value == JsonValue.NULL) { + && value == JsonValue.NULL + && meta.getMetadata().get("tcomp::ui::defaultvalue::value") == null) { errors.add(MESSAGES.required(meta.getPath())); } final Map metadata = meta.getMetadata(); 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..6b9a44d7cf2d1 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 @@ -64,6 +64,7 @@ import org.talend.sdk.component.api.configuration.constraint.Required; 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 +308,76 @@ void validationRequiredList() throws NoSuchMethodException { assertThrows(IllegalArgumentException.class, () -> factory.apply(emptyMap())); } + @Test + void validationRequiredWithDefaultValueMissingKeyPasses() throws NoSuchMethodException { + // Given an existing config missing a key for a @Required + @DefaultValue field, + // validation must not throw — QTDI-2489 + final Function, Object[]> factory = getComponentFactory(RequiredWithDefaultConfig.class); + // Provide only noDefault (truly required, no default) + factory.apply(new HashMap() { + + { + put("root.noDefault", "provided"); + } + }); + } + + @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(new HashMap() { + + { + put("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 IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, + () -> factory.apply(new HashMap() { + + { + put("root.region", "us-east-1"); + // noDefault intentionally absent + } + })); + assertTrue(ex.getMessage().contains("root.noDefault"), + "Error message should mention the missing required field"); + } + + @Test + void validationRequiredWithEmptyDefaultValuePasses() throws NoSuchMethodException { + // Given a @Required + @DefaultValue("") field with an absent key, validation must not throw + final Function, Object[]> factory = getComponentFactory(RequiredWithDefaultConfig.class); + factory.apply(new HashMap() { + + { + put("root.noDefault", "provided"); + // emptyDefault intentionally absent — default is "" + } + }); + } + + @Test + void validationRequiredMultipleAbsentDefaultsPasses() throws NoSuchMethodException { + // Given multiple @Required + @DefaultValue fields all absent, no exception is thrown + final Function, Object[]> factory = getComponentFactory(RequiredWithDefaultConfig.class); + factory.apply(new HashMap() { + + { + put("root.noDefault", "provided"); + // both region and anotherField absent — each has a @DefaultValue + } + }); + } + @Test void validationRequiredListObject() throws NoSuchMethodException { final Function, Object[]> factory = getComponentFactory(RequiredListObject.class); @@ -1013,6 +1084,28 @@ 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"; + } + @EqualsAndHashCode public static class SomeIntegerConfig { @@ -1082,6 +1175,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 } From 14061d607c84ad8b727a3622411defbe3f154d15 Mon Sep 17 00:00:00 2001 From: Thierry Boileau Date: Wed, 15 Jul 2026 11:22:42 +0200 Subject: [PATCH 2/8] fix(QTDI-2489): fix Sonar S3599/S2699 in ReflectionServiceTest Replace anonymous HashMap double-brace initializers with Map.of() calls (S3599) and add assertDoesNotThrow() to test methods that lacked assertions (S2699). --- .../manager/ReflectionServiceTest.java | 39 +++---------------- 1 file changed, 6 insertions(+), 33 deletions(-) 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 6b9a44d7cf2d1..b07806823ffa6 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; @@ -314,12 +315,7 @@ void validationRequiredWithDefaultValueMissingKeyPasses() throws NoSuchMethodExc // validation must not throw — QTDI-2489 final Function, Object[]> factory = getComponentFactory(RequiredWithDefaultConfig.class); // Provide only noDefault (truly required, no default) - factory.apply(new HashMap() { - - { - put("root.noDefault", "provided"); - } - }); + assertDoesNotThrow(() -> factory.apply(Map.of("root.noDefault", "provided"))); } @Test @@ -327,12 +323,7 @@ void validationRequiredWithDefaultValueResolvedValue() throws NoSuchMethodExcept // 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(new HashMap() { - - { - put("root.noDefault", "provided"); - } - })[0]; + (RequiredWithDefaultConfig) factory.apply(Map.of("root.noDefault", "provided"))[0]; assertEquals("DEFAULT", result.region); } @@ -341,13 +332,7 @@ void validationRequiredNoDefaultValueMissingKeyFails() throws NoSuchMethodExcept // Given a @Required field with no @DefaultValue and a missing key, validation must throw final Function, Object[]> factory = getComponentFactory(RequiredWithDefaultConfig.class); final IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, - () -> factory.apply(new HashMap() { - - { - put("root.region", "us-east-1"); - // noDefault intentionally absent - } - })); + () -> factory.apply(Map.of("root.region", "us-east-1"))); assertTrue(ex.getMessage().contains("root.noDefault"), "Error message should mention the missing required field"); } @@ -356,26 +341,14 @@ void validationRequiredNoDefaultValueMissingKeyFails() throws NoSuchMethodExcept void validationRequiredWithEmptyDefaultValuePasses() throws NoSuchMethodException { // Given a @Required + @DefaultValue("") field with an absent key, validation must not throw final Function, Object[]> factory = getComponentFactory(RequiredWithDefaultConfig.class); - factory.apply(new HashMap() { - - { - put("root.noDefault", "provided"); - // emptyDefault intentionally absent — default is "" - } - }); + assertDoesNotThrow(() -> factory.apply(Map.of("root.noDefault", "provided"))); } @Test void validationRequiredMultipleAbsentDefaultsPasses() throws NoSuchMethodException { // Given multiple @Required + @DefaultValue fields all absent, no exception is thrown final Function, Object[]> factory = getComponentFactory(RequiredWithDefaultConfig.class); - factory.apply(new HashMap() { - - { - put("root.noDefault", "provided"); - // both region and anotherField absent — each has a @DefaultValue - } - }); + assertDoesNotThrow(() -> factory.apply(Map.of("root.noDefault", "provided"))); } @Test From a21aaab3d77ad3325d249af10f5613cb85b7fbcb Mon Sep 17 00:00:00 2001 From: Thierry Boileau Date: Wed, 15 Jul 2026 14:07:50 +0200 Subject: [PATCH 3/8] fix(QTDI-2489): fix Sonar S3776/S5785/S4144/S5778 in ReflectionService Extract isRequiredAndMissingWithoutDefault helper in PayloadValidator to reduce onParameter cognitive complexity (S3776). Replace three identical assertDoesNotThrow test methods with a single @ParameterizedTest using @MethodSource with three distinct input maps (S5785, S4144 x2). Extract Map.of() outside assertThrows lambda in validationRequiredNoDefaultValueMissingKeyFails to avoid multiple potentially-throwing invocations in one lambda (S5778). --- .../manager/reflect/ReflectionService.java | 11 +++-- .../manager/ReflectionServiceTest.java | 42 +++++++++---------- 2 files changed, 29 insertions(+), 24 deletions(-) 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 032143c64dcd6..b9d0d2f5ae246 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,9 +913,7 @@ public void onParameter(final ParameterMeta meta, final JsonValue value) { return; } - if (Boolean.parseBoolean(meta.getMetadata().get("tcomp::validation::required")) - && value == JsonValue.NULL - && meta.getMetadata().get("tcomp::ui::defaultvalue::value") == null) { + if (isRequiredAndMissingWithoutDefault(meta, value)) { errors.add(MESSAGES.required(meta.getPath())); } final Map metadata = meta.getMetadata(); @@ -1003,6 +1001,13 @@ public void onParameter(final ParameterMeta meta, final JsonValue value) { } } + 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 b07806823ffa6..14b24fb3ffc2e 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 @@ -57,6 +57,9 @@ 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; @@ -309,13 +312,23 @@ void validationRequiredList() throws NoSuchMethodException { assertThrows(IllegalArgumentException.class, () -> factory.apply(emptyMap())); } - @Test - void validationRequiredWithDefaultValueMissingKeyPasses() throws NoSuchMethodException { - // Given an existing config missing a key for a @Required + @DefaultValue field, - // validation must not throw — QTDI-2489 + @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); - // Provide only noDefault (truly required, no default) - assertDoesNotThrow(() -> factory.apply(Map.of("root.noDefault", "provided"))); + 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 @@ -331,26 +344,13 @@ void validationRequiredWithDefaultValueResolvedValue() throws NoSuchMethodExcept 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(Map.of("root.region", "us-east-1"))); + () -> factory.apply(config)); assertTrue(ex.getMessage().contains("root.noDefault"), "Error message should mention the missing required field"); } - @Test - void validationRequiredWithEmptyDefaultValuePasses() throws NoSuchMethodException { - // Given a @Required + @DefaultValue("") field with an absent key, validation must not throw - final Function, Object[]> factory = getComponentFactory(RequiredWithDefaultConfig.class); - assertDoesNotThrow(() -> factory.apply(Map.of("root.noDefault", "provided"))); - } - - @Test - void validationRequiredMultipleAbsentDefaultsPasses() throws NoSuchMethodException { - // Given multiple @Required + @DefaultValue fields all absent, no exception is thrown - final Function, Object[]> factory = getComponentFactory(RequiredWithDefaultConfig.class); - assertDoesNotThrow(() -> factory.apply(Map.of("root.noDefault", "provided"))); - } - @Test void validationRequiredListObject() throws NoSuchMethodException { final Function, Object[]> factory = getComponentFactory(RequiredListObject.class); From 0f0258e58e5701add5cd01123114bcad74f15fe3 Mon Sep 17 00:00:00 2001 From: Thierry Boileau Date: Wed, 15 Jul 2026 14:48:36 +0200 Subject: [PATCH 4/8] fix(QTDI-2489): decrease method complexity - Sonar comment --- .../manager/reflect/ReflectionService.java | 150 ++++++++++-------- 1 file changed, 87 insertions(+), 63 deletions(-) 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 b9d0d2f5ae246..ebca3b9bb416d 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 @@ -917,86 +917,110 @@ public void onParameter(final ParameterMeta meta, final JsonValue 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) { + if (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())); } } } From 6e5c56d96e37ad343cde03809a27d6fb6e566b5b Mon Sep 17 00:00:00 2001 From: Thierry Boileau Date: Wed, 15 Jul 2026 15:16:03 +0200 Subject: [PATCH 5/8] fix(QTDI-2489): Sonar comment --- .../runtime/manager/reflect/ReflectionService.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) 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 ebca3b9bb416d..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 @@ -941,12 +941,10 @@ private void validateRulePattern(final ParameterMeta meta, final JsonValue value private void validateRuleUniqueItems(final ParameterMeta meta, final JsonValue value, final Map metadata) { 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())); - } + 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())); } } } From 0c290e6dc4762fb94ca0e5681823154e4d5ae714 Mon Sep 17 00:00:00 2001 From: Thierry Boileau Date: Wed, 15 Jul 2026 16:10:21 +0200 Subject: [PATCH 6/8] fix(QTDI-2489): add unit tests for all 8 extracted PayloadValidator validation rules --- .../manager/ReflectionServiceTest.java | 161 ++++++++++++++++++ 1 file changed, 161 insertions(+) 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 14b24fb3ffc2e..a1e1a5f698e41 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 @@ -66,6 +66,7 @@ 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; @@ -451,6 +452,124 @@ void validationNestedListKo() throws NoSuchMethodException { () -> factory.apply(singletonMap("root.nesteds[0].value", "short"))); } + @Test + void validationMinNumberKo() throws NoSuchMethodException { + final Function, Object[]> factory = + getComponentFactory(SomeValidationConstraintsConfig.class); + assertThrows(IllegalArgumentException.class, + () -> factory.apply(Map.of("root.numMin", "3", "root.numMax", "5"))); + } + + @Test + void validationMinNumberOk() throws NoSuchMethodException { + final Function, Object[]> factory = + getComponentFactory(SomeValidationConstraintsConfig.class); + assertDoesNotThrow(() -> factory.apply(Map.of("root.numMin", "7", "root.numMax", "5"))); + } + + @Test + void validationMaxNumberKo() throws NoSuchMethodException { + final Function, Object[]> factory = + getComponentFactory(SomeValidationConstraintsConfig.class); + assertThrows(IllegalArgumentException.class, + () -> factory.apply(Map.of("root.numMin", "7", "root.numMax", "15"))); + } + + @Test + void validationMaxNumberOk() throws NoSuchMethodException { + final Function, Object[]> factory = + getComponentFactory(SomeValidationConstraintsConfig.class); + assertDoesNotThrow(() -> factory.apply(Map.of("root.numMin", "7", "root.numMax", "5"))); + } + + @Test + void validationMinStringLengthKo() throws NoSuchMethodException { + final Function, Object[]> factory = + getComponentFactory(SomeValidationConstraintsConfig.class); + assertThrows(IllegalArgumentException.class, + () -> factory.apply(Map.of("root.strMin", "ab", "root.strMax", "hi"))); + } + + @Test + void validationMinStringLengthOk() throws NoSuchMethodException { + final Function, Object[]> factory = + getComponentFactory(SomeValidationConstraintsConfig.class); + assertDoesNotThrow(() -> factory.apply(Map.of("root.strMin", "hello", "root.strMax", "hi"))); + } + + @Test + void validationMaxStringLengthKo() throws NoSuchMethodException { + final Function, Object[]> factory = + getComponentFactory(SomeValidationConstraintsConfig.class); + assertThrows(IllegalArgumentException.class, + () -> factory.apply(Map.of("root.strMin", "hello", "root.strMax", "toolong"))); + } + + @Test + void validationMaxStringLengthOk() throws NoSuchMethodException { + final Function, Object[]> factory = + getComponentFactory(SomeValidationConstraintsConfig.class); + assertDoesNotThrow(() -> factory.apply(Map.of("root.strMin", "hello", "root.strMax", "hi"))); + } + + @Test + void validationMinItemsKo() throws NoSuchMethodException { + final Function, Object[]> factory = + getComponentFactory(SomeValidationConstraintsConfig.class); + assertThrows(IllegalArgumentException.class, + () -> factory.apply(Map.of("root.listMin[0]", "a"))); + } + + @Test + void validationMinItemsOk() throws NoSuchMethodException { + final Function, Object[]> factory = + getComponentFactory(SomeValidationConstraintsConfig.class); + assertDoesNotThrow(() -> factory.apply(Map.of("root.listMin[0]", "a", "root.listMin[1]", "b"))); + } + + @Test + void validationMaxItemsKo() throws NoSuchMethodException { + final Function, Object[]> factory = + getComponentFactory(SomeValidationConstraintsConfig.class); + assertThrows(IllegalArgumentException.class, () -> factory + .apply(Map.of("root.listMax[0]", "a", "root.listMax[1]", "b", "root.listMax[2]", "c", + "root.listMax[3]", "d"))); + } + + @Test + void validationMaxItemsOk() throws NoSuchMethodException { + final Function, Object[]> factory = + getComponentFactory(SomeValidationConstraintsConfig.class); + assertDoesNotThrow( + () -> factory.apply(Map.of("root.listMax[0]", "a", "root.listMax[1]", "b"))); + } + + @Test + void validationUniqueItemsKo() throws NoSuchMethodException { + final Function, Object[]> factory = getComponentFactory(SomeUniquesConfig.class); + assertThrows(IllegalArgumentException.class, + () -> factory.apply(Map.of("root.tags[0]", "dup", "root.tags[1]", "dup"))); + } + + @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); + assertThrows(IllegalArgumentException.class, + () -> factory.apply(Map.of("root.regex", "UPPERCASE"))); + } + + @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); @@ -1079,6 +1198,40 @@ public static class RequiredWithDefaultConfig { 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 { @@ -1163,6 +1316,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 { From 8b4da727ad48e7501c1d473b6324322af658a165 Mon Sep 17 00:00:00 2001 From: Thierry Boileau Date: Wed, 15 Jul 2026 17:12:23 +0200 Subject: [PATCH 7/8] fix(QTDI-2489): replace S5785/S4144 test issues with @ParameterizedTest Sonar R4: Replace 10 individual test methods with 2 @ParameterizedTest methods and remove 2 S4144 duplicate methods in ReflectionServiceTest. - S5785: replace 6 Ko + 4 Ok individual tests with @ParameterizedTest + @MethodSource - S4144: remove validationMaxNumberOk and validationMaxStringLengthOk (identical to Min variants) - Net: 12 @Test removed, 2 @ParameterizedTest added (10 invocations preserved) - Build: component-runtime-manager 64/64 GREEN --- .../manager/ReflectionServiceTest.java | 103 ++++-------------- 1 file changed, 23 insertions(+), 80 deletions(-) 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 a1e1a5f698e41..b2fbd1636233c 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 @@ -452,96 +452,39 @@ void validationNestedListKo() throws NoSuchMethodException { () -> factory.apply(singletonMap("root.nesteds[0].value", "short"))); } - @Test - void validationMinNumberKo() throws NoSuchMethodException { - final Function, Object[]> factory = - getComponentFactory(SomeValidationConstraintsConfig.class); - assertThrows(IllegalArgumentException.class, - () -> factory.apply(Map.of("root.numMin", "3", "root.numMax", "5"))); - } - - @Test - void validationMinNumberOk() throws NoSuchMethodException { - final Function, Object[]> factory = - getComponentFactory(SomeValidationConstraintsConfig.class); - assertDoesNotThrow(() -> factory.apply(Map.of("root.numMin", "7", "root.numMax", "5"))); - } - - @Test - void validationMaxNumberKo() throws NoSuchMethodException { - final Function, Object[]> factory = - getComponentFactory(SomeValidationConstraintsConfig.class); - assertThrows(IllegalArgumentException.class, - () -> factory.apply(Map.of("root.numMin", "7", "root.numMax", "15"))); - } - - @Test - void validationMaxNumberOk() throws NoSuchMethodException { - final Function, Object[]> factory = - getComponentFactory(SomeValidationConstraintsConfig.class); - assertDoesNotThrow(() -> factory.apply(Map.of("root.numMin", "7", "root.numMax", "5"))); - } - - @Test - void validationMinStringLengthKo() throws NoSuchMethodException { - final Function, Object[]> factory = - getComponentFactory(SomeValidationConstraintsConfig.class); - assertThrows(IllegalArgumentException.class, - () -> factory.apply(Map.of("root.strMin", "ab", "root.strMax", "hi"))); - } - - @Test - void validationMinStringLengthOk() throws NoSuchMethodException { - final Function, Object[]> factory = - getComponentFactory(SomeValidationConstraintsConfig.class); - assertDoesNotThrow(() -> factory.apply(Map.of("root.strMin", "hello", "root.strMax", "hi"))); - } - - @Test - void validationMaxStringLengthKo() throws NoSuchMethodException { - final Function, Object[]> factory = - getComponentFactory(SomeValidationConstraintsConfig.class); - assertThrows(IllegalArgumentException.class, - () -> factory.apply(Map.of("root.strMin", "hello", "root.strMax", "toolong"))); - } - - @Test - void validationMaxStringLengthOk() throws NoSuchMethodException { - final Function, Object[]> factory = - getComponentFactory(SomeValidationConstraintsConfig.class); - assertDoesNotThrow(() -> factory.apply(Map.of("root.strMin", "hello", "root.strMax", "hi"))); - } - - @Test - void validationMinItemsKo() throws NoSuchMethodException { - final Function, Object[]> factory = - getComponentFactory(SomeValidationConstraintsConfig.class); - assertThrows(IllegalArgumentException.class, - () -> factory.apply(Map.of("root.listMin[0]", "a"))); + 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")); } - @Test - void validationMinItemsOk() throws NoSuchMethodException { + @ParameterizedTest + @MethodSource("validationConstraintsKoCases") + void validationConstraintsKo(final Map params) throws NoSuchMethodException { final Function, Object[]> factory = getComponentFactory(SomeValidationConstraintsConfig.class); - assertDoesNotThrow(() -> factory.apply(Map.of("root.listMin[0]", "a", "root.listMin[1]", "b"))); + assertThrows(IllegalArgumentException.class, () -> factory.apply(params)); } - @Test - void validationMaxItemsKo() throws NoSuchMethodException { - final Function, Object[]> factory = - getComponentFactory(SomeValidationConstraintsConfig.class); - assertThrows(IllegalArgumentException.class, () -> factory - .apply(Map.of("root.listMax[0]", "a", "root.listMax[1]", "b", "root.listMax[2]", "c", - "root.listMax[3]", "d"))); + 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")); } - @Test - void validationMaxItemsOk() throws NoSuchMethodException { + @ParameterizedTest + @MethodSource("validationConstraintsOkCases") + void validationConstraintsOk(final Map params) throws NoSuchMethodException { final Function, Object[]> factory = getComponentFactory(SomeValidationConstraintsConfig.class); - assertDoesNotThrow( - () -> factory.apply(Map.of("root.listMax[0]", "a", "root.listMax[1]", "b"))); + assertDoesNotThrow(() -> factory.apply(params)); } @Test From f32dff12cb5b83720afc63ce2dba9c1b0649134e Mon Sep 17 00:00:00 2001 From: Thierry Boileau Date: Wed, 15 Jul 2026 17:52:03 +0200 Subject: [PATCH 8/8] fix(QTDI-2489): Sonar comment --- .../component/runtime/manager/ReflectionServiceTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 b2fbd1636233c..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 @@ -490,8 +490,8 @@ void validationConstraintsOk(final Map params) throws NoSuchMeth @Test void validationUniqueItemsKo() throws NoSuchMethodException { final Function, Object[]> factory = getComponentFactory(SomeUniquesConfig.class); - assertThrows(IllegalArgumentException.class, - () -> factory.apply(Map.of("root.tags[0]", "dup", "root.tags[1]", "dup"))); + final Map map = Map.of("root.tags[0]", "dup", "root.tags[1]", "dup"); + assertThrows(IllegalArgumentException.class, () -> factory.apply(map)); } @Test @@ -503,8 +503,8 @@ void validationUniqueItemsOk() throws NoSuchMethodException { @Test void validationPatternKo() throws NoSuchMethodException { final Function, Object[]> factory = getComponentFactory(SomeConfig5.class); - assertThrows(IllegalArgumentException.class, - () -> factory.apply(Map.of("root.regex", "UPPERCASE"))); + final Map map = Map.of("root.regex", "UPPERCASE"); + assertThrows(IllegalArgumentException.class, () -> factory.apply(map)); } @Test