fix multipart requests on entities with a non-default initialized @Ve…

…rsion property (#2045) * fix multipart requests on entities with a non-default initialized @Version property * Update spring-content-rest/src/main/java/org/springframework/data/rest/extensions/entitycontent/RepositoryEntityMultipartController.java Co-authored-by: Lars Vierbergen <[email protected]> --------- Co-authored-by: Lars Vierbergen <[email protected]>
paulcwarren · Jul 20, 2024 · 9d3158a · 9d3158a
1 parent 24948c7
commit 9d3158a
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 11 deletions.
diff --git a/...ringframework/data/rest/extensions/entitycontent/RepositoryEntityMultipartController.java b/...ringframework/data/rest/extensions/entitycontent/RepositoryEntityMultipartController.java
@@ -97,7 +97,8 @@ public ResponseEntity<RepresentationModel<?>> createEntityAndContent(RootResourc
 
         String store = pathSegments[1];
 
-        boolean entitySaved = false;
+        // Save the entity and re-assign the result to savedEntity, so that it exists in the repository before content is added to it.
+        savedEntity = repoInvokerFactory.getInvokerFor(domainType).invokeSave(savedEntity);
 
         StoreInfo info = this.stores.getStore(Store.class, StoreUtils.withStorePath(store));
         if (info != null) {
@@ -114,15 +115,9 @@ public ResponseEntity<RepresentationModel<?>> createEntityAndContent(RootResourc
 
                 headers.setContentLength(file.getSize());
                 service.setContent(req, resp, headers, new InputStreamResourceWithFilename(file.getInputStream(), file.getOriginalFilename()), MediaType.parseMediaType(file.getContentType()), storeResource);
-                entitySaved = true;
             }
         }
 
-        // if we didn't find store info, or there weren't any files in the request, the entity has not been saved yet
-        if (!entitySaved) {
-            repoInvokerFactory.getInvokerFor(domainType).invokeSave(savedEntity);
-        }
-
         Optional<PersistentEntityResource> resource = Optional.ofNullable(assembler.toFullResource(savedEntity));
         headers.setContentType(new MediaType("application", "hal+json"));
 

diff --git a/...ent-rest/src/test/java/internal/org/springframework/content/rest/support/TestEntity4.java b/...ent-rest/src/test/java/internal/org/springframework/content/rest/support/TestEntity4.java
@@ -25,7 +25,7 @@ public class TestEntity4 implements ContentEntity {
 	public @ContentId UUID contentId;
 	public @ContentLength Long len;
 	public @MimeType String mimeType;
-	private @Version Long version;
+	private @Version Long version = 0L;
 	private @CreatedDate Date createdDate;
 	private @LastModifiedDate Date modifiedDate;
 	private @OriginalFileName String originalFileName;

diff --git a/...t/internal/org/springframework/content/rest/controllers/ContentEntityRestEndpointsIT.java b/...t/internal/org/springframework/content/rest/controllers/ContentEntityRestEndpointsIT.java
@@ -271,6 +271,70 @@ public class ContentEntityRestEndpointsIT {
 				});
 			});
 
+			Context("given a multipart/form POST to an entity with a non-default initialized @Version property (#Issue 2044)", () -> {
+				Context("with content", () -> {
+					It("should create a new entity and its content and respond with a 201 Created", () -> {
+
+						String newContent = "This is some new content";
+
+						MockMultipartFile file = new MockMultipartFile("content", "filename.txt", "text/plain",
+								newContent.getBytes());
+
+						// POST the entity
+						MockHttpServletResponse response = mvc.perform(multipart("/testEntity4s")
+										.file(file)
+										.param("name", "foo")
+										.param("title", "bar"))
+
+								.andExpect(status().isCreated())
+								.andReturn().getResponse();
+
+						String location = response.getHeader("Location");
+
+						// assert that the entity exists
+						Optional<TestEntity4> fetchedEntity = repo4.findById(
+								Long.valueOf(StringUtils.substringAfterLast(location, "/")));
+						assertThat(fetchedEntity.get().getName(), is("foo"));
+						assertThat(fetchedEntity.get().getTitle(), is("bar"));
+						assertThat(fetchedEntity.get().getContentId(), is(not(nullValue())));
+						assertThat(fetchedEntity.get().getLen(), is(file.getSize()));
+						assertThat(fetchedEntity.get().getOriginalFileName(), is(file.getOriginalFilename()));
+
+						// assert that the content now exists
+						response = mvc.perform(get(location)
+										.accept("text/plain"))
+								.andExpect(status().isOk())
+								.andReturn().getResponse();
+
+						assertThat(response.getContentAsString(), is(newContent));
+					});
+				});
+
+				Context("without content", () -> {
+					It("should create a new entity and respond with a 201 Created", () -> {
+
+						// POST the entity
+						MockHttpServletResponse response = mvc.perform(multipart("/testEntity4s")
+										.param("name", "foo")
+										.param("title", "bar"))
+
+								.andExpect(status().isCreated())
+								.andReturn().getResponse();
+
+						String location = response.getHeader("Location");
+
+						// assert that the entity exists
+						Optional<TestEntity4> fetchedEntity = repo4.findById(
+								Long.valueOf(StringUtils.substringAfterLast(location, "/")));
+						assertThat(fetchedEntity.get().getName(), is("foo"));
+						assertThat(fetchedEntity.get().getTitle(), is("bar"));
+						assertThat(fetchedEntity.get().getContentId(), is(nullValue()));
+						assertThat(fetchedEntity.get().getLen(), is(nullValue()));
+						assertThat(fetchedEntity.get().getOriginalFileName(), is(nullValue()));
+					});
+				});
+			});
+
 			Context("given an entity with a single correlated content property", () -> {
                 BeforeEach(() -> {
                     testEntity9 = repo9.save(new TestEntity9());
@@ -303,7 +367,7 @@ public class ContentEntityRestEndpointsIT {
                 });
             });
 
-			Context("given a a multipart/form POST to an entity with a single correlated content property", () -> {
+			Context("given a multipart/form POST to an entity with a single correlated content property", () -> {
 				It("should create a new entity and its content and respond with a 201 Created", () -> {
 					// assert content does not exist
 					String newContent = "This is some new content";
@@ -336,7 +400,7 @@ public class ContentEntityRestEndpointsIT {
 				});
 			});
 
-			Context("given a a multipart/form POST that doesn't include the content property", () -> {
+			Context("given a multipart/form POST that doesn't include the content property", () -> {
 				It("should create a new entity with no content and respond with a 201 Created", () -> {
 
 					var testEntity4Id = repo4.save(new TestEntity4()).getId();
@@ -366,7 +430,7 @@ public class ContentEntityRestEndpointsIT {
 				});
 			});
 
-			Context("given a a multipart/form POST to an entity with a mapped content property", () -> {
+			Context("given a multipart/form POST to an entity with a mapped content property", () -> {
 				It("should create a new entity and its content and respond with a 201 Created", () -> {
 					// assert content does not exist
 					String newContent = "This is some new content";