feature(aws-s3-connector): AWS s3 connector #3744

mathias-vandaele · 2024-12-04T14:48:36Z

Description

Related issues

closes #

Checklist

PR has a milestone or the no milestone label.

johnBgood

Some questions, overall great!

johnBgood · 2024-12-10T09:32:54Z

connectors/aws/aws-s3/src/main/java/io/camunda/connector/aws/s3/S3ConnectorFunction.java

+      @ElementTemplate.PropertyGroup(id = "uploadObject", label = "Upload an object"),
+      @ElementTemplate.PropertyGroup(id = "downloadObject", label = "Download an object"),
+    },
+    documentationRef = "https://docs.camunda.io/docs/",


Don't forget to update this link

johnBgood · 2024-12-10T10:23:51Z

connectors/aws/aws-s3/src/main/java/io/camunda/connector/aws/s3/model/DeleteS3Action.java

+            feel = Property.FeelMode.optional,
+            binding = @TemplateProperty.PropertyBinding(name = "action.bucket"))
+        @Valid
+        @NotNull


johnBgood · 2024-12-10T10:24:12Z

connectors/aws/aws-s3/src/main/java/io/camunda/connector/aws/s3/model/DeleteS3Action.java

+            tooltip = "Bucket from where an object should be deleted",
+            feel = Property.FeelMode.optional,
+            binding = @TemplateProperty.PropertyBinding(name = "action.bucket"))
+        @Valid


Not needed I guess

johnBgood · 2024-12-10T10:24:28Z

connectors/aws/aws-s3/src/main/java/io/camunda/connector/aws/s3/model/DeleteS3Action.java

+            tooltip = "Key of the object which should be deleted",
+            feel = Property.FeelMode.optional,
+            binding = @TemplateProperty.PropertyBinding(name = "action.key"))
+        @Valid


johnBgood · 2024-12-10T10:24:55Z

connectors/aws/aws-s3/src/main/java/io/camunda/connector/aws/s3/model/DownloadS3Action.java

+            label = "Create document",
+            id = "downloadActionAsFile",
+            group = "downloadObject",
+            tooltip = "....",


johnBgood · 2024-12-10T10:36:33Z

connectors/aws/aws-s3/src/main/java/io/camunda/connector/aws/s3/core/S3Executor.java

