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";