Repository navigation
Conversation
…d enforce callback restrictions
…t destination parameter calling send(destination, message) on fixed-destination producers — a JMS spec violation that the old code silently allowed
fix send calls to remove the destination
…thread instead of a global thread local We need the check to be session scoped and to JVM scoped per spec.
…roperties
JMSProducer's typed property getters must treat a property that was never
set as if it exists with a null value, i.e. apply the JMS property
conversion table to null -- the same semantics Boolean.valueOf(null),
Integer.valueOf(null) and friends have:
boolean -> false
byte, short, int, long -> NumberFormatException
float, double -> NullPointerException
String, Object -> null
ActiveMQProducer instead threw NullPointerException from all of them. Six
of the nine were wrong; float and double were correct only by coincidence.
ActiveMQMessage -- the Message API, bound by the same rule -- already
implements this correctly, and ActiveMQProducer reuses its exact error text
("property " + name + " was null") while collapsing every case to NPE, so
this reads as a copy that flattened the per-type exception types. The fix
mirrors ActiveMQMessage.
Only the missing-property path changes; it could not succeed before. Values
that were actually set convert exactly as they did.
Fixes core20/jmsproducer{queue,topic}tests/Client#msgPropertiesTest.
…ules JMS separates an identified producer (created with a destination) from an unidentified one (created without), and reserves a different set of send methods for each. Misusing either raises UnsupportedOperationException, as the MessageProducer javadoc -- already reproduced verbatim in ActiveMQMessageProducerSupport -- specifies: identified + send(Message, ...) allowed identified + send(Destination, Message, ...) UnsupportedOperationException unidentified + send(Destination, Message, ...) allowed unidentified + send(Message, ...) UnsupportedOperationException ActiveMQ enforced neither direction: an unidentified producer threw InvalidDestinationException (a checked JMSException, so callers expecting the unchecked UnsupportedOperationException never caught it), and an identified producer silently accepted an explicit destination. Enforcement is OPT-IN via the connection's existing strictCompliance flag, the same switch ActiveMQMessage uses for strict Jakarta 3.1 property rules. ActiveMQ has accepted either send form on either producer for many years and a great deal of code depends on that -- 94 test files in this repo alone use the identified + send(Destination, Message) form. Enforcing unconditionally would break them and, by extension, a lot of user code. With strictCompliance false (the default) behaviour is unchanged. Every public send path converges on send(Destination, ...), and the destination-less overloads reach it by passing getDestination(), so a guard placed at the convergence point would reject the legal call. The destination-less overloads now route through a private sendToProducerDestination() straight to the internal send path, and the guards sit only on the genuine destination-taking entry points. checkClosed() deliberately runs before both guards: a closed producer must report IllegalStateException whichever form is used (JmsMessageProcuderClosedTest asserts exactly this for an identified producer calling send(Destination, Message)). With the TCK harness running under strictCompliance this fixes: core/exceptionQueue/ExceptionQueueTests#xUnsupportedOperationExceptionQTest1 core/exceptionQueue/ExceptionQueueTests#xUnsupportedOperationExceptionQTest2 core/exceptionTopic/ExceptionTopicTests#xUnsupportedOperationExceptionTTest1 core20/messageproducertests/Client#queueUnsupportedOperationExceptionTests core20/messageproducertests/Client#topicUnsupportedOperationExceptionTests The CompletionListener send overloads enforced the identified-producer mirror rule unconditionally; they now use the same strict-gated guard, and send(Message)/send(Message, int, int, long) route through sendToProducerDestination() rather than the destination-taking overload so an identified producer does not reject its own destination. With the default lenient mode the null-listener check is reached first, so the async null-listener tests in ActiveMQJMS2ContextTest expect IllegalArgumentException.
…is on These calls behave as they do on main again. The stricter behavior applies only under strict compliance. - close or stop from a MessageListener is allowed - send(Message) on a producer created without a destination throws InvalidDestinationException - a JMSProducer typed getter on a property that was never set throws NullPointerException The shared consumer methods report a null topic or a bad selector before they report that they are not supported, with or without strict compliance. The two JMSConsumerTest tests that close the connection from a listener are back to how they are on main.
… async support The pool only passes the CompletionListener through to the provider. The test asserted the UnsupportedOperationException the ActiveMQ Classic client threw while async send was unimplemented, which ties the pool to that state of the client; it now accepts either outcome and, when the provider supports async send, checks that the listener completes and the message arrives.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This new PR applies a few changes to @jeanouii original PR to allow for it to be merged in a non-major version due to exception type changes.
Changes to existing calls, compared with
main, and how this branch handles each one:mainsend(Destination, Message)on a producer created with that destinationUnsupportedOperationException(#1728)UnsupportedOperationExceptiononly whenstrictComplianceis on.send(Message)on a producer created without a destinationInvalidDestinationExceptionUnsupportedOperationException(#1728)InvalidDestinationException.UnsupportedOperationExceptiononly whenstrictComplianceis on.Connection.close(),Connection.stop()orSession.close()from aMessageListenerIllegalStateException(#1728)IllegalStateExceptiononly whenstrictComplianceis on.createSharedConsumerandcreateSharedDurableConsumerwith a null topic or a bad selectorInvalidDestinationExceptionorInvalidSelectorExceptionUnsupportedOperationException(#1728)mainJMSProducertyped getters on a property that was never setNullPointerExceptionfalse,nullorNumberFormatException, depending on the type (#2604)NullPointerException. The JMS conversion applies only whenstrictComplianceis on.send(...)with an explicit delivery mode and priorityJMSException(#1728)JMSException, not changed hereThe last row is the one change that still applies by default.
The
CompletionListenersends,setAsyncandgetAsyncthrewUnsupportedOperationExceptionbefore. Moving away from that is part of the communicated Jakarta transition plan, so they are not listed.