diff --git a/src/main/java/org/example/videoclips/application/VideoAssetService.java b/src/main/java/org/example/videoclips/application/VideoAssetService.java index a2d7888..6cfdc67 100644 --- a/src/main/java/org/example/videoclips/application/VideoAssetService.java +++ b/src/main/java/org/example/videoclips/application/VideoAssetService.java @@ -321,7 +321,7 @@ public class VideoAssetService { Instant expiresAt = Instant.now().plus(properties.getDownloadUrlTtlMinutes(), ChronoUnit.MINUTES); return new SignedUrlResponse( clipId, - objectStoragePort.createClipDownloadUrl(clipId, expiresAt), + objectStoragePort.createClipDownloadUrl(clip.objectKey(), expiresAt), expiresAt.toString() ); } diff --git a/src/main/java/org/example/videoclips/application/VideoClippingRepository.java b/src/main/java/org/example/videoclips/application/VideoClippingRepository.java index 798a486..0b8fe98 100644 --- a/src/main/java/org/example/videoclips/application/VideoClippingRepository.java +++ b/src/main/java/org/example/videoclips/application/VideoClippingRepository.java @@ -32,6 +32,8 @@ public interface VideoClippingRepository { Clip findClipById(String clipId); + void deleteClip(String clipId); + List findExpiredAssets(Instant cutoff); List findExpiredClips(Instant cutoff); diff --git a/src/main/java/org/example/videoclips/infrastructure/InMemoryVideoAssetRepository.java b/src/main/java/org/example/videoclips/infrastructure/InMemoryVideoAssetRepository.java index 0559fd8..b275391 100644 --- a/src/main/java/org/example/videoclips/infrastructure/InMemoryVideoAssetRepository.java +++ b/src/main/java/org/example/videoclips/infrastructure/InMemoryVideoAssetRepository.java @@ -90,6 +90,11 @@ public class InMemoryVideoAssetRepository implements VideoClippingRepository { return clips.get(clipId); } + @Override + public void deleteClip(String clipId) { + clips.remove(clipId); + } + @Override public List findExpiredAssets(Instant cutoff) { List result = new ArrayList<>(); diff --git a/src/main/java/org/example/videoclips/operations/RetentionCleanupJob.java b/src/main/java/org/example/videoclips/operations/RetentionCleanupJob.java index 532c3fb..89453b1 100644 --- a/src/main/java/org/example/videoclips/operations/RetentionCleanupJob.java +++ b/src/main/java/org/example/videoclips/operations/RetentionCleanupJob.java @@ -4,6 +4,7 @@ import org.example.videoclips.application.VideoClippingRepository; import org.example.videoclips.config.VideoClippingProperties; import org.example.videoclips.domain.Clip; import org.example.videoclips.domain.VideoAsset; +import org.example.videoclips.storage.ObjectStoragePort; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; @@ -14,10 +15,16 @@ public class RetentionCleanupJob { private final VideoClippingRepository repository; private final VideoClippingProperties properties; + private final ObjectStoragePort objectStoragePort; - public RetentionCleanupJob(VideoClippingRepository repository, VideoClippingProperties properties) { + public RetentionCleanupJob( + VideoClippingRepository repository, + VideoClippingProperties properties, + ObjectStoragePort objectStoragePort + ) { this.repository = repository; this.properties = properties; + this.objectStoragePort = objectStoragePort; } @Scheduled(fixedDelayString = "${video-clipping.cleanup.retention-poll-interval-ms:3600000}") @@ -29,11 +36,12 @@ public class RetentionCleanupJob { Instant now = Instant.now(); for (Clip clip : repository.findExpiredClips(now)) { - // Current retention behavior is metadata-only soft expiration for the scaffold. - // Real object storage deletion should hang off the storage port once lifecycle policy is finalized. + objectStoragePort.deleteObject(clip.objectKey()); + repository.deleteClip(clip.id()); } for (VideoAsset asset : repository.findExpiredAssets(now)) { + objectStoragePort.deleteObject(asset.sourceObjectKey()); repository.saveAsset(asset.withDeleted()); } } diff --git a/src/main/java/org/example/videoclips/persistence/JpaVideoClippingRepository.java b/src/main/java/org/example/videoclips/persistence/JpaVideoClippingRepository.java index 0fe3482..9f2fb11 100644 --- a/src/main/java/org/example/videoclips/persistence/JpaVideoClippingRepository.java +++ b/src/main/java/org/example/videoclips/persistence/JpaVideoClippingRepository.java @@ -114,6 +114,11 @@ public class JpaVideoClippingRepository implements VideoClippingRepository { return clipJpaRepository.findById(clipId).map(mapper::toDomain).orElse(null); } + @Override + public void deleteClip(String clipId) { + clipJpaRepository.deleteById(clipId); + } + @Override public List findExpiredAssets(java.time.Instant cutoff) { return videoAssetJpaRepository.findByExpiresAtBeforeAndStatusNot(cutoff, "DELETED").stream() diff --git a/src/main/java/org/example/videoclips/storage/InMemoryObjectStorageAdapter.java b/src/main/java/org/example/videoclips/storage/InMemoryObjectStorageAdapter.java index e53cde7..5f4de02 100644 --- a/src/main/java/org/example/videoclips/storage/InMemoryObjectStorageAdapter.java +++ b/src/main/java/org/example/videoclips/storage/InMemoryObjectStorageAdapter.java @@ -53,7 +53,17 @@ public class InMemoryObjectStorageAdapter implements ObjectStoragePort { } @Override - public String createClipDownloadUrl(String clipId, Instant expiresAt) { - return "https://storage.example/clips/" + clipId + "?expiresAt=" + expiresAt.toString() + "&signature=demo"; + public void deleteObject(String objectKey) { + try { + Path targetPath = Path.of("tmp/in-memory-storage").resolve(objectKey); + Files.deleteIfExists(targetPath); + } catch (IOException ex) { + throw new IllegalStateException("Unable to delete object from in-memory storage", ex); + } + } + + @Override + public String createClipDownloadUrl(String objectKey, Instant expiresAt) { + return "https://storage.example/" + objectKey + "?expiresAt=" + expiresAt.toString() + "&signature=demo"; } } diff --git a/src/main/java/org/example/videoclips/storage/ObjectStoragePort.java b/src/main/java/org/example/videoclips/storage/ObjectStoragePort.java index a480062..2bde2d4 100644 --- a/src/main/java/org/example/videoclips/storage/ObjectStoragePort.java +++ b/src/main/java/org/example/videoclips/storage/ObjectStoragePort.java @@ -14,7 +14,9 @@ public interface ObjectStoragePort { void uploadGeneratedClip(Path sourcePath, String objectKey); - String createClipDownloadUrl(String clipId, Instant expiresAt); + void deleteObject(String objectKey); + + String createClipDownloadUrl(String objectKey, Instant expiresAt); record UploadPartDescriptor(int partNumber, String url) { } diff --git a/src/main/java/org/example/videoclips/storage/S3ObjectStorageAdapter.java b/src/main/java/org/example/videoclips/storage/S3ObjectStorageAdapter.java index 69fa2ab..9ab64f5 100644 --- a/src/main/java/org/example/videoclips/storage/S3ObjectStorageAdapter.java +++ b/src/main/java/org/example/videoclips/storage/S3ObjectStorageAdapter.java @@ -10,6 +10,7 @@ import software.amazon.awssdk.services.s3.S3Configuration; import software.amazon.awssdk.services.s3.model.CompleteMultipartUploadRequest; import software.amazon.awssdk.services.s3.model.CompletedMultipartUpload; import software.amazon.awssdk.services.s3.model.CompletedPart; +import software.amazon.awssdk.services.s3.model.DeleteObjectRequest; import software.amazon.awssdk.services.s3.model.GetObjectRequest; import software.amazon.awssdk.services.s3.model.PutObjectRequest; import software.amazon.awssdk.services.s3.model.UploadPartRequest; @@ -116,8 +117,15 @@ public class S3ObjectStorageAdapter implements ObjectStoragePort { } @Override - public String createClipDownloadUrl(String clipId, Instant expiresAt) { - String objectKey = "clips/" + clipId + ".mp4"; + public void deleteObject(String objectKey) { + s3Client.deleteObject(DeleteObjectRequest.builder() + .bucket(properties.getS3().getBucket()) + .key(objectKey) + .build()); + } + + @Override + public String createClipDownloadUrl(String objectKey, Instant expiresAt) { PresignedGetObjectRequest presignedRequest = s3Presigner.presignGetObject(GetObjectPresignRequest.builder() .signatureDuration(durationUntil(expiresAt)) .getObjectRequest(GetObjectRequest.builder() diff --git a/src/test/java/org/example/videoclips/api/VideoAssetControllerTest.java b/src/test/java/org/example/videoclips/api/VideoAssetControllerTest.java index 648c7b5..557fbc7 100644 --- a/src/test/java/org/example/videoclips/api/VideoAssetControllerTest.java +++ b/src/test/java/org/example/videoclips/api/VideoAssetControllerTest.java @@ -153,6 +153,87 @@ class VideoAssetControllerTest { .andExpect(jsonPath("$.type").value("https://api.example.com/problems/not-found")); } + @Test + void createsSignedDownloadUrlForGeneratedClip() throws Exception { + String assetResponse = mockMvc.perform(post("/v1/video-assets") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "fileName": "downloadable.mp4", + "contentType": "video/mp4", + "contentLengthBytes": 4096, + "checksumSha256": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + } + """)) + .andExpect(status().isCreated()) + .andReturn() + .getResponse() + .getContentAsString(); + + String assetId = JsonTestHelper.read(assetResponse, "$.assetId"); + String uploadId = JsonTestHelper.read(assetResponse, "$.upload.uploadId"); + + mockMvc.perform(post("/v1/video-assets/{assetId}/uploads:complete", assetId) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "uploadId": "%s", + "sourceDurationSeconds": 17, + "parts": [ + { + "partNumber": 1, + "etag": "\\"etag-1\\"" + } + ] + } + """.formatted(uploadId))) + .andExpect(status().isOk()); + + String jobResponse = mockMvc.perform(post("/v1/video-assets/{assetId}/clip-jobs", assetId) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "segmentDurationSeconds": 8, + "accuracyMode": "FAST", + "outputContainer": "mp4", + "overwriteExisting": false + } + """)) + .andExpect(status().isCreated()) + .andReturn() + .getResponse() + .getContentAsString(); + + String jobId = JsonTestHelper.read(jobResponse, "$.jobId"); + + for (int attempt = 0; attempt < 20; attempt++) { + String statusBody = mockMvc.perform(get("/v1/clip-jobs/{jobId}", jobId)) + .andExpect(status().isOk()) + .andReturn() + .getResponse() + .getContentAsString(); + if ("SUCCEEDED".equals(JsonTestHelper.read(statusBody, "$.status"))) { + break; + } + Thread.sleep(50L); + } + + String clipsResponse = mockMvc.perform(get("/v1/clip-jobs/{jobId}/clips", jobId)) + .andExpect(status().isOk()) + .andReturn() + .getResponse() + .getContentAsString(); + + String clipId = JsonTestHelper.read(clipsResponse, "$.clips[0].clipId"); + String objectKey = JsonTestHelper.read(clipsResponse, "$.clips[0].objectKey"); + + mockMvc.perform(post("/v1/clips/{clipId}/download-url", clipId) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.clipId").value(clipId)) + .andExpect(jsonPath("$.url").value(org.hamcrest.Matchers.containsString(objectKey))); + } + @Test void rejectsOversizedAssetRequest() throws Exception { mockMvc.perform(post("/v1/video-assets") diff --git a/src/test/java/org/example/videoclips/operations/RetentionCleanupJobTest.java b/src/test/java/org/example/videoclips/operations/RetentionCleanupJobTest.java new file mode 100644 index 0000000..e394832 --- /dev/null +++ b/src/test/java/org/example/videoclips/operations/RetentionCleanupJobTest.java @@ -0,0 +1,84 @@ +package org.example.videoclips.operations; + +import org.example.videoclips.application.VideoClippingRepository; +import org.example.videoclips.config.VideoClippingProperties; +import org.example.videoclips.domain.Clip; +import org.example.videoclips.domain.VideoAsset; +import org.example.videoclips.storage.ObjectStoragePort; +import org.junit.jupiter.api.Test; + +import java.time.Instant; +import java.util.List; + +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class RetentionCleanupJobTest { + + @Test + void deletesExpiredClipObjectsAndMarksExpiredAssetsDeleted() { + VideoClippingRepository repository = mock(VideoClippingRepository.class); + ObjectStoragePort objectStoragePort = mock(ObjectStoragePort.class); + VideoClippingProperties properties = new VideoClippingProperties(); + + RetentionCleanupJob job = new RetentionCleanupJob(repository, properties, objectStoragePort); + Instant expiredAt = Instant.now().minusSeconds(60); + Clip clip = new Clip( + "clip_1", + "job_1", + "asset_1", + "clips/job_1/0.mp4", + 0, + 0, + 8, + "video/mp4", + 1024, + expiredAt, + expiredAt + ); + VideoAsset asset = new VideoAsset( + "asset_1", + "source.mp4", + "video/mp4", + 4096, + null, + "mp4-h264-aac", + "uploads/upl_1/source", + VideoAsset.Status.READY, + "upl_1", + 17L, + expiredAt, + expiredAt, + expiredAt + ); + + when(repository.findExpiredClips(org.mockito.ArgumentMatchers.any())).thenReturn(List.of(clip)); + when(repository.findExpiredAssets(org.mockito.ArgumentMatchers.any())).thenReturn(List.of(asset)); + + job.cleanupExpiredRecords(); + + verify(objectStoragePort).deleteObject("clips/job_1/0.mp4"); + verify(repository).deleteClip("clip_1"); + verify(objectStoragePort).deleteObject("uploads/upl_1/source"); + verify(repository).saveAsset(argThat(saved -> saved.id().equals("asset_1") + && saved.status() == VideoAsset.Status.DELETED)); + } + + @Test + void skipsCleanupWhenDisabled() { + VideoClippingRepository repository = mock(VideoClippingRepository.class); + ObjectStoragePort objectStoragePort = mock(ObjectStoragePort.class); + VideoClippingProperties properties = new VideoClippingProperties(); + properties.getCleanup().setEnabled(false); + + RetentionCleanupJob job = new RetentionCleanupJob(repository, properties, objectStoragePort); + + job.cleanupExpiredRecords(); + + verify(repository, never()).findExpiredClips(org.mockito.ArgumentMatchers.any()); + verify(repository, never()).findExpiredAssets(org.mockito.ArgumentMatchers.any()); + } +} diff --git a/tmp/in-memory-storage/clips/job_de0b3d0c1d8a/0.mp4 b/tmp/in-memory-storage/clips/job_de0b3d0c1d8a/0.mp4 new file mode 100644 index 0000000..fe5a375 --- /dev/null +++ b/tmp/in-memory-storage/clips/job_de0b3d0c1d8a/0.mp4 @@ -0,0 +1 @@ +stub-clip-content-clip_a58ccb424fa0 \ No newline at end of file diff --git a/tmp/in-memory-storage/clips/job_de0b3d0c1d8a/1.mp4 b/tmp/in-memory-storage/clips/job_de0b3d0c1d8a/1.mp4 new file mode 100644 index 0000000..8210c38 --- /dev/null +++ b/tmp/in-memory-storage/clips/job_de0b3d0c1d8a/1.mp4 @@ -0,0 +1 @@ +stub-clip-content-clip_59a1c0bee8ed \ No newline at end of file diff --git a/tmp/in-memory-storage/clips/job_de0b3d0c1d8a/2.mp4 b/tmp/in-memory-storage/clips/job_de0b3d0c1d8a/2.mp4 new file mode 100644 index 0000000..fb9dd54 --- /dev/null +++ b/tmp/in-memory-storage/clips/job_de0b3d0c1d8a/2.mp4 @@ -0,0 +1 @@ +stub-clip-content-clip_aeaf1155bc22 \ No newline at end of file