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

Creating new annotation to exclude beans fields and classes from the GSON Serialization #763

Merged
merged 6 commits into from
Sep 4, 2014

Conversation

renatorroliveira
Copy link
Collaborator

This is a feature that is extremely desirable.
Creating an annotation (@GsonOmitField) that marks a field or a class to be skipped at the JSON serialization reduces a lot the duplication of exclude() calls in some cases.
Imagine a User model where are some private information like password and phone number stored.
Today you have to add a .exclude() call for those fields every time you want to serialize a user object.
With this feature, you just annotate those fields with @GsonOmitField and the job is done, you don't need to worry about those informations ending up at the client side.

It is a very simple implementation and adds a wonderful feature.

@@ -68,6 +73,7 @@ private boolean isCompatiblePath(Entry<String, Class<?>> path, Class<?> definedI

@Override
public boolean shouldSkipClass(Class<?> clazz) {
return false;
GsonOmitField annotation = clazz.getAnnotation(GsonOmitField.class);
return (annotation != null);
Copy link
Member

Choose a reason for hiding this comment

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

return clazz.isAnnotationPresent(GsonOmitField)?

@Turini
Copy link
Member

Turini commented Aug 27, 2014

this is a really nice PR, thanks doing it @renatorroliveira! Two points:

(1) Can you add a simple example of usage at VRaptor's site?
(2) And what about deserialisation? (it doesn't, but should affect both?)

@garcia-jj
Copy link
Member

Where are the tests?

GsonOmitField annotation = f.getAnnotation(GsonOmitField.class);
if (annotation != null)
return true;

Copy link
Member

Choose a reason for hiding this comment

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

The same here for isAnnotationPresent.

Copy link
Member

Choose a reason for hiding this comment

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

GSON FieldAttributes don't have this method

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here it has, with GSON 2.2.4.
I even tested my app with this implementation and works fine.
The unit tests passed like a charm =)

Copy link
Member

Choose a reason for hiding this comment

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

I mean the isAnnotationPresent, this is not present on FieldAttributes. right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, it isn't. The Class<?> type has it, but FieldAttributes hasn't.

@garcia-jj
Copy link
Member

I like this feature, thank you for coding. After changing the code with suggestions, are ready to merge (by my mind).

@renatorroliveira
Copy link
Collaborator Author

Yes, it should be isAnnotationPresent, my mistake =)

@Turini I can add the example of usage. I should do this in this same PR?
@garcia-jj I will add the unit tests too.

@garcia-jj
Copy link
Member

Same PR.

@Turini
Copy link
Member

Turini commented Aug 27, 2014

sorry my delay, please add docs at the same PR. thanks

@renatorroliveira
Copy link
Collaborator Author

I looking at the site.
I noticed that there isn't a page specific for serialization content.
Should i create a new page? If not, in which page you suggest to add this content?

@lucascs
Copy link
Member

lucascs commented Aug 27, 2014

Cool PR. I just don't like the name @GsonOmitField.

You are tying the feature to the implementation. If we choose to replace GSon to another framework in the future, the annotation will be awkward.

Rename to something like @OmitField or @NotSerializable and move it to b.c.c.v.serialization package.

@garcia-jj
Copy link
Member

Great Lucas. I like your suggestion.

@renatorroliveira
Copy link
Collaborator Author

You is right @lucascs i will modify these points.

@lucascs
Copy link
Member

lucascs commented Aug 27, 2014

Have you tried this?
http://www.javacreed.com/gson-annotations-example/

@renatorroliveira
Copy link
Collaborator Author

Yes @lucascs, i've looked at the Expose annotation, but it has 2 problems:

  • To use the Expose annotation, you need to change the behavior of Gson object, calling GsonBuilder.excludeFieldsWithoutExposeAnnotation(). If you call this method it will serialize and deserialize only the fields annotated.
  • In most cases, fields to be ommited are fewer than the fields to be exposed. Another advantage is that the OmitField annotation doesn't require to change the default behavior of Gson, increasing the flexibility in some cases.

For me these 2 points are enough to justify the creation of a OmitField annotation =)

@renatorroliveira
Copy link
Collaborator Author

Another point that i forget, the capability to omit a class is something you don't acomplish with Expose, since all fields are ommited, except those that were annotated. I see the OmitField as a much more clear am easier approach.

@lucascs
Copy link
Member

lucascs commented Aug 27, 2014

I'm not arguing against the OmitField... just to see if gson annotations are already supported by default.

@renatorroliveira
Copy link
Collaborator Author

Yes i udnerstood that, i just want to clarify my motivation to create the annotation =)

@garcia-jj
Copy link
Member

I think that if GSON annotations are supported, may we can do this approaching, changing GSON components to always uses annotations. Why? Because if Google team create another annotations in the future will supported without any modifications in vraptor implementation.

Makes any sense?

@renatorroliveira
Copy link
Collaborator Author

Makes sense, but i see two concerns here.
To enable the native Expose annotation of GSON, it must be done by a configuration or something similar, because the side effect of enabling the Expose annotation is a breaking change for most projects.
The second concern is that with native annotations of GSON we can't achieve the "desirable" behavior that we are aiming with this new annotation. We just register a simple logic to the ExclusionStrategy. In theory this should be a very compatible implementation with newer versions of GSON.

Another point is that the version 2.2.4 was released in May of 2013, and next release was two weeks ago, the version 2.3, more than one year without updates. Maybe is not necessary to worry about GSON updates. =D

@renatorroliveira
Copy link
Collaborator Author

I believe that we should create a way to enable the Expose annotation without the need to overwrite the GsonBuilderWrapper.

@garcia-jj
Copy link
Member

As I see with this option (enabling expose annotation), we need to annotate all fields with Expose to serialize/deserialize. So can break existent applications. And its to boring to annotate all fields :).

@renatorroliveira
Copy link
Collaborator Author

Yeah, very boring =)
If it is added, it should be an optional behavior

@garcia-jj
Copy link
Member

@lucascs @Turini Can I merge? I think that the core are good to merge.

public class UserPrivateInfo {
String document;
String phone;
String address;
Copy link
Member

Choose a reason for hiding this comment

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

can you add private on fields and a code comment
about omitted getters and setters? Same for the User.


When serialiazing to JSON you can use the annotation 'SkipSerialization' to omit the serialization of a field or class.

~~~
Copy link
Member

Choose a reason for hiding this comment

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

we can maybe add an text introducing the situation here, for example:
"The class below has all of your fields omitted for serialization."

@renatorroliveira
Copy link
Collaborator Author

I agree, and i will improve the docs.

@Turini
Copy link
Member

Turini commented Sep 4, 2014

thanks @renatorroliveira!

Turini added a commit that referenced this pull request Sep 4, 2014
Creating new annotation to exclude beans fields and classes from the GSON Serialization
@Turini Turini merged commit 7d858fc into caelum:master Sep 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants