You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by cshannon <gi...@git.apache.org> on 2018/01/23 12:48:43 UTC

[GitHub] activemq-artemis pull request #1807: ARTEMIS-1627 - Support removing address...

GitHub user cshannon opened a pull request:

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

    ARTEMIS-1627 - Support removing addresses that do not have direct bindings

    If there are no direct bindings on an address and only linked bindings
    then the address should be able to be removed from the broker

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

    $ git pull https://github.com/cshannon/activemq-artemis ARTEMIS-1627

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

    https://github.com/apache/activemq-artemis/pull/1807.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 #1807
    
----
commit 95d1092ce448006dea5180aaf16b924ccca78aac
Author: Christopher L. Shannon (cshannon) <ch...@...>
Date:   2018-01-23T12:46:33Z

    ARTEMIS-1627 - Support removing addresses that do not have direct
    bindings
    
    If there are no direct bindings on an address and only linked bindings
    then the address should be able to be removed from the broker

----


---

[GitHub] activemq-artemis pull request #1807: ARTEMIS-1627 - Support removing address...

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

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


---

[GitHub] activemq-artemis issue #1807: ARTEMIS-1627 - Support removing addresses that...

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

    https://github.com/apache/activemq-artemis/pull/1807
  
    Updated the PR with a slight change


---

[GitHub] activemq-artemis pull request #1807: ARTEMIS-1627 - Support removing address...

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/1807#discussion_r163301265
  
    --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java ---
    @@ -532,7 +532,7 @@ public AddressInfo updateAddressInfo(SimpleString addressName,
        public AddressInfo removeAddressInfo(SimpleString address) throws Exception {
           synchronized (addressLock) {
              server.callBrokerPlugins(server.hasBrokerPlugins() ? plugin -> plugin.beforeRemoveAddress(address) : null);
    -         Bindings bindingsForAddress = getBindingsForAddress(address);
    +         final Bindings bindingsForAddress = getDirectBindings(address);
    --- End diff --
    
    Looking at this, it seems it completely will ignore wildcard config. Im not so sure this is a good idea, as it takes into account these.
    
    e.g. what happens if someone is using different wildcards? Also it be good to add test cases where alternative wildcards are used to make sure removeAddress still works.


---

[GitHub] activemq-artemis pull request #1807: ARTEMIS-1627 - Support removing address...

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

    https://github.com/apache/activemq-artemis/pull/1807#discussion_r163305830
  
    --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java ---
    @@ -532,7 +532,7 @@ public AddressInfo updateAddressInfo(SimpleString addressName,
        public AddressInfo removeAddressInfo(SimpleString address) throws Exception {
           synchronized (addressLock) {
              server.callBrokerPlugins(server.hasBrokerPlugins() ? plugin -> plugin.beforeRemoveAddress(address) : null);
    -         Bindings bindingsForAddress = getBindingsForAddress(address);
    +         final Bindings bindingsForAddress = getDirectBindings(address);
    --- End diff --
    
    The getDirectBindings() method just does an equals() call and compares address names to see if the match exactly (ie simple strings).  It's not doing a match so there is no wild card involved.


---

[GitHub] activemq-artemis issue #1807: ARTEMIS-1627 - Support removing addresses that...

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

    https://github.com/apache/activemq-artemis/pull/1807
  
    I added a couple more tests for the WildcardAddressManager to show that getDirectBindings() works with a different wildcard setting


---

[GitHub] activemq-artemis pull request #1807: ARTEMIS-1627 - Support removing address...

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

    https://github.com/apache/activemq-artemis/pull/1807#discussion_r163306080
  
    --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java ---
    @@ -532,7 +532,7 @@ public AddressInfo updateAddressInfo(SimpleString addressName,
        public AddressInfo removeAddressInfo(SimpleString address) throws Exception {
           synchronized (addressLock) {
              server.callBrokerPlugins(server.hasBrokerPlugins() ? plugin -> plugin.beforeRemoveAddress(address) : null);
    -         Bindings bindingsForAddress = getBindingsForAddress(address);
    +         final Bindings bindingsForAddress = getDirectBindings(address);
    --- End diff --
    
    But I can still add some tests for a different wildcard anyways.


---