VMRouter should add message tracking information#6368
VMRouter should add message tracking information#6368rogin wants to merge 4 commits intonextgenhealthcare:developmentfrom
Conversation
|
Basic testing successful.
Test channels: |
| database.enable-read-write-split = true | ||
|
|
||
| # If true, enable routing enhancements for improved message tracking | ||
| routing.enable-enhancements = true |
There was a problem hiding this comment.
Why should this be a configurable option? What harmful behavior could happen if the routing was done for a use case that did not expect it?
There was a problem hiding this comment.
I had in mind any existing scripts that are looking for specific data that may break.
| * | ||
| * TrackingEnhancer | ||
| */ | ||
| private class TrackingEnhancer { |
There was a problem hiding this comment.
Is there any value in making this a not inner class?
There was a problem hiding this comment.
Perhaps for testing. Can certainly do so if folks find it beneficial.
| */ | ||
| private TrackingEnhancer(String channelId, Long messageId, SourceMap sourceMap) { | ||
| this.channelId = channelId != null ? channelId : "NONE"; | ||
| this.messageId = messageId != null ? messageId : -1L; |
There was a problem hiding this comment.
Why is this -1L and not zero?
There was a problem hiding this comment.
I followed the existing template logic which is a reasonable "non-value" value.
| * @return a new Map | ||
| */ | ||
| private Map<String, Object> enrich(Map<String, Object> messageSourceMap) { | ||
| if (messageSourceMap == null) { |
There was a problem hiding this comment.
Leaving the condition is fine but is there any chance of messageSourceMap ever being null?
Should the @Null or @NotNull annotations be used?
There was a problem hiding this comment.
Yes, I see it can come in as null from at least one method.
I don't see other places where @Null and @NotNull are used that I could copy, so I defer to others on properly decorating and validating the code.
Initial attempt to enhance the VMRouter using @tonygermano's existing code template.
Has not yet been tested locally.