refactor(geschichte): assemble GeschichteView in controller — break GeschichteService↔JourneyItemService cycle
GeschichteService.getById() now returns the Geschichte entity (with the DRAFT visibility guard intact). The controller calls journeyItemService.getItems() and geschichteService.toView() to assemble the GeschichteView, removing the need for GeschichteService to hold a reference to JourneyItemService. Tests updated accordingly: GeschichteServiceTest tests toView() directly; GeschichteControllerTest stubs both service calls; integration test uses the two-step pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -8,6 +8,7 @@ import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemView;
|
|||||||
import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyReorderDTO;
|
import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyReorderDTO;
|
||||||
import org.raddatz.familienarchiv.security.Permission;
|
import org.raddatz.familienarchiv.security.Permission;
|
||||||
import org.raddatz.familienarchiv.security.RequirePermission;
|
import org.raddatz.familienarchiv.security.RequirePermission;
|
||||||
|
import io.swagger.v3.oas.annotations.Operation;
|
||||||
import org.springframework.http.HttpStatus;
|
import org.springframework.http.HttpStatus;
|
||||||
import org.springframework.http.ResponseEntity;
|
import org.springframework.http.ResponseEntity;
|
||||||
import org.springframework.web.bind.annotation.DeleteMapping;
|
import org.springframework.web.bind.annotation.DeleteMapping;
|
||||||
@@ -45,7 +46,9 @@ public class GeschichteController {
|
|||||||
|
|
||||||
@GetMapping("/{id}")
|
@GetMapping("/{id}")
|
||||||
public GeschichteView getById(@PathVariable UUID id) {
|
public GeschichteView getById(@PathVariable UUID id) {
|
||||||
return geschichteService.getById(id);
|
Geschichte g = geschichteService.getById(id);
|
||||||
|
List<JourneyItemView> items = journeyItemService.getItems(g.getId());
|
||||||
|
return geschichteService.toView(g, items);
|
||||||
}
|
}
|
||||||
|
|
||||||
@PostMapping
|
@PostMapping
|
||||||
|
|||||||
@@ -6,7 +6,6 @@ import org.owasp.html.HtmlPolicyBuilder;
|
|||||||
import org.owasp.html.PolicyFactory;
|
import org.owasp.html.PolicyFactory;
|
||||||
import org.raddatz.familienarchiv.exception.DomainException;
|
import org.raddatz.familienarchiv.exception.DomainException;
|
||||||
import org.raddatz.familienarchiv.exception.ErrorCode;
|
import org.raddatz.familienarchiv.exception.ErrorCode;
|
||||||
import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemService;
|
|
||||||
import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemView;
|
import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemView;
|
||||||
import org.raddatz.familienarchiv.user.AppUser;
|
import org.raddatz.familienarchiv.user.AppUser;
|
||||||
import org.raddatz.familienarchiv.person.Person;
|
import org.raddatz.familienarchiv.person.Person;
|
||||||
@@ -36,7 +35,6 @@ public class GeschichteService {
|
|||||||
private final PersonService personService;
|
private final PersonService personService;
|
||||||
private final DocumentService documentService;
|
private final DocumentService documentService;
|
||||||
private final UserService userService;
|
private final UserService userService;
|
||||||
private final JourneyItemService journeyItemService;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Allow-list policy for Geschichte body HTML. Tiptap on the writer side
|
* Allow-list policy for Geschichte body HTML. Tiptap on the writer side
|
||||||
@@ -57,7 +55,7 @@ public class GeschichteService {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Transactional(readOnly = true)
|
@Transactional(readOnly = true)
|
||||||
public GeschichteView getById(UUID id) {
|
public Geschichte getById(UUID id) {
|
||||||
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));
|
||||||
@@ -66,13 +64,10 @@ public class GeschichteService {
|
|||||||
throw DomainException.notFound(
|
throw DomainException.notFound(
|
||||||
ErrorCode.GESCHICHTE_NOT_FOUND, "Geschichte not found: " + id);
|
ErrorCode.GESCHICHTE_NOT_FOUND, "Geschichte not found: " + id);
|
||||||
}
|
}
|
||||||
// Items loaded via repository query — never through the LAZY collection on Geschichte.
|
return g;
|
||||||
// This keeps open-in-view:false safe without Hibernate.initialize.
|
|
||||||
List<JourneyItemView> items = journeyItemService.getItems(id);
|
|
||||||
return toView(g, items);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private GeschichteView toView(Geschichte g, List<JourneyItemView> items) {
|
GeschichteView toView(Geschichte g, List<JourneyItemView> items) {
|
||||||
AppUser author = g.getAuthor();
|
AppUser author = g.getAuthor();
|
||||||
GeschichteView.AuthorView authorView = null;
|
GeschichteView.AuthorView authorView = null;
|
||||||
if (author != null) {
|
if (author != null) {
|
||||||
|
|||||||
@@ -105,7 +105,10 @@ class GeschichteControllerTest {
|
|||||||
@WithMockUser(authorities = "READ_ALL")
|
@WithMockUser(authorities = "READ_ALL")
|
||||||
void getById_returns200_whenFound() throws Exception {
|
void getById_returns200_whenFound() throws Exception {
|
||||||
UUID id = UUID.randomUUID();
|
UUID id = UUID.randomUUID();
|
||||||
when(geschichteService.getById(id)).thenReturn(viewStub(id, "Hello"));
|
Geschichte g = published(id, "Hello");
|
||||||
|
when(geschichteService.getById(id)).thenReturn(g);
|
||||||
|
when(journeyItemService.getItems(id)).thenReturn(List.of());
|
||||||
|
when(geschichteService.toView(g, List.of())).thenReturn(viewStub(id, "Hello"));
|
||||||
|
|
||||||
mockMvc.perform(get("/api/geschichten/{id}", id))
|
mockMvc.perform(get("/api/geschichten/{id}", id))
|
||||||
.andExpect(status().isOk())
|
.andExpect(status().isOk())
|
||||||
|
|||||||
@@ -8,10 +8,12 @@ import org.raddatz.familienarchiv.geschichte.GeschichteUpdateDTO;
|
|||||||
import org.raddatz.familienarchiv.user.AppUser;
|
import org.raddatz.familienarchiv.user.AppUser;
|
||||||
import org.raddatz.familienarchiv.geschichte.Geschichte;
|
import org.raddatz.familienarchiv.geschichte.Geschichte;
|
||||||
import org.raddatz.familienarchiv.geschichte.GeschichteStatus;
|
import org.raddatz.familienarchiv.geschichte.GeschichteStatus;
|
||||||
|
import org.raddatz.familienarchiv.geschichte.GeschichteType;
|
||||||
import org.raddatz.familienarchiv.geschichte.GeschichteView;
|
import org.raddatz.familienarchiv.geschichte.GeschichteView;
|
||||||
import org.raddatz.familienarchiv.person.Person;
|
import org.raddatz.familienarchiv.person.Person;
|
||||||
import org.raddatz.familienarchiv.user.AppUserRepository;
|
import org.raddatz.familienarchiv.user.AppUserRepository;
|
||||||
import org.raddatz.familienarchiv.geschichte.GeschichteRepository;
|
import org.raddatz.familienarchiv.geschichte.GeschichteRepository;
|
||||||
|
import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemService;
|
||||||
import org.raddatz.familienarchiv.person.PersonRepository;
|
import org.raddatz.familienarchiv.person.PersonRepository;
|
||||||
import org.raddatz.familienarchiv.security.Permission;
|
import org.raddatz.familienarchiv.security.Permission;
|
||||||
import org.springframework.beans.factory.annotation.Autowired;
|
import org.springframework.beans.factory.annotation.Autowired;
|
||||||
@@ -40,6 +42,7 @@ class GeschichteServiceIntegrationTest {
|
|||||||
S3Client s3Client;
|
S3Client s3Client;
|
||||||
|
|
||||||
@Autowired GeschichteService geschichteService;
|
@Autowired GeschichteService geschichteService;
|
||||||
|
@Autowired JourneyItemService journeyItemService;
|
||||||
@Autowired GeschichteRepository geschichteRepository;
|
@Autowired GeschichteRepository geschichteRepository;
|
||||||
@Autowired PersonRepository personRepository;
|
@Autowired PersonRepository personRepository;
|
||||||
@Autowired AppUserRepository appUserRepository;
|
@Autowired AppUserRepository appUserRepository;
|
||||||
@@ -105,9 +108,10 @@ class GeschichteServiceIntegrationTest {
|
|||||||
authenticateAs(reader, Permission.READ_ALL);
|
authenticateAs(reader, Permission.READ_ALL);
|
||||||
assertThat(geschichteService.list(null, List.of(), 50)).hasSize(1);
|
assertThat(geschichteService.list(null, List.of(), 50)).hasSize(1);
|
||||||
assertThat(geschichteService.list(null, List.of(franz.getId()), 50)).hasSize(1);
|
assertThat(geschichteService.list(null, List.of(franz.getId()), 50)).hasSize(1);
|
||||||
GeschichteView fetched = geschichteService.getById(draftId);
|
Geschichte fetched = geschichteService.getById(draftId);
|
||||||
assertThat(fetched.title()).isEqualTo("Erinnerung an Opa Franz");
|
GeschichteView fetchedView = geschichteService.toView(fetched, journeyItemService.getItems(draftId));
|
||||||
assertThat(fetched.persons()).extracting(GeschichteView.PersonView::id).containsExactly(franz.getId());
|
assertThat(fetchedView.title()).isEqualTo("Erinnerung an Opa Franz");
|
||||||
|
assertThat(fetchedView.persons()).extracting(GeschichteView.PersonView::id).containsExactly(franz.getId());
|
||||||
|
|
||||||
// Delete as writer; join rows go with it
|
// Delete as writer; join rows go with it
|
||||||
authenticateAs(writer, Permission.BLOG_WRITE);
|
authenticateAs(writer, Permission.BLOG_WRITE);
|
||||||
|
|||||||
@@ -9,7 +9,6 @@ import org.mockito.Mock;
|
|||||||
import org.mockito.junit.jupiter.MockitoExtension;
|
import org.mockito.junit.jupiter.MockitoExtension;
|
||||||
import org.raddatz.familienarchiv.exception.DomainException;
|
import org.raddatz.familienarchiv.exception.DomainException;
|
||||||
import org.raddatz.familienarchiv.exception.ErrorCode;
|
import org.raddatz.familienarchiv.exception.ErrorCode;
|
||||||
import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemService;
|
|
||||||
import org.raddatz.familienarchiv.user.AppUser;
|
import org.raddatz.familienarchiv.user.AppUser;
|
||||||
import org.raddatz.familienarchiv.person.Person;
|
import org.raddatz.familienarchiv.person.Person;
|
||||||
import org.raddatz.familienarchiv.security.Permission;
|
import org.raddatz.familienarchiv.security.Permission;
|
||||||
@@ -46,7 +45,6 @@ class GeschichteServiceTest {
|
|||||||
@Mock PersonService personService;
|
@Mock PersonService personService;
|
||||||
@Mock DocumentService documentService;
|
@Mock DocumentService documentService;
|
||||||
@Mock UserService userService;
|
@Mock UserService userService;
|
||||||
@Mock JourneyItemService journeyItemService;
|
|
||||||
|
|
||||||
@InjectMocks GeschichteService geschichteService;
|
@InjectMocks GeschichteService geschichteService;
|
||||||
|
|
||||||
@@ -58,7 +56,6 @@ class GeschichteServiceTest {
|
|||||||
SecurityContextHolder.clearContext();
|
SecurityContextHolder.clearContext();
|
||||||
writer = AppUser.builder().id(UUID.randomUUID()).email("writer@test").build();
|
writer = AppUser.builder().id(UUID.randomUUID()).email("writer@test").build();
|
||||||
reader = AppUser.builder().id(UUID.randomUUID()).email("reader@test").build();
|
reader = AppUser.builder().id(UUID.randomUUID()).email("reader@test").build();
|
||||||
lenient().when(journeyItemService.getItems(any())).thenReturn(List.of());
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@AfterEach
|
@AfterEach
|
||||||
@@ -88,10 +85,10 @@ class GeschichteServiceTest {
|
|||||||
Geschichte draft = draft(id);
|
Geschichte draft = draft(id);
|
||||||
when(geschichteRepository.findById(id)).thenReturn(Optional.of(draft));
|
when(geschichteRepository.findById(id)).thenReturn(Optional.of(draft));
|
||||||
|
|
||||||
GeschichteView result = geschichteService.getById(id);
|
Geschichte result = geschichteService.getById(id);
|
||||||
|
|
||||||
assertThat(result.id()).isEqualTo(id);
|
assertThat(result.getId()).isEqualTo(id);
|
||||||
assertThat(result.status()).isEqualTo(GeschichteStatus.DRAFT);
|
assertThat(result.getStatus()).isEqualTo(GeschichteStatus.DRAFT);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@@ -101,90 +98,10 @@ class GeschichteServiceTest {
|
|||||||
Geschichte published = published(id);
|
Geschichte published = published(id);
|
||||||
when(geschichteRepository.findById(id)).thenReturn(Optional.of(published));
|
when(geschichteRepository.findById(id)).thenReturn(Optional.of(published));
|
||||||
|
|
||||||
GeschichteView result = geschichteService.getById(id);
|
Geschichte result = geschichteService.getById(id);
|
||||||
|
|
||||||
assertThat(result.id()).isEqualTo(id);
|
assertThat(result.getId()).isEqualTo(id);
|
||||||
assertThat(result.status()).isEqualTo(GeschichteStatus.PUBLISHED);
|
assertThat(result.getStatus()).isEqualTo(GeschichteStatus.PUBLISHED);
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
|
||||||
void getById_author_displayName_uses_firstName_lastName() {
|
|
||||||
authenticateAs(reader, Permission.READ_ALL);
|
|
||||||
UUID id = UUID.randomUUID();
|
|
||||||
Geschichte published = published(id);
|
|
||||||
published.setAuthor(AppUser.builder()
|
|
||||||
.id(UUID.randomUUID()).email("author@test")
|
|
||||||
.firstName("Hans").lastName("Raddatz").build());
|
|
||||||
when(geschichteRepository.findById(id)).thenReturn(Optional.of(published));
|
|
||||||
|
|
||||||
GeschichteView result = geschichteService.getById(id);
|
|
||||||
|
|
||||||
assertThat(result.author().displayName()).isEqualTo("Hans Raddatz");
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
|
||||||
void getById_author_displayName_falls_back_to_Unbekannt_when_names_blank() {
|
|
||||||
authenticateAs(reader, Permission.READ_ALL);
|
|
||||||
UUID id = UUID.randomUUID();
|
|
||||||
Geschichte published = published(id);
|
|
||||||
published.setAuthor(AppUser.builder()
|
|
||||||
.id(UUID.randomUUID()).email("anon@test").build());
|
|
||||||
when(geschichteRepository.findById(id)).thenReturn(Optional.of(published));
|
|
||||||
|
|
||||||
GeschichteView result = geschichteService.getById(id);
|
|
||||||
|
|
||||||
assertThat(result.author().displayName()).isEqualTo("[Unbekannt]");
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
|
||||||
void getById_author_email_is_not_in_author_view() {
|
|
||||||
authenticateAs(reader, Permission.READ_ALL);
|
|
||||||
UUID id = UUID.randomUUID();
|
|
||||||
Geschichte published = published(id);
|
|
||||||
published.setAuthor(AppUser.builder()
|
|
||||||
.id(UUID.randomUUID()).email("secret@test")
|
|
||||||
.firstName("Max").lastName("M").build());
|
|
||||||
when(geschichteRepository.findById(id)).thenReturn(Optional.of(published));
|
|
||||||
|
|
||||||
GeschichteView result = geschichteService.getById(id);
|
|
||||||
|
|
||||||
// AuthorView exposes only id + displayName — no email field at all
|
|
||||||
assertThat(result.author()).isInstanceOf(GeschichteView.AuthorView.class);
|
|
||||||
assertThat(result.author().displayName()).doesNotContain("secret@test");
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
|
||||||
void getById_persons_are_mapped_to_PersonView() {
|
|
||||||
authenticateAs(reader, Permission.READ_ALL);
|
|
||||||
UUID id = UUID.randomUUID();
|
|
||||||
UUID personId = UUID.randomUUID();
|
|
||||||
Geschichte published = published(id);
|
|
||||||
published.setPersons(new HashSet<>(List.of(
|
|
||||||
Person.builder().id(personId).firstName("Franz").lastName("Raddatz").build()
|
|
||||||
)));
|
|
||||||
when(geschichteRepository.findById(id)).thenReturn(Optional.of(published));
|
|
||||||
|
|
||||||
GeschichteView result = geschichteService.getById(id);
|
|
||||||
|
|
||||||
assertThat(result.persons()).hasSize(1);
|
|
||||||
GeschichteView.PersonView pv = result.persons().iterator().next();
|
|
||||||
assertThat(pv.id()).isEqualTo(personId);
|
|
||||||
assertThat(pv.firstName()).isEqualTo("Franz");
|
|
||||||
assertThat(pv.lastName()).isEqualTo("Raddatz");
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
|
||||||
void getById_items_come_from_journeyItemService() {
|
|
||||||
authenticateAs(reader, Permission.READ_ALL);
|
|
||||||
UUID id = UUID.randomUUID();
|
|
||||||
Geschichte published = published(id);
|
|
||||||
when(geschichteRepository.findById(id)).thenReturn(Optional.of(published));
|
|
||||||
when(journeyItemService.getItems(id)).thenReturn(List.of());
|
|
||||||
|
|
||||||
GeschichteView result = geschichteService.getById(id);
|
|
||||||
|
|
||||||
assertThat(result.items()).isEmpty();
|
|
||||||
verify(journeyItemService).getItems(id);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@@ -199,6 +116,74 @@ class GeschichteServiceTest {
|
|||||||
.isEqualTo(ErrorCode.GESCHICHTE_NOT_FOUND);
|
.isEqualTo(ErrorCode.GESCHICHTE_NOT_FOUND);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void toView_author_displayName_uses_firstName_lastName() {
|
||||||
|
UUID id = UUID.randomUUID();
|
||||||
|
Geschichte published = published(id);
|
||||||
|
published.setAuthor(AppUser.builder()
|
||||||
|
.id(UUID.randomUUID()).email("author@test")
|
||||||
|
.firstName("Hans").lastName("Raddatz").build());
|
||||||
|
|
||||||
|
GeschichteView result = geschichteService.toView(published, List.of());
|
||||||
|
|
||||||
|
assertThat(result.author().displayName()).isEqualTo("Hans Raddatz");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void toView_author_displayName_falls_back_to_Unbekannt_when_names_blank() {
|
||||||
|
UUID id = UUID.randomUUID();
|
||||||
|
Geschichte published = published(id);
|
||||||
|
published.setAuthor(AppUser.builder()
|
||||||
|
.id(UUID.randomUUID()).email("anon@test").build());
|
||||||
|
|
||||||
|
GeschichteView result = geschichteService.toView(published, List.of());
|
||||||
|
|
||||||
|
assertThat(result.author().displayName()).isEqualTo("[Unbekannt]");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void toView_author_email_is_not_in_author_view() {
|
||||||
|
UUID id = UUID.randomUUID();
|
||||||
|
Geschichte published = published(id);
|
||||||
|
published.setAuthor(AppUser.builder()
|
||||||
|
.id(UUID.randomUUID()).email("secret@test")
|
||||||
|
.firstName("Max").lastName("M").build());
|
||||||
|
|
||||||
|
GeschichteView result = geschichteService.toView(published, List.of());
|
||||||
|
|
||||||
|
// AuthorView exposes only id + displayName — no email field at all
|
||||||
|
assertThat(result.author()).isInstanceOf(GeschichteView.AuthorView.class);
|
||||||
|
assertThat(result.author().displayName()).doesNotContain("secret@test");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void toView_persons_are_mapped_to_PersonView() {
|
||||||
|
UUID id = UUID.randomUUID();
|
||||||
|
UUID personId = UUID.randomUUID();
|
||||||
|
Geschichte published = published(id);
|
||||||
|
published.setPersons(new HashSet<>(List.of(
|
||||||
|
Person.builder().id(personId).firstName("Franz").lastName("Raddatz").build()
|
||||||
|
)));
|
||||||
|
|
||||||
|
GeschichteView result = geschichteService.toView(published, List.of());
|
||||||
|
|
||||||
|
assertThat(result.persons()).hasSize(1);
|
||||||
|
GeschichteView.PersonView pv = result.persons().iterator().next();
|
||||||
|
assertThat(pv.id()).isEqualTo(personId);
|
||||||
|
assertThat(pv.firstName()).isEqualTo("Franz");
|
||||||
|
assertThat(pv.lastName()).isEqualTo("Raddatz");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void toView_items_are_passed_through() {
|
||||||
|
UUID id = UUID.randomUUID();
|
||||||
|
Geschichte published = published(id);
|
||||||
|
|
||||||
|
GeschichteView result = geschichteService.toView(published, List.of());
|
||||||
|
|
||||||
|
assertThat(result.items()).isEmpty();
|
||||||
|
}
|
||||||
|
|
||||||
// ─── list ─────────────────────────────────────────────────────────────────
|
// ─── list ─────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
Reference in New Issue
Block a user