-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(document): intrinsic operations #4128
base: main
Are you sure you want to change the base?
Conversation
bundle/default-bundle/src/test/resources/application.properties
Outdated
Show resolved
Hide resolved
connector-sdk/document/src/main/java/io/camunda/document/operation/Base64Operation.java
Outdated
Show resolved
Hide resolved
...on-datatype-document/src/main/java/io/camunda/connector/document/jackson/OperationModel.java
Outdated
Show resolved
Hide resolved
603b7ca
to
a3f95cf
Compare
...ector-sdk/document/src/main/java/io/camunda/operation/DefaultIntrinsicOperationExecutor.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This pull request introduces intrinsic operation support by adding new operation interfaces and refactoring the Jackson module to better support operation deserialization and improve the developer experience.
- Added new intrinsic operation interfaces (IntrinsicOperation, IntrinsicOperationExecutor, and IntrinsicOperationProvider) with stricter input/output typing.
- Implemented new operations, including Base64 and CreateLink, and updated supporting code such as parameter binding and document link generation.
- Updated test and factory files along with minor refactorings in document store implementations.
Reviewed Changes
File | Description |
---|---|
connector-sdk/document/src/main/java/io/camunda/operation/impl/Base64Operation.java | Introduces a base64 conversion operation with proper handling of Document and String inputs. |
connector-sdk/document/src/main/java/io/camunda/operation/IntrinsicOperation.java | Adds the new annotation interface to mark intrinsic operations. |
connector-sdk/document/src/main/java/io/camunda/operation/IntrinsicOperationParameterBinder.java | Implements parameter binding for intrinsic operations with improved type conversion and error reporting. |
connector-sdk/document/src/main/java/io/camunda/operation/IntrinsicOperationProvider.java | Defines the marker interface for intrinsic operation providers. |
connector-sdk/document/src/main/java/io/camunda/operation/DefaultIntrinsicOperationExecutor.java | Provides an executor that discovers and executes intrinsic operations, including duplicate operation detection. |
connector-sdk/document/src/main/java/io/camunda/operation/impl/CreateLinkOperation.java | Adds support for the link creation intrinsic operation with optional TTL. |
connector-sdk/document/src/main/java/io/camunda/operation/IntrinsicOperationParams.java | Updates the parameter structure to use a sealed interface supporting positional parameters. |
connector-sdk/document/src/main/java/io/camunda/document/CamundaDocument.java | Enhances the Document implementation with new generateLink methods. |
connector-sdk/document/src/main/java/io/camunda/document/store/* | Refactors document store implementations to streamline link generation and content retrieval. |
connector-sdk/core/src/test/java/io/camunda/connector/api/json/* | Adjusts tests to reflect changes in JSON deserialization and updated model packages. |
connector-runtime/spring-boot-starter-camunda-connectors/src/main/java/io/camunda/connector/runtime/OutboundConnectorsAutoConfiguration.java | Updates the Jackson module imports to the new package structure. |
Copilot reviewed 60 out of 60 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (3)
connector-sdk/document/src/main/java/io/camunda/operation/DefaultIntrinsicOperationExecutor.java:63
- [nitpick] Consider including method names and additional context in the error message for duplicated operations to improve debugging clarity.
throw new IllegalArgumentException("Operation with name: " + entry.getKey() + " duplicated in providers: " + provider.getClass().getName() + " and " + operationSources.get(entry.getKey()).provider.getClass().getName());
connector-sdk/document/src/main/java/io/camunda/operation/IntrinsicOperationParameterBinder.java:60
- [nitpick] Consider including the expected parameter type in the error message to aid in debugging parameter binding issues.
throw new IllegalArgumentException("Parameter at index " + index + " is required but not provided");
connector-sdk/document/src/main/java/io/camunda/operation/impl/CreateLinkOperation.java:28
- [nitpick] Consider renaming the parameter 'ttl' to 'timeToLive' for consistency with other parts of the codebase.
public String execute(Document document, @Nullable Duration ttl) {
...ector-sdk/document/src/main/java/io/camunda/operation/DefaultIntrinsicOperationExecutor.java
Outdated
Show resolved
Hide resolved
connector-sdk/document/src/main/java/io/camunda/operation/IntrinsicOperation.java
Outdated
Show resolved
Hide resolved
connector-sdk/document/src/main/java/io/camunda/operation/IntrinsicOperationExecutor.java
Outdated
Show resolved
Hide resolved
public String execute(Object input) { | ||
if (input instanceof Document) { | ||
return ((Document) input).asBase64(); | ||
} | ||
if (input instanceof String) { | ||
return Base64.getEncoder().encodeToString(((String) input).getBytes()); | ||
} | ||
throw new IllegalArgumentException( | ||
"Unsupported input type: " + input.getClass() + ". Expected Document or String."); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super nice and concise. Could be even nicer with a switch
statement :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Connector SDK is still on Java 17 😢
connector-sdk/document/src/main/java/io/camunda/operation/impl/CreateLinkOperation.java
Outdated
Show resolved
Hide resolved
connector-sdk/document/src/main/java/io/camunda/operation/impl/GetTextOperation.java
Outdated
Show resolved
Hide resolved
connector-sdk/document/src/main/java/io/camunda/document/store/DocumentLinkCreationRequest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work, nice feature 🚀
Why is the prefixed "chore" and not "feature"?
I tried to test this, but couldn't get it working. I used camunda platform connectors-8.7.0-alpha2
. I did a maven clean install in connectors. I used desktop modeler with this BPMN:
but the issue that is created has not description:
connector-sdk/document/src/main/java/io/camunda/intrinsic/IntrinsicFunctionParams.java
Outdated
Show resolved
Hide resolved
...t/src/main/java/io/camunda/connector/document/jackson/deserializer/AbstractDeserializer.java
Show resolved
Hide resolved
Right, let's change it back to feat, I initially decided to make it a chore because I was considering to split the implementation in several PRs and only get a QA approval for the last one. However, we added more functions in this PR so it makes sense to call it a feature and require QA approval.
This would not work as we decided not to implement the FEEL part yet. For now, if you want to invoke an operation, you need to provide the whole payload in the input, i.e.:
|
QA informationTest Environment
Test ScopePlease describe the test scope, happy path, edge cases that come to your mind, and whatever you think might relevant to test Test DataPlease provide the test data, if needed (files, URLs, code snippets, FEEL expressions) |
56d943c
to
33e1eeb
Compare
@ztefanie it should work now, just a heads up that I also just changed the discriminator field name from operation to function so the payload should look like this now:
|
🚀 Deployment Results 🚀Please find below the results of the latest deployments.
|
Description
For an introduction into intrinsic operations concept, please see #3843
This PR adds intrinsic operation support. It consists of two parts:
IntrinsicOperation
,IntrinsicOperationExecutor
andIntrinsicOperationProvider
interfaces with stricter input/output typing and hopefully a better developer experience)Use-cases considered in the Jackson module:
Related issues
closes #3843
related https://github.com/camunda/team-connectors/issues/1004
Checklist
no milestone
label.