Skip to content

feat: Implement correlation on event filter#1386

Open
matheusandre1 wants to merge 2 commits into
serverlessworkflow:mainfrom
matheusandre1:feature/issue1206
Open

feat: Implement correlation on event filter#1386
matheusandre1 wants to merge 2 commits into
serverlessworkflow:mainfrom
matheusandre1:feature/issue1206

Conversation

@matheusandre1
Copy link
Copy Markdown
Contributor

@matheusandre1 matheusandre1 commented May 18, 2026

Many thanks for submitting your Pull Request ❤️!

What this PR does / why we need it: Closes: #1206

Special notes for reviewers:

Additional information (if needed):

@matheusandre1 matheusandre1 marked this pull request as ready for review May 18, 2026 21:56
@matheusandre1 matheusandre1 requested a review from fjtirado as a code owner May 18, 2026 21:56
Copilot AI review requested due to automatic review settings May 18, 2026 21:56
Copilot AI reviewed May 18, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

fjtirado
fjtirado reviewed May 19, 2026
View reviewed changes
Comment thread impl/test/src/test/java/io/serverlessworkflow/impl/test/DSLWorkflowSamplesTest.java Outdated
fjtirado
fjtirado reviewed May 19, 2026
View reviewed changes
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/DefaultCloudEventPredicate.java Outdated
fjtirado
fjtirado reviewed May 19, 2026
View reviewed changes
Comment thread fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/UseBuilder.java
fjtirado
fjtirado reviewed May 19, 2026
View reviewed changes
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/AbstractTypeConsumer.java Outdated
fjtirado
fjtirado requested changes May 19, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@fjtirado fjtirado left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to separate the DSL gap PR you were working on from the Correlation one.

@matheusandre1 matheusandre1 force-pushed the feature/issue1206 branch 2 times, most recently from d8d98e5 to 77143c1 Compare May 19, 2026 16:00
@matheusandre1 matheusandre1 marked this pull request as draft May 19, 2026 16:00
@matheusandre1 matheusandre1 marked this pull request as ready for review May 19, 2026 16:01
Copilot AI review requested due to automatic review settings May 19, 2026 16:01
@matheusandre1 matheusandre1 marked this pull request as draft May 19, 2026 16:01
Copilot AI reviewed May 19, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/AbstractTypeConsumer.java Outdated
Comment thread impl/test/src/test/java/io/serverlessworkflow/impl/test/CorrelationTest.java Outdated
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/CorrelationPredicate.java Outdated
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/CorrelationPredicate.java Outdated
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/CorrelationPredicate.java Outdated
Comment thread impl/test/src/test/java/io/serverlessworkflow/impl/test/CorrelationTest.java Outdated
Comment thread impl/test/src/test/java/io/serverlessworkflow/impl/test/CorrelationTest.java Outdated
@matheusandre1
Copy link
Copy Markdown
Contributor Author

matheusandre1 commented May 19, 2026

@fjtirado I will send the other file separately after you review this one.

fjtirado
fjtirado reviewed May 20, 2026
View reviewed changes
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/AbstractTypeConsumer.java Outdated
fjtirado
fjtirado reviewed May 20, 2026
View reviewed changes
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/CorrelationPredicate.java Outdated
fjtirado
fjtirado reviewed May 20, 2026
View reviewed changes
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/AbstractTypeConsumer.java Outdated
fjtirado
fjtirado reviewed May 20, 2026
View reviewed changes
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/CorrelationPredicate.java Outdated
fjtirado
fjtirado reviewed May 20, 2026
View reviewed changes
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/AbstractTypeConsumer.java Outdated
fjtirado
fjtirado reviewed May 26, 2026
View reviewed changes
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/AbstractTypeConsumer.java Outdated
Copilot AI review requested due to automatic review settings May 26, 2026 12:09
Copilot AI reviewed May 26, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/AbstractTypeConsumer.java
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/AbstractTypeConsumer.java
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/TypeEventRegistrationBuilder.java Outdated
fjtirado
fjtirado reviewed May 26, 2026
View reviewed changes
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowMutableInstance.java Outdated
fjtirado
fjtirado reviewed May 26, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@fjtirado fjtirado left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matheusandre1 Please remove changes in WorkflowMutalbeInstance, we are almost there!!!

fjtirado
fjtirado reviewed May 26, 2026
View reviewed changes
Comment thread pom.xml
Copy link
Copy Markdown
Collaborator

@fjtirado fjtirado May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the changes of version in these three poms intentional?

Copy link
Copy Markdown
Contributor Author

@matheusandre1 matheusandre1 May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, It was a poorly done rebase on my part; I didn't perform a git push force, and then I had to cherry-pick your changes (I had never done that before).

Copy link
Copy Markdown
Collaborator

@fjtirado fjtirado May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the version is already <version.org.junit.jupiter>6.1.0</version.org.junit.jupiter> in main, can not you avoid this files appearing in the review?

@fjtirado
Copy link
Copy Markdown
Collaborator

fjtirado commented May 26, 2026

@matheusandre1 You need to remove the three commits that bump the versions

