Skip to content

Updated jackson version#339

Merged
asfgit merged 4 commits intoapache:masterfrom
Graeme-Miller:jackson_version_update
Sep 27, 2016
Merged

Updated jackson version#339
asfgit merged 4 commits intoapache:masterfrom
Graeme-Miller:jackson_version_update

Conversation

@Graeme-Miller
Copy link
Copy Markdown
Contributor

@Graeme-Miller Graeme-Miller commented Sep 21, 2016

This is successor to this PR.
After the rest client was moved to brooklyn-client in this PR two new PRs had to be created, one for brooklyn-server and one for brooklyn-client.

Please merge this PR at the same time.

This PR updates jackson to 2.7.5.

@aledsage
Copy link
Copy Markdown
Contributor

As stated by @Graeme-Miller in #317, the reason for the version bump is that another downstream project has a dependency on https://github.com/fabric8io/kubernetes-client which requires the later version.

@aledsage
Copy link
Copy Markdown
Contributor

@sjcorbett observed in #317 that io.swagger pulls in 2.4.5 of dataformat-jackson-dataformat-yaml etc.

@neykov said:

That's a good spot @sjcorbett. The jackson library version mix seems important. 
I think it's better to stick to the same version (even if higher than required 
for swagger), than to risk incompatibility. It's higher risk to use two parts 
of the library with different versions than upgrade all of them.

Another reason to upgrade everything is that in Karaf we are explicitly adding 
the dependencies and pinning them to the fasterxml.jackson.version. This causes 
the classic distribution to have a different mix of jars than the karaf one - 
best to keep them consistent.

Even in karaf, when using brooklyn-tosca-karaf-init, we'll get the same mix of class versions: jackson-datatype-joda-2.4.5.jar along with the jackson-annotations bundle version 2.7.5.

I'm not sure how safe that version mix is. But I'm also not sure how safe it is to upgrade datatype-jackson-datatype-joda etc to 2.7.5 (e.g. will that be binary compatible with swagger 2.4.5, or will it break in any code paths?).

The options seem to be:

  1. Reject this PR (and force downstream projects to figure out how to cope with Brooklyn using an old version of jackson-core).
  2. Just upgrade jackson to 2.7.5 (as proposed in this PR): trust our QA, and trust jackson to be doing semantic versioning.
  3. Build a custom swagger-core 1.5.6 OSGi bundle, so it depends on exactly jackson 2.4.5 (and thus have both versions safely available in Karaf)

And we could also optionally:

  1. Have brooklyn-tosca-karaf-init also bundle jackson-core-2.4.5.jar

Option (3) sounds horrible.

Option (1) passes the pain onto downstream projects. Even if using Karaf, if that downstream project was to cause jackson-core 2.7.5 bundle to be included in Karaf, then swagger would immediately pick it up instead (because it depends on import version [2.4,3)). It could try including the jars in the OSGi bundle's Bundle-ClassPath, but that's annoying to have to do!

I'd be willing to risk option (2) and merge this as-is, given we've tested it. Also swagger have declared jackson dependency as [2.4,3) so hopefully it means they trust jackson compatibility!

I hate figuring out the right dependency versions!


Investigating the dependencies, brooklyn-dist/dist/target/brooklyn-dist/brooklyn has:

./lib/brooklyn/com.fasterxml.jackson.core-jackson-annotations-2.7.5.jar
./lib/brooklyn/com.fasterxml.jackson.core-jackson-core-2.7.5.jar
./lib/brooklyn/com.fasterxml.jackson.core-jackson-databind-2.7.5.jar
./lib/brooklyn/com.fasterxml.jackson.dataformat-jackson-dataformat-xml-2.4.5.jar
./lib/brooklyn/com.fasterxml.jackson.dataformat-jackson-dataformat-yaml-2.4.5.jar
./lib/brooklyn/com.fasterxml.jackson.datatype-jackson-datatype-joda-2.4.5.jar
./lib/brooklyn/com.fasterxml.jackson.jaxrs-jackson-jaxrs-base-2.7.5.jar
./lib/brooklyn/com.fasterxml.jackson.jaxrs-jackson-jaxrs-json-provider-2.7.5.jar
./lib/brooklyn/com.fasterxml.jackson.module-jackson-module-jaxb-annotations-2.7.5.jar

