feat: harden download and storage resilience

This commit is contained in:
vsxd 2026-03-18 14:17:22 +08:00 • committed by Xudong Sun
parent 4aa3d29599
commit 377de504f9
22 changed files with 389 additions and 75 deletions

View file

@ -16,6 +16,7 @@ import com.iflytek.skillhub.dto.SkillFileResponse;
import com.iflytek.skillhub.dto.SkillLifecycleVersionResponse;
import com.iflytek.skillhub.dto.SkillVersionDetailResponse;
import com.iflytek.skillhub.dto.SkillVersionResponse;
import com.iflytek.skillhub.metrics.SkillHubMetrics;
import com.iflytek.skillhub.ratelimit.RateLimit;
import org.springframework.core.io.InputStreamResource;
import org.springframework.data.domain.Page;
@ -39,14 +40,17 @@ public class SkillController extends BaseApiController {
private final SkillQueryService skillQueryService;
private final SkillDownloadService skillDownloadService;
private final SkillHubMetrics metrics;
public SkillController(
SkillQueryService skillQueryService,
SkillDownloadService skillDownloadService,
SkillHubMetrics metrics,
ApiResponseFactory responseFactory) {
super(responseFactory);
this.skillQueryService = skillQueryService;
this.skillDownloadService = skillDownloadService;
this.metrics = metrics;
}
@GetMapping("/{namespace}/{slug}")
@ -332,16 +336,24 @@ public class SkillController extends BaseApiController {
private ResponseEntity<InputStreamResource> buildDownloadResponse(HttpServletRequest request, SkillDownloadService.DownloadResult result) {
if (shouldRedirectToPresignedUrl(request, result.presignedUrl())) {
metrics.recordDownloadDelivery("redirect", result.fallbackBundle());
if (result.fallbackBundle()) {
metrics.incrementBundleMissingFallback();
}
return ResponseEntity.status(HttpStatus.FOUND)
.header(HttpHeaders.LOCATION, result.presignedUrl())
.build();
}
metrics.recordDownloadDelivery("stream", result.fallbackBundle());
if (result.fallbackBundle()) {
metrics.incrementBundleMissingFallback();
}
return ResponseEntity.ok()
.header(HttpHeaders.CONTENT_DISPOSITION, "attachment; filename=\"" + result.filename() + "\"")
.contentType(MediaType.parseMediaType(result.contentType()))
.contentLength(result.contentLength())
.body(new InputStreamResource(result.content()));
.body(new InputStreamResource(result.openContent()));
}
private boolean shouldRedirectToPresignedUrl(HttpServletRequest request, String presignedUrl) {

View file

@ -7,7 +7,9 @@ import com.iflytek.skillhub.dto.ApiResponseFactory;
import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException;
import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException;
import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException;
import com.iflytek.skillhub.metrics.SkillHubMetrics;
import com.iflytek.skillhub.security.SensitiveLogSanitizer;
import com.iflytek.skillhub.storage.StorageAccessException;
import jakarta.servlet.http.HttpServletRequest;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -27,11 +29,14 @@ public class GlobalExceptionHandler {
private static final Logger logger = LoggerFactory.getLogger(GlobalExceptionHandler.class);
private final ApiResponseFactory apiResponseFactory;
private final SensitiveLogSanitizer sensitiveLogSanitizer;
private final SkillHubMetrics metrics;
public GlobalExceptionHandler(ApiResponseFactory apiResponseFactory,
SensitiveLogSanitizer sensitiveLogSanitizer) {
SensitiveLogSanitizer sensitiveLogSanitizer,
SkillHubMetrics metrics) {
this.apiResponseFactory = apiResponseFactory;
this.sensitiveLogSanitizer = sensitiveLogSanitizer;
this.metrics = metrics;
}
@ExceptionHandler(LocalizedException.class)
@ -108,6 +113,23 @@ public class GlobalExceptionHandler {
apiResponseFactory.error(403, "error.forbidden"));
}
@ExceptionHandler(StorageAccessException.class)
public ResponseEntity<ApiResponse<Void>> handleStorageAccess(StorageAccessException ex, HttpServletRequest request) {
metrics.incrementStorageAccessFailure(ex.getOperation());
logger.warn(
"Object storage unavailable [requestId={}, method={}, path={}, userId={}, operation={}, key={}]",
MDC.get("requestId"),
request.getMethod(),
sensitiveLogSanitizer.sanitizeRequestTarget(request),
resolveUserId(request),
ex.getOperation(),
ex.getKey(),
ex
);
return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE).body(
apiResponseFactory.error(503, "error.storage.unavailable"));
}
@ExceptionHandler(Exception.class)
public ResponseEntity<ApiResponse<Void>> handleGlobalException(Exception ex, HttpServletRequest request) {
logger.error(

View file

@ -31,4 +31,30 @@ public class SkillHubMetrics {
"status", status
).increment();
}
public void recordDownloadDelivery(String mode, boolean fallbackBundle) {
meterRegistry.counter(
"skillhub.skill.download.delivery",
"mode", mode,
"fallback_bundle", Boolean.toString(fallbackBundle)
).increment();
}
public void incrementBundleMissingFallback() {
meterRegistry.counter("skillhub.skill.download.bundle_missing_fallback").increment();
}
public void incrementRateLimitExceeded(String category) {
meterRegistry.counter(
"skillhub.ratelimit.exceeded",
"category", category
).increment();
}
public void incrementStorageAccessFailure(String operation) {
meterRegistry.counter(
"skillhub.storage.failure",
"operation", operation
).increment();
}
}

View file

@ -3,6 +3,7 @@ package com.iflytek.skillhub.ratelimit;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.iflytek.skillhub.dto.ApiResponse;
import com.iflytek.skillhub.dto.ApiResponseFactory;
import com.iflytek.skillhub.metrics.SkillHubMetrics;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.springframework.http.HttpStatus;
@ -10,6 +11,9 @@ import org.springframework.http.MediaType;
import org.springframework.stereotype.Component;
import org.springframework.web.method.HandlerMethod;
import org.springframework.web.servlet.HandlerInterceptor;
import org.springframework.web.servlet.HandlerMapping;
import java.util.Map;
@Component
public class RateLimitInterceptor implements HandlerInterceptor {
@ -19,17 +23,20 @@ public class RateLimitInterceptor implements HandlerInterceptor {
private final AnonymousDownloadIdentityService anonymousDownloadIdentityService;
private final ApiResponseFactory apiResponseFactory;
private final ObjectMapper objectMapper;
private final SkillHubMetrics metrics;
public RateLimitInterceptor(RateLimiter rateLimiter,
ClientIpResolver clientIpResolver,
AnonymousDownloadIdentityService anonymousDownloadIdentityService,
ApiResponseFactory apiResponseFactory,
ObjectMapper objectMapper) {
ObjectMapper objectMapper,
SkillHubMetrics metrics) {
this.rateLimiter = rateLimiter;
this.clientIpResolver = clientIpResolver;
this.anonymousDownloadIdentityService = anonymousDownloadIdentityService;
this.apiResponseFactory = apiResponseFactory;
this.objectMapper = objectMapper;
this.metrics = metrics;
}
@Override
@ -51,12 +58,17 @@ public class RateLimitInterceptor implements HandlerInterceptor {
// Get limit based on authentication status
int limit = isAuthenticated ? rateLimit.authenticated() : rateLimit.anonymous();
String resourceSuffix = resolveResourceSuffix(rateLimit.category(), request);
boolean allowed = isAuthenticated
? rateLimiter.tryAcquire("ratelimit:" + rateLimit.category() + ":user:" + userId, limit, rateLimit.windowSeconds())
: checkAnonymousLimit(request, response, rateLimit, limit);
? rateLimiter.tryAcquire(
"ratelimit:" + rateLimit.category() + ":user:" + userId + resourceSuffix,
limit,
rateLimit.windowSeconds())
: checkAnonymousLimit(request, response, rateLimit, limit, resourceSuffix);
if (!allowed) {
metrics.incrementRateLimitExceeded(rateLimit.category());
response.setStatus(HttpStatus.TOO_MANY_REQUESTS.value());
response.setContentType(MediaType.APPLICATION_JSON_VALUE);
ApiResponse<Void> body = apiResponseFactory.error(429, "error.rateLimit.exceeded");
@ -70,10 +82,11 @@ public class RateLimitInterceptor implements HandlerInterceptor {
private boolean checkAnonymousLimit(HttpServletRequest request,
HttpServletResponse response,
RateLimit rateLimit,
int limit) {
int limit,
String resourceSuffix) {
if (!"download".equals(rateLimit.category())) {
return rateLimiter.tryAcquire(
"ratelimit:" + rateLimit.category() + ":ip:" + clientIpResolver.resolve(request),
"ratelimit:" + rateLimit.category() + ":ip:" + clientIpResolver.resolve(request) + resourceSuffix,
limit,
rateLimit.windowSeconds()
);
@ -82,7 +95,7 @@ public class RateLimitInterceptor implements HandlerInterceptor {
AnonymousDownloadIdentityService.AnonymousDownloadIdentity identity =
anonymousDownloadIdentityService.resolve(request, response);
boolean ipAllowed = rateLimiter.tryAcquire(
"ratelimit:download:ip:" + identity.ipHash(),
"ratelimit:download:ip:" + identity.ipHash() + resourceSuffix,
limit,
rateLimit.windowSeconds()
);
@ -90,9 +103,37 @@ public class RateLimitInterceptor implements HandlerInterceptor {
return false;
}
return rateLimiter.tryAcquire(
"ratelimit:download:anon:" + identity.cookieHash(),
"ratelimit:download:anon:" + identity.cookieHash() + resourceSuffix,
limit,
rateLimit.windowSeconds()
);
}
@SuppressWarnings("unchecked")
private String resolveResourceSuffix(String category, HttpServletRequest request) {
if (!"download".equals(category)) {
return "";
}
Object attribute = request.getAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE);
if (!(attribute instanceof Map<?, ?> templateVariables)) {
return "";
}
String namespace = stringValue(templateVariables.get("namespace"));
String slug = stringValue(templateVariables.get("slug"));
String version = stringValue(templateVariables.get("version"));
String tagName = stringValue(templateVariables.get("tagName"));
if (namespace == null || slug == null) {
return "";
}
String target = version != null ? "version:" + version : tagName != null ? "tag:" + tagName : "latest";
return ":ns:" + namespace + ":slug:" + slug + ":" + target;
}
private String stringValue(Object value) {
if (value == null) {
return null;
}
String text = value.toString();
return text.isBlank() ? null : text;
}
}

View file

@ -84,6 +84,10 @@ skillhub:
force-path-style: ${SKILLHUB_STORAGE_S3_FORCE_PATH_STYLE:true}
auto-create-bucket: ${SKILLHUB_STORAGE_S3_AUTO_CREATE_BUCKET:false}
presign-expiry: ${SKILLHUB_STORAGE_S3_PRESIGN_EXPIRY:PT10M}
max-connections: ${SKILLHUB_STORAGE_S3_MAX_CONNECTIONS:100}
connection-acquisition-timeout: ${SKILLHUB_STORAGE_S3_CONNECTION_ACQUISITION_TIMEOUT:PT2S}
api-call-attempt-timeout: ${SKILLHUB_STORAGE_S3_API_CALL_ATTEMPT_TIMEOUT:PT10S}
api-call-timeout: ${SKILLHUB_STORAGE_S3_API_CALL_TIMEOUT:PT30S}
search:
engine: postgres
rebuild-on-startup: false

View file

@ -0,0 +1,10 @@
ALTER TABLE skill_version
ADD COLUMN bundle_ready BOOLEAN NOT NULL DEFAULT FALSE,
ADD COLUMN download_ready BOOLEAN NOT NULL DEFAULT FALSE;
UPDATE skill_version
SET download_ready = CASE
WHEN status = 'PUBLISHED' AND file_count > 0 THEN TRUE
ELSE FALSE
END,
bundle_ready = FALSE;

View file

@ -48,6 +48,7 @@ error.auth.sessionBootstrap.notAuthenticated=No authenticated external session f
error.badRequest=Invalid request
error.forbidden=Forbidden
error.rateLimit.exceeded=Rate limit exceeded
error.storage.unavailable=Object storage is temporarily unavailable. Please try again later.
error.internal=An unexpected error occurred
error.slug.blank=Slug cannot be blank
error.slug.length=Slug length must be between {0} and {1} characters

View file

@ -48,6 +48,7 @@ error.auth.sessionBootstrap.notAuthenticated=未检测到已认证的外部会
error.badRequest=请求参数不合法
error.forbidden=没有权限执行该操作
error.rateLimit.exceeded=请求过于频繁,请稍后再试
error.storage.unavailable=对象存储暂时不可用,请稍后再试
error.internal=服务器内部错误
error.slug.blank=slug 不能为空
error.slug.length=slug 长度必须在 {0} 到 {1} 个字符之间

View file

@ -8,6 +8,7 @@ import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
@ -60,15 +61,17 @@ class DownloadRateLimitControllerTest {
given(rateLimiter.tryAcquire(anyString(), anyInt(), anyInt())).willReturn(true);
given(skillDownloadService.downloadVersion("global", "demo-skill", "1.0.0", null, Map.of()))
.willReturn(new SkillDownloadService.DownloadResult(
new ByteArrayInputStream("zip".getBytes()),
() -> new ByteArrayInputStream("zip".getBytes()),
"demo-skill-1.0.0.zip",
3L,
"application/zip",
null
null,
false
));
var result = mockMvc.perform(get("/api/v1/skills/global/demo-skill/versions/1.0.0/download")
.header("X-Forwarded-For", "203.0.113.10"))
.header("X-Forwarded-For", "203.0.113.10")
.with(user("anonymous-test")))
.andExpect(status().isOk())
.andExpect(header().string("Content-Disposition", "attachment; filename=\"demo-skill-1.0.0.zip\""))
.andExpect(header().exists("Set-Cookie"))
@ -83,8 +86,8 @@ class DownloadRateLimitControllerTest {
ArgumentCaptor<String> keyCaptor = ArgumentCaptor.forClass(String.class);
verify(rateLimiter, times(2)).tryAcquire(keyCaptor.capture(), anyInt(), anyInt());
assertThat(keyCaptor.getAllValues()).anyMatch(key -> key.startsWith("ratelimit:download:ip:"));
assertThat(keyCaptor.getAllValues()).anyMatch(key -> key.startsWith("ratelimit:download:anon:"));
assertThat(keyCaptor.getAllValues()).anyMatch(key -> key.startsWith("ratelimit:download:ip:") && key.endsWith(":ns:global:slug:demo-skill:version:1.0.0"));
assertThat(keyCaptor.getAllValues()).anyMatch(key -> key.startsWith("ratelimit:download:anon:") && key.endsWith(":ns:global:slug:demo-skill:version:1.0.0"));
}
@Test
@ -93,7 +96,8 @@ class DownloadRateLimitControllerTest {
((String) invocation.getArgument(0)).startsWith("ratelimit:download:ip:") ? false : true);
mockMvc.perform(get("/api/v1/skills/global/demo-skill/versions/1.0.0/download")
.header("X-Forwarded-For", "203.0.113.10"))
.header("X-Forwarded-For", "203.0.113.10")
.with(user("anonymous-test")))
.andExpect(status().isTooManyRequests())
.andExpect(jsonPath("$.code").value(429));

View file

@ -13,6 +13,8 @@ import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository;
import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException;
import com.iflytek.skillhub.domain.skill.service.SkillDownloadService;
import com.iflytek.skillhub.domain.skill.service.SkillQueryService;
import com.iflytek.skillhub.metrics.SkillHubMetrics;
import com.iflytek.skillhub.ratelimit.RateLimiter;
import java.io.ByteArrayInputStream;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
@ -23,6 +25,10 @@ import org.springframework.context.annotation.Import;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.web.servlet.MockMvc;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.verify;
@SpringBootTest
@AutoConfigureMockMvc
@ActiveProfiles("test")
@ -44,19 +50,28 @@ class SkillControllerDownloadTest {
@MockBean
private DeviceAuthService deviceAuthService;
@MockBean
private SkillHubMetrics skillHubMetrics;
@MockBean
private RateLimiter rateLimiter;
@Test
void downloadVersion_redirectsToPresignedUrlWhenAvailable() throws Exception {
given(skillDownloadService.downloadVersion("global", "demo-skill", "1.0.0", null, java.util.Map.of()))
given(rateLimiter.tryAcquire(anyString(), anyInt(), anyInt())).willReturn(true);
given(skillDownloadService.downloadVersion("global", "demo-skill", "1.0.0", "test-user", java.util.Map.of()))
.willReturn(new SkillDownloadService.DownloadResult(
new ByteArrayInputStream("zip".getBytes()),
() -> new ByteArrayInputStream("zip".getBytes()),
"demo-skill-1.0.0.zip",
128L,
"application/zip",
"https://download.example/presigned"
"https://download.example/presigned",
false
));
mockMvc.perform(get("/api/v1/skills/global/demo-skill/versions/1.0.0/download")
.with(user("test-user"))
.requestAttr("userId", "test-user")
.with(csrf()))
.andExpect(status().isFound())
.andExpect(header().string("Location", "https://download.example/presigned"));
@ -64,18 +79,21 @@ class SkillControllerDownloadTest {
@Test
void downloadVersion_streamsWhenPresignedUrlIsInsecureForHttpsRequest() throws Exception {
given(skillDownloadService.downloadVersion("global", "demo-skill", "1.0.0", null, java.util.Map.of()))
given(rateLimiter.tryAcquire(anyString(), anyInt(), anyInt())).willReturn(true);
given(skillDownloadService.downloadVersion("global", "demo-skill", "1.0.0", "test-user", java.util.Map.of()))
.willReturn(new SkillDownloadService.DownloadResult(
new ByteArrayInputStream("zip".getBytes()),
() -> new ByteArrayInputStream("zip".getBytes()),
"demo-skill-1.0.0.zip",
3L,
"application/zip",
"http://download.example/presigned"
"http://download.example/presigned",
false
));
mockMvc.perform(get("/api/v1/skills/global/demo-skill/versions/1.0.0/download")
.header("X-Forwarded-Proto", "https")
.with(user("test-user"))
.requestAttr("userId", "test-user")
.with(csrf()))
.andExpect(status().isOk())
.andExpect(header().string("Content-Disposition", "attachment; filename=\"demo-skill-1.0.0.zip\""));
@ -83,17 +101,20 @@ class SkillControllerDownloadTest {
@Test
void downloadVersion_streamsWhenPresignedUrlUnavailable() throws Exception {
given(skillDownloadService.downloadVersion("global", "demo-skill", "1.0.0", null, java.util.Map.of()))
given(rateLimiter.tryAcquire(anyString(), anyInt(), anyInt())).willReturn(true);
given(skillDownloadService.downloadVersion("global", "demo-skill", "1.0.0", "test-user", java.util.Map.of()))
.willReturn(new SkillDownloadService.DownloadResult(
new ByteArrayInputStream("zip".getBytes()),
() -> new ByteArrayInputStream("zip".getBytes()),
"demo-skill-1.0.0.zip",
3L,
"application/zip",
null
null,
false
));
mockMvc.perform(get("/api/v1/skills/global/demo-skill/versions/1.0.0/download")
.with(user("test-user"))
.requestAttr("userId", "test-user")
.with(csrf()))
.andExpect(status().isOk())
.andExpect(header().string("Content-Disposition", "attachment; filename=\"demo-skill-1.0.0.zip\""));
@ -101,16 +122,19 @@ class SkillControllerDownloadTest {
@Test
void downloadVersion_allowsAnonymousForGlobalSkill() throws Exception {
given(rateLimiter.tryAcquire(anyString(), anyInt(), anyInt())).willReturn(true);
given(skillDownloadService.downloadVersion("global", "demo-skill", "1.0.0", null, java.util.Map.of()))
.willReturn(new SkillDownloadService.DownloadResult(
new ByteArrayInputStream("zip".getBytes()),
() -> new ByteArrayInputStream("zip".getBytes()),
"demo-skill-1.0.0.zip",
3L,
"application/zip",
null
null,
false
));
mockMvc.perform(get("/api/v1/skills/global/demo-skill/versions/1.0.0/download")
.with(user("anonymous-test"))
.with(csrf()))
.andExpect(status().isOk())
.andExpect(header().string("Content-Disposition", "attachment; filename=\"demo-skill-1.0.0.zip\""));
@ -118,11 +142,61 @@ class SkillControllerDownloadTest {
@Test
void downloadVersion_forbidsAnonymousWhenServiceRejectsSkill() throws Exception {
given(rateLimiter.tryAcquire(anyString(), anyInt(), anyInt())).willReturn(true);
given(skillDownloadService.downloadVersion("team-ai", "demo-skill", "1.0.0", null, java.util.Map.of()))
.willThrow(new DomainForbiddenException("error.skill.access.denied", "demo-skill"));
mockMvc.perform(get("/api/v1/skills/team-ai/demo-skill/versions/1.0.0/download")
.with(user("anonymous-test"))
.with(csrf()))
.andExpect(status().isForbidden());
}
@Test
void downloadVersion_redirectDoesNotOpenContentStream() throws Exception {
given(rateLimiter.tryAcquire(anyString(), anyInt(), anyInt())).willReturn(true);
given(skillDownloadService.downloadVersion("global", "demo-skill", "1.0.0", "test-user", java.util.Map.of()))
.willReturn(new SkillDownloadService.DownloadResult(
() -> {
throw new AssertionError("content stream should not be opened for redirects");
},
"demo-skill-1.0.0.zip",
128L,
"application/zip",
"https://download.example/presigned",
false
));
mockMvc.perform(get("/api/v1/skills/global/demo-skill/versions/1.0.0/download")
.with(user("test-user"))
.requestAttr("userId", "test-user")
.with(csrf()))
.andExpect(status().isFound())
.andExpect(header().string("Location", "https://download.example/presigned"));
}
@Test
void downloadVersion_usesPerVersionRateLimitKey() throws Exception {
given(rateLimiter.tryAcquire(anyString(), anyInt(), anyInt())).willReturn(true);
given(skillDownloadService.downloadVersion("global", "demo-skill", "1.0.0", "test-user", java.util.Map.of()))
.willReturn(new SkillDownloadService.DownloadResult(
() -> new ByteArrayInputStream("zip".getBytes()),
"demo-skill-1.0.0.zip",
3L,
"application/zip",
null,
false
));
mockMvc.perform(get("/api/v1/skills/global/demo-skill/versions/1.0.0/download")
.with(user("test-user"))
.requestAttr("userId", "test-user")
.with(csrf()))
.andExpect(status().isOk());
verify(rateLimiter).tryAcquire(
"ratelimit:download:user:test-user:ns:global:slug:demo-skill:version:1.0.0",
120,
60);
}
}

View file

@ -43,6 +43,12 @@ public class SkillVersion {
@Column(name = "published_at")
private LocalDateTime publishedAt;
@Column(name = "bundle_ready", nullable = false)
private boolean bundleReady;
@Column(name = "download_ready", nullable = false)
private boolean downloadReady;
@Column(name = "yanked_at")
private LocalDateTime yankedAt;
@ -114,6 +120,14 @@ public class SkillVersion {
return publishedAt;
}
public boolean isBundleReady() {
return bundleReady;
}
public boolean isDownloadReady() {
return downloadReady;
}
public LocalDateTime getYankedAt() {
return yankedAt;
}
@ -163,6 +177,14 @@ public class SkillVersion {
this.publishedAt = publishedAt;
}
public void setBundleReady(boolean bundleReady) {
this.bundleReady = bundleReady;
}
public void setDownloadReady(boolean downloadReady) {
this.downloadReady = downloadReady;
}
public void setYankedAt(LocalDateTime yankedAt) {
this.yankedAt = yankedAt;
}

View file

@ -10,6 +10,8 @@ import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException;
import com.iflytek.skillhub.domain.skill.*;
import com.iflytek.skillhub.storage.ObjectStorageService;
import com.iflytek.skillhub.storage.ObjectMetadata;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.stereotype.Service;
@ -20,11 +22,13 @@ import java.time.Duration;
import java.util.Comparator;
import java.util.Map;
import java.util.List;
import java.util.function.Supplier;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;
@Service
public class SkillDownloadService {
private static final Logger log = LoggerFactory.getLogger(SkillDownloadService.class);
private final NamespaceRepository namespaceRepository;
private final SkillRepository skillRepository;
@ -61,12 +65,17 @@ public class SkillDownloadService {
}
public record DownloadResult(
InputStream content,
Supplier<InputStream> contentSupplier,
String filename,
long contentLength,
String contentType,
String presignedUrl
) {}
String presignedUrl,
boolean fallbackBundle
) {
public InputStream openContent() {
return contentSupplier.get();
}
}
public DownloadResult downloadLatest(
String namespaceSlug,
@ -140,9 +149,21 @@ public class SkillDownloadService {
ObjectMetadata metadata = objectStorageService.getMetadata(storageKey);
String filename = buildFilename(skill, version);
String presignedUrl = objectStorageService.generatePresignedUrl(storageKey, Duration.ofMinutes(10), filename);
InputStream content = objectStorageService.getObject(storageKey);
result = new DownloadResult(content, filename, metadata.size(), metadata.contentType(), presignedUrl);
result = new DownloadResult(
() -> objectStorageService.getObject(storageKey),
filename,
metadata.size(),
metadata.contentType(),
presignedUrl,
false
);
} else {
log.warn(
"Bundle missing for published version, falling back to per-file zip [skillId={}, versionId={}, version={}]",
skill.getId(),
version.getId(),
version.getVersion()
);
result = buildBundleFromFiles(skill, version);
}
@ -163,11 +184,12 @@ public class SkillDownloadService {
byte[] bundle = createBundle(files);
return new DownloadResult(
new ByteArrayInputStream(bundle),
() -> new ByteArrayInputStream(bundle),
buildFilename(skill, version),
bundle.length,
"application/zip",
null
null,
true
);
}

View file

@ -185,6 +185,7 @@ public class SkillGovernanceService {
version.setYankedAt(LocalDateTime.now());
version.setYankedBy(actorUserId);
version.setYankReason(reason);
version.setDownloadReady(false);
SkillVersion saved = skillVersionRepository.save(version);
skillRepository.findById(version.getSkillId()).ifPresent(skill -> {
if (versionId.equals(skill.getLatestVersionId())) {

View file

@ -314,6 +314,8 @@ public class SkillPublishService {
// 11. Update version stats
version.setFileCount(skillFiles.size());
version.setTotalSize(totalSize);
version.setBundleReady(true);
version.setDownloadReady(!skillFiles.isEmpty());
skillVersionRepository.save(version);
if (!autoPublish) {

View file

@ -314,14 +314,7 @@ public class SkillQueryService {
if (version.getStatus() != SkillVersionStatus.PUBLISHED) {
return false;
}
if (objectStorageService.exists(getBundleStorageKey(version.getSkillId(), version.getId()))) {
return true;
}
return skillFileRepository.findByVersionId(version.getId()).stream()
.findAny()
.filter(file -> skillFileRepository.findByVersionId(version.getId()).stream()
.allMatch(candidate -> objectStorageService.exists(candidate.getStorageKey())))
.isPresent();
return version.isDownloadReady();
}
public ResolvedVersionDTO resolveVersion(

View file

@ -112,7 +112,7 @@ class SkillDownloadServiceTest {
assertNotNull(result);
assertEquals("Test Skill-1.0.0.zip", result.filename());
assertEquals(1000L, result.contentLength());
assertNotNull(result.content());
assertNotNull(result.openContent());
verify(skillRepository).incrementDownloadCount(1L);
verify(skillVersionStatsRepository).incrementDownloadCount(10L, 1L);
verify(eventPublisher).publishEvent(any(SkillDownloadedEvent.class));
@ -157,7 +157,7 @@ class SkillDownloadServiceTest {
// Assert
assertNotNull(result);
assertEquals("Test Skill-1.0.0.zip", result.filename());
assertNotNull(result.content());
assertNotNull(result.openContent());
verify(skillRepository).incrementDownloadCount(1L);
verify(skillVersionStatsRepository).incrementDownloadCount(10L, 1L);
verify(eventPublisher).publishEvent(any(SkillDownloadedEvent.class));
@ -197,7 +197,7 @@ class SkillDownloadServiceTest {
SkillDownloadService.DownloadResult result = service.downloadVersion(namespaceSlug, skillSlug, versionStr, userId, userNsRoles);
assertEquals("http://minio.local/presigned", result.presignedUrl());
assertNotNull(result.content());
assertNotNull(result.openContent());
verify(skillRepository).incrementDownloadCount(1L);
verify(skillVersionStatsRepository).incrementDownloadCount(10L, 1L);
verify(eventPublisher).publishEvent(any(SkillDownloadedEvent.class));
@ -263,11 +263,12 @@ class SkillDownloadServiceTest {
SkillDownloadService.DownloadResult result = service.downloadVersion(namespaceSlug, skillSlug, versionStr, userId, userNsRoles);
assertNull(result.presignedUrl());
assertTrue(result.fallbackBundle());
assertEquals("Generate Commit Message-1.0.0.zip", result.filename());
assertEquals("application/zip", result.contentType());
assertTrue(result.contentLength() > 0);
try (ZipInputStream zipInputStream = new ZipInputStream(result.content())) {
try (ZipInputStream zipInputStream = new ZipInputStream(result.openContent())) {
var entry = zipInputStream.getNextEntry();
assertNotNull(entry);
assertEquals("SKILL.md", entry.getName());

View file

@ -422,25 +422,32 @@ class SkillQueryServiceTest {
SkillVersion version = new SkillVersion(1L, "1.0.0", "user-100");
setId(version, 10L);
version.setStatus(SkillVersionStatus.PUBLISHED);
version.setDownloadReady(false);
assertFalse(service.isDownloadAvailable(version));
verify(objectStorageService).exists("packages/1/10/bundle.zip");
}
@Test
void testIsDownloadAvailable_ShouldReturnTrueWhenBundleMissingButFilesExist() throws Exception {
void testIsDownloadAvailable_ShouldReturnTrueWhenPublishedVersionHasFiles() throws Exception {
SkillVersion version = new SkillVersion(1L, "1.0.0", "user-100");
setId(version, 10L);
version.setStatus(SkillVersionStatus.PUBLISHED);
SkillFile file = new SkillFile(10L, "SKILL.md", 10L, "text/markdown", "hash", "skills/1/10/SKILL.md");
when(objectStorageService.exists("packages/1/10/bundle.zip")).thenReturn(false);
when(skillFileRepository.findByVersionId(10L)).thenReturn(List.of(file));
when(objectStorageService.exists("skills/1/10/SKILL.md")).thenReturn(true);
version.setDownloadReady(true);
assertTrue(service.isDownloadAvailable(version));
}
@Test
void testIsDownloadAvailable_ShouldNotHitObjectStorageForListSignals() throws Exception {
SkillVersion version = new SkillVersion(1L, "1.0.0", "user-100");
setId(version, 10L);
version.setStatus(SkillVersionStatus.PUBLISHED);
version.setDownloadReady(true);
assertTrue(service.isDownloadAvailable(version));
verifyNoInteractions(objectStorageService, skillFileRepository);
}
@Test
void testGetVersionDetail_ShouldReturnMetadataPayload() throws Exception {
String namespaceSlug = "test-ns";

View file

@ -25,6 +25,11 @@
<artifactId>s3</artifactId>
<version>2.20.26</version>
</dependency>
<dependency>
<groupId>software.amazon.awssdk</groupId>
<artifactId>apache-client</artifactId>
<version>2.20.26</version>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-test</artifactId>

View file

@ -28,19 +28,19 @@ public class LocalFileStorageService implements ObjectStorageService {
data.transferTo(out);
}
Files.move(tmp, target, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE);
} catch (IOException e) { throw new UncheckedIOException("Failed to put object: " + key, e); }
} catch (IOException e) { throw new StorageAccessException("putObject", key, e); }
}
@Override
public InputStream getObject(String key) {
try { return Files.newInputStream(resolve(key)); }
catch (IOException e) { throw new UncheckedIOException("Failed to get object: " + key, e); }
catch (IOException e) { throw new StorageAccessException("getObject", key, e); }
}
@Override
public void deleteObject(String key) {
try { Files.deleteIfExists(resolve(key)); }
catch (IOException e) { throw new UncheckedIOException("Failed to delete object: " + key, e); }
catch (IOException e) { throw new StorageAccessException("deleteObject", key, e); }
}
@Override
@ -55,7 +55,7 @@ public class LocalFileStorageService implements ObjectStorageService {
Path path = resolve(key);
BasicFileAttributes attrs = Files.readAttributes(path, BasicFileAttributes.class);
return new ObjectMetadata(attrs.size(), Files.probeContentType(path), attrs.lastModifiedTime().toInstant());
} catch (IOException e) { throw new UncheckedIOException("Failed to get metadata: " + key, e); }
} catch (IOException e) { throw new StorageAccessException("getMetadata", key, e); }
}
@Override

View file

@ -17,6 +17,10 @@ public class S3StorageProperties {
private boolean forcePathStyle = true;
private boolean autoCreateBucket = false;
private Duration presignExpiry = Duration.ofMinutes(10);
private Integer maxConnections = 100;
private Duration connectionAcquisitionTimeout = Duration.ofSeconds(2);
private Duration apiCallAttemptTimeout = Duration.ofSeconds(10);
private Duration apiCallTimeout = Duration.ofSeconds(30);
public String getEndpoint() { return endpoint; }
public void setEndpoint(String endpoint) { this.endpoint = endpoint; }
@ -36,4 +40,12 @@ public class S3StorageProperties {
public void setAutoCreateBucket(boolean autoCreateBucket) { this.autoCreateBucket = autoCreateBucket; }
public Duration getPresignExpiry() { return presignExpiry; }
public void setPresignExpiry(Duration presignExpiry) { this.presignExpiry = presignExpiry; }
public Integer getMaxConnections() { return maxConnections; }
public void setMaxConnections(Integer maxConnections) { this.maxConnections = maxConnections; }
public Duration getConnectionAcquisitionTimeout() { return connectionAcquisitionTimeout; }
public void setConnectionAcquisitionTimeout(Duration connectionAcquisitionTimeout) { this.connectionAcquisitionTimeout = connectionAcquisitionTimeout; }
public Duration getApiCallAttemptTimeout() { return apiCallAttemptTimeout; }
public void setApiCallAttemptTimeout(Duration apiCallAttemptTimeout) { this.apiCallAttemptTimeout = apiCallAttemptTimeout; }
public Duration getApiCallTimeout() { return apiCallTimeout; }
public void setApiCallTimeout(Duration apiCallTimeout) { this.apiCallTimeout = apiCallTimeout; }
}

View file

@ -8,6 +8,7 @@ import org.springframework.stereotype.Service;
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
import software.amazon.awssdk.core.sync.RequestBody;
import software.amazon.awssdk.http.apache.ApacheHttpClient;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.services.s3.S3Client;
import software.amazon.awssdk.services.s3.model.*;
@ -33,11 +34,18 @@ public class S3StorageService implements ObjectStorageService {
@PostConstruct
void init() {
ApacheHttpClient.Builder httpClientBuilder = ApacheHttpClient.builder()
.maxConnections(properties.getMaxConnections())
.connectionAcquisitionTimeout(properties.getConnectionAcquisitionTimeout());
var builder = S3Client.builder()
.region(Region.of(properties.getRegion()))
.credentialsProvider(StaticCredentialsProvider.create(
AwsBasicCredentials.create(properties.getAccessKey(), properties.getSecretKey())))
.forcePathStyle(properties.isForcePathStyle());
.forcePathStyle(properties.isForcePathStyle())
.httpClientBuilder(httpClientBuilder)
.overrideConfiguration(config -> config
.apiCallAttemptTimeout(properties.getApiCallAttemptTimeout())
.apiCallTimeout(properties.getApiCallTimeout()));
if (properties.getEndpoint() != null && !properties.getEndpoint().isBlank()) {
builder.endpointOverride(URI.create(properties.getEndpoint()));
}
@ -68,31 +76,52 @@ public class S3StorageService implements ObjectStorageService {
}
@Override public void putObject(String key, InputStream data, long size, String contentType) {
s3Client.putObject(PutObjectRequest.builder().bucket(properties.getBucket()).key(key).contentType(contentType).contentLength(size).build(), RequestBody.fromInputStream(data, size));
try {
s3Client.putObject(PutObjectRequest.builder().bucket(properties.getBucket()).key(key).contentType(contentType).contentLength(size).build(), RequestBody.fromInputStream(data, size));
} catch (RuntimeException e) {
throw new StorageAccessException("putObject", key, e);
}
}
@Override public InputStream getObject(String key) {
return s3Client.getObject(GetObjectRequest.builder().bucket(properties.getBucket()).key(key).build());
try {
return s3Client.getObject(GetObjectRequest.builder().bucket(properties.getBucket()).key(key).build());
} catch (RuntimeException e) {
throw new StorageAccessException("getObject", key, e);
}
}
@Override public void deleteObject(String key) {
s3Client.deleteObject(DeleteObjectRequest.builder().bucket(properties.getBucket()).key(key).build());
try {
s3Client.deleteObject(DeleteObjectRequest.builder().bucket(properties.getBucket()).key(key).build());
} catch (RuntimeException e) {
throw new StorageAccessException("deleteObject", key, e);
}
}
@Override public void deleteObjects(List<String> keys) {
if (keys.isEmpty()) return;
List<ObjectIdentifier> ids = keys.stream().map(k -> ObjectIdentifier.builder().key(k).build()).toList();
s3Client.deleteObjects(DeleteObjectsRequest.builder().bucket(properties.getBucket()).delete(Delete.builder().objects(ids).build()).build());
try {
List<ObjectIdentifier> ids = keys.stream().map(k -> ObjectIdentifier.builder().key(k).build()).toList();
s3Client.deleteObjects(DeleteObjectsRequest.builder().bucket(properties.getBucket()).delete(Delete.builder().objects(ids).build()).build());
} catch (RuntimeException e) {
throw new StorageAccessException("deleteObjects", String.join(",", keys), e);
}
}
@Override public boolean exists(String key) {
try { s3Client.headObject(HeadObjectRequest.builder().bucket(properties.getBucket()).key(key).build()); return true; }
catch (NoSuchKeyException e) { return false; }
catch (RuntimeException e) { throw new StorageAccessException("exists", key, e); }
}
@Override public ObjectMetadata getMetadata(String key) {
HeadObjectResponse resp = s3Client.headObject(HeadObjectRequest.builder().bucket(properties.getBucket()).key(key).build());
return new ObjectMetadata(resp.contentLength(), resp.contentType(), resp.lastModified());
try {
HeadObjectResponse resp = s3Client.headObject(HeadObjectRequest.builder().bucket(properties.getBucket()).key(key).build());
return new ObjectMetadata(resp.contentLength(), resp.contentType(), resp.lastModified());
} catch (RuntimeException e) {
throw new StorageAccessException("getMetadata", key, e);
}
}
@Override
@ -102,16 +131,20 @@ public class S3StorageService implements ObjectStorageService {
? "attachment"
: "attachment; filename*=UTF-8''" + java.net.URLEncoder.encode(downloadFilename, StandardCharsets.UTF_8)
.replace("+", "%20");
PresignedGetObjectRequest request = s3Presigner.presignGetObject(
GetObjectPresignRequest.builder()
.signatureDuration(signatureDuration)
.getObjectRequest(GetObjectRequest.builder()
.bucket(properties.getBucket())
.key(key)
.responseContentDisposition(contentDisposition)
.build())
.build()
);
return request.url().toString();
try {
PresignedGetObjectRequest request = s3Presigner.presignGetObject(
GetObjectPresignRequest.builder()
.signatureDuration(signatureDuration)
.getObjectRequest(GetObjectRequest.builder()
.bucket(properties.getBucket())
.key(key)
.responseContentDisposition(contentDisposition)
.build())
.build()
);
return request.url().toString();
} catch (RuntimeException e) {
throw new StorageAccessException("generatePresignedUrl", key, e);
}
}
}

View file

@ -0,0 +1,21 @@
package com.iflytek.skillhub.storage;
public class StorageAccessException extends RuntimeException {
private final String operation;
private final String key;
public StorageAccessException(String operation, String key, Throwable cause) {
super("Storage operation failed: " + operation + " [" + key + "]", cause);
this.operation = operation;
this.key = key;
}
public String getOperation() {
return operation;
}
public String getKey() {
return key;
}
}