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

Change date/time format to ISO8601 on JSON serialization #548

Merged
merged 7 commits into from
Jul 31, 2013

Conversation

dipold
Copy link
Member

@dipold dipold commented Jul 8, 2013

This is my approach to issue #544 to provide support to ISO8601 standart on JSON serialization.

The ISO8601 is fully supported only by SimpleDateFormat in a JDK 7 and JodaTime.
To support 100% ISO8601 specs and maintaining compatibility to JDK 1.5 compliant, the solution was to use convertion by regexp rules.

This solution can impact in existing code, and needs to be analyzed.

@lucascs
Copy link
Member

lucascs commented Jul 8, 2013

I'd prefer to have a separate package to register these converters... something like ...serialization.iso and ...deserialization.iso...

Existing converters should accept the current format (not ISO), so we don't break compatibility.

@garcia-jj
Copy link
Member

Me too, @lucascs. Using the iso is important because many libraries use then.

But how @dipold reports, Java don't have pattern conversion in versions prior JDK7. May be we can create a package using joda time, or with JDK 7 warning users that this package only works in JDK 7+.

What you think, guys?

@dipold
Copy link
Member Author

dipold commented Jul 8, 2013

@lucascs I will change the pull request to register these converters in a separate package.

@garcia-jj I was able to create a class helper in this pull request to convert to and from ISO8601 string by regexp rules.. so... is no longer needed JDK 7 or JodaTime =)

@garcia-jj
Copy link
Member

Thank you, @dipold. I'll look sooner.

@dipold
Copy link
Member Author

dipold commented Jul 9, 2013

@lucascs I can´t see your point.
This converters (like CalendarDeserializer and CalendarSerializer) are @components.
How Vraptor can register/set one approach (not ISO - serialization.gson.adapters) of converters over another approach (ISO - serialization.iso8601)?

@lucascs
Copy link
Member

lucascs commented Jul 9, 2013

If you create @Components in a subpackage (.iso), we can choose to not register them by default (on BaseComponents), so if the user wants these converters he can register them by package.

@garcia-jj
Copy link
Member

@dipold and @lucascs, since these components are optional, I think that may be better to use joda-time to format/parse dates.

Add ISO8601 Util Test class
Add Calendar and Date Gson Deserializer
Change DefaultJsonSerializers to register optionally ISO8601 serializers
Add Calendar ISO8601 like serialization test
@dipold
Copy link
Member Author

dipold commented Jul 27, 2013

@lucascs I change PR and move gson serializers/deserializers to a subpackage.
To register this ISO8601 serializers, I change the DefaultJsonSerializers class to add these serializers when web.xml package configuration exists.
That is what you wanted?

@garcia-jj
Copy link
Member

Good commit, but I think that we can remove ISO8601Util in favor to joda-time.

