marcel/familienarchiv
1
0
Fork 0
Code Issues 109 Pull Requests Actions Packages Projects Releases Wiki Activity

feat(relationship): add PUT update endpoint, align DELETE mismatch to 404

Browse Source 
PUT /api/persons/{id}/relationships/{relId} (@RequirePermission WRITE_ALL)
updates a relationship's type, related person, dates and notes in place,
preserving the original createdAt. The update re-runs every create invariant —
self-relation (VALIDATION_ERROR), date coherence/order, reverse PARENT_OF
(CIRCULAR_RELATIONSHIP) and the (person, relatedPerson, type) unique constraint
via saveAndFlush (DUPLICATE_RELATIONSHIP) — and re-flags both endpoints as
family members when edited into a family type. The directed orientation is
preserved per viewpoint, so a PARENT_OF edge stays parent->child whether edited
from either person's page.

Ownership mismatch now returns 404 RELATIONSHIP_NOT_FOUND (shared loadOwned
helper) for both PUT and DELETE — anti-enumeration, replacing DELETE's former
403. No @Version: last-write-wins, matching person edit (single-writer archive).

REQ-004, REQ-005, REQ-006, REQ-007, REQ-008, REQ-009, REQ-012, REQ-013, REQ-018

Refs #837
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-06-14 18:36:43 +02:00
parent 0f6e9f7bc7
commit 6d2b6f3d2b
5 changed files with 291 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
backend/src/main/java/org/raddatz/familienarchiv/person/relationship/RelationshipController.java
View File

@@ -68,6 +68,15 @@ public class RelationshipController {
.body(relationshipService.addRelationship(id, dto));
}
@PutMapping("/api/persons/{id}/relationships/{relId}")
@RequirePermission(Permission.WRITE_ALL)
public RelationshipDTO updateRelationship(
@PathVariable UUID id,
@PathVariable UUID relId,
@Valid @RequestBody RelationshipUpsertRequest dto) {
return relationshipService.updateRelationship(id, relId, dto);
}
@DeleteMapping("/api/persons/{id}/relationships/{relId}")
@ResponseStatus(HttpStatus.NO_CONTENT)
@RequirePermission(Permission.WRITE_ALL)

68
backend/src/main/java/org/raddatz/familienarchiv/person/relationship/RelationshipService.java
View File

@@ -145,19 +145,81 @@ public class RelationshipService {
return toDTO(saved);
}
@Transactional
public RelationshipDTO updateRelationship(UUID personId, UUID relId, RelationshipUpsertRequest dto) {
PersonRelationship rel = loadOwnedRelationship(personId, relId);
// The other party from {personId}'s viewpoint cannot be {personId} itself.
if (personId.equals(dto.relatedPersonId())) {
throw DomainException.badRequest(
ErrorCode.VALIDATION_ERROR, "Cannot relate a person to themselves");
}
validateRelationshipDates(dto.fromDate(), dto.fromDatePrecision(), dto.toDate(), dto.toDatePrecision());
// Preserve the directed orientation: {personId} keeps whichever role (subject or
// object) it already holds on the row, and the edited "related person" takes the
// other role. So a PARENT_OF edge stays parent→child whether the curator edits it
// from the parent's page or the child's.
Person viewpoint = personId.equals(rel.getPerson().getId()) ? rel.getPerson() : rel.getRelatedPerson();
Person other = personService.getById(dto.relatedPersonId());
boolean viewpointIsSubject = personId.equals(rel.getPerson().getId());
Person newSubject = viewpointIsSubject ? viewpoint : other;
Person newObject = viewpointIsSubject ? other : viewpoint;
if (dto.relationType() == RelationType.PARENT_OF
&& relationshipRepository.existsByPersonIdAndRelatedPersonIdAndRelationType(
newObject.getId(), newSubject.getId(), RelationType.PARENT_OF)) {
throw DomainException.conflict(
ErrorCode.CIRCULAR_RELATIONSHIP,
"Reverse PARENT_OF already exists between " + newSubject.getId() + " and " + newObject.getId());
}
rel.setPerson(newSubject);
rel.setRelatedPerson(newObject);
rel.setRelationType(dto.relationType());
rel.setFromDate(dto.fromDate());
rel.setFromDatePrecision(normalizePrecision(dto.fromDatePrecision()));
rel.setToDate(dto.toDate());
rel.setToDatePrecision(normalizePrecision(dto.toDatePrecision()));
rel.setNotes(blankToNull(dto.notes()));
PersonRelationship saved;
try {
saved = relationshipRepository.saveAndFlush(rel);
} catch (DataIntegrityViolationException e) {
throw DomainException.conflict(
ErrorCode.DUPLICATE_RELATIONSHIP,
"Relationship already exists for (" + newSubject.getId() + ", " + newObject.getId() + ", " + dto.relationType() + ")");
}
// An edit into a family type flags both endpoints; never auto-unflags (additive).
if (FAMILY_RELATION_TYPES.contains(dto.relationType())) {
personService.setFamilyMember(newSubject.getId(), true);
personService.setFamilyMember(newObject.getId(), true);
}
return toDTO(saved);
}
@Transactional
public void deleteRelationship(UUID personId, UUID relId) {
PersonRelationship rel = loadOwnedRelationship(personId, relId);
relationshipRepository.delete(rel);
}
// Loads the row and verifies {personId} is one of its endpoints. A mismatch is 404
// (not 403): an anti-enumeration choice so a curator cannot probe relationship ids
// belonging to people they cannot see. Shared by update + delete for consistency.
private PersonRelationship loadOwnedRelationship(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(
throw DomainException.notFound(
ErrorCode.RELATIONSHIP_NOT_FOUND,
"Relationship " + relId + " does not belong to person " + personId);
}
relationshipRepository.delete(rel);
return rel;
}
@Transactional

