Feat/content preservation (#1406)

* wip

* feat: add a new setContent method accepting a SetContentParams arg with content length and overwriteExistingContent

- implement in filesystem storage module

* test: fix tests broken by setContent(T, InputStream) refactor

* feat: dont refactor setContent(T, InputStream)

- it breaks backward compatibility

* ci: set up a tmate if ci fails

* ci: redfine the action/cache key

- I think each build is getting a new key because the pom hash is probably different

* ci: ensure maven cache is saved

* c
i: dont restore the cache for the build job, just save it

- otherwise the save fails with a 'already creating' error

* wip

* feat: implement the repository version of setContnt with SetContentParams

* feat: s3 module implements setContent with SetContentParams (unsupported operation)

* feat: wire overwriteExistingContent property through from auto configuration to content store content service

* feat: implement new setContent with SetContentParams method in azure storage module

* feat: implement new setContent with SetContentParams method in the mongo storage module

* feat: implement new setContent with SetContentParams method in gcs storage module

* feat: implement new setContent with SetContentParams method in jpa storage module

* feat: implement new setContent with SetContentParams method in s3 storage module

* feat: encryption storage module now support new ContentStore

- as does ContentStoreAware interface optionally implemented by fragments

* fix: store rest controller multipart form put handler mis-specified an argument meaning the input Resource could not be resolved

* feat: add unsetContent with UnsetContentParams to all storage modules allowing content to be kept

* feat: refactor SetContentParams.overwriteExistingContent as ContentDisposition

* feat: refactor boot and rest layer overwrite existing content property to use SetContentDisposition enum

- add and wire through an UnsetContentDisposition enum

* test: update tests broken by introduction of UnsetContentParams

- Unfocus accidentally focussed tests

* feat: encrypting content store interface should implement UnsetContentParams

* ci: use feat/content-preservation branch of gettingstarted to verify

* code: refactor StoreImpl

* fix: StoreImpl fragment should pass modified content through to delegate

* docs: update docs to explain new SetContentParams, UnsetContentParams and associated spring boot properties

.github/workflows/prs.yml

@@ -70,6 +70,10 @@ jobs:
           git checkout main
           mvn -B clean install
         popd
+    - name: Setup tmate session if anything fails
+      if: ${{ failure() }}
+      uses: mxschmitt/action-tmate@v3
+      timeout-minutes: 360
 
   validate-with-gettingstarteds:
     runs-on: ubuntu-latest
@@ -93,7 +97,7 @@ jobs:
       with:
         repository: paulcwarren/spring-content-gettingstarted
         path: spring-content-gettingstarted
-        ref: refs/heads/main
+        ref: refs/heads/feat/content-preservation
     - name: Validate against Getting Started Guides
       run: |
         export AWS_REGION=us-west-1

spring-content-autoconfigure/src/main/java/internal/org/springframework/content/rest/boot/autoconfigure/ContentRestAutoConfiguration.java

@@ -7,8 +7,9 @@
 import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication;
 import org.springframework.boot.context.properties.ConfigurationProperties;
 import org.springframework.content.rest.config.RestConfiguration;
+import org.springframework.content.rest.config.SetContentDisposition;
+import org.springframework.content.rest.config.UnsetContentDisposition;
 import org.springframework.context.annotation.Bean;
-import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Import;
 import org.springframework.stereotype.Component;
 
@@ -25,6 +26,10 @@ public static class ContentRestProperties {
 		private URI baseUri;
 		private boolean fullyQualifiedLinks = RestConfiguration.FULLY_QUALIFIED_DEFAULTS_DEFAULT;
 		private ShortcutRequestMappings requestMappings = new ShortcutRequestMappings();
+		private boolean overwriteExistingContent = RestConfiguration.OVERWRITE_EXISTING_CONTENT_DEFAULT;
+
+		private SetContentDisposition setContentDisposition = RestConfiguration.SETCONTENT_CONTENT_DISPOSITION_DEFAULT;
+		private UnsetContentDisposition unsetContentDisposition = RestConfiguration.UNSETCONTENT_CONTENT_DISPOSITION_DEFAULT;
 
 		public URI getBaseUri() {
 			return baseUri;
@@ -50,6 +55,30 @@ public void setShortcutRequestMappings(ShortcutRequestMappings requestMappings)
 		    this.requestMappings = requestMappings;
 		}
 
+		public boolean getOverwriteExistingContent() {
+			return this.overwriteExistingContent;
+		}
+
+		public void setOverwriteExistingContent(boolean overwriteExistingContent) {
+			this.overwriteExistingContent = overwriteExistingContent;
+		}
+
+		public SetContentDisposition getSetContentDisposition() {
+			return this.setContentDisposition;
+		}
+
+		public void setSetContentDisposition(SetContentDisposition setContentDisposition) {
+			this.setContentDisposition = setContentDisposition;
+		}
+
+		public UnsetContentDisposition getUnsetContentDisposition() {
+			return this.unsetContentDisposition;
+		}
+
+		public void setUnsetContentDisposition(UnsetContentDisposition unsetContentDisposition) {
+			this.unsetContentDisposition = unsetContentDisposition;
+		}
+
 		public static class ShortcutRequestMappings {
 
 		    private boolean disabled = false;

spring-content-autoconfigure/src/main/java/internal/org/springframework/content/rest/boot/autoconfigure/SpringBootContentRestConfigurer.java

@@ -57,5 +57,10 @@ public void configure(RestConfiguration config) {
                 }
             }
         }
+
+        config.setOverwriteExistingContent(properties.getOverwriteExistingContent());
+
+        config.setSetContentDisposition(properties.getSetContentDisposition());
+        config.setUnsetContentDisposition(properties.getUnsetContentDisposition());
     }
 }

