From b956dd8f5c2134e94a39b11a60094e29538f42cd Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Wed, 26 Jun 2019 16:21:05 +0200 Subject: [PATCH] DATAREST-1404 - Avoid repeated lookups in CollectionResourceMapping implementations. Tweaked implementation of lookups to use Lazy to avoid repeated annotation inspections. --- .../RepositoryCollectionResourceMapping.java | 130 +++++++++--------- .../TypeBasedCollectionResourceMapping.java | 93 +++++++------ ...oryCollectionResourceMappingUnitTests.java | 15 ++ 3 files changed, 131 insertions(+), 107 deletions(-) diff --git a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/RepositoryCollectionResourceMapping.java b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/RepositoryCollectionResourceMapping.java index 000fb2768..3848c00ab 100644 --- a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/RepositoryCollectionResourceMapping.java +++ b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/RepositoryCollectionResourceMapping.java @@ -15,6 +15,8 @@ */ package org.springframework.data.rest.core.mapping; +import java.util.Optional; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.core.annotation.AnnotationUtils; @@ -22,6 +24,8 @@ import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.rest.core.Path; import org.springframework.data.rest.core.annotation.RepositoryRestResource; import org.springframework.data.rest.core.annotation.RestResource; +import org.springframework.data.util.Lazy; +import org.springframework.data.util.Optionals; import org.springframework.hateoas.RelProvider; import org.springframework.hateoas.core.EvoInflectorRelProvider; import org.springframework.util.Assert; @@ -40,11 +44,13 @@ class RepositoryCollectionResourceMapping implements CollectionResourceMapping { private static final Logger LOGGER = LoggerFactory.getLogger(RepositoryCollectionResourceMapping.class); private static final boolean EVO_INFLECTOR_IS_PRESENT = ClassUtils.isPresent("org.atteo.evo.inflector.English", null); - private final RestResource annotation; - private final RepositoryRestResource repositoryAnnotation; - private final CollectionResourceMapping domainTypeMapping; private final boolean repositoryExported; private final RepositoryMetadata metadata; + private final Path path; + + private final Lazy rel, itemResourceRel; + private final Lazy description, itemDescription; + private final Lazy> excerptProjection; public RepositoryCollectionResourceMapping(RepositoryMetadata metadata, RepositoryDetectionStrategy strategy) { this(metadata, strategy, new EvoInflectorRelProvider()); @@ -67,17 +73,60 @@ class RepositoryCollectionResourceMapping implements CollectionResourceMapping { Class repositoryType = metadata.getRepositoryInterface(); + Optional annotation = Optional + .ofNullable(AnnotationUtils.findAnnotation(repositoryType, RestResource.class)); + Optional repositoryAnnotation = Optional + .ofNullable(AnnotationUtils.findAnnotation(repositoryType, RepositoryRestResource.class)); + this.metadata = metadata; - this.annotation = AnnotationUtils.findAnnotation(repositoryType, RestResource.class); - this.repositoryAnnotation = AnnotationUtils.findAnnotation(repositoryType, RepositoryRestResource.class); this.repositoryExported = strategy.isExported(metadata); Class domainType = metadata.getDomainType(); - this.domainTypeMapping = EVO_INFLECTOR_IS_PRESENT + CollectionResourceMapping domainTypeMapping = EVO_INFLECTOR_IS_PRESENT ? new EvoInflectorTypeBasedCollectionResourceMapping(domainType, relProvider) : new TypeBasedCollectionResourceMapping(domainType, relProvider); - if (annotation != null) { + this.rel = Lazy.of(() -> Optionals.firstNonEmpty(// + () -> repositoryAnnotation.map(RepositoryRestResource::collectionResourceRel), // + () -> annotation.map(RestResource::rel)) // + .filter(StringUtils::hasText) // + .orElseGet(domainTypeMapping::getRel)); + + this.itemResourceRel = Lazy.of(() -> repositoryAnnotation.map(RepositoryRestResource::itemResourceRel) // + .filter(StringUtils::hasText) // + .orElseGet(domainTypeMapping::getItemResourceRel)); + + this.path = Optionals.firstNonEmpty(// + () -> repositoryAnnotation.map(RepositoryRestResource::path), // + () -> annotation.map(RestResource::path)) // + .filter(StringUtils::hasText) // + .map(Path::new)// + .orElseGet(domainTypeMapping::getPath); + + this.description = Lazy.of(() -> { + + ResourceDescription fallback = SimpleResourceDescription.defaultFor(getRel()); + + return repositoryAnnotation.map(RepositoryRestResource::collectionResourceDescription) // + . map(it -> new AnnotationBasedResourceDescription(it, fallback)) // + .orElse(fallback); + }); + + this.itemDescription = Lazy.of(() -> { + + ResourceDescription fallback = SimpleResourceDescription.defaultFor(getItemResourceRel()); + + return repositoryAnnotation.map(RepositoryRestResource::itemResourceDescription) // + . map(it -> new AnnotationBasedResourceDescription(it, fallback)) // + .orElse(fallback); + }); + + this.excerptProjection = Lazy.of(() -> repositoryAnnotation// + .map(RepositoryRestResource::excerptProjection)// + .filter(it -> !it.equals(RepositoryRestResource.None.class)) // + .orElse(null)); + + if (annotation.isPresent()) { LOGGER.warn( "@RestResource detected to customize the repository resource for {}! Use @RepositoryRestResource instead!", metadata.getRepositoryInterface().getName()); @@ -90,20 +139,7 @@ class RepositoryCollectionResourceMapping implements CollectionResourceMapping { */ @Override public Path getPath() { - - Path fallback = domainTypeMapping.getPath(); - - if (repositoryAnnotation != null) { - String path = repositoryAnnotation.path(); - return StringUtils.hasText(path) ? new Path(path) : fallback; - } - - if (annotation != null) { - String path = annotation.path(); - return StringUtils.hasText(path) ? new Path(path) : fallback; - } - - return fallback; + return path; } /* @@ -112,20 +148,7 @@ class RepositoryCollectionResourceMapping implements CollectionResourceMapping { */ @Override public String getRel() { - - String fallback = domainTypeMapping.getRel(); - - if (repositoryAnnotation != null) { - String rel = repositoryAnnotation.collectionResourceRel(); - return StringUtils.hasText(rel) ? rel : fallback; - } - - if (annotation != null) { - String rel = annotation.rel(); - return StringUtils.hasText(rel) ? rel : fallback; - } - - return fallback; + return rel.get(); } /* @@ -134,15 +157,7 @@ class RepositoryCollectionResourceMapping implements CollectionResourceMapping { */ @Override public String getItemResourceRel() { - - String fallback = domainTypeMapping.getItemResourceRel(); - - if (repositoryAnnotation != null) { - String rel = repositoryAnnotation.itemResourceRel(); - return StringUtils.hasText(rel) ? rel : fallback; - } - - return fallback; + return itemResourceRel.get(); } /* @@ -169,14 +184,7 @@ class RepositoryCollectionResourceMapping implements CollectionResourceMapping { */ @Override public ResourceDescription getDescription() { - - ResourceDescription fallback = SimpleResourceDescription.defaultFor(getRel()); - - if (repositoryAnnotation != null) { - return new AnnotationBasedResourceDescription(repositoryAnnotation.collectionResourceDescription(), fallback); - } - - return fallback; + return description.get(); } /* @@ -185,14 +193,7 @@ class RepositoryCollectionResourceMapping implements CollectionResourceMapping { */ @Override public ResourceDescription getItemResourceDescription() { - - ResourceDescription fallback = SimpleResourceDescription.defaultFor(getItemResourceRel()); - - if (repositoryAnnotation != null) { - return new AnnotationBasedResourceDescription(repositoryAnnotation.itemResourceDescription(), fallback); - } - - return fallback; + return itemDescription.get(); } /* @@ -201,13 +202,6 @@ class RepositoryCollectionResourceMapping implements CollectionResourceMapping { */ @Override public Class getExcerptProjection() { - - if (repositoryAnnotation == null) { - return null; - } - - Class excerptProjection = repositoryAnnotation.excerptProjection(); - - return excerptProjection.equals(RepositoryRestResource.None.class) ? null : excerptProjection; + return excerptProjection.getOptional().orElse(null); } } diff --git a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/TypeBasedCollectionResourceMapping.java b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/TypeBasedCollectionResourceMapping.java index 3fec61fd8..4b7366ca4 100644 --- a/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/TypeBasedCollectionResourceMapping.java +++ b/spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/TypeBasedCollectionResourceMapping.java @@ -16,11 +16,14 @@ package org.springframework.data.rest.core.mapping; import java.lang.reflect.Modifier; +import java.util.Optional; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.rest.core.Path; import org.springframework.data.rest.core.annotation.Description; import org.springframework.data.rest.core.annotation.RestResource; +import org.springframework.data.util.Lazy; +import org.springframework.data.util.Optionals; import org.springframework.hateoas.RelProvider; import org.springframework.hateoas.core.EvoInflectorRelProvider; import org.springframework.util.Assert; @@ -36,8 +39,11 @@ class TypeBasedCollectionResourceMapping implements CollectionResourceMapping { private final Class type; private final RelProvider relProvider; - private final RestResource annotation; - private final Description description; + private final Optional annotation; + + private final Lazy path; + private final Lazy rel; + private final Lazy description, itemResourceDescription; /** * Creates a new {@link TypeBasedCollectionResourceMapping} using the given type. @@ -61,8 +67,44 @@ class TypeBasedCollectionResourceMapping implements CollectionResourceMapping { this.type = type; this.relProvider = relProvider; - this.annotation = AnnotationUtils.findAnnotation(type, RestResource.class); - this.description = AnnotationUtils.findAnnotation(type, Description.class); + this.annotation = Optional.ofNullable(AnnotationUtils.findAnnotation(type, RestResource.class)); + + this.path = Lazy.of(() -> annotation.map(RestResource::path) // + .map(String::trim) // + .filter(StringUtils::hasText) // + .orElseGet(() -> getDefaultPathFor(type)))// + .map(Path::new); + + this.rel = Lazy.of(() -> annotation // + .map(RestResource::rel) // + .filter(StringUtils::hasText) // + .orElseGet(() -> relProvider.getCollectionResourceRelFor(type))); + + Optional descriptionAnnotation = Optional + .ofNullable(AnnotationUtils.findAnnotation(type, Description.class)); + + this.description = Lazy.of(() -> { + + ResourceDescription fallback = SimpleResourceDescription.defaultFor(getRel()); + + return Optionals. firstNonEmpty(// + () -> descriptionAnnotation.map(it -> new AnnotationBasedResourceDescription(it, fallback)), // + () -> annotation.map(RestResource::description) + .map(it -> new AnnotationBasedResourceDescription(it, fallback))) // + .orElse(fallback); + }); + + this.itemResourceDescription = Lazy.of(() -> { + + ResourceDescription fallback = SimpleResourceDescription.defaultFor(getItemResourceRel()); + + return Optionals. firstNonEmpty(// + () -> annotation.map(RestResource::description) // + .filter(it -> StringUtils.hasText(it.value())) // + .map(it -> new AnnotationBasedResourceDescription(it, fallback)), // + () -> descriptionAnnotation.map(it -> new AnnotationBasedResourceDescription(it, fallback))) // + .orElse(fallback); + }); } /* @@ -71,10 +113,7 @@ class TypeBasedCollectionResourceMapping implements CollectionResourceMapping { */ @Override public Path getPath() { - - String path = annotation == null ? null : annotation.path().trim(); - path = StringUtils.hasText(path) ? path : getDefaultPathFor(type); - return new Path(path); + return path.get(); } /* @@ -83,7 +122,10 @@ class TypeBasedCollectionResourceMapping implements CollectionResourceMapping { */ @Override public boolean isExported() { - return annotation != null ? annotation.exported() : Modifier.isPublic(type.getModifiers()); + + return annotation // + .map(RestResource::exported) // + .orElseGet(() -> Modifier.isPublic(type.getModifiers())); } /* @@ -92,12 +134,7 @@ class TypeBasedCollectionResourceMapping implements CollectionResourceMapping { */ @Override public String getRel() { - - if (annotation == null || !StringUtils.hasText(annotation.rel())) { - return relProvider.getCollectionResourceRelFor(type); - } - - return annotation.rel(); + return rel.get(); } /* @@ -124,18 +161,7 @@ class TypeBasedCollectionResourceMapping implements CollectionResourceMapping { */ @Override public ResourceDescription getDescription() { - - ResourceDescription fallback = SimpleResourceDescription.defaultFor(getRel()); - - if (description != null) { - return new AnnotationBasedResourceDescription(description, fallback); - } - - if (annotation != null) { - return new AnnotationBasedResourceDescription(annotation.description(), fallback); - } - - return fallback; + return description.get(); } /* @@ -144,18 +170,7 @@ class TypeBasedCollectionResourceMapping implements CollectionResourceMapping { */ @Override public ResourceDescription getItemResourceDescription() { - - ResourceDescription fallback = SimpleResourceDescription.defaultFor(getItemResourceRel()); - - if (annotation != null && StringUtils.hasText(annotation.description().value())) { - return new AnnotationBasedResourceDescription(annotation.description(), fallback); - } - - if (description != null) { - return new AnnotationBasedResourceDescription(description, fallback); - } - - return fallback; + return itemResourceDescription.get(); } /* diff --git a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/mapping/RepositoryCollectionResourceMappingUnitTests.java b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/mapping/RepositoryCollectionResourceMappingUnitTests.java index 7faa319af..15dda9327 100755 --- a/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/mapping/RepositoryCollectionResourceMappingUnitTests.java +++ b/spring-data-rest-core/src/test/java/org/springframework/data/rest/core/mapping/RepositoryCollectionResourceMappingUnitTests.java @@ -105,6 +105,13 @@ public class RepositoryCollectionResourceMappingUnitTests { assertThat(mapping.getPath()).isEqualTo(new Path("/objects")); } + @Test // DATAREST-1401 + public void exposesProjectionTypeIfConfigured() { + + assertThat(getResourceMappingFor(WithProjection.class).getExcerptProjection()).isEqualTo(Object.class); + assertThat(getResourceMappingFor(WithoutProjection.class).getExcerptProjection()).isNull(); + } + private static CollectionResourceMapping getResourceMappingFor(Class repositoryInterface) { RepositoryMetadata metadata = new DefaultRepositoryMetadata(repositoryInterface); @@ -132,4 +139,12 @@ public class RepositoryCollectionResourceMappingUnitTests { @RepositoryRestResource(collectionResourceRel = "foo", itemResourceRel = "bar") interface RepositoryAnnotatedRepository extends Repository {} + + @RepositoryRestResource(path = "first/second") + interface InvalidPath extends Repository {} + + @RepositoryRestResource(excerptProjection = Object.class) + interface WithProjection extends Repository {} + + interface WithoutProjection extends Repository {} }