Use JsonPointer syntax - #348
Conversation
|
JsonPointer syntax only needs to be used for InEnum.BODY. |
What if we have a query parameter which would be a list, shouldn't we add the index in the name? And what I mean by that is of the query param is named 'parameter.id', if we have an issue with the second member of the list, then the name in the issue should be 'parameter.id/1'? |
I have applied changes to the implementation to allow "." in name |
I think it's fine to keep using |
|
| public class JsonPointerUtil { | ||
|
|
||
| // e.g: /, field, /field, /field/0, /field/0/nested | ||
| private static final Pattern JSON_POINTER_PATTERN = Pattern.compile("/+[a-zA-Z0-9-]*+(/[a-zA-Z0-9-]++)*+"); |
There was a problem hiding this comment.
Maybe we are too strict with this regex (e.g. allowing underscores).
Also, technically the empty string "" is also a valid JSON Pointer. It can be used to point to the body itself.
Currently empty string will be converted to null in transformName.
There was a problem hiding this comment.
Maybe it's sufficient to check if it's either an empty string or starts with /.
We don't really need full JSON Pointer syntax validation, it's only a heuristic to check whether we should trigger auto-conversion.
| int index = 0; | ||
| for (String ssin : ssins.getValue()) { | ||
| ssin(new Input<>(ssins.getIn(), ssins.getName() + "[" + index + "]", ssin)); | ||
| String name = JsonPointerUtil.transformName(ssins.getIn(), |
There was a problem hiding this comment.
Shouldn't we remove the transformName call here?
We want auto-conversion with a warning when this is called with Input.body("ssins", myList); instead of "/ssins" no? Same for the refData methods below.
| String indexFormat = ProblemConfig.isJsonPointerEnabled() && in == InEnum.BODY ? ("/" + source.indexOf(value)) | ||
| : ("[" + source.indexOf(value) + "]"); | ||
| String nameWithIndex = name + indexFormat; | ||
| return referencedResourceNotFound(in, JsonPointerUtil.transformName(in, nameWithIndex), value); |
There was a problem hiding this comment.
Same here, shouldn't we remove the transformName call and rely on auto-conversion with warning instead?
| } | ||
|
|
||
| @Test | ||
| void bodyProperty() throws Exception { |
There was a problem hiding this comment.
You could have used ParametrizedTest to pass the jsonPointer enable flag. But like this is fine for me to. You can keep what you prefer.
| } | ||
| } | ||
|
|
||
| private String autoConvertToJsonPointerIfNeeded(InEnum in, String propertyPath) { |
There was a problem hiding this comment.
It is strange to overload a method but that do not really to the same. I would personally choose another name. Why not convertToJsonPointerIfNeeded() ?
* Rename to convertToJsonPointerIfNeeded * Don't call transformName for user-provided names, rely on auto-covnersion instead * Simplified JSON Pointer detection and accept "" as valid JSON Pointer * Extract JsonPointerUtil.addIndex
|
3 participants
No description provided.