Compare commits

..

11 Commits

Author SHA1 Message Date
Marcel
b5239f515f fix(notification): address review suggestions
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m23s
CI / OCR Service Tests (pull_request) Successful in 20s
CI / Backend Unit Tests (pull_request) Successful in 3m17s
CI / fail2ban Regex (pull_request) Successful in 40s
CI / Semgrep Security Scan (pull_request) Successful in 20s
CI / Compose Bucket Idempotency (pull_request) Successful in 58s
- ChronikFuerDichBox: move update() inside the failure branch so success
  path skips it, matching NotificationDropdown's pattern
- NotificationDropdown test: add role=alert assertion for mark-all-read
  failure to match existing dismiss-failure coverage in ChronikFuerDichBox
- +page.server.ts: use getErrorMessage(undefined) instead of null so the
  missing-notificationId 400 goes through the same i18n pipeline as other errors

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-20 07:31:26 +02:00
Marcel
f2bb58e294 fix(chronik): surface action failures in ChronikFuerDichBox with accessible error banner
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m35s
CI / OCR Service Tests (pull_request) Successful in 20s
CI / Backend Unit Tests (pull_request) Successful in 3m17s
CI / fail2ban Regex (pull_request) Successful in 41s
CI / Semgrep Security Scan (pull_request) Successful in 20s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m1s
Add $state errorMessage + role=alert banner to ChronikFuerDichBox. Both enhance callbacks
now inspect result.type and set the error message on 'failure' or 'error'; errorMessage
is cleared on each new submit attempt.

Upgrade both test files to the mockFormResult pattern (via vi.hoisted) so the result
callback is exercised. Add a failing-action test in each file that asserts role=alert
appears after a form submit with type='failure'.

Fix bare Function cast → explicit typed cast to satisfy @typescript-eslint/no-unsafe-function-type.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-20 07:06:58 +02:00
Marcel
2adb98895d fix(aktivitaeten): narrow File cast and use null payload for missing notificationId
Replace 'as string | null' cast (which silently accepts File values) with an explicit
typeof check. Use error: null instead of hardcoded German so the client falls through
to the generic i18n-keyed error banner.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-20 07:06:15 +02:00
Marcel
6049dcadd3 fix(notification-dropdown): handle error result type, add role=alert, fix update ordering
- Add role="alert" to error banner so screen-reader users hear failures
- Handle result.type === 'error' (network failure) alongside 'failure' in both enhance callbacks
- Clear errorMessage at the start of each submit so stale errors don't persist on retry
- On dismiss success: skip update() entirely since goto() navigates away from the page
- On dismiss failure: await update() then set error message
- On mark-all success: skip update() (optimistic state already applied)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-20 07:05:50 +02:00
Marcel
7fe8842b57 fix(notifications): surface action failures as an error banner
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m25s
CI / OCR Service Tests (pull_request) Successful in 20s
CI / Backend Unit Tests (pull_request) Successful in 3m23s
CI / fail2ban Regex (pull_request) Successful in 39s
CI / Semgrep Security Scan (pull_request) Successful in 18s
CI / Compose Bucket Idempotency (pull_request) Successful in 58s
When dismiss-notification or mark-all-read returns a failure the dropdown
now shows a localised error message above the list. Added
notification_error_generic key (de/en/es) as the fallback when the
action response carries no explicit error string.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 23:56:33 +02:00
Marcel
f9340366d1 fix(notifications): move onClose/goto into enhance result callback
onClose() and goto() were firing before the server responded, making it
impossible for a fail() response to cancel navigation. Moved them inside
the result callback behind a result.type !== 'failure' guard.

Updated the $app/forms enhance mock to always invoke the returned async
callback with a configurable mockFormResult, and added three tests:
- success path calls onClose + goto with the correct deep-link URL
- failure path skips onClose and goto
- annotationId is appended to the URL when present

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 23:54:15 +02:00
Marcel
af84ffc379 fix(notifications): guard against null notificationId in dismiss action
Casting null to string caused PATCH to fire against /api/notifications/null/read
when the field was absent. Added an early-return fail(400) and a test that
submitting an empty form returns 400 without calling the API.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 23:48:37 +02:00
Marcel
23439e581a refactor(chronik): replace callback props with form actions in ChronikFuerDichBox
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m20s
CI / OCR Service Tests (pull_request) Successful in 21s
CI / Backend Unit Tests (pull_request) Successful in 3m22s
CI / fail2ban Regex (pull_request) Successful in 46s
CI / Semgrep Security Scan (pull_request) Successful in 19s
CI / Compose Bucket Idempotency (pull_request) Successful in 58s
Dismiss (X) button and mark-all-read button now submit forms to
/aktivitaeten?/dismiss-notification and /aktivitaeten?/mark-all-read respectively.
Props renamed onMarkRead/onMarkAllRead → optimisticMarkRead/optimisticMarkAllRead.

aktivitaeten/+page.svelte drops the now-deleted onMarkRead/onMarkAllRead wrapper functions
and passes notificationStore.optimisticMarkRead/optimisticMarkAllRead directly to the box.

Tests: $app/forms enhance mock added to both spec files so dismiss and mark-all assertions
work synchronously against form-submit events.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 23:16:58 +02:00
Marcel
2c6b59d0c7 refactor(notification): replace callback props with form actions in Dropdown and Bell
NotificationDropdown now wraps each row in a <form action="/aktivitaeten?/dismiss-notification">
and the mark-all control in <form action="/aktivitaeten?/mark-all-read">, wired via use:enhance
for optimistic UI. Props renamed onMarkRead/onMarkAllRead → optimisticMarkRead/optimisticMarkAllRead
to match the simplified store API. NotificationBell passes the store helpers directly; handleMarkRead
is removed.

Test mocks updated: $app/forms enhance mock fires SubmitFunction synchronously on form submit so
callback assertions work without a real HTTP round-trip.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 23:15:56 +02:00
Marcel
c0a7408ef4 refactor(notification): rename markRead/markAllRead to optimistic helpers without fetch
Removes raw fetch() calls from the store. optimisticMarkRead(id) and
optimisticMarkAllRead() now only mutate local $state — the actual API
calls move to SvelteKit form actions on /aktivitaeten.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 22:59:01 +02:00
Marcel
9d283c4500 feat(notification): add dismiss-notification and mark-all-read form actions to aktivitaeten
Adds two SvelteKit form actions to /aktivitaeten/+page.server.ts so the
notification bell can POST there instead of calling the backend directly
from the browser.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 22:51:08 +02:00
84 changed files with 250 additions and 2627 deletions

View File

@@ -79,7 +79,6 @@ jobs:
IMPORT_HOST_DIR=/srv/familienarchiv-staging/import IMPORT_HOST_DIR=/srv/familienarchiv-staging/import
POSTGRES_USER=archiv POSTGRES_USER=archiv
SENTRY_DSN=${{ secrets.SENTRY_DSN }} SENTRY_DSN=${{ secrets.SENTRY_DSN }}
VITE_SENTRY_DSN=${{ secrets.VITE_SENTRY_DSN }}
EOF EOF
- name: Verify backend /import:ro mount is wired - name: Verify backend /import:ro mount is wired

View File

@@ -25,14 +25,11 @@ import java.util.UUID;
@NamedEntityGraph(name = "Document.full", attributeNodes = { @NamedEntityGraph(name = "Document.full", attributeNodes = {
@NamedAttributeNode("sender"), @NamedAttributeNode("sender"),
@NamedAttributeNode("receivers"), @NamedAttributeNode("receivers"),
@NamedAttributeNode("tags"), @NamedAttributeNode("tags")
@NamedAttributeNode("trainingLabels")
}) })
@NamedEntityGraph(name = "Document.list", attributeNodes = { @NamedEntityGraph(name = "Document.list", attributeNodes = {
@NamedAttributeNode("sender"), @NamedAttributeNode("sender"),
@NamedAttributeNode("receivers"), @NamedAttributeNode("tags")
@NamedAttributeNode("tags"),
@NamedAttributeNode("trainingLabels")
}) })
@Entity @Entity
@Table(name = "documents") @Table(name = "documents")

View File

@@ -43,7 +43,7 @@ public class TranscriptionBlockController {
@PostMapping @PostMapping
@ResponseStatus(HttpStatus.CREATED) @ResponseStatus(HttpStatus.CREATED)
@RequirePermission({Permission.ANNOTATE_ALL, Permission.WRITE_ALL}) @RequirePermission(Permission.WRITE_ALL)
public TranscriptionBlock createBlock( public TranscriptionBlock createBlock(
@PathVariable UUID documentId, @PathVariable UUID documentId,
@Valid @RequestBody CreateTranscriptionBlockDTO dto, @Valid @RequestBody CreateTranscriptionBlockDTO dto,
@@ -53,7 +53,7 @@ public class TranscriptionBlockController {
} }
@PutMapping("/{blockId}") @PutMapping("/{blockId}")
@RequirePermission({Permission.ANNOTATE_ALL, Permission.WRITE_ALL}) @RequirePermission(Permission.WRITE_ALL)
public TranscriptionBlock updateBlock( public TranscriptionBlock updateBlock(
@PathVariable UUID documentId, @PathVariable UUID documentId,
@PathVariable UUID blockId, @PathVariable UUID blockId,
@@ -65,7 +65,7 @@ public class TranscriptionBlockController {
@DeleteMapping("/{blockId}") @DeleteMapping("/{blockId}")
@ResponseStatus(HttpStatus.NO_CONTENT) @ResponseStatus(HttpStatus.NO_CONTENT)
@RequirePermission({Permission.ANNOTATE_ALL, Permission.WRITE_ALL}) @RequirePermission(Permission.WRITE_ALL)
public void deleteBlock( public void deleteBlock(
@PathVariable UUID documentId, @PathVariable UUID documentId,
@PathVariable UUID blockId) { @PathVariable UUID blockId) {
@@ -73,7 +73,7 @@ public class TranscriptionBlockController {
} }
@PutMapping("/reorder") @PutMapping("/reorder")
@RequirePermission({Permission.ANNOTATE_ALL, Permission.WRITE_ALL}) @RequirePermission(Permission.WRITE_ALL)
public List<TranscriptionBlock> reorderBlocks( public List<TranscriptionBlock> reorderBlocks(
@PathVariable UUID documentId, @PathVariable UUID documentId,
@RequestBody ReorderTranscriptionBlocksDTO dto) { @RequestBody ReorderTranscriptionBlocksDTO dto) {
@@ -82,7 +82,7 @@ public class TranscriptionBlockController {
} }
@PutMapping("/{blockId}/review") @PutMapping("/{blockId}/review")
@RequirePermission({Permission.ANNOTATE_ALL, Permission.WRITE_ALL}) @RequirePermission(Permission.WRITE_ALL)
public TranscriptionBlock reviewBlock( public TranscriptionBlock reviewBlock(
@PathVariable UUID documentId, @PathVariable UUID documentId,
@PathVariable UUID blockId, @PathVariable UUID blockId,
@@ -92,7 +92,7 @@ public class TranscriptionBlockController {
} }
@PutMapping("/review-all") @PutMapping("/review-all")
@RequirePermission({Permission.ANNOTATE_ALL, Permission.WRITE_ALL}) @RequirePermission(Permission.WRITE_ALL)
public List<TranscriptionBlock> markAllBlocksReviewed( public List<TranscriptionBlock> markAllBlocksReviewed(
@PathVariable UUID documentId, @PathVariable UUID documentId,
Authentication authentication) { Authentication authentication) {

View File

@@ -56,17 +56,9 @@ public class MassImportService {
public enum State { IDLE, RUNNING, DONE, FAILED } public enum State { IDLE, RUNNING, DONE, FAILED }
public enum SkipReason {
INVALID_FILENAME_PATH_TRAVERSAL,
INVALID_PDF_SIGNATURE,
FILE_READ_ERROR,
ALREADY_EXISTS,
S3_UPLOAD_FAILED
}
public record SkippedFile( public record SkippedFile(
@Schema(requiredMode = Schema.RequiredMode.REQUIRED) String filename, @Schema(requiredMode = Schema.RequiredMode.REQUIRED) String filename,
@Schema(requiredMode = Schema.RequiredMode.REQUIRED) SkipReason reason @Schema(requiredMode = Schema.RequiredMode.REQUIRED) String reason
) {} ) {}
public record ImportStatus( public record ImportStatus(
@@ -299,11 +291,6 @@ public class MassImportService {
if (index.isBlank()) continue; if (index.isBlank()) continue;
String filename = index.contains(".") ? index : index + ".pdf"; String filename = index.contains(".") ? index : index + ".pdf";
if (!isValidImportFilename(filename)) {
log.warn("Skipping import row {}: filename rejected — {}", i, filename);
skippedFiles.add(new SkippedFile(filename, SkipReason.INVALID_FILENAME_PATH_TRAVERSAL));
continue;
}
Optional<File> fileOnDisk = findFileRecursive(filename); Optional<File> fileOnDisk = findFileRecursive(filename);
if (fileOnDisk.isEmpty()) { if (fileOnDisk.isEmpty()) {
log.warn("Datei nicht gefunden, importiere nur Metadaten: {}", filename); log.warn("Datei nicht gefunden, importiere nur Metadaten: {}", filename);
@@ -313,17 +300,17 @@ public class MassImportService {
try { try {
if (!isPdfMagicBytes(fileOnDisk.get())) { if (!isPdfMagicBytes(fileOnDisk.get())) {
log.warn("Überspringe {}: Datei beginnt nicht mit %PDF-Signatur", filename); log.warn("Überspringe {}: Datei beginnt nicht mit %PDF-Signatur", filename);
skippedFiles.add(new SkippedFile(filename, SkipReason.INVALID_PDF_SIGNATURE)); skippedFiles.add(new SkippedFile(filename, "INVALID_PDF_SIGNATURE"));
continue; continue;
} }
} catch (IOException e) { } catch (IOException e) {
log.error("Fehler beim Prüfen der Magic-Bytes für {}", filename, e); log.error("Fehler beim Prüfen der Magic-Bytes für {}", filename, e);
skippedFiles.add(new SkippedFile(filename, SkipReason.FILE_READ_ERROR)); skippedFiles.add(new SkippedFile(filename, "FILE_READ_ERROR"));
continue; continue;
} }
} }
Optional<SkipReason> skipReason = importSingleDocument(cells, fileOnDisk, filename, index); Optional<String> skipReason = importSingleDocument(cells, fileOnDisk, filename, index);
if (skipReason.isPresent()) { if (skipReason.isPresent()) {
skippedFiles.add(new SkippedFile(filename, skipReason.get())); skippedFiles.add(new SkippedFile(filename, skipReason.get()));
} else { } else {
@@ -333,23 +320,6 @@ public class MassImportService {
return new ProcessResult(processed, skippedFiles); return new ProcessResult(processed, skippedFiles);
} }
private boolean isValidImportFilename(String filename) {
if (filename == null || filename.isBlank()) return false;
if (filename.contains("/")) return false;
if (filename.contains("\\")) return false;
if (filename.contains("")) return false; // U+2215 DIVISION SLASH
if (filename.contains("")) return false; // U+FF0F FULLWIDTH SOLIDUS
if (filename.contains("")) return false; // U+29F5 REVERSE SOLIDUS OPERATOR
if (filename.contains("..")) return false;
if (filename.equals(".")) return false;
if (filename.contains("\0")) return false;
// Paths.get() is safe here on Linux for all inputs that passed the checks above;
// it may throw InvalidPathException for OS-specific illegal chars on Windows,
// but those are not reachable in production.
if (Paths.get(filename).isAbsolute()) return false;
return true;
}
// package-private: Mockito spy in tests can override to inject IOException // package-private: Mockito spy in tests can override to inject IOException
InputStream openFileStream(File file) throws IOException { InputStream openFileStream(File file) throws IOException {
return new FileInputStream(file); return new FileInputStream(file);
@@ -372,11 +342,11 @@ public class MassImportService {
* @return empty Optional on success; an Optional containing the skip reason on failure/skip. * @return empty Optional on success; an Optional containing the skip reason on failure/skip.
*/ */
@Transactional @Transactional
protected Optional<SkipReason> importSingleDocument(List<String> cells, Optional<File> file, String originalFilename, String index) { protected Optional<String> importSingleDocument(List<String> cells, Optional<File> file, String originalFilename, String index) {
Optional<Document> existing = documentService.findByOriginalFilename(originalFilename); Optional<Document> existing = documentService.findByOriginalFilename(originalFilename);
if (existing.isPresent() && existing.get().getStatus() != DocumentStatus.PLACEHOLDER) { if (existing.isPresent() && existing.get().getStatus() != DocumentStatus.PLACEHOLDER) {
log.info("Dokument {} existiert bereits, überspringe.", originalFilename); log.info("Dokument {} existiert bereits, überspringe.", originalFilename);
return Optional.of(SkipReason.ALREADY_EXISTS); return Optional.of("ALREADY_EXISTS");
} }
String archiveBox = getCell(cells, colBox); String archiveBox = getCell(cells, colBox);
@@ -412,7 +382,7 @@ public class MassImportService {
status = DocumentStatus.UPLOADED; status = DocumentStatus.UPLOADED;
} catch (Exception e) { } catch (Exception e) {
log.error("S3 Upload Fehler für {}", file.get().getName(), e); log.error("S3 Upload Fehler für {}", file.get().getName(), e);
return Optional.of(SkipReason.S3_UPLOAD_FAILED); return Optional.of("S3_UPLOAD_FAILED");
} }
} }
@@ -490,18 +460,11 @@ public class MassImportService {
} }
private Optional<File> findFileRecursive(String filename) { private Optional<File> findFileRecursive(String filename) {
File baseDir = new File(importDir); try (Stream<Path> walk = Files.walk(Paths.get(importDir))) {
try (Stream<Path> walk = Files.walk(baseDir.toPath())) { return walk.filter(p -> !Files.isDirectory(p))
Optional<Path> match = walk.filter(p -> !Files.isDirectory(p))
.filter(p -> p.getFileName().toString().equals(filename)) .filter(p -> p.getFileName().toString().equals(filename))
.map(Path::toFile)
.findFirst(); .findFirst();
if (match.isEmpty()) return Optional.empty();
File candidate = match.get().toFile();
String baseDirCanonical = baseDir.getCanonicalPath();
if (!candidate.getCanonicalPath().startsWith(baseDirCanonical + File.separator)) {
throw DomainException.internal(ErrorCode.INTERNAL_ERROR, "Path escape detected: " + candidate);
}
return Optional.of(candidate);
} catch (IOException e) { } catch (IOException e) {
return Optional.empty(); return Optional.empty();
} }

View File

@@ -154,10 +154,10 @@ class MassImportServiceTest {
.build(); .build();
when(documentService.findByOriginalFilename("doc001.pdf")).thenReturn(Optional.of(existing)); when(documentService.findByOriginalFilename("doc001.pdf")).thenReturn(Optional.of(existing));
Optional<MassImportService.SkipReason> result = service.importSingleDocument(minimalCells("doc001.pdf"), Optional.empty(), "doc001.pdf", "doc001"); Optional<String> result = service.importSingleDocument(minimalCells("doc001.pdf"), Optional.empty(), "doc001.pdf", "doc001");
verify(documentService, never()).save(any()); verify(documentService, never()).save(any());
assertThat(result).isPresent().contains(MassImportService.SkipReason.ALREADY_EXISTS); assertThat(result).isPresent().contains("ALREADY_EXISTS");
} }
// ─── importSingleDocument — already-exists guard fires before file I/O ───── // ─── importSingleDocument — already-exists guard fires before file I/O ─────
@@ -179,10 +179,10 @@ class MassImportServiceTest {
byte[] pdfHeader = {0x25, 0x50, 0x44, 0x46, 0x2D}; // %PDF- byte[] pdfHeader = {0x25, 0x50, 0x44, 0x46, 0x2D}; // %PDF-
Files.write(physicalFile, pdfHeader); Files.write(physicalFile, pdfHeader);
Optional<MassImportService.SkipReason> result = service.importSingleDocument( Optional<String> result = service.importSingleDocument(
minimalCells("present.pdf"), Optional.of(physicalFile.toFile()), "present.pdf", "present"); minimalCells("present.pdf"), Optional.of(physicalFile.toFile()), "present.pdf", "present");
assertThat(result).isPresent().contains(MassImportService.SkipReason.ALREADY_EXISTS); assertThat(result).isPresent().contains("ALREADY_EXISTS");
verify(s3Client, never()).putObject(any(PutObjectRequest.class), any(RequestBody.class)); verify(s3Client, never()).putObject(any(PutObjectRequest.class), any(RequestBody.class));
verify(documentService, never()).save(any()); verify(documentService, never()).save(any());
} }
@@ -204,7 +204,7 @@ class MassImportServiceTest {
assertThat(service.getStatus().skipped()).isEqualTo(1); assertThat(service.getStatus().skipped()).isEqualTo(1);
assertThat(service.getStatus().skippedFiles()) assertThat(service.getStatus().skippedFiles())
.extracting(MassImportService.SkippedFile::filename, MassImportService.SkippedFile::reason) .extracting(MassImportService.SkippedFile::filename, MassImportService.SkippedFile::reason)
.containsExactly(org.assertj.core.groups.Tuple.tuple("upload_fail.pdf", MassImportService.SkipReason.S3_UPLOAD_FAILED)); .containsExactly(org.assertj.core.groups.Tuple.tuple("upload_fail.pdf", "S3_UPLOAD_FAILED"));
} }
@Test @Test
@@ -223,7 +223,7 @@ class MassImportServiceTest {
assertThat(service.getStatus().skipped()).isEqualTo(1); assertThat(service.getStatus().skipped()).isEqualTo(1);
assertThat(service.getStatus().skippedFiles()) assertThat(service.getStatus().skippedFiles())
.extracting(MassImportService.SkippedFile::reason) .extracting(MassImportService.SkippedFile::reason)
.containsExactly(MassImportService.SkipReason.ALREADY_EXISTS); .containsExactly("ALREADY_EXISTS");
} }
// ─── importSingleDocument — create new document (metadata only) ─────────── // ─── importSingleDocument — create new document (metadata only) ───────────
@@ -283,11 +283,11 @@ class MassImportServiceTest {
doThrow(new RuntimeException("S3 error")) doThrow(new RuntimeException("S3 error"))
.when(s3Client).putObject(any(PutObjectRequest.class), any(RequestBody.class)); .when(s3Client).putObject(any(PutObjectRequest.class), any(RequestBody.class));
Optional<MassImportService.SkipReason> result = service.importSingleDocument( Optional<String> result = service.importSingleDocument(
minimalCells("fail.pdf"), Optional.of(tempFile.toFile()), "fail.pdf", "fail"); minimalCells("fail.pdf"), Optional.of(tempFile.toFile()), "fail.pdf", "fail");
verify(documentService, never()).save(any()); verify(documentService, never()).save(any());
assertThat(result).isPresent().contains(MassImportService.SkipReason.S3_UPLOAD_FAILED); assertThat(result).isPresent().contains("S3_UPLOAD_FAILED");
} }
// ─── importSingleDocument — sender handling ─────────────────────────────── // ─── importSingleDocument — sender handling ───────────────────────────────
@@ -438,110 +438,6 @@ class MassImportServiceTest {
verify(documentService).findByOriginalFilename("doc002.pdf"); verify(documentService).findByOriginalFilename("doc002.pdf");
} }
// ─── isValidImportFilename — security regression — do not remove ─────────
@Test
void isValidImportFilename_returnsFalse_whenFilenameIsNull() {
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", (String) null);
assertThat(result).isFalse();
}
@Test
void isValidImportFilename_returnsFalse_whenFilenameIsBlank() {
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", " ");
assertThat(result).isFalse();
}
@Test
void isValidImportFilename_returnsFalse_whenFilenameContainsForwardSlash() {
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "etc/passwd");
assertThat(result).isFalse();
}
@Test
void isValidImportFilename_returnsFalse_whenFilenameContainsBackslash() {
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "..\\etc\\passwd");
assertThat(result).isFalse();
}
@Test
void isValidImportFilename_returnsFalse_whenFilenameContainsDotDot() {
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "doc..evil.pdf");
assertThat(result).isFalse();
}
@Test
void isValidImportFilename_returnsFalse_whenFilenameIsDotDot() {
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "..");
assertThat(result).isFalse();
}
@Test
void isValidImportFilename_returnsFalse_whenFilenameIsAbsolutePath() {
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "/etc/passwd");
assertThat(result).isFalse();
}
@Test
void isValidImportFilename_returnsFalse_whenFilenameContainsNullByte() {
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "file\0.pdf");
assertThat(result).isFalse();
}
@Test
void isValidImportFilename_returnsTrue_whenFilenameIsPlainBasename() {
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "document.pdf");
assertThat(result).isTrue();
}
@Test
void isValidImportFilename_returnsFalse_whenFilenameContainsUnicodeDivisionSlash() {
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "foobar.pdf");
assertThat(result).isFalse();
}
@Test
void isValidImportFilename_returnsFalse_whenFilenameContainsFullwidthSlash() {
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "foobar.pdf");
assertThat(result).isFalse();
}
@Test
void isValidImportFilename_returnsFalse_whenFilenameContainsUnicodeReverseSolidus() {
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "foobar.pdf");
assertThat(result).isFalse();
}
@Test
void isValidImportFilename_returnsTrue_whenFilenameHasLeadingDot() {
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", ".hidden.pdf");
assertThat(result).isTrue();
}
@Test
void isValidImportFilename_returnsTrue_whenFilenameHasSpaces() {
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "Brief an Oma.pdf");
assertThat(result).isTrue();
}
@Test
void processRows_skipsRowAndContinues_whenFilenameIsPathTraversal() {
when(documentService.findByOriginalFilename("legitimate.pdf")).thenReturn(Optional.empty());
when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0));
List<List<String>> rows = List.of(
List.of("header"),
minimalCells("../evil"), // row 1: path traversal — should be skipped
minimalCells("legitimate.pdf") // row 2: valid — should be processed
);
MassImportService.ProcessResult result = ReflectionTestUtils.invokeMethod(service, "processRows", rows);
assertThat(result.processed()).isEqualTo(1);
assertThat(result.skippedFiles())
.extracting(MassImportService.SkippedFile::reason)
.containsExactly(MassImportService.SkipReason.INVALID_FILENAME_PATH_TRAVERSAL);
}
// ─── importSingleDocument — non-blank optional fields ──────────────────── // ─── importSingleDocument — non-blank optional fields ────────────────────
@Test @Test
@@ -755,22 +651,7 @@ class MassImportServiceTest {
assertThat(spyService.getStatus().skipped()).isEqualTo(1); assertThat(spyService.getStatus().skipped()).isEqualTo(1);
assertThat(spyService.getStatus().skippedFiles()) assertThat(spyService.getStatus().skippedFiles())
.extracting(MassImportService.SkippedFile::reason) .extracting(MassImportService.SkippedFile::reason)
.containsExactly(MassImportService.SkipReason.FILE_READ_ERROR); .containsExactly("FILE_READ_ERROR");
}
// ─── findFileRecursive — symlink escape security regression — do not remove ─
@Test
void findFileRecursive_throwsDomainException_whenSymlinkEscapesImportDir(
@TempDir Path importDirPath, @TempDir Path outsideDir) throws Exception {
Path outsideFile = outsideDir.resolve("secret.pdf");
Files.writeString(outsideFile, "sensitive content");
Files.createSymbolicLink(importDirPath.resolve("secret.pdf"), outsideFile);
ReflectionTestUtils.setField(service, "importDir", importDirPath.toString());
assertThatThrownBy(() -> ReflectionTestUtils.invokeMethod(service, "findFileRecursive", "secret.pdf"))
.isInstanceOf(DomainException.class);
} }
// ─── readOds — XXE security regression ─────────────────────────────────── // ─── readOds — XXE security regression ───────────────────────────────────

