[AMQ-9530]Fix SelectorAwareVirtualTopicInterceptor ClassCastException if next is not Topic.

[Nikita-Shupletsov]

next is not guaranteed to be Topic. If there is plugin that injects another DestinationInterceptor, the code will fail with ClassCastException.

Added a method to correctly unwrap next.

Ran:

• AMQ4899Test.java
• VirtualTopicSelectorTest.java
• JavaVirtualDestTest.java
• SelectorAwareVTThatDropsMessagesWhenNoConsumer.java
• TwoBrokerVirtualTopicSelectorAwareForwardingTest.java
• VirtualConsumerDemandTest.java
  to make sure there is no regression

[mattrpav]

What is the use case for this?

[Nikita-Shupletsov]

What is the use case for this?

you mean the use case of having a DestinationFilter after SelectorAwareVirtualTopicInterceptor?

I my case I add an interceptor to all topics. and that interceptor coexists with the rest of interceptors. and some topics can be vistual topics. it's technically possible on the plugin side to check if there is a virtual topic interceptor and not add my interceptor. But I checked other interceptors(this one for example) and they handle this situation correctly, so I thought it would be a positive change in general. In case people decide to do something similar in the future, they will not need to work around the edge case.

[mattrpav]

A couple of points:

1. This change needs to be a JIRA
2. This needs a unit test with an example configured w/ another DestinationFilter on top of Virtual Topic
3. If we are going to fix it, we should adapt all the Virtual Destinations to use the same set of methods.

[Nikita-Shupletsov]

1. This change needs to be a JIRA

Done. https://issues.apache.org/jira/browse/AMQ-9530

2. This needs a unit test with an example configured w/ another DestinationFilter on top of Virtual Topic

Updated VirtualTopicSelectorTest, as it's basically tests SelectorAwareVirtualTopicInterceptor. Also used it to initially reproduce the issue(see the ticket).

3. If we are going to fix it, we should adapt all the Virtual Destinations to use the same set of methods.

There are no issues like this in other pars of the Virtual Destination code. only in SelectorAwareVirtualTopicInterceptor

[mattrpav]

There are no issues like this in other pars of the Virtual Destination code. only in SelectorAwareVirtualTopicInterceptor

MappedQueueFilter has a nearly identical method:

ref:

activemq/activemq-broker/src/main/java/org/apache/activemq/broker/region/virtual/MappedQueueFilter.java

Line 90 in 5309352

private BaseDestination getBaseDestination(Destination virtualDest) {

[Nikita-Shupletsov]

There are no issues like this in other pars of the Virtual Destination code. only in SelectorAwareVirtualTopicInterceptor

MappedQueueFilter has a nearly identical method:

ref:

activemq/activemq-broker/src/main/java/org/apache/activemq/broker/region/virtual/MappedQueueFilter.java

Line 90 in 5309352

private BaseDestination getBaseDestination(Destination virtualDest) {

are you proposing to extract this method to a helper class? If I create a new class, let's say VirtualTopicSupport or VistualTopicHelper, will it be okay? or do you have a particular place in mind fir it?

[mattrpav]

Yes, a base class that contains the lookup method to unwrap the destination to get to the root (real) destination that can be used by VirtualTopic and MappedQueueFilter.

ie BaseVirtualDestination(Inteceptor|Filter)

[Nikita-Shupletsov]

Yes, a base class that contains the lookup method to unwrap the destination to get to the root (real) destination that can be used by VirtualTopic and MappedQueueFilter.

ie BaseVirtualDestination(Inteceptor|Filter)

sounds good! done