RESTWS-979 - Custom representations are not parsed correctly #655
Conversation
| @@ -539,56 +543,46 @@ public static DelegatingResourceDescription getCustomRepresentationDescription(C | |||
| String def = representation.getRepresentation(); | |||
| def = def.startsWith("(") ? def.substring(1) : def; | |||
| def = def.endsWith(")") ? def.substring(0, def.length() - 1) : def; | |||
There was a problem hiding this comment.
I don't know where this original code came from - I am shown on the blame in github, but I have no recollection writing this and it doesn't look like my style, so I probably moved it from somewhere and that history is lost.
In any event, this seems to work in some cases (obviously it must), but does not work for certain combinations of nested properties. As can be seen below, the original parsing was pretty brittle, splitting on commas even though commas can be used to separate fields both at the top level and within nested objects, and the code is not resilient to all cases.
There was a problem hiding this comment.
I think from the history, you moved it into this class pretty much wholesale from BaseDelegatingResource The bulk of the code is from the initial implementation and then this support for nested properties.
| @@ -539,56 +543,46 @@ public static DelegatingResourceDescription getCustomRepresentationDescription(C | |||
| String def = representation.getRepresentation(); | |||
| def = def.startsWith("(") ? def.substring(1) : def; | |||
| def = def.endsWith(")") ? def.substring(0, def.length() - 1) : def; | |||
There was a problem hiding this comment.
I think from the history, you moved it into this class pretty much wholesale from BaseDelegatingResource The bulk of the code is from the initial implementation and then this support for nested properties.
omod-common/src/main/java/org/openmrs/module/webservices/rest/web/ConversionUtil.java
Outdated
Show resolved
Hide resolved
…web/ConversionUtil.java
|
Very interesting that for all these years we have never got a problem with this. 😊 |
see https://issues.openmrs.org/browse/RESTWS-979
Note, in addition to the unit test included here which demonstrates the issue, I have verified that this also fixes the test that I was attempting to write in EMR API that first caused me to investigate this issue.