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

[Avro] Add support for @Stringable annotation #53

Closed

Conversation

baharclerode
Copy link
Contributor

This adds support for the @Stringable annotation, and adds support for stringable classes (Those with @Stringable, and URL/URI/File/BigInteger/BigDecimal). This fixes BigInteger/BigDecimal compatibility with the apache implementation.

Of particular note, BigDecimal is no longer a Type.DOUBLE in schema generation, nor is BigInteger a Type.LONG in schema generation (since they are "stringable" to avro), but they should continue to serialize/deserialize properly from int/long/float/double scalars in existing avro schemas.

I treated @Stringable on a class as having a creator annotation on the constructor which takes a single string argument. I'm not sure if there is a better way to do this.

@baharclerode baharclerode force-pushed the bah.StringableAnnotation branch from 4afec93 to f3cb333 Compare March 2, 2017 06:34

@Override
public Object findSerializer(Annotated a) {
if (a instanceof AnnotatedClass && a.hasAnnotation(Stringable.class) || a.getRawType() == File.class) {
Copy link
Member

Choose a reason for hiding this comment

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

First part makes sense I guess, but do we really need File? It's serialized using value.getAbsolutePath() already.
Alternatively if this really needs to be changed, I think we should just register serializer via AvroModule, and not through annotation introspector.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Avro uses toString() / getPath() instead of getAbsolutePath(), so this was a quick fix to make the behavior match up. I'll replace it with a proper serializer in the module.

@cowtowncoder
Copy link
Member

Hate to to this but... would it be possible to re-base it to master, and only add in 2.9?
There are some aspects that have changed in code regarding annotation introspection (regarding creator detection), and I think I'd like to make some tweaks. I will try to get this in for 2.9.0.pr2.

@baharclerode
Copy link
Contributor Author

I can rebase this into master. After this, I have two more planned PRs, one for @Union / type ID support, and one for @AvroEncode support.

@baharclerode
Copy link
Contributor Author

Closed and reopened against master as #57

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