jbertram commented on PR #4833:
URL:
https://github.com/apache/activemq-artemis/pull/4833#issuecomment-1969949109
If you executed the `JMSCorrelationIDTest` I added in d38b5a7 without the
other changes this would be the result:
- Correlation ID as `byte[]`
- :white_check_mark: AMQP to AMQP
- :x: AMQP to Core
- :x: AMQP to OpenWire
- :white_check_mark: Core to Core
- :x: Core to AMQP
- :x: Core to OpenWire
- :white_check_mark: OpenWire to OpenWire
- :x: OpenWire to AMQP
- :x: OpenWire to Core
- Correlation ID as `String`
- :white_check_mark: AMQP to AMQP
- :white_check_mark: AMQP to Core
- :white_check_mark: AMQP to OpenWire
- :white_check_mark: Core to Core
- :white_check_mark: Core to AMQP
- :white_check_mark: Core to OpenWire
- :white_check_mark: OpenWire to OpenWire
- :white_check_mark: OpenWire to AMQP
- :white_check_mark: OpenWire to Core
So...right now there is full compatibility with `String` among JMS clients
and no compatibility with `byte[]`.
If you executed the `JMSCorrelationIDTest` I added in d38b5a7 along with the
other changes this would be the result:
- Correlation ID as `byte[]`
- :white_check_mark: AMQP to AMQP
- :white_check_mark: AMQP to Core
- :white_check_mark: AMQP to OpenWire
- :white_check_mark: Core to Core
- :white_check_mark: Core to AMQP
- :white_check_mark: Core to OpenWire
- :white_check_mark: OpenWire to OpenWire
- :white_check_mark: OpenWire to AMQP
- :white_check_mark: OpenWire to Core
- Correlation ID as `String`
- :white_check_mark: AMQP to AMQP
- :white_check_mark: AMQP to Core
- :white_check_mark: AMQP to OpenWire
- :white_check_mark: Core to Core
- :white_check_mark: Core to AMQP
- :white_check_mark: Core to OpenWire
- :white_check_mark: OpenWire to OpenWire
- :x: OpenWire to AMQP
- :x: OpenWire to Core
So...the changes in d38b5a7 are sacrificing 2 OpenWire `String` use-cases
for full compatibility with `byte[]`. Whether or not this is OK is the subject
of this PR.
If I hard-code the broker to deal with the OpenWire correlation ID as a
**`String`** these are the results:
- Correlation ID as `byte[]`
- :white_check_mark: AMQP to AMQP
- :white_check_mark: AMQP to Core
- :x: AMQP to OpenWire
- :white_check_mark: Core to Core
- :white_check_mark: Core to AMQP
- :x: Core to OpenWire
- :white_check_mark: OpenWire to OpenWire
- :x: OpenWire to AMQP
- :x: OpenWire to Core
- Correlation ID as `String`
- :white_check_mark: AMQP to AMQP
- :white_check_mark: AMQP to Core
- :white_check_mark: AMQP to OpenWire
- :white_check_mark: Core to Core
- :white_check_mark: Core to AMQP
- :white_check_mark: Core to OpenWire
- :white_check_mark: OpenWire to OpenWire
- :white_check_mark: OpenWire to AMQP
- :white_check_mark: OpenWire to Core
In short, full compatibility with `String` and no compatibility with
OpenWire with `byte[]`.
Perhaps our hands are tied here and we just can't support OpenWire
correlation ID as `byte[]` since it will break existing use-cases. I'm running
the full test-suite now, and if there are existing test-cases that break then
that will help convince me. However, at this point it seems to me like using
`byte[]` to gain compatibility between JMS and a "native" format (e.g. core) is
actually the right thing to do so supporting `byte[]` across all protocols at
the sacrifice of `String` is appropriate especially since this will open the
door to compatibility with MQTT 5 (although I grant that's likely an edge case).
I'm, of course, open to being convinced otherwise. I'd love your thoughts.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
