You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/02/23 19:48:42 UTC
[GitHub] [activemq-artemis] jbertram opened a new pull request #3466: ARTEMIS-3137 support XPath filters
jbertram opened a new pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r584868805
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import java.net.URI;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier AMQPConnection = () -> createConnection();
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "AMQP,OPENWIRE,CORE";
+ }
+
+ @Override
+ protected URI getBrokerQpidJMSConnectionURI() {
+ try {
+ return new URI(getBrokerQpidJMSConnectionString() + "?jms.validateSelector=false");
+ } catch (Exception e) {
+ throw new RuntimeException();
+ }
+ }
+
+ @Override
+ protected void addConfiguration(ActiveMQServer server) {
+ server.getConfiguration().setPersistenceEnabled(false);
+ server.getAddressSettingsRepository().addMatch(NORMAL_QUEUE_NAME, new AddressSettings());
+ }
+
+ @Override
+ protected void createAddressAndQueues(ActiveMQServer server) throws Exception {
+ super.createAddressAndQueues(server);
+
+ //Add Standard Queue
+ server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(NORMAL_QUEUE_NAME), RoutingType.ANYCAST));
+ server.createQueue(new QueueConfiguration(NORMAL_QUEUE_NAME).setRoutingType(RoutingType.ANYCAST));
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerCoreConsumer() throws Exception {
Review comment:
I was before as they seemed totally duplicate initially but, since there is a difference, at this point I guess not.
I guess I'm mainly just saying I'd have done things a different way in one of the cases, just using a quicker unit test, since the other systest largely encompasses it and seems likely to catch the same issues given how it this functionality works overall. The existing tests are so slow such that even most CI isnt running most of them, so its nice to rationalise systests where possible.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r585896398
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
Review comment:
> ...I dont think the parents need moved at all for the very small thing I am suggesting of putting the class in the package most appropriate for it.
I guess I misunderstood. I thought you wanted the test to move away from the `org.apache.activemq.artemis.tests.integration.amqp` package since it wasn't specifically related to AMQP. Given that, it didn't (and still doesn't) make sense to me for a non-AMQP-specific test to have a parent that is AMQP-specific. That's why I concluded that the generic multi-protocol functionality would have to be refactored out of the AMQP-specific test parent classes.
> Not every parent needs to be in the same package, they often arent.
Agreed. However, to have a generic test with a more specific parent doesn't make a lot of sense to me.
> Everything in the multiprotocol package is already parented by [different] classes outwith that package.
Which package are you referring to specifically here?
> Looking at moving e.g. the existing selector test class now...
Can you clarify which package this test would move to? Is it logically/heirarchically "above" or "lateral" to `org.apache.activemq.artemis.tests.integration.amqp`?
To be frank, this is starting to feel a bit like bike shedding to me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r582717325
##########
File path: artemis-selector/src/main/java/org/apache/activemq/artemis/selector/filter/XPathExpression.java
##########
@@ -79,4 +97,15 @@ public boolean matches(Filterable message) throws FilterException {
return object == Boolean.TRUE;
}
+ protected static void setupFeatures(DocumentBuilderFactory factory) throws ParserConfigurationException {
+ Properties properties = System.getProperties();
+ for (Map.Entry<Object, Object> prop : properties.entrySet()) {
+ String key = (String) prop.getKey();
+ if (key.startsWith(DOCUMENT_BUILDER_FACTORY_FEATURE)) {
+ String uri = key.split(DOCUMENT_BUILDER_FACTORY_FEATURE + ":")[1];
Review comment:
Shouldnt it check if key.startsWith(DOCUMENT_BUILDER_FACTORY_FEATURE + ":") since thats the documented prefix and what it then splits on? Also, it would seem simpler and nicer to just use key.substring(<prefix length>) rather than split.
##########
File path: artemis-selector/src/main/java/org/apache/activemq/artemis/selector/filter/JAXPXPathEvaluator.java
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.selector.filter;
+
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.xpath.XPath;
+import javax.xml.xpath.XPathConstants;
+import javax.xml.xpath.XPathFactory;
+import java.io.StringReader;
+
+import org.xml.sax.InputSource;
+
+public class JAXPXPathEvaluator implements XPathExpression.XPathEvaluator {
+
+ private static final XPathFactory FACTORY = XPathFactory.newInstance();
Review comment:
I notice the javadoc for XPathFactory explicitly notes it isnt thread-safe and recommends synchronization to protect uses of the factory. It seems like instances of this JAXPXPathEvaluator class might get created by multiple threads, and if so in turn the factory could be used concurrently.
I dont know what the actual impact would be with just uses of .newXPath(), but perhaps worth looking into.
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import java.net.URI;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier AMQPConnection = () -> createConnection();
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "AMQP,OPENWIRE,CORE";
+ }
+
+ @Override
+ protected URI getBrokerQpidJMSConnectionURI() {
+ try {
+ return new URI(getBrokerQpidJMSConnectionString() + "?jms.validateSelector=false");
+ } catch (Exception e) {
+ throw new RuntimeException();
+ }
+ }
+
+ @Override
+ protected void addConfiguration(ActiveMQServer server) {
+ server.getConfiguration().setPersistenceEnabled(false);
+ server.getAddressSettingsRepository().addMatch(NORMAL_QUEUE_NAME, new AddressSettings());
+ }
+
+ @Override
+ protected void createAddressAndQueues(ActiveMQServer server) throws Exception {
+ super.createAddressAndQueues(server);
+
+ //Add Standard Queue
+ server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(NORMAL_QUEUE_NAME), RoutingType.ANYCAST));
+ server.createQueue(new QueueConfiguration(NORMAL_QUEUE_NAME).setRoutingType(RoutingType.ANYCAST));
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerCoreConsumer() throws Exception {
Review comment:
Isnt this essentially a duplicate, with the test in the ConsumerTest class?
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import java.net.URI;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier AMQPConnection = () -> createConnection();
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "AMQP,OPENWIRE,CORE";
+ }
+
+ @Override
+ protected URI getBrokerQpidJMSConnectionURI() {
+ try {
+ return new URI(getBrokerQpidJMSConnectionString() + "?jms.validateSelector=false");
+ } catch (Exception e) {
+ throw new RuntimeException();
+ }
+ }
+
+ @Override
+ protected void addConfiguration(ActiveMQServer server) {
+ server.getConfiguration().setPersistenceEnabled(false);
+ server.getAddressSettingsRepository().addMatch(NORMAL_QUEUE_NAME, new AddressSettings());
+ }
+
+ @Override
+ protected void createAddressAndQueues(ActiveMQServer server) throws Exception {
+ super.createAddressAndQueues(server);
+
+ //Add Standard Queue
+ server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(NORMAL_QUEUE_NAME), RoutingType.ANYCAST));
+ server.createQueue(new QueueConfiguration(NORMAL_QUEUE_NAME).setRoutingType(RoutingType.ANYCAST));
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerCoreConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerCoreConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerCoreConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, AMQPConnection);
+ }
+
+ public void testJMSSelectors(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier) throws Exception {
+ testJMSSelector(producerConnectionSupplier, consumerConnectionSupplier, NORMAL_QUEUE_NAME, "<root><a key='first' num='1'/><b key='second' num='2'>b</b></root>", "XPATH 'root/a'");
+ }
+
+ public void testJMSSelector(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector) throws Exception {
+ testJMSSelector(producerConnectionSupplier, consumerConnectionSupplier, queueName, body, selector, Message.DEFAULT_DELIVERY_MODE, Message.DEFAULT_PRIORITY, Message.DEFAULT_TIME_TO_LIVE);
+ }
+
+ public void testJMSSelector(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector, int deliveryMode, int priority, long timeToLive) throws Exception {
+ sendMessage(producerConnectionSupplier, queueName, body, deliveryMode, priority, timeToLive);
+ receive(consumerConnectionSupplier, queueName, body, selector);
+ }
+
+ private void receive(ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector) throws JMSException {
+ try (Connection consumerConnection = consumerConnectionSupplier.createConnection()) {
+ Session consumerSession = consumerConnection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+ Queue consumerQueue = consumerSession.createQueue(queueName);
+ MessageConsumer consumer = consumerSession.createConsumer(consumerQueue, selector);
+ TextMessage msg = (TextMessage) consumer.receive(1000);
+ assertNotNull(msg);
+ assertEquals(body, msg.getText());
+ assertNull(consumer.receiveNoWait());
+ consumer.close();
+ }
+ }
+
+ private void sendMessage(ConnectionSupplier producerConnectionSupplier, String queueName, String body, int deliveryMode, int priority, long timeToLive) throws JMSException {
+ try (Connection producerConnection = producerConnectionSupplier.createConnection()) {
+ Session producerSession = producerConnection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+ Queue queue1 = producerSession.createQueue(queueName);
+ MessageProducer p = producerSession.createProducer(null);
+
+ TextMessage message1 = producerSession.createTextMessage();
+ message1.setText("hello");
Review comment:
Might it be a fuller test to also send a 'complex' message that doesnt entirely match the filter, but at least begins to...e.g the filter matches on element root/a, and the tests send an a message with root/a and root/b...send another with e.g just root/z?
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
Review comment:
The crossprotocol package might be better than the amqp one, given what all is being tested? Alternatively, even a new specific package might be best since its more of an internal feature-specific test rather than to do with any given protocol.
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import java.net.URI;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier AMQPConnection = () -> createConnection();
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "AMQP,OPENWIRE,CORE";
+ }
+
+ @Override
+ protected URI getBrokerQpidJMSConnectionURI() {
+ try {
+ return new URI(getBrokerQpidJMSConnectionString() + "?jms.validateSelector=false");
+ } catch (Exception e) {
+ throw new RuntimeException();
+ }
+ }
+
+ @Override
+ protected void addConfiguration(ActiveMQServer server) {
+ server.getConfiguration().setPersistenceEnabled(false);
+ server.getAddressSettingsRepository().addMatch(NORMAL_QUEUE_NAME, new AddressSettings());
+ }
+
+ @Override
+ protected void createAddressAndQueues(ActiveMQServer server) throws Exception {
+ super.createAddressAndQueues(server);
+
+ //Add Standard Queue
+ server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(NORMAL_QUEUE_NAME), RoutingType.ANYCAST));
+ server.createQueue(new QueueConfiguration(NORMAL_QUEUE_NAME).setRoutingType(RoutingType.ANYCAST));
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerCoreConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerCoreConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerCoreConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, AMQPConnection);
+ }
+
+ public void testJMSSelectors(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier) throws Exception {
+ testJMSSelector(producerConnectionSupplier, consumerConnectionSupplier, NORMAL_QUEUE_NAME, "<root><a key='first' num='1'/><b key='second' num='2'>b</b></root>", "XPATH 'root/a'");
+ }
+
+ public void testJMSSelector(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector) throws Exception {
+ testJMSSelector(producerConnectionSupplier, consumerConnectionSupplier, queueName, body, selector, Message.DEFAULT_DELIVERY_MODE, Message.DEFAULT_PRIORITY, Message.DEFAULT_TIME_TO_LIVE);
+ }
+
+ public void testJMSSelector(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector, int deliveryMode, int priority, long timeToLive) throws Exception {
+ sendMessage(producerConnectionSupplier, queueName, body, deliveryMode, priority, timeToLive);
+ receive(consumerConnectionSupplier, queueName, body, selector);
+ }
+
+ private void receive(ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector) throws JMSException {
+ try (Connection consumerConnection = consumerConnectionSupplier.createConnection()) {
+ Session consumerSession = consumerConnection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+ Queue consumerQueue = consumerSession.createQueue(queueName);
+ MessageConsumer consumer = consumerSession.createConsumer(consumerQueue, selector);
+ TextMessage msg = (TextMessage) consumer.receive(1000);
+ assertNotNull(msg);
+ assertEquals(body, msg.getText());
+ assertNull(consumer.receiveNoWait());
+ consumer.close();
+ }
+ }
+
+ private void sendMessage(ConnectionSupplier producerConnectionSupplier, String queueName, String body, int deliveryMode, int priority, long timeToLive) throws JMSException {
+ try (Connection producerConnection = producerConnectionSupplier.createConnection()) {
+ Session producerSession = producerConnection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+ Queue queue1 = producerSession.createQueue(queueName);
+ MessageProducer p = producerSession.createProducer(null);
+
+ TextMessage message1 = producerSession.createTextMessage();
+ message1.setText("hello");
+ p.send(queue1, message1);
+
+ TextMessage message2 = producerSession.createTextMessage();
+ if (body != null) {
Review comment:
Is this needed? Setting null text is allowed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r584844260
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import java.net.URI;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier AMQPConnection = () -> createConnection();
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "AMQP,OPENWIRE,CORE";
+ }
+
+ @Override
+ protected URI getBrokerQpidJMSConnectionURI() {
+ try {
+ return new URI(getBrokerQpidJMSConnectionString() + "?jms.validateSelector=false");
+ } catch (Exception e) {
+ throw new RuntimeException();
+ }
+ }
+
+ @Override
+ protected void addConfiguration(ActiveMQServer server) {
+ server.getConfiguration().setPersistenceEnabled(false);
+ server.getAddressSettingsRepository().addMatch(NORMAL_QUEUE_NAME, new AddressSettings());
+ }
+
+ @Override
+ protected void createAddressAndQueues(ActiveMQServer server) throws Exception {
+ super.createAddressAndQueues(server);
+
+ //Add Standard Queue
+ server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(NORMAL_QUEUE_NAME), RoutingType.ANYCAST));
+ server.createQueue(new QueueConfiguration(NORMAL_QUEUE_NAME).setRoutingType(RoutingType.ANYCAST));
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerCoreConsumer() throws Exception {
Review comment:
It's not clear to me...are you asking for a change here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r581389025
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "OPENWIRE,CORE";
Review comment:
The body for text messages sent by STOMP, OpenWire, and core JMS are all nullable `SimpleString` so it was super easy to pull that out in the filter implementation. I just didn't have the desire to fish around in the AMQP code to figure out how to do it although nothing is stopping anybody from implementing this in the future.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r583365677
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
##########
@@ -1271,4 +1271,22 @@ public Object getOwner() {
public void setOwner(Object object) {
this.owner = object;
}
+
+ @Override
+ public String getStringBody() {
+ String body = null;
+
+ if (type == TEXT_TYPE) {
+ try {
+ SimpleString simpleBody = getBodyBuffer().readNullableSimpleString();
Review comment:
Large message are not supported. This is already noted in the documentation.
I'll update the buffer code with your suggestion.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r585581258
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
Review comment:
All the aforementioned parent classes are in `org.apache.activemq.artemis.tests.integration.amqp` together so moving `org.apache.activemq.artemis.tests.integration.amqp.JMSXPathSelectorTest` out of `org.apache.activemq.artemis.tests.integration.amqp` and into something more generic like `org.apache.activemq.artemis.tests.integration.multiprotocol` would require refactoring those parent classes. Again, I think refactoring these is a good idea ultimately, just not in this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#issuecomment-785364961
I made some significant changes to the PR. Take a look and let me know what you think.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r582789225
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
##########
@@ -1271,4 +1271,22 @@ public Object getOwner() {
public void setOwner(Object object) {
this.owner = object;
}
+
+ @Override
+ public String getStringBody() {
+ String body = null;
+
+ if (type == TEXT_TYPE) {
+ try {
+ SimpleString simpleBody = getBodyBuffer().readNullableSimpleString();
Review comment:
You should return the position of the BodyBuffer after read. or duplicate the bodyBuffer here.
```
getBodyBuffer().duplicate().readNullableSimpleString();
```
otherwise the conversion will be broken if you send A Core Message, treat it with Path and receive with AMQP.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r585896398
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
Review comment:
> ...I dont think the parents need moved at all for the very small thing I am suggesting of putting the class in the package most appropriate for it.
I guess I misunderstood. I thought you wanted the test to move away from the `org.apache.activemq.artemis.tests.integration.amqp` package since it wasn't specifically related to AMQP. Given that, it didn't (and still doesn't) make sense to me for a non-AMQP-specific test to have a parent that is AMQP-specific. That's why I concluded that the generic multi-protocol functionality would have to be refactored out of the AMQP-specific test parent classes.
> Not every parent needs to be in the same package, they often arent.
Agreed. However, to have a generic test with a more specific parent doesn't make a lot of sense to me.
> Everything in the multiprotocol package is already parented by [different] classes outwith that package.
Which package are you referring to specifically here?
> Looking at moving e.g. the existing selector test class now...
Can you clarify which package this test would move to? Is it logically/heirarchically "above" `org.apache.activemq.artemis.tests.integration.amqp`?
To be frank, this is starting to feel a bit like bike shedding to me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r582947088
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
##########
@@ -1271,4 +1271,22 @@ public Object getOwner() {
public void setOwner(Object object) {
this.owner = object;
}
+
+ @Override
+ public String getStringBody() {
+ String body = null;
+
+ if (type == TEXT_TYPE) {
+ try {
+ SimpleString simpleBody = getBodyBuffer().readNullableSimpleString();
Review comment:
Or even better.. use getReadOnlyBodyBuffer() instead of getbodyBuffer() here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] asfgit closed pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r584863705
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import java.net.URI;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier AMQPConnection = () -> createConnection();
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "AMQP,OPENWIRE,CORE";
+ }
+
+ @Override
+ protected URI getBrokerQpidJMSConnectionURI() {
+ try {
+ return new URI(getBrokerQpidJMSConnectionString() + "?jms.validateSelector=false");
+ } catch (Exception e) {
+ throw new RuntimeException();
+ }
+ }
+
+ @Override
+ protected void addConfiguration(ActiveMQServer server) {
+ server.getConfiguration().setPersistenceEnabled(false);
+ server.getAddressSettingsRepository().addMatch(NORMAL_QUEUE_NAME, new AddressSettings());
+ }
+
+ @Override
+ protected void createAddressAndQueues(ActiveMQServer server) throws Exception {
+ super.createAddressAndQueues(server);
+
+ //Add Standard Queue
+ server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(NORMAL_QUEUE_NAME), RoutingType.ANYCAST));
+ server.createQueue(new QueueConfiguration(NORMAL_QUEUE_NAME).setRoutingType(RoutingType.ANYCAST));
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerCoreConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerCoreConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerCoreConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, AMQPConnection);
+ }
+
+ public void testJMSSelectors(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier) throws Exception {
+ testJMSSelector(producerConnectionSupplier, consumerConnectionSupplier, NORMAL_QUEUE_NAME, "<root><a key='first' num='1'/><b key='second' num='2'>b</b></root>", "XPATH 'root/a'");
+ }
+
+ public void testJMSSelector(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector) throws Exception {
+ testJMSSelector(producerConnectionSupplier, consumerConnectionSupplier, queueName, body, selector, Message.DEFAULT_DELIVERY_MODE, Message.DEFAULT_PRIORITY, Message.DEFAULT_TIME_TO_LIVE);
+ }
+
+ public void testJMSSelector(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector, int deliveryMode, int priority, long timeToLive) throws Exception {
+ sendMessage(producerConnectionSupplier, queueName, body, deliveryMode, priority, timeToLive);
+ receive(consumerConnectionSupplier, queueName, body, selector);
+ }
+
+ private void receive(ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector) throws JMSException {
+ try (Connection consumerConnection = consumerConnectionSupplier.createConnection()) {
+ Session consumerSession = consumerConnection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+ Queue consumerQueue = consumerSession.createQueue(queueName);
+ MessageConsumer consumer = consumerSession.createConsumer(consumerQueue, selector);
+ TextMessage msg = (TextMessage) consumer.receive(1000);
+ assertNotNull(msg);
+ assertEquals(body, msg.getText());
+ assertNull(consumer.receiveNoWait());
+ consumer.close();
+ }
+ }
+
+ private void sendMessage(ConnectionSupplier producerConnectionSupplier, String queueName, String body, int deliveryMode, int priority, long timeToLive) throws JMSException {
+ try (Connection producerConnection = producerConnectionSupplier.createConnection()) {
+ Session producerSession = producerConnection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+ Queue queue1 = producerSession.createQueue(queueName);
+ MessageProducer p = producerSession.createProducer(null);
+
+ TextMessage message1 = producerSession.createTextMessage();
+ message1.setText("hello");
Review comment:
Done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r582947088
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
##########
@@ -1271,4 +1271,22 @@ public Object getOwner() {
public void setOwner(Object object) {
this.owner = object;
}
+
+ @Override
+ public String getStringBody() {
+ String body = null;
+
+ if (type == TEXT_TYPE) {
+ try {
+ SimpleString simpleBody = getBodyBuffer().readNullableSimpleString();
Review comment:
Just found the method you have to use...
use getDataBuffer() instead...
This method will return a getReadOnlyBodyBuffer() for StandardMessages
or it will read the whole LargeMessage into Memory before parsing it.
Which is actually a decision we have to make.. if LargeMessages should be part of the filter here.
if you do, we should at least keep a documentation with a warning about large messages and the feature.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r581364583
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "OPENWIRE,CORE";
Review comment:
Why not amqp too? I dont get why its excluded here
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r584862620
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
Review comment:
This test extends `org.apache.activemq.artemis.tests.integration.amqp.JMSClientTestSupport` which in turn extends `org.apache.activemq.artemis.tests.integration.amqp.AmqpClientTestSupport` which in turn extends `org.apache.activemq.artemis.tests.integration.amqp.AmqpTestSupport`. Therefore either all these would need to be visible to wherever I move this test or they would need to be refactored and their multi-protocol functionality extracted into something more generic. I'd like to avoid pulling that thread in this PR, although I think it would be a great idea to address multi-protocol JMS tests generally in another PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r583368875
##########
File path: artemis-selector/src/main/java/org/apache/activemq/artemis/selector/filter/JAXPXPathEvaluator.java
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.selector.filter;
+
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.xpath.XPath;
+import javax.xml.xpath.XPathConstants;
+import javax.xml.xpath.XPathFactory;
+import java.io.StringReader;
+
+import org.xml.sax.InputSource;
+
+public class JAXPXPathEvaluator implements XPathExpression.XPathEvaluator {
+
+ private static final XPathFactory FACTORY = XPathFactory.newInstance();
Review comment:
Thanks for the heads-up. I'll add protection.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] tabish121 commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
tabish121 commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r581399069
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "OPENWIRE,CORE";
Review comment:
Of the top of my head....
if (message instanceof AMQPMessage) {
final Section body = ((AMQPMessage) message).getBody();
if (body instanceof AmqpValue && ((AmqpValue) body).getValue() instanceof String) {
return (String) ((AmqpValue) body).getValue();
}
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r582443031
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
##########
@@ -1271,4 +1271,14 @@ public Object getOwner() {
public void setOwner(Object object) {
this.owner = object;
}
+
+ @Override
+ public String getStringBody() {
Review comment:
you would get wrong decoding, and that could give you some head aches here. Please check the type.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r585597559
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
Review comment:
I disagree it would need any significant refactoring of the parents to move the test class, as I dont think the parents need moved at all for the very small thing I am suggesting of putting the class in the package most appropriate for it. Not every parent needs to be in the same package, they often arent.
EVerything in the multiprotocol package is already parented by [differnt] classes outwith that package. I dont think theres a requirement they all share the same parent, but that could certainly be done if there was a desire and sufficient benefit. I would agree that would come later as its a very distinct task, separate from my comments.
Looking at moving e.g. the existing selector test class now, it looks like it might only need either adding the word public to the ConnectionSupplier interface definition, or maybe creating its own file...or even just removing it since we could probably use java.util.function.Supplier. Presumably the classes already in other packages dont use that interface.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r583364050
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import java.net.URI;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier AMQPConnection = () -> createConnection();
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "AMQP,OPENWIRE,CORE";
+ }
+
+ @Override
+ protected URI getBrokerQpidJMSConnectionURI() {
+ try {
+ return new URI(getBrokerQpidJMSConnectionString() + "?jms.validateSelector=false");
+ } catch (Exception e) {
+ throw new RuntimeException();
+ }
+ }
+
+ @Override
+ protected void addConfiguration(ActiveMQServer server) {
+ server.getConfiguration().setPersistenceEnabled(false);
+ server.getAddressSettingsRepository().addMatch(NORMAL_QUEUE_NAME, new AddressSettings());
+ }
+
+ @Override
+ protected void createAddressAndQueues(ActiveMQServer server) throws Exception {
+ super.createAddressAndQueues(server);
+
+ //Add Standard Queue
+ server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(NORMAL_QUEUE_NAME), RoutingType.ANYCAST));
+ server.createQueue(new QueueConfiguration(NORMAL_QUEUE_NAME).setRoutingType(RoutingType.ANYCAST));
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerCoreConsumer() throws Exception {
Review comment:
They are similar, but not equivalent in my opinion. `org.apache.activemq.artemis.tests.integration.client.ConsumerTest#testConsumerXpathSelector` is using the core API. This test is using the JMS API with the core client.
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import java.net.URI;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier AMQPConnection = () -> createConnection();
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "AMQP,OPENWIRE,CORE";
+ }
+
+ @Override
+ protected URI getBrokerQpidJMSConnectionURI() {
+ try {
+ return new URI(getBrokerQpidJMSConnectionString() + "?jms.validateSelector=false");
+ } catch (Exception e) {
+ throw new RuntimeException();
+ }
+ }
+
+ @Override
+ protected void addConfiguration(ActiveMQServer server) {
+ server.getConfiguration().setPersistenceEnabled(false);
+ server.getAddressSettingsRepository().addMatch(NORMAL_QUEUE_NAME, new AddressSettings());
+ }
+
+ @Override
+ protected void createAddressAndQueues(ActiveMQServer server) throws Exception {
+ super.createAddressAndQueues(server);
+
+ //Add Standard Queue
+ server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(NORMAL_QUEUE_NAME), RoutingType.ANYCAST));
+ server.createQueue(new QueueConfiguration(NORMAL_QUEUE_NAME).setRoutingType(RoutingType.ANYCAST));
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerCoreConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerCoreConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerCoreConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, AMQPConnection);
+ }
+
+ public void testJMSSelectors(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier) throws Exception {
+ testJMSSelector(producerConnectionSupplier, consumerConnectionSupplier, NORMAL_QUEUE_NAME, "<root><a key='first' num='1'/><b key='second' num='2'>b</b></root>", "XPATH 'root/a'");
+ }
+
+ public void testJMSSelector(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector) throws Exception {
+ testJMSSelector(producerConnectionSupplier, consumerConnectionSupplier, queueName, body, selector, Message.DEFAULT_DELIVERY_MODE, Message.DEFAULT_PRIORITY, Message.DEFAULT_TIME_TO_LIVE);
+ }
+
+ public void testJMSSelector(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector, int deliveryMode, int priority, long timeToLive) throws Exception {
+ sendMessage(producerConnectionSupplier, queueName, body, deliveryMode, priority, timeToLive);
+ receive(consumerConnectionSupplier, queueName, body, selector);
+ }
+
+ private void receive(ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector) throws JMSException {
+ try (Connection consumerConnection = consumerConnectionSupplier.createConnection()) {
+ Session consumerSession = consumerConnection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+ Queue consumerQueue = consumerSession.createQueue(queueName);
+ MessageConsumer consumer = consumerSession.createConsumer(consumerQueue, selector);
+ TextMessage msg = (TextMessage) consumer.receive(1000);
+ assertNotNull(msg);
+ assertEquals(body, msg.getText());
+ assertNull(consumer.receiveNoWait());
+ consumer.close();
+ }
+ }
+
+ private void sendMessage(ConnectionSupplier producerConnectionSupplier, String queueName, String body, int deliveryMode, int priority, long timeToLive) throws JMSException {
+ try (Connection producerConnection = producerConnectionSupplier.createConnection()) {
+ Session producerSession = producerConnection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+ Queue queue1 = producerSession.createQueue(queueName);
+ MessageProducer p = producerSession.createProducer(null);
+
+ TextMessage message1 = producerSession.createTextMessage();
+ message1.setText("hello");
+ p.send(queue1, message1);
+
+ TextMessage message2 = producerSession.createTextMessage();
+ if (body != null) {
Review comment:
No, that's not needed. It was a hold-over from `org.apache.activemq.artemis.tests.integration.amqp.JMSSelectorTest` which I copied this from.
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import java.net.URI;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier AMQPConnection = () -> createConnection();
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "AMQP,OPENWIRE,CORE";
+ }
+
+ @Override
+ protected URI getBrokerQpidJMSConnectionURI() {
+ try {
+ return new URI(getBrokerQpidJMSConnectionString() + "?jms.validateSelector=false");
+ } catch (Exception e) {
+ throw new RuntimeException();
+ }
+ }
+
+ @Override
+ protected void addConfiguration(ActiveMQServer server) {
+ server.getConfiguration().setPersistenceEnabled(false);
+ server.getAddressSettingsRepository().addMatch(NORMAL_QUEUE_NAME, new AddressSettings());
+ }
+
+ @Override
+ protected void createAddressAndQueues(ActiveMQServer server) throws Exception {
+ super.createAddressAndQueues(server);
+
+ //Add Standard Queue
+ server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(NORMAL_QUEUE_NAME), RoutingType.ANYCAST));
+ server.createQueue(new QueueConfiguration(NORMAL_QUEUE_NAME).setRoutingType(RoutingType.ANYCAST));
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerCoreConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerCoreConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerCoreConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, AMQPConnection);
+ }
+
+ public void testJMSSelectors(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier) throws Exception {
+ testJMSSelector(producerConnectionSupplier, consumerConnectionSupplier, NORMAL_QUEUE_NAME, "<root><a key='first' num='1'/><b key='second' num='2'>b</b></root>", "XPATH 'root/a'");
+ }
+
+ public void testJMSSelector(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector) throws Exception {
+ testJMSSelector(producerConnectionSupplier, consumerConnectionSupplier, queueName, body, selector, Message.DEFAULT_DELIVERY_MODE, Message.DEFAULT_PRIORITY, Message.DEFAULT_TIME_TO_LIVE);
+ }
+
+ public void testJMSSelector(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector, int deliveryMode, int priority, long timeToLive) throws Exception {
+ sendMessage(producerConnectionSupplier, queueName, body, deliveryMode, priority, timeToLive);
+ receive(consumerConnectionSupplier, queueName, body, selector);
+ }
+
+ private void receive(ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector) throws JMSException {
+ try (Connection consumerConnection = consumerConnectionSupplier.createConnection()) {
+ Session consumerSession = consumerConnection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+ Queue consumerQueue = consumerSession.createQueue(queueName);
+ MessageConsumer consumer = consumerSession.createConsumer(consumerQueue, selector);
+ TextMessage msg = (TextMessage) consumer.receive(1000);
+ assertNotNull(msg);
+ assertEquals(body, msg.getText());
+ assertNull(consumer.receiveNoWait());
+ consumer.close();
+ }
+ }
+
+ private void sendMessage(ConnectionSupplier producerConnectionSupplier, String queueName, String body, int deliveryMode, int priority, long timeToLive) throws JMSException {
+ try (Connection producerConnection = producerConnectionSupplier.createConnection()) {
+ Session producerSession = producerConnection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+ Queue queue1 = producerSession.createQueue(queueName);
+ MessageProducer p = producerSession.createProducer(null);
+
+ TextMessage message1 = producerSession.createTextMessage();
+ message1.setText("hello");
Review comment:
Fair enough.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r584866965
##########
File path: artemis-selector/src/main/java/org/apache/activemq/artemis/selector/filter/XPathExpression.java
##########
@@ -79,4 +97,15 @@ public boolean matches(Filterable message) throws FilterException {
return object == Boolean.TRUE;
}
+ protected static void setupFeatures(DocumentBuilderFactory factory) throws ParserConfigurationException {
+ Properties properties = System.getProperties();
+ for (Map.Entry<Object, Object> prop : properties.entrySet()) {
+ String key = (String) prop.getKey();
+ if (key.startsWith(DOCUMENT_BUILDER_FACTORY_FEATURE)) {
+ String uri = key.split(DOCUMENT_BUILDER_FACTORY_FEATURE + ":")[1];
Review comment:
I agree, and that's why I fixed it. I fixed some other stuff that was part of copy/paste, but I didn't think this was a problem at the time. I was just clarifying where it came from originally. Thanks for pointing it out.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] tabish121 commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
tabish121 commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r582071067
##########
File path: artemis-distribution/src/main/assembly/dep.xml
##########
@@ -110,6 +110,8 @@
<include>com.sun.xml.bind:jaxb-core</include>
<include>javax.activation:activation</include>
<include>org.apache.geronimo.specs:geronimo-jaspic_1.0_spec</include>
+ <include>xalan:xalan</include>
+ <include>xml-apis:xml-apis</include>
Review comment:
Checking 5.x and it has always been optional there as this is a very seldom used feature so would probably be a good idea not to include them always as that project doesn't seem to be doing regular maintenance releases these days.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r584657811
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
Review comment:
I think it would make more sense to put this new test where it actually best fits now, rather than some worse place simply because something else may have done it already. A followup improvement would be fine I guess if it happened, though seems like that could just improve the existing stuff rather than us needing to add yet more stuff it has to improve.
This basically seems like the choice between making things incrementally better now as we do things, or making it incrementally worse 'because someone else might already have' and adding to future cleanup, the type of thing that rarely ever tends to happen. That doesn't seem like much of a choice at all to me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r583365292
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
Review comment:
That's a fair point, and I thought about that while I was writing the test. However, it's worth noting that there are lots of tests in this package that test more than just AMQP (e.g. `org.apache.activemq.artemis.tests.integration.amqp.JMSSelectorTest` which was the basis for this test). I'm not saying that's a good idea necessarily, but it is consistent at this point and I think if we wanted to refactor this we should do it all at one time (i.e. not part of this PR).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r584661640
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import java.net.URI;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier AMQPConnection = () -> createConnection();
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "AMQP,OPENWIRE,CORE";
+ }
+
+ @Override
+ protected URI getBrokerQpidJMSConnectionURI() {
+ try {
+ return new URI(getBrokerQpidJMSConnectionString() + "?jms.validateSelector=false");
+ } catch (Exception e) {
+ throw new RuntimeException();
+ }
+ }
+
+ @Override
+ protected void addConfiguration(ActiveMQServer server) {
+ server.getConfiguration().setPersistenceEnabled(false);
+ server.getAddressSettingsRepository().addMatch(NORMAL_QUEUE_NAME, new AddressSettings());
+ }
+
+ @Override
+ protected void createAddressAndQueues(ActiveMQServer server) throws Exception {
+ super.createAddressAndQueues(server);
+
+ //Add Standard Queue
+ server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(NORMAL_QUEUE_NAME), RoutingType.ANYCAST));
+ server.createQueue(new QueueConfiguration(NORMAL_QUEUE_NAME).setRoutingType(RoutingType.ANYCAST));
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerCoreConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerCoreConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(CoreConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerCoreConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, CoreConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerOpenWireConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, OpenWireConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsOpenWireProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(OpenWireConnection, AMQPConnection);
+ }
+
+ public void testJMSSelectors(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier) throws Exception {
+ testJMSSelector(producerConnectionSupplier, consumerConnectionSupplier, NORMAL_QUEUE_NAME, "<root><a key='first' num='1'/><b key='second' num='2'>b</b></root>", "XPATH 'root/a'");
+ }
+
+ public void testJMSSelector(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector) throws Exception {
+ testJMSSelector(producerConnectionSupplier, consumerConnectionSupplier, queueName, body, selector, Message.DEFAULT_DELIVERY_MODE, Message.DEFAULT_PRIORITY, Message.DEFAULT_TIME_TO_LIVE);
+ }
+
+ public void testJMSSelector(ConnectionSupplier producerConnectionSupplier, ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector, int deliveryMode, int priority, long timeToLive) throws Exception {
+ sendMessage(producerConnectionSupplier, queueName, body, deliveryMode, priority, timeToLive);
+ receive(consumerConnectionSupplier, queueName, body, selector);
+ }
+
+ private void receive(ConnectionSupplier consumerConnectionSupplier, String queueName, String body, String selector) throws JMSException {
+ try (Connection consumerConnection = consumerConnectionSupplier.createConnection()) {
+ Session consumerSession = consumerConnection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+ Queue consumerQueue = consumerSession.createQueue(queueName);
+ MessageConsumer consumer = consumerSession.createConsumer(consumerQueue, selector);
+ TextMessage msg = (TextMessage) consumer.receive(1000);
+ assertNotNull(msg);
+ assertEquals(body, msg.getText());
+ assertNull(consumer.receiveNoWait());
+ consumer.close();
+ }
+ }
+
+ private void sendMessage(ConnectionSupplier producerConnectionSupplier, String queueName, String body, int deliveryMode, int priority, long timeToLive) throws JMSException {
+ try (Connection producerConnection = producerConnectionSupplier.createConnection()) {
+ Session producerSession = producerConnection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+ Queue queue1 = producerSession.createQueue(queueName);
+ MessageProducer p = producerSession.createProducer(null);
+
+ TextMessage message1 = producerSession.createTextMessage();
+ message1.setText("hello");
Review comment:
I actually meant sending it in addition to the clearly-not-xml body rather than instead of it, i.e one plain text, one partial-but-not-fully matching xml, and one matching xml body. Kinf of like covering the edge and middle cases.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r583366430
##########
File path: artemis-selector/src/main/java/org/apache/activemq/artemis/selector/filter/XPathExpression.java
##########
@@ -79,4 +97,15 @@ public boolean matches(Filterable message) throws FilterException {
return object == Boolean.TRUE;
}
+ protected static void setupFeatures(DocumentBuilderFactory factory) throws ParserConfigurationException {
+ Properties properties = System.getProperties();
+ for (Map.Entry<Object, Object> prop : properties.entrySet()) {
+ String key = (String) prop.getKey();
+ if (key.startsWith(DOCUMENT_BUILDER_FACTORY_FEATURE)) {
+ String uri = key.split(DOCUMENT_BUILDER_FACTORY_FEATURE + ":")[1];
Review comment:
Fair enough. FWIW, I took this code from the 5.x code-base where it's been working happily since 2014.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r581811554
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "OPENWIRE,CORE";
Review comment:
One could probably use the "jms.validateSelector" URI option [1] (or related setter on the connection factory) to disable the clients selector validation and let the illegal-for-JMS selector string value through unchecked.
[1] http://qpid.apache.org/releases/qpid-jms-0.56.0/docs/index.html#jms-configuration-options
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r581576964
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "OPENWIRE,CORE";
Review comment:
@tabish121, thanks for the tip.
@michaelandrepearce, I updated the PR with support for AMQP with the caveat that the Qpid JMS client chokes on the `XPATH` selector syntax.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r582456099
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
##########
@@ -1271,4 +1271,14 @@ public Object getOwner() {
public void setOwner(Object object) {
this.owner = object;
}
+
+ @Override
+ public String getStringBody() {
Review comment:
Thanks for the review, @clebertsuconic. I've made the change you suggested.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r582436640
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
##########
@@ -1271,4 +1271,14 @@ public Object getOwner() {
public void setOwner(Object object) {
this.owner = object;
}
+
+ @Override
+ public String getStringBody() {
Review comment:
I think you check CoreMessage.type before you do this
That is... if (type != Message.TEXT_TYPE) return null;
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r585415245
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
Review comment:
I believe other test classes are using them from different packages already, which should mean they are visible.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r583365292
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
Review comment:
That's a fair point, and I thought about that while I was writing the test. However, it's worth noting that there are lots of tests in this package that test more than just AMQP (e.g. `org.apache.activemq.artemis.tests.integration.amqp.JMSSelectorTest` which was the basis for this test). I'm not saying that's a good idea necessarily, but it is consistent at this point and I think if we wanted to refactor this we should do it all at one time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r584660164
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import java.net.URI;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier AMQPConnection = () -> createConnection();
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "AMQP,OPENWIRE,CORE";
+ }
+
+ @Override
+ protected URI getBrokerQpidJMSConnectionURI() {
+ try {
+ return new URI(getBrokerQpidJMSConnectionString() + "?jms.validateSelector=false");
+ } catch (Exception e) {
+ throw new RuntimeException();
+ }
+ }
+
+ @Override
+ protected void addConfiguration(ActiveMQServer server) {
+ server.getConfiguration().setPersistenceEnabled(false);
+ server.getAddressSettingsRepository().addMatch(NORMAL_QUEUE_NAME, new AddressSettings());
+ }
+
+ @Override
+ protected void createAddressAndQueues(ActiveMQServer server) throws Exception {
+ super.createAddressAndQueues(server);
+
+ //Add Standard Queue
+ server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(NORMAL_QUEUE_NAME), RoutingType.ANYCAST));
+ server.createQueue(new QueueConfiguration(NORMAL_QUEUE_NAME).setRoutingType(RoutingType.ANYCAST));
+ }
+
+ @Test
+ public void testJMSSelectorsAMQPProducerAMQPConsumer() throws Exception {
+ testJMSSelectors(AMQPConnection, AMQPConnection);
+ }
+
+ @Test
+ public void testJMSSelectorsCoreProducerCoreConsumer() throws Exception {
Review comment:
Ok, so not a 100% duplicate then (I didnt notice it was using the non-JMS API, I thought it was just impl classes) but still largely encompassing (unless perhaps the JMS client is using non-public APIs in its underlying use of the Core client). I'd probably have only kept this systest to verify the overall workings since its essentially all server side. Maybe a faster unit test for the other just to ensure the args got passed though as expected.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r585597559
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
Review comment:
I disagree it would need any significant refactoring of the parents to move the test class, as I dont think the parents need moved at all for the very small thing I am suggesting of putting the class in the package most appropriate for it. Not every parent needs to be in the same package, they often arent.
Everything in the multiprotocol package is already parented by [different] classes outwith that package. I dont think theres a requirement they all share the same parent, but that could certainly be done if there was a desire and sufficient benefit. I would agree that would come later as its a very distinct task, separate from my comments.
Looking at moving e.g. the existing selector test class now, it looks like it might only need either adding the word public to the ConnectionSupplier interface definition, or maybe creating its own file...or even just removing it since we could probably use java.util.function.Supplier. Presumably the classes already in other packages dont use that interface.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r584650747
##########
File path: artemis-selector/src/main/java/org/apache/activemq/artemis/selector/filter/XPathExpression.java
##########
@@ -79,4 +97,15 @@ public boolean matches(Filterable message) throws FilterException {
return object == Boolean.TRUE;
}
+ protected static void setupFeatures(DocumentBuilderFactory factory) throws ParserConfigurationException {
+ Properties properties = System.getProperties();
+ for (Map.Entry<Object, Object> prop : properties.entrySet()) {
+ String key = (String) prop.getKey();
+ if (key.startsWith(DOCUMENT_BUILDER_FACTORY_FEATURE)) {
+ String uri = key.split(DOCUMENT_BUILDER_FACTORY_FEATURE + ":")[1];
Review comment:
Sure, but adding stuff to a fresh codebase is a good time to actually tidy it up and fix things. Rather than introducing it as-is, meaning it likely never happens and folks have to see it and/or possibly hit it later. Essentially, if a trivial bug/improvement is easily spotted in review, and people are being asked to review...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r581819596
##########
File path: docs/user-manual/en/filter-expressions.md
##########
@@ -62,12 +62,39 @@ previous example to be `convert_string_expressions:age > 18`, then it would
match the aforementioned message.
The JMS spec also states that property identifiers (and therefore the
-identifiers which are valid for use in a filter expression) are an,
-"unlimited-length sequence of letters and digits, the first of which must be
-a letter. A letter is any character for which the method
-`Character.isJavaLetter` returns `true`. This includes `_` and `$`. A letter
-or digit is any character for which the method `Character.isJavaLetterOrDigit`
-returns `true`." This constraint means that hyphens (i.e. `-`) cannot be used.
+identifiers which are valid for use in a filter expression) are an:
+
+> unlimited-length sequence of letters and digits, the first of which must be
+> a letter. A letter is any character for which the method
+> `Character.isJavaLetter` returns `true`. This includes `_` and `$`. A letter
+> or digit is any character for which the method `Character.isJavaLetterOrDigit`
+> returns `true`.
+
+This constraint means that hyphens (i.e. `-`) cannot be used.
However, this constraint can be overcome by using the `hyphenated_props:`
prefix. For example, if a message had the `foo-bar` property set to `0` then
-the filter expression `hyphenated_props:foo-bar = 0` would match it.
\ No newline at end of file
+the filter expression `hyphenated_props:foo-bar = 0` would match it.
+
+## XPath
+
+Apache ActiveMQ Artemis also supports special [XPath](https://en.wikipedia.org/wiki/XPath)
+filters which operate on the *body* of a message. The body must be XML. To
+use an XPath filter use this syntax:
+```
+XPATH '<xpath-expression>'
+```
+### Caveats
+
+XPath filters are supported with and between producers and consumers using
+the following protocols:
+
+ - OpenWire JMS
+ - Core (and Core JMS)
+ - STOMP
+ - AMQP with the caveat that the Qpid JMS client throws an exception when
+ trying to parse the `XPATH '<xpath-expression>'` syntax.
Review comment:
Could instead just document how to disable the JMS selector validation to allow using the illegal value, per earlier comment.
##########
File path: artemis-distribution/src/main/assembly/dep.xml
##########
@@ -110,6 +110,8 @@
<include>com.sun.xml.bind:jaxb-core</include>
<include>javax.activation:activation</include>
<include>org.apache.geronimo.specs:geronimo-jaspic_1.0_spec</include>
+ <include>xalan:xalan</include>
+ <include>xml-apis:xml-apis</include>
Review comment:
I'd probably continue to leave these out personally. It doesnt seem unreasonable for folks to have to drop the (very old) dep in for a highly specific feature many likely wont use (and noone could until now?). Its not difficult and could be documented.
I see further down they were listed as optional before but this change removed that, essentially I'd restore that and just update the tests module.
##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSXPathSelectorTest.java
##########
@@ -0,0 +1,156 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import javax.jms.Connection;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.junit.Ignore;
+import org.junit.Test;
+
+public class JMSXPathSelectorTest extends JMSClientTestSupport {
+
+ private static final String NORMAL_QUEUE_NAME = "NORMAL";
+
+ private ConnectionSupplier AMQPConnection = () -> createConnection();
+ private ConnectionSupplier CoreConnection = () -> createCoreConnection();
+ private ConnectionSupplier OpenWireConnection = () -> createOpenWireConnection();
+
+ @Override
+ protected String getConfiguredProtocols() {
+ return "AMQP,OPENWIRE,CORE";
+ }
+
+ @Override
+ protected void addConfiguration(ActiveMQServer server) {
+ server.getConfiguration().setPersistenceEnabled(false);
+ server.getAddressSettingsRepository().addMatch(NORMAL_QUEUE_NAME, new AddressSettings());
+ }
+
+ @Override
+ protected void createAddressAndQueues(ActiveMQServer server) throws Exception {
+ super.createAddressAndQueues(server);
+
+ //Add Standard Queue
+ server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(NORMAL_QUEUE_NAME), RoutingType.ANYCAST));
+ server.createQueue(new QueueConfiguration(NORMAL_QUEUE_NAME).setRoutingType(RoutingType.ANYCAST));
+ }
+
+ @Test
+ @Ignore // Qpid JMS client throws javax.jms.InvalidSelectorException: XPATH 'root/a'
Review comment:
If disabling the validation works, can remove all these ignores. Otherwise, removing the useless test would seem better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r583368875
##########
File path: artemis-selector/src/main/java/org/apache/activemq/artemis/selector/filter/JAXPXPathEvaluator.java
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.selector.filter;
+
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.xpath.XPath;
+import javax.xml.xpath.XPathConstants;
+import javax.xml.xpath.XPathFactory;
+import java.io.StringReader;
+
+import org.xml.sax.InputSource;
+
+public class JAXPXPathEvaluator implements XPathExpression.XPathEvaluator {
+
+ private static final XPathFactory FACTORY = XPathFactory.newInstance();
Review comment:
Thanks for the heads-up.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r582789225
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
##########
@@ -1271,4 +1271,22 @@ public Object getOwner() {
public void setOwner(Object object) {
this.owner = object;
}
+
+ @Override
+ public String getStringBody() {
+ String body = null;
+
+ if (type == TEXT_TYPE) {
+ try {
+ SimpleString simpleBody = getBodyBuffer().readNullableSimpleString();
Review comment:
You should return the position of the BodyBuffer after read. or duplicate the bodyBuffer here.
```
getBodyBuffer().duplicate().readNullableSimpleString();
```
otherwise the conversion will be broken if you send a Core Message, treat it with Path and receive with AMQP.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3466: ARTEMIS-3137 support XPath filters
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3466:
URL: https://github.com/apache/activemq-artemis/pull/3466#discussion_r582951568
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
##########
@@ -1271,4 +1271,22 @@ public Object getOwner() {
public void setOwner(Object object) {
this.owner = object;
}
+
+ @Override
+ public String getStringBody() {
+ String body = null;
+
+ if (type == TEXT_TYPE) {
+ try {
+ SimpleString simpleBody = getBodyBuffer().readNullableSimpleString();
Review comment:
so the code should be:
```java
SimpleString simpleBody = getDataBuffer().readNullableSimpleString();
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org