Skip to content

Conversation

pjfanning
Copy link
Member

@pjfanning pjfanning commented Jun 13, 2022

Relates to #749

@pjfanning pjfanning marked this pull request as draft June 13, 2022 09:50
@pjfanning
Copy link
Member Author

@cowtowncoder this is WIP and miles away from being complete - master and v2.14 are so different that most of the code in this PR has been neutered to get the code to compile. I'll need help to work out how features are enabled in master.

For instance, how do you create a JsonParser with a JsonReadFeature enabled?

TokenStreamFactory has public abstract JsonParser createParser(ObjectReadContext readCtxt, InputStream in) but ObjectStreamFactory only seems to have an empty implementation (ie ObjectReadContext.empty()). I can't find another class that implements ObjectReadContext.

@cowtowncoder
Copy link
Member

@pjfanning ObjectReadContext is typically passed by databind where context (DeserializationContext for parser, and similarly SerializerProvider for output, ObjectWriteContext) is implemented. I have been thinking of whether to implement one for testing purposes in jackson-core.

However: right now you can construct a JsonFactory with suitable defaults

JsonParser p = JsonParser.builder()
   .enable(JsonReadFeature.FEATURE)
   .build();

which is probably what tests should use.

Now: I'd be perfectly happy to convert these from 2.14 to master as I know how to modify changes -- all I'd need is the initial scaffolding. This is why I asked for a subset, to get first tricky (but small) part merged. After this, rest of the code is fine.
I actually prefer single PR for 2.14 over having separate full PRs for 2.14 and master since latter can just defer conflicts. Although if you want to go over conflict resolution that's fine too. It's just that I often end up doing those anyway so that's fine :)

@pjfanning
Copy link
Member Author

@cowtowncoder thanks - I can use this just to keep track of the merge issues and the changes that have been made to get master working - so it doesn't have to be merged

@cowtowncoder
Copy link
Member

I suspect this can also be closed now that things are merged to master?

@pjfanning pjfanning closed this Jun 21, 2022
@pjfanning pjfanning deleted the double-parser-master branch June 21, 2022 00:12
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

Successfully merging this pull request may close these issues.

2 participants