From e39d6248323d7e55ebc26b4b2d02db6dae029fda Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 5 Jan 2015 13:55:07 +0100 Subject: [PATCH] DATAREST-427 - Refactorings in UriToEntityConverter. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of using a DomainClassConverter directly we now use a raw ConversionService in UriToEntityConverter. This allos us to get rid off the bean definitions for UriToEntityConverter and DomainClassConverter. The population of the ConversionService is now taken care of by calling SpringDataWebConfiguration's addFormatter(…) in defaultConversionService(). Added unit tests for UriToEntityConverter. --- .../data/rest/core/UriToEntityConverter.java | 28 ++-- .../data/rest/core/RepositoryTestsConfig.java | 13 +- .../core/UriToEntityConverterUnitTests.java | 141 ++++++++++++++++++ .../RepositoryRestMvcConfiguration.java | 22 +-- .../webmvc/json/RepositoryTestsConfig.java | 30 ++-- 5 files changed, 175 insertions(+), 59 deletions(-) create mode 100644 spring-data-rest-core/src/test/java/org/springframework/data/rest/core/UriToEntityConverterUnitTests.java diff --git a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/UriToEntityConverter.java b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/UriToEntityConverter.java index a1c439556..eb9f32602 100644 --- a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/UriToEntityConverter.java +++ b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/UriToEntityConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2014 the original author or authors. + * Copyright 2012-2015 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. @@ -21,8 +21,10 @@ import java.util.HashSet; import java.util.Set; import org.springframework.core.convert.ConversionFailedException; +import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.converter.ConditionalGenericConverter; +import org.springframework.core.convert.converter.GenericConverter; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.context.PersistentEntities; import org.springframework.data.repository.support.DomainClassConverter; @@ -30,7 +32,7 @@ import org.springframework.data.util.TypeInformation; import org.springframework.util.Assert; /** - * A {@link ConditionalGenericConverter} that can convert a {@link URI} domain entity. + * A {@link GenericConverter} that can convert a {@link URI} domain entity. * * @author Jon Brisbin * @author Oliver Gierke @@ -41,30 +43,36 @@ public class UriToEntityConverter implements ConditionalGenericConverter { private static final TypeDescriptor STRING_TYPE = TypeDescriptor.valueOf(String.class); private final PersistentEntities entities; - private final DomainClassConverter domainClassConverter; private final Set convertiblePairs; + private final ConversionService conversionService; /** * Creates a new {@link UriToEntityConverter} using the given {@link PersistentEntities} and * {@link DomainClassConverter}. * * @param entities must not be {@literal null}. - * @param domainClassConverter must not be {@literal null}. + * @param conversionService must not be {@literal null}. */ - public UriToEntityConverter(PersistentEntities entities, DomainClassConverter domainClassConverter) { + public UriToEntityConverter(PersistentEntities entities, ConversionService conversionService) { Assert.notNull(entities, "PersistentEntities must not be null!"); - Assert.notNull(domainClassConverter, "DomainClassConverter must not be null!"); + Assert.notNull(conversionService, "ConversionService must not be null!"); Set convertiblePairs = new HashSet(); for (TypeInformation domainType : entities.getManagedTypes()) { - convertiblePairs.add(new ConvertiblePair(URI.class, domainType.getType())); + + Class rawType = domainType.getType(); + PersistentEntity entity = entities.getPersistentEntity(rawType); + + if (entity.hasIdProperty()) { + convertiblePairs.add(new ConvertiblePair(URI.class, domainType.getType())); + } } this.convertiblePairs = Collections.unmodifiableSet(convertiblePairs); this.entities = entities; - this.domainClassConverter = domainClassConverter; + this.conversionService = conversionService; } /* @@ -73,7 +81,7 @@ public class UriToEntityConverter implements ConditionalGenericConverter { */ @Override public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { - return domainClassConverter.matches(URI_TYPE, targetType); + return conversionService.canConvert(STRING_TYPE, targetType); } /* @@ -107,6 +115,6 @@ public class UriToEntityConverter implements ConditionalGenericConverter { "Cannot resolve URI " + uri + ". Is it local or remote? Only local URIs are resolvable.")); } - return domainClassConverter.convert(parts[parts.length - 1], STRING_TYPE, targetType); + return conversionService.convert(parts[parts.length - 1], targetType.getType()); } } diff --git a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/RepositoryTestsConfig.java b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/RepositoryTestsConfig.java index 0ebf74435..6874732fa 100644 --- a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/RepositoryTestsConfig.java +++ b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/RepositoryTestsConfig.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2014 the original author or authors. + * Copyright 2012-2015 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. @@ -25,7 +25,6 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mapping.context.PersistentEntities; -import org.springframework.data.repository.support.DomainClassConverter; import org.springframework.data.repository.support.Repositories; import org.springframework.data.rest.core.config.RepositoryRestConfiguration; import org.springframework.data.rest.core.domain.jpa.ConfiguredPersonRepository; @@ -71,18 +70,8 @@ public class RepositoryTestsConfig { return new DefaultFormattingConversionService(); } - @Bean - public DomainClassConverter domainClassConverter() { - return new DomainClassConverter(defaultConversionService()); - } - @Bean public PersistentEntities persistentEntities() { return new PersistentEntities(mappingContexts); } - - @Bean - public UriToEntityConverter uriToEntityConverter() { - return new UriToEntityConverter(persistentEntities(), domainClassConverter()); - } } diff --git a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/UriToEntityConverterUnitTests.java b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/UriToEntityConverterUnitTests.java new file mode 100644 index 000000000..f15392216 --- /dev/null +++ b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/UriToEntityConverterUnitTests.java @@ -0,0 +1,141 @@ +/* + * Copyright 2015 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.rest.core; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + +import java.net.URI; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.core.convert.ConversionFailedException; +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair; +import org.springframework.data.mapping.context.PersistentEntities; +import org.springframework.data.mongodb.core.mapping.MongoMappingContext; + +/** + * Unit tests for {@link UriToEntityConverter}. + * + * @author Oliver Gierke + */ +@RunWith(MockitoJUnitRunner.class) +public class UriToEntityConverterUnitTests { + + static final TypeDescriptor URI_TYPE = TypeDescriptor.valueOf(URI.class); + static final TypeDescriptor STRING_TYPE = TypeDescriptor.valueOf(String.class); + static final TypeDescriptor ENTITY_TYPE = TypeDescriptor.valueOf(Entity.class); + + @Mock ConversionService conversionService; + + MongoMappingContext context; + UriToEntityConverter converter; + + @Before + @SuppressWarnings("unchecked") + public void setUp() { + + this.context = new MongoMappingContext(); + this.context.setInitialEntitySet(new HashSet>(Arrays.asList(Entity.class, NonEntity.class))); + this.context.afterPropertiesSet(); + + this.converter = new UriToEntityConverter(new PersistentEntities(Arrays.asList(this.context)), conversionService); + } + + /** + * @see DATAREST-427 + */ + @Test + public void supportsOnlyEntitiesWithIdProperty() { + + Set result = converter.getConvertibleTypes(); + + assertThat(result, hasItem(new ConvertiblePair(URI.class, Entity.class))); + assertThat(result, not(hasItem(new ConvertiblePair(URI.class, NonEntity.class)))); + } + + /** + * @see DATAREST-427 + */ + @Test + public void cannotConvertEntityWithIdPropertyIfStringConversionMissing() { + assertThat(converter.matches(URI_TYPE, ENTITY_TYPE), is(false)); + } + + /** + * @see DATAREST-427 + */ + @Test + public void canConvertEntityWithIdPropertyAndFromStringConversionPossible() { + + when(conversionService.canConvert(STRING_TYPE, ENTITY_TYPE)).thenReturn(true); + + assertThat(converter.matches(URI_TYPE, ENTITY_TYPE), is(true)); + } + + /** + * @see DATAREST-427 + */ + @Test + public void cannotConvertEntityWithoutIdentifier() { + assertThat(converter.matches(URI_TYPE, TypeDescriptor.valueOf(NonEntity.class)), is(false)); + } + + /** + * @see DATAREST-427 + */ + @Test + public void invokesConverterWithLastUriPathSegment() { + + Entity reference = new Entity(); + when(conversionService.convert("1", Entity.class)).thenReturn(reference); + + assertThat(converter.convert(URI.create("/foo/bar/1"), URI_TYPE, ENTITY_TYPE), is((Object) reference)); + } + + /** + * @see DATAREST-427 + */ + @Test(expected = ConversionFailedException.class) + public void rejectsUnknownType() { + converter.convert(URI.create("/foo/1"), URI_TYPE, STRING_TYPE); + } + + /** + * @see DATAREST-427 + */ + @Test(expected = ConversionFailedException.class) + public void rejectsUriWithLessThanTwoSegments() { + converter.convert(URI.create("1"), URI_TYPE, ENTITY_TYPE); + } + + static class Entity { + String id; + } + + static class NonEntity { + String value; + } +} diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java index 79d6e4b06..f4c5c8aba 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java @@ -43,15 +43,10 @@ import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.convert.support.ConfigurableConversionService; import org.springframework.core.io.ClassPathResource; import org.springframework.data.domain.PageRequest; -import org.springframework.data.geo.Distance; import org.springframework.data.geo.GeoModule; -import org.springframework.data.geo.Point; -import org.springframework.data.geo.format.DistanceFormatter; -import org.springframework.data.geo.format.PointFormatter; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mapping.context.PersistentEntities; import org.springframework.data.repository.support.DefaultRepositoryInvokerFactory; -import org.springframework.data.repository.support.DomainClassConverter; import org.springframework.data.repository.support.Repositories; import org.springframework.data.repository.support.RepositoryInvokerFactory; import org.springframework.data.rest.core.UriToEntityConverter; @@ -191,16 +186,6 @@ public class RepositoryRestMvcConfiguration extends HateoasAwareSpringDataWebCon return conversionService; } - @Bean - public DomainClassConverter domainClassConverter() { - return new DomainClassConverter(defaultConversionService()); - } - - @Bean - public UriToEntityConverter uriToEntityConverter() { - return new UriToEntityConverter(persistentEntities(), domainClassConverter()); - } - /** * {@link org.springframework.context.ApplicationListener} implementation for invoking * {@link org.springframework.validation.Validator} instances assigned to specific domain types. @@ -534,8 +519,11 @@ public class RepositoryRestMvcConfiguration extends HateoasAwareSpringDataWebCon * @return */ private Module persistentEntityJackson2Module() { - return new PersistentEntityJackson2Module(resourceMappings(), persistentEntities(), config(), - uriToEntityConverter()); + + PersistentEntities entities = persistentEntities(); + + return new PersistentEntityJackson2Module(resourceMappings(), entities, config(), new UriToEntityConverter( + entities, defaultConversionService())); } /** diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/RepositoryTestsConfig.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/RepositoryTestsConfig.java index 7f2c17b39..72b63cf8d 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/RepositoryTestsConfig.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/RepositoryTestsConfig.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2014 the original author or authors. + * Copyright 2012-2015 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. @@ -15,7 +15,6 @@ */ package org.springframework.data.rest.webmvc.json; -import java.net.URI; import java.util.Collections; import java.util.List; @@ -36,6 +35,7 @@ import org.springframework.data.rest.webmvc.jpa.Person; import org.springframework.data.rest.webmvc.jpa.PersonRepository; import org.springframework.data.rest.webmvc.mongodb.MongoDbRepositoryConfig; import org.springframework.format.support.DefaultFormattingConversionService; +import org.springframework.format.support.FormattingConversionService; import org.springframework.hateoas.RelProvider; import org.springframework.hateoas.core.EvoInflectorRelProvider; import org.springframework.hateoas.hal.Jackson2HalModule; @@ -69,27 +69,22 @@ public class RepositoryTestsConfig { config.setResourceMappingForDomainType(Person.class).setRel("person"); - // config.setResourceMappingForRepository(ConfiguredPersonRepository.class) - // .setRel("people") - // .setPath("people") - // .setExported(false); - config.setResourceMappingForRepository(PersonRepository.class).setRel("people").setPath("people") .addResourceMappingFor("findByFirstName").setRel("firstname").setPath("firstname"); - config.setBaseUri(URI.create("http://localhost:8080")); - return config; } @Bean public DefaultFormattingConversionService defaultConversionService() { - return new DefaultFormattingConversionService(); - } - @Bean - public DomainClassConverter domainClassConverter() { - return new DomainClassConverter(defaultConversionService()); + DefaultFormattingConversionService conversionService = new DefaultFormattingConversionService(); + + DomainClassConverter converter = new DomainClassConverter( + conversionService); + converter.setApplicationContext(appCtx); + + return conversionService; } @Bean @@ -97,15 +92,10 @@ public class RepositoryTestsConfig { return new PersistentEntities(mappingContexts); } - @Bean - public UriToEntityConverter uriToEntityConverter() { - return new UriToEntityConverter(persistentEntities(), domainClassConverter()); - } - @Bean public Module persistentEntityModule() { return new PersistentEntityJackson2Module(new RepositoryResourceMappings(config(), repositories()), - persistentEntities(), config(), uriToEntityConverter()); + persistentEntities(), config(), new UriToEntityConverter(persistentEntities(), defaultConversionService())); } @Bean