Whereas brooklyn-dist/karaf/apache-brooklyn/target/assembly has:

./system/com/fasterxml/jackson/core/jackson-annotations/2.7.5/jackson-annotations-2.7.5.jar
./system/com/fasterxml/jackson/core/jackson-core/2.7.5/jackson-core-2.7.5.jar
./system/com/fasterxml/jackson/core/jackson-databind/2.7.5/jackson-databind-2.7.5.jar
./system/com/fasterxml/jackson/dataformat/jackson-dataformat-yaml/2.7.5/jackson-dataformat-yaml-2.7.5.jar
./system/com/fasterxml/jackson/jaxrs/jackson-jaxrs-base/2.7.5/jackson-jaxrs-base-2.7.5.jar
./system/com/fasterxml/jackson/jaxrs/jackson-jaxrs-json-provider/2.7.5/jackson-jaxrs-json-provider-2.7.5.jar

Looking at a downstream karaf that also bundles the brooklyn-tosca feature (with the bundle brooklyn-tosca-karaf-init/0.10.0-SNAPSHOT), the system dir has:

./system/com/fasterxml/jackson/core/jackson-annotations/2.4.5/jackson-annotations-2.4.5.jar
./system/com/fasterxml/jackson/core/jackson-annotations/2.7.5/jackson-annotations-2.7.5.jar
./system/com/fasterxml/jackson/core/jackson-core/2.4.5/jackson-core-2.4.5.jar
./system/com/fasterxml/jackson/core/jackson-core/2.7.5/jackson-core-2.7.5.jar
./system/com/fasterxml/jackson/core/jackson-databind/2.4.5/jackson-databind-2.4.5.jar
./system/com/fasterxml/jackson/core/jackson-databind/2.7.5/jackson-databind-2.7.5.jar
./system/com/fasterxml/jackson/dataformat/jackson-dataformat-yaml/2.4.5/jackson-dataformat-yaml-2.4.5.jar
./system/com/fasterxml/jackson/dataformat/jackson-dataformat-yaml/2.7.5/jackson-dataformat-yaml-2.7.5.jar
./system/com/fasterxml/jackson/jaxrs/jackson-jaxrs-base/2.4.5/jackson-jaxrs-base-2.4.5.jar
./system/com/fasterxml/jackson/jaxrs/jackson-jaxrs-base/2.7.5/jackson-jaxrs-base-2.7.5.jar
./system/com/fasterxml/jackson/jaxrs/jackson-jaxrs-json-provider/2.4.5/jackson-jaxrs-json-provider-2.4.5.jar
./system/com/fasterxml/jackson/jaxrs/jackson-jaxrs-json-provider/2.7.5/jackson-jaxrs-json-provider-2.7.5.jar

But using the karaf client, after doing feature:install brooklyn-tosca and confirming that brooklyn-tosca-karaf-init is active, its bundles are:

karaf@amp>bundle:list -t 0 -s | grep jackson
 59 | Active   |  80 | 2.7.5                         | com.fasterxml.jackson.core.jackson-annotations
 60 | Active   |  80 | 2.7.5                         | com.fasterxml.jackson.core.jackson-core
 61 | Active   |  80 | 2.7.5                         | com.fasterxml.jackson.core.jackson-databind
 62 | Active   |  80 | 2.7.5                         | com.fasterxml.jackson.dataformat.jackson-dataformat-yaml
 63 | Active   |  80 | 2.7.5                         | com.fasterxml.jackson.jaxrs.jackson-jaxrs-base
 64 | Active   |  80 | 2.7.5                         | com.fasterxml.jackson.jaxrs.jackson-jaxrs-json-provider

