From 90a41a1710efad45bf2a831aa1b814441294b8ec Mon Sep 17 00:00:00 2001 From: John Doe Date: Fri, 23 Oct 2026 19:06:43 +0530 Subject: [PATCH] fix: validate Bedrock conversation messages --- .../kestra/plugin/aws/bedrock/Converse.java | 6 ++- .../plugin/aws/bedrock/ConverseStream.java | 6 ++- .../aws/bedrock/ConverseStreamTest.java | 49 ++++++++++++++++++ .../plugin/aws/bedrock/ConverseTest.java | 50 +++++++++++++++++++ 4 files changed, 107 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/kestra/plugin/aws/bedrock/Converse.java b/src/main/java/io/kestra/plugin/aws/bedrock/Converse.java index 746fec41..08e736ed 100644 --- a/src/main/java/io/kestra/plugin/aws/bedrock/Converse.java +++ b/src/main/java/io/kestra/plugin/aws/bedrock/Converse.java @@ -16,7 +16,6 @@ import io.kestra.plugin.aws.shared.ConnectionUtils; import io.swagger.v3.oas.annotations.media.Schema; -import jakarta.validation.constraints.NotEmpty; import jakarta.validation.constraints.NotNull; import lombok.*; import lombok.experimental.SuperBuilder; @@ -84,7 +83,7 @@ public class Converse extends AbstractConnection implements RunnableTask>> messages; @Schema( @@ -115,6 +114,9 @@ public Output run(RunContext runContext) throws Exception { } catch (ClassCastException e) { throw new IllegalArgumentException("messages must be a list of {role, content} maps", e); } + if (rawMessages.isEmpty()) { + throw new IllegalArgumentException("messages must not be empty"); + } var sdkMessages = BedrockUtils.buildMessages(rawMessages); diff --git a/src/main/java/io/kestra/plugin/aws/bedrock/ConverseStream.java b/src/main/java/io/kestra/plugin/aws/bedrock/ConverseStream.java index 1d3f519e..52cfed4d 100644 --- a/src/main/java/io/kestra/plugin/aws/bedrock/ConverseStream.java +++ b/src/main/java/io/kestra/plugin/aws/bedrock/ConverseStream.java @@ -19,7 +19,6 @@ import io.kestra.plugin.aws.shared.ConnectionUtils; import io.swagger.v3.oas.annotations.media.Schema; -import jakarta.validation.constraints.NotEmpty; import jakarta.validation.constraints.NotNull; import lombok.*; import lombok.experimental.SuperBuilder; @@ -87,7 +86,7 @@ public class ConverseStream extends AbstractConnection implements RunnableTask>> messages; @Schema( @@ -118,6 +117,9 @@ public Output run(RunContext runContext) throws Exception { } catch (ClassCastException e) { throw new IllegalArgumentException("messages must be a list of {role, content} maps", e); } + if (rawMessages.isEmpty()) { + throw new IllegalArgumentException("messages must not be empty"); + } var sdkMessages = BedrockUtils.buildMessages(rawMessages); diff --git a/src/test/java/io/kestra/plugin/aws/bedrock/ConverseStreamTest.java b/src/test/java/io/kestra/plugin/aws/bedrock/ConverseStreamTest.java index b3bc5a89..33e164e8 100644 --- a/src/test/java/io/kestra/plugin/aws/bedrock/ConverseStreamTest.java +++ b/src/test/java/io/kestra/plugin/aws/bedrock/ConverseStreamTest.java @@ -12,6 +12,8 @@ import io.kestra.core.runners.RunContextFactory; import jakarta.inject.Inject; +import jakarta.validation.MessageInterpolator; +import jakarta.validation.Validation; import software.amazon.awssdk.services.bedrockruntime.BedrockRuntimeAsyncClient; import software.amazon.awssdk.services.bedrockruntime.model.*; @@ -129,4 +131,51 @@ void givenInvalidRole_whenConverseStream_thenThrowsIllegalArgumentException() th assertThrows(IllegalArgumentException.class, () -> spy.run(runContext)); } + + @Test + void givenEmptyMessages_whenConverseStream_thenThrowsExpectedException() { + RunContext runContext = runContextFactory.of(); + + ConverseStream task = ConverseStream.builder() + .id("test-converse-stream-empty-messages") + .type(ConverseStream.class.getName()) + .region(Property.ofValue("us-east-1")) + .modelId(Property.ofValue("anthropic.claude-3-5-sonnet-20241022-v2:0")) + .messages(Property.ofValue(List.of())) + .build(); + + var exception = assertThrows(IllegalArgumentException.class, () -> task.run(runContext)); + + assertThat(exception.getMessage(), is("messages must not be empty")); + } + + @Test + void givenValidMessages_whenValidated_thenDoesNotProduceHv000030() { + ConverseStream task = ConverseStream.builder() + .id("test-converse-stream-validation") + .type(ConverseStream.class.getName()) + .region(Property.ofValue("us-east-1")) + .modelId(Property.ofValue("anthropic.claude-3-5-sonnet-20241022-v2:0")) + .messages(Property.ofValue(List.of(Map.of("role", "user", "content", "Hello!")))) + .build(); + + var violations = Validation.byDefaultProvider() + .configure() + .messageInterpolator(new MessageInterpolator() { + @Override + public String interpolate(String messageTemplate, Context context) { + return messageTemplate; + } + + @Override + public String interpolate(String messageTemplate, Context context, java.util.Locale locale) { + return messageTemplate; + } + }) + .buildValidatorFactory() + .getValidator() + .validate(task); + + assertThat(violations, empty()); + } } diff --git a/src/test/java/io/kestra/plugin/aws/bedrock/ConverseTest.java b/src/test/java/io/kestra/plugin/aws/bedrock/ConverseTest.java index d064568c..8f52e2aa 100644 --- a/src/test/java/io/kestra/plugin/aws/bedrock/ConverseTest.java +++ b/src/test/java/io/kestra/plugin/aws/bedrock/ConverseTest.java @@ -11,11 +11,14 @@ import io.kestra.core.runners.RunContextFactory; import jakarta.inject.Inject; +import jakarta.validation.MessageInterpolator; +import jakarta.validation.Validation; import software.amazon.awssdk.services.bedrockruntime.BedrockRuntimeClient; import software.amazon.awssdk.services.bedrockruntime.model.*; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; @@ -77,4 +80,51 @@ void givenValidMessages_whenConverse_thenOutputContainsContent() throws Exceptio assertThat(output.getInputTokens(), is(10)); assertThat(output.getOutputTokens(), is(15)); } + + @Test + void givenEmptyMessages_whenConverse_thenThrowsExpectedException() { + RunContext runContext = runContextFactory.of(); + + Converse task = Converse.builder() + .id("test-converse-empty-messages") + .type(Converse.class.getName()) + .region(Property.ofValue("us-east-1")) + .modelId(Property.ofValue("anthropic.claude-3-5-sonnet-20241022-v2:0")) + .messages(Property.ofValue(List.of())) + .build(); + + var exception = assertThrows(IllegalArgumentException.class, () -> task.run(runContext)); + + assertThat(exception.getMessage(), is("messages must not be empty")); + } + + @Test + void givenValidMessages_whenValidated_thenDoesNotProduceHv000030() { + Converse task = Converse.builder() + .id("test-converse-validation") + .type(Converse.class.getName()) + .region(Property.ofValue("us-east-1")) + .modelId(Property.ofValue("anthropic.claude-3-5-sonnet-20241022-v2:0")) + .messages(Property.ofValue(List.of(Map.of("role", "user", "content", "Hello!")))) + .build(); + + var violations = Validation.byDefaultProvider() + .configure() + .messageInterpolator(new MessageInterpolator() { + @Override + public String interpolate(String messageTemplate, Context context) { + return messageTemplate; + } + + @Override + public String interpolate(String messageTemplate, Context context, java.util.Locale locale) { + return messageTemplate; + } + }) + .buildValidatorFactory() + .getValidator() + .validate(task); + + assertThat(violations, empty()); + } }