MappingContext now generates a single content path per content property

- Fixes #1048
paulcwarren · Aug 27, 2022 · 0d0dc6d · 0d0dc6d
1 parent cdce3c8
commit 0d0dc6d
Show file tree

Hide file tree

Showing 17 changed files with 161 additions and 280 deletions.
diff --git a/...ava/org/springframework/content/commons/mappingcontext/ContentPropertyBuilderVisitor.java b/...ava/org/springframework/content/commons/mappingcontext/ContentPropertyBuilderVisitor.java
@@ -25,8 +25,6 @@
 /**
  * Returns a map of "path"'s to content properties for the given class.
  *
- *
- *
  * @author warrenpa
  *
  */
@@ -102,8 +100,11 @@ public boolean visitClassEnd(Class<?> klazz) {
             }
 
             this.properties = new HashMap<String,ContentProperty>();
-            this.properties.put(fullyQualify(propertyName(""), this.getKeySeparator()), contentProperty);
-            this.properties.put(contentProperty.getContentPropertyPath().replace(this.getContentPropertySeparator(), this.getKeySeparator()), contentProperty);
+            if (isNotRootContentProperty()) {
+                this.properties.put(fullyQualify(propertyName(""), this.getKeySeparator()), contentProperty);
+            } else {
+                this.properties.put(contentProperty.getContentPropertyPath().replace(this.getContentPropertySeparator(), this.getKeySeparator()), contentProperty);
+            }
         }
 
         for (Entry<String, ContentProperty> entry : subProperties.entrySet()) {
@@ -214,6 +215,10 @@ protected String propertyName(String name) {
         return segments[0];
     }
 
+    private boolean isNotRootContentProperty() {
+        return StringUtils.hasLength(fullyQualify(propertyName(""), this.getKeySeparator()));
+    }
+
     private static String[] split(String name) {
         if (!StringUtils.hasLength(name)) {
             return new String[]{};

diff --git a/...ons/src/test/java/org/springframework/content/commons/mappingcontext/ClassWalkerTest.java b/...ons/src/test/java/org/springframework/content/commons/mappingcontext/ClassWalkerTest.java
@@ -60,7 +60,7 @@ public class ClassWalkerTest {
                 expectedSubClassProperty.setContentLengthPropertyPath("child.contentLength");
                 expectedSubClassProperty.setMimeTypePropertyPath("child.contentMimeType");
                 expectedSubClassProperty.setOriginalFileNamePropertyPath("child.contentOriginalFileName");
-                assertThat(visitor.getProperties(), hasEntry("child/content", expectedSubClassProperty));
+                assertThat(visitor.getProperties(), hasEntry("child", expectedSubClassProperty));
 
                 ContentProperty expectedSubSubClassProperty = new ContentProperty();
                 expectedSubSubClassProperty.setContentPropertyPath("child.subChild.content");
@@ -69,7 +69,6 @@ public class ClassWalkerTest {
                 expectedSubSubClassProperty.setMimeTypePropertyPath("child.subChild.contentMimeType");
                 expectedSubSubClassProperty.setOriginalFileNamePropertyPath("child.subChild.contentOriginalFileName");
                 assertThat(visitor.getProperties(), hasEntry("child/subChild", expectedSubSubClassProperty));
-                assertThat(visitor.getProperties(), hasEntry("child/subChild/content", expectedSubSubClassProperty));
             });
         });
 
@@ -79,14 +78,6 @@ public class ClassWalkerTest {
                 ClassWalker walker = new ClassWalker(UncorrelatedAttrClass.class);
                 walker.accept(visitor);
 
-                ContentProperty expectedProperty = new ContentProperty();
-                expectedProperty.setContentPropertyPath("content");
-                expectedProperty.setContentIdPropertyPath("contentId");
-                expectedProperty.setContentLengthPropertyPath("len");
-                expectedProperty.setMimeTypePropertyPath("mimeType");
-                expectedProperty.setOriginalFileNamePropertyPath("originalFileName");
-                assertThat(visitor.getProperties(), hasEntry("", expectedProperty));
-
                 ContentProperty expectedProperty2 = new ContentProperty();
                 expectedProperty2.setContentPropertyPath("content");
                 expectedProperty2.setContentIdPropertyPath("contentId");
@@ -103,14 +94,6 @@ public class ClassWalkerTest {
                 ClassWalker walker = new ClassWalker(CorrelatedAttrClass.class);
                 walker.accept(visitor);
 
-                ContentProperty expectedProperty = new ContentProperty();
-                expectedProperty.setContentPropertyPath("content");
-                expectedProperty.setContentIdPropertyPath("contentId");
-                expectedProperty.setContentLengthPropertyPath("contentLen");
-                expectedProperty.setMimeTypePropertyPath("contentMimeType");
-                expectedProperty.setOriginalFileNamePropertyPath("contentOriginalFileName");
-                assertThat(visitor.getProperties(), hasEntry("", expectedProperty));
-
                 ContentProperty expectedProperty2 = new ContentProperty();
                 expectedProperty2.setContentPropertyPath("content");
                 expectedProperty2.setContentIdPropertyPath("contentId");
@@ -134,14 +117,6 @@ public class ClassWalkerTest {
                 expectedProperty.setMimeTypePropertyPath("child.mimeType");
                 expectedProperty.setOriginalFileNamePropertyPath("child.originalFileName");
                 assertThat(visitor.getProperties(), hasEntry("child", expectedProperty));
-
-                ContentProperty expectedProperty2 = new ContentProperty();
-                expectedProperty2.setContentPropertyPath("child.content");
-                expectedProperty2.setContentIdPropertyPath("child.contentId");
-                expectedProperty2.setContentLengthPropertyPath("child.len");
-                expectedProperty2.setMimeTypePropertyPath("child.mimeType");
-                expectedProperty2.setOriginalFileNamePropertyPath("child.originalFileName");
-                assertThat(visitor.getProperties(), hasEntry("child/content", expectedProperty2));
             });
         });
 
