From c49754ebb3f110ca2002554c653557573e7ebe9e Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 11 Jan 2016 19:20:04 +0100 Subject: [PATCH] DATAREST-747 - ProjectionDefinitionConfiguration now returns the most concrete projection. In case multiple projections are defined with a given name, we now return the projection defined for the most concrete type match. --- .../ProjectionDefinitionConfiguration.java | 51 ++++++++++----- ...ctionDefinitionConfigurationUnitTests.java | 64 ++++++++++++++----- 2 files changed, 82 insertions(+), 33 deletions(-) diff --git a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfiguration.java b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfiguration.java index a7474d0dc..a13665062 100644 --- a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfiguration.java +++ b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2015 the original author or authors. + * Copyright 2014-2016 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. @@ -16,8 +16,9 @@ package org.springframework.data.rest.core.config; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; -import java.util.Map.Entry; +import java.util.Set; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.rest.core.projection.ProjectionDefinitions; @@ -35,14 +36,14 @@ public class ProjectionDefinitionConfiguration implements ProjectionDefinitions private static final String PROJECTION_ANNOTATION_NOT_FOUND = "Projection annotation not found on %s! Either add the annotation or hand source type to the registration manually!"; private static final String DEFAULT_PROJECTION_PARAMETER_NAME = "projection"; - private final Map> projectionDefinitions; + private final Set projectionDefinitions; private String parameterName = DEFAULT_PROJECTION_PARAMETER_NAME; /** * Creates a new {@link ProjectionDefinitionConfiguration}. */ public ProjectionDefinitionConfiguration() { - this.projectionDefinitions = new HashMap>(); + this.projectionDefinitions = new HashSet(); } /* @@ -117,7 +118,7 @@ public class ProjectionDefinitionConfiguration implements ProjectionDefinitions Assert.notEmpty(sourceTypes, "Source types must not be null!"); for (Class sourceType : sourceTypes) { - this.projectionDefinitions.put(new ProjectionDefinitionKey(sourceType, name), projectionType); + this.projectionDefinitions.add(new ProjectionDefinition(sourceType, projectionType, name)); } return this; @@ -139,8 +140,8 @@ public class ProjectionDefinitionConfiguration implements ProjectionDefinitions @Override public boolean hasProjectionFor(Class sourceType) { - for (ProjectionDefinitionKey key : projectionDefinitions.keySet()) { - if (key.sourceType.isAssignableFrom(sourceType)) { + for (ProjectionDefinition definition : projectionDefinitions) { + if (definition.sourceType.isAssignableFrom(sourceType)) { return true; } } @@ -159,39 +160,55 @@ public class ProjectionDefinitionConfiguration implements ProjectionDefinitions Assert.notNull(sourceType, "Source type must not be null!"); Class userType = ClassUtils.getUserClass(sourceType); + Map byName = new HashMap(); Map> result = new HashMap>(); - for (Entry> entry : projectionDefinitions.entrySet()) { - if (entry.getKey().sourceType.isAssignableFrom(userType)) { - result.put(entry.getKey().name, entry.getValue()); + for (ProjectionDefinition entry : projectionDefinitions) { + + if (!entry.sourceType.isAssignableFrom(userType)) { + continue; + } + + ProjectionDefinition existing = byName.get(entry.name); + + if (existing == null || isSubTypeOf(entry.sourceType, existing.sourceType)) { + byName.put(entry.name, entry); + result.put(entry.name, entry.targetType); } } return result; } + private static boolean isSubTypeOf(Class left, Class right) { + return right.isAssignableFrom(left) && !left.equals(right); + } + /** * Value object to define lookup keys for projections. * * @author Oliver Gierke */ - static final class ProjectionDefinitionKey { + static final class ProjectionDefinition { - private final Class sourceType; + private final Class sourceType, targetType; private final String name; /** * Creates a new {@link ProjectionDefinitionKey} for the given source type and name; * * @param sourceType must not be {@literal null}. + * @param targetType must not be {@literal null}. * @param name must not be {@literal null} or empty. */ - public ProjectionDefinitionKey(Class sourceType, String name) { + public ProjectionDefinition(Class sourceType, Class targetType, String name) { Assert.notNull(sourceType, "Source type must not be null!"); + Assert.notNull(targetType, "Target type must not be null!"); Assert.hasText(name, "Name must not be null or empty!"); this.sourceType = sourceType; + this.targetType = targetType; this.name = name; } @@ -202,12 +219,13 @@ public class ProjectionDefinitionConfiguration implements ProjectionDefinitions @Override public boolean equals(Object obj) { - if (!(obj instanceof ProjectionDefinitionKey)) { + if (!(obj instanceof ProjectionDefinition)) { return false; } - ProjectionDefinitionKey that = (ProjectionDefinitionKey) obj; - return this.name.equals(that.name) && this.sourceType.equals(that.sourceType); + ProjectionDefinition that = (ProjectionDefinition) obj; + return this.name.equals(that.name) && this.sourceType.equals(that.sourceType) + && this.sourceType.equals(that.sourceType); } /* @@ -221,6 +239,7 @@ public class ProjectionDefinitionConfiguration implements ProjectionDefinitions result += name.hashCode(); result += sourceType.hashCode(); + result += targetType.hashCode(); return result; } diff --git a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfigurationUnitTests.java b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfigurationUnitTests.java index 8c987374d..a0cc7833c 100644 --- a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfigurationUnitTests.java +++ b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/config/ProjectionDefinitionConfigurationUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2015 the original author or authors. + * Copyright 2014-2016 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. @@ -18,8 +18,10 @@ package org.springframework.data.rest.core.config; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; +import java.util.Map; + import org.junit.Test; -import org.springframework.data.rest.core.config.ProjectionDefinitionConfiguration.ProjectionDefinitionKey; +import org.springframework.data.rest.core.config.ProjectionDefinitionConfiguration.ProjectionDefinition; /** * Unit tests for {@link ProjectionDefinitionConfiguration}. @@ -117,22 +119,24 @@ public class ProjectionDefinitionConfigurationUnitTests { * @see DATAREST-221 */ @Test - public void definitionKeyEquals() { + public void definitionEquals() { - ProjectionDefinitionKey objectNameKey = new ProjectionDefinitionKey(Object.class, "name"); - ProjectionDefinitionKey sameObjectNameKey = new ProjectionDefinitionKey(Object.class, "name"); - ProjectionDefinitionKey stringNameKey = new ProjectionDefinitionKey(String.class, "name"); - ProjectionDefinitionKey objectOtherNameKey = new ProjectionDefinitionKey(Object.class, "otherName"); + ProjectionDefinition objectName = new ProjectionDefinition(Object.class, Object.class, "name"); + ProjectionDefinition sameObjectName = new ProjectionDefinition(Object.class, Object.class, "name"); + ProjectionDefinition stringName = new ProjectionDefinition(String.class, Object.class, "name"); + ProjectionDefinition objectOtherNameKey = new ProjectionDefinition(Object.class, Object.class, "otherName"); - assertThat(objectNameKey, is(objectNameKey)); - assertThat(objectNameKey, is(sameObjectNameKey)); - assertThat(sameObjectNameKey, is(objectNameKey)); + assertThat(objectName, is(objectName)); + assertThat(objectName, is(sameObjectName)); + assertThat(sameObjectName, is(objectName)); - assertThat(objectNameKey, is(not(stringNameKey))); - assertThat(stringNameKey, is(not(objectNameKey))); + assertThat(objectName, is(not(stringName))); + assertThat(stringName, is(not(objectName))); - assertThat(objectNameKey, is(not(objectOtherNameKey))); - assertThat(objectOtherNameKey, is(not(objectNameKey))); + assertThat(objectName, is(not(objectOtherNameKey))); + assertThat(objectOtherNameKey, is(not(objectName))); + + assertThat(objectName, is(not(new Object()))); } /** @@ -146,8 +150,31 @@ public class ProjectionDefinitionConfigurationUnitTests { assertThat(configuration.hasProjectionFor(Child.class), is(true)); assertThat(configuration.getProjectionsFor(Child.class).values(), hasItem(ParentProjection.class)); - assertThat(configuration.getProjectionType(Child.class, "parentProjection"), - is(typeCompatibleWith(ParentProjection.class))); + assertThat(configuration.getProjectionType(Child.class, "summary"), is(typeCompatibleWith(ParentProjection.class))); + } + + /** + * @see DATAREST-221 + */ + @Test + public void defaultsParamternameToProjection() { + assertThat(new ProjectionDefinitionConfiguration().getParameterName(), is("projection")); + } + + /** + * @see DATAREST-747 + */ + @Test + public void returnsMostConcreteProjectionForSourceType() { + + ProjectionDefinitionConfiguration configuration = new ProjectionDefinitionConfiguration(); + configuration.addProjection(ParentProjection.class); + configuration.addProjection(ChildProjection.class); + + Map> projections = configuration.getProjectionsFor(Child.class); + + assertThat(projections.values(), hasSize(1)); + assertThat(projections.values(), hasItem(ChildProjection.class)); } @Projection(name = "name", types = Integer.class) @@ -162,6 +189,9 @@ public class ProjectionDefinitionConfigurationUnitTests { class Child extends Parent {} - @Projection(types = Parent.class) + @Projection(name = "summary", types = Parent.class) interface ParentProjection {} + + @Projection(name = "summary", types = Child.class) + interface ChildProjection {} }