[Fix_#3353] Adding source to process definition data event #3358
Conversation
| @@ -37,9 +40,10 @@ public class ProcessDefinitionRegistration { | |||
| ProcessDefinitionEventRegistry processDefinitionRegistry; | |||
|
|
|||
| @Inject | |||
| public ProcessDefinitionRegistration(Application application, KogitoRuntimeConfig runtimeConfig, Instance<Processes> processes) { | |||
| public ProcessDefinitionRegistration(Application application, KogitoRuntimeConfig runtimeConfig, Instance<Processes> processes, Instance<SourceFilesProvider> sourceFilesProvider) { | |||
There was a problem hiding this comment.
I keep SourceFilesProvider optional, so source addon should still be present for source to be added to the event.
The alternative is to make this mandatory and move SourceFilesProviderFactory to quarkus common extension.
I wanted to be conservative since we are close to the release deadline
There was a problem hiding this comment.
LGTM, a good improvement would be to add an IT for it, maybe on https://github.com/apache/incubator-kie-kogito-apps/blob/main/apps-integration-tests/integration-tests-data-index-service/integration-tests-data-index-service-common/src/test/java/org/kie/kogito/index/AbstractProcessDataIndexIT.java#L173
| sourceFilesProvider.flatMap(provider -> provider.getProcessSourceFile(p.id())).map(s -> { | ||
| try { | ||
| return new String(s.readContents()); | ||
| } catch (IOException e) { | ||
| LOGGER.warn("Error reading source for process {}", p.id(), e); | ||
| return null; | ||
| } | ||
| }).ifPresentOrElse(s -> builder.setSource(s), () -> LOGGER.warn("Not source found for process id {}", p.id())); |
There was a problem hiding this comment.
You can do like other fields using the builder, like .setMetadata(metadata).source(getSource(...))... just to extract this into a method, similar .setNodes(getNodesDefinitions(p)) just to keep it cleaner.
There was a problem hiding this comment.
if source (in case the addon is not in the set of dependencies) is not there, getSource() will return null, and we will be setting source to null in the builder when it is not really needed.
However, I agree that this can be done more readable, please check the new version
@tiagodolphine Thanks for the hint. Added in the linked PR |
* [Fix_#3353] Adding source to process definition data event * [Fix_#3353] Tiagos comments
Fixes #3353
Integration test here apache/incubator-kie-kogito-apps#1958