@@ -185,29 +160,13 @@ public class ClassWalkerTest {
                 expectedProperty.setOriginalFileNamePropertyPath("child1.originalFileName");
                 assertThat(visitor.getProperties(), hasEntry("child1", expectedProperty));
 
-                ContentProperty expectedProperty2 = new ContentProperty();
-                expectedProperty2.setContentPropertyPath("child1.content");
-                expectedProperty2.setContentIdPropertyPath("child1.contentId");
-                expectedProperty2.setContentLengthPropertyPath("child1.len");
-                expectedProperty2.setMimeTypePropertyPath("child1.mimeType");
-                expectedProperty2.setOriginalFileNamePropertyPath("child1.originalFileName");
-                assertThat(visitor.getProperties(), hasEntry("child1/content", expectedProperty2));
-
                 ContentProperty expectedProperty3 = new ContentProperty();
                 expectedProperty3.setContentPropertyPath("child2.content");
                 expectedProperty3.setContentIdPropertyPath("child2.contentId");
                 expectedProperty3.setContentLengthPropertyPath("child2.len");
                 expectedProperty3.setMimeTypePropertyPath("child2.mimeType");
                 expectedProperty3.setOriginalFileNamePropertyPath("child2.originalFileName");
                 assertThat(visitor.getProperties(), hasEntry("child2", expectedProperty3));
-
-                ContentProperty expectedProperty4 = new ContentProperty();
-                expectedProperty4.setContentPropertyPath("child2.content");
-                expectedProperty4.setContentIdPropertyPath("child2.contentId");
-                expectedProperty4.setContentLengthPropertyPath("child2.len");
-                expectedProperty4.setMimeTypePropertyPath("child2.mimeType");
-                expectedProperty4.setOriginalFileNamePropertyPath("child2.originalFileName");
-                assertThat(visitor.getProperties(), hasEntry("child2/content", expectedProperty4));
             });
         });
     }

diff --git a/spring-content-rest/src/main/asciidoc/rest-store.adoc b/spring-content-rest/src/main/asciidoc/rest-store.adoc
@@ -413,20 +413,3 @@ All content types except `application/json`
 ==== DELETE
 
 Removes the Resource's content
