You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by gtully <gi...@git.apache.org> on 2018/01/26 15:38:11 UTC

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

GitHub user gtully opened a pull request:

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

    [ARTEMIS-550] fix up test to validate CORE and implement FQQN check b…

    …efore core auto createQueue call

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

    $ git pull https://github.com/gtully/activemq-artemis ARTEMIS-550

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

    https://github.com/apache/activemq-artemis/pull/1820.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 #1820
    
----
commit 4c0e164bea121ea3a03fc0bc9063f223e7a7f1f7
Author: gtully <ga...@...>
Date:   2018-01-26T15:28:59Z

    [ARTEMIS-550] fix up test to validate CORE and implement FQQN check before core auto createQueue call

----


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164268415
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    I don’t see how that’s breaking, it’s been clear virtual topics as it is in activemq5 isn’t going to be ported as is, as use cases will be covered either with JMS 2.0 spec or FQQN (note FQQN != virtual topic). 
    
    As noted here your use case is covered by jms shared subscriptions.
    
    This PR atm breaks the documented address model to JMS mapping’s that exists. That a JMS queue maps onto Anycast Address and a JMS topic maps onto an Multicast Address.
    



---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164170164
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    Please also see https://github.com/apache/activemq-artemis/blob/701ab1feba25d660b118dac79f77205326c9e90d/docs/migration-guide/en/VirtualTopics.md


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by graben <gi...@git.apache.org>.
Github user graben commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164173035
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    Got it. I misinterpreted spec. I did a test and it works. Fine for me. Thx ;-)


---

[GitHub] activemq-artemis issue #1820: [ARTEMIS-550] fix up test to validate CORE and...

Posted by gtully <gi...@git.apache.org>.
Github user gtully commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1820
  
    Hope we don't lose the pictures, but this PR as it stands is too simplistic and the use case is already covered by a shared durable sub.
    the prefixed fqqn will be the way to go if we need to improve in this area.
    https://activemq.apache.org/artemis/docs/2.3.0/address-model.html#configuring-a-prefix-to-connect-to-a-specific-routing-type
    
    thanks for the feedback :-)


---

