ARTEMIS-565 Replace json.org with javax.json #640

johnament · 2016-07-17T23:06:01Z

Javax.json is a newer JSR, but has an ASF compliant version, is pretty close to the original JSON.org API and will support a standard annotation based JSON-B solution at some point soon.

This is for discussion purposes only at this point.

mtaylor · 2016-07-19T15:15:16Z

@johnament +1 on this. TBH I don't really mind which library we use as long as it's Apache compliant and doesn't have silly phrases in the license :P. This looks good to me.

puntogil · 2016-07-20T09:54:22Z

hi
thanks for your work
is possible remove from LICENSE file the json.org (license) referencies
regards

johnament · 2016-07-21T12:55:05Z

@puntogil yep, that's why i'm not saying this is ready.

@mtaylor Ok, I'll clean it up.

Thanks for looking at this guys!

johnament · 2016-07-21T21:14:23Z

Ok, the changes should be good. I manually ran the integration tests that were touched, and fixed a bunch of issues identified there.

I'm going to create a ticket to create more lower level json tests. They're a good way to verify contracts in a situation like this. If I were starting this over, I would have added the contracts first.

johnament · 2016-07-23T15:15:56Z

I'm not sure why Jenkins didn't write the result back, but the build did pass in Jenkins.

clebertsuconic · 2016-07-25T18:36:30Z

@johnament this looks good to me. Are you done here? Can I merge it.

clebertsuconic · 2016-07-25T18:40:51Z

you can't run a server. There is some changes needed on the release packaging:

Exception in thread "main" java.lang.NoClassDefFoundError: javax/json/JsonValue
    at org.apache.activemq.artemis.uri.schema.connector.TCPTransportConfigurationSchema.getTransportConfigurations(TCPTransportConfigurationSchema.java:68)
    at org.apache.activemq.artemis.uri.schema.serverLocator.TCPServerLocatorSchema.internalNewObject(TCPServerLocatorSchema.java:43)
    at org.apache.activemq.artemis.uri.schema.serverLocator.TCPServerLocatorSchema.internalNewObject(TCPServerLocatorSchema.java:33)
    at org.apache.activemq.artemis.utils.uri.URISchema.newObject(URISchema.java:87)
    at org.apache.activemq.artemis.utils.uri.URISchema.newObject(URISchema.java:30)
    at org.apache.activemq.artemis.utils.uri.URIFactory.newObject(URIFactory.java:59)
    at org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl.newLocator(ServerLocatorImpl.java:413)
    at org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory.<init>(ActiveMQConnectionFactory.java:179)
    at org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory.<init>(ActiveMQConnectionFactory.java:199)
    at org.apache.activemq.artemis.cli.commands.Producer.execute(Producer.java:52)
    at org.apache.activemq.artemis.cli.Artemis.internalExecute(Artemis.java:110)
    at org.apache.activemq.artemis.cli.Artemis.execute(Artemis.java:72)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)
    at org.apache.activemq.artemis.boot.Artemis.execute(Artemis.java:122)
    at org.apache.activemq.artemis.boot.Artemis.main(Artemis.java:45)
Caused by: java.lang.ClassNotFoundException: javax.json.JsonValue

johnament · 2016-07-25T18:46:29Z

Ah good catch. I can fix that, probably some assembly issues. I'm assuming shipping johnzon as the default is fine?

clebertsuconic · 2016-07-25T18:56:46Z

Looks fine to me.

I reversed engineered the code and I'm not sure how you specify the JSON provider. it seems you need something on the CLASSINFO?

Also, I just realized the main pom doesn't specify the licenses. a small thing but since you're changing it, can you also do it? will comment at the pom to make it easier to find it.

clebertsuconic · 2016-07-25T18:58:01Z

pom.xml

+         <dependency>
+            <groupId>org.apache.geronimo.specs</groupId>
+            <artifactId>geronimo-json_1.0_spec</artifactId>
+            <version>${json-p.spec.version}</version>


Do you mind mentioning Apache license here, just to keep up the list?

clebertsuconic · 2016-07-25T18:59:44Z

@johnament how users doing embedded will use json? anything special they have to do? or just the maven dependencies would catch it?

johnament · 2016-07-25T19:00:55Z

@clebertsuconic yes, once i fix the distribution the johnzon one will ship by default.

clebertsuconic · 2016-07-25T19:02:33Z

@johnament I was thinking about embedded cases (using the library direct), like wildfly.

We can fix whatever needs on wildfly for next versions.. so I was really thinking about other cases.. I will look through the examples.

Maybe you could also check it by running artemis-distribution/src/test/scripts/run-examples.sh after building the server.

johnament · 2016-07-25T19:07:05Z

@clebertsuconic oh, for wildfly, you'll need to add dependencies on javax.json.api and org.glassfish.javax.json modules.

