Skip to content

VMRouter should add message tracking information#6368

Open
rogin wants to merge 4 commits intonextgenhealthcare:developmentfrom
rogin:enhance-vmrouter
Open

VMRouter should add message tracking information#6368
rogin wants to merge 4 commits intonextgenhealthcare:developmentfrom
rogin:enhance-vmrouter

Conversation

@rogin
Copy link

@rogin rogin commented Nov 28, 2024

Initial attempt to enhance the VMRouter using @tonygermano's existing code template.

Has not yet been tested locally.

@rogin
Copy link
Author

rogin commented Dec 2, 2024

Basic testing successful.

  1. Able to toggle enhancement on via boolean property routing.enable-enhancements
  2. Able to view the new methods in the JS editor
    editor-details
  3. Message tracking info properly attached
    with-enhancement
  4. Omitting or disabling the boolean omits the extra data
    omitted-enhancement-option

Test channels:
2024-11-30 Mirth Backup.xml.txt

@rogin rogin changed the title initial enhancement VMRouter should add message tracking information Dec 2, 2024
database.enable-read-write-split = true

# If true, enable routing enhancements for improved message tracking
routing.enable-enhancements = true
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Author

Choose a reason for hiding this comment

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

I had in mind any existing scripts that are looking for specific data that may break.

*
* TrackingEnhancer
*/
private class TrackingEnhancer {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any value in making this a not inner class?

Copy link
Author

Choose a reason for hiding this comment

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

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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this -1L and not zero?

Copy link
Author

Choose a reason for hiding this comment

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

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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Leaving the condition is fine but is there any chance of messageSourceMap ever being null?

Should the @Null or @NotNull annotations be used?

Copy link
Author

Choose a reason for hiding this comment

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

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants