From 9d3158a3fd1b8f3ac01627f26982d286d4898ff2 Mon Sep 17 00:00:00 2001 From: NielsCW <99152561+NielsCW@users.noreply.github.com> Date: Sat, 20 Jul 2024 05:27:51 +0200 Subject: [PATCH] fix multipart requests on entities with a non-default initialized @Version 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 --------- Co-authored-by: Lars Vierbergen --- .../RepositoryEntityMultipartController.java | 9 +-- .../content/rest/support/TestEntity4.java | 2 +- .../ContentEntityRestEndpointsIT.java | 70 ++++++++++++++++++- 3 files changed, 70 insertions(+), 11 deletions(-) diff --git a/spring-content-rest/src/main/java/org/springframework/data/rest/extensions/entitycontent/RepositoryEntityMultipartController.java b/spring-content-rest/src/main/java/org/springframework/data/rest/extensions/entitycontent/RepositoryEntityMultipartController.java index b9fb8370e..2ed0fb340 100644 --- a/spring-content-rest/src/main/java/org/springframework/data/rest/extensions/entitycontent/RepositoryEntityMultipartController.java +++ b/spring-content-rest/src/main/java/org/springframework/data/rest/extensions/entitycontent/RepositoryEntityMultipartController.java @@ -97,7 +97,8 @@ public ResponseEntity> 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> 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 resource = Optional.ofNullable(assembler.toFullResource(savedEntity)); headers.setContentType(new MediaType("application", "hal+json")); diff --git a/spring-content-rest/src/test/java/internal/org/springframework/content/rest/support/TestEntity4.java b/spring-content-rest/src/test/java/internal/org/springframework/content/rest/support/TestEntity4.java index 9a359fb6d..115d0c7d2 100644 --- a/spring-content-rest/src/test/java/internal/org/springframework/content/rest/support/TestEntity4.java +++ b/spring-content-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/spring-content-rest/src/test/java/it/internal/org/springframework/content/rest/controllers/ContentEntityRestEndpointsIT.java b/spring-content-rest/src/test/java/it/internal/org/springframework/content/rest/controllers/ContentEntityRestEndpointsIT.java index 0edec10dc..91547f3cf 100644 --- a/spring-content-rest/src/test/java/it/internal/org/springframework/content/rest/controllers/ContentEntityRestEndpointsIT.java +++ b/spring-content-rest/src/test/java/it/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 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 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";