View File

@@ -252,8 +252,6 @@ services:
OTEL_METRICS_EXPORTER: none OTEL_METRICS_EXPORTER: none
MANAGEMENT_METRICS_TAGS_APPLICATION: Familienarchiv MANAGEMENT_METRICS_TAGS_APPLICATION: Familienarchiv
MANAGEMENT_TRACING_SAMPLING_PROBABILITY: ${MANAGEMENT_TRACING_SAMPLING_PROBABILITY:-0.1} MANAGEMENT_TRACING_SAMPLING_PROBABILITY: ${MANAGEMENT_TRACING_SAMPLING_PROBABILITY:-0.1}
SENTRY_DSN: ${SENTRY_DSN:-}
LOGGING_STRUCTURED_FORMAT_CONSOLE: ecs
networks: networks:
- archiv-net - archiv-net
healthcheck: healthcheck:
@@ -268,10 +266,6 @@ services:
build: build:
context: ./frontend context: ./frontend
target: production target: production
args:
# Vite build-time variable — baked into the JS bundle at build time.
# Empty default so deploys succeed before the secret is configured.
VITE_SENTRY_DSN: ${VITE_SENTRY_DSN:-}
restart: unless-stopped restart: unless-stopped
depends_on: depends_on:
backend: backend:

View File