-
-== The Collection Property Resource
-
-Spring Content REST exports Property Collection Resources to `/{store}/{id}/{property}`.  The resource path can be
-customized using the `@StoreRestResource` annotation on the Store interface.
-
-=== Supported HTTP Methods
-
-Content collection resources support `PUT` and `POST`.
-
-==== PUT/POST
-
-Appends new content to the collection of Resources
-
-===== Supported media types
-
-All content types except `application/json`
diff --git a/.../internal/org/springframework/content/rest/contentservice/ContentStoreContentService.java b/.../internal/org/springframework/content/rest/contentservice/ContentStoreContentService.java
@@ -71,7 +71,7 @@ public void getContent(HttpServletRequest request, HttpServletResponse response,
         AssociatedStoreResource storeResource = (AssociatedStoreResource)resource;
         ContentProperty property = storeResource.getContentProperty();
 
-        Method[] methodsToUse = getExportedMethodsFor(storeResource.getStoreInfo().getInterface(), PropertyPath.from(property.getContentPropertyPath())).getContentMethods();
+        Method[] methodsToUse = getExportedMethodsFor(storeResource.getStoreInfo().getInterface(), storeResource.getPropertyPath()).getContentMethods();
 
         if (methodsToUse.length > 1) {
             throw new IllegalStateException("Too many getContent methods");
@@ -156,7 +156,7 @@ public void setContent(HttpServletRequest request, HttpServletResponse response,
             property.setOriginalFileName(domainObject, source.getFilename());
         }
 
-        Method[] methodsToUse = getExportedMethodsFor(storeResource.getStoreInfo().getInterface(), PropertyPath.from(property.getContentPropertyPath())).setContentMethods();
+        Method[] methodsToUse = getExportedMethodsFor(storeResource.getStoreInfo().getInterface(), storeResource.getPropertyPath()).setContentMethods();
 
         if (methodsToUse.length > 1) {
             RestConfiguration.DomainTypeConfig dtConfig = config.forDomainType(storeResource.getStoreInfo().getDomainObjectClass());
@@ -177,7 +177,7 @@ public void setContent(HttpServletRequest request, HttpServletResponse response,
         try {
             Object targetObj = storeResource.getStoreInfo().getImplementation(ContentStore.class);
             ReflectionUtils.makeAccessible(methodToUse);
-            Object updatedDomainObj = ReflectionUtils.invokeMethod(methodToUse, targetObj, domainObject, PropertyPath.from(property.getContentPropertyPath()), contentArg);
+            Object updatedDomainObj = ReflectionUtils.invokeMethod(methodToUse, targetObj, domainObject, storeResource.getPropertyPath(), contentArg);
 
             repoInvoker.invokeSave(updatedDomainObj);
         } finally {
@@ -191,7 +191,7 @@ public void unsetContent(Resource resource) throws MethodNotAllowedException {
         AssociatedStoreResource storeResource = (AssociatedStoreResource)resource;
         ContentProperty property = storeResource.getContentProperty();
 
-        Method[] methodsToUse = getExportedMethodsFor(storeResource.getStoreInfo().getInterface(), PropertyPath.from(property.getContentPropertyPath())).unsetContentMethods();
+        Method[] methodsToUse = getExportedMethodsFor(storeResource.getStoreInfo().getInterface(), storeResource.getPropertyPath()).unsetContentMethods();
 
         if (methodsToUse.length == 0) {
             throw new MethodNotAllowedException();
@@ -207,7 +207,7 @@ public void unsetContent(Resource resource) throws MethodNotAllowedException {
 
         ReflectionUtils.makeAccessible(methodsToUse[0]);
 
-        Object updatedDomainObj = ReflectionUtils.invokeMethod(methodsToUse[0], targetObj, updateObject, PropertyPath.from(property.getContentPropertyPath()));
+        Object updatedDomainObj = ReflectionUtils.invokeMethod(methodsToUse[0], targetObj, updateObject, storeResource.getPropertyPath());
 
         updateObject = updatedDomainObj;
         property.setMimeType(updateObject, null);

diff --git a/...l/org/springframework/content/rest/controllers/ResourceHandlerMethodArgumentResolver.java b/...l/org/springframework/content/rest/controllers/ResourceHandlerMethodArgumentResolver.java
@@ -58,8 +58,8 @@ public ResourceHandlerMethodArgumentResolver(ApplicationContext context, RestCon
 
         this.entityResolvers = entityResolvers;
 
-        resolvers.add(new StoreResourceResolver());
-        resolvers.add(new AssociativeStoreResourceResolver());
+        resolvers.add(new StoreResourceResolver(this.mappingContext));
+        resolvers.add(new AssociativeStoreResourceResolver(this.mappingContext));
     }
 
     RestConfiguration getConfig() {
@@ -126,7 +126,7 @@ public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer m
                 }
             }
 
-            return matchedResolver.resolve(webRequest, info, result.getEntity(), result.getContentProperty());
+            return matchedResolver.resolve(webRequest, info, result.getEntity(), result.getProperty());
 
         } else if (Store.class.isAssignableFrom(info.getInterface())) {
 

diff --git a/.../springframework/content/rest/controllers/resolvers/AssociativeStoreResourceResolver.java b/.../springframework/content/rest/controllers/resolvers/AssociativeStoreResourceResolver.java
@@ -1,6 +1,6 @@
 package internal.org.springframework.content.rest.controllers.resolvers;
 
-import org.springframework.content.commons.mappingcontext.ContentProperty;
+import org.springframework.content.commons.mappingcontext.MappingContext;
 import org.springframework.content.commons.property.PropertyPath;
 import org.springframework.content.commons.repository.AssociativeStore;
 import org.springframework.content.commons.storeservice.StoreInfo;
@@ -11,14 +11,20 @@
 
 public class AssociativeStoreResourceResolver implements ResourceResolver {
 
+    private MappingContext mappingContext;
+
+    public AssociativeStoreResourceResolver(MappingContext mappingContext) {
+        this.mappingContext = mappingContext;
+    }
+
     @Override
     public String getMapping() {
         return "/{repository}/{id}/**";
     }
 
     @Override
-    public Resource resolve(NativeWebRequest nativeWebRequest, StoreInfo info, Object domainObj, ContentProperty property) {
-        Resource r = info.getImplementation(AssociativeStore.class).getResource(domainObj, PropertyPath.from(property.getContentPropertyPath()));
-        return new AssociatedStoreResourceImpl(info, property, domainObj, r);
+    public Resource resolve(NativeWebRequest nativeWebRequest, StoreInfo info, Object domainObj, PropertyPath propertyPath) {
+        Resource r = info.getImplementation(AssociativeStore.class).getResource(domainObj, propertyPath);
+        return new AssociatedStoreResourceImpl(info, domainObj, propertyPath, mappingContext.getContentProperty(domainObj.getClass(), propertyPath.getName()), r);
     }
 }
diff --git a/...nternal/org/springframework/content/rest/controllers/resolvers/DefaultEntityResolver.java b/...nternal/org/springframework/content/rest/controllers/resolvers/DefaultEntityResolver.java
@@ -12,6 +12,7 @@
 import java.util.Iterator;
 import java.util.Locale;
 import java.util.Map;
+import java.util.NoSuchElementException;
 import java.util.Optional;
 
 import javax.servlet.AsyncContext;
@@ -31,6 +32,7 @@
 
 import org.springframework.content.commons.mappingcontext.ContentProperty;
 import org.springframework.content.commons.mappingcontext.MappingContext;
+import org.springframework.content.commons.property.PropertyPath;
 import org.springframework.content.commons.repository.Store;
 import org.springframework.content.commons.storeservice.StoreInfo;
 import org.springframework.content.commons.storeservice.Stores;
@@ -46,6 +48,7 @@
 import org.springframework.http.ResponseEntity;
 import org.springframework.util.AntPathMatcher;
 import org.springframework.util.ReflectionUtils;
+import org.springframework.util.StringUtils;
 import org.springframework.web.HttpRequestMethodNotSupportedException;
 import org.springframework.web.bind.WebDataBinder;
 import org.springframework.web.bind.annotation.RequestMapping;
@@ -71,19 +74,10 @@ public class DefaultEntityResolver implements EntityResolver {
     private ApplicationContext context;
     private Repositories repositories;
     private Stores stores;
-//    private StoreInfo storeInfo;
     private ConversionService converters;
     private String mapping;
     private MappingContext mappingContext;
 
-//    public DefaultEntityResolver(ApplicationContext context, Repositories repositories, StoreInfo storeInfo, String[] pathSegments, ConversionService converters) {
-//        this.context = context;
-//        this.repositories = repositories;
-//        this.storeInfo = storeInfo;
-//        this.pathSegments = pathSegments;
-//        this.converters = converters;
-//    }
-
     public DefaultEntityResolver(ApplicationContext context, Repositories repositories, Stores stores, ConversionService converters, String mapping, MappingContext mappingContext) {
         this.context = context;
         this.repositories = repositories;
@@ -127,17 +121,12 @@ public EntityResolution resolve(String pathInfo) {
         }
 
         String propertyPath = matcher.extractPathWithinPattern(this.mapping, pathInfo);
-        ContentProperty contentProperty = null;
-        if (propertyPath != null) {
-            contentProperty = mappingContext.getContentProperty(info.getDomainObjectClass(), propertyPath);
-        } else {
-            contentProperty = selectPrimaryContentProperty(info);
-        }
-        if (contentProperty == null) {
-            throw new ResourceNotFoundException();
+        if (!StringUtils.hasText(propertyPath)) {
+            ContentProperty property = selectPrimaryContentProperty(info);
+            propertyPath = property.getContentPropertyPath();
         }
 
-        return new EntityResolution(domainObj, contentProperty);
+        return new EntityResolution(domainObj, PropertyPath.from(propertyPath));
     }
 
     @Override
@@ -268,8 +257,10 @@ private RepositoryInvoker resolveRootResourceInformation(StoreInfo info, String
     }
 
     private ContentProperty selectPrimaryContentProperty(StoreInfo info) {
-        ContentProperty contentProperty;
-        contentProperty = mappingContext.getContentProperties(info.getDomainObjectClass()).iterator().next();
+        ContentProperty contentProperty = null;
+        try {
+            contentProperty = mappingContext.getContentProperties(info.getDomainObjectClass()).iterator().next();
+        } catch (NoSuchElementException nsee) {}
         return contentProperty;
     }
 

diff --git a/...ava/internal/org/springframework/content/rest/controllers/resolvers/EntityResolution.java b/...ava/internal/org/springframework/content/rest/controllers/resolvers/EntityResolution.java
@@ -1,6 +1,6 @@
 package internal.org.springframework.content.rest.controllers.resolvers;
 
-import org.springframework.content.commons.mappingcontext.ContentProperty;
+import org.springframework.content.commons.property.PropertyPath;
 
 import lombok.AllArgsConstructor;
 import lombok.Getter;
@@ -9,5 +9,5 @@
 @AllArgsConstructor
 public class EntityResolution {
     private Object entity;
-    private ContentProperty contentProperty;
+    private PropertyPath property;
 }
diff --git a/...ava/internal/org/springframework/content/rest/controllers/resolvers/ResourceResolver.java b/...ava/internal/org/springframework/content/rest/controllers/resolvers/ResourceResolver.java
@@ -1,11 +1,11 @@
 package internal.org.springframework.content.rest.controllers.resolvers;
 
-import org.springframework.content.commons.mappingcontext.ContentProperty;
+import org.springframework.content.commons.property.PropertyPath;
 import org.springframework.content.commons.storeservice.StoreInfo;
 import org.springframework.core.io.Resource;
 import org.springframework.web.context.request.NativeWebRequest;
 
 public interface ResourceResolver {
     public String getMapping();
-    public Resource resolve(NativeWebRequest nativeWebRequest, StoreInfo info, Object domainObj, ContentProperty property);
+    public Resource resolve(NativeWebRequest nativeWebRequest, StoreInfo info, Object domainObj, PropertyPath propertyPath);
 }
diff --git a/...ternal/org/springframework/content/rest/controllers/resolvers/RevisionEntityResolver.java b/...ternal/org/springframework/content/rest/controllers/resolvers/RevisionEntityResolver.java
@@ -6,8 +6,8 @@
 import java.util.Map;
 import java.util.Optional;
 
-import org.springframework.content.commons.mappingcontext.ContentProperty;
 import org.springframework.content.commons.mappingcontext.MappingContext;
+import org.springframework.content.commons.property.PropertyPath;
 import org.springframework.content.commons.repository.Store;
 import org.springframework.content.commons.storeservice.StoreInfo;
 import org.springframework.content.commons.storeservice.Stores;
@@ -76,9 +76,8 @@ public EntityResolution resolve(String pathInfo) {
         if (propertyPath == null) {
             propertyPath = "";
         }
-        ContentProperty property = mappingContext.getContentProperty(info.getDomainObjectClass(), propertyPath);
 
-        return new EntityResolution(domainObj, property);
+        return new EntityResolution(domainObj, PropertyPath.from(propertyPath));
     }