From b6f74fd6fc87db9016d3f2cbed3167b2623ea931 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 13 Apr 2026 16:48:18 +0200 Subject: [PATCH] refactor(annotations): remove overlap check to allow intersecting regions Historical letter lines often intersect, so the system must support overlapping annotation regions. Removed the overlap guard from createAnnotation(), deleted ErrorCode.ANNOTATION_OVERLAP, and cleaned up all tests and frontend error mappings that referenced it. Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/exception/ErrorCode.java | 2 - .../service/AnnotationService.java | 19 -- .../controller/AnnotationControllerTest.java | 14 +- .../service/AnnotationServiceTest.java | 275 ++++++------------ frontend/messages/de.json | 1 - frontend/messages/en.json | 1 - frontend/messages/es.json | 1 - frontend/src/lib/errors.ts | 3 - 8 files changed, 92 insertions(+), 224 deletions(-) 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 d8a03b83..60930824 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java @@ -49,8 +49,6 @@ public enum ErrorCode { // --- Annotations --- /** The annotation with the given ID does not exist. 404 */ ANNOTATION_NOT_FOUND, - /** The new annotation overlaps an existing one on the same page. 409 */ - ANNOTATION_OVERLAP, // --- Transcription Blocks --- /** The transcription block with the given ID does not exist. 404 */ diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java index 6735ef31..d7192cce 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java @@ -24,15 +24,6 @@ public class AnnotationService { @Transactional public DocumentAnnotation createAnnotation(UUID documentId, CreateAnnotationDTO dto, UUID userId, String fileHash) { - List existing = - annotationRepository.findByDocumentIdAndPageNumber(documentId, dto.getPageNumber()); - - boolean overlaps = existing.stream().anyMatch(a -> overlaps(a, dto)); - if (overlaps) { - throw DomainException.conflict( - ErrorCode.ANNOTATION_OVERLAP, "Annotation overlaps an existing one on this page"); - } - DocumentAnnotation annotation = DocumentAnnotation.builder() .documentId(documentId) .pageNumber(dto.getPageNumber()) @@ -90,14 +81,4 @@ public class AnnotationService { }); } - // ─── private helpers ────────────────────────────────────────────────────── - - private boolean overlaps(DocumentAnnotation existing, CreateAnnotationDTO dto) { - double ex2 = existing.getX() + existing.getWidth(); - double ey2 = existing.getY() + existing.getHeight(); - double dx2 = dto.getX() + dto.getWidth(); - double dy2 = dto.getY() + dto.getHeight(); - return existing.getX() < dx2 && ex2 > dto.getX() - && existing.getY() < dy2 && ey2 > dto.getY(); - } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/AnnotationControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/AnnotationControllerTest.java index ac353a97..4b95d877 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/AnnotationControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/AnnotationControllerTest.java @@ -123,15 +123,19 @@ class AnnotationControllerTest { @Test @WithMockUser(authorities = "ANNOTATE_ALL") - void createAnnotation_returns409_whenOverlap() throws Exception { + void createAnnotation_returns201_whenAnnotationsOverlap() throws Exception { + // Overlapping annotations are allowed — historical letter lines often intersect + UUID docId = UUID.randomUUID(); + DocumentAnnotation saved = DocumentAnnotation.builder() + .id(UUID.randomUUID()).documentId(docId).pageNumber(1) + .x(0.1).y(0.1).width(0.3).height(0.3).color("#ff0000").build(); when(documentService.getDocumentById(any())).thenReturn(Document.builder().build()); - when(annotationService.createAnnotation(any(), any(), any(), any())) - .thenThrow(DomainException.conflict(ErrorCode.ANNOTATION_OVERLAP, "Overlap")); + when(annotationService.createAnnotation(any(), any(), any(), any())).thenReturn(saved); - mockMvc.perform(post("/api/documents/" + UUID.randomUUID() + "/annotations") + mockMvc.perform(post("/api/documents/" + docId + "/annotations") .contentType(MediaType.APPLICATION_JSON) .content(ANNOTATION_JSON)) - .andExpect(status().isConflict()); + .andExpect(status().isCreated()); } // ─── DELETE /api/documents/{documentId}/annotations/{annotationId} ───────── diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java index 37652179..b4813491 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java @@ -20,7 +20,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.springframework.http.HttpStatus.CONFLICT; import static org.springframework.http.HttpStatus.FORBIDDEN; import static org.springframework.http.HttpStatus.NOT_FOUND; @@ -33,34 +32,13 @@ class AnnotationServiceTest { // ─── createAnnotation ───────────────────────────────────────────────────── @Test - void createAnnotation_throwsConflict_whenAnnotationOverlapsExisting() { + void createAnnotation_savesAnnotation() { UUID docId = UUID.randomUUID(); UUID userId = UUID.randomUUID(); CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.1, 0.1, 0.3, 0.3, "#ff0000"); - - DocumentAnnotation existing = DocumentAnnotation.builder() - .id(UUID.randomUUID()).documentId(docId).pageNumber(1) - .x(0.2).y(0.2).width(0.3).height(0.3).color("#00ff00").build(); - when(annotationRepository.findByDocumentIdAndPageNumber(docId, 1)) - .thenReturn(List.of(existing)); - - assertThatThrownBy(() -> annotationService.createAnnotation(docId, dto, userId, null)) - .isInstanceOf(DomainException.class) - .satisfies(e -> assertThat(((DomainException) e).getStatus()).isEqualTo(CONFLICT)); - - verify(annotationRepository, never()).save(any()); - } - - @Test - void createAnnotation_savesAndReturns_whenNoOverlap() { - UUID docId = UUID.randomUUID(); - UUID userId = UUID.randomUUID(); - CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.0, 0.0, 0.05, 0.05, "#ff0000"); - - when(annotationRepository.findByDocumentIdAndPageNumber(docId, 1)).thenReturn(List.of()); DocumentAnnotation saved = DocumentAnnotation.builder() .id(UUID.randomUUID()).documentId(docId).pageNumber(1) - .x(0.0).y(0.0).width(0.05).height(0.05).color("#ff0000").createdBy(userId).build(); + .x(0.1).y(0.1).width(0.3).height(0.3).color("#ff0000").createdBy(userId).build(); when(annotationRepository.save(any())).thenReturn(saved); DocumentAnnotation result = annotationService.createAnnotation(docId, dto, userId, null); @@ -69,6 +47,77 @@ class AnnotationServiceTest { verify(annotationRepository).save(any()); } + @Test + void createAnnotation_allowsOverlappingAnnotations() { + UUID docId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.1, 0.1, 0.3, 0.3, "#ff0000"); + when(annotationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + // Should not throw even when overlapping annotations exist on the same page + annotationService.createAnnotation(docId, dto, userId, null); + + verify(annotationRepository).save(any()); + } + + @Test + void createAnnotation_setsFileHash_whenProvided() { + UUID docId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.0, 0.0, 0.05, 0.05, "#ff0000"); + when(annotationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + DocumentAnnotation result = annotationService.createAnnotation(docId, dto, userId, "abc123"); + + assertThat(result.getFileHash()).isEqualTo("abc123"); + } + + @Test + void createAnnotation_setsNullFileHash_whenNoneProvided() { + UUID docId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.0, 0.0, 0.05, 0.05, "#ff0000"); + when(annotationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + DocumentAnnotation result = annotationService.createAnnotation(docId, dto, userId, null); + + assertThat(result.getFileHash()).isNull(); + } + + // ─── createOcrAnnotation ────────────────────────────────────────────────── + + @Test + void createOcrAnnotation_savesWithPolygon() { + UUID docId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.1, 0.1, 0.8, 0.04, "#00C7B1"); + List> polygon = List.of( + List.of(0.1, 0.1), List.of(0.9, 0.11), + List.of(0.89, 0.14), List.of(0.11, 0.13)); + when(annotationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + DocumentAnnotation result = annotationService.createOcrAnnotation( + docId, dto, userId, "filehash", polygon); + + assertThat(result.getPolygon()).isEqualTo(polygon); + assertThat(result.getDocumentId()).isEqualTo(docId); + verify(annotationRepository).save(any()); + } + + @Test + void createOcrAnnotation_savesWithNullPolygon_whenPolygonNotProvided() { + UUID docId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.1, 0.1, 0.8, 0.04, "#00C7B1"); + when(annotationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + DocumentAnnotation result = annotationService.createOcrAnnotation( + docId, dto, userId, "filehash", null); + + assertThat(result.getPolygon()).isNull(); + verify(annotationRepository).save(any()); + } + // ─── deleteAnnotation ───────────────────────────────────────────────────── @Test @@ -118,32 +167,19 @@ class AnnotationServiceTest { } @Test - void createAnnotation_setsFileHash_whenProvided() { + void deleteAnnotation_throwsForbidden_whenUserIdIsNull() { UUID docId = UUID.randomUUID(); - UUID userId = UUID.randomUUID(); - CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.0, 0.0, 0.05, 0.05, "#ff0000"); - String fileHash = "abc123"; + UUID annotId = UUID.randomUUID(); + UUID ownerId = UUID.randomUUID(); - when(annotationRepository.findByDocumentIdAndPageNumber(docId, 1)).thenReturn(List.of()); - when(annotationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + DocumentAnnotation annotation = DocumentAnnotation.builder() + .id(annotId).documentId(docId).createdBy(ownerId).build(); + when(annotationRepository.findByIdAndDocumentId(annotId, docId)) + .thenReturn(Optional.of(annotation)); - DocumentAnnotation result = annotationService.createAnnotation(docId, dto, userId, fileHash); - - assertThat(result.getFileHash()).isEqualTo(fileHash); - } - - @Test - void createAnnotation_setsNullFileHash_whenNoneProvided() { - UUID docId = UUID.randomUUID(); - UUID userId = UUID.randomUUID(); - CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.0, 0.0, 0.05, 0.05, "#ff0000"); - - when(annotationRepository.findByDocumentIdAndPageNumber(docId, 1)).thenReturn(List.of()); - when(annotationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); - - DocumentAnnotation result = annotationService.createAnnotation(docId, dto, userId, null); - - assertThat(result.getFileHash()).isNull(); + assertThatThrownBy(() -> annotationService.deleteAnnotation(docId, annotId, null)) + .isInstanceOf(DomainException.class) + .satisfies(e -> assertThat(((DomainException) e).getStatus()).isEqualTo(FORBIDDEN)); } // ─── listAnnotations ────────────────────────────────────────────────────── @@ -183,149 +219,4 @@ class AnnotationServiceTest { verify(annotationRepository, never()).save(any()); } - - // ─── deleteAnnotation — null userId ─────────────────────────────────────── - - @Test - void deleteAnnotation_throwsForbidden_whenUserIdIsNull() { - UUID docId = UUID.randomUUID(); - UUID annotId = UUID.randomUUID(); - UUID ownerId = UUID.randomUUID(); - - DocumentAnnotation annotation = DocumentAnnotation.builder() - .id(annotId).documentId(docId).createdBy(ownerId).build(); - when(annotationRepository.findByIdAndDocumentId(annotId, docId)) - .thenReturn(Optional.of(annotation)); - - assertThatThrownBy(() -> annotationService.deleteAnnotation(docId, annotId, null)) - .isInstanceOf(DomainException.class) - .satisfies(e -> assertThat(((DomainException) e).getStatus()).isEqualTo(FORBIDDEN)); - } - - // ─── overlaps — partial overlap cases ──────────────────────────────────── - - @Test - void createAnnotation_noConflict_whenAnnotationIsToTheLeft() { - // existing: x=0.5, w=0.3 (x2=0.8); dto: x=0.0, w=0.4 (dx2=0.4) - // existing.getX() < dx2 → 0.5 < 0.4 → FALSE → no overlap (first && fails) - UUID docId = UUID.randomUUID(); - DocumentAnnotation existing = DocumentAnnotation.builder() - .id(UUID.randomUUID()).documentId(docId).pageNumber(1) - .x(0.5).y(0.0).width(0.3).height(0.5).color("#ff0000").build(); - CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.0, 0.0, 0.4, 0.5, "#0000ff"); - when(annotationRepository.findByDocumentIdAndPageNumber(docId, 1)).thenReturn(List.of(existing)); - when(annotationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); - - annotationService.createAnnotation(docId, dto, UUID.randomUUID(), null); - - verify(annotationRepository).save(any()); - } - - @Test - void createAnnotation_noConflict_whenAnnotationIsToTheRight() { - // existing: x=0.0, w=0.1 (ex2=0.1); dto: x=0.2, w=0.3 (dx2=0.5) - // existing.getX() < dx2 → 0.0 < 0.5 → TRUE - // ex2 > dto.getX() → 0.1 > 0.2 → FALSE → no overlap (second && fails) - UUID docId = UUID.randomUUID(); - DocumentAnnotation existing = DocumentAnnotation.builder() - .id(UUID.randomUUID()).documentId(docId).pageNumber(1) - .x(0.0).y(0.0).width(0.1).height(0.5).color("#ff0000").build(); - CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.2, 0.0, 0.3, 0.5, "#0000ff"); - when(annotationRepository.findByDocumentIdAndPageNumber(docId, 1)).thenReturn(List.of(existing)); - when(annotationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); - - annotationService.createAnnotation(docId, dto, UUID.randomUUID(), null); - - verify(annotationRepository).save(any()); - } - - @Test - void createAnnotation_noConflict_whenAnnotationIsBelow() { - // x ranges overlap, but y ranges don't - // existing: x=0.0, w=0.5, y=0.5, h=0.2 (ey2=0.7) - // dto: x=0.1, w=0.3 (dx2=0.4), y=0.0, h=0.4 (dy2=0.4) - // existing.getX() < dx2 → 0.0 < 0.4 → TRUE - // ex2 > dto.getX() → 0.5 > 0.1 → TRUE - // existing.getY() < dy2 → 0.5 < 0.4 → FALSE → no overlap (third && fails) - UUID docId = UUID.randomUUID(); - DocumentAnnotation existing = DocumentAnnotation.builder() - .id(UUID.randomUUID()).documentId(docId).pageNumber(1) - .x(0.0).y(0.5).width(0.5).height(0.2).color("#ff0000").build(); - CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.1, 0.0, 0.3, 0.4, "#0000ff"); - when(annotationRepository.findByDocumentIdAndPageNumber(docId, 1)).thenReturn(List.of(existing)); - when(annotationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); - - annotationService.createAnnotation(docId, dto, UUID.randomUUID(), null); - - verify(annotationRepository).save(any()); - } - - // ─── createOcrAnnotation ────────────────────────────────────────────────── - - @Test - void createOcrAnnotation_skipsOverlapCheck_andSavesWithPolygon() { - UUID docId = UUID.randomUUID(); - UUID userId = UUID.randomUUID(); - CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.1, 0.1, 0.8, 0.04, "#00C7B1"); - List> polygon = List.of( - List.of(0.1, 0.1), List.of(0.9, 0.11), - List.of(0.89, 0.14), List.of(0.11, 0.13)); - when(annotationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); - - DocumentAnnotation result = annotationService.createOcrAnnotation( - docId, dto, userId, "filehash", polygon); - - assertThat(result.getPolygon()).isEqualTo(polygon); - assertThat(result.getDocumentId()).isEqualTo(docId); - verify(annotationRepository).save(any()); - verify(annotationRepository, never()).findByDocumentIdAndPageNumber(any(), any(int.class)); - } - - @Test - void createOcrAnnotation_savesWithNullPolygon_whenPolygonNotProvided() { - UUID docId = UUID.randomUUID(); - UUID userId = UUID.randomUUID(); - CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.1, 0.1, 0.8, 0.04, "#00C7B1"); - when(annotationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); - - DocumentAnnotation result = annotationService.createOcrAnnotation( - docId, dto, userId, "filehash", null); - - assertThat(result.getPolygon()).isNull(); - verify(annotationRepository).save(any()); - } - - @Test - void createOcrAnnotation_doesNotCheckOverlap_evenWhenOverlappingAnnotationExists() { - UUID docId = UUID.randomUUID(); - UUID userId = UUID.randomUUID(); - CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.1, 0.1, 0.3, 0.3, "#00C7B1"); - when(annotationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); - - annotationService.createOcrAnnotation(docId, dto, userId, "hash", null); - - verify(annotationRepository, never()).findByDocumentIdAndPageNumber(any(), any(int.class)); - } - - // ─── overlaps — partial overlap cases ──────────────────────────────────── - - @Test - void createAnnotation_noConflict_whenAnnotationIsAbove() { - // x ranges overlap, y ranges don't — existing is ABOVE the new annotation - // existing: x=0.0, w=0.5, y=0.0, h=0.1 (ey2=0.1) - // dto: x=0.1, w=0.3 (dx2=0.4), y=0.2, h=0.3 (dy2=0.5) - // A: 0.0 < 0.4 → TRUE, B: 0.5 > 0.1 → TRUE, C: 0.0 < 0.5 → TRUE - // D: ey2 > dto.getY() → 0.1 > 0.2 → FALSE → no overlap (fourth && fails) - UUID docId = UUID.randomUUID(); - DocumentAnnotation existing = DocumentAnnotation.builder() - .id(UUID.randomUUID()).documentId(docId).pageNumber(1) - .x(0.0).y(0.0).width(0.5).height(0.1).color("#ff0000").build(); - CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.1, 0.2, 0.3, 0.3, "#0000ff"); - when(annotationRepository.findByDocumentIdAndPageNumber(docId, 1)).thenReturn(List.of(existing)); - when(annotationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); - - annotationService.createAnnotation(docId, dto, UUID.randomUUID(), null); - - verify(annotationRepository).save(any()); - } } diff --git a/frontend/messages/de.json b/frontend/messages/de.json index 5fc0389f..1a7912f1 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -1,7 +1,6 @@ { "$schema": "https://inlang.com/schema/inlang-message-format", "error_annotation_not_found": "Die Annotation wurde nicht gefunden.", - "error_annotation_overlap": "Die Annotation überschneidet sich mit einer vorhandenen.", "annotation_outdated_notice": "Einige Annotationen beziehen sich auf eine frühere Dateiversion und werden nicht angezeigt.", "error_document_not_found": "Das Dokument wurde nicht gefunden.", "error_document_no_file": "Diesem Dokument ist noch keine Datei zugeordnet.", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index e4572a4b..58633166 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -1,7 +1,6 @@ { "$schema": "https://inlang.com/schema/inlang-message-format", "error_annotation_not_found": "Annotation not found.", - "error_annotation_overlap": "The annotation overlaps an existing one.", "annotation_outdated_notice": "Some annotations refer to an earlier file version and are not shown.", "error_document_not_found": "Document not found.", "error_document_no_file": "No file is associated with this document.", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index d858ce36..74051eaa 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -1,7 +1,6 @@ { "$schema": "https://inlang.com/schema/inlang-message-format", "error_annotation_not_found": "Anotación no encontrada.", - "error_annotation_overlap": "La anotación se superpone con una existente.", "annotation_outdated_notice": "Algunas anotaciones hacen referencia a una versión anterior del archivo y no se muestran.", "error_document_not_found": "Documento no encontrado.", "error_document_no_file": "No hay ningún archivo asociado a este documento.", diff --git a/frontend/src/lib/errors.ts b/frontend/src/lib/errors.ts index 56073568..da769462 100644 --- a/frontend/src/lib/errors.ts +++ b/frontend/src/lib/errors.ts @@ -18,7 +18,6 @@ export type ErrorCode = | 'IMPORT_ALREADY_RUNNING' | 'INVALID_RESET_TOKEN' | 'ANNOTATION_NOT_FOUND' - | 'ANNOTATION_OVERLAP' | 'TRANSCRIPTION_BLOCK_NOT_FOUND' | 'TRANSCRIPTION_BLOCK_CONFLICT' | 'COMMENT_NOT_FOUND' @@ -82,8 +81,6 @@ export function getErrorMessage(code: ErrorCode | string | undefined): string { return m.error_invalid_reset_token(); case 'ANNOTATION_NOT_FOUND': return m.error_annotation_not_found(); - case 'ANNOTATION_OVERLAP': - return m.error_annotation_overlap(); case 'TRANSCRIPTION_BLOCK_NOT_FOUND': return m.error_transcription_block_not_found(); case 'TRANSCRIPTION_BLOCK_CONFLICT':