@@ -80,14 +80,6 @@ _See also [DocumentStatus lifecycle](#documentstatus-lifecycle)._
**Sütterlin** — A specific standardized style of Kurrent taught in German schools from 1915 to 1941. **Sütterlin** — A specific standardized style of Kurrent taught in German schools from 1915 to 1941.
**Illegible word** — a word whose recognition confidence falls below the configured threshold; replaced with the literal token `[unleserlich]` in the rendered block text and counted in the `ocr_illegible_words_total` Prometheus counter.
**Models-ready gauge** — the `ocr_models_ready` Prometheus gauge, flipped from `0` to `1` once the FastAPI lifespan startup has finished loading the Kraken model and the spell-checker. Used both for the `/health` endpoint and as the supervised signal for the `ocr_models_ready < 1 for 2m` alert.
**Recognition model accuracy** — the accuracy reported by `ketos train` for the recognition (text-line) model, exposed as `ocr_model_accuracy{kind="recognition"}`. Sourced from `_parse_best_checkpoint` on the highest-scoring checkpoint after training.
**Segmentation model accuracy** — the accuracy reported by `ketos segtrain` for the baseline layout analysis (`blla`) model, exposed as `ocr_model_accuracy{kind="segmentation"}`. Distinct from recognition accuracy because the two models are trained and improved independently.
--- ---
## Other Domain Terms ## Other Domain Terms

View File

@@ -118,14 +118,11 @@ To find a trace for a specific request in staging/production, either increase th
## Metrics (Prometheus → Grafana) ## Metrics (Prometheus → Grafana)
Prometheus scrapes two targets every 15 s: Prometheus scrapes the backend management endpoint every 15 s:
``` ```
Target: backend:8081/actuator/prometheus Target: backend:8081/actuator/prometheus
Labels: job="spring-boot", application="Familienarchiv" Labels: job="spring-boot", application="Familienarchiv"
Target: ocr:8000/metrics
Labels: job="ocr-service"
``` ```
All Spring Boot metrics carry the `application="Familienarchiv"` tag, which is how the Grafana Spring Boot Observability dashboard (ID 17175) filters to this service. All Spring Boot metrics carry the `application="Familienarchiv"` tag, which is how the Grafana Spring Boot Observability dashboard (ID 17175) filters to this service.
@@ -149,70 +146,6 @@ jvm_memory_used_bytes{area="heap", application="Familienarchiv"}
hikaricp_connections_active hikaricp_connections_active
``` ```
### OCR-service custom metrics
Exposed at `ocr:8000/metrics` by `prometheus-fastapi-instrumentator`. The
`http_*` metrics describe the FastAPI request layer; the `ocr_*` series are
domain-specific. **Never label these with PII or document content** — labels
have unbounded cardinality risk and are visible to anyone with Grafana access.
| Metric | Type | Labels | Unit | What it tracks |
|---|---|---|---|---|
| `ocr_jobs_total` | Counter | `engine` (`surya`/`kraken`), `script_type` | jobs | OCR jobs that started after a successful PDF download |
| `ocr_pages_total` | Counter | `engine` | pages | Successfully OCR'd pages in the streaming generator |
| `ocr_skipped_pages_total` | Counter | — | pages | Pages skipped because the engine raised on them |
| `ocr_words_total` | Counter | — | words | Recognized words summed across every block |
| `ocr_illegible_words_total` | Counter | — | words | Words below the confidence threshold (rendered as `[unleserlich]`) |
| `ocr_processing_seconds` | Histogram | `engine` | seconds | Per-page (stream) or per-document (`/ocr`) engine time, excluding preprocessing |
| `ocr_training_runs_total` | Counter | `kind` (`recognition`/`segmentation`), `outcome` (`success`/`error`) | runs | Completed training runs |
| `ocr_model_accuracy` | Gauge | `kind` | ratio (01) | Latest accuracy reported by a successful training run |
| `ocr_models_ready` | Gauge | — | 0\|1 | 1 once the lifespan startup has finished loading models |
Canonical example queries (the same ones referenced in issue #652):
```promql
# OCR throughput by engine
sum by (engine) (rate(ocr_pages_total[5m]))
# Share of words rendered as [unleserlich]
sum(rate(ocr_illegible_words_total[5m]))
/ sum(rate(ocr_words_total[5m]))
# p95 page processing time per engine
histogram_quantile(0.95, sum by (engine, le) (
rate(ocr_processing_seconds_bucket[5m])
))
# Training error rate
sum(rate(ocr_training_runs_total{outcome="error"}[1h]))
/ sum(rate(ocr_training_runs_total[1h]))
# Latest recognition vs segmentation accuracy
ocr_model_accuracy
```
### Internal-only endpoints
`/metrics` is exposed by the OCR service over plain HTTP without
authentication. The container is reachable only on the internal Docker
network — Caddy never proxies to it directly. If the service is ever
exposed (e.g. a `ports:` mapping is added), block the endpoint at the
reverse proxy:
```caddy
ocr.example.com {
@internal_only path /metrics /health
respond @internal_only 404
reverse_proxy ocr:8000
}
```
The `MetricsPathFilter` in `ocr-service/main.py` suppresses uvicorn's
**stdout** access log lines for `/metrics` and `/health` so the container
console stays focused on real OCR traffic. Promtail/Loki still receive
access lines from any other source. Treat the filter as console
noise-control, not an audit-suppression mechanism.
## Errors (GlitchTip) ## Errors (GlitchTip)
GlitchTip receives errors from both the backend (via Sentry Java SDK) and the frontend (via Sentry JavaScript SDK). It groups events by fingerprint, tracks first/last seen times, and links to the release that introduced the error. GlitchTip receives errors from both the backend (via Sentry Java SDK) and the frontend (via Sentry JavaScript SDK). It groups events by fingerprint, tracks first/last seen times, and links to the release that introduced the error.

View File

@@ -1,94 +0,0 @@
# ADR-023: Prometheus Instrumentator and Metrics Registry Injection
## Status
Accepted
## Context
Until issue #652 the OCR service exposed no `/metrics` endpoint. The
observability stack already scrapes the Spring Boot backend's actuator
endpoint, but it had nothing to scrape on the Python side. Without HTTP-
and domain-level metrics from `ocr-service` we cannot answer questions
like "what is the share of words rendered as `[unleserlich]`" or
"is the training error rate above its budget" from Grafana.
Two implementation requirements influenced the design:
1. **Counter / gauge isolation in tests.** `prometheus_client` collectors
are module-level singletons keyed by name on the global `REGISTRY`.
Re-importing or naively re-instantiating them raises a duplicated-
collector error and cross-test state leaks (a `.inc()` in test A is
still readable by test B). A test harness needs a way to swap the
active container for a fresh per-test instance.
2. **Minimal blast radius on the request path.** We did not want to
hand-instrument every endpoint with FastAPI middleware. The
`prometheus-fastapi-instrumentator` library already provides
`http_requests_total`, `http_request_duration_seconds`, and the
`/metrics` exposition route, all idiomatic Prometheus names.
## Decision
- Add `prometheus-fastapi-instrumentator==7.0.0` and pin its transitive
dependency `prometheus-client==0.25.0` explicitly in
`ocr-service/requirements.txt`.
- Mount the instrumentator once at module load:
`Instrumentator(excluded_handlers=["/health", "/metrics"]).instrument(app).expose(app)`.
This adds `/metrics` and an HTTP-level dashboard surface without
changing any endpoint code.
- Define every domain metric (`ocr_jobs_total`, `ocr_pages_total`,
`ocr_processing_seconds`, …) inside a `build_metrics(registry)`
factory in `ocr-service/metrics.py` that returns a frozen `OcrMetrics`
dataclass. Production code binds the container to the default
`REGISTRY` once: `metrics: OcrMetrics = build_metrics(REGISTRY)`.
- Tests use a `fresh_metrics` fixture that builds a new
`CollectorRegistry()` per test and monkeypatches `main.metrics` with
a container bound to it. The endpoint code keeps reading
`metrics.<name>` without knowing whether it is talking to the global
registry or a per-test one.
## Consequences
**Positive**
- One reusable factory captures the metric definitions; future metrics
go in one place.
- Tests run with full counter isolation. Cross-test state leakage is
impossible because each test sees its own dataclass instance.
- The instrumentator gives us `http_*` metrics for free, including a
Grafana-ready histogram that pairs with the Spring Boot one.
**Negative**
- One extra level of indirection: any test that asserts on metric
values must remember to monkeypatch `main.metrics`, not the registry
directly. Rebinding through the registry is harmless but useless —
the dataclass holds references to the original collectors.
- `prometheus-client` is now pinned. Upgrading it requires an explicit
bump and re-checking the instrumentator's compatibility range.
- `/metrics` is exposed unauthenticated and relies on the Docker
internal network for confidentiality. See
[docs/OBSERVABILITY.md §Internal-only endpoints](../OBSERVABILITY.md)
for the Caddy snippet that must be added if the service ever gets a
host-side port mapping.
## Alternatives considered
- **Hand-roll the `/metrics` endpoint.** Rejected: would have meant
duplicating what `prometheus-fastapi-instrumentator` ships, plus
middleware for the HTTP histograms.
- **Skip the factory; pass `registry` as a function argument
everywhere.** Rejected: clutters every endpoint signature and breaks
the symmetry with the Spring Boot side, which also relies on a
process-global Micrometer registry.
- **Use a `pytest` autouse fixture that resets `REGISTRY` between
tests.** Rejected: `prometheus_client` does not expose a clean
"unregister all" hook, and we would be relying on private APIs.
## References
- Issue: [#652](https://git.raddatz.cloud/marcel/familienarchiv/issues/652)
- Library: <https://github.com/trallnag/prometheus-fastapi-instrumentator>
- Code: `ocr-service/metrics.py`, `ocr-service/main.py`,
`ocr-service/test_metrics.py`

View File

@@ -43,8 +43,6 @@ Rel(ocr, storage, "Fetches PDF via presigned URL", "HTTP / S3 presigned")
Rel(mc, storage, "Bootstraps bucket + service account on startup", "MinIO Client CLI") Rel(mc, storage, "Bootstraps bucket + service account on startup", "MinIO Client CLI")
Rel(promtail, loki, "Pushes log streams", "HTTP/Loki push API") Rel(promtail, loki, "Pushes log streams", "HTTP/Loki push API")
Rel(backend, tempo, "Sends distributed traces via OTLP", "HTTP / OTLP / port 4318 (archiv-net)") Rel(backend, tempo, "Sends distributed traces via OTLP", "HTTP / OTLP / port 4318 (archiv-net)")
Rel(prometheus, backend, "Scrapes JVM + HTTP metrics", "HTTP 8081 /actuator/prometheus")
Rel(prometheus, ocr, "Scrapes OCR + http_* metrics", "HTTP 8000 /metrics")
Rel(grafana, prometheus, "Queries metrics", "HTTP 9090") Rel(grafana, prometheus, "Queries metrics", "HTTP 9090")
Rel(grafana, loki, "Queries logs", "HTTP 3100") Rel(grafana, loki, "Queries logs", "HTTP 3100")
Rel(grafana, tempo, "Queries traces", "HTTP 3200") Rel(grafana, tempo, "Queries traces", "HTTP 3200")

View File

@@ -16,10 +16,6 @@ CMD ["npm", "run", "dev"]
# Compiles the SvelteKit Node-adapter output to /app/build. # Compiles the SvelteKit Node-adapter output to /app/build.
FROM node:20.19.0-alpine3.21 AS build FROM node:20.19.0-alpine3.21 AS build
WORKDIR /app WORKDIR /app
# VITE_SENTRY_DSN is a build-time variable — Vite bakes it into the bundle.
# Passed via docker-compose build.args; empty string disables the SDK.
ARG VITE_SENTRY_DSN
ENV VITE_SENTRY_DSN=$VITE_SENTRY_DSN
COPY package.json package-lock.json ./ COPY package.json package-lock.json ./
RUN npm ci RUN npm ci
COPY . . COPY . .

View File

@@ -106,31 +106,6 @@ export default defineConfig(
] ]
} }
}, },
{
// Forbid test fixtures (*.test-fixture.svelte) from being imported by
// production code. Tree-shaking keeps them out of the production bundle
// today (no route reaches them), but a lint rule makes the boundary
// explicit so an accidental autocomplete import in a route or component
// fails fast. Test files (*.spec.ts / *.test.ts) and the fixtures
// themselves are exempt — see the next block. Nora #2 on PR #629
// round 3.
files: ['**/*.svelte', '**/*.svelte.ts', '**/*.svelte.js', '**/*.ts'],
ignores: ['**/*.spec.ts', '**/*.test.ts', '**/*.test-fixture.svelte'],
rules: {
'no-restricted-imports': [
'error',
{
patterns: [
{
group: ['**/*.test-fixture.svelte'],
message:
'Test fixtures (*.test-fixture.svelte) are test-only — do not import from production code. Tracked by #637.'
}
]
}
]
}
},
{ {
plugins: { boundaries }, plugins: { boundaries },
settings: { settings: {

View File

@@ -445,12 +445,8 @@
"person_mention_load_error": "Person konnte nicht geladen werden.", "person_mention_load_error": "Person konnte nicht geladen werden.",
"person_mention_loading": "Lade Person…", "person_mention_loading": "Lade Person…",
"person_mention_popup_empty": "Keine Personen gefunden", "person_mention_popup_empty": "Keine Personen gefunden",
"person_mention_search_label": "Person suchen",
"person_mention_search_prompt": "Namen eingeben…",
"person_mention_btn_label": "Person verlinken", "person_mention_btn_label": "Person verlinken",
"person_mention_create_new": "Neue Person anlegen", "person_mention_create_new": "Neue Person anlegen",
"person_mention_results_count_singular": "1 Person gefunden",
"person_mention_results_count_plural": "{count} Personen gefunden",
"transcription_editor_aria_label": "Transkriptionstext", "transcription_editor_aria_label": "Transkriptionstext",
"person_born_name_prefix": "geb.", "person_born_name_prefix": "geb.",
"page_title_home": "Archiv", "page_title_home": "Archiv",
@@ -638,9 +634,6 @@
"transcription_block_review": "Als geprüft markieren", "transcription_block_review": "Als geprüft markieren",
"transcription_block_unreview": "Markierung aufheben", "transcription_block_unreview": "Markierung aufheben",
"transcription_reviewed_count": "{reviewed} von {total} geprüft", "transcription_reviewed_count": "{reviewed} von {total} geprüft",
"transcription_mark_all_reviewed": "Alle als fertig markieren",
"transcription_mark_all_reviewed_disabled": "Alle Blöcke sind bereits als fertig markiert",
"transcription_mark_all_reviewed_error": "Markierung fehlgeschlagen. Bitte versuchen Sie es erneut.",
"training_ocr_heading": "Kurrent-Erkennung trainieren", "training_ocr_heading": "Kurrent-Erkennung trainieren",
"training_ocr_description": "Starte ein neues Training mit den bisher geprüften OCR-Blöcken, um die Erkennungsgenauigkeit für Kurrentschrift zu verbessern.", "training_ocr_description": "Starte ein neues Training mit den bisher geprüften OCR-Blöcken, um die Erkennungsgenauigkeit für Kurrentschrift zu verbessern.",
"training_ocr_blocks_ready": "{blocks} geprüfte Blöcke bereit / {docs} Dokumente", "training_ocr_blocks_ready": "{blocks} geprüfte Blöcke bereit / {docs} Dokumente",

View File

@@ -445,12 +445,8 @@
"person_mention_load_error": "Could not load person.", "person_mention_load_error": "Could not load person.",
"person_mention_loading": "Loading person…", "person_mention_loading": "Loading person…",
"person_mention_popup_empty": "No persons found", "person_mention_popup_empty": "No persons found",
"person_mention_search_label": "Search for a person",
"person_mention_search_prompt": "Enter a name…",
"person_mention_btn_label": "Link person", "person_mention_btn_label": "Link person",
"person_mention_create_new": "Create new person", "person_mention_create_new": "Create new person",
"person_mention_results_count_singular": "1 person found",
"person_mention_results_count_plural": "{count} persons found",
"transcription_editor_aria_label": "Transcription text", "transcription_editor_aria_label": "Transcription text",
"person_born_name_prefix": "née", "person_born_name_prefix": "née",
"page_title_home": "Archive", "page_title_home": "Archive",
@@ -638,9 +634,6 @@
"transcription_block_review": "Mark as reviewed", "transcription_block_review": "Mark as reviewed",
"transcription_block_unreview": "Unmark as reviewed", "transcription_block_unreview": "Unmark as reviewed",
"transcription_reviewed_count": "{reviewed} of {total} reviewed", "transcription_reviewed_count": "{reviewed} of {total} reviewed",
"transcription_mark_all_reviewed": "Mark all as reviewed",
"transcription_mark_all_reviewed_disabled": "All blocks are already marked as reviewed",
"transcription_mark_all_reviewed_error": "Failed to mark all as reviewed. Please try again.",
"training_ocr_heading": "Train Kurrent recognition", "training_ocr_heading": "Train Kurrent recognition",
"training_ocr_description": "Start a new training run using the reviewed OCR blocks to improve recognition accuracy for Kurrent script.", "training_ocr_description": "Start a new training run using the reviewed OCR blocks to improve recognition accuracy for Kurrent script.",
"training_ocr_blocks_ready": "{blocks} reviewed blocks ready / {docs} documents", "training_ocr_blocks_ready": "{blocks} reviewed blocks ready / {docs} documents",

View File

@@ -445,12 +445,8 @@
"person_mention_load_error": "No se pudo cargar la persona.", "person_mention_load_error": "No se pudo cargar la persona.",
"person_mention_loading": "Cargando persona…", "person_mention_loading": "Cargando persona…",
"person_mention_popup_empty": "No se encontraron personas", "person_mention_popup_empty": "No se encontraron personas",
"person_mention_search_label": "Buscar persona",
"person_mention_search_prompt": "Escribe un nombre…",
"person_mention_btn_label": "Vincular persona", "person_mention_btn_label": "Vincular persona",
"person_mention_create_new": "Crear nueva persona", "person_mention_create_new": "Crear nueva persona",
"person_mention_results_count_singular": "1 persona encontrada",
"person_mention_results_count_plural": "{count} personas encontradas",
"transcription_editor_aria_label": "Texto de transcripción", "transcription_editor_aria_label": "Texto de transcripción",
"person_born_name_prefix": "n.", "person_born_name_prefix": "n.",
"page_title_home": "Archivo", "page_title_home": "Archivo",
@@ -638,9 +634,6 @@
"transcription_block_review": "Marcar como revisado", "transcription_block_review": "Marcar como revisado",
"transcription_block_unreview": "Desmarcar como revisado", "transcription_block_unreview": "Desmarcar como revisado",
"transcription_reviewed_count": "{reviewed} de {total} revisados", "transcription_reviewed_count": "{reviewed} de {total} revisados",
"transcription_mark_all_reviewed": "Marcar todo como revisado",
"transcription_mark_all_reviewed_disabled": "Todos los bloques ya están marcados como revisados",
"transcription_mark_all_reviewed_error": "Error al marcar como revisado. Intente de nuevo.",
"training_ocr_heading": "Entrenar reconocimiento Kurrent", "training_ocr_heading": "Entrenar reconocimiento Kurrent",
"training_ocr_description": "Inicia un nuevo entrenamiento con los bloques OCR revisados para mejorar la precisión de reconocimiento del script Kurrent.", "training_ocr_description": "Inicia un nuevo entrenamiento con los bloques OCR revisados para mejorar la precisión de reconocimiento del script Kurrent.",
"training_ocr_blocks_ready": "{blocks} bloques revisados listos / {docs} documentos", "training_ocr_blocks_ready": "{blocks} bloques revisados listos / {docs} documentos",

View File

@@ -17,7 +17,6 @@ import PdfViewer from '$lib/document/viewer/PdfViewer.svelte';
import { bulkTitleFromFilename } from '$lib/document/filename'; import { bulkTitleFromFilename } from '$lib/document/filename';
import type { Tag } from '$lib/tag/TagInput.svelte'; import type { Tag } from '$lib/tag/TagInput.svelte';
import type { components } from '$lib/generated/api'; import type { components } from '$lib/generated/api';
import { withCsrf } from '$lib/shared/cookies';
type Person = components['schemas']['Person']; type Person = components['schemas']['Person'];
@@ -184,10 +183,7 @@ async function saveUpload() {
// FormData with per-chunk progress. Session cookie is sent automatically // FormData with per-chunk progress. Session cookie is sent automatically
// by the browser for same-origin requests. // by the browser for same-origin requests.
try { try {
const res = await fetch( const res = await fetch('/api/documents/quick-upload', { method: 'POST', body: formData });
'/api/documents/quick-upload',
withCsrf({ method: 'POST', body: formData })
);
const body = await res.json().catch(() => ({ errors: [] })); const body = await res.json().catch(() => ({ errors: [] }));
const errorFilenames = new Set<string>( const errorFilenames = new Set<string>(
(body.errors ?? []).map((err: { filename: string }) => err.filename) (body.errors ?? []).map((err: { filename: string }) => err.filename)

View File

@@ -1,7 +1,7 @@
import { describe, it, expect, vi, afterEach } from 'vitest'; import { describe, it, expect, vi, afterEach } from 'vitest';
import { cleanup, render } from 'vitest-browser-svelte'; import { cleanup, render } from 'vitest-browser-svelte';
import { page } from 'vitest/browser'; import { page } from 'vitest/browser';
import TranscriptionBlockHost from './TranscriptionBlock.test-fixture.svelte'; import TranscriptionBlockHost from './TranscriptionBlock.test-host.svelte';
import type { ConfirmService } from '$lib/shared/services/confirm.svelte.js'; import type { ConfirmService } from '$lib/shared/services/confirm.svelte.js';
afterEach(cleanup); afterEach(cleanup);

View File

@@ -6,7 +6,6 @@ import TranscribeCoachEmptyState from '$lib/shared/help/TranscribeCoachEmptyStat
import type { PersonMention, TranscriptionBlockData } from '$lib/shared/types'; import type { PersonMention, TranscriptionBlockData } from '$lib/shared/types';
import { createBlockAutoSave } from '$lib/document/transcription/useBlockAutoSave.svelte'; import { createBlockAutoSave } from '$lib/document/transcription/useBlockAutoSave.svelte';
import { createBlockDragDrop } from '$lib/document/transcription/useBlockDragDrop.svelte'; import { createBlockDragDrop } from '$lib/document/transcription/useBlockDragDrop.svelte';
import { withCsrf } from '$lib/shared/cookies';
type Props = { type Props = {
documentId: string; documentId: string;
@@ -50,7 +49,6 @@ let activeBlockId: string | null = $state(null);
let localLabels: string[] = $derived.by(() => [...trainingLabels]); let localLabels: string[] = $derived.by(() => [...trainingLabels]);
let listEl: HTMLElement | null = $state(null); let listEl: HTMLElement | null = $state(null);
let markingAllReviewed = $state(false); let markingAllReviewed = $state(false);
let markAllError = $state<string | null>(null);
const sortedBlocks = $derived([...blocks].sort((a, b) => a.sortOrder - b.sortOrder)); const sortedBlocks = $derived([...blocks].sort((a, b) => a.sortOrder - b.sortOrder));
const hasBlocks = $derived(blocks.length > 0); const hasBlocks = $derived(blocks.length > 0);
@@ -69,11 +67,8 @@ $effect(() => {
async function handleMarkAllReviewed() { async function handleMarkAllReviewed() {
if (!onMarkAllReviewed) return; if (!onMarkAllReviewed) return;
markingAllReviewed = true; markingAllReviewed = true;
markAllError = null;
try { try {
await onMarkAllReviewed(); await onMarkAllReviewed();
} catch {
markAllError = m.transcription_mark_all_reviewed_error();
} finally { } finally {
markingAllReviewed = false; markingAllReviewed = false;
} }
@@ -114,14 +109,11 @@ function handleDelete(blockId: string) {
async function reorder(newOrder: string[]) { async function reorder(newOrder: string[]) {
try { try {
const res = await fetch( const res = await fetch(`/api/documents/${documentId}/transcription-blocks/reorder`, {
`/api/documents/${documentId}/transcription-blocks/reorder`,
withCsrf({
method: 'PUT', method: 'PUT',
headers: { 'Content-Type': 'application/json' }, headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ blockIds: newOrder }) body: JSON.stringify({ blockIds: newOrder })
}) });
);
if (!res.ok) return; if (!res.ok) return;
const updated = await res.json(); const updated = await res.json();
for (const b of updated) { for (const b of updated) {
@@ -177,7 +169,7 @@ async function handleLabelToggle(label: string) {
<button <button
onclick={handleMarkAllReviewed} onclick={handleMarkAllReviewed}
disabled={allReviewed || markingAllReviewed} disabled={allReviewed || markingAllReviewed}
title={allReviewed ? m.transcription_mark_all_reviewed_disabled() : undefined} title={allReviewed ? 'Alle Blöcke sind bereits als fertig markiert' : undefined}
class="flex min-h-[44px] items-center gap-1.5 rounded-sm px-3 font-sans text-xs font-medium text-brand-navy/80 transition-colors hover:text-brand-navy focus-visible:ring-2 focus-visible:ring-brand-navy disabled:opacity-40" class="flex min-h-[44px] items-center gap-1.5 rounded-sm px-3 font-sans text-xs font-medium text-brand-navy/80 transition-colors hover:text-brand-navy focus-visible:ring-2 focus-visible:ring-brand-navy disabled:opacity-40"
> >
{#if markingAllReviewed} {#if markingAllReviewed}
@@ -215,7 +207,7 @@ async function handleLabelToggle(label: string) {
<path stroke-linecap="round" stroke-linejoin="round" d="M5 13l4 4L19 7" /> <path stroke-linecap="round" stroke-linejoin="round" d="M5 13l4 4L19 7" />
</svg> </svg>
{/if} {/if}
{m.transcription_mark_all_reviewed()} Alle als fertig markieren
</button> </button>
{/if} {/if}
</div> </div>
@@ -225,31 +217,6 @@ async function handleLabelToggle(label: string) {
style="width: {reviewProgress}%" style="width: {reviewProgress}%"
></div> ></div>
</div> </div>
{#if markAllError}
<div
role="alert"
class="mt-1.5 flex items-center gap-2 rounded-sm border border-red-200 bg-red-50 px-3 py-2 font-sans text-sm text-red-700"
>
<span class="flex-1">{markAllError}</span>
<button
onclick={() => (markAllError = null)}
aria-label={m.comp_dismiss()}
class="flex min-h-[44px] min-w-[44px] items-center justify-center rounded text-red-600 hover:text-red-700 focus-visible:ring-2 focus-visible:ring-red-500"
>
<svg
class="h-4 w-4"
fill="none"
stroke="currentColor"
stroke-width="2"
viewBox="0 0 24 24"
xmlns="http://www.w3.org/2000/svg"
aria-hidden="true"
>
<path stroke-linecap="round" stroke-linejoin="round" d="M6 18L18 6M6 6l12 12" />
</svg>
</button>
</div>
{/if}
</div> </div>
<div class="p-4"> <div class="p-4">
<!-- svelte-ignore a11y_no_static_element_interactions --> <!-- svelte-ignore a11y_no_static_element_interactions -->

View File

@@ -3,7 +3,6 @@ import { cleanup, render } from 'vitest-browser-svelte';
import { page, userEvent } from 'vitest/browser'; import { page, userEvent } from 'vitest/browser';
import TranscriptionEditView from './TranscriptionEditView.svelte'; import TranscriptionEditView from './TranscriptionEditView.svelte';
import { createConfirmService, CONFIRM_KEY } from '$lib/shared/services/confirm.svelte.js'; import { createConfirmService, CONFIRM_KEY } from '$lib/shared/services/confirm.svelte.js';
import { m } from '$lib/paraglide/messages.js';
afterEach(cleanup); afterEach(cleanup);
@@ -313,14 +312,14 @@ describe('TranscriptionEditView — mark all reviewed', () => {
onMarkAllReviewed: vi.fn().mockResolvedValue(undefined) onMarkAllReviewed: vi.fn().mockResolvedValue(undefined)
}); });
await expect await expect
.element(page.getByRole('button', { name: m.transcription_mark_all_reviewed() })) .element(page.getByRole('button', { name: /Alle als fertig markieren/ }))
.toBeInTheDocument(); .toBeInTheDocument();
}); });
it('does not show "Alle als fertig markieren" button when onMarkAllReviewed is not provided', async () => { it('does not show "Alle als fertig markieren" button when onMarkAllReviewed is not provided', async () => {
renderView({ blocks: [unreviewedBlock1, unreviewedBlock2] }); renderView({ blocks: [unreviewedBlock1, unreviewedBlock2] });
await expect await expect
.element(page.getByRole('button', { name: m.transcription_mark_all_reviewed() })) .element(page.getByRole('button', { name: /Alle als fertig markieren/ }))
.not.toBeInTheDocument(); .not.toBeInTheDocument();
}); });
@@ -330,7 +329,7 @@ describe('TranscriptionEditView — mark all reviewed', () => {
onMarkAllReviewed: vi.fn().mockResolvedValue(undefined) onMarkAllReviewed: vi.fn().mockResolvedValue(undefined)
}); });
await expect await expect
.element(page.getByRole('button', { name: m.transcription_mark_all_reviewed() })) .element(page.getByRole('button', { name: /Alle als fertig markieren/ }))
.toBeDisabled(); .toBeDisabled();
}); });
@@ -344,7 +343,7 @@ describe('TranscriptionEditView — mark all reviewed', () => {
// userEvent.click() via Playwright CDP doesn't reliably trigger Svelte 5 onclick // userEvent.click() via Playwright CDP doesn't reliably trigger Svelte 5 onclick
// handlers when a TipTap editor is mounted in the same component tree. // handlers when a TipTap editor is mounted in the same component tree.
const btn = (await page const btn = (await page
.getByRole('button', { name: m.transcription_mark_all_reviewed() }) .getByRole('button', { name: /Alle als fertig markieren/ })
.element()) as HTMLButtonElement; .element()) as HTMLButtonElement;
btn.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); btn.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await vi.waitFor(() => expect(onMarkAllReviewed).toHaveBeenCalledTimes(1)); await vi.waitFor(() => expect(onMarkAllReviewed).toHaveBeenCalledTimes(1));
@@ -362,83 +361,12 @@ describe('TranscriptionEditView — mark all reviewed', () => {
// Same CDP click workaround: dispatch from browser JS to reliably fire Svelte 5 onclick // Same CDP click workaround: dispatch from browser JS to reliably fire Svelte 5 onclick
const btnEl = (await page const btnEl = (await page
.getByRole('button', { name: m.transcription_mark_all_reviewed() }) .getByRole('button', { name: /Alle als fertig markieren/ })
.element()) as HTMLButtonElement; .element()) as HTMLButtonElement;
btnEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); btnEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await expect await expect
.element(page.getByRole('button', { name: m.transcription_mark_all_reviewed() })) .element(page.getByRole('button', { name: /Alle als fertig markieren/ }))
.toBeDisabled(); .toBeDisabled();
resolveMarkAll(); resolveMarkAll();
}); });
it('shows error message when onMarkAllReviewed callback rejects', async () => {
const onMarkAllReviewed = vi.fn().mockRejectedValue(new Error('INTERNAL_ERROR'));
renderView({ blocks: [unreviewedBlock1, unreviewedBlock2], onMarkAllReviewed });
const btnEl = (await page
.getByRole('button', { name: m.transcription_mark_all_reviewed() })
.element()) as HTMLButtonElement;
btnEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await expect.element(page.getByRole('alert')).toBeInTheDocument();
await expect
.element(page.getByRole('alert'))
.toHaveTextContent(m.transcription_mark_all_reviewed_error());
});
it('clears error when dismiss button is clicked', async () => {
const onMarkAllReviewed = vi.fn().mockRejectedValue(new Error('INTERNAL_ERROR'));
renderView({ blocks: [unreviewedBlock1, unreviewedBlock2], onMarkAllReviewed });
const btnEl = (await page
.getByRole('button', { name: m.transcription_mark_all_reviewed() })
.element()) as HTMLButtonElement;
btnEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await expect.element(page.getByRole('alert')).toBeInTheDocument();
const dismissEl = (await page
.getByRole('button', { name: m.comp_dismiss() })
.element()) as HTMLButtonElement;
dismissEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await expect.element(page.getByRole('alert')).not.toBeInTheDocument();
});
it('clears error on next successful markAllReviewed call', async () => {
const onMarkAllReviewed = vi
.fn()
.mockRejectedValueOnce(new Error('INTERNAL_ERROR'))
.mockResolvedValue(undefined);
renderView({ blocks: [unreviewedBlock1, unreviewedBlock2], onMarkAllReviewed });
const btnEl = (await page
.getByRole('button', { name: m.transcription_mark_all_reviewed() })
.element()) as HTMLButtonElement;
btnEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await expect.element(page.getByRole('alert')).toBeInTheDocument();
// Wait for the button to be re-enabled before the second click — ensures the first
// async rejection has fully settled and Svelte has flushed state changes
await expect
.element(page.getByRole('button', { name: m.transcription_mark_all_reviewed() }))
.not.toBeDisabled();
btnEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await expect.element(page.getByRole('alert')).not.toBeInTheDocument();
});
it('re-enables button after markAllReviewed failure', async () => {
const onMarkAllReviewed = vi.fn().mockRejectedValue(new Error('INTERNAL_ERROR'));
renderView({ blocks: [unreviewedBlock1, unreviewedBlock2], onMarkAllReviewed });
const btnEl = (await page
.getByRole('button', { name: m.transcription_mark_all_reviewed() })
.element()) as HTMLButtonElement;
btnEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await expect.element(page.getByRole('alert')).toBeInTheDocument();
await expect
.element(page.getByRole('button', { name: m.transcription_mark_all_reviewed() }))
.not.toBeDisabled();
});
}); });

View File

@@ -1,6 +1,5 @@
import { SvelteMap } from 'svelte/reactivity'; import { SvelteMap } from 'svelte/reactivity';
import type { PersonMention } from '$lib/shared/types'; import type { PersonMention } from '$lib/shared/types';
import { withCsrf } from '$lib/shared/cookies';
export type SaveState = 'idle' | 'saving' | 'saved' | 'fading' | 'error'; export type SaveState = 'idle' | 'saving' | 'saved' | 'fading' | 'error';
@@ -117,15 +116,12 @@ export function createBlockAutoSave({ saveFn, documentId }: Options) {
for (const [blockId, text] of pendingTexts) { for (const [blockId, text] of pendingTexts) {
const mentions = pendingMentions.get(blockId) ?? []; const mentions = pendingMentions.get(blockId) ?? [];
clearDebounce(blockId); clearDebounce(blockId);
void fetch( void fetch(`/api/documents/${documentId}/transcription-blocks/${blockId}`, {
`/api/documents/${documentId}/transcription-blocks/${blockId}`,
withCsrf({
method: 'PUT', method: 'PUT',
headers: { 'Content-Type': 'application/json' }, headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ text, mentionedPersons: mentions }), body: JSON.stringify({ text, mentionedPersons: mentions }),
keepalive: true keepalive: true
}) });
);
pendingTexts.delete(blockId); pendingTexts.delete(blockId);
pendingMentions.delete(blockId); pendingMentions.delete(blockId);
} }

View File

@@ -259,15 +259,12 @@ describe('createTranscriptionBlocks.markAllReviewed', () => {
expect(ctrl.blocks.every((b) => b.reviewed)).toBe(true); expect(ctrl.blocks.every((b) => b.reviewed)).toBe(true);
}); });
it('throws and leaves blocks unchanged when PUT returns non-OK', async () => { it('is a no-op when PUT returns non-OK', async () => {
const fetchImpl = vi.fn(async (url: RequestInfo | URL, init?: RequestInit) => { const fetchImpl = vi.fn(async (url: RequestInfo | URL, init?: RequestInit) => {
const u = url.toString(); const u = url.toString();
const method = init?.method ?? 'GET'; const method = init?.method ?? 'GET';
if (u.includes('/review-all') && method === 'PUT') { if (u.includes('/review-all') && method === 'PUT') {
return new Response(JSON.stringify({ code: 'INTERNAL_ERROR' }), { return new Response('', { status: 500 });
status: 500,
headers: { 'Content-Type': 'application/json' }
});
} }
return new Response(JSON.stringify([baseBlock({ id: 'b-1', reviewed: false })]), { return new Response(JSON.stringify([baseBlock({ id: 'b-1', reviewed: false })]), {
status: 200, status: 200,
@@ -277,26 +274,7 @@ describe('createTranscriptionBlocks.markAllReviewed', () => {
const ctrl = createTranscriptionBlocks({ documentId: () => 'doc-1', fetchImpl }); const ctrl = createTranscriptionBlocks({ documentId: () => 'doc-1', fetchImpl });
await ctrl.load(); await ctrl.load();
await expect(ctrl.markAllReviewed()).rejects.toThrow('INTERNAL_ERROR'); await ctrl.markAllReviewed();
expect(ctrl.blocks[0].reviewed).toBe(false);
});
it('throws INTERNAL_ERROR when PUT returns non-JSON body (e.g. nginx 502)', async () => {
const fetchImpl = vi.fn(async (url: RequestInfo | URL, init?: RequestInit) => {
const u = url.toString();
const method = init?.method ?? 'GET';
if (u.includes('/review-all') && method === 'PUT') {
return new Response('Bad Gateway', { status: 502 });
}
return new Response(JSON.stringify([baseBlock({ id: 'b-1', reviewed: false })]), {
status: 200,
headers: { 'Content-Type': 'application/json' }
});
});
const ctrl = createTranscriptionBlocks({ documentId: () => 'doc-1', fetchImpl });
await ctrl.load();
await expect(ctrl.markAllReviewed()).rejects.toThrow('INTERNAL_ERROR');
expect(ctrl.blocks[0].reviewed).toBe(false); expect(ctrl.blocks[0].reviewed).toBe(false);
}); });
}); });

View File

@@ -2,7 +2,6 @@
lastEditedAt's $derived are scope-local to one computation; they're never lastEditedAt's $derived are scope-local to one computation; they're never
stored on $state. */ stored on $state. */
import type { TranscriptionBlockData, PersonMention } from '$lib/shared/types'; import type { TranscriptionBlockData, PersonMention } from '$lib/shared/types';
import { makeCsrfFetch } from '$lib/shared/cookies';
import { saveBlockWithConflictRetry } from './saveBlockWithConflictRetry'; import { saveBlockWithConflictRetry } from './saveBlockWithConflictRetry';
import { BlockConflictResolvedError } from './blockConflictMerge'; import { BlockConflictResolvedError } from './blockConflictMerge';
@@ -42,7 +41,7 @@ export function createTranscriptionBlocks(
options: TranscriptionBlocksOptions options: TranscriptionBlocksOptions
): TranscriptionBlocksController { ): TranscriptionBlocksController {
const { documentId } = options; const { documentId } = options;
const fetchImpl = makeCsrfFetch(options.fetchImpl ?? fetch); const fetchImpl = options.fetchImpl ?? fetch;
let blocks = $state<TranscriptionBlockData[]>([]); let blocks = $state<TranscriptionBlockData[]>([]);
let annotationReloadKey = $state(0); let annotationReloadKey = $state(0);
@@ -120,11 +119,7 @@ export function createTranscriptionBlocks(
const res = await fetchImpl(`/api/documents/${documentId()}/transcription-blocks/review-all`, { const res = await fetchImpl(`/api/documents/${documentId()}/transcription-blocks/review-all`, {
method: 'PUT' method: 'PUT'
}); });
if (!res.ok) { if (!res.ok) return;
const body = await res.json().catch(() => ({}));
// Never render body.message — route through getErrorMessage() to prevent leaking backend internals
throw new Error((body as { code?: string })?.code ?? 'INTERNAL_ERROR');
}
const updated = (await res.json()) as { id: string; reviewed: boolean }[]; const updated = (await res.json()) as { id: string; reviewed: boolean }[];
for (const b of updated) { for (const b of updated) {
const existing = blocks.find((x) => x.id === b.id); const existing = blocks.find((x) => x.id === b.id);

View File

@@ -2,7 +2,6 @@
import TrainingHistory from './TrainingHistory.svelte'; import TrainingHistory from './TrainingHistory.svelte';
import { m } from '$lib/paraglide/messages.js'; import { m } from '$lib/paraglide/messages.js';
import type { TrainingRun } from '$lib/ocr/training.js'; import type { TrainingRun } from '$lib/ocr/training.js';
import { withCsrf } from '$lib/shared/cookies';
interface TrainingInfo { interface TrainingInfo {
availableBlocks?: number; availableBlocks?: number;
@@ -34,7 +33,7 @@ async function startTraining() {
successMessage = null; successMessage = null;
errorMessage = null; errorMessage = null;
try { try {
const res = await fetch('/api/ocr/train', withCsrf({ method: 'POST' })); const res = await fetch('/api/ocr/train', { method: 'POST' });
if (res.ok) { if (res.ok) {
successMessage = m.training_success(); successMessage = m.training_success();
setTimeout(() => { setTimeout(() => {

View File

@@ -2,7 +2,6 @@
import TrainingHistory from './TrainingHistory.svelte'; import TrainingHistory from './TrainingHistory.svelte';
import { m } from '$lib/paraglide/messages.js'; import { m } from '$lib/paraglide/messages.js';
import type { TrainingRun } from '$lib/ocr/training.js'; import type { TrainingRun } from '$lib/ocr/training.js';
import { withCsrf } from '$lib/shared/cookies';
interface TrainingInfo { interface TrainingInfo {
availableSegBlocks?: number; availableSegBlocks?: number;
@@ -28,7 +27,7 @@ async function startTraining() {
training = true; training = true;
successMessage = null; successMessage = null;
try { try {
const res = await fetch('/api/ocr/segtrain', withCsrf({ method: 'POST' })); const res = await fetch('/api/ocr/segtrain', { method: 'POST' });
if (res.ok) { if (res.ok) {
successMessage = m.training_success(); successMessage = m.training_success();
setTimeout(() => { setTimeout(() => {

View File

@@ -1,20 +0,0 @@
import { describe, it, expect } from 'vitest';
import { extractErrorCode } from './api.server';
describe('extractErrorCode', () => {
it('returns the code string when error has a code property', () => {
expect(extractErrorCode({ code: 'DOCUMENT_NOT_FOUND' })).toBe('DOCUMENT_NOT_FOUND');
});
it('returns undefined when error is undefined', () => {
expect(extractErrorCode(undefined)).toBeUndefined();
});
it('returns undefined when error is null', () => {
expect(extractErrorCode(null)).toBeUndefined();
});
it('returns undefined when error is a plain string', () => {
expect(extractErrorCode('oops')).toBeUndefined();
});
it('returns undefined when error object has no code property', () => {
expect(extractErrorCode({ message: 'fail' })).toBeUndefined();
});
});

View File

@@ -23,11 +23,3 @@ export function createApiClient(fetch: typeof globalThis.fetch) {
fetch fetch
}); });
} }
export interface ApiError {
code?: string;
}
export function extractErrorCode(error: unknown): string | undefined {
return (error as ApiError | undefined)?.code;
}

View File

@@ -1,46 +1,3 @@
/**
* Reads the XSRF-TOKEN cookie set by Spring Security's CookieCsrfTokenRepository.
* Returns null outside the browser or when the cookie is absent.
*/
export function getCsrfToken(): string | null {
if (typeof document === 'undefined') return null;
const match = document.cookie.match(/(?:^|;\s*)XSRF-TOKEN=([^;]+)/);
return match ? decodeURIComponent(match[1]) : null;
}
/**
* Merges the X-XSRF-TOKEN header into a RequestInit so Spring Security's
* CSRF filter accepts the request. Safe to call server-side (no-op when the
* cookie is absent).
*/
export function withCsrf(init?: RequestInit): RequestInit {
const token = getCsrfToken();
if (!token) return init ?? {};
const headers = new Headers(init?.headers);
headers.set('X-XSRF-TOKEN', token);
return { ...init, headers };
}
/**
* Wraps a fetch implementation so that every state-mutating call (POST, PUT,
* PATCH, DELETE) automatically includes the X-XSRF-TOKEN header. GET/HEAD
* requests pass through unchanged.
*
* Used to CSRF-protect client-side hooks that accept an injectable fetchImpl.
* In unit tests the injected mock is wrapped but getCsrfToken() returns null
* (no browser cookie), so no header is added and existing test expectations
* are unaffected.
*/
export function makeCsrfFetch(inner: typeof fetch): typeof fetch {
return (input: RequestInfo | URL, init?: RequestInit): Promise<Response> => {
const method = (init?.method ?? 'GET').toUpperCase();
if (['POST', 'PUT', 'PATCH', 'DELETE'].includes(method)) {
return inner(input, withCsrf(init));
}
return inner(input, init);
};
}
/** /**
* Extracts the fa_session cookie value from a list of Set-Cookie response headers. * Extracts the fa_session cookie value from a list of Set-Cookie response headers.
* *

View File

@@ -2,18 +2,7 @@
import type { components } from '$lib/generated/api'; import type { components } from '$lib/generated/api';
// eslint-disable-next-line boundaries/dependencies -- mention dropdown needs person date formatting; extract to shared if it becomes reusable // eslint-disable-next-line boundaries/dependencies -- mention dropdown needs person date formatting; extract to shared if it becomes reusable
import { formatLifeDateRange } from '$lib/person/personLifeDates'; import { formatLifeDateRange } from '$lib/person/personLifeDates';
import { untrack } from 'svelte';
import { m } from '$lib/paraglide/messages.js'; import { m } from '$lib/paraglide/messages.js';
// Layered defence cap on the @mention search query length (CWE-400
// amplification). The <input maxlength> attribute below caps direct
// user edits, but the editor-mirror path (Tiptap contenteditable -> mirror
// $effect -> searchQuery) is not covered by `maxlength` since the
// contenteditable has no such enforcement. Clipping at the mirror keeps
// the cap honest from both paths. Tracked server-side separately.
// Nora #1 on PR #629. Hoisted to mentionConstants.ts so the host editor
// (PersonMentionEditor) can clip the inserted displayName to the same cap
// — see Felix #3 on PR #629.
import { MAX_QUERY_LENGTH } from './mentionConstants';
type Person = components['schemas']['Person']; type Person = components['schemas']['Person'];
@@ -28,46 +17,7 @@ type DropdownState = {
clientRect: (() => DOMRect | null) | null; clientRect: (() => DOMRect | null) | null;
}; };
let { let { model }: { model: DropdownState } = $props();
model,
editorQuery = '',
onSearch = () => {}
}: {
model: DropdownState;
/** Text typed after `@` in the host editor. Mirrors into the search input
* until the user takes manual ownership by typing into the input itself. */
editorQuery?: string;
onSearch?: (query: string) => void;
} = $props();
let searchQuery = $state(untrack(() => editorQuery.slice(0, MAX_QUERY_LENGTH)));
let userHasEdited = $state(false);
// Intent-revealing alias used by both the persistent aria-live announcer and
// the visible empty-state copy. Folding the duplicated rule into one $derived
// keeps the two branches in lockstep. Felix #3 on PR #629 round 4.
const isQueryEmpty = $derived(searchQuery.trim() === '');
// Mirror the editor's typed text until the user takes ownership.
//
// Why `$state + $effect` (not `$derived`): `searchQuery` is also written by
// `bind:value` on the <input> below, so it needs to be a mutable `$state`.
// A `$derived` would be read-only and would clobber direct user edits on
// every editor keystroke. The `userHasEdited` latch pins ownership once the
// user types into the input. Felix #1 on PR #629.
$effect(() => {
if (!userHasEdited) {
searchQuery = editorQuery.slice(0, MAX_QUERY_LENGTH);
}
});
// Fire onSearch whenever the effective query changes — covers both the
// editor mirror and direct input edits. This is the only place onSearch
// fires; when the dropdown is unmounted, the effect is disposed and no
// further fetches occur.
$effect(() => {
onSearch(searchQuery);
});
// highlightedIndex must be both writable (keyboard handler mutates it) and // highlightedIndex must be both writable (keyboard handler mutates it) and
// reset when `items` changes (so it never points past the end of a new list). // reset when `items` changes (so it never points past the end of a new list).
@@ -162,70 +112,16 @@ function selectItem(item: Person) {
unauthenticated users. unauthenticated users.
--> -->
<div <div
class="fixed z-50 w-72 max-w-[calc(100vw-1rem)] overflow-hidden rounded-sm border border-line bg-surface shadow-lg" class="fixed z-50 w-72 overflow-hidden rounded-sm border border-line bg-surface shadow-lg"
role="listbox" role="listbox"
aria-label={m.person_mention_btn_label()} aria-label={m.person_mention_btn_label()}
style:top={position.top} style:top={position.top}
style:bottom={position.bottom} style:bottom={position.bottom}
style:left={position.left} style:left={position.left}
> >
<div class="border-b border-line px-3 py-2">
<label class="sr-only" for="mention-search">{m.person_mention_search_label()}</label>
<div class="flex items-center gap-2">
<svg
aria-hidden="true"
viewBox="0 0 24 24"
fill="none"
stroke="currentColor"
stroke-width="2"
class="h-5 w-5 shrink-0 text-ink-2"
>
<circle cx="11" cy="11" r="7" />
<path d="m20 20-3.5-3.5" stroke-linecap="round" />
</svg>
<input
id="mention-search"
type="search"
data-test-search-input
maxlength={MAX_QUERY_LENGTH}
class="min-h-[44px] w-full bg-transparent font-sans text-base text-ink placeholder:text-ink-3 focus:outline-none focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-inset"
placeholder={m.person_mention_search_prompt()}
bind:value={searchQuery}
oninput={() => {
userHasEdited = true;
}}
onmousedown={(e) => e.stopPropagation()}
/>
</div>
</div>
<!--
Persistent aria-live region — lives ABOVE the conditional branches so the
element never unmounts when items transition between empty and populated.
VoiceOver in particular swallows announcements from freshly-mounted live
regions, and the previous (conditional-inside) markup silently dropped
the "N persons found" announcement when results populated. Leonie #3 on
PR #629 round 3.
-->
<p class="sr-only" aria-live="polite">
{#if model.items.length === 0} {#if model.items.length === 0}
{isQueryEmpty ? m.person_mention_search_prompt() : m.person_mention_popup_empty()} <p class="px-3 py-2.5 font-sans text-sm text-ink-3">
{:else if model.items.length === 1} {m.person_mention_popup_empty()}
{m.person_mention_results_count_singular()}
{:else}
{m.person_mention_results_count_plural({ count: model.items.length })}
{/if}
</p>
{#if model.items.length === 0}
<!--
Visible empty-state copy — visual-only. The persistent sr-only <p>
above is the sole AT announcer; this one is hidden from screen readers
via aria-hidden="true" so VoiceOver does not double-announce
(NVDA de-dups, VoiceOver does not). Leonie S-2 on PR #629 round 4.
Do NOT add an aria-live attribute here — that would re-introduce
the duplicate announcement.
-->
<p aria-hidden="true" class="px-3 py-2.5 font-sans text-sm text-ink-3">
{isQueryEmpty ? m.person_mention_search_prompt() : m.person_mention_popup_empty()}
</p> </p>
<!-- <!--
Empty-state escape hatch — without it the transcriber has to close Empty-state escape hatch — without it the transcriber has to close
@@ -236,7 +132,7 @@ function selectItem(item: Person) {
<a <a
href="/persons/new" href="/persons/new"
target="_blank" target="_blank"
rel="noopener noreferrer" rel="noopener"
class="flex min-h-[44px] items-center gap-2 border-t border-line px-3 py-2.5 font-sans text-sm font-medium text-brand-navy hover:bg-canvas focus:bg-canvas focus:outline-none" class="flex min-h-[44px] items-center gap-2 border-t border-line px-3 py-2.5 font-sans text-sm font-medium text-brand-navy hover:bg-canvas focus:bg-canvas focus:outline-none"
onmousedown={(e) => e.preventDefault()} onmousedown={(e) => e.preventDefault()}
> >

View File

@@ -1,37 +1,22 @@
import { describe, it, expect, vi, afterEach } from 'vitest'; import { describe, it, expect, vi, afterEach } from 'vitest';
import { cleanup, render } from 'vitest-browser-svelte'; import { cleanup, render } from 'vitest-browser-svelte';
import { page, userEvent } from 'vitest/browser'; import { page } from 'vitest/browser';
import { flushSync, mount, tick, unmount } from 'svelte';
import MentionDropdown from './MentionDropdown.svelte'; import MentionDropdown from './MentionDropdown.svelte';
import MentionDropdownFixture from './MentionDropdown.test-fixture.svelte';
import { m } from '$lib/paraglide/messages.js';
import type { components } from '$lib/generated/api';
type Person = components['schemas']['Person'];
afterEach(cleanup); afterEach(cleanup);
const makePerson = (id: string, name: string, overrides: Partial<Person> = {}): Person => { const makePerson = (id: string, name: string, overrides: Record<string, unknown> = {}) => ({
const parts = name.split(' ');
return {
id, id,
firstName: parts[0], firstName: name.split(' ')[0] ?? null,
lastName: parts.slice(1).join(' ') || name, lastName: name.split(' ').slice(1).join(' ') || name,
displayName: name, displayName: name,
personType: 'PERSON', birthYear: null as number | null,
familyMember: false, deathYear: null as number | null,
...overrides ...overrides
}; });
};
type DropdownState = { const baseModel = (overrides: Record<string, unknown> = {}) => ({
items: Person[]; items: [] as ReturnType<typeof makePerson>[],
command: (item: Person) => void;
clientRect: (() => DOMRect | null) | null;
};
const baseModel = (overrides: Partial<DropdownState> = {}): DropdownState => ({
items: [],
command: vi.fn(), command: vi.fn(),
clientRect: () => new DOMRect(100, 100, 0, 24), clientRect: () => new DOMRect(100, 100, 0, 24),
...overrides ...overrides
@@ -44,32 +29,14 @@ describe('MentionDropdown', () => {
await expect.element(page.getByRole('listbox', { name: /person verlinken/i })).toBeVisible(); await expect.element(page.getByRole('listbox', { name: /person verlinken/i })).toBeVisible();
}); });
it('shows the "enter a name" prompt when the search field is empty', async () => { it('renders the empty placeholder when items is empty', async () => {
render(MentionDropdown, { props: { model: baseModel() } }); render(MentionDropdown, { props: { model: baseModel() } });
// Scope to the visible empty-state <p> (text-ink-3) — the persistent await expect.element(page.getByText('Keine Personen gefunden')).toBeVisible();
// sr-only aria-live region above also contains the same prompt copy.
const visibleEmptyP = document.querySelector(
'[role="listbox"] p.text-ink-3'
) as HTMLElement | null;
expect(visibleEmptyP).not.toBeNull();
expect(visibleEmptyP!.textContent ?? '').toContain(m.person_mention_search_prompt());
expect(visibleEmptyP!.textContent ?? '').not.toContain(m.person_mention_popup_empty());
});
it('shows "no persons found" when the search has a query but the list is empty', async () => {
render(MentionDropdown, { props: { model: baseModel(), editorQuery: 'WdG' } });
const visibleEmptyP = document.querySelector(
'[role="listbox"] p.text-ink-3'
) as HTMLElement | null;
expect(visibleEmptyP).not.toBeNull();
expect(visibleEmptyP!.textContent ?? '').toContain(m.person_mention_popup_empty());
expect(visibleEmptyP!.textContent ?? '').not.toContain(m.person_mention_search_prompt());
}); });
it('shows the create-new escape hatch link in the empty state', async () => { it('shows the create-new escape hatch link in the empty state', async () => {
render(MentionDropdown, { props: { model: baseModel(), editorQuery: 'unknown' } }); render(MentionDropdown, { props: { model: baseModel() } });
const link = (await page const link = (await page
.getByRole('link', { name: /neue person anlegen/i }) .getByRole('link', { name: /neue person anlegen/i })
@@ -77,7 +44,6 @@ describe('MentionDropdown', () => {
expect(link.href).toContain('/persons/new'); expect(link.href).toContain('/persons/new');
expect(link.target).toBe('_blank'); expect(link.target).toBe('_blank');
expect(link.rel).toContain('noopener'); expect(link.rel).toContain('noopener');
expect(link.rel).toContain('noreferrer');
}); });
it('renders one option per item when populated', async () => { it('renders one option per item when populated', async () => {
@@ -138,315 +104,3 @@ describe('MentionDropdown', () => {
expect(dropdown.style.left).toBe('123px'); expect(dropdown.style.left).toBe('123px');
}); });
}); });
// ─── Search input — Issue #380 ────────────────────────────────────────────────
describe('MentionDropdown — search input', () => {
it('renders a search input pre-filled with the editorQuery prop', async () => {
render(MentionDropdown, {
props: { model: baseModel(), editorQuery: 'WdG' }
});
await expect.element(page.getByRole('searchbox')).toHaveValue('WdG');
});
it('exposes a data-test-search-input attribute for E2E selectors', async () => {
render(MentionDropdown, { props: { model: baseModel() } });
const input = document.querySelector('[data-test-search-input]');
expect(input).not.toBeNull();
expect((input as HTMLInputElement).type).toBe('search');
});
it('search input wrapper meets the 44px touch target (WCAG 2.2 AA)', async () => {
render(MentionDropdown, { props: { model: baseModel() } });
const input = document.querySelector('[data-test-search-input]') as HTMLElement;
expect(input).not.toBeNull();
expect(input.className).toContain('min-h-[44px]');
});
it('renders a persistent aria-live="polite" region (does not remount on items transition; Leonie #3 on PR #629)', async () => {
render(MentionDropdown, { props: { model: baseModel() } });
const listbox = document.querySelector('[role="listbox"]');
expect(listbox).not.toBeNull();
const live = listbox!.querySelector('p[aria-live="polite"]');
expect(live).not.toBeNull();
// Empty + empty-query → "Namen eingeben…" prompt
expect(live!.textContent ?? '').toContain(m.person_mention_search_prompt());
});
it('announces the result count in the persistent live region when items populate (Leonie #3 on PR #629)', async () => {
render(MentionDropdown, {
props: {
model: baseModel({
items: [
makePerson('p1', 'Anna Schmidt'),
makePerson('p2', 'Bert Meier'),
makePerson('p3', 'Carl Vogel')
]
})
}
});
const listbox = document.querySelector('[role="listbox"]');
expect(listbox).not.toBeNull();
const live = listbox!.querySelector('p[aria-live="polite"]');
expect(live).not.toBeNull();
// Populated → "3 Personen gefunden" (plural)
expect(live!.textContent ?? '').toContain('3');
});
it('announces the singular form when exactly one item is present (Sara #4 on PR #629)', async () => {
render(MentionDropdown, {
props: {
model: baseModel({
items: [makePerson('p1', 'Anna Schmidt')]
})
}
});
const listbox = document.querySelector('[role="listbox"]');
expect(listbox).not.toBeNull();
const live = listbox!.querySelector('p[aria-live="polite"]');
expect(live).not.toBeNull();
// Singular branch — "1 Person gefunden" / "1 person found" / "1 persona encontrada"
// (locale-dependent; resolved via the Paraglide message helper).
expect(live!.textContent ?? '').toContain(m.person_mention_results_count_singular());
});
it('keeps the visible empty-state copy without its own aria-live and hides it from AT (Leonie #3 on PR #629 round 3; Leonie S-2 round 4)', async () => {
render(MentionDropdown, { props: { model: baseModel(), editorQuery: 'WdG' } });
// Visible empty-state <p> exists with the empty-result copy ...
const empty = document.querySelector('p.text-ink-3') as HTMLElement | null;
expect(empty).not.toBeNull();
expect(empty!.textContent ?? '').toContain(m.person_mention_popup_empty());
// ... but it must NOT carry its own aria-live (the persistent sr-only
// region above the conditional is the announcer now).
expect(empty!.hasAttribute('aria-live')).toBe(false);
// ... and it MUST be hidden from screen readers via aria-hidden="true"
// so VoiceOver does not double-announce (the persistent sr-only region
// is the sole AT source of truth). Leonie S-2 on PR #629 round 4.
expect(empty!.getAttribute('aria-hidden')).toBe('true');
});
it('renders the magnifier icon at h-5 w-5 with text-ink-2 (Leonie BLOCKER on PR #629)', async () => {
render(MentionDropdown, { props: { model: baseModel() } });
const icon = document.querySelector('[data-test-search-input]')
?.previousElementSibling as SVGElement | null;
expect(icon).not.toBeNull();
expect(icon!.tagName.toLowerCase()).toBe('svg');
expect(icon!.getAttribute('class') ?? '').toContain('h-5');
expect(icon!.getAttribute('class') ?? '').toContain('w-5');
expect(icon!.getAttribute('class') ?? '').toContain('text-ink-2');
});
it('caps the search input at maxlength=100 (CWE-400 amplification — Nora on PR #629)', async () => {
render(MentionDropdown, { props: { model: baseModel() } });
const input = document.querySelector('[data-test-search-input]') as HTMLInputElement;
expect(input).not.toBeNull();
expect(input.maxLength).toBe(100);
});
it('clips a long editorQuery mirror to 100 chars (CWE-400 layered — Nora #1 on PR #629)', async () => {
const longQuery = 'A'.repeat(200);
render(MentionDropdown, { props: { model: baseModel(), editorQuery: longQuery } });
const input = document.querySelector('[data-test-search-input]') as HTMLInputElement;
expect(input).not.toBeNull();
expect(input.value.length).toBe(100);
expect(input.value).toBe('A'.repeat(100));
});
it('caps the listbox width to the viewport (320 px reflow guard — Leonie FINDING-MENTION-005)', async () => {
render(MentionDropdown, { props: { model: baseModel() } });
const listbox = document.querySelector('[role="listbox"]') as HTMLElement;
expect(listbox).not.toBeNull();
expect(listbox.className).toContain('max-w-[calc(100vw-1rem)]');
});
it('renders the @mention search input at text-base (16 px senior-audience floor — Leonie FINDING-MENTION-006)', async () => {
render(MentionDropdown, { props: { model: baseModel() } });
const input = document.querySelector('[data-test-search-input]') as HTMLInputElement;
expect(input).not.toBeNull();
expect(input.className).toContain('text-base');
expect(input.className).not.toContain('text-sm');
});
it('invokes onSearch with the current value whenever the user types', async () => {
const onSearch = vi.fn();
render(MentionDropdown, { props: { model: baseModel(), onSearch } });
await userEvent.type(page.getByRole('searchbox'), 'Walter');
await vi.waitFor(() => {
expect(onSearch).toHaveBeenCalled();
expect(onSearch).toHaveBeenLastCalledWith('Walter');
});
});
it('keeps the user-edited search value when editorQuery changes after the takeover (Felix on PR #629)', async () => {
let setEditorQuery!: (q: string) => void;
render(MentionDropdownFixture, {
model: baseModel(),
initialEditorQuery: 'WdG',
onReady: (s: (q: string) => void) => {
setEditorQuery = s;
}
});
await expect.element(page.getByRole('searchbox')).toHaveValue('WdG');
await page.getByRole('searchbox').fill('Walter');
await expect.element(page.getByRole('searchbox')).toHaveValue('Walter');
setEditorQuery('WdGruyter');
// Flush pending Svelte reactivity so any (non-)update from the mirror
// $effect has landed before we assert. expect.element already polls, so
// no fixed-timeout fallback is needed. Sara on PR #629 round 3.
await tick();
await expect.element(page.getByRole('searchbox')).toHaveValue('Walter');
});
});
// ─── ArrowDown via exported onKeyDown (Sara #3 on PR #629) ──────────────────
//
// In production, Tiptap intercepts ArrowDown/ArrowUp/Enter at the editor level
// and forwards them to the dropdown via its exported onKeyDown(event) function
// — the dropdown itself has no DOM keydown listener. This test exercises the
// same export so a regression in highlightedIndex/selection logic is caught
// at the unit level. The full E2E focus-chain test is deferred to a separate
// issue (Playwright).
//
// These unit tests directly invoke the exported `onKeyDown` to pin its
// behaviour in isolation. They do NOT exercise the Tiptap forwarding
// chain (PersonMentionEditor.suggestion.render() returning { onKeyDown })
// — that integration is covered by the 'ArrowDown moves the highlight'
// test in PersonMentionEditor.svelte.spec.ts. Sara on PR #629 round 3.
describe('MentionDropdown — onKeyDown forwarding', () => {
// flushSync ensures Svelte reactivity propagation completes before
// asserting (uniform across all four key tests so the next reader
// doesn't have to figure out why some are wrapped and others aren't).
// Felix #1 suggestion on PR #629 round 3.
it('ArrowDown advances aria-selected to the next option in the listbox', async () => {
const container = document.createElement('div');
document.body.appendChild(container);
const instance = mount(MentionDropdown, {
target: container,
props: {
model: baseModel({
items: [makePerson('p1', 'Anna Schmidt'), makePerson('p2', 'Bert Meier')]
})
}
});
try {
const exports = instance as unknown as { onKeyDown: (e: KeyboardEvent) => boolean };
// First option starts highlighted.
const first = container.querySelector('[data-test-person-id="p1"]') as HTMLElement;
const second = container.querySelector('[data-test-person-id="p2"]') as HTMLElement;
expect(first.getAttribute('aria-selected')).toBe('true');
expect(second.getAttribute('aria-selected')).toBe('false');
let consumed = false;
flushSync(() => {
consumed = exports.onKeyDown(new KeyboardEvent('keydown', { key: 'ArrowDown' }));
});
expect(consumed).toBe(true);
expect(first.getAttribute('aria-selected')).toBe('false');
expect(second.getAttribute('aria-selected')).toBe('true');
} finally {
unmount(instance);
container.remove();
}
});
it('ArrowUp wraps from the first option to the last', async () => {
const container = document.createElement('div');
document.body.appendChild(container);
const instance = mount(MentionDropdown, {
target: container,
props: {
model: baseModel({
items: [makePerson('p1', 'Anna Schmidt'), makePerson('p2', 'Bert Meier')]
})
}
});
try {
const exports = instance as unknown as { onKeyDown: (e: KeyboardEvent) => boolean };
let consumed = false;
flushSync(() => {
consumed = exports.onKeyDown(new KeyboardEvent('keydown', { key: 'ArrowUp' }));
});
expect(consumed).toBe(true);
const second = container.querySelector('[data-test-person-id="p2"]') as HTMLElement;
expect(second.getAttribute('aria-selected')).toBe('true');
} finally {
unmount(instance);
container.remove();
}
});
it('Enter invokes model.command with the currently highlighted item', async () => {
const command = vi.fn();
const container = document.createElement('div');
document.body.appendChild(container);
const instance = mount(MentionDropdown, {
target: container,
props: {
model: baseModel({
items: [makePerson('p1', 'Anna Schmidt'), makePerson('p2', 'Bert Meier')],
command
})
}
});
try {
const exports = instance as unknown as { onKeyDown: (e: KeyboardEvent) => boolean };
let consumed = false;
flushSync(() => {
consumed = exports.onKeyDown(new KeyboardEvent('keydown', { key: 'Enter' }));
});
expect(consumed).toBe(true);
expect(command).toHaveBeenCalledTimes(1);
expect(command.mock.calls[0][0].id).toBe('p1');
} finally {
unmount(instance);
container.remove();
}
});
it('Escape returns false so the suggestion plugin can handle it', async () => {
const container = document.createElement('div');
document.body.appendChild(container);
const instance = mount(MentionDropdown, {
target: container,
props: {
model: baseModel({ items: [makePerson('p1', 'Anna Schmidt')] })
}
});
try {
const exports = instance as unknown as { onKeyDown: (e: KeyboardEvent) => boolean };
let consumed = true;
flushSync(() => {
consumed = exports.onKeyDown(new KeyboardEvent('keydown', { key: 'Escape' }));
});
expect(consumed).toBe(false);
} finally {
unmount(instance);
container.remove();
}
});
});

View File

@@ -1,32 +0,0 @@
<script lang="ts">
import { untrack } from 'svelte';
import MentionDropdown from './MentionDropdown.svelte';
import type { components } from '$lib/generated/api';
type Person = components['schemas']['Person'];
type DropdownState = {
items: Person[];
command: (item: Person) => void;
clientRect: (() => DOMRect | null) | null;
};
type Props = {
model: DropdownState;
initialEditorQuery: string;
/** Test hook: receives a setter for editorQuery so the test can mutate it. */
onReady?: (setEditorQuery: (q: string) => void) => void;
onSearch?: (q: string) => void;
};
let { model, initialEditorQuery, onReady, onSearch = () => {} }: Props = $props();
let editorQuery = $state(untrack(() => initialEditorQuery));
$effect(() => {
onReady?.((q) => {
editorQuery = q;
});
});
</script>
<MentionDropdown model={model} editorQuery={editorQuery} onSearch={onSearch} />

View File

@@ -7,9 +7,7 @@ import { m } from '$lib/paraglide/messages.js';
import type { components } from '$lib/generated/api'; import type { components } from '$lib/generated/api';
import type { PersonMention } from '$lib/shared/types'; import type { PersonMention } from '$lib/shared/types';
import { deserialize, serialize } from '$lib/shared/discussion/mentionSerializer'; import { deserialize, serialize } from '$lib/shared/discussion/mentionSerializer';
import { debounce } from '$lib/shared/utils/debounce';
import MentionDropdown from './MentionDropdown.svelte'; import MentionDropdown from './MentionDropdown.svelte';
import { MAX_QUERY_LENGTH, SEARCH_DEBOUNCE_MS, SEARCH_RESULT_LIMIT } from './mentionConstants';
type Person = components['schemas']['Person']; type Person = components['schemas']['Person'];
@@ -35,13 +33,6 @@ let {
let editorEl: HTMLDivElement; let editorEl: HTMLDivElement;
let editor: Editor | null = null; let editor: Editor | null = null;
// Hoisted so onDestroy can guarantee the imperatively-mounted dropdown is
// torn down even if Tiptap's suggestion plugin onExit didn't fire (e.g. when
// the host component is unmounted while the dropdown is still open).
let mountedDropdown: object | null = null;
// Hoisted so onDestroy can cancel any pending fetch — otherwise a trailing
// debounced search can fire after the editor is gone and pollute later tests.
let cancelPendingSearch: (() => void) | null = null;
// Single reactive state object shared with MentionDropdown. Mutating these // Single reactive state object shared with MentionDropdown. Mutating these
// fields propagates to the mounted dropdown via Svelte's $state proxy — // fields propagates to the mounted dropdown via Svelte's $state proxy —
@@ -51,12 +42,10 @@ let dropdownState = $state<{
items: Person[]; items: Person[];
command: (item: Person) => void; command: (item: Person) => void;
clientRect: (() => DOMRect | null) | null; clientRect: (() => DOMRect | null) | null;
editorQuery: string;
}>({ }>({
items: [], items: [],
command: () => {}, command: () => {},
clientRect: null, clientRect: null
editorQuery: ''
}); });
type DropdownExports = { type DropdownExports = {
@@ -149,13 +138,16 @@ onMount(() => {
// Nora #5618 #3 — separate issue tracks the GET /api/persons // Nora #5618 #3 — separate issue tracks the GET /api/persons
// response-shape audit (PersonSummaryDTO leaks `notes`). // response-shape audit (PersonSummaryDTO leaks `notes`).
// ───────────────────────────────────────────────────────────── // ─────────────────────────────────────────────────────────────
// Tiptap's suggestion plugin requires an `items()` callback to keep items: async ({ query }: { query: string }) => {
// the dropdown alive, but the actual fetch is owned by `runSearch` if (!query) return [];
// below — routed through the dropdown's search input via the try {
// debounced `onSearch` channel. Returning `[]` here keeps Tiptap const res = await fetch(`/api/persons?q=${encodeURIComponent(query)}`);
// happy without firing a duplicate per-keystroke fetch. if (!res.ok) return [];
// Markus #5616 / Felix / Nora / Sara on PR #629. return ((await res.json()) as Person[]).slice(0, 5);
items: async () => [], } catch {
return [];
}
},
// AC-1 fix: insert the typed query as displayName, not person.displayName. // AC-1 fix: insert the typed query as displayName, not person.displayName.
command({ editor: ed, range, props }) { command({ editor: ed, range, props }) {
const p = props as unknown as { personId: string; displayName: string }; const p = props as unknown as { personId: string; displayName: string };
@@ -173,6 +165,7 @@ onMount(() => {
.run(); .run();
}, },
render() { render() {
let component: object | null = null;
let exports: DropdownExports | null = null; let exports: DropdownExports | null = null;
// Tiptap's SuggestionProps types `command` against the default // Tiptap's SuggestionProps types `command` against the default
@@ -185,84 +178,25 @@ onMount(() => {
clientRect?: (() => DOMRect | null) | null; clientRect?: (() => DOMRect | null) | null;
}; };
// Request-token guard: every onSearch invocation bumps `requestId`;
// runSearch captures the id active when its fetch starts and discards
// the response if a newer onSearch has fired since. Without this, a
// late response can repopulate the dropdown after the user cleared
// the search input. Sara on PR #629.
let requestId = 0;
const runSearch = async (query: string) => {
const id = requestId;
try {
// Defensive client-side cap — server-side enforcement is tracked
// separately. Markus on PR #629.
const res = await fetch(
`/api/persons?q=${encodeURIComponent(query)}&limit=${SEARCH_RESULT_LIMIT}`
);
if (id !== requestId) return;
if (!res.ok) {
dropdownState.items = [];
return;
}
const data = (await res.json()) as Person[];
if (id !== requestId) return;
dropdownState.items = data.slice(0, SEARCH_RESULT_LIMIT);
} catch {
if (id !== requestId) return;
dropdownState.items = [];
}
};
const debouncedSearch = debounce(runSearch, SEARCH_DEBOUNCE_MS);
cancelPendingSearch = () => debouncedSearch.cancel();
const onSearch = (query: string) => {
requestId++;
if (query.trim() === '') {
debouncedSearch.cancel();
dropdownState.items = [];
return;
}
debouncedSearch(query);
};
const updateState = (renderProps: LooseRenderProps) => { const updateState = (renderProps: LooseRenderProps) => {
// Clip once here so both the inserted displayName and the dropdownState.items = renderProps.items as Person[];
// dropdown's editor-mirror see the same value. The dropdown
// already clips the mirror (Nora #1 CWE-400), but without
// clipping at the command boundary an unclipped query would
// still flow through as the inserted displayName — visible
// UI divergence between "what I searched" and "what was
// inserted". Felix #3 on PR #629.
const clippedQuery = renderProps.query.slice(0, MAX_QUERY_LENGTH);
// AC-1: pass typed query as displayName, not person.displayName // AC-1: pass typed query as displayName, not person.displayName
dropdownState.command = (item: Person) => dropdownState.command = (item: Person) =>
renderProps.command({ renderProps.command({
personId: item.id, personId: item.id,
displayName: clippedQuery displayName: renderProps.query
}); });
dropdownState.clientRect = renderProps.clientRect ?? null; dropdownState.clientRect = renderProps.clientRect ?? null;
dropdownState.editorQuery = clippedQuery;
}; };
return { return {
onStart(renderProps) { onStart(renderProps) {
const loose = renderProps as unknown as LooseRenderProps; updateState(renderProps as unknown as LooseRenderProps);
updateState(loose);
// MentionDropdown reads `editorQuery` off the shared state
// proxy via its `editorQuery` prop binding below — this is
// the same pattern as `model.items`. We do not pass it as a
// separate prop because Svelte 5's mount() does not expose
// settable prop accessors, so we route through the proxy.
const mounted = mount(MentionDropdown, { const mounted = mount(MentionDropdown, {
target: document.body, target: document.body,
props: { props: { model: dropdownState }
model: dropdownState,
get editorQuery() {
return dropdownState.editorQuery;
},
onSearch
}
}); });
mountedDropdown = mounted as object; component = mounted as object;
exports = mounted as unknown as DropdownExports; exports = mounted as unknown as DropdownExports;
}, },
onUpdate(renderProps) { onUpdate(renderProps) {
@@ -274,16 +208,9 @@ onMount(() => {
return exports?.onKeyDown(event) ?? false; return exports?.onKeyDown(event) ?? false;
}, },
onExit() { onExit() {
// Cancel any pending debounce so a closed dropdown's trailing if (component) {
// runSearch cannot fire against the *next* dropdown's state. unmount(component);
// The hoisted `cancelPendingSearch` would be overwritten by component = null;
// the next render()'s onStart before the trailing call fires,
// so we cancel locally via the closure-scoped debouncedSearch.
// Felix #1 on PR #629.
debouncedSearch.cancel();
if (mountedDropdown) {
unmount(mountedDropdown);
mountedDropdown = null;
exports = null; exports = null;
} }
} }
@@ -326,15 +253,7 @@ onMount(() => {
}); });
onDestroy(() => { onDestroy(() => {
cancelPendingSearch?.();
editor?.destroy(); editor?.destroy();
// Tiptap suggestion onExit usually unmounts the dropdown, but if the host
// component is destroyed while a suggestion is active the dropdown can
// outlive the editor — clean it up explicitly.
if (mountedDropdown) {
unmount(mountedDropdown);
mountedDropdown = null;
}
}); });
// Keep the data-placeholder attribute in sync with actual emptiness so the // Keep the data-placeholder attribute in sync with actual emptiness so the

View File

@@ -8,45 +8,29 @@
import { describe, it, expect, vi, afterEach } from 'vitest'; import { describe, it, expect, vi, afterEach } from 'vitest';
import { cleanup, render } from 'vitest-browser-svelte'; import { cleanup, render } from 'vitest-browser-svelte';
import { page, userEvent } from 'vitest/browser'; import { page, userEvent } from 'vitest/browser';
import { tick } from 'svelte'; import PersonMentionEditorHost from './PersonMentionEditor.test-host.svelte';
import PersonMentionEditorHost from './PersonMentionEditor.test-fixture.svelte';
import type { components } from '$lib/generated/api'; import type { components } from '$lib/generated/api';
import { m } from '$lib/paraglide/messages.js'; import { m } from '$lib/paraglide/messages.js';
// Single source of truth for the debounce window — imported from the shared
// module so the test cannot drift from production. Sara on PR #629 round 3.
import { SEARCH_DEBOUNCE_MS } from './mentionConstants';
type Person = components['schemas']['Person']; type Person = components['schemas']['Person'];
type PersonMention = components['schemas']['PersonMention']; type PersonMention = components['schemas']['PersonMention'];
/**
* Headroom above SEARCH_DEBOUNCE_MS for the debounce-window wait
* assertions in this file. 350 ms is calibrated against CI-runner jitter
* we observed pre-#629; dropping it below ~200 ms reintroduces flake.
* See PR #629 round-2 review comment #10935 (Sara).
*/
const POST_DEBOUNCE_SLACK_MS = 350;
const AUGUSTE: Person = { const AUGUSTE: Person = {
id: 'p-aug', id: 'p-aug',
firstName: 'Auguste', firstName: 'Auguste',
lastName: 'Raddatz', lastName: 'Raddatz',
displayName: 'Auguste Raddatz', displayName: 'Auguste Raddatz',
personType: 'PERSON',
familyMember: false,
birthYear: 1882, birthYear: 1882,
deathYear: 1944 deathYear: 1944
}; } as unknown as Person;
const ANNA: Person = { const ANNA: Person = {
id: 'p-anna', id: 'p-anna',
firstName: 'Anna', firstName: 'Anna',
lastName: 'Schmidt', lastName: 'Schmidt',
displayName: 'Anna Schmidt', displayName: 'Anna Schmidt',
personType: 'PERSON',
familyMember: false,
birthYear: 1860 birthYear: 1860
}; } as unknown as Person;
function mockFetchWithPersons(persons: Person[] = [AUGUSTE, ANNA]) { function mockFetchWithPersons(persons: Person[] = [AUGUSTE, ANNA]) {
vi.stubGlobal( vi.stubGlobal(
@@ -141,20 +125,6 @@ describe('PersonMentionEditor — typeahead', () => {
}); });
}); });
it('appends &limit=5 to the fetch URL (defensive client-side cap, Markus on PR #629)', async () => {
const fetchMock = vi
.fn()
.mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) });
vi.stubGlobal('fetch', fetchMock);
renderHost();
await userEvent.type(page.getByRole('textbox'), '@Aug');
await vi.waitFor(() => {
expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('limit=5'));
});
});
it('shows life dates next to the name in the dropdown', async () => { it('shows life dates next to the name in the dropdown', async () => {
mockFetchWithPersons(); mockFetchWithPersons();
renderHost(); renderHost();
@@ -172,15 +142,8 @@ describe('PersonMentionEditor — typeahead', () => {
await userEvent.type(page.getByRole('textbox'), '@xyz'); await userEvent.type(page.getByRole('textbox'), '@xyz');
// The visible empty-state <p> (text-ink-3) shows the copy. The persistent await vi.waitFor(async () => {
// sr-only aria-live region also contains the same copy, so we scope to the await expect.element(page.getByText('Keine Personen gefunden')).toBeInTheDocument();
// visible element to avoid a multi-match resolution in expect.element.
await vi.waitFor(() => {
const visibleEmptyP = document.querySelector(
'[role="listbox"] p.text-ink-3'
) as HTMLElement | null;
expect(visibleEmptyP).not.toBeNull();
expect(visibleEmptyP!.textContent ?? '').toContain('Keine Personen gefunden');
}); });
}); });
@@ -198,254 +161,6 @@ describe('PersonMentionEditor — typeahead', () => {
}); });
}); });
// ─── AC-2/3: search input drives the person fetch (debounced) ───────────────
describe('PersonMentionEditor — AC-2/3: search input drives fetch', () => {
it('editing the search input fires a debounced fetch with the new query', async () => {
const fetchMock = vi
.fn()
.mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) });
vi.stubGlobal('fetch', fetchMock);
renderHost();
// Open the dropdown so the search input is reachable.
await userEvent.type(page.getByRole('textbox'), '@');
await vi.waitFor(async () => {
await expect.element(page.getByRole('searchbox')).toBeVisible();
});
const fetchesBeforeSearch = fetchMock.mock.calls.length;
// `fill` simulates a single input event with the final value — sidesteps
// per-keystroke timing of userEvent.type so the test can deterministically
// assert that one input event collapses into one debounced fetch.
await page.getByRole('searchbox').fill('Walter');
await vi.waitFor(
() => {
expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('q=Walter'));
},
{ timeout: 1000 }
);
const fetchesAfterSearch = fetchMock.mock.calls.length - fetchesBeforeSearch;
expect(fetchesAfterSearch).toBe(1);
});
it('fires exactly one /api/persons fetch when the user searches for Walter (regression guard)', async () => {
// Regression guard: a previous version of PersonMentionEditor had a
// duplicated `items()` callback in the Tiptap suggestion config that
// fetched per-keystroke in addition to the debounced search-input fetch
// (Markus & Felix round-1). To catch that regression, we must NOT
// subtract any baseline — every fetch from render onwards counts.
// Sara on PR #629 round 3.
const fetchMock = vi
.fn()
.mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) });
vi.stubGlobal('fetch', fetchMock);
renderHost();
// Open the dropdown, then drive the search input via fill() — sidesteps
// per-keystroke timing of userEvent.type that Sara flagged round 2.
await userEvent.type(page.getByRole('textbox'), '@');
await vi.waitFor(async () => {
await expect.element(page.getByRole('searchbox')).toBeVisible();
});
await page.getByRole('searchbox').fill('Walter');
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
// No baseline subtraction — count ALL /api/persons fetches since render.
// If the legacy per-keystroke items() callback returns, typing `@` alone
// would already produce one fetch and `fill('Walter')` another, breaking
// this assertion.
const personsFetches = fetchMock.mock.calls.filter(
([url]) => typeof url === 'string' && url.startsWith('/api/persons')
);
expect(personsFetches.length).toBe(1);
});
it('clearing the search input clears the list without firing a fetch', async () => {
const fetchMock = vi
.fn()
.mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) });
vi.stubGlobal('fetch', fetchMock);
renderHost();
await userEvent.type(page.getByRole('textbox'), '@Aug');
await vi.waitFor(async () => {
await expect.element(page.getByText('Auguste Raddatz')).toBeInTheDocument();
});
const fetchesBeforeClear = fetchMock.mock.calls.length;
await userEvent.clear(page.getByRole('searchbox'));
// Negative assertion: wait past the debounce window to confirm no
// trailing fetch was scheduled. Removing this wait would mask a
// re-introduction of the keystroke-driven items() fetch.
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
expect(fetchMock.mock.calls.length).toBe(fetchesBeforeClear);
await expect.element(page.getByText('Auguste Raddatz')).not.toBeInTheDocument();
});
});
// ─── Whitespace-only query (Elicit AC-4 ambiguity on PR #629) ───────────────
describe('PersonMentionEditor — whitespace-only query', () => {
it('keeps the "Namen eingeben…" prompt and fires no fetch when @ is followed only by spaces', async () => {
const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([]) });
vi.stubGlobal('fetch', fetchMock);
renderHost();
await userEvent.type(page.getByRole('textbox'), '@ ');
await vi.waitFor(async () => {
await expect.element(page.getByRole('searchbox')).toBeVisible();
});
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
// Scope to the visible empty-state <p> (text-ink-3) — the persistent
// sr-only aria-live region above contains the same copy.
const visibleEmptyP = document.querySelector(
'[role="listbox"] p.text-ink-3'
) as HTMLElement | null;
expect(visibleEmptyP).not.toBeNull();
expect(visibleEmptyP!.textContent ?? '').toContain(m.person_mention_search_prompt());
expect(fetchMock).not.toHaveBeenCalled();
});
});
// ─── Stale-response race (Sara on PR #629) ───────────────────────────────────
describe('PersonMentionEditor — stale-response race', () => {
it('discards a stale response that resolves after the search has been cleared', async () => {
let resolveFetch!: (v: { ok: boolean; json: () => Promise<Person[]> }) => void;
const pendingResponse = new Promise<{ ok: boolean; json: () => Promise<Person[]> }>((r) => {
resolveFetch = r;
});
const fetchMock = vi.fn().mockReturnValue(pendingResponse);
vi.stubGlobal('fetch', fetchMock);
renderHost();
// Open the dropdown and let the debounce fire so a fetch is in flight.
await userEvent.type(page.getByRole('textbox'), '@Aug');
await vi.waitFor(() => {
expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('/api/persons?q=Aug'));
});
// Clear the search input *before* the fetch resolves.
await userEvent.clear(page.getByRole('searchbox'));
await expect.element(page.getByRole('searchbox')).toHaveValue('');
// The stale fetch now resolves with persons. The dropdown must stay empty.
resolveFetch({ ok: true, json: () => Promise.resolve([AUGUSTE]) });
// Flush pending Svelte reactivity so any (non-)update from the stale
// fetch resolution has landed before we assert. expect.element already
// polls, so no fixed-timeout fallback is needed. Sara on PR #629 round 4.
await tick();
await expect.element(page.getByText('Auguste Raddatz')).not.toBeInTheDocument();
});
});
// ─── Server failure characterization (Sara #2 on PR #629) ───────────────────
describe('PersonMentionEditor — server failure', () => {
it('on 500 response keeps the dropdown open with the empty-state copy (silent failure pinned; distinct error UX tracked separately)', async () => {
const fetchMock = vi
.fn()
.mockResolvedValue({ ok: false, status: 500, json: vi.fn().mockResolvedValue({}) });
vi.stubGlobal('fetch', fetchMock);
renderHost();
await userEvent.type(page.getByRole('textbox'), '@Aug');
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
// Pins current silent-failure behaviour. The day someone implements a
// distinct error UX (toast / "Suche fehlgeschlagen" copy), this test
// goes red and forces them to update the assertion. Scope to the
// visible <p> (text-ink-3) — the persistent sr-only live region
// above contains the same copy.
const visibleEmptyP = document.querySelector(
'[role="listbox"] p.text-ink-3'
) as HTMLElement | null;
expect(visibleEmptyP).not.toBeNull();
expect(visibleEmptyP!.textContent ?? '').toContain(m.person_mention_popup_empty());
});
it('on a fetch reject (network failure) keeps the dropdown open with the empty-state copy', async () => {
const fetchMock = vi.fn().mockRejectedValue(new TypeError('NetworkError'));
vi.stubGlobal('fetch', fetchMock);
renderHost();
await userEvent.type(page.getByRole('textbox'), '@Aug');
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
const visibleEmptyP = document.querySelector(
'[role="listbox"] p.text-ink-3'
) as HTMLElement | null;
expect(visibleEmptyP).not.toBeNull();
expect(visibleEmptyP!.textContent ?? '').toContain(m.person_mention_popup_empty());
});
});
// ─── onExit cancels pending debounce (Felix #1 on PR #629) ───────────────────
describe('PersonMentionEditor — onExit cancels pending debounce', () => {
it('cancels the pending debounced fetch when Escape closes the dropdown before the debounce fires', async () => {
const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([]) });
vi.stubGlobal('fetch', fetchMock);
renderHost();
// Open the dropdown by typing @ + a query in the editor.
await userEvent.type(page.getByRole('textbox'), '@A');
await vi.waitFor(async () => {
await expect.element(page.getByRole('searchbox')).toBeVisible();
});
// Wait for any in-flight fetch from opening the dropdown to settle.
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
const fetchesBeforeEscape = fetchMock.mock.calls.length;
// Trigger a new debounced search (queues runSearch after 150 ms), then
// immediately Escape *while focus is back in the editor* so Tiptap's
// suggestion-plugin Escape handler fires onExit before the debounce.
// Without onExit cancelling the pending debounce, runSearch executes
// against the now-unmounted dropdown's state.
await page.getByRole('searchbox').fill('Walter');
// Focus the editor so the Escape lands on Tiptap's suggestion handler.
(page.getByRole('textbox').element() as HTMLElement).focus();
await userEvent.keyboard('{Escape}');
// Wait past the debounce window. If onExit did not cancel the pending
// debounce, a fetch with q=Walter would still fire here.
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
const newFetches = fetchMock.mock.calls.slice(fetchesBeforeEscape);
const walterFetches = newFetches.filter(
([url]) => typeof url === 'string' && url.includes('q=Walter')
);
expect(walterFetches.length).toBe(0);
});
});
// ─── AC-1: search input prefilled with text typed after @ ───────────────────
describe('PersonMentionEditor — AC-1: search input prefill', () => {
it('prefills the dropdown search input with the text typed after @', async () => {
mockFetchEmpty();
renderHost();
await userEvent.type(page.getByRole('textbox'), '@WdG');
await vi.waitFor(async () => {
await expect.element(page.getByRole('searchbox')).toHaveValue('WdG');
});
});
});
// ─── AC-1: typed text becomes displayName, not DB name ─────────────────────── // ─── AC-1: typed text becomes displayName, not DB name ───────────────────────
describe('PersonMentionEditor — AC-1: typed text as displayName', () => { describe('PersonMentionEditor — AC-1: typed text as displayName', () => {
@@ -514,39 +229,6 @@ describe('PersonMentionEditor — AC-1: typed text as displayName', () => {
}); });
}); });
it('clips the inserted displayName to MAX_QUERY_LENGTH=100 chars (Felix #3 on PR #629)', async () => {
// CWE-400 amplification: the dropdown clips its search input + mirror at
// 100 chars (Nora #1), but the host editor was passing the unclipped
// renderProps.query straight through to displayName — so a 105-char
// @-suffix in the editor could insert a 105-char displayName into the
// sidecar even though the dropdown only searched the first 100.
mockFetchWithPersons();
const host = renderHost();
// Type @ + 105 'A' chars in the contenteditable. The renderProps.query
// fed into the command callback derives from the editor text after `@`,
// not the dropdown's searchbox — so we must drive the editor.
await userEvent.type(page.getByRole('textbox'), '@' + 'A'.repeat(105));
// The mocked /api/persons returns AUGUSTE for any query — wait for it.
await vi.waitFor(async () => {
await expect.element(page.getByRole('option', { name: /Auguste Raddatz/ })).toBeVisible();
});
const option = (await page
.getByRole('option', { name: /Auguste Raddatz/ })
.element()) as HTMLElement;
option.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, cancelable: true }));
await vi.waitFor(() => {
expect(host.snapshot.mentionedPersons).toHaveLength(1);
// Tight assertion: input is 105 chars, cap is exactly 100. Using
// `toHaveLength(100)` discriminates "clip works" from "clip works
// AND nothing weakened it to e.g. 95". Sara on PR #629 round 4.
expect(host.snapshot.mentionedPersons[0].displayName).toHaveLength(100);
});
});
it('does not duplicate the sidecar entry when the same person is selected twice', async () => { it('does not duplicate the sidecar entry when the same person is selected twice', async () => {
mockFetchWithPersons(); mockFetchWithPersons();
const host = renderHost({ const host = renderHost({

View File

@@ -1,10 +0,0 @@
/** Shared knobs for the @mention typeahead. Single source of truth for
* the dropdown component and the host editor — keeps the layered length
* cap and the debounce window consistent across both files. */
export const MAX_QUERY_LENGTH = 100;
export const SEARCH_DEBOUNCE_MS = 150;
/** Defensive client-side cap on the result list. Single consumer today
* (PersonMentionEditor), kept here for symmetry with the other limit
* knobs so the @mention configuration lives in one place. Felix #1 on
* PR #629 round 4. */
export const SEARCH_RESULT_LIMIT = 5;

View File

@@ -1,7 +1,7 @@
import { describe, it, expect, afterEach } from 'vitest'; import { describe, it, expect, afterEach } from 'vitest';
import { cleanup, render } from 'vitest-browser-svelte'; import { cleanup, render } from 'vitest-browser-svelte';
import { page, userEvent } from 'vitest/browser'; import { page, userEvent } from 'vitest/browser';
import TestHost from './confirm.test-fixture.svelte'; import TestHost from './confirm.test-host.svelte';
import type { ConfirmService } from './confirm.svelte.js'; import type { ConfirmService } from './confirm.svelte.js';
afterEach(cleanup); afterEach(cleanup);

View File

@@ -1,25 +1,12 @@
/** /**
* Returns a debounced version of fn that delays invocation until after * Returns a debounced version of fn that delays invocation until after
* `delay` ms have elapsed since the last call. The returned function * `delay` ms have elapsed since the last call.
* exposes a `cancel()` method that DROPS (does not flush) the pending
* trailing invocation — essential when the host context (a destroyed
* component, an unmounted editor) shouldn't fire the trailing call.
*/ */
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any
export function debounce<T extends (...args: any[]) => void>( export function debounce<T extends (...args: any[]) => void>(fn: T, delay: number): T {
fn: T, let timer: ReturnType<typeof setTimeout>;
delay: number return ((...args: Parameters<T>) => {
): T & { cancel: () => void } {
let timer: ReturnType<typeof setTimeout> | undefined;
const wrapped = ((...args: Parameters<T>) => {
if (timer !== undefined) clearTimeout(timer);
timer = setTimeout(() => fn(...args), delay);
}) as T & { cancel: () => void };
wrapped.cancel = () => {
if (timer !== undefined) {
clearTimeout(timer); clearTimeout(timer);
timer = undefined; timer = setTimeout(() => fn(...args), delay);
} }) as T;
};
return wrapped;
} }

View File

@@ -1,6 +1,6 @@
import { error } from '@sveltejs/kit'; import { error } from '@sveltejs/kit';
import { env } from '$env/dynamic/private'; import { env } from '$env/dynamic/private';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
import type { components } from '$lib/generated/api'; import type { components } from '$lib/generated/api';
@@ -34,16 +34,16 @@ export async function load({ fetch, locals }) {
]); ]);
if (!usersResult.response.ok) { if (!usersResult.response.ok) {
throw error(usersResult.response.status, getErrorMessage(extractErrorCode(usersResult.error))); const code = (usersResult.error as unknown as { code?: string })?.code;
throw error(usersResult.response.status, getErrorMessage(code));
} }
if (!groupsResult.response.ok) { if (!groupsResult.response.ok) {
throw error( const code = (groupsResult.error as unknown as { code?: string })?.code;
groupsResult.response.status, throw error(groupsResult.response.status, getErrorMessage(code));
getErrorMessage(extractErrorCode(groupsResult.error))
);
} }
if (!tagsResult.response.ok) { if (!tagsResult.response.ok) {
throw error(tagsResult.response.status, getErrorMessage(extractErrorCode(tagsResult.error))); const code = (tagsResult.error as unknown as { code?: string })?.code;
throw error(tagsResult.response.status, getErrorMessage(code));
} }
let inviteCount = 0; let inviteCount = 0;

View File

@@ -1,6 +1,6 @@
import { error, fail, redirect } from '@sveltejs/kit'; import { error, fail, redirect } from '@sveltejs/kit';
import type { PageServerLoad, Actions } from './$types'; import type { PageServerLoad, Actions } from './$types';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
export const load: PageServerLoad = async ({ params, parent }) => { export const load: PageServerLoad = async ({ params, parent }) => {
@@ -24,9 +24,8 @@ export const actions: Actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
error: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { error: getErrorMessage(code) });
});
} }
return { success: true }; return { success: true };
@@ -39,9 +38,8 @@ export const actions: Actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
error: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { error: getErrorMessage(code) });
});
} }
throw redirect(303, '/admin/groups'); throw redirect(303, '/admin/groups');

View File

@@ -7,8 +7,7 @@ const mockApi = {
}; };
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({
createApiClient: () => mockApi, createApiClient: () => mockApi
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
})); }));
beforeEach(() => vi.clearAllMocks()); beforeEach(() => vi.clearAllMocks());

View File

@@ -1,10 +1,7 @@
import { describe, expect, it, vi, beforeEach } from 'vitest'; import { describe, expect, it, vi, beforeEach } from 'vitest';
import { load } from './+layout.server'; import { load } from './+layout.server';
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({ createApiClient: vi.fn() }));
createApiClient: vi.fn(),
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
}));
import { createApiClient } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';

View File

@@ -1,6 +1,6 @@
import { fail, redirect } from '@sveltejs/kit'; import { fail, redirect } from '@sveltejs/kit';
import type { Actions } from './$types'; import type { Actions } from './$types';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
export const actions: Actions = { export const actions: Actions = {
@@ -16,9 +16,8 @@ export const actions: Actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
error: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { error: getErrorMessage(code) });
});
} }
throw redirect(303, '/admin/groups'); throw redirect(303, '/admin/groups');

