Refactor JSON Pointer resolution

Author: jg-rp

CHANGELOG.md

@@ -4,7 +4,7 @@
 
 **Fixes**
 
-- Fixed JSON pointers using negative indices. The JSON Pointer specification (RFC 6901) does not allow negative array indexes. We now raise a `JSONPointerIndexError` if a JSON Pointer attempts to resolve an array item with a negative index. See [#115](https://github.com/jg-rp/python-jsonpath/issues/115).
+- Fixed JSON pointers using negative indices. The JSON Pointer specification (RFC 6901) does not allow negative array indexes. We now raise a `JSONPointerIndexError` if a JSON Pointer attempts to resolve an array item with a negative index. See [#115](https://github.com/jg-rp/python-jsonpath/issues/115). For anyone needing JSON Pointers that support negative indexes, set `JSONPointer.min_int_index` to a suitably negative integer, like `JSONPointer.min_int_index = -(2**53) + 1`.
 
 ## Version 2.0.0
 

jsonpath/exceptions.py

@@ -160,21 +160,21 @@ class JSONPointerIndexError(JSONPointerResolutionError, IndexError):
     """An exception raised when an array index is out of range."""
 
     def __str__(self) -> str:
-        return f"pointer index error {super().__str__()}"
+        return f"pointer index error: {super().__str__()}"
 
 
 class JSONPointerKeyError(JSONPointerResolutionError, KeyError):
     """An exception raised when a pointer references a mapping with a missing key."""
 
     def __str__(self) -> str:
-        return f"pointer key error {super().__str__()}"
+        return f"pointer key error: {super().__str__()}"
 
 
 class JSONPointerTypeError(JSONPointerResolutionError, TypeError):
     """An exception raised when a pointer resolves a string against a sequence."""
 
     def __str__(self) -> str:
-        return f"pointer type error {super().__str__()}"
+        return f"pointer type error: {super().__str__()}"
 
 
 class RelativeJSONPointerError(Exception):

jsonpath/pointer.py

@@ -10,6 +10,7 @@
 from typing import Any
 from typing import Iterable
 from typing import Mapping
+from typing import Optional
 from typing import Sequence
 from typing import Tuple
 from typing import Union
@@ -58,14 +59,14 @@ class JSONPointer:
         max_int_index (int): The maximum integer allowed when resolving array
             items by index. Defaults to `(2**53) - 1`.
         min_int_index (int): The minimum integer allowed when resolving array
-            items by index. Defaults to `-(2**53) + 1`.
+            items by index. Defaults to `0`.
     """
 
     __slots__ = ("_s", "parts")
 
     keys_selector = "~"
     max_int_index = (2**53) - 1
-    min_int_index = -(2**53) + 1
+    min_int_index = 0
 
     def __init__(
         self,
@@ -75,11 +76,15 @@ def __init__(
         unicode_escape: bool = True,
         uri_decode: bool = False,
     ) -> None:
-        self.parts = parts or self._parse(
-            pointer,
-            unicode_escape=unicode_escape,
-            uri_decode=uri_decode,
-        )
+        if parts:
+            self.parts = tuple(str(part) for part in parts)
+        else:
+            self.parts = self._parse(
+                pointer,
+                unicode_escape=unicode_escape,
+                uri_decode=uri_decode,
+            )
+
         self._s = self._encode(self.parts)
 
     def __str__(self) -> str:
@@ -91,7 +96,7 @@ def _parse(
         *,
         unicode_escape: bool,
         uri_decode: bool,
-    ) -> Tuple[Union[int, str], ...]:
+    ) -> Tuple[str, ...]:
         if uri_decode:
             s = unquote(s)
         if unicode_escape:
@@ -103,43 +108,49 @@ def _parse(
                 "pointer must start with a slash or be the empty string"
             )
 
-        return tuple(
-            self._index(p.replace("~1", "/").replace("~0", "~")) for p in s.split("/")
-        )[1:]
+        return tuple(p.replace("~1", "/").replace("~0", "~") for p in s.split("/"))[1:]
+
+    def _index(self, key: str) -> Optional[int]:
+        """Return an array index for `key`.
 
-    def _index(self, s: str) -> Union[str, int]:
-        # Reject non-zero ints that start with a zero and negative integers.
-        if len(s) > 1 and s.startswith(("0", "-")):
-            return s
+        Return `None` if key can't be converted to an index.
+        """
+        # Reject indexes that start with a zero.
+        if len(key) > 1 and key.startswith("0"):
+            return None
 
         try:
-            index = int(s)
-            if index < self.min_int_index or index > self.max_int_index:
-                raise JSONPointerError("index out of range")
-            return index
+            index = int(key)
         except ValueError:
-            return s
+            return None
 
-    def _getitem(self, obj: Any, key: Any) -> Any:
+        if index < self.min_int_index or index > self.max_int_index:
+            raise JSONPointerIndexError(
+                f"array indices must be between {self.min_int_index}"
+                f" and {self.max_int_index}"
+            )
+
+        return index
+
+    def _getitem(self, obj: Any, key: str) -> Any:
         try:
             # Handle the most common cases. A mapping with a string key, or a sequence
             # with an integer index.
-            #
-            # Note that `obj` does not have to be a Mapping or Sequence here. Any object
-            # implementing `__getitem__` will do.
+            if isinstance(obj, Sequence) and not isinstance(obj, str):
+                index = self._index(key)
+                if isinstance(index, int):
+                    return getitem(obj, index)
             return getitem(obj, key)
         except KeyError as err:
             return self._handle_key_error(obj, key, err)
         except TypeError as err:
             return self._handle_type_error(obj, key, err)
         except IndexError as err:
-            raise JSONPointerIndexError(f"index out of range: {key}") from err
-
-    def _handle_key_error(self, obj: Any, key: Any, err: Exception) -> object:
-        if isinstance(key, int):
-            # Try a string repr of the index-like item as a mapping key.
-            return self._getitem(obj, str(key))
+            if not isinstance(err, JSONPointerIndexError):
+                raise JSONPointerIndexError(f"index out of range: {key}") from err
+            raise
 
+    def _handle_key_error(self, obj: Any, key: str, err: Exception) -> object:
         # Handle non-standard key/property selector/pointer.
         #
         # For the benefit of `RelativeJSONPointer.to()` and `JSONPathMatch.pointer()`,
@@ -149,26 +160,18 @@ def _handle_key_error(self, obj: Any, key: Any, err: Exception) -> object:
         # Note that if a key with a leading `#`/`~` exists in `obj`, it will have been
         # handled by `_getitem`.
         if (
-            isinstance(key, str)
-            and isinstance(obj, Mapping)
+            isinstance(obj, Mapping)
             and key.startswith((self.keys_selector, "#"))
             and key[1:] in obj
         ):
             return key[1:]
 
         raise JSONPointerKeyError(key) from err
 
-    def _handle_type_error(self, obj: Any, key: Any, err: Exception) -> object:
-        if (
-            isinstance(obj, str)
-            or not isinstance(obj, Sequence)
-            or not isinstance(key, str)
-        ):
+    def _handle_type_error(self, obj: Any, key: str, err: Exception) -> object:
+        if not isinstance(obj, Sequence) or not isinstance(key, str):
             raise JSONPointerTypeError(f"{key}: {err}") from err
 
-        # `obj` is array-like
-        # `key` is a string
-
         if key == "-":
             # "-" is a valid index when appending to a JSON array with JSON Patch, but
             # not when resolving a JSON Pointer.
@@ -185,16 +188,6 @@ def _handle_type_error(self, obj: Any, key: Any, err: Exception) -> object:
                 raise JSONPointerIndexError(f"index out of range: {_index}") from err
             return _index
 
-        # Try int index. Reject non-zero ints that start with a zero.
-        index = self._index(key)
-        if isinstance(index, int):
-            return self._getitem(obj, index)
-
-        if re.match(r"-[0-9]+", index):
-            raise JSONPointerIndexError(
-                f"{key}: array indices must be positive integers or zero"
-            ) from err
-
         raise JSONPointerTypeError(f"{key}: {err}") from err
 
     def resolve(
@@ -354,13 +347,13 @@ def is_relative_to(self, other: JSONPointer) -> bool:
         )
 
     def __eq__(self, other: object) -> bool:
-        return isinstance(other, JSONPointer) and self.parts == other.parts
+        return isinstance(other, self.__class__) and self.parts == other.parts
 
     def __hash__(self) -> int:
-        return hash(self.parts)  # pragma: no cover
+        return hash((self.__class__, self.parts))  # pragma: no cover
 
     def __repr__(self) -> str:
-        return f"JSONPointer({self._s!r})"  # pragma: no cover
+        return f"{self.__class__.__name__}({self._s!r})"  # pragma: no cover
 
     def exists(
         self, data: Union[str, IOBase, Sequence[object], Mapping[str, object]]
@@ -396,7 +389,7 @@ def parent(self) -> JSONPointer:
         if not self.parts:
             return self
         parent_parts = self.parts[:-1]
-        return JSONPointer(
+        return self.__class__(
             self._encode(parent_parts),
             parts=parent_parts,
             unicode_escape=False,
@@ -420,14 +413,13 @@ def __truediv__(self, other: object) -> JSONPointer:
 
         other = self._unicode_escape(other.lstrip())
         if other.startswith("/"):
-            return JSONPointer(other, unicode_escape=False, uri_decode=False)
+            return self.__class__(other, unicode_escape=False, uri_decode=False)
 
         parts = self.parts + tuple(
-            self._index(p.replace("~1", "/").replace("~0", "~"))
-            for p in other.split("/")
+            p.replace("~1", "/").replace("~0", "~") for p in other.split("/")
         )
 
-        return JSONPointer(
+        return self.__class__(
             self._encode(parts), parts=parts, unicode_escape=False, uri_decode=False
         )
 
@@ -617,7 +609,7 @@ def to(
                 raise RelativeJSONPointerIndexError(
                     f"index offset out of range {new_index}"
                 )
-            parts[-1] = int(parts[-1]) + self.index
+            parts[-1] = str(int(parts[-1]) + self.index)
 
         # Pointer or index/property
         if isinstance(self.pointer, JSONPointer):

tests/test_cli.py

@@ -377,7 +377,7 @@ def test_pointer_command_resolution_error(
 
     captured = capsys.readouterr()
     assert err.value.code == 1
-    assert captured.err.startswith("pointer key error 'foo'")
+    assert captured.err.startswith("pointer key error: 'foo'")
 
 
 def test_pointer_command_resolution_error_debug(

tests/test_json_patch_rfc6902.py

@@ -30,6 +30,7 @@
 ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 POSSIBILITY OF SUCH DAMAGE.
 """
+
 import copy
 import dataclasses
 import re
@@ -181,7 +182,7 @@ def test_test_op_failure() -> None:
 def test_add_to_nonexistent_target() -> None:
     patch = JSONPatch().add(path="/baz/bat", value="qux")
     with pytest.raises(
-        JSONPatchError, match=re.escape("pointer key error 'baz' (add:0)")
+        JSONPatchError, match=re.escape("pointer key error: 'baz' (add:0)")
     ):
         patch.apply({"foo": "bar"})
 

tests/test_json_pointer.py

@@ -40,15 +40,11 @@ def test_resolve_with_default() -> None:
     assert pointer.resolve(data, default=None) is None
 
 
-def test_pointer_index_out_of_range() -> None:
-    max_plus_one = JSONPointer.max_int_index + 1
-    # min_minus_one = JSONPointer.min_int_index - 1
-
-    with pytest.raises(jsonpath.JSONPointerError):
-        JSONPointer(f"/some/thing/{max_plus_one}")
-
-    # with pytest.raises(jsonpath.JSONPointerError):
-    #     JSONPointer(f"/some/thing/{min_minus_one}")
+def test_pointer_min_int_index() -> None:
+    data = {"some": {"thing": [1, 2, 3]}}
+    pointer = JSONPointer(f"/some/thing/{JSONPointer.min_int_index - 1}")
+    with pytest.raises(jsonpath.JSONPointerIndexError):
+        pointer.resolve(data)
 
 
 def test_resolve_int_key() -> None:
@@ -320,3 +316,16 @@ def test_trailing_slash() -> None:
     data = {"foo": {"": [1, 2, 3], " ": [4, 5, 6]}}
     assert JSONPointer("/foo/").resolve(data) == [1, 2, 3]
     assert JSONPointer("/foo/ ").resolve(data) == [4, 5, 6]
+
+
+def test_index_token_on_string_value() -> None:
+    data = {"foo": "bar"}
+    pointer = JSONPointer("/foo/1")
+    with pytest.raises(JSONPointerTypeError):
+        pointer.resolve(data)
+
+
+def test_index_like_token_on_object_value() -> None:
+    data = {"foo": {"-1": "bar"}}
+    pointer = JSONPointer("/foo/-1")
+    assert pointer.resolve(data) == "bar"