[GitHub] activemq-artemis issue #1820: [ARTEMIS-550] fix up test to validate CORE and...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1820
  
    @gtully agreed.
    
    I think it be great if @mattrpav for the use cases we think we can't cover today, theyre detailed each in a JIRA, this way we can tackle one by one, or if they are possible today, then we can create Documents in how.


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by gtully <gi...@git.apache.org>.
Github user gtully commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164445884
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    @michaelandrepearce  I agree with you, I will close out this PR. 
    The discussion is the valuable bit.
    For this use case, SDS is the answer. 
    For the more general CORE FQQN scenario, using a prefix with a FQQN would be the other approach. It probably should support the prefix notation via the acceptor.
     ```multicast://VirtualTopic.Orders::Consumer.A```
    This says, create me a sub queue called ```Consumer.A`` that is bound using ```multicast``` routing to address ```VirtualTopic.Orders```.



---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164167446
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    No they don’t. Please see spec. Client ID is optional for a shared subscription.  Using a SharedDurableConsumer without setting a client id will give you queue like consuming semantics on a JMS Topic


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by graben <gi...@git.apache.org>.
Github user graben commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164181992
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    Well it depends. FQQN is a documented feature so usage this way might be possible. Make it work or document as not working. 


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by gtully <gi...@git.apache.org>.
Github user gtully commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164159618
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    it is, at least the use case in the test. It may be that some FQQN scenarios are not applicable to CORE.
    I am not sure how to determine the routingTypeToUse if both the address and subscription queue need to be created.
    Is there a way to know this is a fanout consumer queue/subscription queue rather than a simple ANYCAST address.
    On the server in the openwire case there is a binding query such the address provides this information but I did not want to go down the route of another round trip.


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by graben <gi...@git.apache.org>.
Github user graben commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164166706
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    Sorry, that's not correct vt and shared subscriptions are only similar but not equal. Shared subscriptions still need connection to have a clientid. Therefor those connections are bounded to a special use case. VTs has queue semantic. No "special" connections are needed. Such pooling of connections for almost everything sending/consuming/... is possible. Artemis still leaks such a feature on its core protocol.


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164168395
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    This is there today.


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by gtully <gi...@git.apache.org>.
Github user gtully commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164162902
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    The original test case that I added to, had both artemis and openwire jms clients in the mix, a parameterised test. It may be that the vt use case needs to be constrained to openwire because I think your expectation is correct. I guess in the case that the address already existed I may be able to find the routing type there. 
    But when both address and queue need to be created core would be out of luck.



---

[GitHub] activemq-artemis issue #1820: [ARTEMIS-550] fix up test to validate CORE and...

Posted by mattrpav <gi...@git.apache.org>.
Github user mattrpav commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1820
  
    @michaelandrepearce the IBM MQ scenario I was referring to was the ability to use a named queue to back a topic subscription, not cloned subscriptions-- this provides the separation of producers from consumers in terms of "its a different destination". This enables better wild card management for permissions and other access features to destinations. ie.. topic://ORDERING.**  and queue://BILLING.**
    
    I agree JMS 2.0 topic subscriptions solve the "same broker" and "same destination" use case for application-side ability to dynamically add additional consumers to a message flow without broker config or existing producer and consumer(s) impacts. 
    
    Note: on the exclusive consumer part, it sounds like Artemis has a gap vs 5.x in that the exclusive consumer config is server-side only (separate issues raised as [ARTEMIS-853] and [ARTEMIS-856]).
    
    The key feature in the broker-to-broker message scenarios is that the publishing destination is different than the consuming destination(s). In ActiveMQ 5.x, this allows for two important multi-broker messaging patterns, as well as enable the network of broker destination filtering (include / exclude), client permissions and destination policy mapping via wild card syntax sugar for administrators. See attached graphics
    
    Multi-broker Use Case 1:
    ![activemq-vt-multibroker-uc1](https://user-images.githubusercontent.com/512119/35476052-297380ae-036f-11e8-98c7-e78ffbff9774.png)
    
    Multi-broker Use Case 2:
    ![activemq-vt-multibroker-uc2](https://user-images.githubusercontent.com/512119/35476051-2962c994-036f-11e8-93d7-e4f919f6bf4f.png)
    



---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by gtully <gi...@git.apache.org>.
Github user gtully closed the pull request at:

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


---

[GitHub] activemq-artemis issue #1820: [ARTEMIS-550] fix up test to validate CORE and...

Posted by mattrpav <gi...@git.apache.org>.
Github user mattrpav commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1820
  
    Will do


---

[GitHub] activemq-artemis issue #1820: [ARTEMIS-550] fix up test to validate CORE and...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1820
  
    If it helps so here is the conceptual logical model of VT:
    
    ![vt](https://user-images.githubusercontent.com/1387822/35473197-3304146c-0374-11e8-9e33-3506a3633e04.png)
    
    And here is the conceptual model of how JMS 2.0 SharedDurableSubscriber maps into Core Address model.
    
    ![shared](https://user-images.githubusercontent.com/1387822/35473219-694e916e-0374-11e8-90d8-5310d900e73a.png)
    
    If you note essentially the VT is the Address (its multicast), and then the queues are.....queues.
    
    I don't see much difference now.



---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164188968
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    And it does what it should that on a queue an anycast destination is created


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164165293
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    Eg I think it should be constrained on core. As really virtual topics was a pre cursor custom impl of shared subscriptions in JMS 2 spec which we have already


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164147983
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    Is this related to Virtual Topics support for OpenWire?


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164176155
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    Great news!!!


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164168934
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    https://docs.oracle.com/javaee/7/api/javax/jms/Session.html#createSharedDurableConsumer-javax.jms.Topic-java.lang.String-


---

[GitHub] activemq-artemis issue #1820: [ARTEMIS-550] fix up test to validate CORE and...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1820
  
    @mattrpav i pinged on IRC but no response.
    
    From the JIRA i see you raised three points.
    
    
    > Consumption is in a different destination altogether
    
    As noted already a core queue is used for delivery either sharing a core queue (as like a JMS Queue, or JMS 2.0 Shared subscription) or having a core queue per consumer (as like JMS Topic consumer - non shared).
    
    > Consumption is handled by a queue-- which supports wider range of consuming options-- exclusive consumer, etc.
    
    In Artemis a jms 2.0 shared subscription is a queue (note this is part of the core model) on this core queues are possible to set this up with max consumers = 1 to represent an exclusive consumer.
    
    
    > Client-consumers drive the behavior without the need for server-side configuration
    
    Exactly the point of JMS 2.0 Topics, here you can send to a topic, and now you can get either queue like consuming semantics by using Shared Durable Subscription, or you can have JMS 1.1 pub sub semantic using standard Topic Consumer.
    
    
    On that note, i think if anything more is needed (such as if an address or queue is not found, detailing the requested routing type) it should be done with extending the FQQN which overrides the protocol managers, not by changing the core client just to blindly do something breaking others.



---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164163557
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    So if this is related to virtual topics why complicate things for the Core JMS client and introduce such problems it has JMS 2.0 spec as such shared durable subscribers, which is the spec equiv of such feature.


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164147250
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    If this is a JMS Queue (des.isQueue), it should only be an ANYCAST


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164272887
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    @mattrpav im on IRC. fyi im not saying all use cases are solved by SDS, but in this case it was. We should be looking to map use cases / functionality over, not lifting and shifting virt topics as is.


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by graben <gi...@git.apache.org>.
Github user graben commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164266537
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    Look at my test case in JIRA ARTEMIS-550 and you'll see that there is still a problem concerning usage of FQQN via core protocol.


---

[GitHub] activemq-artemis pull request #1820: [ARTEMIS-550] fix up test to validate C...

Posted by mattrpav <gi...@git.apache.org>.
Github user mattrpav commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1820#discussion_r164272477
  
    --- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
    @@ -698,8 +699,17 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
                  */
                 if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) {
                    if (response.isAutoCreateQueues()) {
    +                  SimpleString queueNameToUse = dest.getSimpleAddress();
    +                  SimpleString addressToUse = queueNameToUse;
    +                  RoutingType routingTypeToUse = RoutingType.ANYCAST;
    +                  if (CompositeAddress.isFullyQualified(queueNameToUse.toString())) {
    +                     CompositeAddress compositeAddress = CompositeAddress.getQueueName(queueNameToUse.toString());
    +                     addressToUse = new SimpleString(compositeAddress.getAddress());
    +                     queueNameToUse = new SimpleString(compositeAddress.getQueueName());
    +                     routingTypeToUse = RoutingType.MULTICAST;
    --- End diff --
    
    @michaelandrepearce I shared some use cases and scenarios regarding 5.x virtual topics on 550. JMS 2.0 SharedDurable subscriptions is only one use case that virtual topics help with. However, there are several that JMS 2.0 SDS won’t cover that 5.x virtual topics do. Additionally, other brokers have these features, so I believe they are “messaging patterns” vs “ActiveMQ 5.x-only” thing. 
    
    Check it out, and let me know if you want to hop on irc or a chat to go review it.  
    
    At any rate, I think Artemis would benefit from supporting virtual topics (or whatever new name is more suitable)


---