From 3fba7404695691393358d896a5be520fcb8ee74d Mon Sep 17 00:00:00 2001 From: Marcel Date: Thu, 16 Apr 2026 15:26:23 +0200 Subject: [PATCH] feat(#221): tag entity hierarchy fields, service, repository, controller - Tag entity: add parentId (UUID FK) and color (String) fields - TagUpdateDTO and TagTreeNodeDTO records - ErrorCode: INVALID_TAG_COLOR, TAG_CYCLE_DETECTED - TagRepository: findAncestorIds() recursive CTE query - TagService: cycle detection, color validation, getTagTree() - TagController: use TagUpdateDTO, add GET /api/tags/tree endpoint Co-Authored-By: Claude Sonnet 4.6 --- .../controller/TagController.java | 12 +- .../familienarchiv/dto/TagTreeNodeDTO.java | 6 + .../familienarchiv/dto/TagUpdateDTO.java | 5 + .../familienarchiv/exception/ErrorCode.java | 6 + .../org/raddatz/familienarchiv/model/Tag.java | 7 + .../repository/TagRepository.java | 26 +++- .../familienarchiv/service/TagService.java | 100 ++++++++++++- .../controller/TagControllerTest.java | 29 +++- .../service/TagServiceTest.java | 132 +++++++++++++++++- 9 files changed, 313 insertions(+), 10 deletions(-) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/dto/TagTreeNodeDTO.java create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/dto/TagUpdateDTO.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/TagController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/TagController.java index c3d99299..71bf1263 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/TagController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/TagController.java @@ -1,9 +1,10 @@ package org.raddatz.familienarchiv.controller; import java.util.List; -import java.util.Map; import java.util.UUID; +import org.raddatz.familienarchiv.dto.TagTreeNodeDTO; +import org.raddatz.familienarchiv.dto.TagUpdateDTO; import org.raddatz.familienarchiv.model.Tag; import org.raddatz.familienarchiv.security.Permission; import org.raddatz.familienarchiv.security.RequirePermission; @@ -31,8 +32,8 @@ public class TagController { @PutMapping("/{id}") @RequirePermission(Permission.ADMIN_TAG) - public ResponseEntity updateTag(@PathVariable UUID id, @RequestBody Map payload) { - return ResponseEntity.ok(tagService.update(id, payload.get("name"))); + public ResponseEntity updateTag(@PathVariable UUID id, @RequestBody TagUpdateDTO dto) { + return ResponseEntity.ok(tagService.update(id, dto)); } @DeleteMapping("/{id}") @@ -46,4 +47,9 @@ public class TagController { public List searchTags(@RequestParam(defaultValue = "") String query) { return tagService.search(query); } + + @GetMapping("/tree") + public List getTagTree() { + return tagService.getTagTree(); + } } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/dto/TagTreeNodeDTO.java b/backend/src/main/java/org/raddatz/familienarchiv/dto/TagTreeNodeDTO.java new file mode 100644 index 00000000..14e59da9 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/dto/TagTreeNodeDTO.java @@ -0,0 +1,6 @@ +package org.raddatz.familienarchiv.dto; + +import java.util.List; +import java.util.UUID; + +public record TagTreeNodeDTO(UUID id, String name, String color, int documentCount, List children) {} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/dto/TagUpdateDTO.java b/backend/src/main/java/org/raddatz/familienarchiv/dto/TagUpdateDTO.java new file mode 100644 index 00000000..7b840228 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/dto/TagUpdateDTO.java @@ -0,0 +1,5 @@ +package org.raddatz.familienarchiv.dto; + +import java.util.UUID; + +public record TagUpdateDTO(String name, UUID parentId, String color) {} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java index 26aab838..279b9cef 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java @@ -78,6 +78,12 @@ public enum ErrorCode { /** A training run is already in progress. 409 */ TRAINING_ALREADY_RUNNING, + // --- Tags --- + /** The supplied color token is not in the allowed palette. 400 */ + INVALID_TAG_COLOR, + /** Setting this parent would create a cycle in the tag hierarchy. 400 */ + TAG_CYCLE_DETECTED, + // --- Generic --- /** Request validation failed (missing or malformed fields). 400 */ VALIDATION_ERROR, diff --git a/backend/src/main/java/org/raddatz/familienarchiv/model/Tag.java b/backend/src/main/java/org/raddatz/familienarchiv/model/Tag.java index 5063ffa3..59c173f3 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/model/Tag.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/model/Tag.java @@ -20,4 +20,11 @@ public class Tag { @Column(unique = true, nullable = false) @Schema(requiredMode = Schema.RequiredMode.REQUIRED) private String name; + + /** UUID of the parent tag, or null for root-level tags. */ + @Column(name = "parent_id") + private UUID parentId; + + /** Color token name (e.g. "sage"), only set on root-level tags. Null means no color. */ + private String color; } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/repository/TagRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/repository/TagRepository.java index d4e58a04..a9f2c11d 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/repository/TagRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/repository/TagRepository.java @@ -6,8 +6,32 @@ import java.util.UUID; import org.raddatz.familienarchiv.model.Tag; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; public interface TagRepository extends JpaRepository { + Optional findByNameIgnoreCase(String name); + List findByNameContainingIgnoreCase(String name); -} \ No newline at end of file + + /** + * Returns the IDs of all ancestors of the given tag (parent, grandparent, …) + * via a recursive CTE. Used for cycle detection before assigning a new parent. + * Includes a depth guard of 50 levels to prevent runaway queries. + */ + @Query(value = """ + WITH RECURSIVE ancestors AS ( + SELECT parent_id, 0 AS depth + FROM tag + WHERE id = :tagId AND parent_id IS NOT NULL + UNION ALL + SELECT t.parent_id, a.depth + 1 + FROM tag t + JOIN ancestors a ON t.id = a.parent_id + WHERE t.parent_id IS NOT NULL AND a.depth < 50 + ) + SELECT parent_id FROM ancestors + """, nativeQuery = true) + List findAncestorIds(@Param("tagId") UUID tagId); +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/TagService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/TagService.java index 06b8b862..336ed624 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/TagService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/TagService.java @@ -1,8 +1,16 @@ package org.raddatz.familienarchiv.service; +import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.UUID; +import org.raddatz.familienarchiv.dto.TagTreeNodeDTO; +import org.raddatz.familienarchiv.dto.TagUpdateDTO; +import org.raddatz.familienarchiv.exception.DomainException; +import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.Tag; import org.raddatz.familienarchiv.repository.TagRepository; import org.springframework.http.HttpStatus; @@ -16,6 +24,11 @@ import lombok.RequiredArgsConstructor; @RequiredArgsConstructor public class TagService { + static final Set ALLOWED_TAG_COLORS = Set.of( + "navy", "teal", "ocean", "forest", "sage", + "sienna", "terracotta", "ochre", "rose", "violet" + ); + private final TagRepository tagRepository; public List search(String query) { @@ -34,9 +47,22 @@ public class TagService { } @Transactional - public Tag update(UUID id, String newName) { + public Tag update(UUID id, TagUpdateDTO dto) { Tag tag = getById(id); - tag.setName(newName); + + if (dto.parentId() != null) { + validateNoSelfReference(id, dto.parentId()); + validateNoAncestorCycle(id, dto.parentId()); + getById(dto.parentId()); // ensure parent exists + } + + if (dto.color() != null) { + validateColor(dto.color()); + } + + tag.setName(dto.name()); + tag.setParentId(dto.parentId()); + tag.setColor(dto.color()); return tagRepository.save(tag); } @@ -44,4 +70,74 @@ public class TagService { public void delete(UUID id) { tagRepository.delete(getById(id)); } + + /** + * Returns all tags assembled into a tree. Document counts are not included here + * (they are populated by the controller layer if needed, or set to 0). + */ + public List getTagTree() { + List all = tagRepository.findAll(); + return buildTree(all); + } + + // ─── private helpers ───────────────────────────────────────────────────── + + private void validateNoSelfReference(UUID tagId, UUID proposedParentId) { + if (tagId.equals(proposedParentId)) { + throw DomainException.badRequest(ErrorCode.TAG_CYCLE_DETECTED, + "A tag cannot be its own parent: " + tagId); + } + } + + private void validateNoAncestorCycle(UUID tagId, UUID proposedParentId) { + // TOCTOU: concurrent writes could both pass this check; the self-reference + // CHECK constraint on the DB column is the hard backstop for the self-loop case. + List ancestors = tagRepository.findAncestorIds(proposedParentId); + if (ancestors.contains(tagId)) { + throw DomainException.badRequest(ErrorCode.TAG_CYCLE_DETECTED, + "Assigning parent " + proposedParentId + " to tag " + tagId + " would create a cycle"); + } + } + + private void validateColor(String color) { + if (!ALLOWED_TAG_COLORS.contains(color)) { + throw DomainException.badRequest(ErrorCode.INVALID_TAG_COLOR, + "Color '" + color + "' is not in the allowed palette"); + } + } + + private List buildTree(List tags) { + Map> childrenByParent = new HashMap<>(); + + for (Tag tag : tags) { + childrenByParent.putIfAbsent(tag.getId(), new ArrayList<>()); + if (tag.getParentId() != null) { + childrenByParent.computeIfAbsent(tag.getParentId(), k -> new ArrayList<>()); + } + } + + for (Tag tag : tags) { + TagTreeNodeDTO node = new TagTreeNodeDTO( + tag.getId(), tag.getName(), tag.getColor(), 0, + childrenByParent.getOrDefault(tag.getId(), new ArrayList<>()) + ); + if (tag.getParentId() != null) { + childrenByParent.get(tag.getParentId()).add(node); + } else { + childrenByParent.get(tag.getId()); // ensure root is tracked + } + } + + // Collect root nodes (tags without a parent) + List roots = new ArrayList<>(); + for (Tag tag : tags) { + if (tag.getParentId() == null) { + roots.add(new TagTreeNodeDTO( + tag.getId(), tag.getName(), tag.getColor(), 0, + childrenByParent.getOrDefault(tag.getId(), new ArrayList<>()) + )); + } + } + return roots; + } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/TagControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/TagControllerTest.java index 77c2176f..b61adea6 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/TagControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/TagControllerTest.java @@ -1,6 +1,7 @@ package org.raddatz.familienarchiv.controller; import org.junit.jupiter.api.Test; +import org.raddatz.familienarchiv.dto.TagTreeNodeDTO; import org.raddatz.familienarchiv.model.Tag; import org.raddatz.familienarchiv.security.PermissionAspect; import org.raddatz.familienarchiv.service.CustomUserDetailsService; @@ -19,10 +20,10 @@ import org.springframework.test.web.servlet.MockMvc; import java.util.List; import java.util.UUID; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; +import static org.mockito.ArgumentMatchers.any; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; @WebMvcTest(TagController.class) @Import({SecurityConfig.class, PermissionAspect.class, AopAutoConfiguration.class}) @@ -82,6 +83,30 @@ class TagControllerTest { .andExpect(status().isOk()); } + // ─── GET /api/tags/tree ─────────────────────────────────────────────────── + + @Test + void getTagTree_returns401_whenUnauthenticated() throws Exception { + mockMvc.perform(get("/api/tags/tree")) + .andExpect(status().isUnauthorized()); + } + + @Test + @WithMockUser + void getTagTree_returns200_withTreeStructure() throws Exception { + UUID parentId = UUID.randomUUID(); + UUID childId = UUID.randomUUID(); + TagTreeNodeDTO child = new TagTreeNodeDTO(childId, "Haus", null, 0, List.of()); + TagTreeNodeDTO parent = new TagTreeNodeDTO(parentId, "Immobilie", "teal", 0, List.of(child)); + when(tagService.getTagTree()).thenReturn(List.of(parent)); + + mockMvc.perform(get("/api/tags/tree")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$[0].name").value("Immobilie")) + .andExpect(jsonPath("$[0].color").value("teal")) + .andExpect(jsonPath("$[0].children[0].name").value("Haus")); + } + // ─── DELETE /api/tags/{id} ──────────────────────────────────────────────── @Test diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/TagServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/TagServiceTest.java index 8700e153..5f42e964 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/TagServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/TagServiceTest.java @@ -5,10 +5,14 @@ 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.dto.TagUpdateDTO; +import org.raddatz.familienarchiv.exception.DomainException; +import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.Tag; import org.raddatz.familienarchiv.repository.TagRepository; import org.springframework.web.server.ResponseStatusException; +import java.util.List; import java.util.Optional; import java.util.UUID; @@ -89,22 +93,146 @@ class TagServiceTest { when(tagRepository.findById(id)).thenReturn(Optional.of(tag)); when(tagRepository.save(tag)).thenAnswer(inv -> inv.getArgument(0)); - Tag result = tagService.update(id, "New"); + Tag result = tagService.update(id, new TagUpdateDTO("New", null, null)); assertThat(result.getName()).isEqualTo("New"); } + @Test + void update_savesParentId() { + UUID id = UUID.randomUUID(); + UUID parentId = UUID.randomUUID(); + Tag tag = Tag.builder().id(id).name("Child").build(); + Tag parent = Tag.builder().id(parentId).name("Parent").build(); + when(tagRepository.findById(id)).thenReturn(Optional.of(tag)); + when(tagRepository.findById(parentId)).thenReturn(Optional.of(parent)); + when(tagRepository.findAncestorIds(parentId)).thenReturn(List.of()); + when(tagRepository.save(tag)).thenAnswer(inv -> inv.getArgument(0)); + + Tag result = tagService.update(id, new TagUpdateDTO("Child", parentId, null)); + + assertThat(result.getParentId()).isEqualTo(parentId); + } + + @Test + void update_savesColor() { + UUID id = UUID.randomUUID(); + Tag tag = Tag.builder().id(id).name("Colored").build(); + when(tagRepository.findById(id)).thenReturn(Optional.of(tag)); + when(tagRepository.save(tag)).thenAnswer(inv -> inv.getArgument(0)); + + Tag result = tagService.update(id, new TagUpdateDTO("Colored", null, "sage")); + + assertThat(result.getColor()).isEqualTo("sage"); + } + @Test void update_throwsNotFound_whenTagMissing() { UUID id = UUID.randomUUID(); when(tagRepository.findById(id)).thenReturn(Optional.empty()); - assertThatThrownBy(() -> tagService.update(id, "New")) + assertThatThrownBy(() -> tagService.update(id, new TagUpdateDTO("New", null, null))) .isInstanceOf(ResponseStatusException.class) .extracting(e -> ((ResponseStatusException) e).getStatusCode().value()) .isEqualTo(404); } + // ─── color validation ───────────────────────────────────────────────────── + + @Test + void update_throwsInvalidTagColor_whenColorNotInAllowedPalette() { + UUID id = UUID.randomUUID(); + Tag tag = Tag.builder().id(id).name("Tag").build(); + when(tagRepository.findById(id)).thenReturn(Optional.of(tag)); + + assertThatThrownBy(() -> tagService.update(id, new TagUpdateDTO("Tag", null, "hotpink"))) + .isInstanceOf(DomainException.class) + .extracting(e -> ((DomainException) e).getCode()) + .isEqualTo(ErrorCode.INVALID_TAG_COLOR); + } + + @Test + void update_allowsNullColor() { + UUID id = UUID.randomUUID(); + Tag tag = Tag.builder().id(id).name("Tag").color("sage").build(); + when(tagRepository.findById(id)).thenReturn(Optional.of(tag)); + when(tagRepository.save(tag)).thenAnswer(inv -> inv.getArgument(0)); + + Tag result = tagService.update(id, new TagUpdateDTO("Tag", null, null)); + + assertThat(result.getColor()).isNull(); + } + + // ─── cycle detection ───────────────────────────────────────────────────── + + @Test + void update_throwsCycleDetected_whenTagIsAncestorOfProposedParent() { + UUID tagId = UUID.randomUUID(); + UUID proposedParentId = UUID.randomUUID(); + Tag tag = Tag.builder().id(tagId).name("Tag").build(); + when(tagRepository.findById(tagId)).thenReturn(Optional.of(tag)); + // tagId appears in the ancestor chain of proposedParentId → cycle + when(tagRepository.findAncestorIds(proposedParentId)).thenReturn(List.of(tagId)); + + assertThatThrownBy(() -> tagService.update(tagId, new TagUpdateDTO("Tag", proposedParentId, null))) + .isInstanceOf(DomainException.class) + .extracting(e -> ((DomainException) e).getCode()) + .isEqualTo(ErrorCode.TAG_CYCLE_DETECTED); + } + + @Test + void update_throwsCycleDetected_whenTagIsSameAsProposedParent() { + UUID tagId = UUID.randomUUID(); + Tag tag = Tag.builder().id(tagId).name("Tag").build(); + when(tagRepository.findById(tagId)).thenReturn(Optional.of(tag)); + + assertThatThrownBy(() -> tagService.update(tagId, new TagUpdateDTO("Tag", tagId, null))) + .isInstanceOf(DomainException.class) + .extracting(e -> ((DomainException) e).getCode()) + .isEqualTo(ErrorCode.TAG_CYCLE_DETECTED); + } + + // ─── getTagTree ─────────────────────────────────────────────────────────── + + @Test + void getTagTree_returnsEmptyList_whenNoTags() { + when(tagRepository.findAll()).thenReturn(List.of()); + + assertThat(tagService.getTagTree()).isEmpty(); + } + + @Test + void getTagTree_returnsFlatRootTags_whenNoParentRelationships() { + UUID idA = UUID.randomUUID(); + UUID idB = UUID.randomUUID(); + List tags = List.of( + Tag.builder().id(idA).name("Alpha").build(), + Tag.builder().id(idB).name("Beta").build() + ); + when(tagRepository.findAll()).thenReturn(tags); + + var tree = tagService.getTagTree(); + + assertThat(tree).hasSize(2); + assertThat(tree).allSatisfy(node -> assertThat(node.children()).isEmpty()); + } + + @Test + void getTagTree_nestsChildrenUnderParent() { + UUID parentId = UUID.randomUUID(); + UUID childId = UUID.randomUUID(); + Tag parent = Tag.builder().id(parentId).name("Parent").build(); + Tag child = Tag.builder().id(childId).name("Child").parentId(parentId).build(); + when(tagRepository.findAll()).thenReturn(List.of(parent, child)); + + var tree = tagService.getTagTree(); + + assertThat(tree).hasSize(1); + assertThat(tree.get(0).id()).isEqualTo(parentId); + assertThat(tree.get(0).children()).hasSize(1); + assertThat(tree.get(0).children().get(0).id()).isEqualTo(childId); + } + // ─── delete ─────────────────────────────────────────────────────────────── @Test