fix(geschichte): return GeschichteView from create/update — kill write-path 500

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 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-06-09 23:06:18 +02:00
parent 522d1f3ec9
commit e988c3eae7
6 changed files with 123 additions and 68 deletions

View File

@@ -53,14 +53,14 @@ public class GeschichteController {
@PostMapping @PostMapping
@RequirePermission(Permission.BLOG_WRITE) @RequirePermission(Permission.BLOG_WRITE)
public ResponseEntity<Geschichte> create(@RequestBody GeschichteUpdateDTO dto) { public ResponseEntity<GeschichteView> create(@RequestBody GeschichteUpdateDTO dto) {
Geschichte created = geschichteService.create(dto); GeschichteView created = geschichteService.create(dto);
return ResponseEntity.status(HttpStatus.CREATED).body(created); return ResponseEntity.status(HttpStatus.CREATED).body(created);
} }
@PatchMapping("/{id}") @PatchMapping("/{id}")
@RequirePermission(Permission.BLOG_WRITE) @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); return geschichteService.update(id, dto);
} }

View File

@@ -127,8 +127,12 @@ public class GeschichteService {
// ─── Write API ─────────────────────────────────────────────────────────── // ─── 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 @Transactional
public Geschichte create(GeschichteUpdateDTO dto) { public GeschichteView create(GeschichteUpdateDTO dto) {
requireTitle(dto.getTitle()); requireTitle(dto.getTitle());
Geschichte g = Geschichte.builder() Geschichte g = Geschichte.builder()
.title(dto.getTitle().trim()) .title(dto.getTitle().trim())
@@ -142,11 +146,12 @@ public class GeschichteService {
g.setStatus(GeschichteStatus.PUBLISHED); g.setStatus(GeschichteStatus.PUBLISHED);
g.setPublishedAt(LocalDateTime.now()); g.setPublishedAt(LocalDateTime.now());
} }
return geschichteRepository.save(g); Geschichte saved = geschichteRepository.save(g);
return toView(saved, List.of());
} }
@Transactional @Transactional
public Geschichte update(UUID id, GeschichteUpdateDTO dto) { public GeschichteView update(UUID id, GeschichteUpdateDTO dto) {
Geschichte g = geschichteRepository.findById(id) Geschichte g = geschichteRepository.findById(id)
.orElseThrow(() -> DomainException.notFound( .orElseThrow(() -> DomainException.notFound(
ErrorCode.GESCHICHTE_NOT_FOUND, "Geschichte not found: " + id)); ErrorCode.GESCHICHTE_NOT_FOUND, "Geschichte not found: " + id));
@@ -163,7 +168,8 @@ public class GeschichteService {
if (dto.getStatus() != null && dto.getStatus() != g.getStatus()) { if (dto.getStatus() != null && dto.getStatus() != g.getStatus()) {
applyStatusTransition(g, dto.getStatus()); applyStatusTransition(g, dto.getStatus());
} }
return geschichteRepository.save(g); Geschichte saved = geschichteRepository.save(g);
return toView(saved, journeyItemService.getItems(id));
} }
@Transactional @Transactional

View File

@@ -150,7 +150,7 @@ class GeschichteControllerTest {
void create_returns201_withBlogWrite() throws Exception { void create_returns201_withBlogWrite() throws Exception {
UUID id = UUID.randomUUID(); UUID id = UUID.randomUUID();
when(geschichteService.create(any(GeschichteUpdateDTO.class))) when(geschichteService.create(any(GeschichteUpdateDTO.class)))
.thenReturn(draft(id, "New")); .thenReturn(viewStub(id, "New", GeschichteStatus.DRAFT));
GeschichteUpdateDTO dto = new GeschichteUpdateDTO(); GeschichteUpdateDTO dto = new GeschichteUpdateDTO();
dto.setTitle("New"); dto.setTitle("New");
@@ -178,7 +178,7 @@ class GeschichteControllerTest {
void update_returns200_withBlogWrite() throws Exception { void update_returns200_withBlogWrite() throws Exception {
UUID id = UUID.randomUUID(); UUID id = UUID.randomUUID();
when(geschichteService.update(eq(id), any(GeschichteUpdateDTO.class))) 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()) mockMvc.perform(patch("/api/geschichten/{id}", id).with(csrf())
.contentType(MediaType.APPLICATION_JSON) .contentType(MediaType.APPLICATION_JSON)
@@ -381,35 +381,13 @@ class GeschichteControllerTest {
return new JourneyItemView(id, position, null, note); return new JourneyItemView(id, position, null, note);
} }
private Geschichte published(UUID id, String title) {
return Geschichte.builder()
.id(id)
.title(title)
.body("<p>x</p>")
.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) { 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, "<p>x</p>", return new GeschichteView(id, title, "<p>x</p>",
GeschichteStatus.PUBLISHED, GeschichteType.STORY, status, GeschichteType.STORY,
null, new HashSet<>(), List.of(), null, new HashSet<>(), List.of(),
LocalDateTime.now(), LocalDateTime.now(), LocalDateTime.now()); LocalDateTime.now(), LocalDateTime.now(), LocalDateTime.now());
} }

View File

@@ -6,6 +6,8 @@ import org.raddatz.familienarchiv.PostgresContainerConfig;
import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItem; import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItem;
import org.raddatz.familienarchiv.user.AppUser; import org.raddatz.familienarchiv.user.AppUser;
import org.raddatz.familienarchiv.user.AppUserRepository; 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.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.web.server.LocalServerPort; 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.MediaType;
import org.springframework.http.ResponseEntity; import org.springframework.http.ResponseEntity;
import org.springframework.http.client.ClientHttpResponse; import org.springframework.http.client.ClientHttpResponse;
import org.springframework.http.client.JdkClientHttpRequestFactory;
import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.test.context.bean.override.mockito.MockitoBean;
@@ -28,6 +31,7 @@ import java.time.LocalDateTime;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Set;
import java.util.UUID; import java.util.UUID;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
@@ -49,6 +53,7 @@ class GeschichteHttpTest {
@Autowired GeschichteRepository geschichteRepository; @Autowired GeschichteRepository geschichteRepository;
@Autowired AppUserRepository appUserRepository; @Autowired AppUserRepository appUserRepository;
@Autowired UserGroupRepository userGroupRepository;
@Autowired PasswordEncoder passwordEncoder; @Autowired PasswordEncoder passwordEncoder;
private RestTemplate http; private RestTemplate http;
@@ -63,6 +68,8 @@ class GeschichteHttpTest {
baseUrl = "http://localhost:" + port; baseUrl = "http://localhost:" + port;
geschichteRepository.deleteAll(); geschichteRepository.deleteAll();
appUserRepository.findByEmail(WRITER_EMAIL).ifPresent(appUserRepository::delete); 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() appUserRepository.save(AppUser.builder()
.email(WRITER_EMAIL) .email(WRITER_EMAIL)
.password(passwordEncoder.encode(WRITER_PASSWORD)) .password(passwordEncoder.encode(WRITER_PASSWORD))
@@ -184,15 +191,78 @@ class GeschichteHttpTest {
assertThat(response.getStatusCode().value()).isEqualTo(404); 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<String> 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 ───────────────────────────────────────────────────────────── // ─── 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() { private String loginAsWriter() {
return loginAs(WRITER_EMAIL, WRITER_PASSWORD);
}
private String loginAs(String email, String password) {
String xsrf = UUID.randomUUID().toString(); String xsrf = UUID.randomUUID().toString();
HttpHeaders headers = new HttpHeaders(); HttpHeaders headers = new HttpHeaders();
headers.setContentType(MediaType.APPLICATION_JSON); headers.setContentType(MediaType.APPLICATION_JSON);
headers.set("Cookie", "XSRF-TOKEN=" + xsrf); headers.set("Cookie", "XSRF-TOKEN=" + xsrf);
headers.set("X-XSRF-TOKEN", xsrf); headers.set("X-XSRF-TOKEN", xsrf);
String body = "{\"email\":\"" + WRITER_EMAIL + "\",\"password\":\"" + WRITER_PASSWORD + "\"}"; String body = "{\"email\":\"" + email + "\",\"password\":\"" + password + "\"}";
ResponseEntity<String> resp = http.postForEntity( ResponseEntity<String> resp = http.postForEntity(
baseUrl + "/api/auth/login", new HttpEntity<>(body, headers), String.class); baseUrl + "/api/auth/login", new HttpEntity<>(body, headers), String.class);
return extractFaSessionCookie(resp); return extractFaSessionCookie(resp);
@@ -215,7 +285,8 @@ class GeschichteHttpTest {
} }
private RestTemplate noThrowRestTemplate() { 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() { template.setErrorHandler(new DefaultResponseErrorHandler() {
@Override @Override
public boolean hasError(ClientHttpResponse response) throws IOException { public boolean hasError(ClientHttpResponse response) throws IOException {

View File

@@ -80,11 +80,11 @@ class GeschichteServiceIntegrationTest {
+ "<script>alert('xss')</script>"); + "<script>alert('xss')</script>");
dto.setPersonIds(List.of(franz.getId())); dto.setPersonIds(List.of(franz.getId()));
Geschichte created = geschichteService.create(dto); GeschichteView created = geschichteService.create(dto);
assertThat(created.getId()).isNotNull(); assertThat(created.id()).isNotNull();
assertThat(created.getStatus()).isEqualTo(GeschichteStatus.DRAFT); assertThat(created.status()).isEqualTo(GeschichteStatus.DRAFT);
assertThat(created.getBody()) assertThat(created.body())
.contains("<strong>jeden Sonntag</strong>") .contains("<strong>jeden Sonntag</strong>")
.doesNotContain("<script>"); .doesNotContain("<script>");
@@ -93,7 +93,7 @@ class GeschichteServiceIntegrationTest {
assertThat(geschichteService.list(null, List.of(), null, 50)).isEmpty(); assertThat(geschichteService.list(null, List.of(), null, 50)).isEmpty();
// Reader cannot fetch DRAFT by id (404 via GESCHICHTE_NOT_FOUND) // Reader cannot fetch DRAFT by id (404 via GESCHICHTE_NOT_FOUND)
UUID draftId = created.getId(); UUID draftId = created.id();
org.assertj.core.api.Assertions.assertThatThrownBy(() -> geschichteService.getById(draftId)) org.assertj.core.api.Assertions.assertThatThrownBy(() -> geschichteService.getById(draftId))
.hasMessageContaining("not found"); .hasMessageContaining("not found");
@@ -101,8 +101,8 @@ class GeschichteServiceIntegrationTest {
authenticateAs(writer, Permission.BLOG_WRITE); authenticateAs(writer, Permission.BLOG_WRITE);
GeschichteUpdateDTO publishDto = new GeschichteUpdateDTO(); GeschichteUpdateDTO publishDto = new GeschichteUpdateDTO();
publishDto.setStatus(GeschichteStatus.PUBLISHED); publishDto.setStatus(GeschichteStatus.PUBLISHED);
Geschichte publishedGesch = geschichteService.update(draftId, publishDto); GeschichteView publishedGesch = geschichteService.update(draftId, publishDto);
assertThat(publishedGesch.getPublishedAt()).isNotNull(); assertThat(publishedGesch.publishedAt()).isNotNull();
// Reader can now see and fetch it // Reader can now see and fetch it
authenticateAs(reader, Permission.READ_ALL); authenticateAs(reader, Permission.READ_ALL);
@@ -190,7 +190,7 @@ class GeschichteServiceIntegrationTest {
dto.setBody("<p>body</p>"); dto.setBody("<p>body</p>");
dto.setPersonIds(personIds); dto.setPersonIds(personIds);
dto.setStatus(GeschichteStatus.PUBLISHED); dto.setStatus(GeschichteStatus.PUBLISHED);
return geschichteService.create(dto).getId(); return geschichteService.create(dto).id();
} }
private void authenticateAs(AppUser user, Permission... permissions) { private void authenticateAs(AppUser user, Permission... permissions) {

View File

@@ -306,11 +306,11 @@ class GeschichteServiceTest {
dto.setTitle("My Story"); dto.setTitle("My Story");
dto.setBody("<p>plain text</p>"); dto.setBody("<p>plain text</p>");
Geschichte saved = geschichteService.create(dto); GeschichteView saved = geschichteService.create(dto);
assertThat(saved.getStatus()).isEqualTo(GeschichteStatus.DRAFT); assertThat(saved.status()).isEqualTo(GeschichteStatus.DRAFT);
assertThat(saved.getPublishedAt()).isNull(); assertThat(saved.publishedAt()).isNull();
assertThat(saved.getAuthor()).isSameAs(writer); assertThat(saved.author().id()).isEqualTo(writer.getId());
} }
@Test @Test
@@ -324,9 +324,9 @@ class GeschichteServiceTest {
dto.setTitle("XSS attempt"); dto.setTitle("XSS attempt");
dto.setBody("<p>safe</p><script>alert(1)</script><img src=x onerror=alert(2)>"); dto.setBody("<p>safe</p><script>alert(1)</script><img src=x onerror=alert(2)>");
Geschichte saved = geschichteService.create(dto); GeschichteView saved = geschichteService.create(dto);
assertThat(saved.getBody()) assertThat(saved.body())
.contains("<p>safe</p>") .contains("<p>safe</p>")
.doesNotContain("<script>") .doesNotContain("<script>")
.doesNotContain("onerror") .doesNotContain("onerror")
@@ -345,9 +345,9 @@ class GeschichteServiceTest {
dto.setBody("<h2>Heading</h2><p>Some <strong>bold</strong> and <em>italic</em>.</p>" dto.setBody("<h2>Heading</h2><p>Some <strong>bold</strong> and <em>italic</em>.</p>"
+ "<ul><li>one</li></ul><ol><li>first</li></ol>"); + "<ul><li>one</li></ul><ol><li>first</li></ol>");
Geschichte saved = geschichteService.create(dto); GeschichteView saved = geschichteService.create(dto);
assertThat(saved.getBody()) assertThat(saved.body())
.contains("<h2>Heading</h2>") .contains("<h2>Heading</h2>")
.contains("<strong>bold</strong>") .contains("<strong>bold</strong>")
.contains("<em>italic</em>") .contains("<em>italic</em>")
@@ -370,9 +370,9 @@ class GeschichteServiceTest {
dto.setTitle("Linked"); dto.setTitle("Linked");
dto.setPersonIds(List.of(personId)); dto.setPersonIds(List.of(personId));
Geschichte saved = geschichteService.create(dto); GeschichteView saved = geschichteService.create(dto);
assertThat(saved.getPersons()).containsExactly(person); assertThat(saved.persons()).extracting(GeschichteView.PersonView::id).containsExactly(personId);
} }
@Test @Test
@@ -400,9 +400,9 @@ class GeschichteServiceTest {
dto.setTitle("My Journey"); dto.setTitle("My Journey");
dto.setType(GeschichteType.JOURNEY); dto.setType(GeschichteType.JOURNEY);
Geschichte saved = geschichteService.create(dto); GeschichteView saved = geschichteService.create(dto);
assertThat(saved.getType()).isEqualTo(GeschichteType.JOURNEY); assertThat(saved.type()).isEqualTo(GeschichteType.JOURNEY);
} }
@Test @Test
@@ -415,9 +415,9 @@ class GeschichteServiceTest {
GeschichteUpdateDTO dto = new GeschichteUpdateDTO(); GeschichteUpdateDTO dto = new GeschichteUpdateDTO();
dto.setTitle("My Story"); dto.setTitle("My Story");
Geschichte saved = geschichteService.create(dto); GeschichteView saved = geschichteService.create(dto);
assertThat(saved.getType()).isEqualTo(GeschichteType.STORY); assertThat(saved.type()).isEqualTo(GeschichteType.STORY);
} }
// ─── update ────────────────────────────────────────────────────────────── // ─── update ──────────────────────────────────────────────────────────────
@@ -435,10 +435,10 @@ class GeschichteServiceTest {
GeschichteUpdateDTO dto = new GeschichteUpdateDTO(); GeschichteUpdateDTO dto = new GeschichteUpdateDTO();
dto.setStatus(GeschichteStatus.PUBLISHED); dto.setStatus(GeschichteStatus.PUBLISHED);
Geschichte saved = geschichteService.update(id, dto); GeschichteView saved = geschichteService.update(id, dto);
assertThat(saved.getStatus()).isEqualTo(GeschichteStatus.PUBLISHED); assertThat(saved.status()).isEqualTo(GeschichteStatus.PUBLISHED);
assertThat(saved.getPublishedAt()).isNotNull(); assertThat(saved.publishedAt()).isNotNull();
} }
@Test @Test
@@ -454,10 +454,10 @@ class GeschichteServiceTest {
GeschichteUpdateDTO dto = new GeschichteUpdateDTO(); GeschichteUpdateDTO dto = new GeschichteUpdateDTO();
dto.setStatus(GeschichteStatus.DRAFT); dto.setStatus(GeschichteStatus.DRAFT);
Geschichte saved = geschichteService.update(id, dto); GeschichteView saved = geschichteService.update(id, dto);
assertThat(saved.getStatus()).isEqualTo(GeschichteStatus.DRAFT); assertThat(saved.status()).isEqualTo(GeschichteStatus.DRAFT);
assertThat(saved.getPublishedAt()).isNull(); assertThat(saved.publishedAt()).isNull();
} }
@Test @Test
@@ -471,9 +471,9 @@ class GeschichteServiceTest {
GeschichteUpdateDTO dto = new GeschichteUpdateDTO(); GeschichteUpdateDTO dto = new GeschichteUpdateDTO();
dto.setBody("<p>ok</p><script>alert(1)</script>"); dto.setBody("<p>ok</p><script>alert(1)</script>");
Geschichte saved = geschichteService.update(id, dto); GeschichteView saved = geschichteService.update(id, dto);
assertThat(saved.getBody()).doesNotContain("<script>").contains("<p>ok</p>"); assertThat(saved.body()).doesNotContain("<script>").contains("<p>ok</p>");
} }
@Test @Test