Fix for Issue #80 : Detect annotations on interfaces

ryantenney · Aug 26, 2014 · 5e6ba54 · 5e6ba54
1 parent be9d8d3
commit 5e6ba54
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 20 deletions.
diff --git a/src/main/java/com/ryantenney/metrics/spring/AbstractMetricMethodInterceptor.java b/src/main/java/com/ryantenney/metrics/spring/AbstractMetricMethodInterceptor.java
@@ -24,6 +24,7 @@
 import org.aopalliance.intercept.MethodInvocation;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.util.ReflectionUtils;
 import org.springframework.util.ReflectionUtils.MethodCallback;
 import org.springframework.util.ReflectionUtils.MethodFilter;
@@ -65,10 +66,11 @@ public Object invoke(MethodInvocation invocation) throws Throwable {
 
 	@Override
 	public void doWith(Method method) throws IllegalAccessException {
-		final A annotation = method.getAnnotation(annotationClass);
+		final A annotation = AnnotationUtils.findAnnotation(method, annotationClass);
 		if (annotation != null) {
+			Class declaringClass = getAnnotationDeclaringClass(method);
 			final MethodKey methodKey = MethodKey.forMethod(method);
-			final String metricName = buildMetricName(targetClass, method, annotation);
+			final String metricName = buildMetricName(declaringClass, method, annotation);
 			final M metric = buildMetric(metricRegistry, metricName, annotation);
 
 			if (metric != null) {
@@ -81,6 +83,26 @@ public void doWith(Method method) throws IllegalAccessException {
 		}
 	}
 
+	private Class getAnnotationDeclaringClass(Method method) {
+		Class declaringClass = AnnotationUtils.findAnnotationDeclaringClass(annotationClass, targetClass);
+		if (declaringClass == null) {
+			for(Class interfaceClass : targetClass.getInterfaces()) {
+				try {
+					Method interfaceMethod = interfaceClass.getMethod(method.getName(), method.getParameterTypes());
+					Annotation interfaceAnnotation = AnnotationUtils.getAnnotation(interfaceMethod, annotationClass);
+					if (interfaceAnnotation != null) {
+						declaringClass = interfaceClass;
+						break;
+					}
+				}
+				catch (NoSuchMethodException ex) {
+					// Skip this interface - it doesn't have the method...
+				}
+			}
+		}
+		return declaringClass == null ? targetClass : declaringClass;
+	}
+
 	protected abstract String buildMetricName(Class<?> targetClass, Method method, A annotation);
 
 	protected abstract M buildMetric(MetricRegistry metricRegistry, String metricName, A annotation);

diff --git a/src/main/java/com/ryantenney/metrics/spring/AnnotationFilter.java b/src/main/java/com/ryantenney/metrics/spring/AnnotationFilter.java
@@ -23,6 +23,7 @@
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.util.ReflectionUtils.FieldFilter;
 import org.springframework.util.ReflectionUtils.MethodFilter;
 
@@ -69,7 +70,7 @@ public AnnotationFilter(final Class<? extends Annotation> clazz, final int metho
 
 	@Override
 	public boolean matches(Method method) {
-		if (USER_DECLARED_METHODS.matches(method) && method.isAnnotationPresent(clazz)) {
+		if (USER_DECLARED_METHODS.matches(method) && AnnotationUtils.findAnnotation(method, clazz) != null) {
 			if (checkModifiers(method, methodModifiers)) {
 				return true;
 			}

diff --git a/src/main/java/com/ryantenney/metrics/spring/CachedGaugeAnnotationBeanPostProcessor.java b/src/main/java/com/ryantenney/metrics/spring/CachedGaugeAnnotationBeanPostProcessor.java
@@ -18,6 +18,7 @@
 import java.lang.reflect.Method;
 
 import org.springframework.core.Ordered;
+import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.util.ReflectionUtils;
 
 import com.codahale.metrics.MetricRegistry;
@@ -42,7 +43,7 @@ protected void withMethod(final Object bean, String beanName, Class<?> targetCla
 			throw new IllegalStateException("Method " + method.getName() + " is annotated with @CachedGauge but requires parameters.");
 		}
 
-		final CachedGauge annotation = method.getAnnotation(CachedGauge.class);
+		final CachedGauge annotation = AnnotationUtils.findAnnotation(method, CachedGauge.class);
 		final String metricName = Util.forCachedGauge(targetClass, method, annotation);
 
 		metrics.register(metricName, new com.codahale.metrics.CachedGauge<Object>(annotation.timeout(), annotation.timeoutUnit()) {

diff --git a/src/main/java/com/ryantenney/metrics/spring/GaugeMethodAnnotationBeanPostProcessor.java b/src/main/java/com/ryantenney/metrics/spring/GaugeMethodAnnotationBeanPostProcessor.java
@@ -18,6 +18,7 @@
 import java.lang.reflect.Method;
 
 import org.springframework.core.Ordered;
+import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.util.ReflectionUtils;
 
 import com.codahale.metrics.MetricRegistry;
@@ -42,7 +43,7 @@ protected void withMethod(final Object bean, String beanName, Class<?> targetCla
 			throw new IllegalStateException("Method " + method.getName() + " is annotated with @Gauge but requires parameters.");
 		}
 
-		final Gauge annotation = method.getAnnotation(Gauge.class);
+		final Gauge annotation = AnnotationUtils.findAnnotation(method, Gauge.class);
 		final String metricName = Util.forGauge(targetClass, method, annotation);
 
 		metrics.register(metricName, new com.codahale.metrics.Gauge<Object>() {

diff --git a/src/test/java/com/ryantenney/metrics/spring/CovariantReturnTypeTest.java b/src/test/java/com/ryantenney/metrics/spring/CovariantReturnTypeTest.java
@@ -15,6 +15,10 @@
  */
 package com.ryantenney.metrics.spring;
 
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Timer;
+import org.hamcrest.CoreMatchers;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
@@ -28,7 +32,13 @@
 import com.codahale.metrics.annotation.Timed;
 import com.ryantenney.metrics.annotation.Counted;
 
+import java.util.SortedSet;
+
 public class CovariantReturnTypeTest {
+	public static final String COUNTER_NAME = "com.ryantenney.metrics.spring.CovariantReturnTypeTest.MeteredInterface.countedMethod";
+	public static final String EXCEPTION_METER_NAME = "com.ryantenney.metrics.spring.CovariantReturnTypeTest.MeteredInterface.exceptionMeteredMethod.exceptions";
+	public static final String METER_NAME = "com.ryantenney.metrics.spring.CovariantReturnTypeTest.MeteredInterface.meteredMethod";
+	public static final String TIMER_NAME = "com.ryantenney.metrics.spring.CovariantReturnTypeTest.MeteredInterface.timedMethod";
 
 	ClassPathXmlApplicationContext ctx;
 	MetricRegistry metricRegistry;
@@ -45,8 +55,14 @@ public void destroy() throws Throwable {
 	}
 
 	@Test
-	public void noMetricsRegistered() {
-		Assert.assertTrue("No metrics registered", this.metricRegistry.getNames().isEmpty());
+	public void allMetricsRegistered() {
+		SortedSet<String> metricNames = this.metricRegistry.getNames();
+		Assert.assertThat("4 metrics present", metricNames, CoreMatchers.hasItems(
+				COUNTER_NAME,
+				EXCEPTION_METER_NAME,
+				METER_NAME,
+				TIMER_NAME
+		));
 	}
 
 	@Test
@@ -64,13 +80,17 @@ public void testMeteredInterfaceImpl() {
 	@Test
 	public void testTimedMethod() {
 		ctx.getBean(MeteredInterface.class).timedMethod();
-		Assert.assertTrue("No metrics should be registered", this.metricRegistry.getNames().isEmpty());
+		Timer timer = this.metricRegistry.getTimers().get(TIMER_NAME);
+		Assert.assertNotNull("Timer should be registered", timer);
+		Assert.assertEquals("Timer count should be 0", 0L, timer.getCount());
 	}
 
 	@Test
 	public void testMeteredMethod() {
 		ctx.getBean(MeteredInterface.class).meteredMethod();
-		Assert.assertTrue("No metrics should be registered", this.metricRegistry.getNames().isEmpty());
+		Meter meter = this.metricRegistry.getMeters().get(METER_NAME);
+		Assert.assertNotNull("Meter should be registered", meter);
+		Assert.assertEquals("Meter count should be 0", 0L, meter.getCount());
 	}
 
 	@Test(expected = BogusException.class)
@@ -79,15 +99,19 @@ public void testExceptionMeteredMethod() throws Throwable {
 			ctx.getBean(MeteredInterface.class).exceptionMeteredMethod();
 		}
 		catch (Throwable t) {
-			Assert.assertTrue("No metrics should be registered", this.metricRegistry.getNames().isEmpty());
+			Meter meter = this.metricRegistry.getMeters().get(EXCEPTION_METER_NAME);
+			Assert.assertNotNull("Exception Meter should be registered", meter);
+			Assert.assertEquals("Exception Meter count should be 0", 0L, meter.getCount());
 			throw t;
 		}
 	}
 
 	@Test
 	public void testCountedMethod() {
 		ctx.getBean(MeteredInterface.class).countedMethod();
-		Assert.assertTrue("No metrics should be registered", this.metricRegistry.getNames().isEmpty());
+		Counter counter = this.metricRegistry.getCounters().get(COUNTER_NAME);
+		Assert.assertNotNull("Counter should be registered", counter);
+		Assert.assertEquals("Counter count should be 0", 0L, counter.getCount());
 	}
 
 	public interface MeteredInterface {

diff --git a/src/test/java/com/ryantenney/metrics/spring/MeteredInterfaceTest.java b/src/test/java/com/ryantenney/metrics/spring/MeteredInterfaceTest.java
@@ -15,26 +15,33 @@
  */
 package com.ryantenney.metrics.spring;
 
+import com.codahale.metrics.*;
+import org.hamcrest.CoreMatchers;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
 
-import com.codahale.metrics.MetricRegistry;
 import com.codahale.metrics.annotation.ExceptionMetered;
 import com.codahale.metrics.annotation.Metered;
 import com.codahale.metrics.annotation.Timed;
 import com.ryantenney.metrics.annotation.Counted;
 
+import java.util.SortedSet;
+
 /**
  * Purpose of test:
  * Verify that calling a method that is annotated at the interface
  * level but not the implementation level doesn't throw an NPE.
  * Also verifies that it doesn't register any metrics.
  */
 public class MeteredInterfaceTest {
+	public static final String COUNTER_NAME = "com.ryantenney.metrics.spring.MeteredInterfaceTest.MeteredInterface.countedMethod";
+	public static final String EXCEPTION_METER_NAME = "com.ryantenney.metrics.spring.MeteredInterfaceTest.MeteredInterface.exceptionMeteredMethod.exceptions";
+	public static final String METER_NAME = "com.ryantenney.metrics.spring.MeteredInterfaceTest.MeteredInterface.meteredMethod";
+	public static final String TIMER_NAME = "com.ryantenney.metrics.spring.MeteredInterfaceTest.MeteredInterface.timedMethod";
 
 	ClassPathXmlApplicationContext ctx;
 	MetricRegistry metricRegistry;
@@ -51,8 +58,14 @@ public void destroy() throws Throwable {
 	}
 
 	@Test
-	public void noMetricsRegistered() {
-		Assert.assertTrue("No metrics registered", this.metricRegistry.getNames().isEmpty());
+	public void allMetricsRegistered() {
+		SortedSet<String> metricNames = this.metricRegistry.getNames();
+		Assert.assertThat("4 metrics present", metricNames, CoreMatchers.hasItems(
+				COUNTER_NAME,
+				EXCEPTION_METER_NAME,
+				METER_NAME,
+				TIMER_NAME
+				));
 	}
 
 	@Test
@@ -70,13 +83,17 @@ public void testMeteredInterfaceImpl() {
 	@Test
 	public void testTimedMethod() {
 		ctx.getBean(MeteredInterface.class).timedMethod();
-		Assert.assertTrue("No metrics should be registered", this.metricRegistry.getNames().isEmpty());
+		Timer timer = this.metricRegistry.getTimers().get(TIMER_NAME);
+		Assert.assertNotNull("Timer should be registered", timer);
+		Assert.assertEquals("Timer count should be 1", 1L, timer.getCount());
 	}
 
 	@Test
 	public void testMeteredMethod() {
 		ctx.getBean(MeteredInterface.class).meteredMethod();
-		Assert.assertTrue("No metrics should be registered", this.metricRegistry.getNames().isEmpty());
+		Meter meter = this.metricRegistry.getMeters().get(METER_NAME);
+		Assert.assertNotNull("Meter should be registered", meter);
+		Assert.assertEquals("Meter count should be 1", 1L, meter.getCount());
 	}
 
 	@Test(expected = BogusException.class)
@@ -85,15 +102,19 @@ public void testExceptionMeteredMethod() throws Throwable {
 			ctx.getBean(MeteredInterface.class).exceptionMeteredMethod();
 		}
 		catch (Throwable t) {
-			Assert.assertTrue("No metrics should be registered", this.metricRegistry.getNames().isEmpty());
+			Meter meter = this.metricRegistry.getMeters().get(EXCEPTION_METER_NAME);
+			Assert.assertNotNull("Exception Meter should be registered", meter);
+			Assert.assertEquals("Exception Meter count should be 1", 1L, meter.getCount());
 			throw t;
 		}
 	}
 
 	@Test
 	public void testCountedMethod() {
 		ctx.getBean(MeteredInterface.class).countedMethod();
-		Assert.assertTrue("No metrics should be registered", this.metricRegistry.getNames().isEmpty());
+		Counter counter = this.metricRegistry.getCounters().get(COUNTER_NAME);
+		Assert.assertNotNull("Counter should be registered", counter);
+		Assert.assertEquals("Counter count should be 1", 1L, counter.getCount());
 	}
 
 	public interface MeteredInterface {
@@ -104,10 +125,10 @@ public interface MeteredInterface {
 		@Metered
 		public void meteredMethod();
 
-		@ExceptionMetered
+		@ExceptionMetered(cause = BogusException.class)
 		public void exceptionMeteredMethod() throws Throwable;
 
-		@Counted
+		@Counted(monotonic = true)
 		public void countedMethod();
 
 	}