spring-content-autoconfigure/src/test/java/org/springframework/content/rest/boot/ContentRestAutoConfigurationTest.java

@@ -55,12 +55,14 @@ public class ContentRestAutoConfigurationTest {
 					System.setProperty("spring.content.rest.fully-qualified-links", "false");
                     System.setProperty("spring.content.rest.shortcut-request-mappings.disabled", "true");
 					System.setProperty("spring.content.rest.shortcut-request-mappings.excludes", "GET=a/b,c/d:PUT=*/*");
+					System.setProperty("spring.content.rest.overwrite-existing-content", "false");
 				});
 				AfterEach(() -> {
 					System.clearProperty("spring.content.rest.base-uri");
                     System.clearProperty("spring.content.rest.fully-qualified-links");
                     System.clearProperty("spring.content.rest.shortcut-request-mappings.disabled");
                     System.clearProperty("spring.content.rest.shortcut-request-mappings.excludes");
+					System.clearProperty("spring.content.rest.overwrite-existing-content");
 				});
 				It("should have a filesystem properties bean with the correct properties set", () -> {
 					AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext();
@@ -72,6 +74,7 @@ public class ContentRestAutoConfigurationTest {
 					assertThat(context.getBean(ContentRestAutoConfiguration.ContentRestProperties.class).fullyQualifiedLinks(), is(false));
                     assertThat(context.getBean(ContentRestAutoConfiguration.ContentRestProperties.class).shortcutRequestMappings().disabled(), is(true));
                     assertThat(context.getBean(ContentRestAutoConfiguration.ContentRestProperties.class).shortcutRequestMappings().excludes(), is("GET=a/b,c/d:PUT=*/*"));
+					assertThat(context.getBean(ContentRestAutoConfiguration.ContentRestProperties.class).getOverwriteExistingContent(), is(false));
 
 					assertThat(context.getBean(SpringBootContentRestConfigurer.class), is(not(nullValue())));
 

spring-content-azure-storage/src/main/asciidoc/azure.adoc

@@ -99,24 +99,25 @@ instance of `BlobId`
 
 See <<configuring_converters,Configuring a Spring Converter>> for more information on how to register a converter.
 
-=== Setting Content using a ContentStore
+=== Setting Content
 
-Storing content is achieved using the `ContentStore.setContent(entity, InputStream)` method.
+Storing content is achieved using the `ContentStore.setContent(T entity, PropertyPath path, InputStream content, SetContentParams params)` method.
 
-If content has not yet been stored with this entity and an Id has not been assigned one will be generated
-based in `java.util.UUID`.
+The `PropertyPath` will be used to resolve the content property to update.
+
+If content has not yet been stored with this entity and an Id has not been assigned, one will be generated based in `java.util.UUID`.
 
 The `@ContentId` and `@ContentLength` annotations will be updated on `entity`.
 
-If content has previously been stored it will overwritten updating just the @ContentLength attribute, if present.
+If content has previously been stored it will be overwritten also updating the @ContentLength attribute, if present.  However, using `ContentDisposition.Create` on the `SetContentParams` a new Id will be assigned and content stored, leaving the existing content in place and orphaned.
 
-=== Getting Content from a ContentStore
+=== Getting Content
 
-Content can be accessed using the `ContentStore.getContent(entity)` method.
+Content can be accessed using the `ContentStore.getContent(T entity, PropertyPath path)` method.
 
-=== Unsetting Content from a ContentStore
+=== Unsetting Content
 
-Content can be removed using the `ContentStore.unsetContent(entity)` method.
+Content can be removed using the `ContentStore.unsetContent(T entity, PropertyPath path, UnsetContentParams params)` method.  Using `ContentDisposition.Keep` on `UnsetContentParams` will leave the content in storage and orphaned.
 
 === Configuring a Spring Converter
 [[configuring_converters]]

spring-content-azure-storage/src/main/java/internal/org/springframework/content/azure/store/DefaultAzureStorageImpl.java

@@ -21,9 +21,8 @@
 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.store.AssociativeStore;
-import org.springframework.content.commons.store.GetResourceParams;
-import org.springframework.content.commons.store.StoreAccessException;
+import org.springframework.content.commons.repository.SetContentParams;
+import org.springframework.content.commons.store.*;
 import org.springframework.content.commons.utils.BeanUtils;
 import org.springframework.content.commons.utils.Condition;
 import org.springframework.content.commons.utils.PlacementService;
@@ -45,7 +44,7 @@ public class DefaultAzureStorageImpl<S, SID extends Serializable>
 		implements org.springframework.content.commons.repository.Store<SID>,
         org.springframework.content.commons.repository.AssociativeStore<S, SID>,
         org.springframework.content.commons.repository.ContentStore<S, SID>,
-        AssociativeStore<S, SID> {
+        ContentStore<S, SID> {
 
 	private static Log logger = LogFactory.getLog(DefaultAzureStorageImpl.class);
 
@@ -290,14 +289,26 @@ public S setContent(S entity, PropertyPath propertyPath, InputStream content) {
     @Transactional
     @Override
     public S setContent(S entity, PropertyPath propertyPath, InputStream content, long contentLen) {
+        return this.setContent(entity, propertyPath, content, org.springframework.content.commons.store.SetContentParams.builder().contentLength(contentLen).build());
+    }
+
+    @Override
+    public S setContent(S entity, PropertyPath propertyPath, InputStream content, SetContentParams params) {
+        return this.setContent(entity, propertyPath, content,
+                org.springframework.content.commons.store.SetContentParams.builder()
+                        .contentLength(params.getContentLength())
+                        .overwriteExistingContent(params.isOverwriteExistingContent()).build());
+    }
 
+    @Override
+    public S setContent(S entity, PropertyPath propertyPath, InputStream content, org.springframework.content.commons.store.SetContentParams params) {
         ContentProperty property = this.mappingContext.getContentProperty(entity.getClass(), propertyPath.getName());
         if (property == null) {
             throw new StoreAccessException(String.format("Content property %s does not exist", propertyPath.getName()));
         }
 
         Object contentId = property.getContentId(entity);
-        if (contentId == null) {
+        if (contentId == null || params.getDisposition().equals(org.springframework.content.commons.store.SetContentParams.ContentDisposition.CreateNew)) {
 
             Serializable newId = UUID.randomUUID().toString();
 
@@ -323,7 +334,7 @@ public S setContent(S entity, PropertyPath propertyPath, InputStream content, lo
         }
 
         try {
-            long lenToSet = contentLen;
+            long lenToSet = params.getContentLength();
             if (lenToSet == -1L) {
                 lenToSet = resource.contentLength();
             }
@@ -446,7 +457,22 @@ public boolean matches(Field field) {
     @Transactional
     @Override
     public S unsetContent(S entity, PropertyPath propertyPath) {
+        return this.unsetContent(entity, propertyPath, UnsetContentParams.builder().build());
+    }
+
+    @Transactional
+    @Override
+    public S unsetContent(S entity, PropertyPath propertyPath, org.springframework.content.commons.repository.UnsetContentParams params) {
+        int ordinal = params.getDisposition().ordinal();
+        UnsetContentParams params1 = UnsetContentParams.builder()
+                .disposition(UnsetContentParams.Disposition.values()[ordinal])
+                .build();
+        return this.unsetContent(entity, propertyPath, params1);
+    }
 
+    @Transactional
+    @Override
+    public S unsetContent(S entity, PropertyPath propertyPath, UnsetContentParams params) {
         ContentProperty property = this.mappingContext.getContentProperty(entity.getClass(), propertyPath.getName());
         if (property == null) {
             throw new StoreAccessException(String.format("Content property %s does not exist", propertyPath.getName()));
@@ -456,7 +482,7 @@ public S unsetContent(S entity, PropertyPath propertyPath) {
             return entity;
 
         Resource resource = this.getResource(entity, propertyPath);
-        if (resource != null && resource.exists() && resource instanceof DeletableResource) {
+        if (resource != null && resource.exists() && resource instanceof DeletableResource && params.getDisposition().equals(UnsetContentParams.Disposition.Remove)) {
 
             try {
                 ((DeletableResource)resource).delete();
@@ -474,8 +500,8 @@ public boolean matches(TypeDescriptor descriptor) {
                     if ("jakarta.persistence.Id".equals(
                             annotation.annotationType().getCanonicalName())
                             || "org.springframework.data.annotation.Id"
-                                    .equals(annotation.annotationType()
-                                            .getCanonicalName())) {
+                            .equals(annotation.annotationType()
+                                    .getCanonicalName())) {
                         return false;
                     }
                 }
@@ -488,7 +514,7 @@ public boolean matches(TypeDescriptor descriptor) {
         return entity;
     }
 
-	private String absolutify(String bucket, String location) {
+    private String absolutify(String bucket, String location) {
 		String locationToUse = null;
 		Assert.state(location.startsWith("azure-blob://") == false);
 		if (location.startsWith("/")) {

spring-content-azure-storage/src/test/java/internal/org/springframework/content/azure/it/AzureStorageIT.java

@@ -1,12 +1,9 @@
 package internal.org.springframework.content.azure.it;
 
-import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.AfterEach;
-import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.BeforeEach;
-import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.Context;
-import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.Describe;
-import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.It;
+import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.*;
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -18,6 +15,7 @@
 import java.io.OutputStream;
 import java.util.UUID;
 
+import com.azure.storage.blob.specialized.BlockBlobClient;
 import jakarta.persistence.Entity;
 import jakarta.persistence.GeneratedValue;
 import jakarta.persistence.GenerationType;
@@ -37,6 +35,8 @@
 import org.springframework.content.commons.repository.StoreAccessException;
 import org.springframework.content.commons.store.ContentStore;
 import org.springframework.content.commons.store.GetResourceParams;
+import org.springframework.content.commons.store.SetContentParams;
+import org.springframework.content.commons.store.UnsetContentParams;
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -379,7 +379,31 @@ public class AzureStorageIT {
                     });
                 });
 
-                Context("when content is deleted", () -> {
+                Context("when content is updated and not overwritten", () -> {
+                    It("should have the updated content", () -> {
+                        BlobContainerClient c = builder.buildClient().getBlobContainerClient("azure-test-bucket");
+
+                        String contentId = entity.getContentId();
+                        assertThat(c.getBlobClient(contentId).getBlockBlobClient().exists(), is(true));
+
+                        store.setContent(entity, PropertyPath.from("content"), new ByteArrayInputStream("Hello Updated Spring Content World!".getBytes()), SetContentParams.builder().disposition(SetContentParams.ContentDisposition.CreateNew).build());
+                        entity = repo.save(entity);
+
+                        boolean matches = false;
+                        try (InputStream content = store.getContent(entity)) {
+                            matches = IOUtils.contentEquals(new ByteArrayInputStream("Hello Updated Spring Content World!".getBytes()), content);
+                            assertThat(matches, is(true));
+                        }
+
+                        assertThat(c.getBlobClient(contentId).getBlockBlobClient().exists(), is(true));
+
+                        assertThat(entity.getContentId(), is(not(contentId)));
+
+                        assertThat(c.getBlobClient(entity.getContentId()).getBlockBlobClient().exists(), is(true));
+                    });
+                });
+
+                Context("when content is unset", () -> {
                     BeforeEach(() -> {
                         resourceLocation = entity.getContentId().toString();
                         entity = store.unsetContent(entity);
@@ -396,13 +420,37 @@ public class AzureStorageIT {
                         assertThat(entity.getContentId(), is(Matchers.nullValue()));
                         Assert.assertEquals(entity.getContentLen(), 0);
 
+                        BlobContainerClient c = builder.buildClient().getBlobContainerClient("azure-test-bucket");
+                        assertThat(c.getBlobClient(resourceLocation).getBlockBlobClient().exists(), is(false));
+
                         //rendition
                         try (InputStream content = store.getContent(entity, PropertyPath.from("rendition"))) {
                             assertThat(content, is(Matchers.nullValue()));
                         }
 
+                        assertThat(entity.getRenditionId(), is(Matchers.nullValue()));
+                        Assert.assertEquals(entity.getRenditionLen(), 0);
+                    });
+                });
+
+                Context("when content is unset but kept", () -> {
+                    BeforeEach(() -> {
+                        resourceLocation = entity.getContentId().toString();
+                        entity = store.unsetContent(entity, PropertyPath.from("content"), UnsetContentParams.builder().disposition(UnsetContentParams.Disposition.Keep).build());
+                        entity = repo.save(entity);
+                    });
+
+                    It("should have no content", () -> {
+                        //content
+                        try (InputStream content = store.getContent(entity)) {
+                            assertThat(content, is(Matchers.nullValue()));
+                        }
+
                         assertThat(entity.getContentId(), is(Matchers.nullValue()));
                         Assert.assertEquals(entity.getContentLen(), 0);
+
+                        BlobContainerClient c = builder.buildClient().getBlobContainerClient("azure-test-bucket");
+                        assertThat(c.getBlobClient(resourceLocation).getBlockBlobClient().exists(), is(true));
                     });
                 });