Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add location data to JsonNode so that DOM tree elements could be mapped back to their location in a source document #20

Open
radai-rosenblatt opened this issue Sep 12, 2017 · 10 comments

Comments

@radai-rosenblatt
Copy link

this is useful for emitting helpful error msgs:
System.err.println("there's something i dont like about " + someJsonNode.position()

this is available when JsonNodeDeserializer is reading the tree

@cowtowncoder
Copy link
Member

I can see how this could be useful. Implementation could be rather tricky however.

@radai-rosenblatt
Copy link
Author

the information is (sometimes) already available. for example, here's a demo parsing a json file:

@Test
  public void testWhatever() throws Exception {
    String avsc = TestUtil.load("schemas/Record.avsc");
    JsonFactory jsonFactory = new JsonFactory();
    JsonParser parser = jsonFactory.createParser(avsc);
    while (parser.nextToken() != null) {
      JsonToken token = parser.currentToken();
      System.err.println(token + " @ " + prettyPrint(parser.getTokenLocation()) + "-" + prettyPrint(parser.getCurrentLocation()));
    }
  }
  private static String prettyPrint(JsonLocation location) {
    return location.getLineNr() + ":" + location.getColumnNr();
  }

which produces

START_OBJECT @ 1:1-1:2
FIELD_NAME @ 2:3-2:12
VALUE_STRING @ 2:11-2:12
FIELD_NAME @ 3:3-3:17
VALUE_STRING @ 3:16-3:17
FIELD_NAME @ 4:3-4:12
VALUE_STRING @ 4:11-4:12
FIELD_NAME @ 5:3-5:15
START_ARRAY @ 5:14-5:15
VALUE_STRING @ 5:15-5:16
END_ARRAY @ 5:20-5:21
FIELD_NAME @ 6:3-6:15
START_ARRAY @ 6:14-6:15
START_OBJECT @ 7:5-7:6
FIELD_NAME @ 7:6-7:15
VALUE_STRING @ 7:14-7:15
FIELD_NAME @ 7:27-7:36
VALUE_STRING @ 7:35-7:36
END_OBJECT @ 7:41-7:42
START_OBJECT @ 8:5-8:6
FIELD_NAME @ 8:6-8:15
VALUE_STRING @ 8:14-8:15
FIELD_NAME @ 8:28-8:37
START_ARRAY @ 8:36-8:37
VALUE_STRING @ 8:37-8:38
VALUE_STRING @ 8:45-8:46
END_ARRAY @ 8:53-8:54
END_OBJECT @ 8:54-8:55
START_OBJECT @ 9:5-9:6
FIELD_NAME @ 9:6-9:15
VALUE_STRING @ 9:14-9:15
FIELD_NAME @ 9:29-9:38
START_OBJECT @ 9:37-9:38
FIELD_NAME @ 10:7-10:16
VALUE_STRING @ 10:15-10:16
FIELD_NAME @ 11:7-11:16
VALUE_STRING @ 11:15-11:16
FIELD_NAME @ 12:7-12:18
START_ARRAY @ 12:17-12:18
START_OBJECT @ 13:9-13:10
FIELD_NAME @ 13:10-13:19
VALUE_STRING @ 13:18-13:19
FIELD_NAME @ 13:30-13:39
VALUE_STRING @ 13:38-13:39
END_OBJECT @ 13:46-13:47
END_ARRAY @ 14:7-14:8
END_OBJECT @ 15:5-15:6
END_OBJECT @ 15:6-15:7
END_ARRAY @ 16:3-16:4
END_OBJECT @ 17:1-17:2

seems to me JsonNodeDeserializer could pick the same values when building the tree and store then in JsonNodes.

granted, its not complete coverage of all cases from all sources, but its something, and probably useful.

@cowtowncoder
Copy link
Member

Yes I am well aware that location information is parsed by streaming parser. Challenges are due to cost of constructing and storing Location objects. I can see their usefulness for some use cases, although should be optional to avoid adding cpu and memory cost for other cases.

But bigger practical problem is that of JsonNodeFactory not taking location information.
This being the case, perhaps there should be a mutating method in JsonNode (attachLocation?), but right now they are immutable. Maybe withLocation() method instead?

I'll keep this option if anyone feels tackling it at some point. Thank you for suggesting it!

@radai-rosenblatt
Copy link
Author

radai-rosenblatt commented Sep 14, 2017

i would be fine with making this option (off by default) effectively copying JsonNodes by attaching a location to them.

@wangxi761
Copy link

so , can i get the location from json in current version?

@cowtowncoder
Copy link
Member

@wangxi761 You can get location from JsonParser, yes. But no location information is copied and attached to any of object models so if you are asking if JsonNode has it, no.

@wangxi761
Copy link

sometimes it is Needed,and Let people maintain their custom JsonNodeDeserializer is costly

have any future plan to make it?

@cowtowncoder
Copy link
Member

@wangxi761 I don't have time or personal interest to work on this. Maybe someone else can contribute something if they are interested and have time.

@wangxi761
Copy link

i have a plan to solve it , but if we want to achieve it elegantly
it may need to change parser . beacuse parser not processed comma, line break ,such we need to ignore it
and provide a visitor pattern for tree model .
What do you think

@cowtowncoder
Copy link
Member

I think that you may want to tackle problems separately: one would be to get and store Location along with nodes; the other to fix issues with actual Location tracking in parser (for whatever changes you think are needed there).

Depending on your use case creating your own object model, deserializer, might require bit of work but may also be easier than trying to change JsonNode handling.

I am not sure how visitor pattern would work here: one of the challenges is that JsonNode does not have any place to store JsonLocation: while deserializer has easy access, you will have to sub-class all subtypes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants