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
        asotobu added a comment -

        jsonPointer could be an interface as well, so implementors Can decide how to implement json pointer resolution and how to create. WDYT ?

        Show
        asotobu added a comment - jsonPointer could be an interface as well, so implementors Can decide how to implement json pointer resolution and how to create. WDYT ?
        Hide
        kchung added a comment - - edited

        OK. Let's keep both for now. I'll summarize what we have.

        1. JsonPointer is a non-abstract class with a constructor
          public JsonPointer(String pointer);
        2. The syntax for applying a JsonPointer to a JsonValue is
          JsonValue value;
          JsonPointer pointer = new JsonPointer("/a/b/c");
          value.get(pointer);
          
        3. The JsonValue operations should be overloaded to take the raw Json pointer String as a short cut
          value.get("/a/b/c");
          
        4. Add 4 operaions to JsonValue:
          1. getValue # there is already a get in JsonObject
          2. remove
          3. replace
          4. add
        5. An error is raised if the pointer reference a non-existing value.

        Do we still need the alternate syntax

        pointer.get(value)

        ?

        I'll writeup the javadocs for JsonPointer and we can talk more.

        Show
        kchung added a comment - - edited OK. Let's keep both for now. I'll summarize what we have. JsonPointer is a non-abstract class with a constructor public JsonPointer( String pointer); The syntax for applying a JsonPointer to a JsonValue is JsonValue value; JsonPointer pointer = new JsonPointer( "/a/b/c" ); value.get(pointer); The JsonValue operations should be overloaded to take the raw Json pointer String as a short cut value.get( "/a/b/c" ); Add 4 operaions to JsonValue: getValue # there is already a get in JsonObject remove replace add An error is raised if the pointer reference a non-existing value. Do we still need the alternate syntax pointer.get(value) ? I'll writeup the javadocs for JsonPointer and we can talk more.
        Hide
        asotobu added a comment -

        Well let me explain with code. If you look here https://github.com/FasterXML/jackson-core/blob/master/src/main/java/com/fasterxml/jackson/core/JsonPointer.java#L114 you will see the process that requires creating a JsonPointer instance in Jackson. But I have used other Java libraries that implements JsonPointer and it is pretty similar. So as you can see there is some logic like escaping characters and creating segments for each sub-expression.

        This is not good or bad it is simply an implementation details I know but let's suppose next code:

        JsonObject[] persons = new JsonObject[5000];
        //fill persons
        JsonObject[] addresses = new JsonObject[5000]; 
        
        for(int i=0;i<persons.length;i++) {
          addresses[i] = persons[i].get("/personalinformation/address");
        }
        

        In that code the escaping characters and segmentation of json pointer expression will be evaluated 5000 times.

        But if I could do something like:

        JsonObject[] persons = new JsonObject[5000];
        //fill persons
        JsonObject[] addresses = new JsonObject[5000]; 
        
        JsonPointer addressPointer = new JsonPointer("/personalinformation/address");
        for(int i=0;i<persons.length;i++) {
          addresses[i] = persons[i].get(addressPointer);
        }
        

        Then construction would be only one.

        Because we don't know exactly if reusing a jsonpointer would be a requirement or not, I think that would have sense to implement both operations:

        • get("/a/b")
        • get(new JsonPointer("/a/b");

        Basically because we are getting the best of both worlds with minimal requirement. The only requirement is that JsonPointer becomes an interface of the spec.

        Show
        asotobu added a comment - Well let me explain with code. If you look here https://github.com/FasterXML/jackson-core/blob/master/src/main/java/com/fasterxml/jackson/core/JsonPointer.java#L114 you will see the process that requires creating a JsonPointer instance in Jackson. But I have used other Java libraries that implements JsonPointer and it is pretty similar. So as you can see there is some logic like escaping characters and creating segments for each sub-expression. This is not good or bad it is simply an implementation details I know but let's suppose next code: JsonObject[] persons = new JsonObject[5000]; //fill persons JsonObject[] addresses = new JsonObject[5000]; for ( int i=0;i<persons.length;i++) { addresses[i] = persons[i].get( "/personalinformation/address" ); } In that code the escaping characters and segmentation of json pointer expression will be evaluated 5000 times. But if I could do something like: JsonObject[] persons = new JsonObject[5000]; //fill persons JsonObject[] addresses = new JsonObject[5000]; JsonPointer addressPointer = new JsonPointer( "/personalinformation/address" ); for ( int i=0;i<persons.length;i++) { addresses[i] = persons[i].get(addressPointer); } Then construction would be only one. Because we don't know exactly if reusing a jsonpointer would be a requirement or not, I think that would have sense to implement both operations: get("/a/b") get(new JsonPointer("/a/b"); Basically because we are getting the best of both worlds with minimal requirement. The only requirement is that JsonPointer becomes an interface of the spec.
        Hide
        kchung added a comment -

        @asotobu I don't get your argument about extra processing are need for a Json pointer. Isn't that true anyway? This is just an implementation detail, right?

        I still don't know how often a Json pointer is reused. In your example, if the Json pointer is part of the request (maybe part of a Json patch). You'll need to do a comparison to determine if two pointers are same. To me it almost looks like you need a cache of JsonPointer. Note also the same optimization can be realized for the short-cut, because it can also look up the cache to reuse a previous JsonPointer.

        I am not ruling out JsonPointer. I am just exploring what a JSON Pointer API we need to have. If we can make it simpler, the better.

        We don't need to make decisions right now. We should have better ideas when we discuss/implement JSON patch.

        Show
        kchung added a comment - @asotobu I don't get your argument about extra processing are need for a Json pointer. Isn't that true anyway? This is just an implementation detail, right? I still don't know how often a Json pointer is reused. In your example, if the Json pointer is part of the request (maybe part of a Json patch). You'll need to do a comparison to determine if two pointers are same. To me it almost looks like you need a cache of JsonPointer. Note also the same optimization can be realized for the short-cut, because it can also look up the cache to reuse a previous JsonPointer. I am not ruling out JsonPointer. I am just exploring what a JSON Pointer API we need to have. If we can make it simpler, the better. We don't need to make decisions right now. We should have better ideas when we discuss/implement JSON patch.
        Hide
        asotobu added a comment -

        @kchung and @mitemitreski From my point of view I would maintain the JsonPointer class in spec. Let me show why. A JsonPointer expression requires two things when it is created/used; the first one is escaping special characters and the second one (and this is an implementation details but all implementations I have worked of JsonPointer uses) it is that a json pointer expression is tokenized (or segmented) so in this way it is easier to implement the JsonPointer logic and also some operations required by Json Patch.

        So let's suppose next scenario. I have 2000 request per minute which every request uses the same JsonPointer. This means that every minute you will execute 2000 times the escaping the same chunk of string and tokenize the expression. Although these are not an expensive operations it is unnecessary to execute them each time if the business logic is exactly the same.

        So if spec allows to pass a JsonPointer then you will be able to create an static JsonPointer expression and call person.get(staticJsonPointer);

        Of course we can still offer the string method which internally creates a new JsonPointer object, but at least we give to users the option to create only once the expression.

        Show
        asotobu added a comment - @kchung and @mitemitreski From my point of view I would maintain the JsonPointer class in spec. Let me show why. A JsonPointer expression requires two things when it is created/used; the first one is escaping special characters and the second one (and this is an implementation details but all implementations I have worked of JsonPointer uses) it is that a json pointer expression is tokenized (or segmented) so in this way it is easier to implement the JsonPointer logic and also some operations required by Json Patch. So let's suppose next scenario. I have 2000 request per minute which every request uses the same JsonPointer. This means that every minute you will execute 2000 times the escaping the same chunk of string and tokenize the expression. Although these are not an expensive operations it is unnecessary to execute them each time if the business logic is exactly the same. So if spec allows to pass a JsonPointer then you will be able to create an static JsonPointer expression and call person.get(staticJsonPointer); Of course we can still offer the string method which internally creates a new JsonPointer object, but at least we give to users the option to create only once the expression.

          People

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

            Dates

            • Created:
              Updated: