Add defaultEventSource to WorkflowApplication#1556
Conversation
|
@matheusandre1 you must sign your commits with |
Sorry, I always forget about that. |
fjtirado
left a comment
fjtirado
left a comment
There was a problem hiding this comment.
@matheusandre1
Lets make default event source a WorkflowValueResolver, so it returns an URI from the WorkflowContext, TaskContext and Model
Signed-off-by: Matheus André <matheusandr2@gmail.com>
| public class EmitSourceResolver implements WorkflowValueResolver<URI> { | ||
|
|
||
| private static final String SEP = "/"; | ||
| private static final String VER_SEP = ":"; |
There was a problem hiding this comment.
@ricardozanini why a ":" and not a "/"
I think a calculated default source should combine applicationid and definition id and not use any other separator than /, so, in my opinion the code should be
workflow.definition().application().id()+SEP+workflow.definition().id().toString(SEP);
You can remove VER_SEP
| private Optional<URITemplateResolver> templateResolver; | ||
| private Optional<FunctionReader> functionReader; | ||
| private URI defaultCatalogURI; | ||
| private WorkflowValueResolver<URI> defaultEventSource = new EmitSourceResolver(); |
There was a problem hiding this comment.
This should be initialized only if withDefaultEventSource has not being invoked.
| } | ||
|
|
||
| public Builder withDefaultEventSource(String defaultEventSource) { | ||
| return withDefaultEventSource(URI.create(defaultEventSource)); |
There was a problem hiding this comment.
If the string is not a valir URI, application initialization will fail. Im not sure we should have this method to be hones
| WorkflowApplication app = | ||
| WorkflowApplication.builder().withEventConsumer(broker).withEventPublisher(broker).build(); | ||
| try { | ||
| app.workflowDefinition(wf).instance().start().join(); | ||
| } finally { | ||
| app.close(); | ||
| } |
There was a problem hiding this comment.
| WorkflowApplication app = | |
| WorkflowApplication.builder().withEventConsumer(broker).withEventPublisher(broker).build(); | |
| try { | |
| app.workflowDefinition(wf).instance().start().join(); | |
| } finally { | |
| app.close(); | |
| } | |
| try (WorkflowApplication app = | |
| WorkflowApplication.builder().withEventConsumer(broker).withEventPublisher(broker).build()) | |
| { | |
| app.workflowDefinition(wf).instance().start().join(); | |
| } |
| WorkflowApplication app = | ||
| WorkflowApplication.builder() | ||
| .withDefaultEventSource("app.source") | ||
| .withEventConsumer(broker) | ||
| .withEventPublisher(broker) | ||
| .build(); | ||
| app.workflowDefinition(wf).instance().start().join(); | ||
| app.close(); |
There was a problem hiding this comment.
| WorkflowApplication app = | |
| WorkflowApplication.builder() | |
| .withDefaultEventSource("app.source") | |
| .withEventConsumer(broker) | |
| .withEventPublisher(broker) | |
| .build(); | |
| app.workflowDefinition(wf).instance().start().join(); | |
| app.close(); | |
| try (WorkflowApplication app = | |
| WorkflowApplication.builder() | |
| .withDefaultEventSource("app.source") | |
| .withEventConsumer(broker) | |
| .withEventPublisher(broker) | |
| .build()) { | |
| app.workflowDefinition(wf).instance().start().join(); | |
| } |
| WorkflowApplication app = | ||
| WorkflowApplication.builder() | ||
| .withDefaultEventSource( | ||
| (workflow, task, model) -> URI.create("apps/" + workflow.definition().id().name())) | ||
| .withEventConsumer(broker) | ||
| .withEventPublisher(broker) | ||
| .build(); |
There was a problem hiding this comment.
Same here, apparently AI does not know about try {} existance
| List<CloudEvent> events = Collections.synchronizedList(new ArrayList<>()); | ||
| InMemoryEvents broker = new InMemoryEvents(); | ||
| broker.register(eventType, events::add); |
There was a problem hiding this comment.
There is a lot of duplicated code in these test, lets avoid copy paste and use "@beforeeach" annotation
3 participants
Many thanks for submitting your Pull Request ❤️!
What this PR does / why we need it: Closes: #1554
Special notes for reviewers:
Additional information (if needed):