I changed the signed clip download flow so it uses the clip’s persisted objectKey instead of deriving a fake path from clipId. The fix is in src/main/java/org/example/videoclips/application/
VideoAssetService.java, src/main/java/org/example/videoclips/storage/ObjectStoragePort.java, src/main/java/org/example/videoclips/storage/InMemoryObjectStorageAdapter.java, and src/main/java/org/ example/videoclips/storage/S3ObjectStorageAdapter.java. I also added focused coverage for this slice: - API test for generating a signed URL for an actual produced clip in src/test/java/org/example/videoclips/api/VideoAssetControllerTest.java - cleanup job tests for expired clip/object deletion and disabled cleanup in src/test/java/org/example/videoclips/operations/RetentionCleanupJobTest.java
This commit is contained in:
parent
8cb232d70e
commit
73070824df
|
|
@ -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()
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -32,6 +32,8 @@ public interface VideoClippingRepository {
|
|||
|
||||
Clip findClipById(String clipId);
|
||||
|
||||
void deleteClip(String clipId);
|
||||
|
||||
List<VideoAsset> findExpiredAssets(Instant cutoff);
|
||||
|
||||
List<Clip> findExpiredClips(Instant cutoff);
|
||||
|
|
|
|||
|
|
@ -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<VideoAsset> findExpiredAssets(Instant cutoff) {
|
||||
List<VideoAsset> result = new ArrayList<>();
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<VideoAsset> findExpiredAssets(java.time.Instant cutoff) {
|
||||
return videoAssetJpaRepository.findByExpiresAtBeforeAndStatusNot(cutoff, "DELETED").stream()
|
||||
|
|
|
|||
|
|
@ -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";
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1 @@
|
|||
stub-clip-content-clip_a58ccb424fa0
|
||||
|
|
@ -0,0 +1 @@
|
|||
stub-clip-content-clip_59a1c0bee8ed
|
||||
|
|
@ -0,0 +1 @@
|
|||
stub-clip-content-clip_aeaf1155bc22
|
||||
Loading…
Reference in New Issue