mirror of
https://github.com/iflytek/skillhub.git
synced 2026-10-10 03:27:54 +00:00
fix(storage): defer S3 bucket verification until first access (#289)
* fix(storage): defer S3 bucket verification until first access * test(storage): cover deferred S3 bucket verification * fix(runtime): widen backend container healthcheck window * fix(runtime): widen backend container healthcheck window * fix(test): use ddl-auto=create to prevent cross-context table drops Multiple @SpringBootTest classes with different @MockBean configs cause separate Spring contexts sharing the same H2 in-memory database. With create-drop, one context's shutdown drops tables needed by another, causing "Table not found (this database is empty)" errors. * fix(test): widen awaitIndexedDocument timeout to 15s CI runners are resource-constrained and async search indexing may not complete within the previous 5-second window, causing flaky failures.
This commit is contained in:
parent
02b3ac5b62
commit
348eb4e717
6 changed files with 136 additions and 15 deletions
|
|
@ -32,7 +32,7 @@ RUN mkdir -p /var/lib/skillhub/storage && \
|
|||
USER app
|
||||
|
||||
EXPOSE 8080
|
||||
HEALTHCHECK --interval=10s --timeout=3s \
|
||||
HEALTHCHECK --interval=10s --timeout=3s --start-period=60s --retries=12 \
|
||||
CMD wget -qO- http://localhost:8080/actuator/health || exit 1
|
||||
|
||||
ENTRYPOINT ["java", "-XX:MaxRAMPercentage=75.0", "-jar", "app.jar"]
|
||||
|
|
|
|||
|
|
@ -13,7 +13,7 @@ RUN chown -R app:app /app
|
|||
USER app
|
||||
|
||||
EXPOSE 8080
|
||||
HEALTHCHECK --interval=10s --timeout=3s \
|
||||
HEALTHCHECK --interval=10s --timeout=3s --start-period=60s --retries=12 \
|
||||
CMD wget -qO- http://localhost:8080/actuator/health || exit 1
|
||||
|
||||
ENTRYPOINT ["java", "-XX:MaxRAMPercentage=75.0", "-jar", "app.jar"]
|
||||
|
|
|
|||
|
|
@ -155,7 +155,7 @@ class SkillApprovalVisibilityFlowIntegrationTest {
|
|||
}
|
||||
|
||||
private SkillSearchDocumentEntity awaitIndexedDocument(Long skillId) throws InterruptedException {
|
||||
Instant deadline = Instant.now().plus(Duration.ofSeconds(5));
|
||||
Instant deadline = Instant.now().plus(Duration.ofSeconds(15));
|
||||
Optional<SkillSearchDocumentEntity> indexed = skillSearchDocumentJpaRepository.findBySkillId(skillId);
|
||||
while (indexed.isEmpty() && Instant.now().isBefore(deadline)) {
|
||||
Thread.sleep(100L);
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ spring:
|
|||
password:
|
||||
jpa:
|
||||
hibernate:
|
||||
ddl-auto: create-drop
|
||||
ddl-auto: create
|
||||
database-platform: org.hibernate.dialect.H2Dialect
|
||||
flyway:
|
||||
enabled: false
|
||||
|
|
|
|||
|
|
@ -32,8 +32,10 @@ import java.util.List;
|
|||
public class S3StorageService implements ObjectStorageService {
|
||||
private static final Logger log = LoggerFactory.getLogger(S3StorageService.class);
|
||||
private final S3StorageProperties properties;
|
||||
private final Object bucketPreparationLock = new Object();
|
||||
private S3Client s3Client;
|
||||
private S3Presigner s3Presigner;
|
||||
private volatile boolean bucketPrepared;
|
||||
|
||||
public S3StorageService(S3StorageProperties properties) { this.properties = properties; }
|
||||
|
||||
|
|
@ -42,6 +44,13 @@ public class S3StorageService implements ObjectStorageService {
|
|||
ApacheHttpClient.Builder httpClientBuilder = ApacheHttpClient.builder()
|
||||
.maxConnections(properties.getMaxConnections())
|
||||
.connectionAcquisitionTimeout(properties.getConnectionAcquisitionTimeout());
|
||||
this.s3Client = buildS3Client(httpClientBuilder);
|
||||
this.s3Presigner = buildPresigner();
|
||||
log.info("Initialized S3 storage client for bucket '{}' (bucket verification is deferred until first storage access)",
|
||||
properties.getBucket());
|
||||
}
|
||||
|
||||
protected S3Client buildS3Client(ApacheHttpClient.Builder httpClientBuilder) {
|
||||
var builder = S3Client.builder()
|
||||
.region(Region.of(properties.getRegion()))
|
||||
.credentialsProvider(StaticCredentialsProvider.create(
|
||||
|
|
@ -54,9 +63,7 @@ public class S3StorageService implements ObjectStorageService {
|
|||
if (properties.getEndpoint() != null && !properties.getEndpoint().isBlank()) {
|
||||
builder.endpointOverride(URI.create(properties.getEndpoint()));
|
||||
}
|
||||
this.s3Client = builder.build();
|
||||
this.s3Presigner = buildPresigner();
|
||||
ensureBucketExists();
|
||||
return builder.build();
|
||||
}
|
||||
|
||||
S3Presigner buildPresigner() {
|
||||
|
|
@ -75,20 +82,28 @@ public class S3StorageService implements ObjectStorageService {
|
|||
return presignerBuilder.build();
|
||||
}
|
||||
|
||||
private void ensureBucketExists() {
|
||||
if (!properties.isAutoCreateBucket()) {
|
||||
s3Client.headBucket(HeadBucketRequest.builder().bucket(properties.getBucket()).build());
|
||||
private void ensureBucketPrepared() {
|
||||
if (!properties.isAutoCreateBucket() || bucketPrepared) {
|
||||
return;
|
||||
}
|
||||
try { s3Client.headBucket(HeadBucketRequest.builder().bucket(properties.getBucket()).build()); }
|
||||
catch (NoSuchBucketException e) {
|
||||
log.info("Bucket '{}' does not exist, creating...", properties.getBucket());
|
||||
s3Client.createBucket(CreateBucketRequest.builder().bucket(properties.getBucket()).build());
|
||||
|
||||
synchronized (bucketPreparationLock) {
|
||||
if (bucketPrepared) {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
s3Client.headBucket(HeadBucketRequest.builder().bucket(properties.getBucket()).build());
|
||||
} catch (NoSuchBucketException e) {
|
||||
log.info("Bucket '{}' does not exist, creating...", properties.getBucket());
|
||||
s3Client.createBucket(CreateBucketRequest.builder().bucket(properties.getBucket()).build());
|
||||
}
|
||||
bucketPrepared = true;
|
||||
}
|
||||
}
|
||||
|
||||
@Override public void putObject(String key, InputStream data, long size, String contentType) {
|
||||
try {
|
||||
ensureBucketPrepared();
|
||||
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);
|
||||
|
|
@ -97,6 +112,7 @@ public class S3StorageService implements ObjectStorageService {
|
|||
|
||||
@Override public InputStream getObject(String key) {
|
||||
try {
|
||||
ensureBucketPrepared();
|
||||
return s3Client.getObject(GetObjectRequest.builder().bucket(properties.getBucket()).key(key).build());
|
||||
} catch (RuntimeException e) {
|
||||
throw new StorageAccessException("getObject", key, e);
|
||||
|
|
@ -105,6 +121,7 @@ public class S3StorageService implements ObjectStorageService {
|
|||
|
||||
@Override public void deleteObject(String key) {
|
||||
try {
|
||||
ensureBucketPrepared();
|
||||
s3Client.deleteObject(DeleteObjectRequest.builder().bucket(properties.getBucket()).key(key).build());
|
||||
} catch (RuntimeException e) {
|
||||
throw new StorageAccessException("deleteObject", key, e);
|
||||
|
|
@ -114,6 +131,7 @@ public class S3StorageService implements ObjectStorageService {
|
|||
@Override public void deleteObjects(List<String> keys) {
|
||||
if (keys.isEmpty()) return;
|
||||
try {
|
||||
ensureBucketPrepared();
|
||||
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) {
|
||||
|
|
@ -122,13 +140,18 @@ public class S3StorageService implements ObjectStorageService {
|
|||
}
|
||||
|
||||
@Override public boolean exists(String key) {
|
||||
try { s3Client.headObject(HeadObjectRequest.builder().bucket(properties.getBucket()).key(key).build()); return true; }
|
||||
try {
|
||||
ensureBucketPrepared();
|
||||
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) {
|
||||
try {
|
||||
ensureBucketPrepared();
|
||||
HeadObjectResponse resp = s3Client.headObject(HeadObjectRequest.builder().bucket(properties.getBucket()).key(key).build());
|
||||
return new ObjectMetadata(resp.contentLength(), resp.contentType(), resp.lastModified());
|
||||
} catch (RuntimeException e) {
|
||||
|
|
|
|||
|
|
@ -2,12 +2,32 @@ package com.iflytek.skillhub.storage;
|
|||
|
||||
import org.junit.jupiter.api.Test;
|
||||
import software.amazon.awssdk.services.s3.model.GetObjectRequest;
|
||||
import software.amazon.awssdk.core.sync.RequestBody;
|
||||
import software.amazon.awssdk.http.apache.ApacheHttpClient;
|
||||
import software.amazon.awssdk.services.s3.S3Client;
|
||||
import software.amazon.awssdk.services.s3.model.CreateBucketRequest;
|
||||
import software.amazon.awssdk.services.s3.model.CreateBucketResponse;
|
||||
import software.amazon.awssdk.services.s3.model.HeadBucketRequest;
|
||||
import software.amazon.awssdk.services.s3.model.NoSuchBucketException;
|
||||
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
|
||||
import software.amazon.awssdk.services.s3.model.PutObjectResponse;
|
||||
import software.amazon.awssdk.services.s3.presigner.S3Presigner;
|
||||
import software.amazon.awssdk.services.s3.presigner.model.GetObjectPresignRequest;
|
||||
|
||||
import java.io.ByteArrayInputStream;
|
||||
import java.net.URI;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.time.Duration;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.Mockito.doThrow;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.never;
|
||||
import static org.mockito.Mockito.times;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.verifyNoInteractions;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
class S3StorageServiceTest {
|
||||
|
||||
|
|
@ -27,6 +47,63 @@ class S3StorageServiceTest {
|
|||
assertThat(presignedUrl.getPath()).isEqualTo("/artifacts/package.tgz");
|
||||
}
|
||||
|
||||
@Test
|
||||
void initShouldNotProbeBucketWhenAutoCreateIsDisabled() {
|
||||
S3Client client = mock(S3Client.class);
|
||||
S3Presigner presigner = mock(S3Presigner.class);
|
||||
TestableS3StorageService service = new TestableS3StorageService(properties(false), client, presigner);
|
||||
|
||||
service.init();
|
||||
|
||||
verifyNoInteractions(client);
|
||||
}
|
||||
|
||||
@Test
|
||||
void putObjectShouldSkipBucketProbeWhenAutoCreateIsDisabled() {
|
||||
S3Client client = mock(S3Client.class);
|
||||
S3Presigner presigner = mock(S3Presigner.class);
|
||||
when(client.putObject(any(PutObjectRequest.class), any(RequestBody.class)))
|
||||
.thenReturn(PutObjectResponse.builder().eTag("etag").build());
|
||||
TestableS3StorageService service = new TestableS3StorageService(properties(false), client, presigner);
|
||||
|
||||
service.init();
|
||||
byte[] content = "hello".getBytes(StandardCharsets.UTF_8);
|
||||
service.putObject("packages/demo.zip", new ByteArrayInputStream(content), content.length, "application/zip");
|
||||
|
||||
verify(client, never()).headBucket(any(HeadBucketRequest.class));
|
||||
verify(client, never()).createBucket(any(CreateBucketRequest.class));
|
||||
verify(client).putObject(any(PutObjectRequest.class), any(RequestBody.class));
|
||||
}
|
||||
|
||||
@Test
|
||||
void putObjectShouldCreateBucketOnlyOnceWhenAutoCreateIsEnabled() {
|
||||
S3Client client = mock(S3Client.class);
|
||||
S3Presigner presigner = mock(S3Presigner.class);
|
||||
doThrow(NoSuchBucketException.builder().message("missing").build())
|
||||
.when(client).headBucket(any(HeadBucketRequest.class));
|
||||
when(client.createBucket(any(CreateBucketRequest.class)))
|
||||
.thenReturn(CreateBucketResponse.builder().build());
|
||||
when(client.putObject(any(PutObjectRequest.class), any(RequestBody.class)))
|
||||
.thenReturn(PutObjectResponse.builder().eTag("etag").build());
|
||||
TestableS3StorageService service = new TestableS3StorageService(properties(true), client, presigner);
|
||||
|
||||
service.init();
|
||||
byte[] content = "hello".getBytes(StandardCharsets.UTF_8);
|
||||
service.putObject("packages/demo-1.zip", new ByteArrayInputStream(content), content.length, "application/zip");
|
||||
service.putObject("packages/demo-2.zip", new ByteArrayInputStream(content), content.length, "application/zip");
|
||||
|
||||
verify(client, times(1)).headBucket(any(HeadBucketRequest.class));
|
||||
verify(client, times(1)).createBucket(any(CreateBucketRequest.class));
|
||||
verify(client, times(2)).putObject(any(PutObjectRequest.class), any(RequestBody.class));
|
||||
}
|
||||
|
||||
private S3StorageProperties properties(boolean autoCreateBucket) {
|
||||
S3StorageProperties properties = createProperties(true);
|
||||
properties.setBucket("skillhub");
|
||||
properties.setAutoCreateBucket(autoCreateBucket);
|
||||
return properties;
|
||||
}
|
||||
|
||||
private URI presignGetObjectUrl(boolean forcePathStyle) {
|
||||
S3StorageService storageService = new S3StorageService(createProperties(forcePathStyle));
|
||||
try (var presigner = storageService.buildPresigner()) {
|
||||
|
|
@ -53,4 +130,25 @@ class S3StorageServiceTest {
|
|||
properties.setForcePathStyle(forcePathStyle);
|
||||
return properties;
|
||||
}
|
||||
|
||||
private static final class TestableS3StorageService extends S3StorageService {
|
||||
private final S3Client client;
|
||||
private final S3Presigner presigner;
|
||||
|
||||
private TestableS3StorageService(S3StorageProperties properties, S3Client client, S3Presigner presigner) {
|
||||
super(properties);
|
||||
this.client = client;
|
||||
this.presigner = presigner;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected S3Client buildS3Client(ApacheHttpClient.Builder httpClientBuilder) {
|
||||
return client;
|
||||
}
|
||||
|
||||
@Override
|
||||
S3Presigner buildPresigner() {
|
||||
return presigner;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue