refactor(backend): split ThumbnailService.generate into stages with distinct logs
Addresses @felixbrandt — fix(backend): "the two try blocks in generate() overlap — a save failure logs 'generation failed' even though the thumbnail is already in S3 as an orphan". generate() now orchestrates four stages, each in its own try+log: readSourceImage / encodeThumbnail / uploadToStorage / persistThumbnailMetadata persistThumbnailMetadata emits the distinct "orphaned in storage as <key>" log line so an operator can see database-side failures after the upload completed. The deterministic key ensures the next run overwrites cleanly, so the orphan is self-healing. Also extracts THUMBNAIL_KEY_PREFIX/SUFFIX constants with a comment explaining the deterministic-overwrite contract. Adds test: generate_returnsFailed_whenPersistThrows_butUploadSucceeded. Refs #307 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -27,6 +27,7 @@ import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.time.LocalDateTime;
|
||||
import java.util.Set;
|
||||
import java.util.UUID;
|
||||
|
||||
/**
|
||||
* Generates JPEG thumbnail previews for documents (PDF first-page or scaled-down image)
|
||||
@@ -48,6 +49,13 @@ public class ThumbnailService {
|
||||
private static final Set<String> IMAGE_CONTENT_TYPES =
|
||||
Set.of("image/jpeg", "image/png", "image/tiff");
|
||||
|
||||
// Deterministic S3 key — `thumbnails/{docId}.jpg`. When a document's file is replaced
|
||||
// the regenerated thumbnail overwrites this same key via PutObject, so we never
|
||||
// orphan old thumbnails. The URL-level cache buster is the `thumbnail_generated_at`
|
||||
// timestamp (see /api/documents/{id}/thumbnail ?v= param).
|
||||
private static final String THUMBNAIL_KEY_PREFIX = "thumbnails/";
|
||||
private static final String THUMBNAIL_KEY_SUFFIX = ".jpg";
|
||||
|
||||
private final FileService fileService;
|
||||
private final S3Client s3Client;
|
||||
private final DocumentRepository documentRepository;
|
||||
@@ -74,21 +82,47 @@ public class ThumbnailService {
|
||||
return Outcome.SKIPPED;
|
||||
}
|
||||
|
||||
BufferedImage source;
|
||||
BufferedImage source = readSourceImage(doc, contentType);
|
||||
if (source == null) return Outcome.FAILED;
|
||||
|
||||
byte[] jpeg = encodeThumbnail(source, doc.getId());
|
||||
if (jpeg == null) return Outcome.FAILED;
|
||||
|
||||
String thumbnailKey = thumbnailKeyFor(doc.getId());
|
||||
if (!uploadToStorage(thumbnailKey, jpeg, doc.getId())) return Outcome.FAILED;
|
||||
|
||||
return persistThumbnailMetadata(doc, thumbnailKey);
|
||||
}
|
||||
|
||||
private static String thumbnailKeyFor(UUID documentId) {
|
||||
return THUMBNAIL_KEY_PREFIX + documentId + THUMBNAIL_KEY_SUFFIX;
|
||||
}
|
||||
|
||||
private BufferedImage readSourceImage(Document doc, String contentType) {
|
||||
try {
|
||||
source = PDF_CONTENT_TYPE.equals(contentType)
|
||||
return PDF_CONTENT_TYPE.equals(contentType)
|
||||
? renderPdfFirstPage(doc.getFilePath())
|
||||
: readImage(doc.getFilePath());
|
||||
} catch (Exception e) {
|
||||
log.warn("Thumbnail generation failed for doc={} reason={}",
|
||||
log.warn("Thumbnail source read failed for doc={} reason={}",
|
||||
doc.getId(), e.getMessage());
|
||||
return Outcome.FAILED;
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
private byte[] encodeThumbnail(BufferedImage source, UUID documentId) {
|
||||
try {
|
||||
BufferedImage scaled = scaleToWidth(source, THUMBNAIL_WIDTH);
|
||||
byte[] jpeg = encodeJpeg(scaled, JPEG_QUALITY);
|
||||
String thumbnailKey = "thumbnails/" + doc.getId() + ".jpg";
|
||||
return encodeJpeg(scaled, JPEG_QUALITY);
|
||||
} catch (Exception e) {
|
||||
log.warn("Thumbnail JPEG encoding failed for doc={} reason={}",
|
||||
documentId, e.getMessage());
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
private boolean uploadToStorage(String thumbnailKey, byte[] jpeg, UUID documentId) {
|
||||
try {
|
||||
s3Client.putObject(
|
||||
PutObjectRequest.builder()
|
||||
.bucket(bucketName)
|
||||
@@ -96,14 +130,28 @@ public class ThumbnailService {
|
||||
.contentType("image/jpeg")
|
||||
.build(),
|
||||
RequestBody.fromBytes(jpeg));
|
||||
return true;
|
||||
} catch (Exception e) {
|
||||
log.warn("Thumbnail upload failed for doc={} key={} reason={}",
|
||||
documentId, thumbnailKey, e.getMessage());
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
private Outcome persistThumbnailMetadata(Document doc, String thumbnailKey) {
|
||||
try {
|
||||
doc.setThumbnailKey(thumbnailKey);
|
||||
doc.setThumbnailGeneratedAt(LocalDateTime.now());
|
||||
documentRepository.save(doc);
|
||||
return Outcome.SUCCESS;
|
||||
} catch (Exception e) {
|
||||
log.warn("Thumbnail generation failed for doc={} reason={}",
|
||||
doc.getId(), e.getMessage());
|
||||
// Thumbnail is already in S3 but the entity update failed. Because the S3
|
||||
// key is deterministic (thumbnails/{docId}.jpg), the next successful run
|
||||
// — either a re-upload of this document or the admin backfill — will
|
||||
// overwrite it cleanly. Logging distinctly so an operator tracking
|
||||
// backfill totals can spot the database-side issue.
|
||||
log.warn("Thumbnail persist failed for doc={} (orphaned in storage as {}): {}",
|
||||
doc.getId(), thumbnailKey, e.getMessage());
|
||||
return Outcome.FAILED;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -167,6 +167,24 @@ class ThumbnailServiceTest {
|
||||
verify(documentRepository, never()).save(any());
|
||||
}
|
||||
|
||||
@Test
|
||||
void generate_returnsFailed_whenPersistThrows_butUploadSucceeded() throws IOException {
|
||||
// Covers the "orphan thumbnail" edge case: S3 upload succeeded but the
|
||||
// entity update blew up. We must still return FAILED so the backfill
|
||||
// tally is honest, without losing the fact that we already put bytes in S3.
|
||||
Document doc = makeDoc("application/pdf", "documents/letter.pdf");
|
||||
when(fileService.downloadFileStream(anyString()))
|
||||
.thenReturn(new ByteArrayInputStream(createSamplePdf()));
|
||||
when(documentRepository.save(any()))
|
||||
.thenThrow(new RuntimeException("constraint violation"));
|
||||
|
||||
ThumbnailService.Outcome outcome = thumbnailService.generate(doc);
|
||||
|
||||
assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED);
|
||||
verify(s3Client).putObject(any(PutObjectRequest.class), any(RequestBody.class));
|
||||
verify(documentRepository).save(any());
|
||||
}
|
||||
|
||||
// ─── helpers ──────────────────────────────────────────────────────────────
|
||||
|
||||
private Document makeDoc(String contentType, String filePath) {
|
||||
|
||||
Reference in New Issue
Block a user