search

nextgenhealthcare / connect

The swiss army knife of healthcare integration.
Other
921 stars 276 forks source link

[IDEA] Create mirth.properties setting to control wrapping of java objects representing javascript primitives #4630

Open tonygermano opened 3 years ago

tonygermano commented 3 years ago

Is your feature request related to a problem? Please describe. It's very confusing both to new users and more experienced users when there is a mismatch between java objects and javascript primitives. Strict equality does not work, which also affects switch statements. There are numerous issues reported in the forums and slack on the erratic behavior of the replace() method when the user is unaware of which type of string they are calling it on.

Rhino already has an option to change this behavior. With the option disabled (it is enabled by default) certain types returned from field access or method calls on java objects will be automatically converted to javascript primitives according to the mappings below.

Describe your use case Demonstrated in rhino shell

// default behavior
js> var channelMap = new java.util.LinkedHashMap()
js> channelMap.putAll({str: 'mirth', bool: false, num: 1})
js> channelMap.values().toArray().map(x => typeof x)
object,object,object
js> channelMap.values().toArray().map(x => x.getClass().getName())
java.lang.String,java.lang.Boolean,java.lang.Double
// the following tests all have unexpected results due to conversion to java Objects
js> channelMap.get('str') === 'mirth'
false
js> channelMap.get('bool') ? 'truthy' : 'falsy'
truthy
js> channelMap.get('num') === 1
false

// exact same object after turning off JavaPrimitiveWrap
js> org.mozilla.javascript.Context.getCurrentContext().getWrapFactory().setJavaPrimitiveWrap(false)
js> channelMap.values().toArray().map(x => typeof x)
string,boolean,number
js> channelMap.get('str') === 'mirth'
true
js> channelMap.get('bool') ? 'truthy' : 'falsy'
falsy
js> channelMap.get('num') === 1
true

// java objects can still be created explicitly, but method return values are converted to javascript primitives
js> var s = new java.lang.String()
js> s.getClass().getName()
java.lang.String
js> typeof s.toString()
string

Describe the solution you'd like Create a mirth.properties setting to control setting the javaPrimitiveWrap property on the context's WrapFactory instance before evaluating the javascript code. This would allow people to keep the current behavior if their code is dependent on it. I recommend making this the default setting on new installations, but not migrations, similar to how ES6 mode was introduced.

Describe alternatives you've considered Frequently workarounds are employed to always manually convert input to javascript strings rather than trying to understand all of the situations that may produce one type of string vs the other. Many people still don't realize the same problem can occur with numbers and booleans because it happens less frequently than with strings.

Additional context There is a new quirk that would arise, though I think would be a much less common issue than the current behavior. When explicitly placing java objects into a java collection or map, they will be converted to their javascript cousins on retrieval. The biggest surprise is that all listed subclasses of java.lang.Number are treated as javascript numbers, not only Double. The exception starting in Rhino 1.7.14 is that java.math.BigInteger will be converted to a javascript bigint instead of number.

Currently Byte is not accounted for and there is a potential loss of precision with including Long in the list, both of which I am addressing with the Rhino project.

A user that has this new setting enabled, but must maintain the original classes could manually flip the flag for that particular script as I have demonstrated in the example above.

jonbartels commented 3 years ago

Would this change any behaviors for things like numeric IDs in strings?

tonygermano commented 3 years ago

The fact that it's a numeric id shouldn't be relevant. If you are retrieving a numeric id from a java method that returns a java.lang.String, it would instead return a javascript string. You'd likely be using parseInt if you needed it as a number, which wouldn't be affected. The only thing I can think of that might be an issue is if you are calling a java.lang.String specific method on the value, like matches to do validation that you really have a numeric id.

Once again, if the optional setting were to cause a problem with a specific channel, you can always manually flip the flag in the script to return to the current behavior for that particular script without preventing everything from using the feature.