activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From gemmellr <...@git.apache.org>
Subject [GitHub] activemq-artemis pull request #2115: ARTEMIS-1858 Expiry messages are not tr...
Date Wed, 30 May 2018 10:46:50 GMT
Github user gemmellr commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2115#discussion_r191715800
  
    --- Diff: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java
---
    @@ -606,22 +606,36 @@ public String getAddress() {
     
        @Override
        public AMQPMessage setAddress(String address) {
    -      this.address = SimpleString.toSimpleString(address, coreMessageObjectPools == null
? null : coreMessageObjectPools.getAddressStringSimpleStringPool());
    +      internalSetAddress(address);
    +      setProtonAddress(address);
           return this;
        }
     
    +   private void internalSetAddress(String address) {
    +      this.address = SimpleString.toSimpleString(address, coreMessageObjectPools == null
? null : coreMessageObjectPools.getAddressStringSimpleStringPool());
    +   }
    +
        @Override
        public AMQPMessage setAddress(SimpleString address) {
           this.address = address;
    +      setProtonAddress(address.toString());
    --- End diff --
    
    Where all is the outer setAddress method used? The AMQP Properties section is part of
the immutable bare message so we shouldn't in general be setting the 'to' address in it or
creating the section if they weren't present. Exception might be made during cases like protocol
conversion, but it seems like it should be explicit rather than a side effect that might see
unintended use as here.


---

Mime
View raw message