From 616089e65eb10a16cd991ec75688bb3637ae7466 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 27 Apr 2026 14:20:15 +0200 Subject: [PATCH] feat(stammbaum): RelationshipService + family_member toggle (TDD) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add PersonService.setFamilyMember (write, @Transactional) and findAllFamilyMembers; PersonRepository gains the findByFamilyMemberTrueOrderBy projection. - RelationshipService orchestrates PersonService + the inference service; never reaches into PersonRepository directly. addRelationship guards self-relationship, year range, circular PARENT_OF (Nora B2), and DataIntegrityViolation→DUPLICATE_RELATIONSHIP. deleteRelationship enforces ownership from either side (Nora B1). - Extend RelationshipDTO with personDisplayName + birth/death year so the frontend can render rows from either viewpoint. - 8 unit tests, written against a stub (red), then green: FORBIDDEN delete, CIRCULAR add, DUPLICATE add, self-relationship, year range, happy-path persistence, ownership-from-object, RELATIONSHIP_NOT_FOUND. Full backend suite: 1399/1399 green. Refs #358. Co-Authored-By: Claude Sonnet 4.6 --- .../relationship/RelationshipService.java | 179 +++++++++++++++++ .../relationship/dto/RelationshipDTO.java | 14 +- .../repository/PersonRepository.java | 3 + .../familienarchiv/service/PersonService.java | 11 ++ .../relationship/RelationshipServiceTest.java | 185 ++++++++++++++++++ 5 files changed, 389 insertions(+), 3 deletions(-) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/relationship/RelationshipService.java create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/relationship/RelationshipServiceTest.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/relationship/RelationshipService.java b/backend/src/main/java/org/raddatz/familienarchiv/relationship/RelationshipService.java new file mode 100644 index 00000000..03ed5d54 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/relationship/RelationshipService.java @@ -0,0 +1,179 @@ +package org.raddatz.familienarchiv.relationship; + +import lombok.RequiredArgsConstructor; +import org.raddatz.familienarchiv.exception.DomainException; +import org.raddatz.familienarchiv.exception.ErrorCode; +import org.raddatz.familienarchiv.model.Person; +import org.raddatz.familienarchiv.relationship.dto.CreateRelationshipRequest; +import org.raddatz.familienarchiv.relationship.dto.InferredRelationshipDTO; +import org.raddatz.familienarchiv.relationship.dto.InferredRelationshipWithPersonDTO; +import org.raddatz.familienarchiv.relationship.dto.NetworkDTO; +import org.raddatz.familienarchiv.relationship.dto.PersonNodeDTO; +import org.raddatz.familienarchiv.relationship.dto.RelationshipDTO; +import org.raddatz.familienarchiv.service.PersonService; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.UUID; + +/** + * Owns the {@code person_relationships} table and the family_member flag. + * Always orchestrates {@link PersonService} for cross-domain access — never + * touches {@link org.raddatz.familienarchiv.repository.PersonRepository}. + */ +@Service +@RequiredArgsConstructor +public class RelationshipService { + + private final PersonRelationshipRepository relationshipRepository; + private final PersonService personService; + private final RelationshipInferenceService inferenceService; + + public List getRelationships(UUID personId) { + personService.getById(personId); + List rels = relationshipRepository.findAllByPersonIdOrRelatedPersonId(personId); + return rels.stream().map(RelationshipService::toDTO).toList(); + } + + public List getInferredRelationships(UUID personId) { + personService.getById(personId); + return inferenceService.findAllFor(personId); + } + + public Optional getRelationshipBetween(UUID a, UUID b) { + personService.getById(a); + personService.getById(b); + return inferenceService.infer(a, b); + } + + public NetworkDTO getFamilyNetwork() { + // Two queries: 1 for nodes (family members), 1 for edges (family-graph types). + List familyMembers = personService.findAllFamilyMembers(); + Set familyIds = new HashSet<>(familyMembers.size()); + List nodes = new ArrayList<>(familyMembers.size()); + for (Person p : familyMembers) { + familyIds.add(p.getId()); + nodes.add(new PersonNodeDTO( + p.getId(), p.getDisplayName(), p.getBirthYear(), p.getDeathYear(), true)); + } + + List familyEdges = relationshipRepository.findAllByRelationTypeIn( + List.of(RelationType.PARENT_OF, RelationType.SPOUSE_OF, RelationType.SIBLING_OF)); + + List edges = new ArrayList<>(); + for (PersonRelationship r : familyEdges) { + UUID p = r.getPerson().getId(); + UUID rp = r.getRelatedPerson().getId(); + if (familyIds.contains(p) && familyIds.contains(rp)) { + edges.add(toDTO(r)); + } + } + return new NetworkDTO(nodes, edges); + } + + @Transactional + public RelationshipDTO addRelationship(UUID personId, CreateRelationshipRequest dto) { + if (personId.equals(dto.relatedPersonId())) { + throw DomainException.badRequest( + ErrorCode.VALIDATION_ERROR, "Cannot relate a person to themselves"); + } + Person person = personService.getById(personId); + Person relatedPerson = personService.getById(dto.relatedPersonId()); + + RelationType type = parseType(dto.relationType()); + validateYears(dto.fromYear(), dto.toYear()); + + if (type == RelationType.PARENT_OF + && relationshipRepository.existsByPersonIdAndRelatedPersonIdAndRelationType( + relatedPerson.getId(), personId, RelationType.PARENT_OF)) { + throw DomainException.conflict( + ErrorCode.CIRCULAR_RELATIONSHIP, + "Reverse PARENT_OF already exists between " + personId + " and " + relatedPerson.getId()); + } + + PersonRelationship rel = PersonRelationship.builder() + .person(person) + .relatedPerson(relatedPerson) + .relationType(type) + .fromYear(dto.fromYear()) + .toYear(dto.toYear()) + .notes(blankToNull(dto.notes())) + .build(); + + try { + return toDTO(relationshipRepository.save(rel)); + } catch (DataIntegrityViolationException e) { + throw DomainException.conflict( + ErrorCode.DUPLICATE_RELATIONSHIP, + "Relationship already exists for (" + personId + ", " + relatedPerson.getId() + ", " + type + ")"); + } + } + + @Transactional + public void deleteRelationship(UUID personId, UUID relId) { + PersonRelationship rel = relationshipRepository.findById(relId) + .orElseThrow(() -> DomainException.notFound( + ErrorCode.RELATIONSHIP_NOT_FOUND, "Relationship not found: " + relId)); + + UUID storageSubject = rel.getPerson().getId(); + UUID storageObject = rel.getRelatedPerson().getId(); + if (!personId.equals(storageSubject) && !personId.equals(storageObject)) { + throw DomainException.forbidden( + "Relationship " + relId + " does not belong to person " + personId); + } + relationshipRepository.delete(rel); + } + + @Transactional + public Person setFamilyMember(UUID personId, boolean familyMember) { + return personService.setFamilyMember(personId, familyMember); + } + + private static String blankToNull(String s) { + return (s == null || s.isBlank()) ? null : s.trim(); + } + + private static RelationType parseType(String typeName) { + if (typeName == null) { + throw DomainException.badRequest(ErrorCode.VALIDATION_ERROR, "relationType is required"); + } + try { + return RelationType.valueOf(typeName); + } catch (IllegalArgumentException e) { + throw DomainException.badRequest( + ErrorCode.VALIDATION_ERROR, "Invalid relationType: " + typeName); + } + } + + private static void validateYears(Integer fromYear, Integer toYear) { + if (fromYear != null && toYear != null && toYear < fromYear) { + throw DomainException.badRequest( + ErrorCode.VALIDATION_ERROR, "toYear must not be before fromYear"); + } + } + + private static RelationshipDTO toDTO(PersonRelationship r) { + Person p = r.getPerson(); + Person rp = r.getRelatedPerson(); + return new RelationshipDTO( + r.getId(), + p.getId(), + rp.getId(), + p.getDisplayName(), + p.getBirthYear(), + p.getDeathYear(), + rp.getDisplayName(), + rp.getBirthYear(), + rp.getDeathYear(), + r.getRelationType(), + r.getFromYear(), + r.getToYear(), + r.getNotes()); + } +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/relationship/dto/RelationshipDTO.java b/backend/src/main/java/org/raddatz/familienarchiv/relationship/dto/RelationshipDTO.java index 82792ea0..3ecfe018 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/relationship/dto/RelationshipDTO.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/relationship/dto/RelationshipDTO.java @@ -6,14 +6,22 @@ import org.raddatz.familienarchiv.relationship.RelationType; import java.util.UUID; /** - * Wire shape for a stored relationship row. Carries enough context for the - * frontend to render a chip (type), a name (relatedPersonDisplayName), a year - * range, and a delete action (id). + * Wire shape for one stored relationship row. Both sides include name + years + * so the frontend can render the row from either perspective (e.g. on the + * subject's page the row reads "Elternteil von [related]"; on the object's + * page it reads "Kind von [person]"). + * + *