View File

@@ -1,5 +1,5 @@
import { fail } from '@sveltejs/kit'; import { fail } from '@sveltejs/kit';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
import type { Actions, PageServerLoad } from './$types'; import type { Actions, PageServerLoad } from './$types';
import type { components } from '$lib/generated/api'; import type { components } from '$lib/generated/api';
@@ -25,7 +25,8 @@ export const load: PageServerLoad = async ({ url, fetch }) => {
let invites: InviteListItem[] = []; let invites: InviteListItem[] = [];
let loadError: string | null = null; let loadError: string | null = null;
if (!invitesResult.response.ok) { if (!invitesResult.response.ok) {
loadError = extractErrorCode(invitesResult.error) ?? 'INTERNAL_ERROR'; const code = (invitesResult.error as unknown as { code?: string })?.code;
loadError = code ?? 'INTERNAL_ERROR';
} else { } else {
invites = (invitesResult.data ?? []) as InviteListItem[]; invites = (invitesResult.data ?? []) as InviteListItem[];
} }
@@ -33,7 +34,8 @@ export const load: PageServerLoad = async ({ url, fetch }) => {
let groups: UserGroup[] = []; let groups: UserGroup[] = [];
let groupsLoadError: string | null = null; let groupsLoadError: string | null = null;
if (!groupsResult.response.ok) { if (!groupsResult.response.ok) {
groupsLoadError = extractErrorCode(groupsResult.error) ?? 'INTERNAL_ERROR'; const code = (groupsResult.error as unknown as { code?: string })?.code;
groupsLoadError = code ?? 'INTERNAL_ERROR';
} else { } else {
const raw = groupsResult.data ?? []; const raw = groupsResult.data ?? [];
groups = [...raw].sort((a, b) => a.name.localeCompare(b.name)); groups = [...raw].sort((a, b) => a.name.localeCompare(b.name));
@@ -60,9 +62,8 @@ export const actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
createError: extractErrorCode(result.error) ?? 'INTERNAL_ERROR' return fail(result.response.status, { createError: code ?? 'INTERNAL_ERROR' });
});
} }
return { created: result.data! as InviteListItem }; return { created: result.data! as InviteListItem };
@@ -77,9 +78,8 @@ export const actions = {
const result = await api.DELETE('/api/invites/{id}', { params: { path: { id } } }); const result = await api.DELETE('/api/invites/{id}', { params: { path: { id } } });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
revokeError: extractErrorCode(result.error) ?? 'INTERNAL_ERROR' return fail(result.response.status, { revokeError: code ?? 'INTERNAL_ERROR' });
});
} }
return { revoked: id }; return { revoked: id };

