Properly handle associations in nested entities.

odrotbohm · odrotbohm · commit 1ae7efd0b8df · 2021-04-07T17:22:12.000+02:00
Nested entities that contain a reference to an aggregate root get a link to that attached to their representation. Previously, the creation of those links assumed that the reference is a materialized instance of the remote aggregate. That's now altered to be able to deal with associations, use identifiers directly or materialize to an intermediate aggregate instance to potentially use a custom lookup.
diff --git a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/PersistentPropertyResourceMapping.java b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/PersistentPropertyResourceMapping.java
@@ -90,7 +90,7 @@ public boolean isExported() {
 			return false;
 		}
 
-		ResourceMapping typeMapping = mappings.getMetadataFor(property.getActualType());
+		ResourceMapping typeMapping = mappings.getMetadataFor(property.getAssociationTargetType());
 		return !typeMapping.isExported() ? false : annotation.map(it -> it.exported()).orElse(true);
 	}
 
diff --git a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/support/DefaultSelfLinkProvider.java b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/support/DefaultSelfLinkProvider.java
@@ -17,9 +17,13 @@
 
 import java.util.List;
 
+import org.springframework.core.convert.ConversionService;
+import org.springframework.data.mapping.PersistentEntity;
+import org.springframework.data.mapping.PersistentProperty;
 import org.springframework.data.mapping.context.PersistentEntities;
 import org.springframework.hateoas.Link;
 import org.springframework.hateoas.server.EntityLinks;
+import org.springframework.lang.Nullable;
 import org.springframework.plugin.core.PluginRegistry;
 import org.springframework.util.Assert;
 
