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

Improve GsonEngine and ContentType registration #556

Open
8 of 9 tasks
mhagnumdw opened this issue Jul 6, 2020 · 2 comments
Open
8 of 9 tasks

Improve GsonEngine and ContentType registration #556

mhagnumdw opened this issue Jul 6, 2020 · 2 comments
Assignees

Comments

@mhagnumdw
Copy link
Member

mhagnumdw commented Jul 6, 2020

GsonEngine

  • Allow to inherit from GsonEngine and define the creation of the Gson object
  • Extract the Gson TypeAdapters that are inside the GsonEngine class
  • The type adapter ISO8601DateTimeTypeAdapter can make use of com.google.gson.internal.bind.util.ISO8601Utils.parse(String, ParsePosition)
  • GsonEngineTest is in the wrong place and is not even running by automatic tests
  • Add tests for ISO8601
  • The date parse for 2019-12-27T20:00:00.000Z does not work (the problem is this .000Z)
  • Remove this type of thing (highlighted part): (date.getTime() / 1000) * 1000) - example here, here and here
  • Update Gson from 2.3.1 to 2.8.6

ContentType

  • The setTemplateEngine method, commonly called within Application.onInit(), must override the previously defined engine
@mhagnumdw
Copy link
Member Author

Hi all!

What do you think of the above proposal? There is already a draft PR.

@decebals ,

The setTemplateEngine method, commonly called within Application.onInit(), must override the previously defined engine

If I have pippo-gson in pom.xml and register my JSON engine (which inherits from GsonEngine), it is not registered because Pippo previously registered GsonEngine.

I think we have to make some changes to ContentTypeEngines.registerContentTypeEngine. Do you suggest anything?

@mhagnumdw mhagnumdw self-assigned this Jul 7, 2020
@mhagnumdw
Copy link
Member Author

@decebals , when you have a little time, will it be if you can take a look here?

I don't know who else I can ping to take a look here.

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 a pull request may close this issue.

1 participant