diff --git a/vraptor-core/src/main/java/br/com/caelum/vraptor/core/Converters.java b/vraptor-core/src/main/java/br/com/caelum/vraptor/core/Converters.java index 664e7e8c2..ace4319c5 100644 --- a/vraptor-core/src/main/java/br/com/caelum/vraptor/core/Converters.java +++ b/vraptor-core/src/main/java/br/com/caelum/vraptor/core/Converters.java @@ -21,9 +21,11 @@ import br.com.caelum.vraptor.converter.Converter; /** - * Represents a collection of converters.
- * Note: This interface will probably change in the near future to allow - * annotation support. + * Represents a collection of all converters registered by VRaptor. When the application + * starts, the {@link #register(Class)} method is called to register all converters found. + * Before registration the default implementation check the priority of each converter, + * and in the case of multiple converter for the same type, only the converter with highest + * priority will registered. * * @author Guilherme Silveira */ @@ -31,14 +33,19 @@ public interface Converters { /** * Extracts a converter for this specific type. - * - * @param type - * @return */ Converter to(Class type); + /** + * Register a converter. Implementations must guarantee that converters + * are registered always keeping its priority. + * @param converterClass + */ void register(Class> converterClass); + /** + * Returns true if a converter exists for the type. + */ boolean existsFor(Class type); boolean existsTwoWayFor(Class type); diff --git a/vraptor-core/src/main/java/br/com/caelum/vraptor/core/DefaultConverters.java b/vraptor-core/src/main/java/br/com/caelum/vraptor/core/DefaultConverters.java index 732b1faf6..43d83555f 100644 --- a/vraptor-core/src/main/java/br/com/caelum/vraptor/core/DefaultConverters.java +++ b/vraptor-core/src/main/java/br/com/caelum/vraptor/core/DefaultConverters.java @@ -20,7 +20,9 @@ import static com.google.common.base.Preconditions.checkState; import java.util.LinkedList; +import java.util.List; +import javax.annotation.Priority; import javax.enterprise.context.ApplicationScoped; import javax.inject.Inject; @@ -34,15 +36,21 @@ import br.com.caelum.vraptor.converter.Converter; import br.com.caelum.vraptor.ioc.Container; -import com.google.common.base.Predicate; import com.google.common.base.Supplier; -import com.google.common.collect.FluentIterable; +/** + * Default implementation for {@link Converters}. + * + * @author Guilherme Silveira + * @author Rodrigo Turini + * @author Lucas Cavalcanti + * @author Otávio Scherer Garcia + */ @ApplicationScoped public class DefaultConverters implements Converters { private final Logger logger = LoggerFactory.getLogger(DefaultConverters.class); - private final LinkedList>> classes = new LinkedList<>(); + private final List>> classes = new LinkedList<>(); @LRU private final CacheStore, Class>> cache; @@ -67,55 +75,80 @@ public void register(Class> converterClass) { Convert type = converterClass.getAnnotation(Convert.class); checkState(type != null, "The converter type %s should have the Convert annotation", converterClass.getName()); + Class> currentConverter = findConverterType(type.value()); + if (!currentConverter.equals(NullConverter.class)) { + int priority = getConverterPriority(converterClass); + int priorityCurrent = getConverterPriority(currentConverter); + + checkState(priority != priorityCurrent, "Converter %s have same priority than %s", converterClass, currentConverter); + + if (priority > priorityCurrent) { + logger.debug("Overriding converter {} with {} because have more priority", currentConverter, converterClass); + classes.remove(currentConverter); + classes.add(converterClass); + } else { + logger.debug("Converter {} not registered because have less priority than {}", converterClass, currentConverter); + } + } + logger.debug("adding converter {} to {}", converterClass, type.value()); classes.add(converterClass); } + private int getConverterPriority(Class> converter) { + Priority priority = converter.getAnnotation(Priority.class); + return priority == null ? 0 : priority.value(); + } + @SuppressWarnings("unchecked") @Override public Converter to(Class clazz) { - Class> converterType = findConverterType(clazz); + Class> converterType = findConverterTypeFromCache(clazz); checkState(!converterType.equals(NullConverter.class), "Unable to find converter for %s", clazz.getName()); + logger.debug("found converter {} to {}", converterType.getName(), clazz.getName()); return (Converter) container.instanceFor(converterType); } - private Class> findConverterType(final Class clazz) { + private Class> findConverterTypeFromCache(final Class clazz) { return cache.fetch(clazz, new Supplier>>() { + @Override public Class> get() { - return FluentIterable.from(classes).filter(matchConverter(clazz)) - .first().or(NullConverter.class); + return findConverterType(clazz); } + }); } - private Predicate> matchConverter(final Class clazz) { - return new Predicate>() { - @Override - public boolean apply(Class input) { - Class boundType = input.getAnnotation(Convert.class).value(); - return boundType.isAssignableFrom(clazz); + private Class> findConverterType(final Class clazz) { + for (Class> current : classes) { + Class boundType = current.getAnnotation(Convert.class).value(); + if (boundType.isAssignableFrom(clazz)) { + return current; } - }; + } + + logger.debug("Unable to find a converter for {}. Returning NullConverter.", clazz); + return NullConverter.class; } private interface NullConverter extends Converter {}; @Override public boolean existsFor(Class type) { - return !findConverterType(type).equals(NullConverter.class); + return !findConverterTypeFromCache(type).equals(NullConverter.class); } @Override public boolean existsTwoWayFor(Class type) { - return TwoWayConverter.class.isAssignableFrom(findConverterType(type)); + return TwoWayConverter.class.isAssignableFrom(findConverterTypeFromCache(type)); } @Override public TwoWayConverter twoWayConverterFor(Class type) { checkState(existsTwoWayFor(type), "Unable to find two way converter for %s", type.getName()); - return (TwoWayConverter) container.instanceFor(findConverterType(type)); + return (TwoWayConverter) container.instanceFor(findConverterTypeFromCache(type)); } } diff --git a/vraptor-core/src/test/java/br/com/caelum/vraptor/core/DefaultConvertersTest.java b/vraptor-core/src/test/java/br/com/caelum/vraptor/core/DefaultConvertersTest.java index 675bff7cf..19de46b9d 100644 --- a/vraptor-core/src/test/java/br/com/caelum/vraptor/core/DefaultConvertersTest.java +++ b/vraptor-core/src/test/java/br/com/caelum/vraptor/core/DefaultConvertersTest.java @@ -17,11 +17,15 @@ package br.com.caelum.vraptor.core; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.typeCompatibleWith; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.when; +import javax.annotation.Priority; +import javax.interceptor.Interceptor; + import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -66,6 +70,27 @@ public void convertingANonAnnotatedConverterEndsUpComplaining() { converters.register(WrongConverter.class); } + @Test + public void shouldChooseConverterWithGreaterPriority() { + converters.register(MyConverter.class); + converters.register(MySecondConverter.class); + + when(container.instanceFor(MyConverter.class)).thenReturn(new MyConverter()); + when(container.instanceFor(MySecondConverter.class)).thenReturn(new MySecondConverter()); + + Object converter = converters.to(MyData.class); + assertThat(converter, instanceOf(MySecondConverter.class)); + } + + @Test + public void shouldForbidConverterWithSamePriority() { + exception.expect(IllegalStateException.class); + exception.expectMessage(String.format("Converter %s have same priority than %s", MyThirdConverter.class, MySecondConverter.class)); + + converters.register(MySecondConverter.class); + converters.register(MyThirdConverter.class); + } + class WrongConverter implements Converter { @Override @@ -78,7 +103,17 @@ class MyData { } @Convert(MyData.class) - class MyConverter implements Converter { + @Priority(Interceptor.Priority.LIBRARY_BEFORE) + private class MyConverter implements Converter { + @Override + public MyData convert(String value, Class type) { + return null; + } + } + + @Convert(MyData.class) + @Priority(javax.interceptor.Interceptor.Priority.APPLICATION) + private class MySecondConverter implements Converter { @Override public MyData convert(String value, Class type) { return null; @@ -86,7 +121,8 @@ public MyData convert(String value, Class type) { } @Convert(MyData.class) - class MySecondConverter implements Converter { + @Priority(javax.interceptor.Interceptor.Priority.APPLICATION) + private class MyThirdConverter implements Converter { @Override public MyData convert(String value, Class type) { return null;