@@ -36,6 +40,7 @@ public class DefaultSelfLinkProvider implements SelfLinkProvider {
 	private final PersistentEntities entities;
 	private final EntityLinks entityLinks;
 	private final PluginRegistry<EntityLookup<?>, Class<?>> lookups;
+	private final ConversionService conversionService;
 
 	/**
 	 * Creates a new {@link DefaultSelfLinkProvider} from the {@link PersistentEntities}, {@link EntityLinks} and
@@ -46,7 +51,7 @@ public class DefaultSelfLinkProvider implements SelfLinkProvider {
 	 * @param lookups must not be {@literal null}.
 	 */
 	public DefaultSelfLinkProvider(PersistentEntities entities, EntityLinks entityLinks,
-			List<? extends EntityLookup<?>> lookups) {
+			List<? extends EntityLookup<?>> lookups, ConversionService conversionService) {
 
 		Assert.notNull(entities, "PersistentEntities must not be null!");
 		Assert.notNull(entityLinks, "EntityLinks must not be null!");
@@ -55,6 +60,7 @@ public DefaultSelfLinkProvider(PersistentEntities entities, EntityLinks entityLi
 		this.entities = entities;
 		this.entityLinks = entityLinks;
 		this.lookups = PluginRegistry.of(lookups);
+		this.conversionService = conversionService;
 	}
 
 	/*
@@ -65,29 +71,54 @@ public Link createSelfLinkFor(Object instance) {
 
 		Assert.notNull(instance, "Domain object must not be null!");
 
-		return entityLinks.linkToItemResource(instance.getClass(), getResourceId(instance));
+		return createSelfLinkFor(instance.getClass(), instance);
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.rest.core.support.SelfLinkProvider#createSelfLinkFor(java.lang.Class, java.lang.Object)
+	 */
+	public Link createSelfLinkFor(Class<?> type, Object reference) {
+
+		if (type.isInstance(reference)) {
+			return entityLinks.linkToItemResource(type, getResourceId(type, reference));
+		}
+
+		PersistentEntity<?, ?> entity = entities.getRequiredPersistentEntity(type);
+		PersistentProperty<?> idProperty = entity.getRequiredIdProperty();
+
+		Object identifier = conversionService.convert(reference, idProperty.getType());
+
+		if (lookups.hasPluginFor(type)) {
+			identifier = getResourceId(type, conversionService.convert(identifier, type));
+		}
+
+		return entityLinks.linkToItemResource(type, identifier);
 	}
 
 	/**
 	 * Returns the identifier to be used to create the self link URI.
 	 *
-	 * @param instance must not be {@literal null}.
+	 * @param reference must not be {@literal null}.
 	 * @return
 	 */
-	@SuppressWarnings("unchecked")
-	private Object getResourceId(Object instance) {
+	@Nullable
+	private Object getResourceId(Class<?> type, Object reference) {
 
-		Class<? extends Object> instanceType = instance.getClass();
+		if (!lookups.hasPluginFor(type)) {
+			return entityIdentifierOrNull(reference);
+		}
 
-		return lookups.getPluginFor(instanceType)//
+		return lookups.getPluginFor(type)//
 				.map(it -> it.getClass().cast(it))//
-				.map(it -> it.getResourceIdentifier(instance))//
-				.orElseGet(() -> identifierOrNull(instance));
+				.map(it -> it.getResourceIdentifier(reference))//
+				.orElseGet(() -> entityIdentifierOrNull(reference));
 	}
 
-	private Object identifierOrNull(Object instance) {
+	private Object entityIdentifierOrNull(Object instance) {
 
-		return entities.getRequiredPersistentEntity(instance.getClass())//
-				.getIdentifierAccessor(instance).getIdentifier();
+		return entities.getRequiredPersistentEntity(instance.getClass()) //
+				.getIdentifierAccessor(instance) //
+				.getIdentifier();
 	}
 }
diff --git a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/support/SelfLinkProvider.java b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/support/SelfLinkProvider.java
@@ -27,10 +27,24 @@
 public interface SelfLinkProvider {
 
 	/**
-	 * Returns the self link for the given entity instance.
+	 * Returns the self link for the given entity instance. Only call this with an actual entity instance. Otherwise,
+	 * prefer {@link #createSelfLinkFor(Class, Object)}.
 	 *
 	 * @param instance must never be {@literal null}.
 	 * @return will never be {@literal null}.
+	 * @see #createSelfLinkFor(Class, Object)
 	 */
 	Link createSelfLinkFor(Object instance);
+
+	/**
+	 * Returns the self link for the entity of the given type and the given reference. The latter can be an instance of
+	 * the former, an identifier value of the former or anything that can be converted into an identifier in the first
+	 * place.
+	 *
+	 * @param type must not be {@literal null}.
+	 * @param reference must not be {@literal null}.
+	 * @return will never be {@literal null}.
+	 * @since 3.5
+	 */
+	Link createSelfLinkFor(Class<?> type, Object reference);
 }
diff --git a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/support/DefaultSelfLinkProviderUnitTests.java b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/support/DefaultSelfLinkProviderUnitTests.java
@@ -28,7 +28,8 @@
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
-
+import org.springframework.core.convert.ConversionService;
+import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.data.keyvalue.core.mapping.context.KeyValueMappingContext;
 import org.springframework.data.mapping.MappingException;
 import org.springframework.data.mapping.context.PersistentEntities;
@@ -51,6 +52,7 @@ public class DefaultSelfLinkProviderUnitTests {
 	@Mock EntityLinks entityLinks;
 	PersistentEntities entities;
 	List<EntityLookup<?>> lookups;
+	ConversionService conversionService;
 
 	@Before
 	public void setUp() {
@@ -69,22 +71,23 @@ public void setUp() {
 
 		this.entities = new PersistentEntities(Arrays.asList(context));
 		this.lookups = Collections.emptyList();
-		this.provider = new DefaultSelfLinkProvider(entities, entityLinks, lookups);
+		this.conversionService = new DefaultConversionService();
+		this.provider = new DefaultSelfLinkProvider(entities, entityLinks, lookups, conversionService);
 	}
 
 	@Test(expected = IllegalArgumentException.class) // DATAREST-724
 	public void rejectsNullEntities() {
-		new DefaultSelfLinkProvider(null, entityLinks, lookups);
+		new DefaultSelfLinkProvider(null, entityLinks, lookups, conversionService);
 	}
 
 	@Test(expected = IllegalArgumentException.class) // DATAREST-724
 	public void rejectsNullEntityLinks() {
-		new DefaultSelfLinkProvider(entities, null, lookups);
+		new DefaultSelfLinkProvider(entities, null, lookups, conversionService);
 	}
 
 	@Test(expected = IllegalArgumentException.class) // DATAREST-724
 	public void rejectsNullEntityLookups() {
-		new DefaultSelfLinkProvider(entities, entityLinks, null);
+		new DefaultSelfLinkProvider(entities, entityLinks, null, conversionService);
 	}
 
 	@Test // DATAREST-724
@@ -104,7 +107,8 @@ public void usesEntityLookupIfDefined() {
 		when(lookup.supports(Profile.class)).thenReturn(true);
 		when(lookup.getResourceIdentifier(any(Profile.class))).thenReturn("foo");
 
-		this.provider = new DefaultSelfLinkProvider(entities, entityLinks, Collections.singletonList(lookup));
+		this.provider = new DefaultSelfLinkProvider(entities, entityLinks, Collections.singletonList(lookup),
+				conversionService);
 
 		Link link = provider.createSelfLinkFor(new Profile("Name", "Type"));
 
@@ -114,7 +118,8 @@ public void usesEntityLookupIfDefined() {
 	@Test // DATAREST-724, DATAREST-1549
 	public void rejectsLinkCreationForUnknownEntity() {
 
-		assertThatExceptionOfType(MappingException.class).isThrownBy(() -> provider.createSelfLinkFor(new Object())) //
+		assertThatExceptionOfType(MappingException.class) //
+				.isThrownBy(() -> provider.createSelfLinkFor(new Object())) //
 				.withMessageContaining(Object.class.getName()) //
 				.withMessageContaining("Couldn't find PersistentEntity for");
 	}
diff --git a/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/AbstractControllerIntegrationTests.java b/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/AbstractControllerIntegrationTests.java
@@ -23,6 +23,7 @@
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Import;
+import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.data.mapping.PersistentEntity;
 import org.springframework.data.mapping.context.PersistentEntities;
 import org.springframework.data.repository.support.Repositories;
@@ -66,7 +67,8 @@ public static class TestConfiguration {
 		public PersistentEntityResourceAssembler persistentEntityResourceAssembler(PersistentEntities entities,
 				EntityLinks entityLinks, Associations associations) {
 
-			SelfLinkProvider selfLinkProvider = new DefaultSelfLinkProvider(entities, entityLinks, Collections.emptyList());
+			SelfLinkProvider selfLinkProvider = new DefaultSelfLinkProvider(entities, entityLinks, Collections.emptyList(),
+					new DefaultConversionService());
 
 			return new PersistentEntityResourceAssembler(entities, StubProjector.INSTANCE, associations,
 					selfLinkProvider);
diff --git a/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/RepositoryTestsConfig.java b/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/RepositoryTestsConfig.java
@@ -24,6 +24,7 @@
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.data.mapping.context.MappingContext;
 import org.springframework.data.mapping.context.PersistentEntities;
 import org.springframework.data.repository.support.DefaultRepositoryInvokerFactory;
@@ -112,7 +113,7 @@ public Module persistentEntityModule() {
 		EntityLinks entityLinks = new RepositoryEntityLinks(repositories(), mappings, config(),
 				mock(PagingAndSortingTemplateVariables.class), PluginRegistry.of(DefaultIdConverter.INSTANCE));
 		SelfLinkProvider selfLinkProvider = new DefaultSelfLinkProvider(persistentEntities(), entityLinks,
-				Collections.<EntityLookup<?>> emptyList());
+				Collections.<EntityLookup<?>> emptyList(), new DefaultConversionService());
 
 		DefaultRepositoryInvokerFactory invokerFactory = new DefaultRepositoryInvokerFactory(repositories());
 		UriToEntityConverter uriToEntityConverter = new UriToEntityConverter(persistentEntities(), invokerFactory,
diff --git a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/json/RepositoryTestsConfig.java b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/json/RepositoryTestsConfig.java
@@ -24,6 +24,7 @@
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.data.mapping.context.MappingContext;
 import org.springframework.data.mapping.context.PersistentEntities;
 import org.springframework.data.repository.support.DefaultRepositoryInvokerFactory;
@@ -36,7 +37,6 @@
 import org.springframework.data.rest.core.config.RepositoryRestConfiguration;
 import org.springframework.data.rest.core.mapping.RepositoryResourceMappings;
 import org.springframework.data.rest.core.support.DefaultSelfLinkProvider;
-import org.springframework.data.rest.core.support.EntityLookup;
 import org.springframework.data.rest.core.support.SelfLinkProvider;
 import org.springframework.data.rest.webmvc.EmbeddedResourcesAssembler;
 import org.springframework.data.rest.webmvc.jpa.Person;
@@ -120,7 +120,7 @@ public Module persistentEntityModule() {
 		EntityLinks entityLinks = new RepositoryEntityLinks(repositories(), mappings, config(),
 				mock(PagingAndSortingTemplateVariables.class), PluginRegistry.of(DefaultIdConverter.INSTANCE));
 		SelfLinkProvider selfLinkProvider = new DefaultSelfLinkProvider(persistentEntities(), entityLinks,
-				Collections.<EntityLookup<?>> emptyList());
+				Collections.emptyList(), new DefaultConversionService());
 
 		DefaultRepositoryInvokerFactory invokerFactory = new DefaultRepositoryInvokerFactory(repositories());
 		UriToEntityConverter uriToEntityConverter = new UriToEntityConverter(persistentEntities(), invokerFactory,
diff --git a/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/test/java/org/springframework/data/rest/webmvc/PersistentEntityResourceAssemblerIntegrationTests.java b/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/test/java/org/springframework/data/rest/webmvc/PersistentEntityResourceAssemblerIntegrationTests.java
@@ -25,6 +25,8 @@
 import org.junit.Test;
 import org.mockito.internal.stubbing.answers.ReturnsArgumentAt;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.core.convert.ConversionService;
+import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.data.mapping.context.PersistentEntities;
 import org.springframework.data.rest.core.support.DefaultSelfLinkProvider;
 import org.springframework.data.rest.tests.AbstractControllerIntegrationTests;
@@ -33,9 +35,9 @@
 import org.springframework.data.rest.tests.mongodb.User;
 import org.springframework.data.rest.webmvc.mapping.Associations;
 import org.springframework.data.rest.webmvc.support.Projector;
-import org.springframework.hateoas.server.EntityLinks;
 import org.springframework.hateoas.IanaLinkRelations;
 import org.springframework.hateoas.Links;
+import org.springframework.hateoas.server.EntityLinks;
 import org.springframework.test.context.ContextConfiguration;
 
 /**
@@ -57,8 +59,9 @@ public void addsSelfAndSingleResourceLinkToResourceByDefault() throws Exception
 
 		when(projector.projectExcerpt(any())).thenAnswer(new ReturnsArgumentAt(0));
 
+		ConversionService conversionService = new DefaultConversionService();
 		PersistentEntityResourceAssembler assembler = new PersistentEntityResourceAssembler(entities, projector,
-				associations, new DefaultSelfLinkProvider(entities, entityLinks, Collections.emptyList()));
+				associations, new DefaultSelfLinkProvider(entities, entityLinks, Collections.emptyList(), conversionService));
 
 		User user = new User();
 		user.id = BigInteger.valueOf(4711);
diff --git a/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/test/java/org/springframework/data/rest/webmvc/json/PersistentEntitySerializationTests.java b/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/test/java/org/springframework/data/rest/webmvc/json/PersistentEntitySerializationTests.java
@@ -51,6 +51,7 @@
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.jayway.jsonpath.JsonPath;
+import com.jayway.jsonpath.ReadContext;
 
 /**
  * Integration tests for entity (de)serialization.
@@ -129,7 +130,9 @@ public void createsNestedResourceForMap() throws Exception {
 		PersistentEntityResource resource = PersistentEntityResource
 				.build(dave, repositories.getPersistentEntity(User.class)).build();
 
-		assertThat(JsonPath.parse(mapper.writeValueAsString(resource)).read("$.colleaguesMap.carter._links.user.href",
-				String.class)).isNotNull();
+		String result = mapper.writeValueAsString(resource);
+		ReadContext document = JsonPath.parse(result);
+
+		assertThat(document.read("$.colleaguesMap.carter._links.user.href", String.class)).isNotNull();
 	}
 }
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
@@ -829,8 +829,10 @@ public HttpHeadersPreparer httpHeadersPreparer(AuditableBeanWrapperFactory audit
 	}
 
 	@Bean
-	public SelfLinkProvider selfLinkProvider(PersistentEntities persistentEntities, RepositoryEntityLinks entityLinks) {
-		return new DefaultSelfLinkProvider(persistentEntities, entityLinks, getEntityLookups());
+	public SelfLinkProvider selfLinkProvider(PersistentEntities persistentEntities, RepositoryEntityLinks entityLinks,
+			@Qualifier("mvcConversionService") ObjectProvider<ConversionService> conversionService) {
+		return new DefaultSelfLinkProvider(persistentEntities, entityLinks, getEntityLookups(),
+				conversionService.getIfUnique(() -> defaultConversionService));
 	}
 
 	@Bean
diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/mapping/LinkCollector.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/mapping/LinkCollector.java
@@ -183,7 +183,7 @@ private static class NestedLinkCollectingAssociationHandler implements SimpleAss
 		private final SelfLinkProvider selfLinks;
 		private final PersistentPropertyAccessor<?> accessor;
 		private final Associations associations;
-		private final List<Link> links = new ArrayList<Link>();
+		private Links links = Links.NONE;
 
 		public NestedLinkCollectingAssociationHandler(SelfLinkProvider selfLinks,
 				PersistentPropertyAccessor<?> accessor, Associations associations) {
@@ -198,7 +198,7 @@ public NestedLinkCollectingAssociationHandler(SelfLinkProvider selfLinks,
 		}
 
 		public List<Link> getLinks() {
-			return this.links;
+			return this.links.toList();
 		}
 
 		/*
@@ -223,9 +223,10 @@ public void doWithAssociation(Association<? extends PersistentProperty<?>> assoc
 			ResourceMapping propertyMapping = metadata.getMappingFor(property);
 
 			for (Object element : asCollection(value)) {
-				if (element != null) {
-					links.add(getLinkFor(element, propertyMapping));
-				}
+
+				links = links.andIf(element != null,
+						() -> selfLinks.createSelfLinkFor(property.getAssociationTargetType(), element)
+								.withRel(propertyMapping.getRel()));
 			}
 		}
 

Original file line number	Diff line number	Diff line change
`@@ -90,7 +90,7 @@ public boolean isExported() {`
`90`	`90`	`return false;`
`91`	`91`	`}`
`92`	`92`
`93`		`- ResourceMapping typeMapping = mappings.getMetadataFor(property.getActualType());`
	`93`	`+ ResourceMapping typeMapping = mappings.getMetadataFor(property.getAssociationTargetType());`
`94`	`94`	`return !typeMapping.isExported() ? false : annotation.map(it -> it.exported()).orElse(true);`
`95`	`95`	`}`
`96`	`96`
Original file line number	Diff line number	Diff line change
`@@ -829,8 +829,10 @@ public HttpHeadersPreparer httpHeadersPreparer(AuditableBeanWrapperFactory audit`
`829`	`829`	`}`
`830`	`830`
`831`	`831`	`@Bean`
`832`		`- public SelfLinkProvider selfLinkProvider(PersistentEntities persistentEntities, RepositoryEntityLinks entityLinks) {`
`833`		`- return new DefaultSelfLinkProvider(persistentEntities, entityLinks, getEntityLookups());`
	`832`	`+ public SelfLinkProvider selfLinkProvider(PersistentEntities persistentEntities, RepositoryEntityLinks entityLinks,`
	`833`	`+ @Qualifier("mvcConversionService") ObjectProvider<ConversionService> conversionService) {`
	`834`	`+ return new DefaultSelfLinkProvider(persistentEntities, entityLinks, getEntityLookups(),`
	`835`	`+ conversionService.getIfUnique(() -> defaultConversionService));`
`834`	`836`	`}`
`835`	`837`
`836`	`838`	`@Bean`