From 439385dd35fc90666cd099bf683fd32c4ae15ff1 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 12:24:50 +0200 Subject: [PATCH] refactor(geschichte): update service, controller, repository for JourneyItem model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - GeschichteService.list() now returns List via JPQL projection query; accepts (status, personIds, limit); DRAFT clamp for non-BLOG_WRITE users; AND-semantics person filter with sentinel UUID guard - GeschichteService.getById() is @Transactional(readOnly=true) and calls Hibernate.initialize(g.getItems()) to force-init the LAZY bag under open-in-view=false - GeschichteRepository: add findSummaries() JPQL query with person subquery - GeschichteController.list(): remove documentId param, change return type to List - GeschichteSpecifications: remove hasDocument() and documentSubquery() — TODO left for lesereisen-editor follow-on Co-Authored-By: Claude Sonnet 4.6 --- .../geschichte/GeschichteController.java | 8 +-- .../geschichte/GeschichteRepository.java | 33 +++++++++++- .../geschichte/GeschichteService.java | 53 ++++++++----------- .../geschichte/GeschichteSpecifications.java | 20 +------ 4 files changed, 57 insertions(+), 57 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 1da96278..09be9493 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java @@ -1,12 +1,8 @@ package org.raddatz.familienarchiv.geschichte; import lombok.RequiredArgsConstructor; -import org.raddatz.familienarchiv.geschichte.GeschichteUpdateDTO; -import org.raddatz.familienarchiv.geschichte.Geschichte; -import org.raddatz.familienarchiv.geschichte.GeschichteStatus; import org.raddatz.familienarchiv.security.Permission; import org.raddatz.familienarchiv.security.RequirePermission; -import org.raddatz.familienarchiv.geschichte.GeschichteService; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.DeleteMapping; @@ -30,15 +26,13 @@ public class GeschichteController { private final GeschichteService geschichteService; @GetMapping - public List list( + public List list( @RequestParam(required = false) GeschichteStatus status, @RequestParam(name = "personId", required = false) List personIds, - @RequestParam(required = false) UUID documentId, @RequestParam(required = false, defaultValue = "50") int limit) { return geschichteService.list( status, personIds == null ? List.of() : personIds, - documentId, limit); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteRepository.java index b42a47a5..f0947218 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteRepository.java @@ -1,12 +1,43 @@ package org.raddatz.familienarchiv.geschichte; -import org.raddatz.familienarchiv.geschichte.Geschichte; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.JpaSpecificationExecutor; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; +import java.util.Collection; +import java.util.List; import java.util.UUID; @Repository public interface GeschichteRepository extends JpaRepository, JpaSpecificationExecutor { + + /** + * Returns the grid projection. Never carries items (avoids lazy-init 500 under open-in-view:false). + * + *

Status clamp: callers must pass the effective status (PUBLISHED for readers, + * raw status for BLOG_WRITE users). authorId restricts to own drafts when effective=DRAFT. + * + *

Person filter: personCount=0 disables the filter. When personCount>0, the story must + * be associated with ALL person ids in personIds (AND-semantics via counting subquery). + * Pass a non-empty personIds collection when personCount>0 — empty IN() is invalid SQL. + */ + @Query(""" + SELECT g.id AS id, g.title AS title, g.status AS status, g.type AS type, + g.author AS author, g.publishedAt AS publishedAt, g.body AS body + FROM Geschichte g + WHERE g.status = :effectiveStatus + AND (:authorId IS NULL OR g.author.id = :authorId) + AND (:personCount = 0 OR + (SELECT COUNT(DISTINCT p.id) + FROM Geschichte g2 JOIN g2.persons p + WHERE g2.id = g.id AND p.id IN :personIds) = :personCount) + ORDER BY COALESCE(g.publishedAt, g.updatedAt) DESC + """) + List findSummaries( + @Param("effectiveStatus") GeschichteStatus effectiveStatus, + @Param("authorId") UUID authorId, + @Param("personIds") Collection personIds, + @Param("personCount") long personCount); } 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 53443cf4..b290186c 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java @@ -2,30 +2,24 @@ package org.raddatz.familienarchiv.geschichte; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.hibernate.Hibernate; import org.owasp.html.HtmlPolicyBuilder; import org.owasp.html.PolicyFactory; -import org.raddatz.familienarchiv.geschichte.GeschichteUpdateDTO; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.user.AppUser; -import org.raddatz.familienarchiv.document.Document; -import org.raddatz.familienarchiv.geschichte.Geschichte; -import org.raddatz.familienarchiv.geschichte.GeschichteStatus; import org.raddatz.familienarchiv.person.Person; -import org.raddatz.familienarchiv.geschichte.GeschichteRepository; -import org.raddatz.familienarchiv.geschichte.GeschichteSpecifications; import org.raddatz.familienarchiv.security.Permission; import org.raddatz.familienarchiv.document.DocumentService; import org.raddatz.familienarchiv.person.PersonService; import org.raddatz.familienarchiv.user.UserService; -import org.springframework.data.domain.Sort; -import org.springframework.data.jpa.domain.Specification; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import java.time.LocalDateTime; +import java.util.Collection; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; @@ -39,6 +33,8 @@ public class GeschichteService { private final GeschichteRepository geschichteRepository; private final PersonService personService; + // Reserved for lesereisen-editor: JourneyItem document resolution must go through + // DocumentService.getDocumentById to enforce existence and scope checks. private final DocumentService documentService; private final UserService userService; @@ -60,6 +56,7 @@ public class GeschichteService { return geschichteRepository.count(GeschichteSpecifications.hasStatus(GeschichteStatus.PUBLISHED)); } + @Transactional(readOnly = true) public Geschichte getById(UUID id) { Geschichte g = geschichteRepository.findById(id) .orElseThrow(() -> DomainException.notFound( @@ -69,6 +66,10 @@ public class GeschichteService { throw DomainException.notFound( ErrorCode.GESCHICHTE_NOT_FOUND, "Geschichte not found: " + id); } + // Force-initialize LAZY items inside this transaction. + // open-in-view is FALSE — without this touch, Jackson serializes a closed + // Hibernate session and throws LazyInitializationException → HTTP 500. + Hibernate.initialize(g.getItems()); return g; } @@ -76,20 +77,25 @@ public class GeschichteService { * Lists Geschichten with optional filters. {@code personIds} uses AND semantics: the story * must be associated with every person id supplied. An empty or null list applies no * person filter. Result is ordered by {@code COALESCE(publishedAt, updatedAt) DESC}. + * + *

Returns a {@link GeschichteSummary} projection — never carries items, preventing + * LazyInitializationException on the non-transactional list path. */ - public List list(GeschichteStatus status, List personIds, UUID documentId, int limit) { + public List list(GeschichteStatus status, List personIds, int limit) { GeschichteStatus effective = currentUserHasBlogWrite() ? status : GeschichteStatus.PUBLISHED; int safeLimit = limit <= 0 ? DEFAULT_LIMIT : Math.min(limit, MAX_LIMIT); UUID authorId = effective == GeschichteStatus.DRAFT ? currentUser().getId() : null; - Specification spec = Specification.allOf( - GeschichteSpecifications.hasStatus(effective), - GeschichteSpecifications.hasAuthor(authorId), - GeschichteSpecifications.hasAllPersons(personIds), - GeschichteSpecifications.hasDocument(documentId), - GeschichteSpecifications.orderByDisplayDateDesc() - ); - return geschichteRepository.findAll(spec, Sort.unsorted()) + + // When personIds is empty, personCount=0 short-circuits the IN() predicate. + // Pass a sentinel UUID to avoid invalid empty IN() SQL while the predicate is skipped. + Collection safePersonIds = (personIds == null || personIds.isEmpty()) + ? List.of(UUID.fromString("00000000-0000-0000-0000-000000000000")) + : personIds; + long personCount = (personIds == null) ? 0 : personIds.size(); + + return geschichteRepository + .findSummaries(effective, authorId, safePersonIds, personCount) .stream() .limit(safeLimit) .toList(); @@ -106,7 +112,6 @@ public class GeschichteService { .status(GeschichteStatus.DRAFT) .author(currentUser()) .persons(resolvePersons(dto.getPersonIds())) - .documents(resolveDocuments(dto.getDocumentIds())) .build(); if (dto.getStatus() == GeschichteStatus.PUBLISHED) { g.setStatus(GeschichteStatus.PUBLISHED); @@ -130,9 +135,6 @@ public class GeschichteService { if (dto.getPersonIds() != null) { g.setPersons(resolvePersons(dto.getPersonIds())); } - if (dto.getDocumentIds() != null) { - g.setDocuments(resolveDocuments(dto.getDocumentIds())); - } if (dto.getStatus() != null && dto.getStatus() != g.getStatus()) { applyStatusTransition(g, dto.getStatus()); } @@ -176,15 +178,6 @@ public class GeschichteService { return new LinkedHashSet<>(personService.getAllById(ids)); } - private Set resolveDocuments(List ids) { - if (ids == null || ids.isEmpty()) return new HashSet<>(); - Set out = new LinkedHashSet<>(); - for (UUID id : ids) { - out.add(documentService.getDocumentById(id)); - } - return out; - } - private AppUser currentUser() { Authentication auth = SecurityContextHolder.getContext().getAuthentication(); if (auth == null || !auth.isAuthenticated()) { diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteSpecifications.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteSpecifications.java index 42797ffe..b8ff3603 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteSpecifications.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteSpecifications.java @@ -6,9 +6,6 @@ import jakarta.persistence.criteria.Join; import jakarta.persistence.criteria.Predicate; import jakarta.persistence.criteria.Root; import jakarta.persistence.criteria.Subquery; -import org.raddatz.familienarchiv.document.Document; -import org.raddatz.familienarchiv.geschichte.Geschichte; -import org.raddatz.familienarchiv.geschichte.GeschichteStatus; import org.raddatz.familienarchiv.person.Person; import org.springframework.data.jpa.domain.Specification; @@ -48,12 +45,7 @@ public final class GeschichteSpecifications { authorId == null ? null : cb.equal(root.get("author").get("id"), authorId); } - public static Specification hasDocument(UUID documentId) { - return (root, query, cb) -> { - if (documentId == null) return null; - return cb.exists(documentSubquery(root, query, cb, documentId)); - }; - } + // TODO(lesereisen-editor): restore document filter via journey_items join when editor lands /** * AND-filter across persons: the Geschichte must be associated with EVERY id in {@code personIds}. @@ -84,14 +76,4 @@ public final class GeschichteSpecifications { return sub; } - private static Subquery documentSubquery( - Root root, CriteriaQuery query, CriteriaBuilder cb, UUID documentId) { - Subquery sub = query.subquery(UUID.class); - Root subRoot = sub.from(Geschichte.class); - Join documents = subRoot.join("documents"); - sub.select(subRoot.get("id")) - .where(cb.equal(subRoot.get("id"), root.get("id")), - cb.equal(documents.get("id"), documentId)); - return sub; - } }