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 @AvroNamespace annotation to override Avro type namespace #324

Conversation

MichalFoksa
Copy link
Contributor

@MichalFoksa MichalFoksa commented May 7, 2022

New AvroNamespace annotation to override default Avro type namespace.

Simple way how to override default Avro namespace value with a new, custom, annotation.

This solves #310.

Is the new annotation package OK?

com.fasterxml.jackson.dataformat.avro.AvroFixedSize              -  the only existing annotation
com.fasterxml.jackson.dataformat.avro.annotations.AvroNamespace  - new annotation

@MichalFoksa MichalFoksa changed the title ]@AvroNamespace annotation to override Avro schema field name… [Avro] @AvroNamespace annotation to override Avro type namespace May 7, 2022
@MichalFoksa MichalFoksa force-pushed the feature/2.13/avro/#310-override-namespace branch from 8761752 to 4567e4a Compare May 7, 2022 21:11
* Annotation allows to override default Avro type namespace value.
* Default value is Java package name.
*/
@Target(ElementType.TYPE)
Copy link
Member

Choose a reason for hiding this comment

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

To allow "annotation bundle" usage may also want to add ElementType.ANNOTATION_TYPE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, good idea.
ElementType.ANNOTATION_TYPE added to allow "annotation bundle" usage.

@cowtowncoder
Copy link
Member

cowtowncoder commented May 8, 2022

but one thing is that this cannot go in 2.13 patch since it is both slightly backwards-incompatible (changes method signature of the helper method) and (more importantly) adds a new annotation type.
So please re-base against 2.14.

EDIT: Removed a former comment wrt Ion -- for some reason, got things mixed up.

@MichalFoksa MichalFoksa force-pushed the feature/2.13/avro/#310-override-namespace branch from 4567e4a to 657e214 Compare May 8, 2022 05:45
@MichalFoksa MichalFoksa changed the base branch from 2.13 to 2.14 May 8, 2022 05:46
@MichalFoksa MichalFoksa force-pushed the feature/2.13/avro/#310-override-namespace branch from 657e214 to c626067 Compare May 8, 2022 05:48
@MichalFoksa
Copy link
Contributor Author

PR is now rebased on 2.14.

@cowtowncoder
Copy link
Member

Thank you @MichalFoksa!

Also, apologies for nonsensical comment wrt Ion & Amazon developers. This is for Avro, but somehow I managed to get things fixed.

I'll try to get this reviewed soon,

…ield namespace. Current namespace value is Java package name. This annotation allows to override its name.
@MichalFoksa
Copy link
Contributor Author

@cowtowncoder

No problem, really - I understand :).

I have updated my commit - just reorganized imports in AvroSchemaHelper. Moved imports from com.fasterxml.jackson together.

@MichalFoksa MichalFoksa force-pushed the feature/2.13/avro/#310-override-namespace branch from c626067 to 4a9f7d0 Compare May 9, 2022 06:38
@cowtowncoder cowtowncoder marked this pull request as ready for review May 15, 2022 23:12
@cowtowncoder cowtowncoder added this to the 2.14.0 milestone May 15, 2022
@cowtowncoder
Copy link
Member

Will merge: will also change package name from "...avro.annotations" to "...avro.annotation", just to be consistent with other Jackson packages (jackson-annotations, jackson-databind, jackson-dataformat-xml).

@cowtowncoder cowtowncoder merged commit 84f363c into FasterXML:2.14 May 15, 2022
@cowtowncoder cowtowncoder changed the title [Avro] @AvroNamespace annotation to override Avro type namespace Add @AvroNamespace annotation to override Avro type namespace May 15, 2022
@cowtowncoder
Copy link
Member

Thank you again @MichalFoksa for contributing this! It will go in 2.14.0 once we get there.

@MichalFoksa
Copy link
Contributor Author

@cowtowncoder
No problem - thank you for your time!

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

Successfully merging this pull request may close these issues.

2 participants