Skip to content

Commit 04e588d

Browse files
vsxdvsxd
committed
feat: harden download and storage resilience
1 parent f3dbbf8 commit 04e588d

File tree

22 files changed

+388
-74
lines changed

22 files changed

+388
-74
lines changed

‎server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillController.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import com.iflytek.skillhub.dto.SkillLifecycleVersionResponse;
1717
import com.iflytek.skillhub.dto.SkillVersionDetailResponse;
1818
import com.iflytek.skillhub.dto.SkillVersionResponse;
19+
import com.iflytek.skillhub.metrics.SkillHubMetrics;
1920
import com.iflytek.skillhub.ratelimit.RateLimit;
2021
import org.springframework.core.io.InputStreamResource;
2122
import org.springframework.data.domain.Page;
@@ -39,14 +40,17 @@ public class SkillController extends BaseApiController {
3940

4041
private final SkillQueryService skillQueryService;
4142
private final SkillDownloadService skillDownloadService;
43+
private final SkillHubMetrics metrics;
4244

4345
public SkillController(
4446
SkillQueryService skillQueryService,
4547
SkillDownloadService skillDownloadService,
48+
SkillHubMetrics metrics,
4649
ApiResponseFactory responseFactory) {
4750
super(responseFactory);
4851
this.skillQueryService = skillQueryService;
4952
this.skillDownloadService = skillDownloadService;
53+
this.metrics = metrics;
5054
}
5155

5256
@GetMapping("/{namespace}/{slug}")
@@ -332,16 +336,24 @@ public ResponseEntity<InputStreamResource> downloadByTag(
332336

333337
private ResponseEntity<InputStreamResource> buildDownloadResponse(HttpServletRequest request, SkillDownloadService.DownloadResult result) {
334338
if (shouldRedirectToPresignedUrl(request, result.presignedUrl())) {
339+
metrics.recordDownloadDelivery("redirect", result.fallbackBundle());
340+
if (result.fallbackBundle()) {
341+
metrics.incrementBundleMissingFallback();
342+
}
335343
return ResponseEntity.status(HttpStatus.FOUND)
336344
.header(HttpHeaders.LOCATION, result.presignedUrl())
337345
.build();
338346
}
339347

348+
metrics.recordDownloadDelivery("stream", result.fallbackBundle());
349+
if (result.fallbackBundle()) {
350+
metrics.incrementBundleMissingFallback();
351+
}
340352
return ResponseEntity.ok()
341353
.header(HttpHeaders.CONTENT_DISPOSITION, "attachment; filename=\"" + result.filename() + "\"")
342354
.contentType(MediaType.parseMediaType(result.contentType()))
343355
.contentLength(result.contentLength())
344-
.body(new InputStreamResource(result.content()));
356+
.body(new InputStreamResource(result.openContent()));
345357
}
346358

347359
private boolean shouldRedirectToPresignedUrl(HttpServletRequest request, String presignedUrl) {

‎server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77
import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException;
88
import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException;
99
import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException;
10+
import com.iflytek.skillhub.metrics.SkillHubMetrics;
1011
import com.iflytek.skillhub.security.SensitiveLogSanitizer;
12+
import com.iflytek.skillhub.storage.StorageAccessException;
1113
import jakarta.servlet.http.HttpServletRequest;
1214
import org.slf4j.Logger;
1315
import org.slf4j.LoggerFactory;
@@ -27,11 +29,14 @@ public class GlobalExceptionHandler {
2729
private static final Logger logger = LoggerFactory.getLogger(GlobalExceptionHandler.class);
2830
private final ApiResponseFactory apiResponseFactory;
2931
private final SensitiveLogSanitizer sensitiveLogSanitizer;
32+
private final SkillHubMetrics metrics;
3033

3134
public GlobalExceptionHandler(ApiResponseFactory apiResponseFactory,
32-
SensitiveLogSanitizer sensitiveLogSanitizer) {
35+
SensitiveLogSanitizer sensitiveLogSanitizer,
36+
SkillHubMetrics metrics) {
3337
this.apiResponseFactory = apiResponseFactory;
3438
this.sensitiveLogSanitizer = sensitiveLogSanitizer;
39+
this.metrics = metrics;
3540
}
3641

3742
@ExceptionHandler(LocalizedException.class)
@@ -108,6 +113,23 @@ public ResponseEntity<ApiResponse<Void>> handleAccessDenied(AccessDeniedExceptio
108113
apiResponseFactory.error(403, "error.forbidden"));
109114
}
110115

116+
@ExceptionHandler(StorageAccessException.class)
117+
public ResponseEntity<ApiResponse<Void>> handleStorageAccess(StorageAccessException ex, HttpServletRequest request) {
118+
metrics.incrementStorageAccessFailure(ex.getOperation());
119+
logger.warn(
120+
"Object storage unavailable [requestId={}, method={}, path={}, userId={}, operation={}, key={}]",
121+
MDC.get("requestId"),
122+
request.getMethod(),
123+
sensitiveLogSanitizer.sanitizeRequestTarget(request),
124+
resolveUserId(request),
125+
ex.getOperation(),
126+
ex.getKey(),
127+
ex
128+
);
129+
return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE).body(
130+
apiResponseFactory.error(503, "error.storage.unavailable"));
131+
}
132+
111133
@ExceptionHandler(Exception.class)
112134
public ResponseEntity<ApiResponse<Void>> handleGlobalException(Exception ex, HttpServletRequest request) {
113135
logger.error(

‎server/skillhub-app/src/main/java/com/iflytek/skillhub/metrics/SkillHubMetrics.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,30 @@ public void incrementSkillPublish(String namespace, String status) {
3131
"status", status
3232
).increment();
3333
}
34+
35+
public void recordDownloadDelivery(String mode, boolean fallbackBundle) {
36+
meterRegistry.counter(
37+
"skillhub.skill.download.delivery",
38+
"mode", mode,
39+
"fallback_bundle", Boolean.toString(fallbackBundle)
40+
).increment();
41+
}
42+
43+
public void incrementBundleMissingFallback() {
44+
meterRegistry.counter("skillhub.skill.download.bundle_missing_fallback").increment();
45+
}
46+
47+
public void incrementRateLimitExceeded(String category) {
48+
meterRegistry.counter(
49+
"skillhub.ratelimit.exceeded",
50+
"category", category
51+
).increment();
52+
}
53+
54+
public void incrementStorageAccessFailure(String operation) {
55+
meterRegistry.counter(
56+
"skillhub.storage.failure",
57+
"operation", operation
58+
).increment();
59+
}
3460
}

‎server/skillhub-app/src/main/java/com/iflytek/skillhub/ratelimit/RateLimitInterceptor.java‎

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,17 @@
33
import com.fasterxml.jackson.databind.ObjectMapper;
44
import com.iflytek.skillhub.dto.ApiResponse;
55
import com.iflytek.skillhub.dto.ApiResponseFactory;
6+
import com.iflytek.skillhub.metrics.SkillHubMetrics;
67
import jakarta.servlet.http.HttpServletRequest;
78
import jakarta.servlet.http.HttpServletResponse;
89
import org.springframework.http.HttpStatus;
910
import org.springframework.http.MediaType;
1011
import org.springframework.stereotype.Component;
1112
import org.springframework.web.method.HandlerMethod;
1213
import org.springframework.web.servlet.HandlerInterceptor;
14+
import org.springframework.web.servlet.HandlerMapping;
15+
16+
import java.util.Map;
1317

1418
@Component
1519
public class RateLimitInterceptor implements HandlerInterceptor {
@@ -19,17 +23,20 @@ public class RateLimitInterceptor implements HandlerInterceptor {
1923
private final AnonymousDownloadIdentityService anonymousDownloadIdentityService;
2024
private final ApiResponseFactory apiResponseFactory;
2125
private final ObjectMapper objectMapper;
26+
private final SkillHubMetrics metrics;
2227

2328
public RateLimitInterceptor(RateLimiter rateLimiter,
2429
ClientIpResolver clientIpResolver,
2530
AnonymousDownloadIdentityService anonymousDownloadIdentityService,
2631
ApiResponseFactory apiResponseFactory,
27-
ObjectMapper objectMapper) {
32+
ObjectMapper objectMapper,
33+
SkillHubMetrics metrics) {
2834
this.rateLimiter = rateLimiter;
2935
this.clientIpResolver = clientIpResolver;
3036
this.anonymousDownloadIdentityService = anonymousDownloadIdentityService;
3137
this.apiResponseFactory = apiResponseFactory;
3238
this.objectMapper = objectMapper;
39+
this.metrics = metrics;
3340
}
3441

3542
@Override
@@ -51,12 +58,17 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons
5158

5259
// Get limit based on authentication status
5360
int limit = isAuthenticated ? rateLimit.authenticated() : rateLimit.anonymous();
61+
String resourceSuffix = resolveResourceSuffix(rateLimit.category(), request);
5462

5563
boolean allowed = isAuthenticated
56-
? rateLimiter.tryAcquire("ratelimit:" + rateLimit.category() + ":user:" + userId, limit, rateLimit.windowSeconds())
57-
: checkAnonymousLimit(request, response, rateLimit, limit);
64+
? rateLimiter.tryAcquire(
65+
"ratelimit:" + rateLimit.category() + ":user:" + userId + resourceSuffix,
66+
limit,
67+
rateLimit.windowSeconds())
68+
: checkAnonymousLimit(request, response, rateLimit, limit, resourceSuffix);
5869

5970
if (!allowed) {
71+
metrics.incrementRateLimitExceeded(rateLimit.category());
6072
response.setStatus(HttpStatus.TOO_MANY_REQUESTS.value());
6173
response.setContentType(MediaType.APPLICATION_JSON_VALUE);
6274
ApiResponse<Void> body = apiResponseFactory.error(429, "error.rateLimit.exceeded");
@@ -70,10 +82,11 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons
7082
private boolean checkAnonymousLimit(HttpServletRequest request,
7183
HttpServletResponse response,
7284
RateLimit rateLimit,
73-
int limit) {
85+
int limit,
86+
String resourceSuffix) {
7487
if (!"download".equals(rateLimit.category())) {
7588
return rateLimiter.tryAcquire(
76-
"ratelimit:" + rateLimit.category() + ":ip:" + clientIpResolver.resolve(request),
89+
"ratelimit:" + rateLimit.category() + ":ip:" + clientIpResolver.resolve(request) + resourceSuffix,
7790
limit,
7891
rateLimit.windowSeconds()
7992
);
@@ -82,17 +95,45 @@ private boolean checkAnonymousLimit(HttpServletRequest request,
8295
AnonymousDownloadIdentityService.AnonymousDownloadIdentity identity =
8396
anonymousDownloadIdentityService.resolve(request, response);
8497
boolean ipAllowed = rateLimiter.tryAcquire(
85-
"ratelimit:download:ip:" + identity.ipHash(),
98+
"ratelimit:download:ip:" + identity.ipHash() + resourceSuffix,
8699
limit,
87100
rateLimit.windowSeconds()
88101
);
89102
if (!ipAllowed) {
90103
return false;
91104
}
92105
return rateLimiter.tryAcquire(
93-
"ratelimit:download:anon:" + identity.cookieHash(),
106+
"ratelimit:download:anon:" + identity.cookieHash() + resourceSuffix,
94107
limit,
95108
rateLimit.windowSeconds()
96109
);
97110
}
111+
112+
@SuppressWarnings("unchecked")
113+
private String resolveResourceSuffix(String category, HttpServletRequest request) {
114+
if (!"download".equals(category)) {
115+
return "";
116+
}
117+
Object attribute = request.getAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE);
118+
if (!(attribute instanceof Map<?, ?> templateVariables)) {
119+
return "";
120+
}
121+
String namespace = stringValue(templateVariables.get("namespace"));
122+
String slug = stringValue(templateVariables.get("slug"));
123+
String version = stringValue(templateVariables.get("version"));
124+
String tagName = stringValue(templateVariables.get("tagName"));
125+
if (namespace == null || slug == null) {
126+
return "";
127+
}
128+
String target = version != null ? "version:" + version : tagName != null ? "tag:" + tagName : "latest";
129+
return ":ns:" + namespace + ":slug:" + slug + ":" + target;
130+
}
131+
132+
private String stringValue(Object value) {
133+
if (value == null) {
134+
return null;
135+
}
136+
String text = value.toString();
137+
return text.isBlank() ? null : text;
138+
}
98139
}

‎server/skillhub-app/src/main/resources/application.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ skillhub:
8484
force-path-style: ${SKILLHUB_STORAGE_S3_FORCE_PATH_STYLE:true}
8585
auto-create-bucket: ${SKILLHUB_STORAGE_S3_AUTO_CREATE_BUCKET:false}
8686
presign-expiry: ${SKILLHUB_STORAGE_S3_PRESIGN_EXPIRY:PT10M}
87+
max-connections: ${SKILLHUB_STORAGE_S3_MAX_CONNECTIONS:100}
88+
connection-acquisition-timeout: ${SKILLHUB_STORAGE_S3_CONNECTION_ACQUISITION_TIMEOUT:PT2S}
89+
api-call-attempt-timeout: ${SKILLHUB_STORAGE_S3_API_CALL_ATTEMPT_TIMEOUT:PT10S}
90+
api-call-timeout: ${SKILLHUB_STORAGE_S3_API_CALL_TIMEOUT:PT30S}
8791
search:
8892
engine: postgres
8993
rebuild-on-startup: false

‎server/skillhub-app/src/main/resources/db/migration/V15__skill_version_download_state.sql‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
ALTER TABLE skill_version
2+
ADD COLUMN bundle_ready BOOLEAN NOT NULL DEFAULT FALSE,
3+
ADD COLUMN download_ready BOOLEAN NOT NULL DEFAULT FALSE;
4+
5+
UPDATE skill_version
6+
SET download_ready = CASE
7+
WHEN status = 'PUBLISHED' AND file_count > 0 THEN TRUE
8+
ELSE FALSE
9+
END,
10+
bundle_ready = FALSE;

‎server/skillhub-app/src/main/resources/messages.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ error.auth.sessionBootstrap.notAuthenticated=No authenticated external session f
4848
error.badRequest=Invalid request
4949
error.forbidden=Forbidden
5050
error.rateLimit.exceeded=Rate limit exceeded
51+
error.storage.unavailable=Object storage is temporarily unavailable. Please try again later.
5152
error.internal=An unexpected error occurred
5253
error.slug.blank=Slug cannot be blank
5354
error.slug.length=Slug length must be between {0} and {1} characters

‎server/skillhub-app/src/main/resources/messages_zh.properties‎

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

‎server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/DownloadRateLimitControllerTest.java‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import static org.mockito.Mockito.times;
99
import static org.mockito.Mockito.never;
1010
import static org.mockito.Mockito.verify;
11+
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user;
1112
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
1213
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
1314
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
@@ -60,15 +61,17 @@ void anonymousDownloadUsesIpAndSignedCookieBuckets() throws Exception {
6061
given(rateLimiter.tryAcquire(anyString(), anyInt(), anyInt())).willReturn(true);
6162
given(skillDownloadService.downloadVersion("global", "demo-skill", "1.0.0", null, Map.of()))
6263
.willReturn(new SkillDownloadService.DownloadResult(
63-
new ByteArrayInputStream("zip".getBytes()),
64+
() -> new ByteArrayInputStream("zip".getBytes()),
6465
"demo-skill-1.0.0.zip",
6566
3L,
6667
"application/zip",
67-
null
68+
null,
69+
false
6870
));
6971

7072
var result = mockMvc.perform(get("/api/v1/skills/global/demo-skill/versions/1.0.0/download")
71-
.header("X-Forwarded-For", "203.0.113.10"))
73+
.header("X-Forwarded-For", "203.0.113.10")
74+
.with(user("anonymous-test")))
7275
.andExpect(status().isOk())
7376
.andExpect(header().string("Content-Disposition", "attachment; filename=\"demo-skill-1.0.0.zip\""))
7477
.andExpect(header().exists("Set-Cookie"))
@@ -83,8 +86,8 @@ void anonymousDownloadUsesIpAndSignedCookieBuckets() throws Exception {
8386

8487
ArgumentCaptor<String> keyCaptor = ArgumentCaptor.forClass(String.class);
8588
verify(rateLimiter, times(2)).tryAcquire(keyCaptor.capture(), anyInt(), anyInt());
86-
assertThat(keyCaptor.getAllValues()).anyMatch(key -> key.startsWith("ratelimit:download:ip:"));
87-
assertThat(keyCaptor.getAllValues()).anyMatch(key -> key.startsWith("ratelimit:download:anon:"));
89+
assertThat(keyCaptor.getAllValues()).anyMatch(key -> key.startsWith("ratelimit:download:ip:") && key.endsWith(":ns:global:slug:demo-skill:version:1.0.0"));
90+
assertThat(keyCaptor.getAllValues()).anyMatch(key -> key.startsWith("ratelimit:download:anon:") && key.endsWith(":ns:global:slug:demo-skill:version:1.0.0"));
8891
}
8992

9093
@Test
@@ -93,7 +96,8 @@ void anonymousDownloadReturnsTooManyRequestsWhenIpBucketIsExceeded() throws Exce
9396
((String) invocation.getArgument(0)).startsWith("ratelimit:download:ip:") ? false : true);
9497

9598
mockMvc.perform(get("/api/v1/skills/global/demo-skill/versions/1.0.0/download")
96-
.header("X-Forwarded-For", "203.0.113.10"))
99+
.header("X-Forwarded-For", "203.0.113.10")
100+
.with(user("anonymous-test")))
97101
.andExpect(status().isTooManyRequests())
98102
.andExpect(jsonPath("$.code").value(429));
99103

0 commit comments

Comments
 (0)