As far as plain library usage, the dependency tree should come in. If you want to plugin your own, just exclude.

clebertsuconic · 2016-07-25T19:12:27Z

@johnament cool.. thanks a lot.
And sorry for my delay on this. was in vacations last week. :)

Javax.json is a newer JSR, but has an ASF compliant version, is pretty close to the original JSON.org API and will support a standard annotation based JSON-B solution at some point soon. Updated integration tests and removed JSON.org from license.

johnament · 2016-07-26T00:53:52Z

No problems @clebertsuconic hope it went well.

I pushed up fixes, one oddity is that when starting the broker I get this on the console...

20:50:26,422 INFO  [org.apache.activemq.artemis.core.server] Checking...
20:50:28,425 INFO  [org.apache.activemq.artemis.core.server] Checking...
20:50:30,428 INFO  [org.apache.activemq.artemis.core.server] Checking...

Not sure if its related to my changes, or something else.

johnament · 2016-07-26T01:01:47Z

I just checked. its happening with my changes. I can't find this log message anywhere though.

jbertram · 2016-07-26T01:21:33Z

You need to rebase. That issue was resolved last week I believe.

clebertsuconic · 2016-07-26T01:29:32Z

I checked your commit and it was ok.

(I always use this script to merge PRs which will always rebase them, that's probably why it was ok)...
https://github.com/apache/activemq-artemis/blob/master/scripts/merge-PR.sh

no need to rebase on that case

clebertsuconic · 2016-07-26T01:47:41Z

I included 2 lines change features.xml towards the OSGI bundle and I squashed into your commit. I hope you won't mind. It was easier this way.

clebertsuconic · 2016-07-26T01:50:28Z

@johnament can you assign this JIRA to yourself? I couldn't find you (don't know why):

https://issues.apache.org/jira/browse/ARTEMIS-565

johnament · 2016-07-26T02:28:34Z

I pinged the PMC to add me as contributor. That will allow JIRA assignment.

I must have had something cached, as this was after I rebased.

johnament force-pushed the ARTEMIS-565 branch 2 times, most recently from 4439d23 to c655298 Compare July 21, 2016 21:11

clebertsuconic reviewed Jul 25, 2016
View reviewed changes

johnament force-pushed the ARTEMIS-565 branch from c655298 to ed9686a Compare July 26, 2016 00:52

asfgit closed this in 110158b Jul 26, 2016

johnament deleted the ARTEMIS-565 branch July 26, 2016 02:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARTEMIS-565 Replace json.org with javax.json #640

ARTEMIS-565 Replace json.org with javax.json #640

johnament commented Jul 17, 2016

mtaylor commented Jul 19, 2016

puntogil commented Jul 20, 2016

johnament commented Jul 21, 2016

johnament commented Jul 21, 2016

johnament commented Jul 23, 2016

clebertsuconic commented Jul 25, 2016

clebertsuconic commented Jul 25, 2016

johnament commented Jul 25, 2016

clebertsuconic commented Jul 25, 2016

clebertsuconic Jul 25, 2016

clebertsuconic commented Jul 25, 2016

johnament commented Jul 25, 2016

clebertsuconic commented Jul 25, 2016

johnament commented Jul 25, 2016

clebertsuconic commented Jul 25, 2016

johnament commented Jul 26, 2016

johnament commented Jul 26, 2016

jbertram commented Jul 26, 2016

clebertsuconic commented Jul 26, 2016

clebertsuconic commented Jul 26, 2016

clebertsuconic commented Jul 26, 2016

johnament commented Jul 26, 2016

ARTEMIS-565 Replace json.org with javax.json #640

ARTEMIS-565 Replace json.org with javax.json #640

Conversation

johnament commented Jul 17, 2016

mtaylor commented Jul 19, 2016

puntogil commented Jul 20, 2016

johnament commented Jul 21, 2016

johnament commented Jul 21, 2016

johnament commented Jul 23, 2016

clebertsuconic commented Jul 25, 2016

clebertsuconic commented Jul 25, 2016

johnament commented Jul 25, 2016

clebertsuconic commented Jul 25, 2016

clebertsuconic Jul 25, 2016

Choose a reason for hiding this comment

clebertsuconic commented Jul 25, 2016

johnament commented Jul 25, 2016

clebertsuconic commented Jul 25, 2016

johnament commented Jul 25, 2016

clebertsuconic commented Jul 25, 2016

johnament commented Jul 26, 2016

johnament commented Jul 26, 2016

jbertram commented Jul 26, 2016

clebertsuconic commented Jul 26, 2016

clebertsuconic commented Jul 26, 2016

clebertsuconic commented Jul 26, 2016

johnament commented Jul 26, 2016