You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by jbertram <gi...@git.apache.org> on 2018/08/10 19:35:29 UTC
[GitHub] activemq-artemis pull request #2238: ARTEMIS-2023 Support 1x prefixes for JM...
GitHub user jbertram opened a pull request:
https://github.com/apache/activemq-artemis/pull/2238
ARTEMIS-2023 Support 1x prefixes for JMS dests created using session
In some cases users who migrate from 1.x to 2.x may still want to keep
the legacy prefixes for their JMS destinations (i.e. "jms.queue.",
"jms.topic.", etc.). This commit adds a boolean on our ConnectionFactory
implementation so that it will use the old prefixes when invoking the
queue/topic creation methods on the Session implementation.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jbertram/activemq-artemis ARTEMIS-2023
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/2238.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 #2238
----
commit 2cedb95ccc7078fa3d47ccb3f9b4fd233c86b9f0
Author: Justin Bertram <jb...@...>
Date: 2018-08-10T02:32:23Z
ARTEMIS-2023 Support 1x prefixes for JMS dests created using session
In some cases users who migrate from 1.x to 2.x may still want to keep
the legacy prefixes for their JMS destinations (i.e. "jms.queue.",
"jms.topic.", etc.). This commit adds a boolean on our ConnectionFactory
implementation so that it will use the old prefixes when invoking the
queue/topic creation methods on the Session implementation.
----
---
[GitHub] activemq-artemis pull request #2238: ARTEMIS-2023 Support 1x prefixes for JM...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2238#discussion_r209618198
--- Diff: artemis-jms-server/src/main/java/org/apache/activemq/artemis/jms/server/config/impl/ConnectionFactoryConfigurationImpl.java ---
@@ -623,6 +636,8 @@ public void decode(final ActiveMQBuffer buffer) {
deserializationBlackList = BufferHelper.readNullableSimpleStringAsString(buffer);
deserializationWhiteList = BufferHelper.readNullableSimpleStringAsString(buffer);
+
+ enable1xPrefixes = buffer.readBoolean();
--- End diff --
This is a pretty rare use-case involving JMS connection factories configured and stored on the broker-side. It's something that Wildfly does for their client/server JNDI implementation. It's never used by the broker standalone.
---
[GitHub] activemq-artemis pull request #2238: ARTEMIS-2023 Support 1x prefixes for JM...
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/2238#discussion_r209438598
--- Diff: artemis-jms-server/src/main/java/org/apache/activemq/artemis/jms/server/config/impl/ConnectionFactoryConfigurationImpl.java ---
@@ -623,6 +636,8 @@ public void decode(final ActiveMQBuffer buffer) {
deserializationBlackList = BufferHelper.readNullableSimpleStringAsString(buffer);
deserializationWhiteList = BufferHelper.readNullableSimpleStringAsString(buffer);
+
+ enable1xPrefixes = buffer.readBoolean();
--- End diff --
should check buffer has readable bytes before reading, for compatibility if the buffer is written by a previous version
---
[GitHub] activemq-artemis pull request #2238: ARTEMIS-2023 Support 1x prefixes for JM...
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/2238#discussion_r209466147
--- Diff: artemis-jms-server/src/main/java/org/apache/activemq/artemis/jms/server/config/impl/ConnectionFactoryConfigurationImpl.java ---
@@ -623,6 +636,8 @@ public void decode(final ActiveMQBuffer buffer) {
deserializationBlackList = BufferHelper.readNullableSimpleStringAsString(buffer);
deserializationWhiteList = BufferHelper.readNullableSimpleStringAsString(buffer);
+
+ enable1xPrefixes = buffer.readBoolean();
--- End diff --
@michaelandrepearce that's correct. I'm surprised the compatibility tests didn't catch this.. probably there's something missing.
---
[GitHub] activemq-artemis pull request #2238: ARTEMIS-2023 Support 1x prefixes for JM...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2238#discussion_r209626842
--- Diff: artemis-jms-server/src/main/java/org/apache/activemq/artemis/jms/server/config/impl/ConnectionFactoryConfigurationImpl.java ---
@@ -623,6 +636,8 @@ public void decode(final ActiveMQBuffer buffer) {
deserializationBlackList = BufferHelper.readNullableSimpleStringAsString(buffer);
deserializationWhiteList = BufferHelper.readNullableSimpleStringAsString(buffer);
+
+ enable1xPrefixes = buffer.readBoolean();
--- End diff --
That's correct. I pushed a change to deal with the compatibility issue.
---
[GitHub] activemq-artemis pull request #2238: ARTEMIS-2023 Support 1x prefixes for JM...
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/2238#discussion_r209438668
--- Diff: artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java ---
@@ -885,7 +891,7 @@ public TemporaryQueue createTemporaryQueue() throws JMSException {
}
try {
- ActiveMQTemporaryQueue queue = ActiveMQDestination.createTemporaryQueue(this);
+ ActiveMQTemporaryQueue queue = ActiveMQDestination.createTemporaryQueue(this, (enable1xPrefixes ? PacketImpl.OLD_TEMP_QUEUE_PREFIX.toString() : ""));
--- End diff --
nit: but can avoid a string concatenation that occurs in the new createTemporaryQueue method for users not enabling, if the if was moved around, and the existing method called for that.
final ActiveMQTemporaryQueue queue;
if (enable1xPrefixes) {
queue = ActiveMQDestination.createTemporaryQueue(this, PacketImpl.OLD_TEMP_QUEUE_PREFIX.toString());
} else {
queue = ActiveMQDestination.createTemporaryQueue(this);
}
---
[GitHub] activemq-artemis pull request #2238: ARTEMIS-2023 Support 1x prefixes for JM...
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/2238#discussion_r209819327
--- Diff: artemis-jms-server/src/main/java/org/apache/activemq/artemis/jms/server/config/impl/ConnectionFactoryConfigurationImpl.java ---
@@ -623,6 +636,8 @@ public void decode(final ActiveMQBuffer buffer) {
deserializationBlackList = BufferHelper.readNullableSimpleStringAsString(buffer);
deserializationWhiteList = BufferHelper.readNullableSimpleStringAsString(buffer);
+
+ enable1xPrefixes = buffer.readableBytes() > 0 ? buffer.readBoolean() : null;
--- End diff --
@clebertsuconic @jbertram Should default to 0, null would throw npe here, as field being set is a promiti5
---
[GitHub] activemq-artemis pull request #2238: ARTEMIS-2023 Support 1x prefixes for JM...
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/2238#discussion_r209621351
--- Diff: artemis-jms-server/src/main/java/org/apache/activemq/artemis/jms/server/config/impl/ConnectionFactoryConfigurationImpl.java ---
@@ -623,6 +636,8 @@ public void decode(final ActiveMQBuffer buffer) {
deserializationBlackList = BufferHelper.readNullableSimpleStringAsString(buffer);
deserializationWhiteList = BufferHelper.readNullableSimpleStringAsString(buffer);
+
+ enable1xPrefixes = buffer.readBoolean();
--- End diff --
yes.. but didn't you add a boolean? that would from my quick glance break serialization with older versions.. wouldn't it?
---
[GitHub] activemq-artemis pull request #2238: ARTEMIS-2023 Support 1x prefixes for JM...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/2238
---