View File

@@ -1,10 +1,7 @@
import { describe, expect, it, vi, beforeEach } from 'vitest'; import { describe, expect, it, vi, beforeEach } from 'vitest';
import { load } from './+layout.server'; import { load } from './+layout.server';
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({ createApiClient: vi.fn() }));
createApiClient: vi.fn(),
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
}));
import { createApiClient } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';

View File

@@ -1,6 +1,6 @@
import { error } from '@sveltejs/kit'; import { error } from '@sveltejs/kit';
import type { PageServerLoad } from './$types'; import type { PageServerLoad } from './$types';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
export const load: PageServerLoad = async ({ fetch }) => { export const load: PageServerLoad = async ({ fetch }) => {
@@ -8,7 +8,8 @@ export const load: PageServerLoad = async ({ fetch }) => {
const result = await api.GET('/api/ocr/training-info'); const result = await api.GET('/api/ocr/training-info');
if (!result.response.ok) { if (!result.response.ok) {
throw error(result.response.status, getErrorMessage(extractErrorCode(result.error))); const code = (result.error as unknown as { code?: string })?.code;
throw error(result.response.status, getErrorMessage(code));
} }
return { trainingInfo: result.data! }; return { trainingInfo: result.data! };

View File

@@ -1,6 +1,6 @@
import { error } from '@sveltejs/kit'; import { error } from '@sveltejs/kit';
import type { PageServerLoad } from './$types'; import type { PageServerLoad } from './$types';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
export const load: PageServerLoad = async ({ params, fetch }) => { export const load: PageServerLoad = async ({ params, fetch }) => {
@@ -10,7 +10,8 @@ export const load: PageServerLoad = async ({ params, fetch }) => {
}); });
if (!result.response.ok) { if (!result.response.ok) {
throw error(result.response.status, getErrorMessage(extractErrorCode(result.error))); const code = (result.error as unknown as { code?: string })?.code;
throw error(result.response.status, getErrorMessage(code));
} }
return { history: result.data!, personId: params.personId }; return { history: result.data!, personId: params.personId };

View File

@@ -3,10 +3,7 @@ import { load } from './+page.server';
const mockApi = { GET: vi.fn() }; const mockApi = { GET: vi.fn() };
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({ createApiClient: () => mockApi }));
createApiClient: () => mockApi,
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
}));
beforeEach(() => vi.clearAllMocks()); beforeEach(() => vi.clearAllMocks());