String packagesParam = servletContext.getInitParameter(BasicConfiguration.BASE_PACKAGES_PARAMETER_NAME);
if ((packagesParam != null) && (packagesParam.contains("br.com.caelum.vraptor.serialization.gson.adapters.iso8601"))) {
this.serializers.add(new br.com.caelum.vraptor.serialization.gson.adapters.iso8601.CalendarSerializer());
this.serializers.add(new br.com.caelum.vraptor.serialization.gson.adapters.iso8601.DateSerializer());
Copy link
Member

Choose a reason for hiding this comment

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

This should be unnecessary. If these serializers are @Components, it should work without these 4 lines.

Copy link
Member Author

Choose a reason for hiding this comment

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

But doesn´t work without this code, and I don´t know what I can do.
If exists classes @Component adapters.CalendarSerializer and @Component adapters.iso8601.CalendarSerializer, the provider inject in List serializers two instances of @Component adapters.CalendarSerializer and no instance of @Component adapters.iso8601.CalendarSerializer regardless of the web.xml package configuration being configured.
Feel free to call me on the talk if necessary

Copy link
Member

Choose a reason for hiding this comment

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

I see your point =/

Maybe we could include and document a context-param which enables the iso8601 strategy, while using the adapters.CalendarSerializer...

@dipold
Copy link
Member Author

dipold commented Jul 29, 2013

@garcia-jj
Even using the JodaTime, the class ISO8601 remain necessary, because the string conversion has many variations to test:
yyyy-MM
yyyy-MM-dd
yyyy-MM-dd'T'HH: mm
yyyy-MM-dd'T'HH: mm: ss.SSSZ

In my opinion the code became more clean using regex rules, but I may change my opinion if you show me a code simpler, since I am not an expert in JodaTime

web.xml is configured (same as serializer strategy)
Adapt test to new strategy
Add test to deserialize an ISO8601 pattern string
@dipold
Copy link
Member Author

dipold commented Jul 30, 2013

@lucascs I changed ISO8601Util to a VRaptor @Component.
Unfortunately, to work as expected, the classes of the package ...adapters.iso8601 are not@Components =(

String packagesParam = servletContext.getInitParameter(BasicConfiguration.BASE_PACKAGES_PARAMETER_NAME);
if ((packagesParam != null) && (packagesParam.contains("br.com.caelum.vraptor.serialization.gson.adapters.iso8601"))) {
this.serializers.add(new br.com.caelum.vraptor.serialization.gson.adapters.iso8601.CalendarSerializer(new ISO8601Util()));
this.serializers.add(new br.com.caelum.vraptor.serialization.gson.adapters.iso8601.DateSerializer(new ISO8601Util()));
Copy link
Member

Choose a reason for hiding this comment

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

I'm sorry to insist, I really don't like this solution =(

Proposal: move this serializers to package br.com.caelum.vraptor.serialization.iso8601.gson, add @Component and see if everything works.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are not the only one! With your suggestion the behavior changed:

Without context-param:
The VRaptor instances only one of the CalendarSerializers or CalendarDeserializer from gson.adapters package. Fine!

With context-param (weird):
The VRaptor instances duplicate instances of gson.adapters.CalendarSerializer when invokes serialization (oh noes!), but duplicate instances of iso8601.gson.CalendarDeserializer when invokes consumes (ok, fine!).

This weird behavior is caused by guice provider? I think I am not able to fix this without help =(

Copy link
Member

Choose a reason for hiding this comment

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

are you sure?

Isn't it one gson.adapters.CalendarSerializer and one iso8601.gson.CalendarSerializer?

Copy link
Member Author

Choose a reason for hiding this comment

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

image

Copy link
Member

Choose a reason for hiding this comment

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

Components with the same name won't work =/ Guice provider stores them by simple name in a request attribute...

You should change the calendar serializer name =)

Copy link
Member Author

Choose a reason for hiding this comment

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

Bad news yet =(

Without context-param works fine!

With context-param, the order the order of the components alter the product:

Works fine because the serializers are listed at the end:
[br.com.caelum.vraptor.serialization.gson.adapters.CalendarSerializer@75679fa7,
br.com.caelum.vraptor.serialization.gson.adapters.MessageSerializer@6622c928,
br.com.caelum.vraptor.serialization.iso8601.gson.DateISO8601Serializer@75c4957a,
br.com.caelum.vraptor.serialization.iso8601.gson.CalendarISO8601Serializer@3d31b8fd]

Don´t work because the deserializers are listed at the top:
[br.com.caelum.vraptor.serialization.iso8601.gson.DateISO8601Deserializer@4d1c488c,
br.com.caelum.vraptor.serialization.iso8601.gson.CalendarISO8601Deserializer@7bdf06e0,
br.com.caelum.vraptor.serialization.gson.adapters.CalendarDeserializer@6f3c9ba8]

Copy link
Member

Choose a reason for hiding this comment

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

have you tried to change the packages order declaration on web.xml?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I've tried before, and the result is the same =/

Copy link
Member Author

Choose a reason for hiding this comment

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

Eventually the log can help:

  • The order of these registrations occurs regardless of the packages order declaration on web.xml
08:19:30,543 DEBUG [GuiceComponentRegistry] Binding class br.com.caelum.vraptor.serialization.gson.adapters.CalendarSerializer to class br.com.caelum.vraptor.serialization.gson.adapters.CalendarSerializer 08:19:30,546 DEBUG [GuiceComponentRegistry] Ignoring binding of class br.com.caelum.vraptor.serialization.gson.adapters.CalendarSerializer to class br.com.caelum.vraptor.serialization.gson.adapters.CalendarSerializer 08:19:30,546 DEBUG [GuiceComponentRegistry] Binding interface com.google.gson.JsonSerializer to class br.com.caelum.vraptor.serialization.gson.adapters.CalendarSerializer 08:19:30,704 DEBUG [GuiceComponentRegistry] Binding class br.com.caelum.vraptor.serialization.iso8601.gson.CalendarISO8601Deserializer to br.com.caelum.vraptor.serialization.iso8601.gson.CalendarISO8601Deserializer 08:19:30,706 DEBUG [GuiceComponentRegistry] Ignoring binding of class br.com.caelum.vraptor.serialization.iso8601.gson.CalendarISO8601Deserializer to br.com.caelum.vraptor.serialization.iso8601.gson.CalendarISO8601Deserializer 08:19:30,706 DEBUG [GuiceComponentRegistry] Ignoring binding of interface com.google.gson.JsonDeserializer to class br.com.caelum.vraptor.serialization.iso8601.gson.CalendarISO8601Deserializer 08:19:30,767 DEBUG [GuiceComponentRegistry] Binding class br.com.caelum.vraptor.serialization.gson.adapters.CalendarDeserializer to class br.com.caelum.vraptor.serialization.gson.adapters.CalendarDeserializer 08:19:30,769 DEBUG [GuiceComponentRegistry] Ignoring binding of class br.com.caelum.vraptor.serialization.gson.adapters.CalendarDeserializer to class br.com.caelum.vraptor.serialization.gson.adapters.CalendarDeserializer 08:19:30,769 DEBUG [GuiceComponentRegistry] Ignoring binding of interface com.google.gson.JsonDeserializer to class br.com.caelum.vraptor.serialization.gson.adapters.CalendarDeserializer 08:19:30,780 DEBUG [GuiceComponentRegistry] Binding class br.com.caelum.vraptor.serialization.iso8601.gson.CalendarISO8601Serializer to class br.com.caelum.vraptor.serialization.iso8601.gson.CalendarISO8601Serializer 08:19:30,781 DEBUG [GuiceComponentRegistry] Ignoring binding of class br.com.caelum.vraptor.serialization.iso8601.gson.CalendarISO8601Serializer to class br.com.caelum.vraptor.serialization.iso8601.gson.CalendarISO8601Serializer 08:19:30,781 DEBUG [GuiceComponentRegistry] Ignoring binding of interface com.google.gson.JsonSerializer to class br.com.caelum.vraptor.serialization.iso8601.gson.CalendarISO8601Serializer 08:19:30,876 DEBUG [ScopeLifecycleListener] Registering lifecycle listeners for br.com.caelum.vraptor.serialization.gson.adapters.CalendarSerializer 08:19:30,895 DEBUG [ScopeLifecycleListener] Registering lifecycle listeners for br.com.caelum.vraptor.serialization.iso8601.gson.CalendarISO8601Deserializer 08:19:30,900 DEBUG [ScopeLifecycleListener] Registering lifecycle listeners for br.com.caelum.vraptor.serialization.gson.adapters.CalendarDeserializer 08:19:30,902 DEBUG [ScopeLifecycleListener] Registering lifecycle listeners for br.com.caelum.vraptor.serialization.iso8601.gson.CalendarISO8601Serializer

Copy link
Member

Choose a reason for hiding this comment

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

@dipold
Copy link
Member Author

dipold commented Jul 31, 2013

Following new PR for evaluation =)

@lucascs
Copy link
Member

lucascs commented Jul 31, 2013

Much simpler. Thanks!

lucascs added a commit that referenced this pull request Jul 31, 2013
Change date/time format to ISO8601 on JSON serialization
@lucascs lucascs merged commit b58965b into caelum:master Jul 31, 2013
@dipold
Copy link
Member Author

dipold commented Jul 31, 2013

Thank you for help!

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

3 participants