From 5abfa8716ee016f3a601f90c8ad5640e1fd98164 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Mon, 11 Aug 2014 11:25:07 +0200 Subject: [PATCH] DATAREDIS-337 - Improve handling of overridden methods in MessageListenerAdapter. Previously overriding message handler methods of the delegate passed to MessageListenerAdapter resulted in the message handler method called multiple times. We now make sure that a message handler method is only found and invoked once. Original pull request: #95. --- .../adapter/MessageListenerAdapter.java | 83 +++++--- .../listener/adapter/MessageListenerTest.java | 185 +++++++++++++++++- 2 files changed, 241 insertions(+), 27 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/listener/adapter/MessageListenerAdapter.java b/src/main/java/org/springframework/data/redis/listener/adapter/MessageListenerAdapter.java index 1e9b8fb31..1953ccb7f 100644 --- a/src/main/java/org/springframework/data/redis/listener/adapter/MessageListenerAdapter.java +++ b/src/main/java/org/springframework/data/redis/listener/adapter/MessageListenerAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011-2013 the original author or authors. + * Copyright 2011-2014 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. @@ -18,8 +18,8 @@ package org.springframework.data.redis.listener.adapter; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.util.ArrayList; -import java.util.List; +import java.util.HashSet; +import java.util.Set; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -32,6 +32,7 @@ import org.springframework.data.redis.serializer.JdkSerializationRedisSerializer import org.springframework.data.redis.serializer.RedisSerializer; import org.springframework.data.redis.serializer.StringRedisSerializer; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.ReflectionUtils; import org.springframework.util.ReflectionUtils.MethodCallback; @@ -89,26 +90,28 @@ import org.springframework.util.StringUtils; * @author Juergen Hoeller * @author Costin Leau * @author Greg Turnquist + * @author Thomas Darimont + * @author Christoph Strobl * @see org.springframework.jms.listener.adapter.MessageListenerAdapter */ public class MessageListenerAdapter implements InitializingBean, MessageListener { + private class MethodInvoker { + private final Object delegate; private String methodName; - - private List methods; - private boolean lenient = false; + private Set methods; + private boolean lenient; MethodInvoker(Object delegate, final String methodName) { + this.delegate = delegate; this.methodName = methodName; + this.lenient = delegate instanceof MessageListener; + this.methods = new HashSet(); - lenient = delegate instanceof MessageListener; - - Class c = delegate.getClass(); - - methods = new ArrayList(); + final Class c = delegate.getClass(); ReflectionUtils.doWithMethods(c, new MethodCallback() { @@ -117,17 +120,7 @@ public class MessageListenerAdapter implements InitializingBean, MessageListener methods.add(method); } - }, new MethodFilter() { - public boolean matches(Method method) { - if (Modifier.isPublic(method.getModifiers()) && methodName.equals(method.getName())) { - // check out the argument numbers - Class[] parameterTypes = method.getParameterTypes(); - - return ((parameterTypes.length == 2 && String.class.equals(parameterTypes[1])) || parameterTypes.length == 1); - } - return false; - } - }); + }, new MostSpecificMethodFilter(methodName, c)); Assert.isTrue(lenient || !methods.isEmpty(), "Cannot find a suitable method named [" + c.getName() + "#" + methodName + "] - is the method public and has the proper arguments?"); @@ -136,10 +129,22 @@ public class MessageListenerAdapter implements InitializingBean, MessageListener void invoke(Object[] arguments) throws InvocationTargetException, IllegalAccessException { Object[] message = new Object[] { arguments[0] }; + for (Method m : methods) { + Class[] types = m.getParameterTypes(); - Object[] args = (types.length == 2 ? arguments : message); + Object[] args = // + types.length == 2 // + && types[0].isInstance(arguments[0]) // + && types[1].isInstance(arguments[1]) ? arguments : message; + + if (!types[0].isInstance(args[0])) { + continue; + } + m.invoke(delegate, args); + + return; } } @@ -152,7 +157,7 @@ public class MessageListenerAdapter implements InitializingBean, MessageListener return methodName; } } - + /** * Out-of-the-box value for the default listener method: "handleMessage". */ @@ -380,4 +385,34 @@ public class MessageListenerAdapter implements InitializingBean, MessageListener + "' with arguments " + ObjectUtils.nullSafeToString(arguments), ex); } } + + /** + * @since 1.4 + */ + static final class MostSpecificMethodFilter implements MethodFilter { + + private final String methodName; + private final Class c; + + MostSpecificMethodFilter(String methodName, Class c) { + + this.methodName = methodName; + this.c = c; + } + + public boolean matches(Method method) { + + if (Modifier.isPublic(method.getModifiers()) // + && methodName.equals(method.getName()) // + && method.equals(ClassUtils.getMostSpecificMethod(method, c))) { + + // check out the argument numbers + Class[] parameterTypes = method.getParameterTypes(); + + return ((parameterTypes.length == 2 && String.class.equals(parameterTypes[1])) || parameterTypes.length == 1); + } + + return false; + } + } } diff --git a/src/test/java/org/springframework/data/redis/listener/adapter/MessageListenerTest.java b/src/test/java/org/springframework/data/redis/listener/adapter/MessageListenerTest.java index 47f49d1f4..1427572a5 100644 --- a/src/test/java/org/springframework/data/redis/listener/adapter/MessageListenerTest.java +++ b/src/test/java/org/springframework/data/redis/listener/adapter/MessageListenerTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2013 the original author or authors. + * Copyright 2011-2014 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. @@ -17,8 +17,7 @@ package org.springframework.data.redis.listener.adapter; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertSame; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.*; import org.junit.Before; import org.junit.Test; @@ -27,6 +26,8 @@ import org.mockito.MockitoAnnotations; import org.springframework.data.redis.connection.DefaultMessage; import org.springframework.data.redis.connection.Message; import org.springframework.data.redis.connection.MessageListener; +import org.springframework.data.redis.serializer.RedisSerializer; +import org.springframework.data.redis.serializer.SerializationException; import org.springframework.data.redis.serializer.StringRedisSerializer; /** @@ -34,6 +35,7 @@ import org.springframework.data.redis.serializer.StringRedisSerializer; * * @author Costin Leau * @author Greg Turnquist + * @author Thomas Darimont */ public class MessageListenerTest { @@ -156,6 +158,126 @@ public class MessageListenerTest { assertEquals(1, listener.count); } + /** + * @see DATAREDIS-337 + */ + @Test + public void defaultConcreteHandlerMethodShouldOnlyBeInvokedOnce() { + + ConcreteMessageHandler listener = spy(new ConcreteMessageHandler()); + + MessageListenerAdapter adapter = new MessageListenerAdapter(listener); + adapter.afterPropertiesSet(); + + adapter.onMessage(new DefaultMessage("channel1".getBytes(), "body".getBytes()), "".getBytes()); + + verify(listener, times(1)).handleMessage(anyString(), anyString()); + } + + /** + * @see DATAREDIS-337 + */ + @Test + public void defaultConcreteHandlerMethodWithoutSerializerShouldOnlyBeInvokedOnce() { + + ConcreteMessageHandler listener = spy(new ConcreteMessageHandler()); + + MessageListenerAdapter adapter = new MessageListenerAdapter(listener); + adapter.setSerializer(null); + adapter.afterPropertiesSet(); + + adapter.onMessage(new DefaultMessage("channel1".getBytes(), "body".getBytes()), "".getBytes()); + + verify(listener, times(1)).handleMessage(any(byte[].class), anyString()); + } + + /** + * @see DATAREDIS-337 + */ + @Test + public void defaultConcreteHandlerMethodWithCustomSerializerShouldOnlyBeInvokedOnce() { + + ConcreteMessageHandler listener = spy(new ConcreteMessageHandler()); + + MessageListenerAdapter adapter = new MessageListenerAdapter(listener); + adapter.setSerializer(new PojoRedisSerializer()); + adapter.afterPropertiesSet(); + + adapter.onMessage(new DefaultMessage(new byte[0], "body".getBytes()), "".getBytes()); + + verify(listener, times(1)).handleMessage(any(Pojo.class), anyString()); + } + + /** + * @see DATAREDIS-337 + */ + @Test + public void customConcreteHandlerMethodShouldOnlyBeInvokedOnce() { + + ConcreteMessageHandler listener = spy(new ConcreteMessageHandler()); + + MessageListenerAdapter adapter = new MessageListenerAdapter(listener); + adapter.setDefaultListenerMethod("handle"); + adapter.afterPropertiesSet(); + + adapter.onMessage(new DefaultMessage("channel1".getBytes(), "body".getBytes()), "".getBytes()); + + verify(listener, times(1)).handle(anyString(), anyString()); + } + + /** + * @see DATAREDIS-337 + */ + @Test + public void customConcreteMessageOnlyHandlerMethodShouldOnlyBeInvokedOnce() { + + ConcreteMessageHandler listener = spy(new ConcreteMessageHandler()); + + MessageListenerAdapter adapter = new MessageListenerAdapter(listener); + adapter.setDefaultListenerMethod("handleMessageOnly"); + adapter.afterPropertiesSet(); + + adapter.onMessage(new DefaultMessage("channel1".getBytes(), "body".getBytes()), "".getBytes()); + + verify(listener, times(1)).handleMessageOnly(anyString()); + } + + /** + * @see DATAREDIS-337 + */ + @Test + public void customConcreteHandlerMethodWithoutSerializerShouldOnlyBeInvokedOnce() { + + ConcreteMessageHandler listener = spy(new ConcreteMessageHandler()); + + MessageListenerAdapter adapter = new MessageListenerAdapter(listener); + adapter.setDefaultListenerMethod("handle"); + adapter.setSerializer(null); + adapter.afterPropertiesSet(); + + adapter.onMessage(new DefaultMessage("channel1".getBytes(), "body".getBytes()), "".getBytes()); + + verify(listener, times(1)).handle(any(byte[].class), anyString()); + } + + /** + * @see DATAREDIS-337 + */ + @Test + public void customConcreteHandlerMethodWithCustomSerializerShouldOnlyBeInvokedOnce() { + + ConcreteMessageHandler listener = spy(new ConcreteMessageHandler()); + + MessageListenerAdapter adapter = new MessageListenerAdapter(listener); + adapter.setDefaultListenerMethod("handle"); + adapter.setSerializer(new PojoRedisSerializer()); + adapter.afterPropertiesSet(); + + adapter.onMessage(new DefaultMessage(new byte[0], "body".getBytes()), "".getBytes()); + + verify(listener, times(1)).handle(any(Pojo.class), anyString()); + } + class SampleListener implements MessageListener { int count; @@ -164,4 +286,61 @@ public class MessageListenerTest { count++; } } + + /** + * @author Thomas Darimont + * @see DATAREDIS-337 + */ + static class AbstractMessageHandler { + + public void handleMessage(Pojo message, String channel) {} + + public void handleMessage(byte[] message, String channel) {} + + public void handleMessage(String message, String channel) {} + + public void handle(Pojo message, String channel) {} + + public void handle(String message, String channel) {} + + public void handle(byte[] message, String channel) {} + + public void handleMessageOnly(String message) {} + } + + /** + * @author Thomas Darimont + * @see DATAREDIS-337 + */ + static class ConcreteMessageHandler extends AbstractMessageHandler { + + public void handleMessage(Pojo message, String channel) {} + + public void handleMessage(byte[] message, String channel) {} + + public void handleMessage(String message, String channel) {} + + public void handle(Pojo message, String channel) {} + + public void handle(String message, String channel) {} + + public void handle(byte[] message, String channel) {} + + public void handleMessageOnly(String message) {} + } + + static class Pojo {} + + static class PojoRedisSerializer implements RedisSerializer { + + @Override + public byte[] serialize(Pojo t) throws SerializationException { + return new byte[0]; + } + + @Override + public Pojo deserialize(byte[] bytes) throws SerializationException { + return new Pojo(); + } + } }