View File

@@ -1,6 +1,6 @@
import { error } from '@sveltejs/kit'; import { error } from '@sveltejs/kit';
import type { PageServerLoad } from './$types'; import type { PageServerLoad } from './$types';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
export const load: PageServerLoad = async ({ fetch }) => { export const load: PageServerLoad = async ({ fetch }) => {
@@ -8,7 +8,8 @@ export const load: PageServerLoad = async ({ fetch }) => {
const result = await api.GET('/api/ocr/training-info/global'); const result = await api.GET('/api/ocr/training-info/global');
if (!result.response.ok) { if (!result.response.ok) {
throw error(result.response.status, getErrorMessage(extractErrorCode(result.error))); const code = (result.error as unknown as { code?: string })?.code;
throw error(result.response.status, getErrorMessage(code));
} }
return { history: result.data! }; return { history: result.data! };

View File

@@ -3,10 +3,7 @@ import { load } from './+page.server';
const mockApi = { GET: vi.fn() }; const mockApi = { GET: vi.fn() };
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({ createApiClient: () => mockApi }));
createApiClient: () => mockApi,
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
}));
beforeEach(() => vi.clearAllMocks()); beforeEach(() => vi.clearAllMocks());

View File

@@ -3,10 +3,7 @@ import { load } from './+page.server';
const mockApi = { GET: vi.fn() }; const mockApi = { GET: vi.fn() };
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({ createApiClient: () => mockApi }));
createApiClient: () => mockApi,
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
}));
beforeEach(() => vi.clearAllMocks()); beforeEach(() => vi.clearAllMocks());

View File

@@ -1,6 +1,6 @@
import { error, fail, redirect } from '@sveltejs/kit'; import { error, fail, redirect } from '@sveltejs/kit';
import type { PageServerLoad, Actions } from './$types'; import type { PageServerLoad, Actions } from './$types';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
export const load: PageServerLoad = async ({ params, parent, url }) => { export const load: PageServerLoad = async ({ params, parent, url }) => {
@@ -25,9 +25,8 @@ export const actions: Actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
error: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { error: getErrorMessage(code) });
});
} }
return { success: true }; return { success: true };
@@ -44,9 +43,8 @@ export const actions: Actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
error: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { error: getErrorMessage(code) });
});
} }
throw redirect(303, `/admin/tags/${result.data!.id}?merged=1`); throw redirect(303, `/admin/tags/${result.data!.id}?merged=1`);
@@ -67,9 +65,8 @@ export const actions: Actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
error: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { error: getErrorMessage(code) });
});
} }
throw redirect(303, '/admin/tags'); throw redirect(303, '/admin/tags');

View File

@@ -8,8 +8,7 @@ const mockApi = {
}; };
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({
createApiClient: () => mockApi, createApiClient: () => mockApi
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
})); }));
beforeEach(() => vi.clearAllMocks()); beforeEach(() => vi.clearAllMocks());

View File

@@ -1,10 +1,7 @@
import { describe, expect, it, vi, beforeEach } from 'vitest'; import { describe, expect, it, vi, beforeEach } from 'vitest';
import { load } from './+layout.server'; import { load } from './+layout.server';
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({ createApiClient: vi.fn() }));
createApiClient: vi.fn(),
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
}));
import { createApiClient } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';

View File

@@ -1,6 +1,6 @@
import { error, fail, redirect } from '@sveltejs/kit'; import { error, fail, redirect } from '@sveltejs/kit';
import type { PageServerLoad, Actions } from './$types'; import type { PageServerLoad, Actions } from './$types';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
import type { components } from '$lib/generated/api'; import type { components } from '$lib/generated/api';
@@ -55,9 +55,8 @@ export const actions: Actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
error: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { error: getErrorMessage(code) });
});
} }
return { success: true }; return { success: true };
@@ -70,9 +69,8 @@ export const actions: Actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
error: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { error: getErrorMessage(code) });
});
} }
throw redirect(303, '/admin/users'); throw redirect(303, '/admin/users');

View File

@@ -4,10 +4,7 @@ vi.mock('$env/dynamic/private', () => ({
env: { API_INTERNAL_URL: 'http://localhost:8080' } env: { API_INTERNAL_URL: 'http://localhost:8080' }
})); }));
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({ createApiClient: vi.fn() }));
createApiClient: vi.fn(),
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
}));
import { load, actions } from './+page.server'; import { load, actions } from './+page.server';
import { createApiClient } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';

View File

@@ -1,10 +1,7 @@
import { describe, expect, it, vi, beforeEach } from 'vitest'; import { describe, expect, it, vi, beforeEach } from 'vitest';
import { load } from './+layout.server'; import { load } from './+layout.server';
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({ createApiClient: vi.fn() }));
createApiClient: vi.fn(),
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
}));
import { createApiClient } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';

View File

@@ -1,6 +1,6 @@
import { error, fail, redirect } from '@sveltejs/kit'; import { error, fail, redirect } from '@sveltejs/kit';
import type { PageServerLoad, Actions } from './$types'; import type { PageServerLoad, Actions } from './$types';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
export const load: PageServerLoad = async ({ fetch, locals }) => { export const load: PageServerLoad = async ({ fetch, locals }) => {
@@ -35,9 +35,8 @@ export const actions: Actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
error: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { error: getErrorMessage(code) });
});
} }
throw redirect(303, '/admin/users'); throw redirect(303, '/admin/users');

View File

@@ -8,8 +8,7 @@ const mockApi = {
}; };
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({
createApiClient: () => mockApi, createApiClient: () => mockApi
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
})); }));
function buildUrl(search = ''): URL { function buildUrl(search = ''): URL {

View File

@@ -1,6 +1,6 @@
import { error } from '@sveltejs/kit'; import { error } from '@sveltejs/kit';
import type { components } from '$lib/generated/api'; import type { components } from '$lib/generated/api';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
export async function load({ url, fetch, locals }) { export async function load({ url, fetch, locals }) {
@@ -39,7 +39,8 @@ export async function load({ url, fetch, locals }) {
}) })
.then((result) => { .then((result) => {
if (!result.response.ok) { if (!result.response.ok) {
throw error(result.response.status, getErrorMessage(extractErrorCode(result.error))); const code = (result.error as unknown as { code?: string })?.code;
throw error(result.response.status, getErrorMessage(code));
} }
documents = result.data ?? []; documents = result.data ?? [];
}) })
@@ -48,7 +49,8 @@ export async function load({ url, fetch, locals }) {
requests.push( requests.push(
api.GET('/api/persons/{id}', { params: { path: { id: senderId } } }).then((result) => { api.GET('/api/persons/{id}', { params: { path: { id: senderId } } }).then((result) => {
if (!result.response.ok) { if (!result.response.ok) {
throw error(result.response.status, getErrorMessage(extractErrorCode(result.error))); const code = (result.error as unknown as { code?: string })?.code;
throw error(result.response.status, getErrorMessage(code));
} }
const p = result.data as { displayName: string } | undefined; const p = result.data as { displayName: string } | undefined;
if (p) senderName = p.displayName; if (p) senderName = p.displayName;
@@ -60,7 +62,8 @@ export async function load({ url, fetch, locals }) {
requests.push( requests.push(
api.GET('/api/persons/{id}', { params: { path: { id: receiverId } } }).then((result) => { api.GET('/api/persons/{id}', { params: { path: { id: receiverId } } }).then((result) => {
if (!result.response.ok) { if (!result.response.ok) {
throw error(result.response.status, getErrorMessage(extractErrorCode(result.error))); const code = (result.error as unknown as { code?: string })?.code;
throw error(result.response.status, getErrorMessage(code));
} }
const p = result.data as { displayName: string } | undefined; const p = result.data as { displayName: string } | undefined;
if (p) receiverName = p.displayName; if (p) receiverName = p.displayName;

View File

@@ -1,10 +1,7 @@
import { describe, expect, it, vi, beforeEach } from 'vitest'; import { describe, expect, it, vi, beforeEach } from 'vitest';
import { load } from './+page.server'; import { load } from './+page.server';
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({ createApiClient: vi.fn() }));
createApiClient: vi.fn(),
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
}));
vi.mock('$lib/shared/errors', () => ({ vi.mock('$lib/shared/errors', () => ({
getErrorMessage: (code: string) => code ?? 'Unknown error' getErrorMessage: (code: string) => code ?? 'Unknown error'
})); }));

View File

@@ -1,5 +1,5 @@
import { redirect } from '@sveltejs/kit'; import { redirect } from '@sveltejs/kit';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
import type { components } from '$lib/generated/api'; import type { components } from '$lib/generated/api';
@@ -103,7 +103,8 @@ export async function load({ url, fetch }) {
} }
const errorMessage: string | null = !result.response.ok const errorMessage: string | null = !result.response.ok
? (getErrorMessage(extractErrorCode(result.error)) ?? 'Daten konnten nicht geladen werden.') ? (getErrorMessage((result.error as unknown as { code?: string })?.code) ??
'Daten konnten nicht geladen werden.')
: null; : null;
return { return {

View File

@@ -1,5 +1,5 @@
import { error, redirect } from '@sveltejs/kit'; import { error, redirect } from '@sveltejs/kit';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
import { inferredRelationshipLabel } from '$lib/person/relationshipLabels'; import { inferredRelationshipLabel } from '$lib/person/relationshipLabels';
@@ -17,7 +17,8 @@ export async function load({ params, fetch }) {
if (docResult.response.status === 401) throw redirect(302, '/login'); if (docResult.response.status === 401) throw redirect(302, '/login');
if (!docResult.response.ok) { if (!docResult.response.ok) {
throw error(docResult.response.status, getErrorMessage(extractErrorCode(docResult.error))); const code = (docResult.error as unknown as { code?: string })?.code;
throw error(docResult.response.status, getErrorMessage(code));
} }
const document = docResult.data!; const document = docResult.data!;

View File

@@ -1,6 +1,6 @@
import { error, fail, redirect } from '@sveltejs/kit'; import { error, fail, redirect } from '@sveltejs/kit';
import { env } from '$env/dynamic/private'; import { env } from '$env/dynamic/private';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { parseBackendError, getErrorMessage } from '$lib/shared/errors'; import { parseBackendError, getErrorMessage } from '$lib/shared/errors';
export async function load({ export async function load({
@@ -30,7 +30,8 @@ export async function load({
]); ]);
if (!docResult.response.ok) { if (!docResult.response.ok) {
throw error(docResult.response.status, getErrorMessage(extractErrorCode(docResult.error))); const code = (docResult.error as unknown as { code?: string })?.code;
throw error(docResult.response.status, getErrorMessage(code));
} }
if (!personsResult.response.ok) { if (!personsResult.response.ok) {
throw error(personsResult.response.status, getErrorMessage('INTERNAL_ERROR')); throw error(personsResult.response.status, getErrorMessage('INTERNAL_ERROR'));
@@ -75,9 +76,8 @@ export const actions = {
// Fetch current document to preserve all existing fields // Fetch current document to preserve all existing fields
const docResult = await api.GET('/api/documents/{id}', { params: { path: { id: params.id } } }); const docResult = await api.GET('/api/documents/{id}', { params: { path: { id: params.id } } });
if (!docResult.response.ok) { if (!docResult.response.ok) {
return fail(docResult.response.status, { const code = (docResult.error as unknown as { code?: string })?.code;
error: getErrorMessage(extractErrorCode(docResult.error)) return fail(docResult.response.status, { error: getErrorMessage(code) });
});
} }
const doc = docResult.data!; const doc = docResult.data!;

View File

@@ -1,9 +1,6 @@
import { describe, expect, it, vi, beforeEach } from 'vitest'; import { describe, expect, it, vi, beforeEach } from 'vitest';
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({ createApiClient: vi.fn() }));
createApiClient: vi.fn(),
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
}));
vi.mock('$env/dynamic/private', () => ({ env: { API_INTERNAL_URL: 'http://test-backend:8080' } })); vi.mock('$env/dynamic/private', () => ({ env: { API_INTERNAL_URL: 'http://test-backend:8080' } }));
import { load } from './+page.server'; import { load } from './+page.server';

View File

@@ -1,9 +1,6 @@
import { describe, expect, it, vi, beforeEach } from 'vitest'; import { describe, expect, it, vi, beforeEach } from 'vitest';
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({ createApiClient: vi.fn() }));
createApiClient: vi.fn(),
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
}));
import { load } from './+page.server'; import { load } from './+page.server';
import { createApiClient } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';

View File

@@ -1,6 +1,6 @@
import { error, redirect } from '@sveltejs/kit'; import { error, redirect } from '@sveltejs/kit';
import { env } from '$env/dynamic/private'; import { env } from '$env/dynamic/private';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage, parseBackendError } from '$lib/shared/errors'; import { getErrorMessage, parseBackendError } from '$lib/shared/errors';
export async function load({ export async function load({
@@ -31,7 +31,8 @@ export async function load({
]); ]);
if (!docResult.response.ok) { if (!docResult.response.ok) {
throw error(docResult.response.status, getErrorMessage(extractErrorCode(docResult.error))); const code = (docResult.error as unknown as { code?: string })?.code;
throw error(docResult.response.status, getErrorMessage(code));
} }
const incompleteCount = countResult.response.ok ? (countResult.data?.count ?? 0) : 0; const incompleteCount = countResult.response.ok ? (countResult.data?.count ?? 0) : 0;

View File

@@ -1,5 +1,5 @@
import { error } from '@sveltejs/kit'; import { error } from '@sveltejs/kit';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
import type { components } from '$lib/generated/api'; import type { components } from '$lib/generated/api';
import type { PageServerLoad } from './$types'; import type { PageServerLoad } from './$types';
@@ -25,7 +25,8 @@ export const load: PageServerLoad = async ({ url, fetch }) => {
]); ]);
if (!listResult.response.ok) { if (!listResult.response.ok) {
throw error(listResult.response.status, getErrorMessage(extractErrorCode(listResult.error))); const code = (listResult.error as unknown as { code?: string })?.code;
throw error(listResult.response.status, getErrorMessage(code));
} }
const personFilters = personResults const personFilters = personResults

View File

@@ -1,5 +1,5 @@
import { error } from '@sveltejs/kit'; import { error } from '@sveltejs/kit';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
import type { PageServerLoad } from './$types'; import type { PageServerLoad } from './$types';
@@ -9,7 +9,8 @@ export const load: PageServerLoad = async ({ params, fetch }) => {
params: { path: { id: params.id } } params: { path: { id: params.id } }
}); });
if (!result.response.ok) { if (!result.response.ok) {
throw error(result.response.status, getErrorMessage(extractErrorCode(result.error))); const code = (result.error as unknown as { code?: string })?.code;
throw error(result.response.status, getErrorMessage(code));
} }
return { geschichte: result.data! }; return { geschichte: result.data! };
}; };

View File

@@ -1,5 +1,5 @@
import { error, redirect } from '@sveltejs/kit'; import { error, redirect } from '@sveltejs/kit';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
import type { PageServerLoad } from './$types'; import type { PageServerLoad } from './$types';
@@ -13,7 +13,8 @@ export const load: PageServerLoad = async ({ params, fetch, parent }) => {
params: { path: { id: params.id } } params: { path: { id: params.id } }
}); });
if (!result.response.ok) { if (!result.response.ok) {
throw error(result.response.status, getErrorMessage(extractErrorCode(result.error))); const code = (result.error as unknown as { code?: string })?.code;
throw error(result.response.status, getErrorMessage(code));
} }
return { geschichte: result.data! }; return { geschichte: result.data! };
}; };

View File

