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/11 02:19:54 UTC
[GitHub] [activemq-artemis] clebertsuconic opened a new pull request #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
clebertsuconic opened a new pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450
* removing the JMS dependency on AMQP module
* fixing destinations usage.
* refactoring to remove some JMS usage and make exceptions a bit better
Jira: https://issues.apache.org/jira/browse/ARTEMIS-3113
----------------------------------------------------------------
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 pull request #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#issuecomment-777491398
@michaelandrepearce I think this is ready to merge now...
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574450855
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
Review comment:
TTL is handled directly at the converter. I think this method was not being used.
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574864201
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
Review comment:
Totally agree it's not related to this change. E.g. if its a bug was preexisting, just that on reviewing this change in detail it flagged to me this looks like a bug we may not know about, better to discuss it now than someone hit it in prod.
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574311303
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
- // no op
- return 0;
- }
-
- @Override
- public final void setJMSDeliveryTime(long deliveryTime) throws JMSException {
- // no op
- }
- @Override
- public final int getJMSPriority() throws JMSException {
+ public final int getJMSPriority() {
return message.getPriority();
}
- @Override
- public final void setJMSPriority(int priority) throws JMSException {
+ public final void setJMSPriority(int priority) {
message.setPriority((byte) priority);
}
- @Override
- public final void clearProperties() throws JMSException {
+ public final void clearProperties() {
MessageUtil.clearProperties(message);
}
- @Override
- public final boolean propertyExists(String name) throws JMSException {
+ public final boolean propertyExists(String name) {
return MessageUtil.propertyExists(message, name);
}
- @Override
- public final boolean getBooleanProperty(String name) throws JMSException {
+ public final boolean getBooleanProperty(String name) {
return message.getBooleanProperty(name);
}
- @Override
- public final byte getByteProperty(String name) throws JMSException {
+ public final byte getByteProperty(String name) {
return message.getByteProperty(name);
}
- @Override
- public final short getShortProperty(String name) throws JMSException {
+ public final short getShortProperty(String name) {
return message.getShortProperty(name);
}
- @Override
- public final int getIntProperty(String name) throws JMSException {
+ public final int getIntProperty(String name) {
return MessageUtil.getIntProperty(message, name);
}
- @Override
- public final long getLongProperty(String name) throws JMSException {
+ public final long getLongProperty(String name) {
return MessageUtil.getLongProperty(message, name);
}
- @Override
- public final float getFloatProperty(String name) throws JMSException {
+ public final float getFloatProperty(String name) {
return message.getFloatProperty(name);
}
- @Override
- public final double getDoubleProperty(String name) throws JMSException {
+ public final double getDoubleProperty(String name) {
return message.getDoubleProperty(name);
}
- @Override
- public final String getStringProperty(String name) throws JMSException {
+ public final String getStringProperty(String name) {
return MessageUtil.getStringProperty(message, name);
}
- @Override
- public final Object getObjectProperty(String name) throws JMSException {
+ public final Object getObjectProperty(String name) {
return MessageUtil.getObjectProperty(message, name);
}
- @Override
- public final Enumeration getPropertyNames() throws JMSException {
+ public final Enumeration getPropertyNames() {
return Collections.enumeration(MessageUtil.getPropertyNames(message));
}
- @Override
- public final void setBooleanProperty(String name, boolean value) throws JMSException {
+ public final void setBooleanProperty(String name, boolean value) {
message.putBooleanProperty(name, value);
}
- @Override
- public final void setByteProperty(String name, byte value) throws JMSException {
+ public final void setByteProperty(String name, byte value) {
message.putByteProperty(name, value);
}
- @Override
- public final void setShortProperty(String name, short value) throws JMSException {
+ public final void setShortProperty(String name, short value) {
message.putShortProperty(name, value);
}
- @Override
- public final void setIntProperty(String name, int value) throws JMSException {
+ public final void setIntProperty(String name, int value) {
MessageUtil.setIntProperty(message, name, value);
}
- @Override
- public final void setLongProperty(String name, long value) throws JMSException {
+ public final void setLongProperty(String name, long value) {
MessageUtil.setLongProperty(message, name, value);
}
- @Override
- public final void setFloatProperty(String name, float value) throws JMSException {
+ public final void setFloatProperty(String name, float value) {
message.putFloatProperty(name, value);
}
- @Override
- public final void setDoubleProperty(String name, double value) throws JMSException {
+ public final void setDoubleProperty(String name, double value) {
message.putDoubleProperty(name, value);
}
- @Override
- public final void setStringProperty(String name, String value) throws JMSException {
+ public final void setStringProperty(String name, String value) {
MessageUtil.setStringProperty(message, name, value);
}
- @Override
- public final void setObjectProperty(String name, Object value) throws JMSException {
+ public final void setObjectProperty(String name, Object value) {
MessageUtil.setObjectProperty(message, name, value);
}
- @Override
- public final void acknowledge() throws JMSException {
+ public final void acknowledge() {
// no op
}
- @Override
- public void clearBody() throws JMSException {
+ public void clearBody() {
message.getBodyBuffer().clear();
}
- @Override
- public final <T> T getBody(Class<T> c) throws JMSException {
+ public final <T> T getBody(Class<T> c) {
Review comment:
Any point of this noop method. Cleaner to remove it
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574816471
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
Review comment:
I was talking about DeliveryDelay (feature in jms 2.0) not TimeToLive
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574449982
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMapMessageWrapper.java
##########
@@ -0,0 +1,231 @@
+/*
+ * 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.protocol.amqp.converter.coreWrapper;
+
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import org.apache.activemq.artemis.api.core.ActiveMQPropertyConversionException;
+import org.apache.activemq.artemis.api.core.ICoreMessage;
+import org.apache.activemq.artemis.api.core.Message;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.protocol.amqp.converter.AMQPMessageSupport;
+import org.apache.activemq.artemis.utils.collections.TypedProperties;
+import org.apache.qpid.proton.amqp.Binary;
+import org.apache.qpid.proton.amqp.Symbol;
+import org.apache.qpid.proton.amqp.UnsignedByte;
+import org.apache.qpid.proton.amqp.UnsignedInteger;
+import org.apache.qpid.proton.amqp.UnsignedLong;
+import org.apache.qpid.proton.amqp.UnsignedShort;
+import org.apache.qpid.proton.amqp.messaging.AmqpValue;
+import org.apache.qpid.proton.amqp.messaging.Properties;
+import org.apache.qpid.proton.amqp.messaging.Section;
+
+import static org.apache.activemq.artemis.reader.MapMessageUtil.readBodyMap;
+import static org.apache.activemq.artemis.reader.MapMessageUtil.writeBodyMap;
+
+public final class CoreMapMessageWrapper extends CoreMessageWrapper {
+ // Constants -----------------------------------------------------
+
+ public static final byte TYPE = Message.MAP_TYPE;
+
+ // Attributes ----------------------------------------------------
+
+ private final TypedProperties map = new TypedProperties();
+
+ // Static --------------------------------------------------------
+
+ // Constructors --------------------------------------------------
+
+ /*
+ * This constructor is used to construct messages prior to sending
+ */
+ public CoreMapMessageWrapper(ICoreMessage message) {
+ super(message);
+
+ }
+
+ private static Map<String, Object> getMapFromMessageBody(CoreMapMessageWrapper message) {
+ final HashMap<String, Object> map = new LinkedHashMap<>();
+
+ @SuppressWarnings("unchecked")
+ final Enumeration<String> names = message.getMapNames();
+ while (names.hasMoreElements()) {
+ String key = names.nextElement();
+ Object value = message.getObject(key);
+ if (value instanceof byte[]) {
+ value = new Binary((byte[]) value);
+ }
+ map.put(key, value);
+ }
+
+ return map;
+ }
+
+ @Override
+ public Section createAMQPSection(Map<Symbol, Object> maMap, Properties properties) {
+ maMap.put(AMQPMessageSupport.JMS_MSG_TYPE, AMQPMessageSupport.JMS_MAP_MESSAGE);
+ return new AmqpValue(getMapFromMessageBody(this));
+ }
+
+ public void setBoolean(final String name, final boolean value) {
+ map.putBooleanProperty(new SimpleString(name), value);
+ }
+
+ public void setByte(final String name, final byte value) {
+ map.putByteProperty(new SimpleString(name), value);
+ }
+
+ public void setShort(final String name, final short value) {
+ map.putShortProperty(new SimpleString(name), value);
+ }
+
+ public void setChar(final String name, final char value) {
+ map.putCharProperty(new SimpleString(name), value);
+ }
+
+ public void setInt(final String name, final int value) {
+ map.putIntProperty(new SimpleString(name), value);
+ }
+
+ public void setLong(final String name, final long value) {
+ map.putLongProperty(new SimpleString(name), value);
+ }
+
+ public void setFloat(final String name, final float value) {
+ map.putFloatProperty(new SimpleString(name), value);
+ }
+
+ public void setDouble(final String name, final double value) {
+ map.putDoubleProperty(new SimpleString(name), value);
+ }
+
+ public void setString(final String name, final String value) {
+ map.putSimpleStringProperty(new SimpleString(name), value == null ? null : new SimpleString(value));
Review comment:
not part of my change but ok
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574223932
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMapMessageWrapper.java
##########
@@ -137,84 +130,75 @@ public void setObject(final String name, final Object value) throws JMSException
}
TypedProperties.setObjectProperty(new SimpleString(name), val, map);
} catch (ActiveMQPropertyConversionException e) {
- throw new MessageFormatException(e.getMessage());
+ throw new RuntimeException(e.getMessage());
Review comment:
I am fixing these...
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574818330
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
Review comment:
getJMSDeliveryTime
long getJMSDeliveryTime()
throws JMSException
Gets the message's delivery time value.
When a message is sent, the JMSDeliveryTime header field is left unassigned. After completion of the send or publish method, it holds the delivery time of the message. This is the the difference, measured in milliseconds, between the delivery time and midnight, January 1, 1970 UTC.
A message's delivery time is the earliest time when a JMS provider may deliver the message to a consumer. The provider must not deliver messages before the delivery time has been reached.
Returns:
the message's delivery time value
Throws:
JMSException - if the JMS provider fails to get the delivery time due to some internal error.
Since:
JMS 2.0
See Also:
setJMSDeliveryTime(long)
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574445563
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/AMQPMessageSupport.java
##########
@@ -396,36 +371,54 @@ private static CoreMessage newMessage(long id, byte messageType, CoreMessageObje
}
- public static byte destinationType(Destination destination) {
- if (destination instanceof Queue) {
- if (destination instanceof TemporaryQueue) {
- return TEMP_QUEUE_TYPE;
- } else {
- return QUEUE_TYPE;
- }
- } else if (destination instanceof Topic) {
- if (destination instanceof TemporaryTopic) {
- return TEMP_TOPIC_TYPE;
- } else {
- return TOPIC_TYPE;
- }
+ public static String toAddress(String destination) {
+
+ if (destination.startsWith(DestinationUtil.QUEUE_QUALIFIED_PREFIX)) {
+ return destination.substring(DestinationUtil.QUEUE_QUALIFIED_PREFIX.length());
+ } else if (destination.startsWith(DestinationUtil.TOPIC_QUALIFIED_PREFIX)) {
+ return destination.substring(DestinationUtil.TOPIC_QUALIFIED_PREFIX.length());
+ } else if (destination.startsWith(DestinationUtil.TEMP_QUEUE_QUALIFED_PREFIX)) {
+ return destination.substring(DestinationUtil.TEMP_QUEUE_QUALIFED_PREFIX.length());
+ } else if (destination.startsWith(DestinationUtil.TEMP_TOPIC_QUALIFED_PREFIX)) {
+ return destination.substring(DestinationUtil.TEMP_TOPIC_QUALIFED_PREFIX.length());
+ } else {
+ return destination;
}
+ }
+ public static byte destinationType(String destination) {
+ if (destination.startsWith(QUEUE_QUALIFIED_PREFIX)) {
+ return QUEUE_TYPE;
+ }
+ if (destination.startsWith(TOPIC_QUALIFIED_PREFIX)) {
+ return TOPIC_TYPE;
+ }
+ if (destination.startsWith(TEMP_QUEUE_QUALIFED_PREFIX)) {
+ return TEMP_QUEUE_TYPE;
+ }
+ if (destination.startsWith(TEMP_TOPIC_QUALIFED_PREFIX)) {
+ return TEMP_TOPIC_TYPE;
+ }
return QUEUE_TYPE;
}
- public static Destination destination(byte destinationType, String address) {
+ public static String destination(RoutingType destinationType, String address) {
+ String prefix;
+
+ if (destinationType == null) {
+ destinationType = RoutingType.ANYCAST;
+ }
+
switch (destinationType) {
- case TEMP_QUEUE_TYPE:
- return new ActiveMQTemporaryQueue(address, null);
- case TEMP_TOPIC_TYPE:
- return new ActiveMQTemporaryTopic(address, null);
- case TOPIC_TYPE:
- return new ActiveMQTopic(address);
- case QUEUE_TYPE:
- return new ActiveMQQueue(address);
- default:
- return new ActiveMQQueue(address);
+ case ANYCAST: prefix = QUEUE_QUALIFIED_PREFIX; break;
Review comment:
The method that uses routing type has the same semantics 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] michaelandrepearce commented on a change in pull request #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574310967
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
- // no op
- return 0;
- }
-
- @Override
- public final void setJMSDeliveryTime(long deliveryTime) throws JMSException {
- // no op
- }
- @Override
- public final int getJMSPriority() throws JMSException {
+ public final int getJMSPriority() {
return message.getPriority();
}
- @Override
- public final void setJMSPriority(int priority) throws JMSException {
+ public final void setJMSPriority(int priority) {
message.setPriority((byte) priority);
}
- @Override
- public final void clearProperties() throws JMSException {
+ public final void clearProperties() {
MessageUtil.clearProperties(message);
}
- @Override
- public final boolean propertyExists(String name) throws JMSException {
+ public final boolean propertyExists(String name) {
return MessageUtil.propertyExists(message, name);
}
- @Override
- public final boolean getBooleanProperty(String name) throws JMSException {
+ public final boolean getBooleanProperty(String name) {
return message.getBooleanProperty(name);
}
- @Override
- public final byte getByteProperty(String name) throws JMSException {
+ public final byte getByteProperty(String name) {
return message.getByteProperty(name);
}
- @Override
- public final short getShortProperty(String name) throws JMSException {
+ public final short getShortProperty(String name) {
return message.getShortProperty(name);
}
- @Override
- public final int getIntProperty(String name) throws JMSException {
+ public final int getIntProperty(String name) {
return MessageUtil.getIntProperty(message, name);
}
- @Override
- public final long getLongProperty(String name) throws JMSException {
+ public final long getLongProperty(String name) {
return MessageUtil.getLongProperty(message, name);
}
- @Override
- public final float getFloatProperty(String name) throws JMSException {
+ public final float getFloatProperty(String name) {
return message.getFloatProperty(name);
}
- @Override
- public final double getDoubleProperty(String name) throws JMSException {
+ public final double getDoubleProperty(String name) {
return message.getDoubleProperty(name);
}
- @Override
- public final String getStringProperty(String name) throws JMSException {
+ public final String getStringProperty(String name) {
return MessageUtil.getStringProperty(message, name);
}
- @Override
- public final Object getObjectProperty(String name) throws JMSException {
+ public final Object getObjectProperty(String name) {
return MessageUtil.getObjectProperty(message, name);
}
- @Override
- public final Enumeration getPropertyNames() throws JMSException {
+ public final Enumeration getPropertyNames() {
return Collections.enumeration(MessageUtil.getPropertyNames(message));
}
- @Override
- public final void setBooleanProperty(String name, boolean value) throws JMSException {
+ public final void setBooleanProperty(String name, boolean value) {
message.putBooleanProperty(name, value);
}
- @Override
- public final void setByteProperty(String name, byte value) throws JMSException {
+ public final void setByteProperty(String name, byte value) {
message.putByteProperty(name, value);
}
- @Override
- public final void setShortProperty(String name, short value) throws JMSException {
+ public final void setShortProperty(String name, short value) {
message.putShortProperty(name, value);
}
- @Override
- public final void setIntProperty(String name, int value) throws JMSException {
+ public final void setIntProperty(String name, int value) {
MessageUtil.setIntProperty(message, name, value);
}
- @Override
- public final void setLongProperty(String name, long value) throws JMSException {
+ public final void setLongProperty(String name, long value) {
MessageUtil.setLongProperty(message, name, value);
}
- @Override
- public final void setFloatProperty(String name, float value) throws JMSException {
+ public final void setFloatProperty(String name, float value) {
message.putFloatProperty(name, value);
}
- @Override
- public final void setDoubleProperty(String name, double value) throws JMSException {
+ public final void setDoubleProperty(String name, double value) {
message.putDoubleProperty(name, value);
}
- @Override
- public final void setStringProperty(String name, String value) throws JMSException {
+ public final void setStringProperty(String name, String value) {
MessageUtil.setStringProperty(message, name, value);
}
- @Override
- public final void setObjectProperty(String name, Object value) throws JMSException {
+ public final void setObjectProperty(String name, Object value) {
MessageUtil.setObjectProperty(message, name, value);
}
- @Override
- public final void acknowledge() throws JMSException {
+ public final void acknowledge() {
// no op
}
- @Override
- public void clearBody() throws JMSException {
+ public void clearBody() {
message.getBodyBuffer().clear();
}
- @Override
- public final <T> T getBody(Class<T> c) throws JMSException {
+ public final <T> T getBody(Class<T> c) {
// no op.. jms2 not used on the conversion
return null;
}
/**
* Encode the body into the internal message
*/
- public void encode() throws Exception {
+ public void encode() {
if (!message.isLargeMessage()) {
message.getBodyBuffer().resetReaderIndex();
}
}
- public void decode() throws Exception {
+ public void decode() {
if (!message.isLargeMessage()) {
message.getBodyBuffer().resetReaderIndex();
}
}
- @Override
- public final boolean isBodyAssignableTo(Class c) throws JMSException {
+ public final boolean isBodyAssignableTo(Class c) {
Review comment:
If this isnt used and no longer impl interface...would be better to remove. No point having a no op method in a concrete class with no interface contract
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574227295
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMapMessageWrapper.java
##########
@@ -137,84 +130,75 @@ public void setObject(final String name, final Object value) throws JMSException
}
TypedProperties.setObjectProperty(new SimpleString(name), val, map);
} catch (ActiveMQPropertyConversionException e) {
- throw new MessageFormatException(e.getMessage());
+ throw new RuntimeException(e.getMessage());
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] asfgit merged pull request #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574494338
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/AMQPMessageSupport.java
##########
@@ -396,36 +371,54 @@ private static CoreMessage newMessage(long id, byte messageType, CoreMessageObje
}
- public static byte destinationType(Destination destination) {
- if (destination instanceof Queue) {
- if (destination instanceof TemporaryQueue) {
- return TEMP_QUEUE_TYPE;
- } else {
- return QUEUE_TYPE;
- }
- } else if (destination instanceof Topic) {
- if (destination instanceof TemporaryTopic) {
- return TEMP_TOPIC_TYPE;
- } else {
- return TOPIC_TYPE;
- }
+ public static String toAddress(String destination) {
+
+ if (destination.startsWith(DestinationUtil.QUEUE_QUALIFIED_PREFIX)) {
+ return destination.substring(DestinationUtil.QUEUE_QUALIFIED_PREFIX.length());
+ } else if (destination.startsWith(DestinationUtil.TOPIC_QUALIFIED_PREFIX)) {
+ return destination.substring(DestinationUtil.TOPIC_QUALIFIED_PREFIX.length());
+ } else if (destination.startsWith(DestinationUtil.TEMP_QUEUE_QUALIFED_PREFIX)) {
+ return destination.substring(DestinationUtil.TEMP_QUEUE_QUALIFED_PREFIX.length());
+ } else if (destination.startsWith(DestinationUtil.TEMP_TOPIC_QUALIFED_PREFIX)) {
+ return destination.substring(DestinationUtil.TEMP_TOPIC_QUALIFED_PREFIX.length());
+ } else {
+ return destination;
}
+ }
+ public static byte destinationType(String destination) {
+ if (destination.startsWith(QUEUE_QUALIFIED_PREFIX)) {
+ return QUEUE_TYPE;
+ }
+ if (destination.startsWith(TOPIC_QUALIFIED_PREFIX)) {
+ return TOPIC_TYPE;
+ }
+ if (destination.startsWith(TEMP_QUEUE_QUALIFED_PREFIX)) {
+ return TEMP_QUEUE_TYPE;
+ }
+ if (destination.startsWith(TEMP_TOPIC_QUALIFED_PREFIX)) {
+ return TEMP_TOPIC_TYPE;
+ }
return QUEUE_TYPE;
}
- public static Destination destination(byte destinationType, String address) {
+ public static String destination(RoutingType destinationType, String address) {
+ String prefix;
+
+ if (destinationType == null) {
+ destinationType = RoutingType.ANYCAST;
+ }
+
switch (destinationType) {
- case TEMP_QUEUE_TYPE:
- return new ActiveMQTemporaryQueue(address, null);
- case TEMP_TOPIC_TYPE:
- return new ActiveMQTemporaryTopic(address, null);
- case TOPIC_TYPE:
- return new ActiveMQTopic(address);
- case QUEUE_TYPE:
- return new ActiveMQQueue(address);
- default:
- return new ActiveMQQueue(address);
+ case ANYCAST: prefix = QUEUE_QUALIFIED_PREFIX; break;
Review comment:
The original method was creating a Queue type (TemporaryQueue, TemporaryTopic, Queue, Topic), to set on the JMS Object, to then only use the simple name inside the object.
The Queue type was being set somewhere else, and this was not being used.
This method now is being use to replace ActiveMQDEstiation.createQueue
There is a test that was already validating this and it is passing.
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r575325712
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
Review comment:
@michaelandrepearce PR coming soon: https://issues.apache.org/jira/browse/ARTEMIS-3116
It was an issue indeed. I believe you would need AMQP Mirror to hit the issue.
PR coming soon.
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574311518
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
- // no op
- return 0;
- }
-
- @Override
- public final void setJMSDeliveryTime(long deliveryTime) throws JMSException {
- // no op
- }
- @Override
- public final int getJMSPriority() throws JMSException {
+ public final int getJMSPriority() {
return message.getPriority();
}
- @Override
- public final void setJMSPriority(int priority) throws JMSException {
+ public final void setJMSPriority(int priority) {
message.setPriority((byte) priority);
}
- @Override
- public final void clearProperties() throws JMSException {
+ public final void clearProperties() {
MessageUtil.clearProperties(message);
}
- @Override
- public final boolean propertyExists(String name) throws JMSException {
+ public final boolean propertyExists(String name) {
return MessageUtil.propertyExists(message, name);
}
- @Override
- public final boolean getBooleanProperty(String name) throws JMSException {
+ public final boolean getBooleanProperty(String name) {
return message.getBooleanProperty(name);
}
- @Override
- public final byte getByteProperty(String name) throws JMSException {
+ public final byte getByteProperty(String name) {
return message.getByteProperty(name);
}
- @Override
- public final short getShortProperty(String name) throws JMSException {
+ public final short getShortProperty(String name) {
return message.getShortProperty(name);
}
- @Override
- public final int getIntProperty(String name) throws JMSException {
+ public final int getIntProperty(String name) {
return MessageUtil.getIntProperty(message, name);
}
- @Override
- public final long getLongProperty(String name) throws JMSException {
+ public final long getLongProperty(String name) {
return MessageUtil.getLongProperty(message, name);
}
- @Override
- public final float getFloatProperty(String name) throws JMSException {
+ public final float getFloatProperty(String name) {
return message.getFloatProperty(name);
}
- @Override
- public final double getDoubleProperty(String name) throws JMSException {
+ public final double getDoubleProperty(String name) {
return message.getDoubleProperty(name);
}
- @Override
- public final String getStringProperty(String name) throws JMSException {
+ public final String getStringProperty(String name) {
return MessageUtil.getStringProperty(message, name);
}
- @Override
- public final Object getObjectProperty(String name) throws JMSException {
+ public final Object getObjectProperty(String name) {
return MessageUtil.getObjectProperty(message, name);
}
- @Override
- public final Enumeration getPropertyNames() throws JMSException {
+ public final Enumeration getPropertyNames() {
return Collections.enumeration(MessageUtil.getPropertyNames(message));
}
- @Override
- public final void setBooleanProperty(String name, boolean value) throws JMSException {
+ public final void setBooleanProperty(String name, boolean value) {
message.putBooleanProperty(name, value);
}
- @Override
- public final void setByteProperty(String name, byte value) throws JMSException {
+ public final void setByteProperty(String name, byte value) {
message.putByteProperty(name, value);
}
- @Override
- public final void setShortProperty(String name, short value) throws JMSException {
+ public final void setShortProperty(String name, short value) {
message.putShortProperty(name, value);
}
- @Override
- public final void setIntProperty(String name, int value) throws JMSException {
+ public final void setIntProperty(String name, int value) {
MessageUtil.setIntProperty(message, name, value);
}
- @Override
- public final void setLongProperty(String name, long value) throws JMSException {
+ public final void setLongProperty(String name, long value) {
MessageUtil.setLongProperty(message, name, value);
}
- @Override
- public final void setFloatProperty(String name, float value) throws JMSException {
+ public final void setFloatProperty(String name, float value) {
message.putFloatProperty(name, value);
}
- @Override
- public final void setDoubleProperty(String name, double value) throws JMSException {
+ public final void setDoubleProperty(String name, double value) {
message.putDoubleProperty(name, value);
}
- @Override
- public final void setStringProperty(String name, String value) throws JMSException {
+ public final void setStringProperty(String name, String value) {
MessageUtil.setStringProperty(message, name, value);
}
- @Override
- public final void setObjectProperty(String name, Object value) throws JMSException {
+ public final void setObjectProperty(String name, Object value) {
MessageUtil.setObjectProperty(message, name, value);
}
- @Override
- public final void acknowledge() throws JMSException {
+ public final void acknowledge() {
Review comment:
See other comments on noops
----------------------------------------------------------------
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 pull request #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#issuecomment-777781968
@clebertsuconic see comment inline
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574846749
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
Review comment:
This is handled by the converter directly. The "JMS" facade was not using it in any way.
There are open wire scheduled delivery tests for this already.
On AMQP to Core Converter:
https://github.com/apache/activemq-artemis/blob/master/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/AmqpCoreConverter.java#L279-L300
On Core to AMQP Converter, the Converter will remove the Scheduled Delivery on Purpose, as part of:
https://github.com/apache/activemq-artemis/blob/master/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/CoreAmqpConverter.java#L300-L302
The only reason Actually AMQP To Core converter is treating scheduling is because we used to always convert to Core. On the recent versions of the broker the message will stay on its original format until the actuall delivery occurred.
If there is a bug it would not been cause by this change anyways but I think we are safe 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] clebertsuconic commented on a change in pull request #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574450855
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
Review comment:
TTL is handled directly at the converter.this method was not being used.
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574309508
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMapMessageWrapper.java
##########
@@ -0,0 +1,231 @@
+/*
+ * 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.protocol.amqp.converter.coreWrapper;
+
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import org.apache.activemq.artemis.api.core.ActiveMQPropertyConversionException;
+import org.apache.activemq.artemis.api.core.ICoreMessage;
+import org.apache.activemq.artemis.api.core.Message;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.protocol.amqp.converter.AMQPMessageSupport;
+import org.apache.activemq.artemis.utils.collections.TypedProperties;
+import org.apache.qpid.proton.amqp.Binary;
+import org.apache.qpid.proton.amqp.Symbol;
+import org.apache.qpid.proton.amqp.UnsignedByte;
+import org.apache.qpid.proton.amqp.UnsignedInteger;
+import org.apache.qpid.proton.amqp.UnsignedLong;
+import org.apache.qpid.proton.amqp.UnsignedShort;
+import org.apache.qpid.proton.amqp.messaging.AmqpValue;
+import org.apache.qpid.proton.amqp.messaging.Properties;
+import org.apache.qpid.proton.amqp.messaging.Section;
+
+import static org.apache.activemq.artemis.reader.MapMessageUtil.readBodyMap;
+import static org.apache.activemq.artemis.reader.MapMessageUtil.writeBodyMap;
+
+public final class CoreMapMessageWrapper extends CoreMessageWrapper {
+ // Constants -----------------------------------------------------
+
+ public static final byte TYPE = Message.MAP_TYPE;
+
+ // Attributes ----------------------------------------------------
+
+ private final TypedProperties map = new TypedProperties();
+
+ // Static --------------------------------------------------------
+
+ // Constructors --------------------------------------------------
+
+ /*
+ * This constructor is used to construct messages prior to sending
+ */
+ public CoreMapMessageWrapper(ICoreMessage message) {
+ super(message);
+
+ }
+
+ private static Map<String, Object> getMapFromMessageBody(CoreMapMessageWrapper message) {
+ final HashMap<String, Object> map = new LinkedHashMap<>();
+
+ @SuppressWarnings("unchecked")
+ final Enumeration<String> names = message.getMapNames();
+ while (names.hasMoreElements()) {
+ String key = names.nextElement();
+ Object value = message.getObject(key);
+ if (value instanceof byte[]) {
+ value = new Binary((byte[]) value);
+ }
+ map.put(key, value);
+ }
+
+ return map;
+ }
+
+ @Override
+ public Section createAMQPSection(Map<Symbol, Object> maMap, Properties properties) {
+ maMap.put(AMQPMessageSupport.JMS_MSG_TYPE, AMQPMessageSupport.JMS_MAP_MESSAGE);
+ return new AmqpValue(getMapFromMessageBody(this));
+ }
+
+ public void setBoolean(final String name, final boolean value) {
+ map.putBooleanProperty(new SimpleString(name), value);
+ }
+
+ public void setByte(final String name, final byte value) {
+ map.putByteProperty(new SimpleString(name), value);
+ }
+
+ public void setShort(final String name, final short value) {
+ map.putShortProperty(new SimpleString(name), value);
+ }
+
+ public void setChar(final String name, final char value) {
+ map.putCharProperty(new SimpleString(name), value);
+ }
+
+ public void setInt(final String name, final int value) {
+ map.putIntProperty(new SimpleString(name), value);
+ }
+
+ public void setLong(final String name, final long value) {
+ map.putLongProperty(new SimpleString(name), value);
+ }
+
+ public void setFloat(final String name, final float value) {
+ map.putFloatProperty(new SimpleString(name), value);
+ }
+
+ public void setDouble(final String name, final double value) {
+ map.putDoubleProperty(new SimpleString(name), value);
+ }
+
+ public void setString(final String name, final String value) {
+ map.putSimpleStringProperty(new SimpleString(name), value == null ? null : new SimpleString(value));
Review comment:
Isnt there a static method on SimpleString that returns null if string null, so code can be cleaner
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r575325712
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
Review comment:
@michaelandrepearcehttps://issues.apache.org/jira/browse/ARTEMIS-3116
It was an issue indeed. I believe you would need AMQP Mirror to hit the issue.
https://github.com/apache/activemq-artemis/pull/3453
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574312462
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
Review comment:
Have we a possible bug here in use case of delivery delay in jms2.0 and message gets converted from amqp to core / vice versa.
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574307737
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/AMQPMessageSupport.java
##########
@@ -396,36 +371,54 @@ private static CoreMessage newMessage(long id, byte messageType, CoreMessageObje
}
- public static byte destinationType(Destination destination) {
- if (destination instanceof Queue) {
- if (destination instanceof TemporaryQueue) {
- return TEMP_QUEUE_TYPE;
- } else {
- return QUEUE_TYPE;
- }
- } else if (destination instanceof Topic) {
- if (destination instanceof TemporaryTopic) {
- return TEMP_TOPIC_TYPE;
- } else {
- return TOPIC_TYPE;
- }
+ public static String toAddress(String destination) {
+
+ if (destination.startsWith(DestinationUtil.QUEUE_QUALIFIED_PREFIX)) {
+ return destination.substring(DestinationUtil.QUEUE_QUALIFIED_PREFIX.length());
+ } else if (destination.startsWith(DestinationUtil.TOPIC_QUALIFIED_PREFIX)) {
+ return destination.substring(DestinationUtil.TOPIC_QUALIFIED_PREFIX.length());
+ } else if (destination.startsWith(DestinationUtil.TEMP_QUEUE_QUALIFED_PREFIX)) {
+ return destination.substring(DestinationUtil.TEMP_QUEUE_QUALIFED_PREFIX.length());
+ } else if (destination.startsWith(DestinationUtil.TEMP_TOPIC_QUALIFED_PREFIX)) {
+ return destination.substring(DestinationUtil.TEMP_TOPIC_QUALIFED_PREFIX.length());
+ } else {
+ return destination;
}
+ }
+ public static byte destinationType(String destination) {
+ if (destination.startsWith(QUEUE_QUALIFIED_PREFIX)) {
+ return QUEUE_TYPE;
+ }
+ if (destination.startsWith(TOPIC_QUALIFIED_PREFIX)) {
+ return TOPIC_TYPE;
+ }
+ if (destination.startsWith(TEMP_QUEUE_QUALIFED_PREFIX)) {
+ return TEMP_QUEUE_TYPE;
+ }
+ if (destination.startsWith(TEMP_TOPIC_QUALIFED_PREFIX)) {
+ return TEMP_TOPIC_TYPE;
+ }
return QUEUE_TYPE;
}
- public static Destination destination(byte destinationType, String address) {
+ public static String destination(RoutingType destinationType, String address) {
+ String prefix;
+
+ if (destinationType == null) {
+ destinationType = RoutingType.ANYCAST;
+ }
+
switch (destinationType) {
- case TEMP_QUEUE_TYPE:
- return new ActiveMQTemporaryQueue(address, null);
- case TEMP_TOPIC_TYPE:
- return new ActiveMQTemporaryTopic(address, null);
- case TOPIC_TYPE:
- return new ActiveMQTopic(address);
- case QUEUE_TYPE:
- return new ActiveMQQueue(address);
- default:
- return new ActiveMQQueue(address);
+ case ANYCAST: prefix = QUEUE_QUALIFIED_PREFIX; break;
Review comment:
This looks like it no longer handles temporaries
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574853020
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
Review comment:
I will add some testing as a separate task from this. There's one possibility actually, when you send Core Message with a scheduled delivery through the AMQP Mirror the delivery delay may been stripped on that case. This would be a side effect of what you mentioned. I will test it and fix it if it's an issue. (not related to this change but I will validate it).
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574816471
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
Review comment:
I was talking about DeliveryDelay (feature in jms 2.0) not TimeToLive, as in when DeliveryDelay is used this holds the earliest time that a message can be delivered.
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r575325712
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
Review comment:
@michaelandrepearce https://issues.apache.org/jira/browse/ARTEMIS-3116
It was an issue indeed. I believe you would need AMQP Mirror to hit the issue.
https://github.com/apache/activemq-artemis/pull/3453
----------------------------------------------------------------
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 #3450: ARTEMIS-3113 - Artemis AMQP shouldn't depend on JMS.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3450:
URL: https://github.com/apache/activemq-artemis/pull/3450#discussion_r574846749
##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/coreWrapper/CoreMessageWrapper.java
##########
@@ -137,247 +175,187 @@ public final String getJMSCorrelationID() throws JMSException {
}
}
- @Override
- public final void setJMSCorrelationID(String correlationID) throws JMSException {
+ public final void setJMSCorrelationID(String correlationID) {
message.setCorrelationID(correlationID);
}
- @Override
- public final Destination getJMSReplyTo() throws JMSException {
- SimpleString reply = MessageUtil.getJMSReplyTo(message);
- if (reply != null) {
- return ActiveMQDestination.fromPrefixedName(reply.toString());
- } else {
- return null;
- }
+ public final SimpleString getJMSReplyTo() {
+ return MessageUtil.getJMSReplyTo(message);
}
- @Override
- public final void setJMSReplyTo(Destination replyTo) throws JMSException {
- MessageUtil.setJMSReplyTo(message, replyTo == null ? null : ((ActiveMQDestination) replyTo).getSimpleAddress());
+ public final void setJMSReplyTo(String replyTo) {
+ MessageUtil.setJMSReplyTo(message, SimpleString.toSimpleString(replyTo));
}
- @Override
- public Destination getJMSDestination() throws JMSException {
- return ActiveMQDestination.createDestination(message.getRoutingType(), message.getAddressSimpleString());
- }
-
- @Override
- public final void setJMSDestination(Destination destination) throws JMSException {
- if (destination == null) {
- message.setAddress((SimpleString) null);
- } else {
- message.setAddress(((ActiveMQDestination) destination).getSimpleAddress());
+ public SimpleString getDestination() {
+ if (message.getAddress() == null || message.getAddress().isEmpty()) {
+ return null;
}
-
+ return SimpleString.toSimpleString(AMQPMessageSupport.destination(message.getRoutingType(), message.getAddress()));
}
- @Override
- public final int getJMSDeliveryMode() throws JMSException {
- return message.isDurable() ? DeliveryMode.PERSISTENT : DeliveryMode.NON_PERSISTENT;
+ public final void setDestination(String destination) {
+ message.setAddress(destination);
}
- @Override
- public final void setJMSDeliveryMode(int deliveryMode) throws JMSException {
- if (deliveryMode == DeliveryMode.PERSISTENT) {
- message.setDurable(true);
- } else if (deliveryMode == DeliveryMode.NON_PERSISTENT) {
- message.setDurable(false);
- } else {
- throw new JMSException("Invalid mode " + deliveryMode);
- }
+ public final int getJMSDeliveryMode() {
+ return message.isDurable() ? PERSISTENT : NON_PERSISTENT;
}
- @Override
- public final boolean getJMSRedelivered() throws JMSException {
- return false;
+ public final void setDeliveryMode(int deliveryMode) throws ConversionException {
+ switch (deliveryMode) {
+ case PERSISTENT:
+ message.setDurable(true);
+ break;
+ case NON_PERSISTENT:
+ message.setDurable(false);
+ break;
+ default:
+ // There shouldn't be any other values used
+ throw new ConversionException("Invalid mode " + deliveryMode);
+ }
}
- @Override
- public final void setJMSRedelivered(boolean redelivered) throws JMSException {
- // no op
- }
- @Override
- public final String getJMSType() throws JMSException {
+ public final String getJMSType() {
return MessageUtil.getJMSType(message);
}
- @Override
- public final void setJMSType(String type) throws JMSException {
+ public final void setJMSType(String type) {
MessageUtil.setJMSType(message, type);
}
- @Override
- public final long getJMSExpiration() throws JMSException {
+ public final long getExpiration() {
return message.getExpiration();
}
- @Override
- public final void setJMSExpiration(long expiration) throws JMSException {
+ public final void setJMSExpiration(long expiration) {
message.setExpiration(expiration);
}
- @Override
- public final long getJMSDeliveryTime() throws JMSException {
Review comment:
This is handled by the converter directly. The "JMS" facade was not using it in any way.
There are open wire scheduled delivery tests for this already.
On AMQP to Core Converter:
https://github.com/apache/activemq-artemis/blob/master/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/AmqpCoreConverter.java#L279-L300
On Core to AMQP Converter, the Converter will remove the Scheduled Delivery on Purpose, as part of:
https://github.com/apache/activemq-artemis/blob/master/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/CoreAmqpConverter.java#L300-L302
The only reason Actually AMQP To Core converter is treating scheduling is because we used to always convert to Core. On the recent versions of the broker the message will stay on its original format until the actuall delivery occurred.
If there is a bug it would not been caused by this change anyways. I think we are safe 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