+import software.amazon.awssdk.services.s3.model.GetObjectResponse;
+import software.amazon.awssdk.services.s3.model.PutObjectRequest;
+
+public class S3Executor {


maybe can we move the response package in the model package, not sure how we do usually

johnBgood · 2024-12-10T10:37:13Z

connectors/aws/aws-s3/src/test/java/io/camunda/connector/aws/s3/S3ConnectorFunctionTest.java

+  @MethodSource("loadUploadActionVariables")
+  void executeUploadActionReturnsCorrectResult(String variables) {
+
+    var bedrockConnectorFunction = new S3ConnectorFunction();


should rename this :p

johnBgood · 2024-12-10T10:37:28Z

connectors/aws/aws-s3/src/test/java/io/camunda/connector/aws/s3/S3ConnectorFunctionTest.java

+  @MethodSource("loadDownloadActionVariables")
+  void executeDownloadActionReturnsCorrectResult(String variables) {
+
+    var bedrockConnectorFunction = new S3ConnectorFunction();


johnBgood · 2024-12-10T10:37:37Z

connectors/aws/aws-s3/src/test/java/io/camunda/connector/aws/s3/S3ConnectorFunctionTest.java

+  @MethodSource("loadDeleteActionVariables")
+  void executeDeleteActionReturnsCorrectResult(String variables) {
+
+    var bedrockConnectorFunction = new S3ConnectorFunction();


johnBgood · 2024-12-10T10:39:18Z

connectors/aws/aws-s3/src/test/java/io/camunda/connector/aws/s3/BaseTest.java

+
+public class BaseTest {
+
+  public static Stream<String> loadUploadActionVariables() {


What about creating integration tests using https://java.testcontainers.org/modules/localstack/?

Yes, I might created a separated PR for integration tests.

johnBgood · 2024-12-11T11:30:12Z

QA information

Test Environment

Environment:
- SaaS only
- SM only
- Both
Connectors version: <TO_BE_REPLACED>

Test Scope

Please describe the test scope, happy path, edge cases that come to your mind, and whatever you think might relevant to test

Test Data

Please provide the test data, if needed (files, URLs, code snippets, FEEL expressions)

sbuettner · 2024-12-17T08:42:50Z

connectors/aws/aws-s3/element-templates/aws-s3-outbound-connector.json

+    "value" : "uploadObject",
+    "group" : "action",
+    "binding" : {
+      "name" : "actionDiscriminator",


Shouldnt this be part of the action object? Like:

{ "action": {"type":"upload", ...}}

sbuettner · 2024-12-17T08:44:06Z

connectors/aws/aws-s3/src/main/java/io/camunda/connector/aws/s3/model/request/S3Request.java

+  public S3Action getData() {
+    return action;
+  }


Why do we call this getData and not getAction?

sbuettner · 2024-12-17T08:46:22Z

...ctors/aws/aws-s3/src/main/java/io/camunda/connector/aws/s3/model/request/UploadS3Action.java

+@TemplateSubType(id = "uploadObject", label = "Upload document")
+public record UploadS3Action(


We now have 3 names 😄
id: uploadObject,
label: Upload document
record: UploadS3Action

Should we go with a single one? Either PutObject or UploadObject?

sbuettner · 2024-12-17T08:47:39Z

...rs/aws/aws-s3/src/main/java/io/camunda/connector/aws/s3/model/response/DownloadResponse.java

+
+import io.camunda.document.Document;
+
+public record DownloadResponse(String bucket, String key, Document document, Object content) {}


@mathias-vandaele Is it either a Document or the content?
If so please create a simple sealed type which indicates that.

sbuettner · 2024-12-17T08:48:38Z

...ectors/email/src/main/java/io/camunda/connector/email/client/jakarta/utils/JakartaUtils.java

    switch (bodyPart.getContent()) {
-      case InputStream attachment when bodyPart
-              .getDisposition()
-              .equalsIgnoreCase(Part.ATTACHMENT) ->
+      case InputStream attachment when Part.ATTACHMENT.equalsIgnoreCase(
+              bodyPart.getDisposition()) ->
+          emailBodyBuilder.addAttachment(
+              new EmailAttachment(
+                  attachment,
+                  bodyPart.getFileName(),
+                  new ContentType(bodyPart.getContentType()).getBaseType()));
+      case String textAttachment when Part.ATTACHMENT.equalsIgnoreCase(bodyPart.getDisposition()) ->


Is this change related to S3?

johnBgood

One question, but happy to approve :)

johnBgood · 2024-12-19T16:20:55Z

connectors/aws/aws-s3/src/main/java/io/camunda/connector/aws/s3/core/S3Executor.java

+    return new UploadResponse(
+        uploadObject.bucket(),
+        uploadObject.key(),
+        String.format("https://%s.s3.amazonaws.com/%s", uploadObject.bucket(), uploadObject.key()));


we can prob introduce a constant here

johnBgood · 2024-12-19T16:24:16Z

connectors/aws/aws-s3/src/main/java/io/camunda/connector/aws/s3/core/S3Executor.java

+      String bucket, String key, ResponseInputStream<GetObjectResponse> responseResponseInputStream)
+      throws IOException {
+    byte[] rawBytes = responseResponseInputStream.readAllBytes();
+    return switch (responseResponseInputStream.response().contentType()) {


not sure here, but be aware that sometimes the case is weird (uppercase letters etc), and also we might have application/json; charset=utf-8. Maybe it's not applicable here though.

Maybe should we use MimeType class for this

The ContentType returned should always be standard

mathias-vandaele marked this pull request as ready for review December 4, 2024 16:30

mathias-vandaele requested a review from a team as a code owner December 4, 2024 16:30

mathias-vandaele self-assigned this Dec 4, 2024

mathias-vandaele added this to the 8.7.0-alpha3 milestone Dec 4, 2024

mathias-vandaele changed the title ~~Aws s3 connector~~ feature(aws-s3-connector): AWS s3 connector Dec 4, 2024

mathias-vandaele force-pushed the aws-s3-connector branch from f771d56 to c0c81b1 Compare December 4, 2024 16:31

mathias-vandaele requested review from sbuettner and johnBgood December 10, 2024 08:20

johnBgood reviewed Dec 10, 2024

View reviewed changes

mathias-vandaele force-pushed the aws-s3-connector branch from 74ead97 to cfe069d Compare December 11, 2024 08:35

mathias-vandaele added deploy-preview component:qa Task containing all details related to QA qa:required Will trigger the QA workflow labels Dec 11, 2024

github-actions bot temporarily deployed to connectors-aws-s3-connecto-c8sm December 11, 2024 11:30 Destroyed

github-actions bot temporarily deployed to connectors-aws-s3-connecto-c8sm December 11, 2024 11:34 Destroyed

sbuettner reviewed Dec 17, 2024

View reviewed changes

github-actions bot temporarily deployed to connectors-aws-s3-connecto-c8sm December 18, 2024 10:14 Destroyed

mathias-vandaele force-pushed the aws-s3-connector branch from 809fa3f to ecc2b88 Compare December 18, 2024 13:24

github-actions bot temporarily deployed to connectors-aws-s3-connecto-c8sm December 18, 2024 13:24 Destroyed

github-actions bot temporarily deployed to connectors-aws-s3-connecto-c8sm December 18, 2024 13:46 Destroyed

mathias-vandaele added 5 commits December 19, 2024 11:51

feature(s3-connector): initial commit

69f51a6

feature(s3-connector): checkpoint s3 connector

bbd529e

feature(s3-connector): checkpoint s3 connector

7a52539

feature(s3-connector): checkpoint s3 connector

58c4e3d

feature(s3-connector): checkpoint s3 connector 3

bc406ba

mathias-vandaele added 6 commits December 19, 2024 11:51

feature(s3-connector): all features done

491244c

feature(s3-connector): correcting after Jonathan comments

42db6b9

feature(s3-connector): minor corrections

3f3b3c6

feature(s3-connector): minor corrections before rebase

0a9c23f

feature(s3-connector): corrections

cec8e47

feature(s3-connector): corrections 2

f122b34

mathias-vandaele force-pushed the aws-s3-connector branch from 058a8c3 to f122b34 Compare December 19, 2024 10:51

github-actions bot temporarily deployed to connectors-aws-s3-connecto-c8sm December 19, 2024 10:51 Destroyed

johnBgood approved these changes Dec 19, 2024

View reviewed changes

mathias-vandaele added this pull request to the merge queue Dec 19, 2024

Merged via the queue into main with commit 9b775d1 Dec 19, 2024
12 checks passed

mathias-vandaele deleted the aws-s3-connector branch December 19, 2024 18:13

johnBgood unassigned mathias-vandaele Jan 10, 2025

slolatte assigned Szik Jan 10, 2025

johnBgood assigned mathias-vandaele and unassigned Szik Jan 10, 2025

johnBgood added qa:required Will trigger the QA workflow and removed qa:required Will trigger the QA workflow labels Jan 10, 2025

camunda deleted a comment from slolatte Jan 10, 2025

johnBgood added qa:required Will trigger the QA workflow and removed qa:required Will trigger the QA workflow labels Jan 10, 2025

johnBgood assigned Szik Jan 10, 2025

johnBgood removed deploy-preview qa:required Will trigger the QA workflow labels Jan 10, 2025

johnBgood unassigned Szik Jan 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature(aws-s3-connector): AWS s3 connector #3744

feature(aws-s3-connector): AWS s3 connector #3744

mathias-vandaele commented Dec 4, 2024

johnBgood left a comment

johnBgood Dec 10, 2024

johnBgood Dec 10, 2024

johnBgood Dec 10, 2024

johnBgood Dec 10, 2024

johnBgood Dec 10, 2024

johnBgood Dec 10, 2024

johnBgood Dec 10, 2024

johnBgood Dec 10, 2024

johnBgood Dec 10, 2024

johnBgood Dec 10, 2024

mathias-vandaele Dec 11, 2024

johnBgood commented Dec 11, 2024

sbuettner Dec 17, 2024 •

edited

Loading

sbuettner Dec 17, 2024

sbuettner Dec 17, 2024

sbuettner Dec 17, 2024

sbuettner Dec 17, 2024

johnBgood left a comment

johnBgood Dec 19, 2024

johnBgood Dec 19, 2024

johnBgood Dec 19, 2024

mathias-vandaele Dec 19, 2024


		public class BaseTest {

		public static Stream<String> loadUploadActionVariables() {

		@TemplateSubType(id = "uploadObject", label = "Upload document")
		public record UploadS3Action(


		import io.camunda.document.Document;

		public record DownloadResponse(String bucket, String key, Document document, Object content) {}

feature(aws-s3-connector): AWS s3 connector #3744

feature(aws-s3-connector): AWS s3 connector #3744

Conversation

mathias-vandaele commented Dec 4, 2024

Description

Related issues

Checklist

johnBgood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnBgood commented Dec 11, 2024

QA information

Test Environment

Test Scope

Test Data

sbuettner Dec 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnBgood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbuettner Dec 17, 2024 •

edited

Loading