@@ -1,9 +1,6 @@
import { describe, expect, it, vi, beforeEach } from 'vitest'; import { describe, expect, it, vi, beforeEach } from 'vitest';
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({ createApiClient: vi.fn() }));
createApiClient: vi.fn(),
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
}));
import { load } from './+page.server'; import { load } from './+page.server';
import { createApiClient } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';

View File

@@ -1,5 +1,5 @@
import { error } from '@sveltejs/kit'; import { error } from '@sveltejs/kit';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
export async function load({ params, fetch, locals }) { export async function load({ params, fetch, locals }) {
@@ -32,10 +32,8 @@ export async function load({ params, fetch, locals }) {
]); ]);
if (!personResult.response.ok) { if (!personResult.response.ok) {
throw error( const code = (personResult.error as unknown as { code?: string })?.code;
personResult.response.status, throw error(personResult.response.status, getErrorMessage(code));
getErrorMessage(extractErrorCode(personResult.error))
);
} }
return { return {

View File

@@ -1,5 +1,5 @@
import { error, fail, redirect } from '@sveltejs/kit'; import { error, fail, redirect } from '@sveltejs/kit';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
import { import {
normalizePersonType, normalizePersonType,
@@ -25,7 +25,8 @@ export async function load({ params, fetch, locals }) {
]); ]);
if (!result.response.ok) { if (!result.response.ok) {
throw error(result.response.status, getErrorMessage(extractErrorCode(result.error))); const code = (result.error as unknown as { code?: string })?.code;
throw error(result.response.status, getErrorMessage(code));
} }
const person = result.data!; const person = result.data!;
@@ -73,9 +74,8 @@ export const actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
updateError: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { updateError: getErrorMessage(code) });
});
} }
throw redirect(303, `/persons/${params.id}`); throw redirect(303, `/persons/${params.id}`);
@@ -100,9 +100,8 @@ export const actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
mergeError: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { mergeError: getErrorMessage(code) });
});
} }
throw redirect(303, `/persons/${targetPersonId}`); throw redirect(303, `/persons/${targetPersonId}`);
@@ -128,9 +127,8 @@ export const actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
aliasError: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { aliasError: getErrorMessage(code) });
});
} }
return { aliasSuccess: true }; return { aliasSuccess: true };
@@ -150,9 +148,8 @@ export const actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
aliasError: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { aliasError: getErrorMessage(code) });
});
} }
return { aliasSuccess: true }; return { aliasSuccess: true };
@@ -169,9 +166,8 @@ export const actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
relationshipError: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { relationshipError: getErrorMessage(code) });
});
} }
return { relationshipSuccess: true }; return { relationshipSuccess: true };
}, },
@@ -215,9 +211,8 @@ export const actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
relationshipError: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { relationshipError: getErrorMessage(code) });
});
} }
return { relationshipSuccess: true }; return { relationshipSuccess: true };
}, },
@@ -235,9 +230,8 @@ export const actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
relationshipError: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { relationshipError: getErrorMessage(code) });
});
} }
return { relationshipSuccess: true }; return { relationshipSuccess: true };
} }

View File

@@ -1,10 +1,7 @@
import { describe, expect, it, vi, beforeEach } from 'vitest'; import { describe, expect, it, vi, beforeEach } from 'vitest';
import { load } from './+page.server'; import { load } from './+page.server';
vi.mock('$lib/shared/api.server', () => ({ vi.mock('$lib/shared/api.server', () => ({ createApiClient: vi.fn() }));
createApiClient: vi.fn(),
extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
}));
import { createApiClient } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';

View File

@@ -1,5 +1,5 @@
import { error, fail, redirect } from '@sveltejs/kit'; import { error, fail, redirect } from '@sveltejs/kit';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
import { import {
normalizePersonType, normalizePersonType,
@@ -57,8 +57,9 @@ export const actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
const code = (result.error as unknown as { code?: string })?.code;
return fail(result.response.status, { return fail(result.response.status, {
error: getErrorMessage(extractErrorCode(result.error)), error: getErrorMessage(code),
personType, personType,
title, title,
firstName, firstName,

View File

@@ -1,7 +1,7 @@
import { fail } from '@sveltejs/kit'; import { fail } from '@sveltejs/kit';
import { env } from '$env/dynamic/private'; import { env } from '$env/dynamic/private';
import type { PageServerLoad, Actions } from './$types'; import type { PageServerLoad, Actions } from './$types';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
const apiBase = () => env.API_INTERNAL_URL || 'http://localhost:8080'; const apiBase = () => env.API_INTERNAL_URL || 'http://localhost:8080';
@@ -27,9 +27,8 @@ export const actions: Actions = {
const result = await api.PUT('/api/users/me', { body }); const result = await api.PUT('/api/users/me', { body });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
updateError: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { updateError: getErrorMessage(code) });
});
} }
return { updateSuccess: true }; return { updateSuccess: true };
@@ -51,9 +50,8 @@ export const actions: Actions = {
}); });
if (!result.response.ok) { if (!result.response.ok) {
return fail(result.response.status, { const code = (result.error as unknown as { code?: string })?.code;
passwordError: getErrorMessage(extractErrorCode(result.error)) return fail(result.response.status, { passwordError: getErrorMessage(code) });
});
} }
return { passwordSuccess: true }; return { passwordSuccess: true };

View File

