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 JsonGeneratorDecorator to allow decorating JsonGenerators #1051

Merged
merged 1 commit into from Jun 20, 2023

Conversation

digulla
Copy link
Contributor

@digulla digulla commented Jun 14, 2023

Support to register decorators for JsonGenerator in the builder and factory.

@cowtowncoder
Copy link
Member

Cool, I like this. Let me think a bit about it, just to be sure, but I think this could work about as-is.

@digulla
Copy link
Contributor Author

digulla commented Jun 15, 2023

One thing that comes to my mind: Is there anything else in the factory which could use this?

Also, is the _decorate() method in the right class? It could go into TSFBuilder instead.

@cowtowncoder
Copy link
Member

Forgot to ask: any chance for a simple test or two, to verify decoration actually works...

Copy link
Member

@cowtowncoder cowtowncoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall; added notes but will address them post-merge.

@cowtowncoder cowtowncoder merged commit edc944e into FasterXML:2.16 Jun 20, 2023
5 checks passed
@cowtowncoder cowtowncoder changed the title Support for JsonGeneratorDecorator Add JsonGeneratorDecorator to allow decorating JsonGenerators Jun 20, 2023
@cowtowncoder
Copy link
Member

One thing that comes to my mind: Is there anything else in the factory which could use this?

Also, is the _decorate() method in the right class? It could go into TSFBuilder instead.

I think it's in the right class -- it could not really go in factory builder (as that is only part of constructing factory itself, not generator).

One bigger thing we should tackle is to change all format backends to allow decoration. In 3.0 (master) this is relatively easy due to full refactoring of TokenStreamFactory hierarchy. But in 2.x there's much more duplication so pretty much every XxxFactory needs to call _decorate().

I did at least add tests for JsonFactory, fwtw. Changing other backends is not difficult, just lots of work.

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.

None yet

2 participants