[GitHub] activemq-artemis pull request #1307: [ARTEMIS-1196] Fix missing JSON support

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1307: [ARTEMIS-1196] Fix missing JSON support

michaelandrepearce-2
GitHub user gnodet opened a pull request:

    https://github.com/apache/activemq-artemis/pull/1307

    [ARTEMIS-1196] Fix missing JSON support

   

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/gnodet/activemq-artemis ARTEMIS-1196

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/activemq-artemis/pull/1307.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1307
   
----
commit cc938afc745347fb81e98cacd18d1eb7fd961811
Author: Guillaume Nodet <[hidden email]>
Date:   2017-05-19T08:01:53Z

    [ARTEMIS-1196] Fix missing JSON support

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1307: [ARTEMIS-1196] Fix missing JSON support

michaelandrepearce-2
Github user gnodet commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1307
 
    Not to be committed yet I think, as it depends on a SNAPSHOT.  I'll ping when the spec has been released.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1307: [ARTEMIS-1196] Fix missing JSON support

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1307
 
    Thanks for adding a test.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1307: [ARTEMIS-1196] Fix missing JSON support

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1307#discussion_r119298068
 
    --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/karaf/ArtemisFeatureTest.java ---
    @@ -146,11 +155,26 @@ public Boolean call() throws Exception {
              connection = factory.createConnection(USER, PASSWORD);
              connection.start();
     
    -         javax.jms.Session sess = connection.createSession(false, javax.jms.Session.AUTO_ACKNOWLEDGE);
    +         QueueSession sess = (QueueSession) connection.createSession(false, javax.jms.Session.AUTO_ACKNOWLEDGE);
              Queue queue = sess.createQueue("exampleQueue");
              MessageProducer producer = sess.createProducer(queue);
              producer.send(sess.createTextMessage("TEST"));
     
    +         // Test management
    +         Queue managementQueue = sess.createQueue("activemq.management");
    +         QueueRequestor requestor = new QueueRequestor(sess, managementQueue);
    +         connection.start();
    +         TextMessage m = sess.createTextMessage();
    +         m.setStringProperty("_AMQ_ResourceName", "broker");
    +         m.setStringProperty("_AMQ_OperationName", "getQueueNames");
    +         m.setText("[\"ANYCAST\"]");
    +         Message reply = requestor.request(m);
    +         String json = ((TextMessage) reply).getText();
    +         JsonArray array = Json.createReader(new StringReader(json)).readArray();
    +         List<JsonString> queues = (List<JsonString>) array.get(0);
    +         assertNotNull(queues);
    +         System.out.println(queues.stream().map(JsonString::getString).collect(Collectors.joining(", ", "Queues: ", "")));
    --- End diff --
   
    System out? I assume this is left over from debugging


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1307: [ARTEMIS-1196] Fix missing JSON support

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user gnodet commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1307#discussion_r119338658
 
    --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/karaf/ArtemisFeatureTest.java ---
    @@ -146,11 +155,26 @@ public Boolean call() throws Exception {
              connection = factory.createConnection(USER, PASSWORD);
              connection.start();
     
    -         javax.jms.Session sess = connection.createSession(false, javax.jms.Session.AUTO_ACKNOWLEDGE);
    +         QueueSession sess = (QueueSession) connection.createSession(false, javax.jms.Session.AUTO_ACKNOWLEDGE);
              Queue queue = sess.createQueue("exampleQueue");
              MessageProducer producer = sess.createProducer(queue);
              producer.send(sess.createTextMessage("TEST"));
     
    +         // Test management
    +         Queue managementQueue = sess.createQueue("activemq.management");
    +         QueueRequestor requestor = new QueueRequestor(sess, managementQueue);
    +         connection.start();
    +         TextMessage m = sess.createTextMessage();
    +         m.setStringProperty("_AMQ_ResourceName", "broker");
    +         m.setStringProperty("_AMQ_OperationName", "getQueueNames");
    +         m.setText("[\"ANYCAST\"]");
    +         Message reply = requestor.request(m);
    +         String json = ((TextMessage) reply).getText();
    +         JsonArray array = Json.createReader(new StringReader(json)).readArray();
    +         List<JsonString> queues = (List<JsonString>) array.get(0);
    +         assertNotNull(queues);
    +         System.out.println(queues.stream().map(JsonString::getString).collect(Collectors.joining(", ", "Queues: ", "")));
    --- End diff --
   
    The test already spits out lots of things to the console, so I'm happy to remove that line if you want, though it's really burried in the whole test output.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1307: [ARTEMIS-1196] Fix missing JSON support

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1307#discussion_r119352351
 
    --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/karaf/ArtemisFeatureTest.java ---
    @@ -146,11 +155,26 @@ public Boolean call() throws Exception {
              connection = factory.createConnection(USER, PASSWORD);
              connection.start();
     
    -         javax.jms.Session sess = connection.createSession(false, javax.jms.Session.AUTO_ACKNOWLEDGE);
    +         QueueSession sess = (QueueSession) connection.createSession(false, javax.jms.Session.AUTO_ACKNOWLEDGE);
              Queue queue = sess.createQueue("exampleQueue");
              MessageProducer producer = sess.createProducer(queue);
              producer.send(sess.createTextMessage("TEST"));
     
    +         // Test management
    +         Queue managementQueue = sess.createQueue("activemq.management");
    +         QueueRequestor requestor = new QueueRequestor(sess, managementQueue);
    +         connection.start();
    +         TextMessage m = sess.createTextMessage();
    +         m.setStringProperty("_AMQ_ResourceName", "broker");
    +         m.setStringProperty("_AMQ_OperationName", "getQueueNames");
    +         m.setText("[\"ANYCAST\"]");
    +         Message reply = requestor.request(m);
    +         String json = ((TextMessage) reply).getText();
    +         JsonArray array = Json.createReader(new StringReader(json)).readArray();
    +         List<JsonString> queues = (List<JsonString>) array.get(0);
    +         assertNotNull(queues);
    +         System.out.println(queues.stream().map(JsonString::getString).collect(Collectors.joining(", ", "Queues: ", "")));
    --- End diff --
   
    If we could let's not add to the noise


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1307: [ARTEMIS-1196] Fix missing JSON support

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1307#discussion_r119380503
 
    --- Diff: artemis-features/src/main/resources/features.xml ---
    @@ -54,8 +54,8 @@
      <bundle dependency="true">mvn:org.jboss.logging/jboss-logging/${jboss.logging.version}</bundle>
      <bundle dependency="true">mvn:org.jgroups/jgroups/${jgroups.version}</bundle>
     
    - <bundle dependency="true">mvn:org.apache.geronimo.specs/geronimo-json_1.0_spec/${json-p.spec.version}</bundle>
    - <bundle dependency="true">mvn:org.apache.johnzon/johnzon-core/${johnzon.version}</bundle>
    + <bundle dependency="true">mvn:org.apache.servicemix.specs/org.apache.servicemix.specs.json-api-1.1/2.8-SNAPSHOT</bundle>
    --- End diff --
   
    I can't merge this with a snapshot.. besides.. the right way would be to update json-p.spec.version.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1307: [ARTEMIS-1196] Fix missing JSON support

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user gnodet commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1307#discussion_r119382149
 
    --- Diff: artemis-features/src/main/resources/features.xml ---
    @@ -54,8 +54,8 @@
      <bundle dependency="true">mvn:org.jboss.logging/jboss-logging/${jboss.logging.version}</bundle>
      <bundle dependency="true">mvn:org.jgroups/jgroups/${jgroups.version}</bundle>
     
    - <bundle dependency="true">mvn:org.apache.geronimo.specs/geronimo-json_1.0_spec/${json-p.spec.version}</bundle>
    - <bundle dependency="true">mvn:org.apache.johnzon/johnzon-core/${johnzon.version}</bundle>
    + <bundle dependency="true">mvn:org.apache.servicemix.specs/org.apache.servicemix.specs.json-api-1.1/2.8-SNAPSHOT</bundle>
    --- End diff --
   
    In karaf, we usually use the servicemix specs versions.  They do work on their own, while the geronimo ones need an additional bundle.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1307: [ARTEMIS-1196] Fix missing JSON support

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1307#discussion_r119383359
 
    --- Diff: artemis-features/src/main/resources/features.xml ---
    @@ -54,8 +54,8 @@
      <bundle dependency="true">mvn:org.jboss.logging/jboss-logging/${jboss.logging.version}</bundle>
      <bundle dependency="true">mvn:org.jgroups/jgroups/${jgroups.version}</bundle>
     
    - <bundle dependency="true">mvn:org.apache.geronimo.specs/geronimo-json_1.0_spec/${json-p.spec.version}</bundle>
    - <bundle dependency="true">mvn:org.apache.johnzon/johnzon-core/${johnzon.version}</bundle>
    + <bundle dependency="true">mvn:org.apache.servicemix.specs/org.apache.servicemix.specs.json-api-1.1/2.8-SNAPSHOT</bundle>
    --- End diff --
   
    ok, but I don't like snapshot dependencies on main.. 1 year from now we won't be able to compile the code when the snapshot is gone.. and if I ever go back to this tag.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1307: [ARTEMIS-1196] Fix missing JSON support

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user gnodet commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1307#discussion_r119387203
 
    --- Diff: artemis-features/src/main/resources/features.xml ---
    @@ -54,8 +54,8 @@
      <bundle dependency="true">mvn:org.jboss.logging/jboss-logging/${jboss.logging.version}</bundle>
      <bundle dependency="true">mvn:org.jgroups/jgroups/${jgroups.version}</bundle>
     
    - <bundle dependency="true">mvn:org.apache.geronimo.specs/geronimo-json_1.0_spec/${json-p.spec.version}</bundle>
    - <bundle dependency="true">mvn:org.apache.johnzon/johnzon-core/${johnzon.version}</bundle>
    + <bundle dependency="true">mvn:org.apache.servicemix.specs/org.apache.servicemix.specs.json-api-1.1/2.8-SNAPSHOT</bundle>
    --- End diff --
   
    Yeah, that's why I said it's not to be committed.
    Once there is a release of the spec, I'll update the PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1307: [ARTEMIS-1196] Fix missing JSON support

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1307
 
    @gnodet from what i can tell, 2.9 got released :)
   
    https://mvnrepository.com/artifact/org.apache.servicemix.specs/org.apache.servicemix.specs.json-api-1.1/2.9.0


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1307: [ARTEMIS-1196] Fix missing JSON support

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user gnodet commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1307
 
    Yes, I updated the PR accordingly a while ago.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1307: [ARTEMIS-1196] Fix missing JSON support

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1307#discussion_r123557009
 
    --- Diff: artemis-features/src/main/resources/features.xml ---
    @@ -54,8 +54,8 @@
      <bundle dependency="true">mvn:org.jboss.logging/jboss-logging/${jboss.logging.version}</bundle>
      <bundle dependency="true">mvn:org.jgroups/jgroups/${jgroups.version}</bundle>
     
    - <bundle dependency="true">mvn:org.apache.geronimo.specs/geronimo-json_1.0_spec/${json-p.spec.version}</bundle>
    - <bundle dependency="true">mvn:org.apache.johnzon/johnzon-core/${johnzon.version}</bundle>
    + <bundle dependency="true">mvn:org.apache.servicemix.specs/org.apache.servicemix.specs.json-api-1.1/2.8-SNAPSHOT</bundle>
    --- End diff --
   
    @gnodet I'm about to merge this.. although I'm squashing all your committs as one.. (there's no need to keep individual committs)..
   
   
    although... for my education here..what's the difference between org.apache.geronimo.specs and servicemix? just wanted to know the difference.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1307: [ARTEMIS-1196] Fix missing JSON support

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user gnodet commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1307#discussion_r123559429
 
    --- Diff: artemis-features/src/main/resources/features.xml ---
    @@ -54,8 +54,8 @@
      <bundle dependency="true">mvn:org.jboss.logging/jboss-logging/${jboss.logging.version}</bundle>
      <bundle dependency="true">mvn:org.jgroups/jgroups/${jgroups.version}</bundle>
     
    - <bundle dependency="true">mvn:org.apache.geronimo.specs/geronimo-json_1.0_spec/${json-p.spec.version}</bundle>
    - <bundle dependency="true">mvn:org.apache.johnzon/johnzon-core/${johnzon.version}</bundle>
    + <bundle dependency="true">mvn:org.apache.servicemix.specs/org.apache.servicemix.specs.json-api-1.1/2.8-SNAPSHOT</bundle>
    --- End diff --
   
    So the servicemix specs rewrap the standard specs but add some specifics to OSGi, i.e. each spec includes an OSGi activator which scans OSGi bundles for providers of the specification by scanning the META-INF/services folder. It will use the results of the scanning when a user uses the spec to create an instance.  In a J2SE env, this is done by the spec by looking at the classpath and scanning META-INF/services, but in OSGi, the user package and the spec package to not have visibility over those packages from the provider, so it fails.
    Geronimo specs actually do a somewhat similar thing, but in a somewhat different way, in particular it requires an additional bundle to do the scanning.  The karaf ecosystem usually uses the servicemix specs instead.
    Furthermore, in this very case, the OSGi metadata of the geronimo spec is slightly broken and even if the additional bundle was deployed, the json spec bundle would not be able to see it, so the provider loading mechanism in OSGi can't work as is.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1307: [ARTEMIS-1196] Fix missing JSON support

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user asfgit closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/1307


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---