@@ -1,5 +1,5 @@
import { error, redirect } from '@sveltejs/kit'; import { error, redirect } from '@sveltejs/kit';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
export async function load({ fetch }) { export async function load({ fetch }) {
@@ -9,7 +9,8 @@ export async function load({ fetch }) {
if (result.response.status === 401) throw redirect(302, '/login'); if (result.response.status === 401) throw redirect(302, '/login');
if (!result.response.ok) { if (!result.response.ok) {
throw error(result.response.status, getErrorMessage(extractErrorCode(result.error))); const code = (result.error as unknown as { code?: string })?.code;
throw error(result.response.status, getErrorMessage(code));
} }
const network = result.data!; const network = result.data!;

View File

@@ -1,6 +1,6 @@
import { error } from '@sveltejs/kit'; import { error } from '@sveltejs/kit';
import type { PageServerLoad } from './$types'; import type { PageServerLoad } from './$types';
import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { createApiClient } from '$lib/shared/api.server';
import { getErrorMessage } from '$lib/shared/errors'; import { getErrorMessage } from '$lib/shared/errors';
export const load: PageServerLoad = async ({ params, fetch }) => { export const load: PageServerLoad = async ({ params, fetch }) => {
@@ -8,7 +8,8 @@ export const load: PageServerLoad = async ({ params, fetch }) => {
const result = await api.GET('/api/users/{id}', { params: { path: { id: params.id } } }); const result = await api.GET('/api/users/{id}', { params: { path: { id: params.id } } });
if (!result.response.ok) { if (!result.response.ok) {
throw error(result.response.status, getErrorMessage(extractErrorCode(result.error))); const code = (result.error as unknown as { code?: string })?.code;
throw error(result.response.status, getErrorMessage(code));
} }
return { profileUser: result.data! }; return { profileUser: result.data! };

View File

@@ -196,7 +196,7 @@
}, },
"targets": [ "targets": [
{ {
"expr": "{job=\"$app\"} |= \"$search\" | json", "expr": "{job=\"$app\"} |= \"$search\" | logfmt",
"hide": false, "hide": false,
"legendFormat": "", "legendFormat": "",
"refId": "A" "refId": "A"

View File

@@ -20,4 +20,7 @@ scrape_configs:
- job_name: ocr-service - job_name: ocr-service
metrics_path: /metrics metrics_path: /metrics
static_configs: static_configs:
# TODO: remove or add prometheus-client to ocr-service.
# The Python OCR service does not currently expose Prometheus metrics.
# This target will show as DOWN until prometheus-client is added to ocr-service.
- targets: ['ocr:8000'] - targets: ['ocr:8000']

View File

@@ -2,7 +2,6 @@
import asyncio import asyncio
import glob import glob
import inspect
import io import io
import json import json
import logging import logging
@@ -11,11 +10,9 @@ import re
import shutil import shutil
import subprocess import subprocess
import tempfile import tempfile
import time
import zipfile import zipfile
from contextlib import asynccontextmanager from contextlib import asynccontextmanager
from datetime import datetime, timezone from datetime import datetime, timezone
from typing import Awaitable, Callable
from urllib.parse import urlparse from urllib.parse import urlparse
import httpx import httpx
@@ -23,11 +20,8 @@ import pypdfium2 as pdfium
from fastapi import FastAPI, Form, Header, HTTPException, UploadFile from fastapi import FastAPI, Form, Header, HTTPException, UploadFile
from fastapi.responses import StreamingResponse from fastapi.responses import StreamingResponse
from PIL import Image from PIL import Image
from prometheus_client import REGISTRY
from prometheus_fastapi_instrumentator import Instrumentator
from confidence import apply_confidence_markers, get_threshold from confidence import apply_confidence_markers, get_threshold
from metrics import OcrMetrics, build_metrics
from spell_check import correct_text, load_spell_checker from spell_check import correct_text, load_spell_checker
from engines import kraken as kraken_engine from engines import kraken as kraken_engine
from engines import surya as surya_engine from engines import surya as surya_engine
@@ -43,12 +37,6 @@ logger = logging.getLogger(__name__)
_models_ready = False _models_ready = False
# One-shot import-time binding to the default REGISTRY. Tests that need a
# clean counter state must monkeypatch `main.metrics` with a container built
# from a fresh CollectorRegistry — rebinding through the registry directly
# will not retarget the references stored in the OcrMetrics dataclass.
metrics: OcrMetrics = build_metrics(REGISTRY)
ALLOWED_PDF_HOSTS = set( ALLOWED_PDF_HOSTS = set(
h.strip() for h in os.getenv("ALLOWED_PDF_HOSTS", "minio,localhost,127.0.0.1").split(",") h.strip() for h in os.getenv("ALLOWED_PDF_HOSTS", "minio,localhost,127.0.0.1").split(",")
) )
@@ -56,42 +44,6 @@ ALLOWED_PDF_HOSTS = set(
_SPELL_CHECK_SCRIPT_TYPES = {"HANDWRITING_KURRENT", "HANDWRITING_LATIN"} _SPELL_CHECK_SCRIPT_TYPES = {"HANDWRITING_KURRENT", "HANDWRITING_LATIN"}
async def _record_training(
runner: Callable[[], Awaitable[dict] | dict],
kind: str,
) -> dict:
"""Run a training callable and record outcome + accuracy metrics.
Wraps the per-endpoint try/except + outcome counter + accuracy gauge
block that used to be repeated at /train, /train-sender, and /segtrain.
The runner returns a dict with at least an `accuracy` key; if its value
is None, the gauge is left at its default.
"""
try:
result = runner()
if inspect.isawaitable(result):
result = await result
except Exception:
metrics.ocr_training_runs_total.labels(kind=kind, outcome="error").inc()
raise
metrics.ocr_training_runs_total.labels(kind=kind, outcome="success").inc()
if result.get("accuracy") is not None:
metrics.ocr_model_accuracy.labels(kind=kind).set(result["accuracy"])
return result
def _observe_block_words(words: list[dict], threshold: float) -> None:
"""Record per-block word counts and below-threshold word counts.
Pre: `words` is non-empty. Caller checks for that — keeping the helper
branch-free makes the call sites read as a single line.
"""
metrics.ocr_words_total.inc(len(words))
metrics.ocr_illegible_words_total.inc(
sum(1 for w in words if w["confidence"] < threshold)
)
def _validate_url(url: str) -> None: def _validate_url(url: str) -> None:
"""Validate that the PDF URL points to an allowed host (SSRF protection).""" """Validate that the PDF URL points to an allowed host (SSRF protection)."""
parsed = urlparse(url) parsed = urlparse(url)
@@ -111,7 +63,6 @@ async def lifespan(app: FastAPI):
kraken_engine.load_models() kraken_engine.load_models()
load_spell_checker() load_spell_checker()
_models_ready = True _models_ready = True
metrics.ocr_models_ready.set(1)
logger.info("Startup complete — ready to accept requests") logger.info("Startup complete — ready to accept requests")
yield yield
@@ -121,28 +72,6 @@ async def lifespan(app: FastAPI):
app = FastAPI(title="Familienarchiv OCR Service", lifespan=lifespan) app = FastAPI(title="Familienarchiv OCR Service", lifespan=lifespan)
# /metrics is unauthenticated — relies on Docker-internal-network exposure
# only (CWE-200 risk if `ports:` ever maps 8000 to host). See
# docs/OBSERVABILITY.md §Internal-only endpoints for the Caddy block snippet.
Instrumentator(excluded_handlers=["/health", "/metrics"]).instrument(app).expose(app)
class MetricsPathFilter(logging.Filter):
"""Drop uvicorn.access entries for /metrics and /health to keep logs focused."""
_SUPPRESSED_PATHS = {"/metrics", "/health"}
def filter(self, record: logging.LogRecord) -> bool:
# uvicorn.access formats as: '%s - "%s %s HTTP/%s" %d'
if record.args and len(record.args) >= 3:
path = record.args[2]
if isinstance(path, str) and path in self._SUPPRESSED_PATHS:
return False
return True
logging.getLogger("uvicorn.access").addFilter(MetricsPathFilter())
@app.get("/health") @app.get("/health")
def health(): def health():
@@ -170,9 +99,7 @@ async def run_ocr(request: OcrRequest):
del img del img
script_type = request.scriptType.upper() script_type = request.scriptType.upper()
engine_name = "kraken" if script_type == "HANDWRITING_KURRENT" else "surya"
extract_started = time.monotonic()
if script_type == "HANDWRITING_KURRENT": if script_type == "HANDWRITING_KURRENT":
if not kraken_engine.is_available(): if not kraken_engine.is_available():
raise HTTPException( raise HTTPException(
@@ -184,18 +111,11 @@ async def run_ocr(request: OcrRequest):
else: else:
# TYPEWRITER, HANDWRITING_LATIN, UNKNOWN — all use Surya # TYPEWRITER, HANDWRITING_LATIN, UNKNOWN — all use Surya
blocks = await asyncio.to_thread(surya_engine.extract_blocks, images, request.language) blocks = await asyncio.to_thread(surya_engine.extract_blocks, images, request.language)
metrics.ocr_processing_seconds.labels(engine=engine_name).observe(
time.monotonic() - extract_started
)
metrics.ocr_jobs_total.labels(engine=engine_name, script_type=script_type).inc()
threshold = get_threshold(script_type) threshold = get_threshold(script_type)
for block in blocks: for block in blocks:
words = block.get("words") or [] if block.get("words"):
if words: block["text"] = apply_confidence_markers(block["words"], threshold)
_observe_block_words(words, threshold)
block["text"] = apply_confidence_markers(words, threshold)
block.pop("words", None) block.pop("words", None)
if script_type in _SPELL_CHECK_SCRIPT_TYPES: if script_type in _SPELL_CHECK_SCRIPT_TYPES:
block["text"] = correct_text(block["text"]) block["text"] = correct_text(block["text"])
@@ -226,9 +146,6 @@ async def run_ocr_stream(request: OcrRequest):
) )
engine = kraken_engine if use_kraken else surya_engine engine = kraken_engine if use_kraken else surya_engine
engine_name = "kraken" if use_kraken else "surya"
metrics.ocr_jobs_total.labels(engine=engine_name, script_type=script_type).inc()
if request.regions: if request.regions:
# Guided mode: recognize only the user-drawn annotation regions # Guided mode: recognize only the user-drawn annotation regions
@@ -259,15 +176,12 @@ async def run_ocr_stream(request: OcrRequest):
image = await asyncio.to_thread(preprocess_page, image) image = await asyncio.to_thread(preprocess_page, image)
blocks = [] blocks = []
sender_path = request.senderModelPath if use_kraken else None sender_path = request.senderModelPath if use_kraken else None
engine_seconds = 0.0
for region in page_regions: for region in page_regions:
region_started = time.monotonic()
text = await asyncio.to_thread( text = await asyncio.to_thread(
engine.extract_region_text, image, engine.extract_region_text, image,
region.x, region.y, region.width, region.height, region.x, region.y, region.width, region.height,
sender_path, sender_path,
) )
engine_seconds += time.monotonic() - region_started
if script_type in _SPELL_CHECK_SCRIPT_TYPES: if script_type in _SPELL_CHECK_SCRIPT_TYPES:
text = correct_text(text) text = correct_text(text)
blocks.append({ blocks.append({
@@ -281,11 +195,7 @@ async def run_ocr_stream(request: OcrRequest):
"annotationId": region.annotationId, "annotationId": region.annotationId,
}) })
metrics.ocr_processing_seconds.labels(engine=engine_name).observe(
engine_seconds
)
total_blocks += len(blocks) total_blocks += len(blocks)
metrics.ocr_pages_total.labels(engine=engine_name).inc()
yield json.dumps({ yield json.dumps({
"type": "page", "type": "page",
"pageNumber": page_idx, "pageNumber": page_idx,
@@ -295,7 +205,6 @@ async def run_ocr_stream(request: OcrRequest):
except Exception: except Exception:
logger.exception("Guided OCR failed on page %d", page_idx) logger.exception("Guided OCR failed on page %d", page_idx)
skipped_pages += 1 skipped_pages += 1
metrics.ocr_skipped_pages_total.inc()
yield json.dumps({ yield json.dumps({
"type": "error", "type": "error",
"pageNumber": page_idx, "pageNumber": page_idx,
@@ -329,25 +238,18 @@ async def run_ocr_stream(request: OcrRequest):
yield json.dumps({"type": "preprocessing", "pageNumber": page_idx}) + "\n" yield json.dumps({"type": "preprocessing", "pageNumber": page_idx}) + "\n"
image = await asyncio.to_thread(preprocess_page, image) image = await asyncio.to_thread(preprocess_page, image)
sender_path = request.senderModelPath if use_kraken else None sender_path = request.senderModelPath if use_kraken else None
page_started = time.monotonic()
blocks = await asyncio.to_thread( blocks = await asyncio.to_thread(
engine.extract_page_blocks, image, page_idx, request.language, sender_path engine.extract_page_blocks, image, page_idx, request.language, sender_path
) )
metrics.ocr_processing_seconds.labels(engine=engine_name).observe(
time.monotonic() - page_started
)
for block in blocks: for block in blocks:
words = block.get("words") or [] if block.get("words"):
if words: block["text"] = apply_confidence_markers(block["words"], threshold)
_observe_block_words(words, threshold)
block["text"] = apply_confidence_markers(words, threshold)
block.pop("words", None) block.pop("words", None)
if script_type in _SPELL_CHECK_SCRIPT_TYPES: if script_type in _SPELL_CHECK_SCRIPT_TYPES:
block["text"] = correct_text(block["text"]) block["text"] = correct_text(block["text"])
total_blocks += len(blocks) total_blocks += len(blocks)
metrics.ocr_pages_total.labels(engine=engine_name).inc()
yield json.dumps({ yield json.dumps({
"type": "page", "type": "page",
"pageNumber": page_idx, "pageNumber": page_idx,
@@ -357,7 +259,6 @@ async def run_ocr_stream(request: OcrRequest):
except Exception: except Exception:
logger.exception("OCR failed on page %d", page_idx) logger.exception("OCR failed on page %d", page_idx)
skipped_pages += 1 skipped_pages += 1
metrics.ocr_skipped_pages_total.inc()
yield json.dumps({ yield json.dumps({
"type": "error", "type": "error",
"pageNumber": page_idx, "pageNumber": page_idx,
@@ -537,7 +438,8 @@ async def train_model(
return {"loss": None, "accuracy": accuracy, "cer": cer, "epochs": epochs} return {"loss": None, "accuracy": accuracy, "cer": cer, "epochs": epochs}
return await _record_training(lambda: asyncio.to_thread(_run_training), kind="recognition") result = await asyncio.to_thread(_run_training)
return result
@app.post("/train-sender") @app.post("/train-sender")
@@ -616,9 +518,8 @@ async def train_sender_model(
return {"loss": None, "accuracy": accuracy, "cer": cer, "epochs": epochs} return {"loss": None, "accuracy": accuracy, "cer": cer, "epochs": epochs}
return await _record_training( result = await asyncio.to_thread(_run_sender_training)
lambda: asyncio.to_thread(_run_sender_training), kind="recognition" return result
)
@app.post("/segtrain") @app.post("/segtrain")
@@ -727,7 +628,8 @@ async def segtrain_model(
return {"loss": None, "accuracy": accuracy, "cer": cer, "epochs": epochs} return {"loss": None, "accuracy": accuracy, "cer": cer, "epochs": epochs}
return await _record_training(lambda: asyncio.to_thread(_run_segtrain), kind="segmentation") result = await asyncio.to_thread(_run_segtrain)
return result
async def _download_and_convert_pdf(url: str) -> list[Image.Image]: async def _download_and_convert_pdf(url: str) -> list[Image.Image]:

View File

@@ -1,92 +0,0 @@
"""Prometheus metric definitions for the OCR service.
`build_metrics(registry)` returns a fresh `OcrMetrics` instance bound to the
given `CollectorRegistry`. Production code calls it once at module load with
the default `REGISTRY`; tests pass a per-test `CollectorRegistry()` to keep
counter values isolated between cases (decision #3 on issue #652).
"""
from __future__ import annotations
from dataclasses import dataclass
from prometheus_client import CollectorRegistry, Counter, Gauge, Histogram
@dataclass(frozen=True)
class OcrMetrics:
"""Container for every custom OCR metric.
Counters and gauges are immutable references to `prometheus_client`
instances. Mutating them (`.inc()`, `.observe()`, `.set()`) is safe;
rebinding the field on the dataclass is not — use `build_metrics` to get
a new container.
"""
ocr_jobs_total: Counter
ocr_pages_total: Counter
ocr_skipped_pages_total: Counter
ocr_words_total: Counter
ocr_illegible_words_total: Counter
ocr_processing_seconds: Histogram
ocr_training_runs_total: Counter
ocr_model_accuracy: Gauge
ocr_models_ready: Gauge
def build_metrics(registry: CollectorRegistry) -> OcrMetrics:
"""Create one OcrMetrics instance bound to `registry`."""
return OcrMetrics(
ocr_jobs_total=Counter(
"ocr_jobs_total",
"Number of OCR jobs processed, labelled by engine and script type.",
["engine", "script_type"],
registry=registry,
),
ocr_pages_total=Counter(
"ocr_pages_total",
"Number of pages successfully OCR'd, labelled by engine.",
["engine"],
registry=registry,
),
ocr_skipped_pages_total=Counter(
"ocr_skipped_pages_total",
"Number of pages skipped because the OCR engine raised.",
registry=registry,
),
ocr_words_total=Counter(
"ocr_words_total",
"Number of words recognized across all OCR blocks.",
registry=registry,
),
ocr_illegible_words_total=Counter(
"ocr_illegible_words_total",
"Number of words below the confidence threshold "
"(replaced with [unleserlich]).",
registry=registry,
),
ocr_processing_seconds=Histogram(
"ocr_processing_seconds",
"OCR processing time per page (streaming) or per document (non-streaming).",
["engine"],
registry=registry,
),
ocr_training_runs_total=Counter(
"ocr_training_runs_total",
"Number of training runs, labelled by kind (recognition|segmentation) "
"and outcome (success|error).",
["kind", "outcome"],
registry=registry,
),
ocr_model_accuracy=Gauge(
"ocr_model_accuracy",
"Latest model accuracy reported by a successful training run.",
["kind"],
registry=registry,
),
ocr_models_ready=Gauge(
"ocr_models_ready",
"1 once the lifespan startup has finished loading models, 0 before.",
registry=registry,
),
)

View File

@@ -10,5 +10,3 @@ pyvips>=2.2.0
httpx==0.28.1 httpx==0.28.1
pyspellchecker==0.9.0 pyspellchecker==0.9.0
opencv-python-headless==4.11.0.86 opencv-python-headless==4.11.0.86
prometheus-fastapi-instrumentator==7.0.0
prometheus-client==0.25.0

View File

@@ -1,638 +0,0 @@
"""Tests for Prometheus metrics exposed by the OCR service.
Each test that asserts on a counter/gauge value uses a fresh CollectorRegistry
(see decision #3 on issue #652) to keep the metrics isolated between tests.
"""
import contextlib
import io
import zipfile
from unittest.mock import AsyncMock, patch
import pytest
from httpx import ASGITransport, AsyncClient
from PIL import Image
from prometheus_client import CollectorRegistry
from main import app
from metrics import build_metrics
@contextlib.asynccontextmanager
async def ocr_client(*, raise_app_exceptions: bool = True):
"""Yield an AsyncClient with model-loaders patched and _models_ready forced on.
The shared setup for almost every metrics test: stub the heavy lifecycle
hooks (kraken_engine.load_models, load_spell_checker), flip the readiness
flag so request handlers do not 503, and restore it afterwards.
"""
with patch("main.kraken_engine.load_models"), \
patch("main.load_spell_checker"):
transport = ASGITransport(app=app, raise_app_exceptions=raise_app_exceptions)
async with AsyncClient(transport=transport, base_url="http://test") as client:
import main as main_module
main_module._models_ready = True
try:
yield client
finally:
main_module._models_ready = False
def _minimal_zip() -> bytes:
"""Return a ZIP containing one fake .xml so endpoint validation passes."""
buf = io.BytesIO()
with zipfile.ZipFile(buf, "w") as zf:
zf.writestr("page_01.xml", "<PcGts/>")
return buf.getvalue()
def _fake_training_result(accuracy: float = 0.91) -> dict:
return {"loss": None, "accuracy": accuracy, "cer": round(1 - accuracy, 4), "epochs": 5}
@pytest.fixture
def fresh_metrics(monkeypatch):
"""Replace the module-level `main.metrics` with one bound to a fresh registry."""
registry = CollectorRegistry()
test_metrics = build_metrics(registry)
monkeypatch.setattr("main.metrics", test_metrics)
return test_metrics
@pytest.mark.asyncio
async def test_metrics_endpoint_returns_200():
"""`GET /metrics` returns 200 with Prometheus exposition content.
Uses the global REGISTRY by design — does NOT take the `fresh_metrics` fixture.
The `/metrics` endpoint is wired by `prometheus-fastapi-instrumentator`, which
binds to the default REGISTRY at app-construction time; swapping `main.metrics`
via the fixture would not redirect what `/metrics` exposes. This test only
asserts response shape (status code + content-type substring), not numeric
counter values, so cross-test state leakage cannot affect it.
"""
with patch("main.kraken_engine.load_models"), \
patch("main.load_spell_checker"):
async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client:
response = await client.get("/metrics")
assert response.status_code == 200
assert "text/plain" in response.headers.get("content-type", "")
@pytest.mark.asyncio
async def test_metrics_includes_http_request_metrics_after_ocr_call():
"""After a request to /ocr, `/metrics` exposes auto-instrumented http_* metrics.
Uses the global REGISTRY by design — does NOT take the `fresh_metrics` fixture.
The `http_requests_total` / `http_request_duration_seconds` metrics live on
the instrumentator's default REGISTRY (not on `main.metrics`), so a fresh
CollectorRegistry would never see them. This test only asserts response shape
(substring presence in the exposition body), not numeric counter values, so
cross-test state leakage cannot affect it.
"""
mock_images = [Image.new("RGB", (100, 100))]
mock_blocks = [{"pageNumber": 1, "x": 0.0, "y": 0.0, "width": 1.0, "height": 1.0,
"polygon": None, "text": "hi", "words": []}]
with patch("main._download_and_convert_pdf", new_callable=AsyncMock, return_value=mock_images), \
patch("main.preprocess_page", side_effect=lambda img: img), \
patch("main.surya_engine.extract_blocks", return_value=mock_blocks):
async with ocr_client() as client:
ocr_response = await client.post("/ocr", json={
"pdfUrl": "http://minio/doc.pdf",
"scriptType": "TYPEWRITER",
"language": "de",
})
assert ocr_response.status_code == 200, ocr_response.text
metrics_response = await client.get("/metrics")
body = metrics_response.text
assert "http_requests_total" in body
assert "http_request_duration_seconds" in body
def test_build_metrics_registers_all_custom_metrics_on_given_registry():
"""`build_metrics` returns an OcrMetrics bound to the supplied registry."""
registry = CollectorRegistry()
metrics = build_metrics(registry)
metric_names = {m.name for m in registry.collect()}
expected = {
"ocr_jobs",
"ocr_pages",
"ocr_skipped_pages",
"ocr_words",
"ocr_illegible_words",
"ocr_processing_seconds",
"ocr_training_runs",
"ocr_model_accuracy",
"ocr_models_ready",
}
assert expected <= metric_names, f"missing: {expected - metric_names}"
# A second registry yields a separate container — no shared state.
other_metrics = build_metrics(CollectorRegistry())
assert metrics is not other_metrics
async def _drive_ocr(client: AsyncClient, *, script_type: str) -> None:
"""Helper — fires /ocr with a single mocked page and asserts a 200."""
response = await client.post("/ocr", json={
"pdfUrl": "http://minio/doc.pdf",
"scriptType": script_type,
"language": "de",
})
assert response.status_code == 200, response.text
@pytest.mark.asyncio
async def test_ocr_jobs_total_incremented_with_kraken_engine_label_for_kurrent(fresh_metrics):
"""A /ocr call with HANDWRITING_KURRENT increments engine=kraken."""
mock_images = [Image.new("RGB", (100, 100))]
mock_blocks = [{"pageNumber": 1, "x": 0.0, "y": 0.0, "width": 1.0, "height": 1.0,
"polygon": None, "text": "hi", "words": []}]
with patch("main.correct_text", side_effect=lambda t: t), \
patch("main._download_and_convert_pdf", new_callable=AsyncMock, return_value=mock_images), \
patch("main.preprocess_page", side_effect=lambda img: img), \
patch("main.kraken_engine.is_available", return_value=True), \
patch("main.kraken_engine.extract_blocks", return_value=mock_blocks):
async with ocr_client() as client:
await _drive_ocr(client, script_type="HANDWRITING_KURRENT")
value = fresh_metrics.ocr_jobs_total.labels(
engine="kraken", script_type="HANDWRITING_KURRENT"
)._value.get()
assert value == 1.0
@pytest.mark.asyncio
async def test_ocr_jobs_total_incremented_with_surya_engine_label_for_typewriter(fresh_metrics):
"""A /ocr call with TYPEWRITER increments engine=surya."""
mock_images = [Image.new("RGB", (100, 100))]
mock_blocks = [{"pageNumber": 1, "x": 0.0, "y": 0.0, "width": 1.0, "height": 1.0,
"polygon": None, "text": "hi", "words": []}]
with patch("main._download_and_convert_pdf", new_callable=AsyncMock, return_value=mock_images), \
patch("main.preprocess_page", side_effect=lambda img: img), \
patch("main.surya_engine.extract_blocks", return_value=mock_blocks):
async with ocr_client() as client:
await _drive_ocr(client, script_type="TYPEWRITER")
value = fresh_metrics.ocr_jobs_total.labels(
engine="surya", script_type="TYPEWRITER"
)._value.get()
assert value == 1.0
@pytest.mark.asyncio
async def test_ocr_pages_total_incremented_once_per_page_in_stream(fresh_metrics):
"""The /ocr/stream generator increments ocr_pages_total per successful page."""
mock_images = [Image.new("RGB", (100, 100)) for _ in range(3)]
mock_blocks = [{"pageNumber": 1, "x": 0.0, "y": 0.0, "width": 1.0, "height": 1.0,
"polygon": None, "text": "hi", "words": []}]
with patch("main._download_and_convert_pdf", new_callable=AsyncMock, return_value=mock_images), \
patch("main.preprocess_page", side_effect=lambda img: img), \
patch("main.surya_engine.extract_page_blocks", return_value=mock_blocks):
async with ocr_client() as client:
async with client.stream("POST", "/ocr/stream", json={
"pdfUrl": "http://minio/doc.pdf",
"scriptType": "TYPEWRITER",
"language": "de",
}) as response:
assert response.status_code == 200
# Drain the stream so all per-page increments fire.
async for _ in response.aiter_lines():
pass
value = fresh_metrics.ocr_pages_total.labels(engine="surya")._value.get()
assert value == 3.0
@pytest.mark.asyncio
async def test_ocr_skipped_pages_total_incremented_when_engine_raises_for_a_page(fresh_metrics):
"""When the engine raises on a page, ocr_skipped_pages_total bumps and the stream finishes."""
mock_images = [Image.new("RGB", (100, 100)) for _ in range(2)]
good_blocks = [{"pageNumber": 1, "x": 0.0, "y": 0.0, "width": 1.0, "height": 1.0,
"polygon": None, "text": "ok", "words": []}]
call_count = {"n": 0}
def extract_side_effect(*args, **kwargs):
call_count["n"] += 1
if call_count["n"] == 1:
raise RuntimeError("synthetic engine failure")
return good_blocks
with patch("main._download_and_convert_pdf", new_callable=AsyncMock, return_value=mock_images), \
patch("main.preprocess_page", side_effect=lambda img: img), \
patch("main.surya_engine.extract_page_blocks", side_effect=extract_side_effect):
async with ocr_client() as client:
async with client.stream("POST", "/ocr/stream", json={
"pdfUrl": "http://minio/doc.pdf",
"scriptType": "TYPEWRITER",
"language": "de",
}) as response:
assert response.status_code == 200
saw_error = False
async for line in response.aiter_lines():
if line and '"type": "error"' in line:
saw_error = True
assert saw_error
assert fresh_metrics.ocr_skipped_pages_total._value.get() == 1.0
# The second page still succeeds.
assert fresh_metrics.ocr_pages_total.labels(engine="surya")._value.get() == 1.0
@pytest.mark.asyncio
async def test_ocr_words_and_illegible_words_total_sum_across_blocks(fresh_metrics):
"""Counters reflect totals summed over every block in the request.
Threshold defaults to THRESHOLD_DEFAULT (0.3) for non-Kurrent scripts. Two
blocks: 3 words above + 2 words below threshold across blocks.
"""
mock_images = [Image.new("RGB", (100, 100))]
mock_blocks = [
{"pageNumber": 1, "x": 0.0, "y": 0.0, "width": 1.0, "height": 1.0,
"polygon": None, "text": "ignored",
"words": [{"text": "Lieber", "confidence": 0.9},
{"text": "Freund", "confidence": 0.1}]},
{"pageNumber": 1, "x": 0.0, "y": 0.0, "width": 1.0, "height": 1.0,
"polygon": None, "text": "ignored",
"words": [{"text": "Gruss", "confidence": 0.8},
{"text": "verschmiert", "confidence": 0.05},
{"text": "Karl", "confidence": 0.95}]},
]
with patch("main._download_and_convert_pdf", new_callable=AsyncMock, return_value=mock_images), \
patch("main.preprocess_page", side_effect=lambda img: img), \
patch("main.surya_engine.extract_blocks", return_value=mock_blocks):
async with ocr_client() as client:
await _drive_ocr(client, script_type="TYPEWRITER")
assert fresh_metrics.ocr_words_total._value.get() == 5.0
assert fresh_metrics.ocr_illegible_words_total._value.get() == 2.0
def _histogram_count_sum(histogram, **labels) -> tuple[float, float]:
"""Read the per-label-set _count and _sum from a prometheus_client Histogram."""
child = histogram.labels(**labels)
return child._sum.get(), sum(b.get() for b in child._buckets)
@pytest.mark.asyncio
async def test_ocr_processing_seconds_histogram_observed_per_page_in_stream(fresh_metrics):
"""The streaming generator observes ocr_processing_seconds once per page."""
mock_images = [Image.new("RGB", (100, 100)) for _ in range(2)]
mock_blocks = [{"pageNumber": 1, "x": 0.0, "y": 0.0, "width": 1.0, "height": 1.0,
"polygon": None, "text": "ok", "words": []}]
with patch("main._download_and_convert_pdf", new_callable=AsyncMock, return_value=mock_images), \
patch("main.preprocess_page", side_effect=lambda img: img), \
patch("main.surya_engine.extract_page_blocks", return_value=mock_blocks):
async with ocr_client() as client:
async with client.stream("POST", "/ocr/stream", json={
"pdfUrl": "http://minio/doc.pdf",
"scriptType": "TYPEWRITER",
"language": "de",
}) as response:
assert response.status_code == 200
async for _ in response.aiter_lines():
pass
sum_seconds, count = _histogram_count_sum(
fresh_metrics.ocr_processing_seconds, engine="surya"
)
assert count == 2.0
assert sum_seconds >= 0.0
@pytest.mark.asyncio
async def test_ocr_training_runs_total_incremented_with_recognition_success_label(fresh_metrics):
"""/train success increments ocr_training_runs_total{kind=recognition, outcome=success}."""
async def fake_to_thread(func, *args, **kwargs):
return _fake_training_result()
with patch("main.TRAINING_TOKEN", "secret-token"), \
patch("main._models_ready", True), \
patch("main.asyncio.to_thread", side_effect=fake_to_thread):
async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client:
response = await client.post(
"/train",
files={"file": ("training.zip", _minimal_zip(), "application/zip")},
headers={"X-Training-Token": "secret-token"},
)
assert response.status_code == 200
assert fresh_metrics.ocr_training_runs_total.labels(
kind="recognition", outcome="success"
)._value.get() == 1.0
@pytest.mark.asyncio
async def test_ocr_training_runs_total_incremented_with_recognition_error_label(fresh_metrics):
"""When ketos exits non-zero, the error counter bumps and the exception propagates.
Uses the narrowest available seam — `subprocess.run` returning a failing
CompletedProcess — instead of stubbing the asyncio.to_thread boundary,
so the test exercises the real _run_training error path.
"""
from subprocess import CompletedProcess
failing_proc = CompletedProcess(
args=["ketos"], returncode=1, stdout="", stderr="synthetic ketos failure"
)
with patch("main.TRAINING_TOKEN", "secret-token"), \
patch("main._models_ready", True), \
patch("main.subprocess.run", return_value=failing_proc):
transport = ASGITransport(app=app, raise_app_exceptions=False)
async with AsyncClient(transport=transport, base_url="http://test") as client:
response = await client.post(
"/train",
files={"file": ("training.zip", _minimal_zip(), "application/zip")},
headers={"X-Training-Token": "secret-token"},
)
assert response.status_code == 500
assert fresh_metrics.ocr_training_runs_total.labels(
kind="recognition", outcome="error"
)._value.get() == 1.0
@pytest.mark.asyncio
async def test_ocr_training_runs_total_incremented_with_segmentation_success_label(fresh_metrics):
"""/segtrain success increments ocr_training_runs_total{kind=segmentation, outcome=success}."""
async def fake_to_thread(func, *args, **kwargs):
return _fake_training_result(accuracy=0.83)
with patch("main.TRAINING_TOKEN", "secret-token"), \
patch("main._models_ready", True), \
patch("main.asyncio.to_thread", side_effect=fake_to_thread):
async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client:
response = await client.post(
"/segtrain",
files={"file": ("training.zip", _minimal_zip(), "application/zip")},
headers={"X-Training-Token": "secret-token"},
)
assert response.status_code == 200
assert fresh_metrics.ocr_training_runs_total.labels(
kind="segmentation", outcome="success"
)._value.get() == 1.0
@pytest.mark.asyncio
async def test_ocr_training_runs_total_incremented_with_recognition_success_label_for_train_sender(fresh_metrics):
"""/train-sender success increments ocr_training_runs_total{kind=recognition, outcome=success}."""
async def fake_to_thread(func, *args, **kwargs):
return _fake_training_result()
with patch("main.TRAINING_TOKEN", "secret-token"), \
patch("main._models_ready", True), \
patch("main.asyncio.to_thread", side_effect=fake_to_thread):
async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client:
response = await client.post(
"/train-sender",
files={"file": ("training.zip", _minimal_zip(), "application/zip")},
data={"output_model_path": "/app/models/sender_test.mlmodel"},
headers={"X-Training-Token": "secret-token"},
)
assert response.status_code == 200, response.text
assert fresh_metrics.ocr_training_runs_total.labels(
kind="recognition", outcome="success"
)._value.get() == 1.0
@pytest.mark.asyncio
async def test_ocr_model_accuracy_gauge_stays_default_when_training_returns_no_accuracy(fresh_metrics):
"""When the runner returns accuracy=None, ocr_model_accuracy must remain at its default 0."""
async def fake_to_thread(func, *args, **kwargs):
return {"loss": None, "accuracy": None, "cer": None, "epochs": 5}
with patch("main.TRAINING_TOKEN", "secret-token"), \
patch("main._models_ready", True), \
patch("main.asyncio.to_thread", side_effect=fake_to_thread):
async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client:
response = await client.post(
"/train",
files={"file": ("training.zip", _minimal_zip(), "application/zip")},
headers={"X-Training-Token": "secret-token"},
)
assert response.status_code == 200
# Gauge was never .set() — accessing the label child still creates it with default 0.0.
assert fresh_metrics.ocr_model_accuracy.labels(
kind="recognition"
)._value.get() == 0.0
@pytest.mark.asyncio
async def test_ocr_model_accuracy_gauge_set_per_kind_after_successful_training(fresh_metrics):
"""After /train and /segtrain succeed, ocr_model_accuracy{kind=...} reflects the result."""
recognition_accuracy = 0.917
segmentation_accuracy = 0.834
async def fake_recognition_to_thread(func, *args, **kwargs):
return _fake_training_result(accuracy=recognition_accuracy)
async def fake_segmentation_to_thread(func, *args, **kwargs):
return _fake_training_result(accuracy=segmentation_accuracy)
with patch("main.TRAINING_TOKEN", "secret-token"), \
patch("main._models_ready", True):
async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client:
with patch("main.asyncio.to_thread", side_effect=fake_recognition_to_thread):
rec_resp = await client.post(
"/train",
files={"file": ("training.zip", _minimal_zip(), "application/zip")},
headers={"X-Training-Token": "secret-token"},
)
assert rec_resp.status_code == 200
with patch("main.asyncio.to_thread", side_effect=fake_segmentation_to_thread):
seg_resp = await client.post(
"/segtrain",
files={"file": ("training.zip", _minimal_zip(), "application/zip")},
headers={"X-Training-Token": "secret-token"},
)
assert seg_resp.status_code == 200
assert fresh_metrics.ocr_model_accuracy.labels(kind="recognition")._value.get() == pytest.approx(recognition_accuracy)
assert fresh_metrics.ocr_model_accuracy.labels(kind="segmentation")._value.get() == pytest.approx(segmentation_accuracy)
def test_ocr_models_ready_gauge_defaults_to_zero():
"""A freshly-built OcrMetrics has ocr_models_ready=0 before lifespan runs."""
metrics = build_metrics(CollectorRegistry())
assert metrics.ocr_models_ready._value.get() == 0.0
@pytest.mark.asyncio
async def test_ocr_models_ready_gauge_is_one_after_lifespan_startup(fresh_metrics):
"""The lifespan flips ocr_models_ready to 1 once load_models / load_spell_checker return.
ASGITransport does not run lifespan by default, so the lifespan context
manager is driven directly to exercise the startup code path.
"""
assert fresh_metrics.ocr_models_ready._value.get() == 0.0
with patch("main.kraken_engine.load_models"), \
patch("main.load_spell_checker"):
async with app.router.lifespan_context(app):
assert fresh_metrics.ocr_models_ready._value.get() == 1.0
@pytest.mark.asyncio
async def test_ocr_processing_seconds_histogram_observed_per_page_in_guided_stream(fresh_metrics):
"""The guided streaming generator observes ocr_processing_seconds once per page."""
mock_images = [Image.new("RGB", (100, 100)) for _ in range(2)]
regions = [
{"pageNumber": 1, "x": 0.0, "y": 0.0, "width": 0.5, "height": 0.5, "annotationId": "a1"},
{"pageNumber": 2, "x": 0.0, "y": 0.0, "width": 1.0, "height": 1.0, "annotationId": "a2"},
]
with patch("main._download_and_convert_pdf", new_callable=AsyncMock, return_value=mock_images), \
patch("main.preprocess_page", side_effect=lambda img: img), \
patch("main.surya_engine.extract_region_text", return_value="text"):
async with ocr_client() as client:
async with client.stream("POST", "/ocr/stream", json={
"pdfUrl": "http://minio/doc.pdf",
"scriptType": "TYPEWRITER",
"language": "de",
"regions": regions,
}) as response:
assert response.status_code == 200
async for _ in response.aiter_lines():
pass
sum_seconds, count = _histogram_count_sum(
fresh_metrics.ocr_processing_seconds, engine="surya"
)
assert count == 2.0
assert sum_seconds >= 0.0
@pytest.mark.asyncio
async def test_ocr_processing_seconds_histogram_excludes_spell_check_time_in_guided_stream(fresh_metrics):
"""The guided observation must time engine work only, not the spell-check pass.
Wall-clock bound rather than a structural `patch("main.time.monotonic")`:
the patched attribute is the *global* `time.monotonic`, which httpx and
asyncio also consume — they exhaust the deterministic sequence before the
request reaches the engine loop. Bound is sized against the failure mode,
not the noise floor: spell-check sleeps 0.05s × 2 regions = 0.1s, so a
timer that accidentally wrapped `correct_text` would observe >= 0.1s. The
0.09s ceiling catches that bug while leaving ~90ms of slack for slow CI
runners (engine work is instantaneous under the mock).
"""
mock_images = [Image.new("RGB", (100, 100))]
regions = [
{"pageNumber": 1, "x": 0.0, "y": 0.0, "width": 0.5, "height": 0.5, "annotationId": "a1"},
{"pageNumber": 1, "x": 0.5, "y": 0.0, "width": 0.5, "height": 0.5, "annotationId": "a2"},
]
def slow_correct(text):
import time as _time
_time.sleep(0.05)
return text
with patch("main._download_and_convert_pdf", new_callable=AsyncMock, return_value=mock_images), \
patch("main.preprocess_page", side_effect=lambda img: img), \
patch("main.kraken_engine.is_available", return_value=True), \
patch("main.kraken_engine.extract_region_text", return_value="text"), \
patch("main.correct_text", side_effect=slow_correct):
async with ocr_client() as client:
async with client.stream("POST", "/ocr/stream", json={
"pdfUrl": "http://minio/doc.pdf",
"scriptType": "HANDWRITING_KURRENT",
"language": "de",
"regions": regions,
}) as response:
assert response.status_code == 200
async for _ in response.aiter_lines():
pass
sum_seconds, _ = _histogram_count_sum(
fresh_metrics.ocr_processing_seconds, engine="kraken"
)
assert sum_seconds < 0.09, f"timing must exclude spell-check; got sum={sum_seconds}"
@pytest.mark.asyncio
async def test_ocr_jobs_total_not_incremented_when_pdf_download_fails_in_stream(fresh_metrics):
"""If `_download_and_convert_pdf` raises, ocr_jobs_total is NOT incremented.
Mirrors the /ocr endpoint's semantics: the counter only records jobs that
actually started OCR work, not failed downloads.
"""
async def fail_download(url):
raise RuntimeError("synthetic download failure")
with patch("main._download_and_convert_pdf", new=fail_download):
async with ocr_client(raise_app_exceptions=False) as client:
response = await client.post("/ocr/stream", json={
"pdfUrl": "http://minio/doc.pdf",
"scriptType": "TYPEWRITER",
"language": "de",
})
assert response.status_code == 500
assert fresh_metrics.ocr_jobs_total.labels(
engine="surya", script_type="TYPEWRITER"
)._value.get() == 0.0
def test_uvicorn_access_log_filter_fails_open_on_short_or_missing_args():
"""The filter must default-allow records when args is None or shorter than expected.
Locks in fail-open behavior: if uvicorn ever changes its format we keep
forwarding records to the handler rather than silently dropping logs.
"""
import logging as _logging
from main import MetricsPathFilter
filt = MetricsPathFilter()
none_record = _logging.LogRecord(
name="uvicorn.access", level=_logging.INFO, pathname="", lineno=0,
msg="some message", args=None, exc_info=None,
)
short_record = _logging.LogRecord(
name="uvicorn.access", level=_logging.INFO, pathname="", lineno=0,
msg="%s %s", args=("a", "b"), exc_info=None,
)
assert filt.filter(none_record) is True
assert filt.filter(short_record) is True
def test_uvicorn_access_log_filter_skips_metrics_path():
"""The MetricsPathFilter drops uvicorn.access log records that target /metrics."""
import logging as _logging
from main import MetricsPathFilter
filt = MetricsPathFilter()
metrics_record = _logging.LogRecord(
name="uvicorn.access", level=_logging.INFO, pathname="", lineno=0,
msg='%s - "%s %s HTTP/%s" %d',
args=("127.0.0.1:1234", "GET", "/metrics", "1.1", 200),
exc_info=None,
)
health_record = _logging.LogRecord(
name="uvicorn.access", level=_logging.INFO, pathname="", lineno=0,
msg='%s - "%s %s HTTP/%s" %d',
args=("127.0.0.1:1234", "GET", "/health", "1.1", 200),
exc_info=None,
)
ocr_record = _logging.LogRecord(
name="uvicorn.access", level=_logging.INFO, pathname="", lineno=0,
msg='%s - "%s %s HTTP/%s" %d',
args=("127.0.0.1:1234", "POST", "/ocr", "1.1", 200),
exc_info=None,
)
assert filt.filter(metrics_record) is False
assert filt.filter(health_record) is False
assert filt.filter(ocr_record) is True