From 4c0a37d5c7f0ebcc0de0d01c7cc6c4eb2ce5198d Mon Sep 17 00:00:00 2001 From: yuri1969 <1969yuri1969@gmail.com> Date: Wed, 29 Mar 2023 20:52:22 +0200 Subject: [PATCH 1/3] feat(core): input validator step 1 --- .../io/kestra/core/models/flows/Input.java | 32 ++++++++++- .../io/kestra/core/runners/RunnerUtils.java | 16 ++++++ .../core/validations/InputValidation.java | 11 ++++ .../io/kestra/core/validations/Regex.java | 11 ++++ .../core/validations/ValidationFactory.java | 40 ++++++++++++- .../io/kestra/core/runners/InputsTest.java | 56 ++++++++++++++++++- .../core/serializers/YamlFlowParserTest.java | 5 +- .../io/kestra/core/validations/InputTest.java | 37 ++++++++++++ .../io/kestra/core/validations/RegexTest.java | 39 +++++++++++++ .../invalids/inputs-bad-validator-syntax.yaml | 10 ++++ .../test/resources/flows/valids/inputs.yaml | 4 ++ 11 files changed, 255 insertions(+), 6 deletions(-) create mode 100644 core/src/main/java/io/kestra/core/validations/InputValidation.java create mode 100644 core/src/main/java/io/kestra/core/validations/Regex.java create mode 100644 core/src/test/java/io/kestra/core/validations/InputTest.java create mode 100644 core/src/test/java/io/kestra/core/validations/RegexTest.java create mode 100644 core/src/test/resources/flows/invalids/inputs-bad-validator-syntax.yaml diff --git a/core/src/main/java/io/kestra/core/models/flows/Input.java b/core/src/main/java/io/kestra/core/models/flows/Input.java index 78728c03d1a..4752ef68e79 100644 --- a/core/src/main/java/io/kestra/core/models/flows/Input.java +++ b/core/src/main/java/io/kestra/core/models/flows/Input.java @@ -1,7 +1,11 @@ package io.kestra.core.models.flows; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; +import io.kestra.core.validations.InputValidation; +import io.kestra.core.validations.Regex; import io.micronaut.core.annotation.Introspected; +import io.swagger.v3.oas.annotations.media.Schema; import lombok.Builder; import lombok.Getter; import lombok.NoArgsConstructor; @@ -17,6 +21,7 @@ @NoArgsConstructor @Introspected @JsonInclude(JsonInclude.Include.NON_DEFAULT) +@InputValidation public class Input { @NotNull @NotBlank @@ -35,9 +40,28 @@ public class Input { String defaults; + @Schema( + title = "Regular expression validating the value." + ) + @Regex + String validator; + + @JsonIgnore + public boolean canBeValidated() { + if (type == null) { + return false; + } + return type.canBeValidated(); + } + @Introspected public enum Type { - STRING, + STRING() { + @Override + public boolean canBeValidated() { + return true; + } + }, INT, FLOAT, BOOLEAN, @@ -47,6 +71,10 @@ public enum Type { DURATION, FILE, JSON, - URI, + URI; + + public boolean canBeValidated() { + return false; + } } } diff --git a/core/src/main/java/io/kestra/core/runners/RunnerUtils.java b/core/src/main/java/io/kestra/core/runners/RunnerUtils.java index eefb352c732..5a89b64806d 100644 --- a/core/src/main/java/io/kestra/core/runners/RunnerUtils.java +++ b/core/src/main/java/io/kestra/core/runners/RunnerUtils.java @@ -39,6 +39,7 @@ import java.util.function.Predicate; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; import java.util.stream.Collectors; @Singleton @@ -120,6 +121,7 @@ public Map typedInputs(Flow flow, Execution execution, Map( input.getName(), current @@ -234,6 +236,20 @@ public Map typedInputs(Flow flow, Execution execution, Map handleNestedInputs(Map inputs) { Map result = new TreeMap<>(); diff --git a/core/src/main/java/io/kestra/core/validations/InputValidation.java b/core/src/main/java/io/kestra/core/validations/InputValidation.java new file mode 100644 index 00000000000..9b893048d94 --- /dev/null +++ b/core/src/main/java/io/kestra/core/validations/InputValidation.java @@ -0,0 +1,11 @@ +package io.kestra.core.validations; + +import javax.validation.Constraint; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@Retention(RetentionPolicy.RUNTIME) +@Constraint(validatedBy = { }) +public @interface InputValidation { + String message() default "invalid Input"; +} diff --git a/core/src/main/java/io/kestra/core/validations/Regex.java b/core/src/main/java/io/kestra/core/validations/Regex.java new file mode 100644 index 00000000000..c59dd2cc927 --- /dev/null +++ b/core/src/main/java/io/kestra/core/validations/Regex.java @@ -0,0 +1,11 @@ +package io.kestra.core.validations; + +import javax.validation.Constraint; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@Retention(RetentionPolicy.RUNTIME) +@Constraint(validatedBy = { }) +public @interface Regex { + String message() default "invalid pattern ({validatedValue})"; +} diff --git a/core/src/main/java/io/kestra/core/validations/ValidationFactory.java b/core/src/main/java/io/kestra/core/validations/ValidationFactory.java index 694f627ed34..34743750847 100644 --- a/core/src/main/java/io/kestra/core/validations/ValidationFactory.java +++ b/core/src/main/java/io/kestra/core/validations/ValidationFactory.java @@ -3,6 +3,7 @@ import com.cronutils.model.Cron; import com.fasterxml.jackson.databind.ObjectMapper; import io.kestra.core.models.flows.Flow; +import io.kestra.core.models.flows.Input; import io.kestra.core.models.tasks.Task; import io.kestra.core.tasks.flows.Switch; import io.micronaut.context.annotation.Factory; @@ -12,6 +13,7 @@ import java.io.IOException; import java.text.SimpleDateFormat; import java.util.*; +import java.util.regex.PatternSyntaxException; import java.util.stream.Collectors; import javax.validation.ConstraintViolation; @@ -134,7 +136,7 @@ ConstraintValidator flowValidation() { .filter(entry -> Collections.frequency(ids, entry) > 1).collect(Collectors.toList()); if (duplicates.size() > 0) { - violations.add("Duplicate task id with name [" + String.join(", ", duplicates) + "]"); + violations.add("Duplicate task id with name [" + String.join(", ", duplicates) + "]"); } if (violations.size() > 0) { @@ -145,5 +147,41 @@ ConstraintValidator flowValidation() { } }; } + + @Singleton + ConstraintValidator inputValidation() { + return (value, annotationMetadata, context) -> { + if (value == null) { + return true; + } + + if (value.getValidator() != null && !value.canBeValidated()) { + context.messageTemplate( + "Invalid Input: Validator defined at [" + value.getName() + "] is not allowed for type [" + value.getType() + "]" + ); + return false; + } + + return true; + }; + } + + @Singleton + ConstraintValidator patternValidator() { + return (value, annotationMetadata, context) -> { + if (value == null) { + return true; + } + + try { + java.util.regex.Pattern.compile(value); + } catch(PatternSyntaxException e) { + context.messageTemplate("invalid pattern [" + value + "]"); + return false; + } + + return true; + }; + } } diff --git a/core/src/test/java/io/kestra/core/runners/InputsTest.java b/core/src/test/java/io/kestra/core/runners/InputsTest.java index e5fcb37c742..26ece7cb3c5 100644 --- a/core/src/test/java/io/kestra/core/runners/InputsTest.java +++ b/core/src/test/java/io/kestra/core/runners/InputsTest.java @@ -3,6 +3,9 @@ import com.google.common.collect.ImmutableMap; import com.google.common.io.CharStreams; import io.kestra.core.exceptions.MissingRequiredInput; +import io.kestra.core.models.flows.Flow; +import io.kestra.core.serializers.YamlFlowParser; +import io.kestra.core.utils.TestsUtils; import org.junit.jupiter.api.Test; import io.kestra.core.models.executions.Execution; import io.kestra.core.models.flows.State; @@ -10,12 +13,15 @@ import io.kestra.core.storages.StorageInterface; import jakarta.inject.Inject; + +import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.net.URI; import java.net.URISyntaxException; +import java.net.URL; import java.time.Duration; import java.time.Instant; import java.time.LocalDate; @@ -46,6 +52,7 @@ public class InputsTest extends AbstractMemoryRunnerTest { .put("nested.string", "a string") .put("nested.more.int", "123") .put("nested.bool", "true") + .put("validatedString", "A123") .build(); @Inject @@ -54,9 +61,16 @@ public class InputsTest extends AbstractMemoryRunnerTest { @Inject private StorageInterface storageInterface; + @Inject + private YamlFlowParser yamlFlowParser; + private Map typedInputs(Map map) { + return typedInputs(map, flowRepository.findById("io.kestra.tests", "inputs").get()); + } + + private Map typedInputs(Map map, Flow flow) { return runnerUtils.typedInputs( - flowRepository.findById("io.kestra.tests", "inputs").get(), + flow, Execution.builder() .id("test") .namespace("test") @@ -170,6 +184,37 @@ void inputUri() { assertThat(typeds.get("uri"), is("https://www.google.com")); } + @Test + void inputValidatedString() { + Map typeds = typedInputs(inputs); + assertThat(typeds.get("validatedString"), is("A123")); + } + + @Test + void inputValidatedStringBadValue() { + HashMap map = new HashMap<>(inputs); + map.put("validatedString", "foo"); + + MissingRequiredInput e = assertThrows(MissingRequiredInput.class, () -> { + Map typeds = typedInputs(map); + }); + + assertThat(e.getMessage(), containsString("Invalid format for ")); + } + + @Test + void inputValidatedStringBadSyntax() { + final Flow flow = parse("flows/invalids/inputs-bad-validator-syntax.yaml"); + final HashMap map = new HashMap<>(inputs); + map.put("badValidatorSyntax", "foo"); + + final MissingRequiredInput e = assertThrows(MissingRequiredInput.class, () -> { + typedInputs(map, flow); + }); + + assertThat(e.getMessage(), containsString("Invalid validator syntax")); + } + @Test void inputFailed() { HashMap map = new HashMap<>(inputs); @@ -190,4 +235,13 @@ void inputNested() { assertThat(((Map)typeds.get("nested")).get("bool"), is(true)); assertThat(((Map)((Map)typeds.get("nested")).get("more")).get("int"), is(123)); } + + private Flow parse(String path) { + URL resource = TestsUtils.class.getClassLoader().getResource(path); + assert resource != null; + + File file = new File(resource.getFile()); + + return yamlFlowParser.parse(file, Flow.class); + } } diff --git a/core/src/test/java/io/kestra/core/serializers/YamlFlowParserTest.java b/core/src/test/java/io/kestra/core/serializers/YamlFlowParserTest.java index b962baf4606..95471bf78a9 100644 --- a/core/src/test/java/io/kestra/core/serializers/YamlFlowParserTest.java +++ b/core/src/test/java/io/kestra/core/serializers/YamlFlowParserTest.java @@ -120,10 +120,11 @@ void inputsFailed() { void inputs() { Flow flow = this.parse("flows/valids/inputs.yaml"); - assertThat(flow.getInputs().size(), is(17)); + assertThat(flow.getInputs().size(), is(18)); assertThat(flow.getInputs().stream().filter(Input::getRequired).count(), is(6L)); - assertThat(flow.getInputs().stream().filter(r -> !r.getRequired()).count(), is(11L)); + assertThat(flow.getInputs().stream().filter(r -> !r.getRequired()).count(), is(12L)); assertThat(flow.getInputs().stream().filter(r -> r.getDefaults() != null).count(), is(1L)); + assertThat(flow.getInputs().stream().filter(r -> r.getValidator() != null).count(), is(1L)); } @Test diff --git a/core/src/test/java/io/kestra/core/validations/InputTest.java b/core/src/test/java/io/kestra/core/validations/InputTest.java new file mode 100644 index 00000000000..e574835f7be --- /dev/null +++ b/core/src/test/java/io/kestra/core/validations/InputTest.java @@ -0,0 +1,37 @@ +package io.kestra.core.validations; + +import io.kestra.core.models.flows.Input; +import io.kestra.core.models.validations.ModelValidator; +import io.micronaut.test.extensions.junit5.annotation.MicronautTest; +import jakarta.inject.Inject; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; + +@MicronautTest +class InputTest { + @Inject + private ModelValidator modelValidator; + + @Test + void inputValidation() { + final Input validInput = Input.builder() + .name("test") + .type(Input.Type.STRING) + .validator("[A-Z]+") + .build(); + + assertThat(modelValidator.isValid(validInput).isEmpty(), is(true)); + + final Input invalidInput = Input.builder() + .name("test") + .type(Input.Type.INT) + .validator("[A-Z]+") + .build(); + + assertThat(modelValidator.isValid(invalidInput).isPresent(), is(true)); + assertThat(modelValidator.isValid(invalidInput).get().getMessage(), containsString("Invalid Input: Validator")); + } +} diff --git a/core/src/test/java/io/kestra/core/validations/RegexTest.java b/core/src/test/java/io/kestra/core/validations/RegexTest.java new file mode 100644 index 00000000000..23a2f2852ac --- /dev/null +++ b/core/src/test/java/io/kestra/core/validations/RegexTest.java @@ -0,0 +1,39 @@ +package io.kestra.core.validations; + +import io.kestra.core.models.validations.ModelValidator; +import io.micronaut.core.annotation.Introspected; +import io.micronaut.test.extensions.junit5.annotation.MicronautTest; +import jakarta.inject.Inject; +import lombok.AllArgsConstructor; +import lombok.Getter; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; + +@MicronautTest +class RegexTest { + @Inject + private ModelValidator modelValidator; + + @AllArgsConstructor + @Introspected + @Getter + public static class RegexCls { + @Regex + String pattern; + } + + @Test + void inputValidation() { + final RegexCls validRegex = new RegexCls("[A-Z]+"); + + assertThat(modelValidator.isValid(validRegex).isEmpty(), is(true)); + + final RegexCls invalidRegex = new RegexCls("\\"); + + assertThat(modelValidator.isValid(invalidRegex).isPresent(), is(true)); + assertThat(modelValidator.isValid(invalidRegex).get().getMessage(), containsString("invalid pattern")); + } +} diff --git a/core/src/test/resources/flows/invalids/inputs-bad-validator-syntax.yaml b/core/src/test/resources/flows/invalids/inputs-bad-validator-syntax.yaml new file mode 100644 index 00000000000..ac8a78e6437 --- /dev/null +++ b/core/src/test/resources/flows/invalids/inputs-bad-validator-syntax.yaml @@ -0,0 +1,10 @@ +id: empty +namespace: io.kestra.tests +inputs: + - name: badValidatorSyntax + type: STRING + validator: \ +tasks: + - id: date + type: io.kestra.core.tasks.debugs.Return + format: "{{taskrun.startDate}}" diff --git a/core/src/test/resources/flows/valids/inputs.yaml b/core/src/test/resources/flows/valids/inputs.yaml index 32883642e32..a1c2e775a3c 100644 --- a/core/src/test/resources/flows/valids/inputs.yaml +++ b/core/src/test/resources/flows/valids/inputs.yaml @@ -48,6 +48,10 @@ inputs: - name: nested.bool type: BOOLEAN required: false +- name: validatedString + type: STRING + validator: A\d+ + required: false tasks: - id: string type: io.kestra.core.tasks.debugs.Return From e02597a4e86dfa8247ec350f38823b65ced792a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Mathieu?= Date: Thu, 6 Apr 2023 10:00:16 +0200 Subject: [PATCH 2/3] feat(core): input validator step 2 --- .../io/kestra/core/models/flows/Input.java | 50 ++-- .../core/models/flows/input/BooleanInput.java | 18 ++ .../core/models/flows/input/DateInput.java | 19 ++ .../models/flows/input/DateTimeInput.java | 19 ++ .../models/flows/input/DurationInput.java | 19 ++ .../core/models/flows/input/FileInput.java | 19 ++ .../core/models/flows/input/FloatInput.java | 18 ++ .../core/models/flows/input/IntInput.java | 18 ++ .../core/models/flows/input/JsonInput.java | 18 ++ .../core/models/flows/input/StringInput.java | 38 +++ .../core/models/flows/input/TimeInput.java | 19 ++ .../core/models/flows/input/URIInput.java | 18 ++ .../io/kestra/core/runners/RunnerUtils.java | 227 +++++++++--------- .../core/validations/InputValidation.java | 11 - .../core/validations/ValidationFactory.java | 21 +- .../AbstractFlowRepositoryTest.java | 9 +- .../io/kestra/core/runners/InputsTest.java | 19 +- .../schedulers/AbstractSchedulerTest.java | 5 +- .../core/serializers/YamlFlowParserTest.java | 17 +- .../io/kestra/core/validations/InputTest.java | 13 +- 20 files changed, 382 insertions(+), 213 deletions(-) create mode 100644 core/src/main/java/io/kestra/core/models/flows/input/BooleanInput.java create mode 100644 core/src/main/java/io/kestra/core/models/flows/input/DateInput.java create mode 100644 core/src/main/java/io/kestra/core/models/flows/input/DateTimeInput.java create mode 100644 core/src/main/java/io/kestra/core/models/flows/input/DurationInput.java create mode 100644 core/src/main/java/io/kestra/core/models/flows/input/FileInput.java create mode 100644 core/src/main/java/io/kestra/core/models/flows/input/FloatInput.java create mode 100644 core/src/main/java/io/kestra/core/models/flows/input/IntInput.java create mode 100644 core/src/main/java/io/kestra/core/models/flows/input/JsonInput.java create mode 100644 core/src/main/java/io/kestra/core/models/flows/input/StringInput.java create mode 100644 core/src/main/java/io/kestra/core/models/flows/input/TimeInput.java create mode 100644 core/src/main/java/io/kestra/core/models/flows/input/URIInput.java delete mode 100644 core/src/main/java/io/kestra/core/validations/InputValidation.java diff --git a/core/src/main/java/io/kestra/core/models/flows/Input.java b/core/src/main/java/io/kestra/core/models/flows/Input.java index 4752ef68e79..47e7e18256e 100644 --- a/core/src/main/java/io/kestra/core/models/flows/Input.java +++ b/core/src/main/java/io/kestra/core/models/flows/Input.java @@ -1,16 +1,16 @@ package io.kestra.core.models.flows; -import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; -import io.kestra.core.validations.InputValidation; -import io.kestra.core.validations.Regex; +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import io.kestra.core.models.flows.input.*; import io.micronaut.core.annotation.Introspected; -import io.swagger.v3.oas.annotations.media.Schema; import lombok.Builder; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.experimental.SuperBuilder; +import javax.validation.ConstraintViolationException; import javax.validation.Valid; import javax.validation.constraints.NotBlank; import javax.validation.constraints.NotNull; @@ -21,8 +21,21 @@ @NoArgsConstructor @Introspected @JsonInclude(JsonInclude.Include.NON_DEFAULT) -@InputValidation -public class Input { +@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type", visible = true, include = JsonTypeInfo.As.EXISTING_PROPERTY) +@JsonSubTypes({ + @JsonSubTypes.Type(value = BooleanInput.class, name = "BOOLEAN"), + @JsonSubTypes.Type(value = DateInput.class, name = "DATE"), + @JsonSubTypes.Type(value = DateTimeInput.class, name = "DATETIME"), + @JsonSubTypes.Type(value = DurationInput.class, name = "DURATION"), + @JsonSubTypes.Type(value = FileInput.class, name = "FILE"), + @JsonSubTypes.Type(value = FloatInput.class, name = "FLOAT"), + @JsonSubTypes.Type(value = IntInput.class, name = "INT"), + @JsonSubTypes.Type(value = JsonInput.class, name = "JSON"), + @JsonSubTypes.Type(value = StringInput.class, name = "STRING"), + @JsonSubTypes.Type(value = TimeInput.class, name = "TIME"), + @JsonSubTypes.Type(value = URIInput.class, name = "URI") +}) +public abstract class Input { @NotNull @NotBlank @Pattern(regexp="[.a-zA-Z0-9_-]+") @@ -40,28 +53,11 @@ public class Input { String defaults; - @Schema( - title = "Regular expression validating the value." - ) - @Regex - String validator; - - @JsonIgnore - public boolean canBeValidated() { - if (type == null) { - return false; - } - return type.canBeValidated(); - } + public abstract void validate(T input) throws ConstraintViolationException; @Introspected public enum Type { - STRING() { - @Override - public boolean canBeValidated() { - return true; - } - }, + STRING, INT, FLOAT, BOOLEAN, @@ -72,9 +68,5 @@ public boolean canBeValidated() { FILE, JSON, URI; - - public boolean canBeValidated() { - return false; - } } } diff --git a/core/src/main/java/io/kestra/core/models/flows/input/BooleanInput.java b/core/src/main/java/io/kestra/core/models/flows/input/BooleanInput.java new file mode 100644 index 00000000000..5f23e513d33 --- /dev/null +++ b/core/src/main/java/io/kestra/core/models/flows/input/BooleanInput.java @@ -0,0 +1,18 @@ +package io.kestra.core.models.flows.input; + +import io.kestra.core.models.flows.Input; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.experimental.SuperBuilder; + +import javax.validation.ConstraintViolationException; + +@SuperBuilder +@Getter +@NoArgsConstructor +public class BooleanInput extends Input { + @Override + public void validate(Boolean input) throws ConstraintViolationException { + // no validation yet + } +} diff --git a/core/src/main/java/io/kestra/core/models/flows/input/DateInput.java b/core/src/main/java/io/kestra/core/models/flows/input/DateInput.java new file mode 100644 index 00000000000..063119fa431 --- /dev/null +++ b/core/src/main/java/io/kestra/core/models/flows/input/DateInput.java @@ -0,0 +1,19 @@ +package io.kestra.core.models.flows.input; + +import io.kestra.core.models.flows.Input; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.experimental.SuperBuilder; + +import java.time.LocalDate; +import javax.validation.ConstraintViolationException; + +@SuperBuilder +@Getter +@NoArgsConstructor +public class DateInput extends Input { + @Override + public void validate(LocalDate input) throws ConstraintViolationException { + // no validation yet + } +} diff --git a/core/src/main/java/io/kestra/core/models/flows/input/DateTimeInput.java b/core/src/main/java/io/kestra/core/models/flows/input/DateTimeInput.java new file mode 100644 index 00000000000..df8575d1bdd --- /dev/null +++ b/core/src/main/java/io/kestra/core/models/flows/input/DateTimeInput.java @@ -0,0 +1,19 @@ +package io.kestra.core.models.flows.input; + +import io.kestra.core.models.flows.Input; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.experimental.SuperBuilder; + +import java.time.Instant; +import javax.validation.ConstraintViolationException; + +@SuperBuilder +@Getter +@NoArgsConstructor +public class DateTimeInput extends Input { + @Override + public void validate(Instant input) throws ConstraintViolationException { + // no validation yet + } +} diff --git a/core/src/main/java/io/kestra/core/models/flows/input/DurationInput.java b/core/src/main/java/io/kestra/core/models/flows/input/DurationInput.java new file mode 100644 index 00000000000..10456b5905b --- /dev/null +++ b/core/src/main/java/io/kestra/core/models/flows/input/DurationInput.java @@ -0,0 +1,19 @@ +package io.kestra.core.models.flows.input; + +import io.kestra.core.models.flows.Input; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.experimental.SuperBuilder; + +import java.time.Duration; +import javax.validation.ConstraintViolationException; + +@SuperBuilder +@Getter +@NoArgsConstructor +public class DurationInput extends Input { + @Override + public void validate(Duration input) throws ConstraintViolationException { + // no validation yet + } +} diff --git a/core/src/main/java/io/kestra/core/models/flows/input/FileInput.java b/core/src/main/java/io/kestra/core/models/flows/input/FileInput.java new file mode 100644 index 00000000000..a3e8e5a4a87 --- /dev/null +++ b/core/src/main/java/io/kestra/core/models/flows/input/FileInput.java @@ -0,0 +1,19 @@ +package io.kestra.core.models.flows.input; + +import io.kestra.core.models.flows.Input; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.experimental.SuperBuilder; + +import java.net.URI; +import javax.validation.ConstraintViolationException; + +@SuperBuilder +@Getter +@NoArgsConstructor +public class FileInput extends Input { + @Override + public void validate(URI input) throws ConstraintViolationException { + // no validation yet + } +} diff --git a/core/src/main/java/io/kestra/core/models/flows/input/FloatInput.java b/core/src/main/java/io/kestra/core/models/flows/input/FloatInput.java new file mode 100644 index 00000000000..c43861c867e --- /dev/null +++ b/core/src/main/java/io/kestra/core/models/flows/input/FloatInput.java @@ -0,0 +1,18 @@ +package io.kestra.core.models.flows.input; + +import io.kestra.core.models.flows.Input; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.experimental.SuperBuilder; + +import javax.validation.ConstraintViolationException; + +@SuperBuilder +@Getter +@NoArgsConstructor +public class FloatInput extends Input { + @Override + public void validate(Float input) throws ConstraintViolationException { + // no validation yet + } +} diff --git a/core/src/main/java/io/kestra/core/models/flows/input/IntInput.java b/core/src/main/java/io/kestra/core/models/flows/input/IntInput.java new file mode 100644 index 00000000000..fbbd22fbcb7 --- /dev/null +++ b/core/src/main/java/io/kestra/core/models/flows/input/IntInput.java @@ -0,0 +1,18 @@ +package io.kestra.core.models.flows.input; + +import io.kestra.core.models.flows.Input; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.experimental.SuperBuilder; + +import javax.validation.ConstraintViolationException; + +@SuperBuilder +@Getter +@NoArgsConstructor +public class IntInput extends Input { + @Override + public void validate(Integer input) throws ConstraintViolationException { + // no validation yet + } +} diff --git a/core/src/main/java/io/kestra/core/models/flows/input/JsonInput.java b/core/src/main/java/io/kestra/core/models/flows/input/JsonInput.java new file mode 100644 index 00000000000..38303512279 --- /dev/null +++ b/core/src/main/java/io/kestra/core/models/flows/input/JsonInput.java @@ -0,0 +1,18 @@ +package io.kestra.core.models.flows.input; + +import io.kestra.core.models.flows.Input; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.experimental.SuperBuilder; + +import javax.validation.ConstraintViolationException; + +@SuperBuilder +@Getter +@NoArgsConstructor +public class JsonInput extends Input { + @Override + public void validate(Object input) throws ConstraintViolationException { + // no validation yet + } +} diff --git a/core/src/main/java/io/kestra/core/models/flows/input/StringInput.java b/core/src/main/java/io/kestra/core/models/flows/input/StringInput.java new file mode 100644 index 00000000000..378cf45835c --- /dev/null +++ b/core/src/main/java/io/kestra/core/models/flows/input/StringInput.java @@ -0,0 +1,38 @@ +package io.kestra.core.models.flows.input; + +import io.kestra.core.models.flows.Input; +import io.kestra.core.models.validations.ManualConstraintViolation; +import io.kestra.core.validations.Regex; +import io.swagger.v3.oas.annotations.media.Schema; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.experimental.SuperBuilder; + +import java.util.Set; +import java.util.regex.Pattern; +import javax.validation.ConstraintViolationException; + +@SuperBuilder +@Getter +@NoArgsConstructor +public class StringInput extends Input { + @Schema( + title = "Regular expression validating the value." + ) + @Regex + String validator; + + @Override + public void validate(String input) throws ConstraintViolationException { + if (validator != null && ! Pattern.matches(validator, input)) { + throw new ConstraintViolationException("Invalid input '" + input + "', it must match the pattern '" + validator + "'", + Set.of(ManualConstraintViolation.of( + "Invalid input", + this, + StringInput.class, + getName(), + input + ))); + } + } +} diff --git a/core/src/main/java/io/kestra/core/models/flows/input/TimeInput.java b/core/src/main/java/io/kestra/core/models/flows/input/TimeInput.java new file mode 100644 index 00000000000..93b41b10cbe --- /dev/null +++ b/core/src/main/java/io/kestra/core/models/flows/input/TimeInput.java @@ -0,0 +1,19 @@ +package io.kestra.core.models.flows.input; + +import io.kestra.core.models.flows.Input; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.experimental.SuperBuilder; + +import java.time.LocalTime; +import javax.validation.ConstraintViolationException; + +@SuperBuilder +@Getter +@NoArgsConstructor +public class TimeInput extends Input { + @Override + public void validate(LocalTime input) throws ConstraintViolationException { + // no validation yet + } +} diff --git a/core/src/main/java/io/kestra/core/models/flows/input/URIInput.java b/core/src/main/java/io/kestra/core/models/flows/input/URIInput.java new file mode 100644 index 00000000000..469a6cd9614 --- /dev/null +++ b/core/src/main/java/io/kestra/core/models/flows/input/URIInput.java @@ -0,0 +1,18 @@ +package io.kestra.core.models.flows.input; + +import io.kestra.core.models.flows.Input; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.experimental.SuperBuilder; + +import javax.validation.ConstraintViolationException; + +@SuperBuilder +@Getter +@NoArgsConstructor +public class URIInput extends Input { + @Override + public void validate(String input) throws ConstraintViolationException { + // no validation yet + } +} diff --git a/core/src/main/java/io/kestra/core/runners/RunnerUtils.java b/core/src/main/java/io/kestra/core/runners/RunnerUtils.java index 5a89b64806d..fded5655559 100644 --- a/core/src/main/java/io/kestra/core/runners/RunnerUtils.java +++ b/core/src/main/java/io/kestra/core/runners/RunnerUtils.java @@ -119,134 +119,125 @@ public Map typedInputs(Flow flow, Execution execution, Map( - input.getName(), - current - )); + var parsedInput = parseInput(flow, execution, input, current); + parsedInput.ifPresent(parsed -> input.validate(parsed.getValue())); + return parsedInput; + }) + .filter(Optional::isPresent) + .map(Optional::get) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - case INT: - return Optional.of(new AbstractMap.SimpleEntry( - input.getName(), - Integer.valueOf(current) - )); + return handleNestedInputs(results); + } + + private Optional> parseInput(Flow flow, Execution execution, Input input, String current) { + switch (input.getType()) { + case STRING: + return Optional.of(new AbstractMap.SimpleEntry<>( + input.getName(), + current + )); + + case INT: + return Optional.of(new AbstractMap.SimpleEntry<>( + input.getName(), + Integer.valueOf(current) + )); + + case FLOAT: + return Optional.of(new AbstractMap.SimpleEntry<>( + input.getName(), + Float.valueOf(current) + )); + + case BOOLEAN: + return Optional.of(new AbstractMap.SimpleEntry<>( + input.getName(), + Boolean.valueOf(current) + )); + + case DATETIME: + try { + return Optional.of(new AbstractMap.SimpleEntry<>( + input.getName(), + Instant.parse(current) + )); + } catch (DateTimeParseException e) { + throw new MissingRequiredInput("Invalid DATETIME format for '" + input.getName() + "' for '" + current + "' with error " + e.getMessage(), e); + } + + case DATE: + try { + return Optional.of(new AbstractMap.SimpleEntry<>( + input.getName(), + LocalDate.parse(current) + )); + } catch (DateTimeParseException e) { + throw new MissingRequiredInput("Invalid DATE format for '" + input.getName() + "' for '" + current + "' with error " + e.getMessage(), e); + } + + case TIME: + try { + return Optional.of(new AbstractMap.SimpleEntry<>( + input.getName(), + LocalTime.parse(current) + )); + } catch (DateTimeParseException e) { + throw new MissingRequiredInput("Invalid TIME format for '" + input.getName() + "' for '" + current + "' with error " + e.getMessage(), e); + } + + case DURATION: + try { + return Optional.of(new AbstractMap.SimpleEntry<>( + input.getName(), + Duration.parse(current) + )); + } catch (DateTimeParseException e) { + throw new MissingRequiredInput("Invalid DURATION format for '" + input.getName() + "' for '" + current + "' with error " + e.getMessage(), e); + } + + case FILE: + try { + URI uri = URI.create(current.replace(File.separator, "/")); - case FLOAT: - return Optional.of(new AbstractMap.SimpleEntry( + if (uri.getScheme() != null && uri.getScheme().equals("kestra")) { + return Optional.of(new AbstractMap.SimpleEntry<>( input.getName(), - Float.valueOf(current) + uri )); - - case BOOLEAN: - return Optional.of(new AbstractMap.SimpleEntry( + } else { + return Optional.of(new AbstractMap.SimpleEntry<>( input.getName(), - Boolean.valueOf(current) + storageInterface.from(flow, execution, input, new File(current)) )); + } + } catch (Exception e) { + throw new MissingRequiredInput("Invalid input arguments for file on input '" + input.getName() + "'", e); + } - case DATETIME: - try { - return Optional.of(new AbstractMap.SimpleEntry( - input.getName(), - Instant.parse(current) - )); - } catch (DateTimeParseException e) { - throw new MissingRequiredInput("Invalid DATETIME format for '" + input.getName() + "' for '" + current + "' with error " + e.getMessage(), e); - } - - case DATE: - try { - return Optional.of(new AbstractMap.SimpleEntry( - input.getName(), - LocalDate.parse(current) - )); - } catch (DateTimeParseException e) { - throw new MissingRequiredInput("Invalid DATE format for '" + input.getName() + "' for '" + current + "' with error " + e.getMessage(), e); - } - - case TIME: - try { - return Optional.of(new AbstractMap.SimpleEntry( - input.getName(), - LocalTime.parse(current) - )); - } catch (DateTimeParseException e) { - throw new MissingRequiredInput("Invalid TIME format for '" + input.getName() + "' for '" + current + "' with error " + e.getMessage(), e); - } - - case DURATION: - try { - return Optional.of(new AbstractMap.SimpleEntry( - input.getName(), - Duration.parse(current) - )); - } catch (DateTimeParseException e) { - throw new MissingRequiredInput("Invalid DURATION format for '" + input.getName() + "' for '" + current + "' with error " + e.getMessage(), e); - } - - case FILE: - try { - URI uri = URI.create(current.replace(File.separator, "/")); - - if (uri.getScheme() != null && uri.getScheme().equals("kestra")) { - return Optional.of(new AbstractMap.SimpleEntry( - input.getName(), - uri - )); - } else { - return Optional.of(new AbstractMap.SimpleEntry( - input.getName(), - storageInterface.from(flow, execution, input, new File(current)) - )); - } - } catch (Exception e) { - throw new MissingRequiredInput("Invalid input arguments for file on input '" + input.getName() + "'", e); - } - - case JSON: - try { - return Optional.of(new AbstractMap.SimpleEntry<>( - input.getName(), - JacksonMapper.toObject(current) - )); - } catch (JsonProcessingException e) { - throw new MissingRequiredInput("Invalid JSON format for '" + input.getName() + "' for '" + current + "' with error " + e.getMessage(), e); - } - - case URI: - Matcher matcher = URI_PATTERN.matcher(current); - if (matcher.matches()) { - return Optional.of(new AbstractMap.SimpleEntry<>( - input.getName(), - current - )); - } else { - throw new MissingRequiredInput("Invalid URI format for '" + input.getName() + "' for '" + current + "'"); - } - - default: - throw new MissingRequiredInput("Invalid input type '" + input.getType() + "' for '" + input.getName() + "'"); + case JSON: + try { + return Optional.of(new AbstractMap.SimpleEntry<>( + input.getName(), + JacksonMapper.toObject(current) + )); + } catch (JsonProcessingException e) { + throw new MissingRequiredInput("Invalid JSON format for '" + input.getName() + "' for '" + current + "' with error " + e.getMessage(), e); } - }) - .filter(Optional::isPresent) - .map(Optional::get) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - return handleNestedInputs(results); - } + case URI: + Matcher matcher = URI_PATTERN.matcher(current); + if (matcher.matches()) { + return Optional.of(new AbstractMap.SimpleEntry<>( + input.getName(), + current + )); + } else { + throw new MissingRequiredInput("Invalid URI format for '" + input.getName() + "' for '" + current + "'"); + } - private void validateStringInput(Input input, String current) { - final String validator = input.getValidator(); - if (validator == null) { - return; - } - try { - if (!Pattern.matches(validator, current)) { - throw new MissingRequiredInput("Invalid format for '" + input.getName() + "' defined by validator '" + validator + "'"); - } - } catch (PatternSyntaxException e) { - throw new MissingRequiredInput("Invalid validator syntax '" + validator + "' for '" + input.getName() + "'"); + default: + throw new MissingRequiredInput("Invalid input type '" + input.getType() + "' for '" + input.getName() + "'"); } } diff --git a/core/src/main/java/io/kestra/core/validations/InputValidation.java b/core/src/main/java/io/kestra/core/validations/InputValidation.java deleted file mode 100644 index 9b893048d94..00000000000 --- a/core/src/main/java/io/kestra/core/validations/InputValidation.java +++ /dev/null @@ -1,11 +0,0 @@ -package io.kestra.core.validations; - -import javax.validation.Constraint; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; - -@Retention(RetentionPolicy.RUNTIME) -@Constraint(validatedBy = { }) -public @interface InputValidation { - String message() default "invalid Input"; -} diff --git a/core/src/main/java/io/kestra/core/validations/ValidationFactory.java b/core/src/main/java/io/kestra/core/validations/ValidationFactory.java index 34743750847..7c54c83b65c 100644 --- a/core/src/main/java/io/kestra/core/validations/ValidationFactory.java +++ b/core/src/main/java/io/kestra/core/validations/ValidationFactory.java @@ -13,6 +13,7 @@ import java.io.IOException; import java.text.SimpleDateFormat; import java.util.*; +import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; import java.util.stream.Collectors; import javax.validation.ConstraintViolation; @@ -148,24 +149,6 @@ ConstraintValidator flowValidation() { }; } - @Singleton - ConstraintValidator inputValidation() { - return (value, annotationMetadata, context) -> { - if (value == null) { - return true; - } - - if (value.getValidator() != null && !value.canBeValidated()) { - context.messageTemplate( - "Invalid Input: Validator defined at [" + value.getName() + "] is not allowed for type [" + value.getType() + "]" - ); - return false; - } - - return true; - }; - } - @Singleton ConstraintValidator patternValidator() { return (value, annotationMetadata, context) -> { @@ -174,7 +157,7 @@ ConstraintValidator patternValidator() { } try { - java.util.regex.Pattern.compile(value); + Pattern.compile(value); } catch(PatternSyntaxException e) { context.messageTemplate("invalid pattern [" + value + "]"); return false; diff --git a/core/src/test/java/io/kestra/core/repositories/AbstractFlowRepositoryTest.java b/core/src/test/java/io/kestra/core/repositories/AbstractFlowRepositoryTest.java index 33c73fd326c..7640ee857da 100644 --- a/core/src/test/java/io/kestra/core/repositories/AbstractFlowRepositoryTest.java +++ b/core/src/test/java/io/kestra/core/repositories/AbstractFlowRepositoryTest.java @@ -8,6 +8,7 @@ import io.kestra.core.models.flows.Flow; import io.kestra.core.models.flows.FlowWithSource; import io.kestra.core.models.flows.Input; +import io.kestra.core.models.flows.input.StringInput; import io.kestra.core.models.triggers.Trigger; import io.kestra.core.queues.QueueFactoryInterface; import io.kestra.core.queues.QueueInterface; @@ -113,7 +114,7 @@ protected void revision() throws JsonProcessingException { .id(flowId) .namespace("io.kestra.unittest") .tasks(Collections.singletonList(Return.builder().id("test").type(Return.class.getName()).format("test").build())) - .inputs(ImmutableList.of(Input.builder().type(Input.Type.STRING).name("a").build())) + .inputs(ImmutableList.of(StringInput.builder().type(Input.Type.STRING).name("a").build())) .build(); // create with repository FlowWithSource flow = flowRepository.create(first, first.generateSource(), taskDefaultService.injectDefaults(first)); @@ -133,7 +134,7 @@ protected void revision() throws JsonProcessingException { .commands(Collections.singletonList("echo 1").toArray(new String[0])) .build() )) - .inputs(ImmutableList.of(Input.builder().type(Input.Type.STRING).name("b").build())) + .inputs(ImmutableList.of(StringInput.builder().type(Input.Type.STRING).name("b").build())) .build(); // revision is incremented @@ -258,7 +259,7 @@ void updateConflict() { Flow flow = Flow.builder() .id(flowId) .namespace("io.kestra.unittest") - .inputs(ImmutableList.of(Input.builder().type(Input.Type.STRING).name("a").build())) + .inputs(ImmutableList.of(StringInput.builder().type(Input.Type.STRING).name("a").build())) .tasks(Collections.singletonList(Return.builder().id("test").type(Return.class.getName()).format("test").build())) .build(); @@ -269,7 +270,7 @@ void updateConflict() { Flow update = Flow.builder() .id(IdUtils.create()) .namespace("io.kestra.unittest2") - .inputs(ImmutableList.of(Input.builder().type(Input.Type.STRING).name("b").build())) + .inputs(ImmutableList.of(StringInput.builder().type(Input.Type.STRING).name("b").build())) .tasks(Collections.singletonList(Return.builder().id("test").type(Return.class.getName()).format("test").build())) .build(); ; diff --git a/core/src/test/java/io/kestra/core/runners/InputsTest.java b/core/src/test/java/io/kestra/core/runners/InputsTest.java index 26ece7cb3c5..34a71c9a538 100644 --- a/core/src/test/java/io/kestra/core/runners/InputsTest.java +++ b/core/src/test/java/io/kestra/core/runners/InputsTest.java @@ -31,6 +31,8 @@ import java.util.Objects; import java.util.concurrent.TimeoutException; +import javax.validation.ConstraintViolationException; + import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -195,24 +197,11 @@ void inputValidatedStringBadValue() { HashMap map = new HashMap<>(inputs); map.put("validatedString", "foo"); - MissingRequiredInput e = assertThrows(MissingRequiredInput.class, () -> { + ConstraintViolationException e = assertThrows(ConstraintViolationException.class, () -> { Map typeds = typedInputs(map); }); - assertThat(e.getMessage(), containsString("Invalid format for ")); - } - - @Test - void inputValidatedStringBadSyntax() { - final Flow flow = parse("flows/invalids/inputs-bad-validator-syntax.yaml"); - final HashMap map = new HashMap<>(inputs); - map.put("badValidatorSyntax", "foo"); - - final MissingRequiredInput e = assertThrows(MissingRequiredInput.class, () -> { - typedInputs(map, flow); - }); - - assertThat(e.getMessage(), containsString("Invalid validator syntax")); + assertThat(e.getMessage(), is("Invalid input 'foo', it must match the pattern 'A\\d+'")); } @Test diff --git a/core/src/test/java/io/kestra/core/schedulers/AbstractSchedulerTest.java b/core/src/test/java/io/kestra/core/schedulers/AbstractSchedulerTest.java index db6552fb778..62c6d250e7d 100644 --- a/core/src/test/java/io/kestra/core/schedulers/AbstractSchedulerTest.java +++ b/core/src/test/java/io/kestra/core/schedulers/AbstractSchedulerTest.java @@ -4,6 +4,7 @@ import io.kestra.core.models.conditions.ConditionContext; import io.kestra.core.models.flows.Input; import io.kestra.core.models.flows.TaskDefault; +import io.kestra.core.models.flows.input.StringInput; import io.micronaut.context.ApplicationContext; import io.micronaut.test.extensions.junit5.annotation.MicronautTest; import lombok.*; @@ -46,13 +47,13 @@ protected static Flow createFlow(List triggers, List r.getPropertyPath().toString().equals("inputs[0].name")).findFirst().orElseThrow().getMessage(), containsString("must match")); - assertThat(exception.getConstraintViolations().stream().filter(r -> r.getPropertyPath().toString().equals("inputs[0].type")).findFirst().orElseThrow().getMessage(), is("must not be null")); + // FIXME the property path changes: io.kestra.core.models.flows.Flow["inputs"]->java.util.ArrayList[0] +// assertThat(exception.getConstraintViolations().stream().filter(r -> r.getPropertyPath().toString().equals("inputs[0].name")).findFirst().orElseThrow().getMessage(), containsString("must match")); +// assertThat(exception.getConstraintViolations().stream().filter(r -> r.getPropertyPath().toString().equals("inputs[0].type")).findFirst().orElseThrow().getMessage(), is("must not be null")); + + exception.getConstraintViolations().forEach( + c -> assertThat(c.getMessage(), anyOf( + is("Invalid type: null"), + containsString("missing type id property 'type' (for POJO property 'inputs')")) + ) + ); } @Test @@ -124,7 +133,7 @@ void inputs() { assertThat(flow.getInputs().stream().filter(Input::getRequired).count(), is(6L)); assertThat(flow.getInputs().stream().filter(r -> !r.getRequired()).count(), is(12L)); assertThat(flow.getInputs().stream().filter(r -> r.getDefaults() != null).count(), is(1L)); - assertThat(flow.getInputs().stream().filter(r -> r.getValidator() != null).count(), is(1L)); + assertThat(flow.getInputs().stream().filter(r -> r instanceof StringInput && ((StringInput)r).getValidator() != null).count(), is(1L)); } @Test @@ -134,7 +143,7 @@ void inputsBadType() { () -> this.parse("flows/invalids/inputs-bad-type.yaml") ); - assertThat(exception.getMessage(), containsString("not one of the values accepted for Enum class")); + assertThat(exception.getMessage(), containsString("Invalid type: FOO")); } @Test diff --git a/core/src/test/java/io/kestra/core/validations/InputTest.java b/core/src/test/java/io/kestra/core/validations/InputTest.java index e574835f7be..13f167452c4 100644 --- a/core/src/test/java/io/kestra/core/validations/InputTest.java +++ b/core/src/test/java/io/kestra/core/validations/InputTest.java @@ -1,13 +1,13 @@ package io.kestra.core.validations; import io.kestra.core.models.flows.Input; +import io.kestra.core.models.flows.input.StringInput; import io.kestra.core.models.validations.ModelValidator; import io.micronaut.test.extensions.junit5.annotation.MicronautTest; import jakarta.inject.Inject; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; @MicronautTest @@ -17,21 +17,12 @@ class InputTest { @Test void inputValidation() { - final Input validInput = Input.builder() + final Input validInput = StringInput.builder() .name("test") .type(Input.Type.STRING) .validator("[A-Z]+") .build(); assertThat(modelValidator.isValid(validInput).isEmpty(), is(true)); - - final Input invalidInput = Input.builder() - .name("test") - .type(Input.Type.INT) - .validator("[A-Z]+") - .build(); - - assertThat(modelValidator.isValid(invalidInput).isPresent(), is(true)); - assertThat(modelValidator.isValid(invalidInput).get().getMessage(), containsString("Invalid Input: Validator")); } } From b4952601fac956a0ddff173ed7c8b6c34eaa518e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Mathieu?= Date: Thu, 6 Apr 2023 11:02:42 +0200 Subject: [PATCH 3/3] feat: validate INT input with min/max --- .../core/models/flows/input/IntInput.java | 33 ++++++++++++++++++- .../io/kestra/core/runners/InputsTest.java | 26 +++++++++++++++ .../core/serializers/YamlFlowParserTest.java | 8 ++--- .../test/resources/flows/valids/inputs.yaml | 5 +++ .../controllers/FlowControllerTest.java | 3 +- 5 files changed, 67 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/io/kestra/core/models/flows/input/IntInput.java b/core/src/main/java/io/kestra/core/models/flows/input/IntInput.java index fbbd22fbcb7..e38f9f0d272 100644 --- a/core/src/main/java/io/kestra/core/models/flows/input/IntInput.java +++ b/core/src/main/java/io/kestra/core/models/flows/input/IntInput.java @@ -1,18 +1,49 @@ package io.kestra.core.models.flows.input; import io.kestra.core.models.flows.Input; +import io.kestra.core.models.validations.ManualConstraintViolation; +import io.swagger.v3.oas.annotations.media.Schema; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.experimental.SuperBuilder; +import java.util.Set; +import java.util.regex.Pattern; import javax.validation.ConstraintViolationException; @SuperBuilder @Getter @NoArgsConstructor public class IntInput extends Input { + + @Schema(title = "Minimal value.") + Integer min; + + @Schema(title = "Maximal value.") + Integer max; + @Override public void validate(Integer input) throws ConstraintViolationException { - // no validation yet + if (min != null && input.compareTo(min) < 0) { + throw new ConstraintViolationException("Invalid input '" + input + "', it must be more than '" + min + "'", + Set.of(ManualConstraintViolation.of( + "Invalid input", + this, + IntInput.class, + getName(), + input + ))); + } + + if (max != null && input.compareTo(max) > 0) { + throw new ConstraintViolationException("Invalid input '" + input + "', it must be less than '" + max + "'", + Set.of(ManualConstraintViolation.of( + "Invalid input", + this, + IntInput.class, + getName(), + input + ))); + } } } diff --git a/core/src/test/java/io/kestra/core/runners/InputsTest.java b/core/src/test/java/io/kestra/core/runners/InputsTest.java index 34a71c9a538..7568986e957 100644 --- a/core/src/test/java/io/kestra/core/runners/InputsTest.java +++ b/core/src/test/java/io/kestra/core/runners/InputsTest.java @@ -55,6 +55,7 @@ public class InputsTest extends AbstractMemoryRunnerTest { .put("nested.more.int", "123") .put("nested.bool", "true") .put("validatedString", "A123") + .put("validatedInt", "12") .build(); @Inject @@ -204,6 +205,31 @@ void inputValidatedStringBadValue() { assertThat(e.getMessage(), is("Invalid input 'foo', it must match the pattern 'A\\d+'")); } + @Test + void inputValidatedInteger() { + Map typeds = typedInputs(inputs); + assertThat(typeds.get("validatedInt"), is(12)); + } + + @Test + void inputValidatedIntegerBadValue() { + HashMap mapMin = new HashMap<>(inputs); + mapMin.put("validatedInt", "9"); + ConstraintViolationException e = assertThrows(ConstraintViolationException.class, () -> { + Map typeds = typedInputs(mapMin); + }); + assertThat(e.getMessage(), is("Invalid input '9', it must be more than '10'")); + + HashMap mapMax = new HashMap<>(inputs); + mapMax.put("validatedInt", "21"); + + e = assertThrows(ConstraintViolationException.class, () -> { + Map typeds = typedInputs(mapMax); + }); + + assertThat(e.getMessage(), is("Invalid input '21', it must be less than '20'")); + } + @Test void inputFailed() { HashMap map = new HashMap<>(inputs); diff --git a/core/src/test/java/io/kestra/core/serializers/YamlFlowParserTest.java b/core/src/test/java/io/kestra/core/serializers/YamlFlowParserTest.java index 0fdb7ac2285..5bae45f17e9 100644 --- a/core/src/test/java/io/kestra/core/serializers/YamlFlowParserTest.java +++ b/core/src/test/java/io/kestra/core/serializers/YamlFlowParserTest.java @@ -113,10 +113,6 @@ void inputsFailed() { ); assertThat(exception.getConstraintViolations().size(), is(2)); - // FIXME the property path changes: io.kestra.core.models.flows.Flow["inputs"]->java.util.ArrayList[0] -// assertThat(exception.getConstraintViolations().stream().filter(r -> r.getPropertyPath().toString().equals("inputs[0].name")).findFirst().orElseThrow().getMessage(), containsString("must match")); -// assertThat(exception.getConstraintViolations().stream().filter(r -> r.getPropertyPath().toString().equals("inputs[0].type")).findFirst().orElseThrow().getMessage(), is("must not be null")); - exception.getConstraintViolations().forEach( c -> assertThat(c.getMessage(), anyOf( is("Invalid type: null"), @@ -129,9 +125,9 @@ void inputsFailed() { void inputs() { Flow flow = this.parse("flows/valids/inputs.yaml"); - assertThat(flow.getInputs().size(), is(18)); + assertThat(flow.getInputs().size(), is(19)); assertThat(flow.getInputs().stream().filter(Input::getRequired).count(), is(6L)); - assertThat(flow.getInputs().stream().filter(r -> !r.getRequired()).count(), is(12L)); + assertThat(flow.getInputs().stream().filter(r -> !r.getRequired()).count(), is(13L)); assertThat(flow.getInputs().stream().filter(r -> r.getDefaults() != null).count(), is(1L)); assertThat(flow.getInputs().stream().filter(r -> r instanceof StringInput && ((StringInput)r).getValidator() != null).count(), is(1L)); } diff --git a/core/src/test/resources/flows/valids/inputs.yaml b/core/src/test/resources/flows/valids/inputs.yaml index a1c2e775a3c..40c42e6cbe6 100644 --- a/core/src/test/resources/flows/valids/inputs.yaml +++ b/core/src/test/resources/flows/valids/inputs.yaml @@ -52,6 +52,11 @@ inputs: type: STRING validator: A\d+ required: false +- name: validatedInt + type: INT + min: 10 + max: 20 + required: false tasks: - id: string type: io.kestra.core.tasks.debugs.Return diff --git a/webserver/src/test/java/io/kestra/webserver/controllers/FlowControllerTest.java b/webserver/src/test/java/io/kestra/webserver/controllers/FlowControllerTest.java index 95f0bb0bd93..9bbd8b94932 100644 --- a/webserver/src/test/java/io/kestra/webserver/controllers/FlowControllerTest.java +++ b/webserver/src/test/java/io/kestra/webserver/controllers/FlowControllerTest.java @@ -6,6 +6,7 @@ import io.kestra.core.models.flows.Flow; import io.kestra.core.models.flows.FlowWithSource; import io.kestra.core.models.flows.Input; +import io.kestra.core.models.flows.input.StringInput; import io.kestra.core.models.hierarchies.FlowGraph; import io.kestra.core.models.tasks.Task; import io.kestra.core.runners.AbstractMemoryRunnerTest; @@ -647,7 +648,7 @@ private Flow generateFlow(String friendlyId, String namespace, String inputName) return Flow.builder() .id(friendlyId) .namespace(namespace) - .inputs(ImmutableList.of(Input.builder().type(Input.Type.STRING).name(inputName).build())) + .inputs(ImmutableList.of(StringInput.builder().type(Input.Type.STRING).name(inputName).build())) .tasks(Collections.singletonList(generateTask("test", "test"))) .build(); }