bundle:headers brooklyn-tosca-karaf-init shows that Bundle-ClassPath includes things like jackson-datatype-joda-2.4.5.jar, jackson-dataformat-yaml-2.4.5.jar (but not jackson-annotations jar).

The bundle io.swagger.core's Import-Package includes com.fasterxml.jackson.annotation;version="[2.4,3)" so presumably that has just picked up 2.7.5, rather than installing 2.4.5.

This means that we'll be using a mix of class versions: jackson-datatype-joda-2.4.5.jar along with the jackson-annotations bundle version 2.7.5.


According to mvn dependency:tree, swagger apidocs (i.e. utils/rest-swagger/pom.xml`) brings in:

[INFO] ------------------------------------------------------------------------
[INFO] Building Brooklyn REST Swagger Apidoc Utilities 0.10.0-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO] 
[INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ brooklyn-utils-rest-swagger ---
[INFO] org.apache.brooklyn:brooklyn-utils-rest-swagger:jar:0.10.0-SNAPSHOT
[INFO] +- javax.ws.rs:javax.ws.rs-api:jar:2.0.1:compile
[INFO] +- javax.servlet:javax.servlet-api:jar:3.1.0:compile
[INFO] +- io.swagger:swagger-core:jar:1.5.6:compile
[INFO] |  +- org.slf4j:slf4j-api:jar:1.6.6:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-annotations:jar:2.7.5:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-databind:jar:2.7.5:compile
[INFO] |  |  \- com.fasterxml.jackson.core:jackson-core:jar:2.7.5:compile
[INFO] |  +- com.fasterxml.jackson.datatype:jackson-datatype-joda:jar:2.4.5:compile
[INFO] |  |  \- joda-time:joda-time:jar:2.2:compile
[INFO] |  +- com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.4.5:compile
[INFO] |  |  \- org.yaml:snakeyaml:jar:1.11:compile
[INFO] |  +- io.swagger:swagger-models:jar:1.5.6:compile
[INFO] |  |  \- io.swagger:swagger-annotations:jar:1.5.6:compile
[INFO] |  \- javax.validation:validation-api:jar:1.1.0.Final:compile
[INFO] +- io.swagger:swagger-jaxrs:jar:1.5.6:compile
[INFO] |  +- com.fasterxml.jackson.dataformat:jackson-dataformat-xml:jar:2.4.5:compile
[INFO] |  |  +- com.fasterxml.jackson.module:jackson-module-jaxb-annotations:jar:2.7.5:compile
[INFO] |  |  \- org.codehaus.woodstox:stax2-api:jar:3.1.4:compile
[INFO] |  +- org.reflections:reflections:jar:0.9.9-RC1:compile
[INFO] |  |  +- org.javassist:javassist:jar:3.16.1-GA:compile
[INFO] |  |  \- dom4j:dom4j:jar:1.6.1:compile
[INFO] |  |     \- xml-apis:xml-apis:jar:1.0.b2:compile
[INFO] |  \- com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:jar:2.7.5:compile
[INFO] |     \- com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:jar:2.7.5:compile
...

But then in camp server, it just brings in:

[INFO] +- org.apache.brooklyn:brooklyn-utils-rest-swagger:jar:0.10.0-SNAPSHOT:compile
[INFO] |  +- javax.ws.rs:javax.ws.rs-api:jar:2.0.1:compile
[INFO] |  +- io.swagger:swagger-core:jar:1.5.6:compile
[INFO] |  |  +- com.fasterxml.jackson.datatype:jackson-datatype-joda:jar:2.4.5:compile
[INFO] |  |  |  \- joda-time:joda-time:jar:2.2:compile
[INFO] |  |  +- com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.4.5:compile
[INFO] |  |  \- io.swagger:swagger-models:jar:1.5.6:compile
[INFO] |  |     \- io.swagger:swagger-annotations:jar:1.5.6:compile
[INFO] |  \- io.swagger:swagger-jaxrs:jar:1.5.6:compile
[INFO] |     +- com.fasterxml.jackson.dataformat:jackson-dataformat-xml:jar:2.4.5:compile
[INFO] |     |  \- org.codehaus.woodstox:stax2-api:jar:3.1.4:compile
[INFO] |     \- org.reflections:reflections:jar:0.9.9-RC1:compile
[INFO] |        +- org.javassist:javassist:jar:3.16.1-GA:compile
[INFO] |        \- dom4j:dom4j:jar:1.6.1:compile
[INFO] |           \- xml-apis:xml-apis:jar:1.0.b2:compile

Why does it show different transient dependencies for the two uses of brooklyn-utils-rest-swagger?!

The utils/rest-swagger/pom.xml has:

        <dependency>
            <groupId>io.swagger</groupId>
            <artifactId>swagger-core</artifactId>
            <exclusions>
                <exclusion>
                    <groupId>org.slf4j</groupId>
                    <artifactId>slf4j-log4j12</artifactId>
                </exclusion>
                <exclusion>
                    <groupId>org.apache.commons</groupId>
                    <artifactId>commons-lang3</artifactId>
                </exclusion>
                <exclusion>
                  <groupId>com.google.guava</groupId>
                  <artifactId>guava</artifactId>
                </exclusion>
            </exclusions>
        </dependency>

And the camp/camp-server/pom.xml has:

        <dependency>
            <groupId>org.apache.brooklyn</groupId>
            <artifactId>brooklyn-utils-rest-swagger</artifactId>
            <version>${project.version}</version>
        </dependency>

In parent/pom.xml, the dependencyManagement declares the following (so does not exclude or change the jackson-annotations version):

            <dependency>
                <groupId>io.swagger</groupId>
                <artifactId>swagger-core</artifactId>
                <version>${swagger.version}</version>
                <exclusions>
                    <exclusion>
                        <groupId>org.slf4j</groupId>
                        <artifactId>slf4j-log4j12</artifactId>
                    </exclusion>
                </exclusions>
            </dependency>

Looking in http://search.maven.org/#artifactdetails%7Cio.swagger%7Cswagger-core%7C1.5.6%7Cpom and http://search.maven.org/#artifactdetails%7Cio.swagger%7Cswagger-project%7C1.5.6%7Cpom, it definitely depends on com.fasterxml.jackson.core:jackson-annotations:2.4.5 (so I'm not sure why utils/rest-swagger shows it as 2.7.5!)

It also has some surprising use of <excludes>. It explicitly depends on jackson-annotations:2.4.5 and jackson-databind:2.4.5, and then in subsequent dependencies (e.g. jackson-datatype-joda:2.4.5) it excludes jackson-annotations and jackson-databind. This is because http://search.maven.org/#artifactdetails%7Ccom.fasterxml.jackson.datatype%7Cjackson-datatype-joda%7C2.4.5%7Cbundle depends on jackson-annotations:2.4.0 (rather than 2.4.5)!

@Graeme-Miller
Copy link
Copy Markdown
Contributor Author

We have decided to proceed with updating the dependency.
We have also added a commit recommended by Svet in the predecessor to this PR: neykov@62613c0

Lastly Swagger has been forbidden from pulling in any jackson versions

@Graeme-Miller
Copy link
Copy Markdown
Contributor Author

I have tested this with vanilla/karaf brooklyn and karaf AMP.
I have tested deploying a blueprint and using the Swagger API.

@aledsage
Copy link
Copy Markdown
Contributor

LGTM; merging.

@asfgit asfgit merged commit 6453a6f into apache:master Sep 27, 2016
asfgit pushed a commit that referenced this pull request Sep 27, 2016
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.

4 participants