From 940bf18aba733087155838f67070dbc714942d8b Mon Sep 17 00:00:00 2001 From: hsiegeln <37154749+hsiegeln@users.noreply.github.com> Date: Mon, 27 Apr 2026 15:47:37 +0200 Subject: [PATCH] refactor(web): authoritative Content-Length, typed Optional in controller --- .../server/app/security/SecurityBeanConfig.java | 6 ++---- .../app/storage/FilesystemArtifactStore.java | 5 +++++ .../app/web/ArtifactDownloadController.java | 15 +++++++++------ .../app/storage/FilesystemArtifactStoreTest.java | 9 +++++++++ .../app/web/ArtifactDownloadControllerTest.java | 8 +++++--- .../cameleer/server/core/runtime/AppService.java | 8 ++++++++ .../server/core/storage/ArtifactStore.java | 5 +++++ 7 files changed, 43 insertions(+), 13 deletions(-) diff --git a/cameleer-server-app/src/main/java/com/cameleer/server/app/security/SecurityBeanConfig.java b/cameleer-server-app/src/main/java/com/cameleer/server/app/security/SecurityBeanConfig.java index c4cb96a0..b5247114 100644 --- a/cameleer-server-app/src/main/java/com/cameleer/server/app/security/SecurityBeanConfig.java +++ b/cameleer-server-app/src/main/java/com/cameleer/server/app/security/SecurityBeanConfig.java @@ -2,7 +2,6 @@ package com.cameleer.server.app.security; import com.cameleer.server.app.web.ArtifactDownloadTokenSigner; import org.springframework.beans.factory.InitializingBean; -import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -39,9 +38,8 @@ public class SecurityBeanConfig { } @Bean - public ArtifactDownloadTokenSigner artifactDownloadTokenSigner( - @Value("${cameleer.server.security.jwtsecret}") String jwtSecret) { - return new ArtifactDownloadTokenSigner(jwtSecret, Clock.systemUTC()); + public ArtifactDownloadTokenSigner artifactDownloadTokenSigner(SecurityProperties properties) { + return new ArtifactDownloadTokenSigner(properties.getJwtSecret(), Clock.systemUTC()); } @Bean diff --git a/cameleer-server-app/src/main/java/com/cameleer/server/app/storage/FilesystemArtifactStore.java b/cameleer-server-app/src/main/java/com/cameleer/server/app/storage/FilesystemArtifactStore.java index b3a0ce1a..ab5ac074 100644 --- a/cameleer-server-app/src/main/java/com/cameleer/server/app/storage/FilesystemArtifactStore.java +++ b/cameleer-server-app/src/main/java/com/cameleer/server/app/storage/FilesystemArtifactStore.java @@ -43,6 +43,11 @@ public class FilesystemArtifactStore implements ArtifactStore { return Files.newInputStream(pathOf(coords)); } + @Override + public long size(ArtifactCoordinates coords) throws IOException { + return Files.size(pathOf(coords)); + } + @Override public boolean exists(ArtifactCoordinates coords) { return Files.exists(pathOf(coords)); diff --git a/cameleer-server-app/src/main/java/com/cameleer/server/app/web/ArtifactDownloadController.java b/cameleer-server-app/src/main/java/com/cameleer/server/app/web/ArtifactDownloadController.java index cae458c1..3bb27f9b 100644 --- a/cameleer-server-app/src/main/java/com/cameleer/server/app/web/ArtifactDownloadController.java +++ b/cameleer-server-app/src/main/java/com/cameleer/server/app/web/ArtifactDownloadController.java @@ -2,6 +2,7 @@ package com.cameleer.server.app.web; import com.cameleer.server.core.runtime.AppService; import com.cameleer.server.core.runtime.AppVersion; +import com.cameleer.server.core.storage.ArtifactCoordinates; import com.cameleer.server.core.storage.ArtifactStore; import org.springframework.core.io.InputStreamResource; import org.springframework.http.MediaType; @@ -14,6 +15,7 @@ import org.springframework.web.bind.annotation.RestController; import java.io.IOException; import java.io.InputStream; +import java.util.Optional; import java.util.UUID; /** @@ -44,16 +46,17 @@ public class ArtifactDownloadController { if (!signer.verify(appVersionId, exp, sig)) { return ResponseEntity.status(401).build(); } - AppVersion version; - try { - version = appService.getVersion(appVersionId); - } catch (IllegalArgumentException e) { + Optional versionOpt = appService.findVersion(appVersionId); + if (versionOpt.isEmpty()) { return ResponseEntity.notFound().build(); } - InputStream in = artifactStore.get(appService.coordinatesFor(version)); + AppVersion version = versionOpt.get(); + ArtifactCoordinates coords = appService.coordinatesFor(version); + long size = artifactStore.size(coords); + InputStream in = artifactStore.get(coords); return ResponseEntity.ok() .contentType(MediaType.parseMediaType("application/java-archive")) - .contentLength(version.jarSizeBytes()) + .contentLength(size) .body(new InputStreamResource(in)); } } diff --git a/cameleer-server-app/src/test/java/com/cameleer/server/app/storage/FilesystemArtifactStoreTest.java b/cameleer-server-app/src/test/java/com/cameleer/server/app/storage/FilesystemArtifactStoreTest.java index 250eb9e6..abc22ecc 100644 --- a/cameleer-server-app/src/test/java/com/cameleer/server/app/storage/FilesystemArtifactStoreTest.java +++ b/cameleer-server-app/src/test/java/com/cameleer/server/app/storage/FilesystemArtifactStoreTest.java @@ -72,6 +72,15 @@ class FilesystemArtifactStoreTest { } } + @Test + void sizeReturnsActualOnDiskBytes(@TempDir Path tmp) throws Exception { + FilesystemArtifactStore store = new FilesystemArtifactStore(tmp.toString()); + ArtifactCoordinates c = new ArtifactCoordinates("default", UUID.randomUUID(), 1); + byte[] payload = new byte[1234]; + store.put(c, new java.io.ByteArrayInputStream(payload), payload.length); + assertThat(store.size(c)).isEqualTo(1234L); + } + @Test void deleteLeavesAppDirAloneWhenSiblingVersionExists(@TempDir Path tmp) throws Exception { FilesystemArtifactStore store = new FilesystemArtifactStore(tmp.toString()); diff --git a/cameleer-server-app/src/test/java/com/cameleer/server/app/web/ArtifactDownloadControllerTest.java b/cameleer-server-app/src/test/java/com/cameleer/server/app/web/ArtifactDownloadControllerTest.java index 5f69d19b..d55628e2 100644 --- a/cameleer-server-app/src/test/java/com/cameleer/server/app/web/ArtifactDownloadControllerTest.java +++ b/cameleer-server-app/src/test/java/com/cameleer/server/app/web/ArtifactDownloadControllerTest.java @@ -10,6 +10,7 @@ import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.setup.MockMvcBuilders; import java.io.ByteArrayInputStream; +import java.util.Optional; import java.util.UUID; import static org.mockito.ArgumentMatchers.any; @@ -54,8 +55,9 @@ class ArtifactDownloadControllerTest { UUID id = UUID.randomUUID(); when(signer.verify(eq(id), eq(123L), eq("good"))).thenReturn(true); AppVersion v = new AppVersion(id, UUID.randomUUID(), 1, "loc", "h", "a.jar", 5L, null, null, null); - when(appService.getVersion(id)).thenReturn(v); + when(appService.findVersion(id)).thenReturn(Optional.of(v)); when(appService.coordinatesFor(v)).thenReturn(new ArtifactCoordinates("default", v.appId(), 1)); + when(artifactStore.size(any())).thenReturn(5L); when(artifactStore.get(any())).thenReturn(new ByteArrayInputStream("hello".getBytes())); mvc.perform(get("/api/v1/artifacts/{id}", id).param("exp", "123").param("sig", "good")) @@ -73,14 +75,14 @@ class ArtifactDownloadControllerTest { .andExpect(status().isUnauthorized()); // Defence-in-depth: bad sig must short-circuit before any data lookup. - verify(appService, never()).getVersion(any()); + verify(appService, never()).findVersion(any()); } @Test void returns404WhenArtifactMissing() throws Exception { UUID id = UUID.randomUUID(); when(signer.verify(any(), anyLong(), any())).thenReturn(true); - when(appService.getVersion(id)).thenThrow(new IllegalArgumentException("AppVersion not found")); + when(appService.findVersion(id)).thenReturn(Optional.empty()); mvc.perform(get("/api/v1/artifacts/{id}", id).param("exp", "123").param("sig", "ok")) .andExpect(status().isNotFound()); diff --git a/cameleer-server-core/src/main/java/com/cameleer/server/core/runtime/AppService.java b/cameleer-server-core/src/main/java/com/cameleer/server/core/runtime/AppService.java index ad99ddad..4426eff8 100644 --- a/cameleer-server-core/src/main/java/com/cameleer/server/core/runtime/AppService.java +++ b/cameleer-server-core/src/main/java/com/cameleer/server/core/runtime/AppService.java @@ -13,6 +13,7 @@ import java.security.MessageDigest; import java.util.HexFormat; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.UUID; import java.util.regex.Pattern; @@ -57,6 +58,13 @@ public class AppService { .orElseThrow(() -> new IllegalArgumentException("AppVersion not found: " + versionId)); } + /** Typed Optional accessor for callers that distinguish missing-rows from + * other failures without relying on the message-based exception convention + * of {@link #getVersion(UUID)}. */ + public Optional findVersion(UUID versionId) { + return versionRepo.findById(versionId); + } + /** Coordinate corresponding to an AppVersion. Used by callers that need to * pass the artifact identity to {@link ArtifactStore} (download, delete, exists). */ public ArtifactCoordinates coordinatesFor(AppVersion version) { diff --git a/cameleer-server-core/src/main/java/com/cameleer/server/core/storage/ArtifactStore.java b/cameleer-server-core/src/main/java/com/cameleer/server/core/storage/ArtifactStore.java index b25c4ba2..54b80aac 100644 --- a/cameleer-server-core/src/main/java/com/cameleer/server/core/storage/ArtifactStore.java +++ b/cameleer-server-core/src/main/java/com/cameleer/server/core/storage/ArtifactStore.java @@ -22,6 +22,11 @@ public interface ArtifactStore { /** Open the artifact for reading. Caller closes. */ InputStream get(ArtifactCoordinates coords) throws IOException; + /** Size of the stored artifact in bytes. Authoritative — read from the backend + * at serve time so a stale {@code AppVersion.jarSizeBytes} cannot silently + * truncate downloads. */ + long size(ArtifactCoordinates coords) throws IOException; + /** True if an artifact is currently stored under {@code coords}. */ boolean exists(ArtifactCoordinates coords);