From 0fb31c5e3610b897c09dbc23fe0240cc50a4f87e Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 19 Jan 2018 15:32:38 -0500 Subject: [PATCH] Refine "." separator support for STOMP messaging After this commit DefaultUserDestinationResolves no longer looks at whether AntPathMatcher is configured with "." as separator and rather expects to be explicitly told whether to keep the leading slash in translated destinations which actually depends on what the message broker supports (e.g. RabbitMQ "/", Artemis ".") or how it is configured (simple broker could be either way). There is also a minor improvement in SimpMessagingTemplate to ensure user destinations are correctly formed based on what the DefaultUserDestinationResolver expects. When using "." as separtor it allows sending messages to "queue.q1" rather than "/queue.q1". Issue: SPR-16275 --- .../messaging/simp/SimpMessagingTemplate.java | 1 + .../AbstractMessageBrokerConfiguration.java | 16 ++- .../user/DefaultUserDestinationResolver.java | 38 +++++- .../MessageBrokerConfigurationTests.java | 123 +++++++++++++++++- .../DefaultUserDestinationResolverTests.java | 17 +-- 5 files changed, 174 insertions(+), 21 deletions(-) diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/SimpMessagingTemplate.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/SimpMessagingTemplate.java index c4b0ae61ffe..7d7ac067784 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/SimpMessagingTemplate.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/SimpMessagingTemplate.java @@ -225,6 +225,7 @@ public class SimpMessagingTemplate extends AbstractMessageSendingTemplate prefixes = handler.getDestinationPrefixes(); + if (!prefixes.isEmpty() && !prefixes.iterator().next().startsWith("/")) { + ((DefaultUserDestinationResolver) userDestinationResolver()).setRemoveLeadingSlash(true); + } } @Bean @@ -310,6 +322,7 @@ public abstract class AbstractMessageBrokerConfiguration implements ApplicationC subscriptions.put(destination, userRegistryMessageHandler()); } handler.setSystemSubscriptions(subscriptions); + updateUserDestinationResolver(handler); return handler; } @@ -396,7 +409,6 @@ public abstract class AbstractMessageBrokerConfiguration implements ApplicationC if (prefix != null) { resolver.setUserDestinationPrefix(prefix); } - resolver.setPathMatcher(getPathMatcher()); return resolver; } diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolver.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolver.java index 065a16982a7..62735c48909 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolver.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolver.java @@ -59,7 +59,7 @@ public class DefaultUserDestinationResolver implements UserDestinationResolver { private String prefix = "/user/"; - private boolean keepLeadingSlash = true; + private boolean removeLeadingSlash = false; /** @@ -98,6 +98,29 @@ public class DefaultUserDestinationResolver implements UserDestinationResolver { return this.prefix; } + /** + * Use this property to indicate whether the leading slash from translated + * user destinations should be removed or not. This depends on the + * destination prefixes the message broker is configured with. + *

By default this is set to {@code false}, i.e. "do not change the + * target destination", although + * {@link org.springframework.messaging.simp.config.AbstractMessageBrokerConfiguration + * AbstractMessageBrokerConfiguration} may change that to {@code true} if + * the configured destinations do not have a leading slash. + * @param remove whether to remove the leading slash + * @since 4.3.14 + */ + public void setRemoveLeadingSlash(boolean remove) { + this.removeLeadingSlash = remove; + } + + /** + * Whether to remove the leading slash from target destinations. + */ + public boolean isRemoveLeadingSlash() { + return this.removeLeadingSlash; + } + /** * Provide the {@code PathMatcher} in use for working with destinations * which in turn helps to determine whether the leading slash should be @@ -111,11 +134,14 @@ public class DefaultUserDestinationResolver implements UserDestinationResolver { * jms.queue.position-updates}. * @param pathMatcher the PathMatcher used to work with destinations * @since 4.3 + * @deprecated as of 4.3.14 this property is no longer used and is replaced + * by {@link #setRemoveLeadingSlash(boolean)} that indicates more explicitly + * whether to keep the leading slash which may or may not be the case + * regardless of how the {@code PathMatcher} is configured. */ + @Deprecated public void setPathMatcher(@Nullable PathMatcher pathMatcher) { - if (pathMatcher != null) { - this.keepLeadingSlash = pathMatcher.combine("1", "2").equals("1/2"); - } + // Do nothing } @@ -171,7 +197,7 @@ public class DefaultUserDestinationResolver implements UserDestinationResolver { } int prefixEnd = this.prefix.length() - 1; String actualDestination = sourceDestination.substring(prefixEnd); - if (!this.keepLeadingSlash) { + if (isRemoveLeadingSlash()) { actualDestination = actualDestination.substring(1); } Principal principal = SimpMessageHeaderAccessor.getUser(headers); @@ -199,7 +225,7 @@ public class DefaultUserDestinationResolver implements UserDestinationResolver { sessionIds = getSessionIdsByUser(userName, sessionId); } - if (!this.keepLeadingSlash) { + if (isRemoveLeadingSlash()) { actualDest = actualDest.substring(1); } return new ParseResult(sourceDest, actualDest, subscribeDest, sessionIds, userName); diff --git a/spring-messaging/src/test/java/org/springframework/messaging/simp/config/MessageBrokerConfigurationTests.java b/spring-messaging/src/test/java/org/springframework/messaging/simp/config/MessageBrokerConfigurationTests.java index 96ef04d6e76..08cee662940 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/simp/config/MessageBrokerConfigurationTests.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/simp/config/MessageBrokerConfigurationTests.java @@ -17,15 +17,16 @@ package org.springframework.messaging.simp.config; import java.util.ArrayList; +import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import org.hamcrest.Matchers; import org.junit.Test; -import org.springframework.beans.DirectFieldAccessor; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; @@ -33,6 +34,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.support.StaticApplicationContext; import org.springframework.lang.Nullable; import org.springframework.messaging.Message; +import org.springframework.messaging.MessageChannel; import org.springframework.messaging.MessageHandler; import org.springframework.messaging.converter.ByteArrayMessageConverter; import org.springframework.messaging.converter.CompositeMessageConverter; @@ -45,7 +47,9 @@ import org.springframework.messaging.handler.annotation.MessageMapping; import org.springframework.messaging.handler.annotation.SendTo; import org.springframework.messaging.handler.invocation.HandlerMethodArgumentResolver; import org.springframework.messaging.handler.invocation.HandlerMethodReturnValueHandler; +import org.springframework.messaging.simp.SimpMessageHeaderAccessor; import org.springframework.messaging.simp.SimpMessageType; +import org.springframework.messaging.simp.SimpMessagingTemplate; import org.springframework.messaging.simp.annotation.SubscribeMapping; import org.springframework.messaging.simp.annotation.support.SimpAnnotationMethodMessageHandler; import org.springframework.messaging.simp.broker.DefaultSubscriptionRegistry; @@ -414,7 +418,7 @@ public class MessageBrokerConfigurationTests { DefaultUserDestinationResolver resolver = context.getBean(DefaultUserDestinationResolver.class); assertNotNull(resolver); - assertEquals(false, new DirectFieldAccessor(resolver).getPropertyValue("keepLeadingSlash")); + assertEquals(false, resolver.isRemoveLeadingSlash()); } @Test @@ -462,6 +466,67 @@ public class MessageBrokerConfigurationTests { assertNotEquals(UserRegistryMessageHandler.class, messageHandler.getClass()); } + @Test // SPR-16275 + public void dotSeparatorWithBrokerSlashConvention() { + ApplicationContext context = loadConfig(DotSeparatorWithSlashBrokerConventionConfig.class); + testDotSeparator(context, true); + } + + @Test // SPR-16275 + public void dotSeparatorWithBrokerDotConvention() { + ApplicationContext context = loadConfig(DotSeparatorWithDotBrokerConventionConfig.class); + testDotSeparator(context, false); + } + + private void testDotSeparator(ApplicationContext context, boolean expectLeadingSlash) { + MessageChannel inChannel = context.getBean("clientInboundChannel", MessageChannel.class); + TestChannel outChannel = context.getBean("clientOutboundChannel", TestChannel.class); + MessageChannel brokerChannel = context.getBean("brokerChannel", MessageChannel.class); + + + // 1. Subscribe to user destination + + StompHeaderAccessor headers = StompHeaderAccessor.create(StompCommand.SUBSCRIBE); + headers.setSessionId("sess1"); + headers.setSubscriptionId("subs1"); + headers.setDestination("/user/queue.q1"); + Message message = MessageBuilder.createMessage(new byte[0], headers.getMessageHeaders()); + inChannel.send(message); + + // 2. Send message to user via inboundChannel + + headers = StompHeaderAccessor.create(StompCommand.SEND); + headers.setSessionId("sess1"); + headers.setDestination("/user/sess1/queue.q1"); + message = MessageBuilder.createMessage("123".getBytes(), headers.getMessageHeaders()); + inChannel.send(message); + + assertEquals(1, outChannel.messages.size()); + Message outputMessage = outChannel.messages.remove(0); + headers = StompHeaderAccessor.wrap(outputMessage); + + assertEquals(SimpMessageType.MESSAGE, headers.getMessageType()); + assertEquals(expectLeadingSlash ? "/queue.q1-usersess1" : "queue.q1-usersess1", headers.getDestination()); + assertEquals("123", new String((byte[]) outputMessage.getPayload())); + + + // 3. Send message via broker channel + + SimpMessagingTemplate template = new SimpMessagingTemplate(brokerChannel); + SimpMessageHeaderAccessor accessor = SimpMessageHeaderAccessor.create(); + accessor.setSessionId("sess1"); + template.convertAndSendToUser("sess1", "queue.q1", "456".getBytes(), accessor.getMessageHeaders()); + + assertEquals(1, outChannel.messages.size()); + outputMessage = outChannel.messages.remove(0); + headers = StompHeaderAccessor.wrap(outputMessage); + + assertEquals(SimpMessageType.MESSAGE, headers.getMessageType()); + assertEquals(expectLeadingSlash ? "/queue.q1-usersess1" : "queue.q1-usersess1", headers.getDestination()); + assertEquals("456", new String((byte[]) outputMessage.getPayload())); + + } + private AnnotationConfigApplicationContext loadConfig(Class configClass) { return new AnnotationConfigApplicationContext(configClass); } @@ -578,6 +643,60 @@ public class MessageBrokerConfigurationTests { } + @Configuration + static abstract class BaseDotSeparatorConfig extends BaseTestMessageBrokerConfig { + + @Override + protected void configureMessageBroker(MessageBrokerRegistry registry) { + registry.setPathMatcher(new AntPathMatcher(".")); + } + + @Override + @Bean + public AbstractSubscribableChannel clientInboundChannel() { + // synchronous + return new ExecutorSubscribableChannel(null); + } + + @Override + @Bean + public AbstractSubscribableChannel clientOutboundChannel() { + return new TestChannel(); + } + + @Override + @Bean + public AbstractSubscribableChannel brokerChannel() { + // synchronous + return new ExecutorSubscribableChannel(null); + } + } + + @Configuration + static class DotSeparatorWithSlashBrokerConventionConfig extends BaseDotSeparatorConfig { + + // RabbitMQ-style broker convention for STOMP destinations + + @Override + protected void configureMessageBroker(MessageBrokerRegistry registry) { + super.configureMessageBroker(registry); + registry.enableSimpleBroker("/topic", "/queue"); + } + } + + @Configuration + static class DotSeparatorWithDotBrokerConventionConfig extends BaseDotSeparatorConfig { + + // Artemis-style broker convention for STOMP destinations + + @Override + protected void configureMessageBroker(MessageBrokerRegistry registry) { + super.configureMessageBroker(registry); + registry.enableSimpleBroker("topic.", "queue."); + } + } + + private static class TestChannel extends ExecutorSubscribableChannel { private final List> messages = new ArrayList<>(); diff --git a/spring-messaging/src/test/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolverTests.java b/spring-messaging/src/test/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolverTests.java index 8b82fb8e336..c3f137e1a1f 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolverTests.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/simp/user/DefaultUserDestinationResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,9 +16,6 @@ package org.springframework.messaging.simp.user; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; - import java.security.Principal; import org.junit.Before; @@ -29,9 +26,11 @@ import org.springframework.messaging.simp.SimpMessageHeaderAccessor; import org.springframework.messaging.simp.SimpMessageType; import org.springframework.messaging.simp.TestPrincipal; import org.springframework.messaging.support.MessageBuilder; -import org.springframework.util.AntPathMatcher; import org.springframework.util.StringUtils; +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + /** * Unit tests for * {@link org.springframework.messaging.simp.user.DefaultUserDestinationResolver}. @@ -73,9 +72,7 @@ public class DefaultUserDestinationResolverTests { @Test // SPR-14044 public void handleSubscribeForDestinationWithoutLeadingSlash() { - AntPathMatcher pathMatcher = new AntPathMatcher(); - pathMatcher.setPathSeparator("."); - this.resolver.setPathMatcher(pathMatcher); + this.resolver.setRemoveLeadingSlash(true); TestPrincipal user = new TestPrincipal("joe"); String destination = "/user/jms.queue.call"; @@ -141,9 +138,7 @@ public class DefaultUserDestinationResolverTests { @Test // SPR-14044 public void handleMessageForDestinationWithDotSeparator() { - AntPathMatcher pathMatcher = new AntPathMatcher(); - pathMatcher.setPathSeparator("."); - this.resolver.setPathMatcher(pathMatcher); + this.resolver.setRemoveLeadingSlash(true); TestPrincipal user = new TestPrincipal("joe"); String destination = "/user/joe/jms.queue.call";