Details

    • Type: New Feature New Feature
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None

      Description

      Consider adding support for JSON Pointer. The RFC is at http://tools.ietf.org/html/rfc6901

        Activity

        Hide
        kchung added a comment - - edited

        Line 112 - 123: can be replace with

        return jsonPointer.equals(other.jsonPointer); 

        since jsonPointer cannot be null (see javadocs for the constructor)

        hascode(): why not just return jsonPointer.hashCode();

        After reading the spec again, it does seems to say that the target must be a JsonStructure. If so, we should remove the method

        public JsonValue add(JsonValue target, JsonValue value)

        and replace

        public JsonValue getValue(JsonValue target)

        with

        public JsonValue getValue(JsonStructure target)

        According to the spec, we should throw JsonException when value is non-existing (including array index out of bounds). We should push the checking to NodeReference

        Show
        kchung added a comment - - edited Line 112 - 123: can be replace with return jsonPointer.equals(other.jsonPointer); since jsonPointer cannot be null (see javadocs for the constructor) hascode(): why not just return jsonPointer.hashCode(); After reading the spec again, it does seems to say that the target must be a JsonStructure. If so, we should remove the method public JsonValue add(JsonValue target, JsonValue value) and replace public JsonValue getValue(JsonValue target) with public JsonValue getValue(JsonStructure target) According to the spec, we should throw JsonException when value is non-existing (including array index out of bounds). We should push the checking to NodeReference
        Hide
        asotobu added a comment -

        Yes I read it before the PR again and I arrived to same conclusion, for this reason I didn't implement JsonValue but casting to JsonStructure or throwing an exception. We can remove methods without much problem.

        Show
        asotobu added a comment - Yes I read it before the PR again and I arrived to same conclusion, for this reason I didn't implement JsonValue but casting to JsonStructure or throwing an exception. We can remove methods without much problem.
        Hide
        kchung added a comment -

        @Alex I committed and pushed the changes we discussed. Please review and test. You can also check in the tests, if they pass.

        Show
        kchung added a comment - @Alex I committed and pushed the changes we discussed. Please review and test. You can also check in the tests, if they pass.
        Hide
        asotobu added a comment -

        Good this weekend I will incorporate the tests I have already written and start working on JsonPatch as we discussed in the issue.

        Show
        asotobu added a comment - Good this weekend I will incorporate the tests I have already written and start working on JsonPatch as we discussed in the issue.
        Hide
        asotobu added a comment - - edited

        @Kin-man while writing tests for JsonPatch I have found some issues regarding to JsonPointer/NodeReference. For example being target an array and the json pointer something like /~1 which is translated to "/" and not to the number 1 it throws a NumberFormatException. What I have done is catching the NumberFormatException and throw a JsonException wrapping the NumberFormatException. I think this is the correct way to procedure, WDYT?

        And it happens the same when you try to put a value on an index array out of bounds.

        Show
        asotobu added a comment - - edited @Kin-man while writing tests for JsonPatch I have found some issues regarding to JsonPointer/NodeReference. For example being target an array and the json pointer something like /~1 which is translated to "/" and not to the number 1 it throws a NumberFormatException. What I have done is catching the NumberFormatException and throw a JsonException wrapping the NumberFormatException. I think this is the correct way to procedure, WDYT? And it happens the same when you try to put a value on an index array out of bounds.

          People

          • Assignee:
            Unassigned
            Reporter:
            jitu
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated: