You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by clebertsuconic <gi...@git.apache.org> on 2018/01/18 00:21:05 UTC
[GitHub] activemq-artemis pull request #1787: ARTEMIS-1577 Address-settings policies ...
GitHub user clebertsuconic opened a pull request:
https://github.com/apache/activemq-artemis/pull/1787
ARTEMIS-1577 Address-settings policies not working with older clients
This closes #1744
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/clebertsuconic/activemq-artemis compatibility
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/1787.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 #1787
----
commit fcf5bfb3feb2708b0e3ddcae157efd5194fbb5f2
Author: Clebert Suconic <cl...@...>
Date: 2018-01-17T20:03:59Z
ARTEMIS-1577 Address-settings policies not working with older clients
This closes #1744
----
---
[GitHub] activemq-artemis pull request #1787: ARTEMIS-1577 Address-settings policies ...
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/1787#discussion_r162261691
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java ---
@@ -1284,7 +1284,7 @@ private boolean checkDuplicateID(final Message message,
boolean isDuplicate = false;
if (duplicateIDBytes != null) {
- cache = getDuplicateIDCache(message.getAddressSimpleString());
+ cache = getDuplicateIDCache(context.getAddress(message));
--- End diff --
Same as other comment, re making one call to context.getAddress(message) and re-using the result
---
[GitHub] activemq-artemis pull request #1787: ARTEMIS-1577 Address-settings policies ...
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/1787#discussion_r162367328
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java ---
@@ -1284,7 +1284,7 @@ private boolean checkDuplicateID(final Message message,
boolean isDuplicate = false;
if (duplicateIDBytes != null) {
- cache = getDuplicateIDCache(message.getAddressSimpleString());
+ cache = getDuplicateIDCache(context.getAddress(message));
--- End diff --
No worries I can do separate pr later
---
[GitHub] activemq-artemis pull request #1787: ARTEMIS-1577 Address-settings policies ...
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/1787#discussion_r162261502
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java ---
@@ -736,11 +736,11 @@ public RoutingStatus route(final Message message,
throw new IllegalStateException("Message cannot be routed more than once");
}
- setPagingStore(message);
+ setPagingStore(context.getAddress(message), message);
AtomicBoolean startedTX = new AtomicBoolean(false);
- final SimpleString address = message.getAddressSimpleString();
+ final SimpleString address = context.getAddress(message);
--- End diff --
Can we not make the call to context.getAddress just the once, and re-use the result throughout the method call?
---
[GitHub] activemq-artemis pull request #1787: ARTEMIS-1577 Address-settings policies ...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1787#discussion_r162353710
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java ---
@@ -736,11 +736,11 @@ public RoutingStatus route(final Message message,
throw new IllegalStateException("Message cannot be routed more than once");
}
- setPagingStore(message);
+ setPagingStore(context.getAddress(message), message);
AtomicBoolean startedTX = new AtomicBoolean(false);
- final SimpleString address = message.getAddressSimpleString();
+ final SimpleString address = context.getAddress(message);
--- End diff --
I'm just going for the simple refactoring here.. every call to message.getAddressSimplString() to be calling context.getAddress()
---
[GitHub] activemq-artemis pull request #1787: ARTEMIS-1577 Address-settings policies ...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1787#discussion_r162358705
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java ---
@@ -1284,7 +1284,7 @@ private boolean checkDuplicateID(final Message message,
boolean isDuplicate = false;
if (duplicateIDBytes != null) {
- cache = getDuplicateIDCache(message.getAddressSimpleString());
+ cache = getDuplicateIDCache(context.getAddress(message));
--- End diff --
@michaelandrepearce caching here goes beyond the scope here.
I would need to change a lot of signatures to pass an address as a parameter.. or cache within each method used during routing.
We can do that.. but it goes beyond the scope of this fix.
---
[GitHub] activemq-artemis pull request #1787: ARTEMIS-1577 Address-settings policies ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1787
---
[GitHub] activemq-artemis pull request #1787: ARTEMIS-1577 Address-settings policies ...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1787#discussion_r162353815
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java ---
@@ -736,11 +736,11 @@ public RoutingStatus route(final Message message,
throw new IllegalStateException("Message cannot be routed more than once");
}
- setPagingStore(message);
+ setPagingStore(context.getAddress(message), message);
AtomicBoolean startedTX = new AtomicBoolean(false);
- final SimpleString address = message.getAddressSimpleString();
+ final SimpleString address = context.getAddress(message);
--- End diff --
Your comment could apply to the previous version as well.. I didn't make any big changes here. nor did want to risk anything else.
---
[GitHub] activemq-artemis issue #1787: ARTEMIS-1577 Address-settings policies not wor...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1787
+1 I’m happy as noted I can pr some bits later
---