Add option to specify custom fragment path delimiters #697
Conversation
…andard JSON Pointers
|
Thanks for submitting this. I'll add some comments. |
| @@ -146,6 +146,7 @@ | |||
|
|
|||
| private boolean formatDateTimes = false; | |||
|
|
|||
| private String customSeparatorCharacters = "#/."; | |||
There was a problem hiding this comment.
Could you rename this property everywhere to refFragmentPathDelimiters? I think this makes it clear where this property will actually be used. We also have another 'delimiters' config option so I feel like we should use that word in preference for consistency.
| @@ -408,6 +408,12 @@ | |||
| </td> | |||
| <td align="center" valign="top">No (default <code>false</code>)</td> | |||
| </tr> | |||
| <tr> | |||
| <td valign="top">customSeparatorCharacters</td> | |||
| <td valign="top">Separators characters to be used to split JSON Pointers ($ref). | |||
There was a problem hiding this comment.
There's a small typo here ("Separators characters"). To make it clear that this is not an array or list, I think this text should mention that it's a string. How about this as a description:
A string containing any characters that should act as path delimiters when resolving $ref fragments. By default,
#,/and.are used in an attempt to support JSON Pointer and JSON Path.
There was a problem hiding this comment.
This description can be used in a variety of places.
| @@ -418,4 +421,9 @@ public boolean isFormatDateTimes() { | |||
| return formatDateTimes; | |||
| } | |||
|
|
|||
| @Override | |||
| public String isCustomSeparatorCharacters() { | |||
There was a problem hiding this comment.
There are a few getters in this and other classes that have an incorrect is prefix instead of get. This and others should be get.
| ClassLoader resultsClassLoader = schemaRule.generateAndCompile("/schema/properties/nonStandardRef.json", "com.example", | ||
| config("customSeparatorCharacters", "#/")); | ||
|
|
||
| assertThat(resultsClassLoader.loadClass("com.example.Foo").getName(), is("com.example.Foo")); |
There was a problem hiding this comment.
These assertions are redundant, two plain calls to loadClass will suffice as this will throw an exception if the class is missing.
|
Applied the requested changes, let me know if there's anything else. |
joelittlejohn
changed the title
|
Thanks! 👍 |
This adds the option suggested in #688, so nonstandard JSON Pointers found in $ref can be split correctly.
The option's default is the current value,
#/., so it should not break any existing projects.Let me know if something is missing and needs to be done before merging, also let me know if the wording of the descriptions is a bit weird.