49
backend/src/test/java/org/raddatz/familienarchiv/person/relationship/RelationshipControllerTest.java
View File

@@ -26,6 +26,8 @@ import java.util.UUID;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
@@ -159,4 +161,51 @@ class RelationshipControllerTest {
mockMvc.perform(delete("/api/persons/{id}/relationships/{relId}", PERSON_ID, relId).with(csrf()))
.andExpect(status().isNoContent());
}
// ─── PUT /api/persons/{id}/relationships/{relId} ──────────────────────────
@Test
@WithMockUser(username = "testuser", authorities = {"WRITE_ALL"})
void updateRelationship_returns200_with_RelationshipDTO_for_WRITE_ALL_user() throws Exception {
UUID relId = UUID.randomUUID();
RelationshipDTO updated = new RelationshipDTO(
relId, PERSON_ID, OTHER_ID,
"Alice Müller", null, null,
"Bob Müller", null, null,
RelationType.SPOUSE_OF, null, DatePrecision.UNKNOWN, null, DatePrecision.UNKNOWN, null);
when(relationshipService.updateRelationship(any(), any(), any())).thenReturn(updated);
mockMvc.perform(put("/api/persons/{id}/relationships/{relId}", PERSON_ID, relId).with(csrf())
.contentType(MediaType.APPLICATION_JSON)
.content("{\"relatedPersonId\":\"" + OTHER_ID + "\",\"relationType\":\"SPOUSE_OF\"}"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.relationType").value("SPOUSE_OF"));
}
@Test
void updateRelationship_returns401_whenUnauthenticated_and_does_not_touch_service() throws Exception {
mockMvc.perform(put("/api/persons/{id}/relationships/{relId}", PERSON_ID, UUID.randomUUID()).with(csrf())
.contentType(MediaType.APPLICATION_JSON)
.content("{\"relatedPersonId\":\"" + OTHER_ID + "\",\"relationType\":\"SPOUSE_OF\"}"))
.andExpect(status().isUnauthorized());
verify(relationshipService, never()).updateRelationship(any(), any(), any());
}
@Test
@WithMockUser(username = "testuser", authorities = {"READ_ALL"})
void updateRelationship_returns403_for_READ_ALL_only_user() throws Exception {
mockMvc.perform(put("/api/persons/{id}/relationships/{relId}", PERSON_ID, UUID.randomUUID()).with(csrf())
.contentType(MediaType.APPLICATION_JSON)
.content("{\"relatedPersonId\":\"" + OTHER_ID + "\",\"relationType\":\"SPOUSE_OF\"}"))
.andExpect(status().isForbidden());
}
@Test
@WithMockUser(username = "testuser", authorities = {"WRITE_ALL"})
void updateRelationship_returns400_when_relationType_is_unknown_value() throws Exception {
mockMvc.perform(put("/api/persons/{id}/relationships/{relId}", PERSON_ID, UUID.randomUUID()).with(csrf())
.contentType(MediaType.APPLICATION_JSON)
.content("{\"relatedPersonId\":\"" + OTHER_ID + "\",\"relationType\":\"NOT_A_REAL_TYPE\"}"))
.andExpect(status().isBadRequest());
}
}

36
backend/src/test/java/org/raddatz/familienarchiv/person/relationship/RelationshipServiceIntegrationTest.java
View File

@@ -109,20 +109,50 @@ class RelationshipServiceIntegrationTest {
}
@Test
void deleteRelationship_throws_403_when_rel_belongs_to_different_person() {
void deleteRelationship_throws_404_when_rel_belongs_to_different_person() {
RelationshipDTO created = relationshipService.addRelationship(alice.getId(),
new RelationshipUpsertRequest(bob.getId(), RelationType.PARENT_OF, null, null, null, null, null));
// Charlie is unrelated to this row.
// Charlie is unrelated to this row. Ownership mismatch is 404, not 403, so a
// curator cannot enumerate relationship ids belonging to people they can't see
// (anti-enumeration; aligned with the PUT endpoint — ADR-044).
assertThatThrownBy(() -> relationshipService.deleteRelationship(charlie.getId(), created.id()))
.isInstanceOf(DomainException.class)
.extracting("code")
.isEqualTo(ErrorCode.FORBIDDEN);
.isEqualTo(ErrorCode.RELATIONSHIP_NOT_FOUND);
// The row is still there.
assertThat(relationshipRepository.findById(created.id())).isPresent();
}
@Test
void updateRelationship_persists_new_type_dates_and_notes() {
RelationshipDTO created = relationshipService.addRelationship(alice.getId(),
new RelationshipUpsertRequest(bob.getId(), RelationType.FRIEND, null, null, null, null, null));
RelationshipDTO updated = relationshipService.updateRelationship(alice.getId(), created.id(),
new RelationshipUpsertRequest(bob.getId(), RelationType.SPOUSE_OF,
LocalDate.of(1923, 5, 12), DatePrecision.DAY, null, null, "wedding day"));
assertThat(updated.id()).isEqualTo(created.id());
assertThat(updated.relationType()).isEqualTo(RelationType.SPOUSE_OF);
assertThat(updated.fromDate()).isEqualTo(LocalDate.of(1923, 5, 12));
assertThat(updated.fromDatePrecision()).isEqualTo(DatePrecision.DAY);
assertThat(updated.notes()).isEqualTo("wedding day");
}
@Test
void updateRelationship_throws_404_when_rel_belongs_to_different_person() {
RelationshipDTO created = relationshipService.addRelationship(alice.getId(),
new RelationshipUpsertRequest(bob.getId(), RelationType.PARENT_OF, null, null, null, null, null));
assertThatThrownBy(() -> relationshipService.updateRelationship(charlie.getId(), created.id(),
new RelationshipUpsertRequest(bob.getId(), RelationType.FRIEND, null, null, null, null, null)))
.isInstanceOf(DomainException.class)
.extracting("code")
.isEqualTo(ErrorCode.RELATIONSHIP_NOT_FOUND);
}
@Test
void addRelationship_throws_409_when_reverse_SPOUSE_OF_pair_already_exists() {
// V55 enforces symmetric uniqueness for SPOUSE_OF. Inserting (alice, bob, SPOUSE_OF)

141
backend/src/test/java/org/raddatz/familienarchiv/person/relationship/RelationshipServiceTest.java
View File

@@ -61,9 +61,9 @@ class RelationshipServiceTest {
charlie = person("Charlie");
}
// --- Nora blocker 1 ---
// --- Nora blocker 1 (anti-enumeration: ownership mismatch is 404, aligned with PUT) ---
@Test
void deleteRelationship_throws_FORBIDDEN_when_rel_belongs_to_different_person() {
void deleteRelationship_throws_NOT_FOUND_when_rel_belongs_to_different_person() {
UUID relId = UUID.randomUUID();
PersonRelationship rel = parentOf(alice, bob, relId);
when(relationshipRepository.findById(relId)).thenReturn(Optional.of(rel));
@@ -71,7 +71,7 @@ class RelationshipServiceTest {
assertThatThrownBy(() -> service.deleteRelationship(charlie.getId(), relId))
.isInstanceOf(DomainException.class)
.extracting("code")
.isEqualTo(ErrorCode.FORBIDDEN);
.isEqualTo(ErrorCode.RELATIONSHIP_NOT_FOUND);
verify(relationshipRepository, never()).delete(any());
}
@@ -249,6 +249,131 @@ class RelationshipServiceTest {
.isEqualTo(ErrorCode.RELATIONSHIP_NOT_FOUND);
}
// --- updateRelationship (REQ-004/006/007/008/009/010/013) ---
@Test
void updateRelationship_throws_NOT_FOUND_when_relId_unknown() {
UUID relId = UUID.randomUUID();
when(relationshipRepository.findById(relId)).thenReturn(Optional.empty());
var dto = new RelationshipUpsertRequest(bob.getId(), RelationType.FRIEND, null, null, null, null, null);
assertThatThrownBy(() -> service.updateRelationship(alice.getId(), relId, dto))
.isInstanceOf(DomainException.class)
.extracting("code")
.isEqualTo(ErrorCode.RELATIONSHIP_NOT_FOUND);
verify(relationshipRepository, never()).saveAndFlush(any());
}
@Test
void updateRelationship_throws_NOT_FOUND_when_rel_belongs_to_different_person() {
UUID relId = UUID.randomUUID();
PersonRelationship rel = parentOf(alice, bob, relId);
when(relationshipRepository.findById(relId)).thenReturn(Optional.of(rel));
var dto = new RelationshipUpsertRequest(bob.getId(), RelationType.FRIEND, null, null, null, null, null);
assertThatThrownBy(() -> service.updateRelationship(charlie.getId(), relId, dto))
.isInstanceOf(DomainException.class)
.extracting("code")
.isEqualTo(ErrorCode.RELATIONSHIP_NOT_FOUND);
verify(relationshipRepository, never()).saveAndFlush(any());
}
@Test
void updateRelationship_throws_VALIDATION_ERROR_on_self_relation() {
UUID relId = UUID.randomUUID();
PersonRelationship rel = relOf(alice, bob, RelationType.FRIEND, relId);
when(relationshipRepository.findById(relId)).thenReturn(Optional.of(rel));
var dto = new RelationshipUpsertRequest(alice.getId(), RelationType.FRIEND, null, null, null, null, null);
assertThatThrownBy(() -> service.updateRelationship(alice.getId(), relId, dto))
.isInstanceOf(DomainException.class)
.extracting("code")
.isEqualTo(ErrorCode.VALIDATION_ERROR);
verify(relationshipRepository, never()).saveAndFlush(any());
}
@Test
void updateRelationship_throws_INVALID_RELATIONSHIP_DATES_when_toDate_before_fromDate() {
UUID relId = UUID.randomUUID();
PersonRelationship rel = relOf(alice, bob, RelationType.FRIEND, relId);
when(relationshipRepository.findById(relId)).thenReturn(Optional.of(rel));
var dto = new RelationshipUpsertRequest(bob.getId(), RelationType.FRIEND,
LocalDate.of(1950, 1, 1), DatePrecision.YEAR,
LocalDate.of(1940, 1, 1), DatePrecision.YEAR, null);
assertThatThrownBy(() -> service.updateRelationship(alice.getId(), relId, dto))
.isInstanceOf(DomainException.class)
.extracting("code")
.isEqualTo(ErrorCode.INVALID_RELATIONSHIP_DATES);
verify(relationshipRepository, never()).saveAndFlush(any());
}
@Test
void updateRelationship_throws_CIRCULAR_when_reverse_PARENT_OF_exists() {
UUID relId = UUID.randomUUID();
PersonRelationship rel = relOf(alice, bob, RelationType.FRIEND, relId);
when(relationshipRepository.findById(relId)).thenReturn(Optional.of(rel));
when(personService.getById(bob.getId())).thenReturn(bob);
when(relationshipRepository.existsByPersonIdAndRelatedPersonIdAndRelationType(
bob.getId(), alice.getId(), RelationType.PARENT_OF)).thenReturn(true);
var dto = new RelationshipUpsertRequest(bob.getId(), RelationType.PARENT_OF, null, null, null, null, null);
assertThatThrownBy(() -> service.updateRelationship(alice.getId(), relId, dto))
.isInstanceOf(DomainException.class)
.extracting("code")
.isEqualTo(ErrorCode.CIRCULAR_RELATIONSHIP);
verify(relationshipRepository, never()).saveAndFlush(any());
}
@Test
void updateRelationship_throws_DUPLICATE_when_db_constraint_violated() {
UUID relId = UUID.randomUUID();
PersonRelationship rel = relOf(alice, bob, RelationType.FRIEND, relId);
when(relationshipRepository.findById(relId)).thenReturn(Optional.of(rel));
when(personService.getById(bob.getId())).thenReturn(bob);
when(relationshipRepository.saveAndFlush(any())).thenThrow(new DataIntegrityViolationException("unique_rel"));
var dto = new RelationshipUpsertRequest(bob.getId(), RelationType.SPOUSE_OF, null, null, null, null, null);
assertThatThrownBy(() -> service.updateRelationship(alice.getId(), relId, dto))
.isInstanceOf(DomainException.class)
.extracting("code")
.isEqualTo(ErrorCode.DUPLICATE_RELATIONSHIP);
}
@Test
void updateRelationship_updates_fields_and_returns_dto() {
UUID relId = UUID.randomUUID();
PersonRelationship rel = relOf(alice, bob, RelationType.FRIEND, relId);
when(relationshipRepository.findById(relId)).thenReturn(Optional.of(rel));
when(personService.getById(bob.getId())).thenReturn(bob);
when(relationshipRepository.saveAndFlush(any())).thenAnswer(inv -> inv.getArgument(0));
var dto = new RelationshipUpsertRequest(bob.getId(), RelationType.SPOUSE_OF,
LocalDate.of(1923, 5, 12), DatePrecision.DAY, null, null, "wedding day");
var result = service.updateRelationship(alice.getId(), relId, dto);
assertThat(result.relationType()).isEqualTo(RelationType.SPOUSE_OF);
assertThat(result.fromDate()).isEqualTo(LocalDate.of(1923, 5, 12));
assertThat(result.fromDatePrecision()).isEqualTo(DatePrecision.DAY);
assertThat(result.toDatePrecision()).isEqualTo(DatePrecision.UNKNOWN);
assertThat(result.notes()).isEqualTo("wedding day");
}
@Test
void updateRelationship_marks_both_endpoints_family_when_updated_to_family_type() {
UUID relId = UUID.randomUUID();
PersonRelationship rel = relOf(alice, bob, RelationType.FRIEND, relId);
when(relationshipRepository.findById(relId)).thenReturn(Optional.of(rel));
when(personService.getById(bob.getId())).thenReturn(bob);
when(relationshipRepository.saveAndFlush(any())).thenAnswer(inv -> inv.getArgument(0));
var dto = new RelationshipUpsertRequest(bob.getId(), RelationType.SIBLING_OF, null, null, null, null, null);
service.updateRelationship(alice.getId(), relId, dto);
verify(personService).setFamilyMember(alice.getId(), true);
verify(personService).setFamilyMember(bob.getId(), true);
}
@Test
void getFamilyNetwork_excludes_edges_where_one_endpoint_is_not_a_family_member() {
// alice and bob are family members; charlie is NOT (not in findAllFamilyMembers result).
@@ -293,11 +418,15 @@ class RelationshipServiceTest {
}
private static PersonRelationship parentOf(Person parent, Person child, UUID id) {
return relOf(parent, child, RelationType.PARENT_OF, id);
}
private static PersonRelationship relOf(Person subject, Person object, RelationType type, UUID id) {
return PersonRelationship.builder()
.id(id)
.person(parent)
.relatedPerson(child)
.relationType(RelationType.PARENT_OF)
.person(subject)
.relatedPerson(object)
.relationType(type)
.createdAt(Instant.now())
.build();
}