jschon icon indicating copy to clipboard operation
jschon copied to clipboard

Need support for error message while forming a `Node` class object in `Jsonpatch`

Open dharmahulk opened this issue 3 years ago • 1 comments

Hi, I was working on removing property values from a json for a given JsonPointer using jsonpatch.remove from main branch / jsonpatch.apply_remove from v9.0

example_json = {
    "foo": variable
}
json_pointer = JsonPointer("/foo/{}".format(pointer))

Scenarios:

  1. While forming a Node class why do we have an assert statemet. In production all the assert statements will be removed.
  2. When the variable has a value of None or integer and assertion was removed In that case we will be getting an error as AttributeError: Node object has no attribute type in line 256
  3. Are we considering string as list of characters (Sequence). when the Variable has a value of String and the pointer has a integer value less than length of the string we will get a error as TypeError: str object doesn't support item deletion in line 260
  4. When the Variable has a value of String and the pointer has a integer value greater than or equal to the length of the string. we will get error as jschon.exceptions.JSONPatchError: Invalid array index {pointer_value} in line 219

Note: In all the scenarios listed above we are giving a invalid JsonPointer for removing the value from json. Whether can we get a error response which says Invalid jsonpointer was passed {str(jsonpointer)}

Doubt:

  1. In line 210 why we are not checking whether the given key can be converted into integer or not by using isdigit()

dharmahulk avatar Oct 01 '22 15:10 dharmahulk

Thanks for raising this issue. I agree with all your points and will look into improving the handling of invalid pointers and other edge cases / ambiguities.

The jsonpatch module in fact has the lowest test coverage (~65%) of all jschon modules, so there's a clear case for more comprehensive testing of the JSON patch functionality, too.

marksparkza avatar Oct 15 '22 07:10 marksparkza