Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

Fix for Issue #80 : Detect annotations on interfaces #85

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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>() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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 {
Expand All @@ -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();

}
Expand Down