Storage truth: {@code personId} is the {@code person_id} column, + * {@code relatedPersonId} is the {@code related_person_id} column. The + * frontend determines orientation by comparing against the viewpoint. */ public record RelationshipDTO( @Schema(requiredMode = Schema.RequiredMode.REQUIRED) UUID id, @Schema(requiredMode = Schema.RequiredMode.REQUIRED) UUID personId, @Schema(requiredMode = Schema.RequiredMode.REQUIRED) UUID relatedPersonId, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) String personDisplayName, + Integer personBirthYear, + Integer personDeathYear, @Schema(requiredMode = Schema.RequiredMode.REQUIRED) String relatedPersonDisplayName, Integer relatedPersonBirthYear, Integer relatedPersonDeathYear, diff --git a/backend/src/main/java/org/raddatz/familienarchiv/repository/PersonRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/repository/PersonRepository.java index 6138510f..3a5eb3af 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/repository/PersonRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/repository/PersonRepository.java @@ -26,6 +26,9 @@ public interface PersonRepository extends JpaRepository { // Hilfsmethode: Alle sortiert laden (für den leeren Status) List findAllByOrderByLastNameAscFirstNameAsc(); + // Stammbaum-Knoten: alle Personen mit family_member = true. + List findByFamilyMemberTrueOrderByLastNameAscFirstNameAsc(); + // Lookup by full alias string, used during ODS mass import Optional findByAliasIgnoreCase(String alias); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java index 93ccdd5e..df04aa38 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java @@ -58,6 +58,17 @@ public class PersonService { return personRepository.findAllById(ids); } + public List findAllFamilyMembers() { + return personRepository.findByFamilyMemberTrueOrderByLastNameAscFirstNameAsc(); + } + + @Transactional + public Person setFamilyMember(UUID personId, boolean familyMember) { + Person person = getById(personId); + person.setFamilyMember(familyMember); + return personRepository.save(person); + } + public Optional findByName(String firstName, String lastName) { return personRepository.findByFirstNameIgnoreCaseAndLastNameIgnoreCase(firstName, lastName); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/relationship/RelationshipServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/relationship/RelationshipServiceTest.java new file mode 100644 index 00000000..4383796a --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/relationship/RelationshipServiceTest.java @@ -0,0 +1,185 @@ +package org.raddatz.familienarchiv.relationship; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.raddatz.familienarchiv.exception.DomainException; +import org.raddatz.familienarchiv.exception.ErrorCode; +import org.raddatz.familienarchiv.model.Person; +import org.raddatz.familienarchiv.relationship.dto.CreateRelationshipRequest; +import org.raddatz.familienarchiv.service.PersonService; +import org.springframework.dao.DataIntegrityViolationException; + +import java.time.Instant; +import java.util.Optional; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Felix Brandt — TDD red for RelationshipService domain rules. + * + *

Required by the plan (Nora blockers 1 + 2): + *

    + *
  • {@code deleteRelationship_throws_FORBIDDEN_when_rel_belongs_to_different_person}
    • + *
  • {@code addRelationship_throws_CIRCULAR_when_reverse_PARENT_OF_exists}
    • + *
+ * Plus: duplicate constraint, self-relationship, year-range, happy-path persistence, + * and ownership permitted from either side. + */ +@ExtendWith(MockitoExtension.class) +class RelationshipServiceTest { + + @Mock PersonRelationshipRepository relationshipRepository; + @Mock PersonService personService; + @Mock RelationshipInferenceService inferenceService; + @InjectMocks RelationshipService service; + + Person alice; + Person bob; + Person charlie; + + @BeforeEach + void seed() { + alice = person("Alice"); + bob = person("Bob"); + charlie = person("Charlie"); + } + + // --- Nora blocker 1 --- + @Test + void deleteRelationship_throws_FORBIDDEN_when_rel_belongs_to_different_person() { + UUID relId = UUID.randomUUID(); + PersonRelationship rel = parentOf(alice, bob, relId); + when(relationshipRepository.findById(relId)).thenReturn(Optional.of(rel)); + + assertThatThrownBy(() -> service.deleteRelationship(charlie.getId(), relId)) + .isInstanceOf(DomainException.class) + .extracting("code") + .isEqualTo(ErrorCode.FORBIDDEN); + verify(relationshipRepository, never()).delete(any()); + } + + // --- Nora blocker 2 --- + @Test + void addRelationship_throws_CIRCULAR_when_reverse_PARENT_OF_exists() { + // alice PARENT_OF bob already exists. Now we try to add bob PARENT_OF alice. + when(personService.getById(bob.getId())).thenReturn(bob); + when(personService.getById(alice.getId())).thenReturn(alice); + when(relationshipRepository.existsByPersonIdAndRelatedPersonIdAndRelationType( + alice.getId(), bob.getId(), RelationType.PARENT_OF)).thenReturn(true); + + var dto = new CreateRelationshipRequest(alice.getId(), "PARENT_OF", null, null, null); + assertThatThrownBy(() -> service.addRelationship(bob.getId(), dto)) + .isInstanceOf(DomainException.class) + .extracting("code") + .isEqualTo(ErrorCode.CIRCULAR_RELATIONSHIP); + verify(relationshipRepository, never()).save(any()); + } + + @Test + void addRelationship_throws_DUPLICATE_when_db_constraint_violated() { + when(personService.getById(alice.getId())).thenReturn(alice); + when(personService.getById(bob.getId())).thenReturn(bob); + when(relationshipRepository.existsByPersonIdAndRelatedPersonIdAndRelationType( + bob.getId(), alice.getId(), RelationType.PARENT_OF)).thenReturn(false); + when(relationshipRepository.save(any())).thenThrow(new DataIntegrityViolationException("unique_rel")); + + var dto = new CreateRelationshipRequest(bob.getId(), "PARENT_OF", null, null, null); + assertThatThrownBy(() -> service.addRelationship(alice.getId(), dto)) + .isInstanceOf(DomainException.class) + .extracting("code") + .isEqualTo(ErrorCode.DUPLICATE_RELATIONSHIP); + } + + @Test + void addRelationship_throws_BAD_REQUEST_when_self_relationship() { + var dto = new CreateRelationshipRequest(alice.getId(), "FRIEND", null, null, null); + assertThatThrownBy(() -> service.addRelationship(alice.getId(), dto)) + .isInstanceOf(DomainException.class) + .extracting("code") + .isEqualTo(ErrorCode.VALIDATION_ERROR); + verify(relationshipRepository, never()).save(any()); + } + + @Test + void addRelationship_throws_BAD_REQUEST_when_to_year_before_from_year() { + when(personService.getById(alice.getId())).thenReturn(alice); + when(personService.getById(bob.getId())).thenReturn(bob); + var dto = new CreateRelationshipRequest(bob.getId(), "FRIEND", 1950, 1940, null); + assertThatThrownBy(() -> service.addRelationship(alice.getId(), dto)) + .isInstanceOf(DomainException.class) + .extracting("code") + .isEqualTo(ErrorCode.VALIDATION_ERROR); + verify(relationshipRepository, never()).save(any()); + } + + @Test + void addRelationship_persists_with_storage_truth() { + when(personService.getById(alice.getId())).thenReturn(alice); + when(personService.getById(bob.getId())).thenReturn(bob); + when(relationshipRepository.existsByPersonIdAndRelatedPersonIdAndRelationType( + bob.getId(), alice.getId(), RelationType.PARENT_OF)).thenReturn(false); + when(relationshipRepository.save(any())).thenAnswer(inv -> { + PersonRelationship r = inv.getArgument(0); + r.setId(UUID.randomUUID()); + r.setCreatedAt(Instant.now()); + return r; + }); + + var dto = new CreateRelationshipRequest(bob.getId(), "PARENT_OF", 1900, null, "first born"); + var result = service.addRelationship(alice.getId(), dto); + + assertThat(result.personId()).isEqualTo(alice.getId()); + assertThat(result.relatedPersonId()).isEqualTo(bob.getId()); + assertThat(result.relationType()).isEqualTo(RelationType.PARENT_OF); + assertThat(result.fromYear()).isEqualTo(1900); + assertThat(result.notes()).isEqualTo("first born"); + } + + @Test + void deleteRelationship_succeeds_when_viewpoint_is_object() { + UUID relId = UUID.randomUUID(); + PersonRelationship rel = parentOf(alice, bob, relId); + when(relationshipRepository.findById(relId)).thenReturn(Optional.of(rel)); + + // Bob is the storage related_person; deleting from his viewpoint should work. + service.deleteRelationship(bob.getId(), relId); + verify(relationshipRepository).delete(rel); + } + + @Test + void deleteRelationship_throws_NOT_FOUND_when_relId_unknown() { + UUID relId = UUID.randomUUID(); + when(relationshipRepository.findById(relId)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> service.deleteRelationship(alice.getId(), relId)) + .isInstanceOf(DomainException.class) + .extracting("code") + .isEqualTo(ErrorCode.RELATIONSHIP_NOT_FOUND); + } + + // --- helpers --- + + private static Person person(String name) { + return Person.builder().id(UUID.randomUUID()).lastName(name).familyMember(true).build(); + } + + private static PersonRelationship parentOf(Person parent, Person child, UUID id) { + return PersonRelationship.builder() + .id(id) + .person(parent) + .relatedPerson(child) + .relationType(RelationType.PARENT_OF) + .createdAt(Instant.now()) + .build(); + } +}