Copilot AI review requested due to automatic review settings May 26, 2026 23:21
Copilot AI reviewed May 26, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 7 comments.

Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/CorrelationPredicate.java
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/AbstractTypeConsumer.java Outdated
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/AbstractTypeConsumer.java Outdated
Comment thread impl/test/src/test/java/io/serverlessworkflow/impl/test/CorrelationTest.java
Comment thread impl/test/src/test/resources/workflows-samples/listen-correlate.yaml Outdated
Comment thread impl/test/src/test/java/io/serverlessworkflow/impl/test/CorrelationTest.java Outdated
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/CorrelationPredicate.java
Copilot AI review requested due to automatic review settings May 27, 2026 04:32
Copilot AI reviewed May 27, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Comment thread impl/test/src/test/java/io/serverlessworkflow/impl/test/CorrelationTest.java Outdated
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/CorrelationPredicate.java
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/AbstractTypeConsumer.java Outdated
Comment thread impl/test/src/test/resources/workflows-samples/listen-correlate.yaml Outdated
@matheusandre1
Copy link
Copy Markdown
Contributor Author

matheusandre1 commented May 27, 2026

This copilot is driving me crazy.

fjtirado
fjtirado reviewed May 27, 2026
View reviewed changes
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowInstance.java Outdated
fjtirado
fjtirado reviewed May 27, 2026
View reviewed changes
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowMutableInstance.java Outdated
fjtirado
fjtirado reviewed May 27, 2026
View reviewed changes
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/AbstractTypeConsumer.java Outdated
fjtirado
fjtirado reviewed May 27, 2026
View reviewed changes
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/AbstractTypeConsumer.java
if (builder.type() == null) {
registerToAll(ce);
return new TypeEventRegistration(null, ce, null, workflow, task);
return new TypeEventRegistration(null, ce, ALWAYS_TRUE, workflow, task);
Copy link
Copy Markdown
Collaborator

@fjtirado fjtirado May 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know the AI was complaining, but this was not really needed, since the predicate was not used for all registration scenario and it was null on purpose.
We can keep your change since there is not a big difference between an unused null and unused anonymous inner class reference, but for future PRS please read my explanations when I resolve a not applicable AI comment.

@fjtirado
Copy link
Copy Markdown
Collaborator

fjtirado commented May 27, 2026

@matheusandre1 Lest change the name of new method in WorkflowInstance to removeMetadata
Just that and I will merge.

Copilot AI review requested due to automatic review settings May 27, 2026 13:34
Copilot AI reviewed May 27, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.

Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/CorrelationPredicate.java
Comment on lines +67 to +69
String stateKey(TaskContext task) {
return expectResolver == null ? correlationStateKey(task) : null;
}
Copy link
Copy Markdown
Collaborator

@fjtirado fjtirado May 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider statekey to return optional and ignore the rest of the comment

Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/AbstractTypeConsumer.java
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/CorrelationPredicate.java
Comment thread impl/test/src/test/java/io/serverlessworkflow/impl/test/CorrelationTest.java
List<?> output = (List<?>) outputValue;
assertThat(output).hasSize(1);
assertThat(output.get(0)).isInstanceOf(Map.class);
Map<String, Object> eventData = (Map<String, Object>) output.get(0);
Copy link
Copy Markdown
Collaborator

@fjtirado fjtirado May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, lets add the unchecked so this guy shut up

Comment thread impl/test/src/test/java/io/serverlessworkflow/impl/test/CorrelationTest.java
.build();
}

private static Workflow listenCorrelateNoExpectWorkflow() {
Copy link
Copy Markdown
Collaborator

@fjtirado fjtirado May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense, but you can ignore since it is test code after all

Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/events/AbstractTypeConsumer.java
}
});
}
cleanupCorrelationState(registration);
Copy link
Copy Markdown
Collaborator

@fjtirado fjtirado May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats a valid concern, Im not really sure we should remove any correlation data associated to the instance.

Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/expressions/AbstractExpressionFactory.java
matheusandre1 and others added 2 commits May 27, 2026 10:39
@matheusandre1
feat: Implement correlation on event filter
75fb4fe 
- Add CorrelationPredicate for evaluating correlation expressions
- Add correlate support in AbstractEventFilterBuilder and AbstractEventFilterSpec
- Update TypeEventRegistration and TypeEventRegistrationBuilder with correlation predicates
- Implement correlation matching in AbstractTypeConsumer
- Add CorrelationTest and listen-correlate.yaml
- Add correlate tests in WorkflowBuilderTest and DSLTest

Signed-off-by: Matheus André <matheusandr2@gmail.com>
@fjtirado @matheusandre1
Review comments
f3a803a 
Signed-off-by: Matheus André <matheusandr2@gmail.com>
@fjtirado
Copy link
Copy Markdown
Collaborator

fjtirado commented May 27, 2026

@matheusandre1 See my comments about remove and my comments over AI comments ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fjtirado fjtirado fjtirado approved these changes

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Feat: Implement correlation on event filter

3 participants

@matheusandre1 @fjtirado