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

Upgrade to jackson 2.3.1 (renaming packages); addition for swift api. #137

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

bluebossa63
Copy link

I made a fork for this because the newer jackson library won't be compatible for every resteasy implementation. As I use the SDK as part of an OpenStack Client for Android, I have a focus on performance and the storage cloud.

I'm sorry but there are a lot of changes that are simply formatting issues.

@woorea
Copy link
Owner

woorea commented Mar 15, 2014

This is very huge! Anyway thank you for the pull, I will merge maintaining Jackson compatibility. I tried with Jersey before and I made it work, but I have no tried the new Jackson with RestEasy. I plan to move to a new version ASAP but i want to check with resteasy people before.

Options are:

Jackson 2
Moxy (with this we have XML and JSON out of the box)

I know XML is less important for android stuff, so another posibility is (ie. keystone)

keystone-client

keystone-model (only interfaces)
keystone-model-jackson
keystone-model-jackson2
keystone-model-moxy

What do you think guys?

@bluebossa63
Copy link
Author

I do not think that it makes a lot of sense to split the code because of the libraries.
Resteasy could easily be ported to the new libraries:

http://docs.jboss.org/resteasy/docs/3.0.2.Final/userguide/html/json.html ( Using Jackson 2.2.x Inside of JBoss AS7 )

I would leave it as it is for now, and if you guys decide to move forward with jackson 2, I would integrate the pull fully...

And my pull is not huge, most of the changes are done by removing the old libraries and with a simple eclipse / source / organize imports. I'm afraid, that a lot of formatting changes were done this way. Then there were basically 3 exceptions to this rule:

SerializationConfiguration.Feature -> SerializationFeature
DeserializationConfiguration.Feature -> DeserializationFeature
Inclusions -> Include

And for jersey I had an issue with the HeadersFactory, please check this!

The rest are the changes I already provided as a patch for the Android Client

https://github.com/bluebossa63/OpenStackIntegration

In the changes of the model there is only one thing a little bit strange:

com.woorea.openstack.keystone.model.User

I had some problems with the password field name
7 weeks ago the fix
@JsonProperty("OS-KSADM:password")
was pushed. I will have to test this in the next days, if it works also on android...

JBoss EAP 6.2.0.GA (AS 7.3.0.Final-redhat-14)
and Jackson 2.3.1
@bluebossa63
Copy link
Author

complete refactoring and successfully tested with
JBoss EAP 6.2.0.GA (AS 7.3.0.Final-redhat-14)
and Jackson 2.3.1

bluebossa63@35ddb92

@bluebossa63
Copy link
Author

I had to roll back! The issue was that too much refactoring would be needed:

{
"roles": [
{
"id": "123",
"name": "compute:admin",
"description": "Nova Administrator"
}
],
"roles_links": []
}

was not possible to deserialize with the actual model. Switching back, everything was parsed as expected. There are some heavy differences in parsing between the jackson libraries 1 and 2.

@olivierlemasle
Copy link

Hi,

@woorea do you plan to accept this pull request in the near future?

I tried the SDK with @bluebossa63's support for Jackson 2.3 and it is great.

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.

3 participants