From e988c3eae78b5c5288e174cc917b714544e3ded5 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 9 Jun 2026 23:06:18 +0200 Subject: [PATCH] =?UTF-8?q?fix(geschichte):=20return=20GeschichteView=20fr?= =?UTF-8?q?om=20create/update=20=E2=80=94=20kill=20write-path=20500?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PATCH /api/geschichten/{id} (save draft, publish) returned the raw entity; with open-in-view false, Jackson serialized the lazy items collection after the transaction closed and every save failed with LazyInitializationException. Write methods now assemble GeschichteView in-transaction, completing the read-model boundary already used by GET — entities no longer cross the controller. Co-Authored-By: Claude Fable 5 --- .../geschichte/GeschichteController.java | 6 +- .../geschichte/GeschichteService.java | 14 +++- .../geschichte/GeschichteControllerTest.java | 36 ++------- .../geschichte/GeschichteHttpTest.java | 75 ++++++++++++++++++- .../GeschichteServiceIntegrationTest.java | 16 ++-- .../geschichte/GeschichteServiceTest.java | 44 +++++------ 6 files changed, 123 insertions(+), 68 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java index 73055218..30b6af59 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java @@ -53,14 +53,14 @@ public class GeschichteController { @PostMapping @RequirePermission(Permission.BLOG_WRITE) - public ResponseEntity create(@RequestBody GeschichteUpdateDTO dto) { - Geschichte created = geschichteService.create(dto); + public ResponseEntity create(@RequestBody GeschichteUpdateDTO dto) { + GeschichteView created = geschichteService.create(dto); return ResponseEntity.status(HttpStatus.CREATED).body(created); } @PatchMapping("/{id}") @RequirePermission(Permission.BLOG_WRITE) - public Geschichte update(@PathVariable UUID id, @RequestBody GeschichteUpdateDTO dto) { + public GeschichteView update(@PathVariable UUID id, @RequestBody GeschichteUpdateDTO dto) { return geschichteService.update(id, dto); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java index 9cb957a2..a683b3cc 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java @@ -127,8 +127,12 @@ public class GeschichteService { // ─── Write API ─────────────────────────────────────────────────────────── + // Write methods return GeschichteView, never the entity: Jackson serializes after + // the transaction closed, where the lazy items collection is a dead proxy. + // The view is assembled in-transaction, so no force-init tricks are needed. + @Transactional - public Geschichte create(GeschichteUpdateDTO dto) { + public GeschichteView create(GeschichteUpdateDTO dto) { requireTitle(dto.getTitle()); Geschichte g = Geschichte.builder() .title(dto.getTitle().trim()) @@ -142,11 +146,12 @@ public class GeschichteService { g.setStatus(GeschichteStatus.PUBLISHED); g.setPublishedAt(LocalDateTime.now()); } - return geschichteRepository.save(g); + Geschichte saved = geschichteRepository.save(g); + return toView(saved, List.of()); } @Transactional - public Geschichte update(UUID id, GeschichteUpdateDTO dto) { + public GeschichteView update(UUID id, GeschichteUpdateDTO dto) { Geschichte g = geschichteRepository.findById(id) .orElseThrow(() -> DomainException.notFound( ErrorCode.GESCHICHTE_NOT_FOUND, "Geschichte not found: " + id)); @@ -163,7 +168,8 @@ public class GeschichteService { if (dto.getStatus() != null && dto.getStatus() != g.getStatus()) { applyStatusTransition(g, dto.getStatus()); } - return geschichteRepository.save(g); + Geschichte saved = geschichteRepository.save(g); + return toView(saved, journeyItemService.getItems(id)); } @Transactional diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java index f6a3b498..8a139e49 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java @@ -150,7 +150,7 @@ class GeschichteControllerTest { void create_returns201_withBlogWrite() throws Exception { UUID id = UUID.randomUUID(); when(geschichteService.create(any(GeschichteUpdateDTO.class))) - .thenReturn(draft(id, "New")); + .thenReturn(viewStub(id, "New", GeschichteStatus.DRAFT)); GeschichteUpdateDTO dto = new GeschichteUpdateDTO(); dto.setTitle("New"); @@ -178,7 +178,7 @@ class GeschichteControllerTest { void update_returns200_withBlogWrite() throws Exception { UUID id = UUID.randomUUID(); when(geschichteService.update(eq(id), any(GeschichteUpdateDTO.class))) - .thenReturn(published(id, "Updated")); + .thenReturn(viewStub(id, "Updated", GeschichteStatus.PUBLISHED)); mockMvc.perform(patch("/api/geschichten/{id}", id).with(csrf()) .contentType(MediaType.APPLICATION_JSON) @@ -381,35 +381,13 @@ class GeschichteControllerTest { return new JourneyItemView(id, position, null, note); } - private Geschichte published(UUID id, String title) { - return Geschichte.builder() - .id(id) - .title(title) - .body("

x

") - .status(GeschichteStatus.PUBLISHED) - .publishedAt(LocalDateTime.now()) - .createdAt(LocalDateTime.now()) - .updatedAt(LocalDateTime.now()) - .persons(new HashSet<>()) - .items(new ArrayList<>()) - .build(); - } - - private Geschichte draft(UUID id, String title) { - return Geschichte.builder() - .id(id) - .title(title) - .status(GeschichteStatus.DRAFT) - .createdAt(LocalDateTime.now()) - .updatedAt(LocalDateTime.now()) - .persons(new HashSet<>()) - .items(new ArrayList<>()) - .build(); - } - private GeschichteView viewStub(UUID id, String title) { + return viewStub(id, title, GeschichteStatus.PUBLISHED); + } + + private GeschichteView viewStub(UUID id, String title, GeschichteStatus status) { return new GeschichteView(id, title, "

x

", - GeschichteStatus.PUBLISHED, GeschichteType.STORY, + status, GeschichteType.STORY, null, new HashSet<>(), List.of(), LocalDateTime.now(), LocalDateTime.now(), LocalDateTime.now()); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteHttpTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteHttpTest.java index 025f5296..3c324b32 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteHttpTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteHttpTest.java @@ -6,6 +6,8 @@ import org.raddatz.familienarchiv.PostgresContainerConfig; import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItem; import org.raddatz.familienarchiv.user.AppUser; import org.raddatz.familienarchiv.user.AppUserRepository; +import org.raddatz.familienarchiv.user.UserGroup; +import org.raddatz.familienarchiv.user.UserGroupRepository; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.web.server.LocalServerPort; @@ -16,6 +18,7 @@ import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.http.client.ClientHttpResponse; +import org.springframework.http.client.JdkClientHttpRequestFactory; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.bean.override.mockito.MockitoBean; @@ -28,6 +31,7 @@ import java.time.LocalDateTime; import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; @@ -49,6 +53,7 @@ class GeschichteHttpTest { @Autowired GeschichteRepository geschichteRepository; @Autowired AppUserRepository appUserRepository; + @Autowired UserGroupRepository userGroupRepository; @Autowired PasswordEncoder passwordEncoder; private RestTemplate http; @@ -63,6 +68,8 @@ class GeschichteHttpTest { baseUrl = "http://localhost:" + port; geschichteRepository.deleteAll(); appUserRepository.findByEmail(WRITER_EMAIL).ifPresent(appUserRepository::delete); + appUserRepository.findByEmail(BLOG_WRITER_EMAIL).ifPresent(appUserRepository::delete); + userGroupRepository.findByName("HttpTest-BlogWriters").ifPresent(userGroupRepository::delete); appUserRepository.save(AppUser.builder() .email(WRITER_EMAIL) .password(passwordEncoder.encode(WRITER_PASSWORD)) @@ -184,15 +191,78 @@ class GeschichteHttpTest { assertThat(response.getStatusCode().value()).isEqualTo(404); } + // ─── PATCH /api/geschichten/{id} ───────────────────────────────────────── + + @Test + void update_returns_200_and_serializes_items_open_in_view_false() { + // Canonical guard for the write path: PATCH must not 500 when the response + // is serialized after the service transaction closed. The raw entity carries + // a dead lazy items proxy at that point — the endpoint must answer with a + // view assembled inside the transaction. + AppUser writer = blogWriter(); + Geschichte journey = Geschichte.builder() + .title("Reise vor dem Umbenennen") + .status(GeschichteStatus.DRAFT) + .type(GeschichteType.JOURNEY) + .author(writer) + .items(new ArrayList<>()) + .persons(new HashSet<>()) + .build(); + journey.getItems().add(JourneyItem.builder() + .geschichte(journey).position(1000).note("Prolog").build()); + Geschichte saved = geschichteRepository.save(journey); + + String session = loginAs(BLOG_WRITER_EMAIL, BLOG_WRITER_PASSWORD); + ResponseEntity response = http.exchange( + baseUrl + "/api/geschichten/" + saved.getId(), HttpMethod.PATCH, + new HttpEntity<>("{\"title\":\"Reise nach dem Umbenennen\"}", csrfJsonHeaders(session)), + String.class); + + assertThat(response.getStatusCode().value()).isEqualTo(200); + assertThat(response.getBody()) + .contains("Reise nach dem Umbenennen") + .contains("Prolog"); + } + // ─── helpers ───────────────────────────────────────────────────────────── + private static final String BLOG_WRITER_EMAIL = "geschichten-http-blogwriter@test.de"; + private static final String BLOG_WRITER_PASSWORD = "pass!Geschichte2"; + + /** A user whose group actually grants BLOG_WRITE — unlike the plain writer above. */ + private AppUser blogWriter() { + UserGroup group = userGroupRepository.save(UserGroup.builder() + .name("HttpTest-BlogWriters") + .permissions(new HashSet<>(Set.of("BLOG_WRITE"))) + .build()); + return appUserRepository.save(AppUser.builder() + .email(BLOG_WRITER_EMAIL) + .password(passwordEncoder.encode(BLOG_WRITER_PASSWORD)) + .groups(new HashSet<>(Set.of(group))) + .build()); + } + + /** Session cookie + double-submit CSRF pair + JSON content type for write requests. */ + private HttpHeaders csrfJsonHeaders(String sessionId) { + String xsrf = UUID.randomUUID().toString(); + HttpHeaders headers = new HttpHeaders(); + headers.set("Cookie", "fa_session=" + sessionId + "; XSRF-TOKEN=" + xsrf); + headers.set("X-XSRF-TOKEN", xsrf); + headers.setContentType(MediaType.APPLICATION_JSON); + return headers; + } + private String loginAsWriter() { + return loginAs(WRITER_EMAIL, WRITER_PASSWORD); + } + + private String loginAs(String email, String password) { String xsrf = UUID.randomUUID().toString(); HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.APPLICATION_JSON); headers.set("Cookie", "XSRF-TOKEN=" + xsrf); headers.set("X-XSRF-TOKEN", xsrf); - String body = "{\"email\":\"" + WRITER_EMAIL + "\",\"password\":\"" + WRITER_PASSWORD + "\"}"; + String body = "{\"email\":\"" + email + "\",\"password\":\"" + password + "\"}"; ResponseEntity resp = http.postForEntity( baseUrl + "/api/auth/login", new HttpEntity<>(body, headers), String.class); return extractFaSessionCookie(resp); @@ -215,7 +285,8 @@ class GeschichteHttpTest { } private RestTemplate noThrowRestTemplate() { - RestTemplate template = new RestTemplate(); + // JDK HttpClient factory — the default HttpURLConnection factory cannot send PATCH. + RestTemplate template = new RestTemplate(new JdkClientHttpRequestFactory()); template.setErrorHandler(new DefaultResponseErrorHandler() { @Override public boolean hasError(ClientHttpResponse response) throws IOException { diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java index f882e4f9..5cef25e0 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java @@ -80,11 +80,11 @@ class GeschichteServiceIntegrationTest { + ""); dto.setPersonIds(List.of(franz.getId())); - Geschichte created = geschichteService.create(dto); + GeschichteView created = geschichteService.create(dto); - assertThat(created.getId()).isNotNull(); - assertThat(created.getStatus()).isEqualTo(GeschichteStatus.DRAFT); - assertThat(created.getBody()) + assertThat(created.id()).isNotNull(); + assertThat(created.status()).isEqualTo(GeschichteStatus.DRAFT); + assertThat(created.body()) .contains("jeden Sonntag") .doesNotContain(""); - Geschichte saved = geschichteService.create(dto); + GeschichteView saved = geschichteService.create(dto); - assertThat(saved.getBody()) + assertThat(saved.body()) .contains("

safe

") .doesNotContain(""); - Geschichte saved = geschichteService.update(id, dto); + GeschichteView saved = geschichteService.update(id, dto); - assertThat(saved.getBody()).doesNotContain("