From 20219b252f4750d1d6fde92b3393e61674b6445a Mon Sep 17 00:00:00 2001 From: iamgreendev <56599296+BanelhaqB@users.noreply.github.com> Date: Wed, 2 Sep 2026 13:52:05 +0200 Subject: [PATCH 1/6] refactor(file-storage): make the storage port backend-agnostic The port used to speak filesystem: get(path), deleteByPath(path) and an upload returning an absolute path. Plugging S3 under that vocabulary would have made the abstraction lie about what it does. Upload now returns a locator that is opaque to the domain: it is persisted as File.uri and handed back to the very same adapter to read or delete the content. Each adapter picks its own format, so the local adapter keeps writing and reading the exact same absolute paths as before and no stored row becomes invalid. Also drops delete(UUID), which no production code ever called. refs: #2321 --- .../output/service/FileStorageService.java | 15 ++++--- .../service/FileResourceServiceImpl.java | 2 +- .../service/FileStorageServiceImpl.java | 43 ++++++------------- .../service/FileStorageServiceMock.java | 16 +++---- .../service/FileResourceServiceImplTest.java | 1 - 5 files changed, 28 insertions(+), 49 deletions(-) diff --git a/src/main/java/fr/avenirsesr/portfolio/file/domain/port/output/service/FileStorageService.java b/src/main/java/fr/avenirsesr/portfolio/file/domain/port/output/service/FileStorageService.java index 2b2ccf457..82480ddb9 100644 --- a/src/main/java/fr/avenirsesr/portfolio/file/domain/port/output/service/FileStorageService.java +++ b/src/main/java/fr/avenirsesr/portfolio/file/domain/port/output/service/FileStorageService.java @@ -2,14 +2,19 @@ import fr.avenirsesr.portfolio.file.domain.exception.FileStorageException; import fr.avenirsesr.portfolio.file.domain.model.FileResource; -import java.util.UUID; +/** + * Storage backend abstraction. + * + *

Uploading returns a locator that is opaque to the domain: it is persisted as {@code File.uri} + * and handed back to the very same adapter to read or delete the content. Each adapter picks its + * own format — an absolute path for local storage, an object key for S3 — so the domain never has + * to interpret it. + */ public interface FileStorageService { - byte[] get(String path) throws FileStorageException; - String upload(FileResource fileResource) throws FileStorageException; - void delete(UUID fileId) throws FileStorageException; + byte[] get(String locator) throws FileStorageException; - void deleteByPath(String path) throws FileStorageException; + void delete(String locator) throws FileStorageException; } diff --git a/src/main/java/fr/avenirsesr/portfolio/file/domain/service/FileResourceServiceImpl.java b/src/main/java/fr/avenirsesr/portfolio/file/domain/service/FileResourceServiceImpl.java index f66f0d44b..89ceaf519 100644 --- a/src/main/java/fr/avenirsesr/portfolio/file/domain/service/FileResourceServiceImpl.java +++ b/src/main/java/fr/avenirsesr/portfolio/file/domain/service/FileResourceServiceImpl.java @@ -84,7 +84,7 @@ public FileDownload download(UUID fileId) { @Override public void delete(UUID fileId) { var file = fileRepository.findById(fileId).orElseThrow(FileNotFoundException::new); - fileStorageService.deleteByPath(file.getUri()); + fileStorageService.delete(file.getUri()); fileRepository.removeFromDatabase(file); log.info("File deleted: {}", file); } diff --git a/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageServiceImpl.java b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageServiceImpl.java index a692375ee..9a3074a58 100644 --- a/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageServiceImpl.java +++ b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageServiceImpl.java @@ -8,11 +8,14 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; -import java.util.UUID; import lombok.extern.slf4j.Slf4j; import org.springframework.context.annotation.Primary; import org.springframework.stereotype.Component; +/** + * Stores files on the local filesystem. The locator handed back to the domain is the absolute path + * of the written file. + */ @Slf4j @Component @Primary @@ -39,8 +42,8 @@ public String upload(FileResource fileResource) { } @Override - public byte[] get(String path) { - File file = new File(path); + public byte[] get(String locator) { + File file = new File(locator); if (!file.exists()) { throw new FileNotFoundException(); @@ -49,45 +52,23 @@ public byte[] get(String path) { try { return java.nio.file.Files.readAllBytes(file.toPath()); } catch (IOException e) { - throw new FileStorageException("Failed to read file at path " + path, e); + throw new FileStorageException("Failed to read file at path " + locator, e); } } @Override - public void delete(UUID id) { - String uploadDir = System.getProperty("user.dir") + FileStorageConstants.STORAGE_PATH; - - File dir = new File(uploadDir); - if (!dir.exists()) { - throw new IllegalStateException("File storage directory does not exist"); - } - - File[] matchingFiles = dir.listFiles((d, name) -> name.startsWith(id.toString() + ".")); - if (matchingFiles == null || matchingFiles.length == 0) { - log.error("No file with id {} found", id); - throw new FileNotFoundException(); - } - - File fileToDelete = matchingFiles[0]; - if (!fileToDelete.delete()) { - throw new FileStorageException("Failed to delete file with id " + id, null); - } - log.info("File with id {} has been deleted", id); - } - - @Override - public void deleteByPath(String path) { - File file = new File(path); + public void delete(String locator) { + File file = new File(locator); if (!file.exists()) { - log.error("No file found at path {}", path); + log.error("No file found at path {}", locator); throw new FileNotFoundException(); } if (!file.delete()) { - throw new FileStorageException("Failed to delete file at path " + path, null); + throw new FileStorageException("Failed to delete file at path " + locator, null); } - log.info("File at path {} has been deleted", path); + log.info("File at path {} has been deleted", locator); } } diff --git a/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageServiceMock.java b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageServiceMock.java index ab3a8f304..67fd427c6 100644 --- a/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageServiceMock.java +++ b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageServiceMock.java @@ -9,7 +9,6 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; -import java.util.UUID; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Component; @@ -65,9 +64,9 @@ private boolean ensurePlaceholderExists(String path) { } @Override - public byte[] get(String path) { - log.debug("Mocking get file resource {} return placeholder file", path); - File file = new File(path); + public byte[] get(String locator) { + log.debug("Mocking get file resource {} return placeholder file", locator); + File file = new File(locator); if (!file.exists()) { throw new FileNotFoundException(); @@ -81,12 +80,7 @@ public byte[] get(String path) { } @Override - public void delete(UUID id) { - log.debug("Mocking delete file resource {}", id); - } - - @Override - public void deleteByPath(String path) throws FileStorageException { - log.debug("Mocking delete file resource by path {}", path); + public void delete(String locator) { + log.debug("Mocking delete file resource {}", locator); } } diff --git a/src/test/java/fr/avenirsesr/portfolio/file/domain/service/FileResourceServiceImplTest.java b/src/test/java/fr/avenirsesr/portfolio/file/domain/service/FileResourceServiceImplTest.java index 053dcf690..0074e90f9 100644 --- a/src/test/java/fr/avenirsesr/portfolio/file/domain/service/FileResourceServiceImplTest.java +++ b/src/test/java/fr/avenirsesr/portfolio/file/domain/service/FileResourceServiceImplTest.java @@ -123,7 +123,6 @@ void thenItShouldLeaveTheSourceFileUntouched() { fileResourceService.copy(sourceId); - verify(fileStorageService, never()).deleteByPath(any()); verify(fileStorageService, never()).delete(any()); verify(fileRepository, never()).removeFromDatabase(any()); } From 7ab5c6ee1dff48d6baf04365d190f0b6f721577b Mon Sep 17 00:00:00 2001 From: iamgreendev <56599296+BanelhaqB@users.noreply.github.com> Date: Wed, 2 Sep 2026 13:56:36 +0200 Subject: [PATCH 2/6] fix(file): build file URLs from the storage endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FileDtoMapper exposed File.uri straight as the DTO url, and the shared FileDTOMapper concatenated the request origin with it. Both leaked a storage locator to clients: in production the second one produced URLs like https://host/workspace/app/target/storage/.png, which no route serves — the Apache overlay has no alias for that path. Both now point at the /storage/{fileId} endpoint, the way FileDataMapper already did. The construction is centralised in FileStorageConstants so the three call sites cannot drift apart again. This changes the url field returned by the trace, feedback, user photo and activity file endpoints. refs: #2321 --- .../adapter/mapper/FileDtoMapper.java | 18 ++++++++++++++---- .../file/domain/mapper/FileDataMapper.java | 2 +- .../configuration/FileStorageConstants.java | 10 ++++++++++ .../adapter/mapper/FileDTOMapper.java | 4 +++- .../mapper/FeedbackOverviewDTOMapperTest.java | 10 +++++++++- 5 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/main/java/fr/avenirsesr/portfolio/file/application/adapter/mapper/FileDtoMapper.java b/src/main/java/fr/avenirsesr/portfolio/file/application/adapter/mapper/FileDtoMapper.java index f762c90cf..4f3219ca1 100644 --- a/src/main/java/fr/avenirsesr/portfolio/file/application/adapter/mapper/FileDtoMapper.java +++ b/src/main/java/fr/avenirsesr/portfolio/file/application/adapter/mapper/FileDtoMapper.java @@ -2,15 +2,25 @@ import fr.avenirsesr.portfolio.file.application.adapter.dto.FileDTO; import fr.avenirsesr.portfolio.file.domain.model.File; +import fr.avenirsesr.portfolio.file.infrastructure.configuration.FileStorageConstants; import java.util.Optional; import org.mapstruct.Mapper; -import org.mapstruct.Mapping; @Mapper(componentModel = "spring") public interface FileDtoMapper { - @Mapping(source = "size", target = "fileSize") - @Mapping(source = "uri", target = "url") - FileDTO fromDomain(File file); + default FileDTO fromDomain(File file) { + if (file == null) { + return null; + } + + return new FileDTO( + file.getId(), + file.getFileName(), + file.getFileType(), + file.getSize(), + FileStorageConstants.publicUrlOf(file.getId()), + file.getUploadedAt()); + } default FileDTO fromDomain(Optional file) { return file.map(this::fromDomain).orElse(null); diff --git a/src/main/java/fr/avenirsesr/portfolio/file/domain/mapper/FileDataMapper.java b/src/main/java/fr/avenirsesr/portfolio/file/domain/mapper/FileDataMapper.java index f60c756d5..41f599fa9 100644 --- a/src/main/java/fr/avenirsesr/portfolio/file/domain/mapper/FileDataMapper.java +++ b/src/main/java/fr/avenirsesr/portfolio/file/domain/mapper/FileDataMapper.java @@ -18,6 +18,6 @@ static FileData mapFileData(File file, String defaultUrl) { return new FileData( Optional.ofNullable(file.getId()), Optional.ofNullable(file.getFileName()), - FileStorageConstants.PHOTO_ENDPOINT_PREFIX + "/" + file.getId()); + FileStorageConstants.publicUrlOf(file.getId())); } } diff --git a/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/FileStorageConstants.java b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/FileStorageConstants.java index 68b96cfed..a3691e235 100644 --- a/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/FileStorageConstants.java +++ b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/FileStorageConstants.java @@ -45,6 +45,16 @@ public class FileStorageConstants { public static String DEFAULT_PROFILE_FILE_URL; public static String DEFAULT_COVER_FILE_URL; + /** + * Builds the URL serving the content of the given file, handled by {@code StorageController}. + * + *

This is the only supported way to expose a file: {@code File.uri} holds a storage locator + * that is meaningful to the storage adapter alone, never to a client. + */ + public static String publicUrlOf(UUID fileId) { + return PHOTO_ENDPOINT_PREFIX + "/" + fileId; + } + @PostConstruct private void init() { STORAGE_PATH = storagePath; diff --git a/src/main/java/fr/avenirsesr/portfolio/shared/application/adapter/mapper/FileDTOMapper.java b/src/main/java/fr/avenirsesr/portfolio/shared/application/adapter/mapper/FileDTOMapper.java index e38f332f3..83cb9c3c0 100644 --- a/src/main/java/fr/avenirsesr/portfolio/shared/application/adapter/mapper/FileDTOMapper.java +++ b/src/main/java/fr/avenirsesr/portfolio/shared/application/adapter/mapper/FileDTOMapper.java @@ -1,6 +1,7 @@ package fr.avenirsesr.portfolio.shared.application.adapter.mapper; import fr.avenirsesr.portfolio.file.domain.model.File; +import fr.avenirsesr.portfolio.file.infrastructure.configuration.FileStorageConstants; import fr.avenirsesr.portfolio.shared.application.adapter.dto.FileDTO; import java.util.List; import org.mapstruct.Mapper; @@ -9,7 +10,8 @@ public interface FileDTOMapper { default FileDTO toFileDTO(File file, String baseUrl) { - return new FileDTO(file.getId(), file.getFileName(), baseUrl + file.getUri()); + return new FileDTO( + file.getId(), file.getFileName(), baseUrl + FileStorageConstants.publicUrlOf(file.getId())); } default List toFileDTOs(List files, String baseUrl) { diff --git a/src/test/java/fr/avenirsesr/portfolio/student/activity/application/adapter/mapper/FeedbackOverviewDTOMapperTest.java b/src/test/java/fr/avenirsesr/portfolio/student/activity/application/adapter/mapper/FeedbackOverviewDTOMapperTest.java index f17aafafa..560af8f17 100644 --- a/src/test/java/fr/avenirsesr/portfolio/student/activity/application/adapter/mapper/FeedbackOverviewDTOMapperTest.java +++ b/src/test/java/fr/avenirsesr/portfolio/student/activity/application/adapter/mapper/FeedbackOverviewDTOMapperTest.java @@ -6,6 +6,7 @@ import fr.avenirsesr.portfolio.common.testutils.BddLogger; import fr.avenirsesr.portfolio.file.application.adapter.mapper.FileDtoMapper; import fr.avenirsesr.portfolio.file.domain.model.File; +import fr.avenirsesr.portfolio.file.infrastructure.configuration.FileStorageConstants; import fr.avenirsesr.portfolio.file.infrastructure.fixture.FileFixture; import fr.avenirsesr.portfolio.student.activity.application.adapter.dto.FeedbackOverviewDTO; import fr.avenirsesr.portfolio.student.activity.domain.model.DeclaredActivity; @@ -16,6 +17,7 @@ import java.time.Instant; import java.util.List; import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mapstruct.factory.Mappers; @@ -30,6 +32,12 @@ class FeedbackOverviewDTOMapperTest { @InjectMocks private FeedbackOverviewDTOMapperImpl mapper; + @BeforeEach + void setUpStorageEndpoint() { + // Normally injected by Spring from file.storage.endpoint-prefix; there is no context here. + FileStorageConstants.PHOTO_ENDPOINT_PREFIX = "/storage"; + } + @Test void shouldMapFeedbackToOverviewDTO_withStudentAndStaffFromActivity() { BddLogger.given( @@ -181,6 +189,6 @@ void shouldMapFeedbackAttachmentsToOverviewDTO_whenAttachmentsPresent() { var attachmentDTO = dto.attachments().get(0); assertEquals(attachment.getId(), attachmentDTO.id()); assertEquals(attachment.getFileName(), attachmentDTO.fileName()); - assertEquals(attachment.getUri(), attachmentDTO.url()); + assertEquals("/storage/" + attachment.getId(), attachmentDTO.url()); } } From 4a657df528722f904831da31de4b237d29099ac4 Mon Sep 17 00:00:00 2001 From: iamgreendev <56599296+BanelhaqB@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:04:19 +0200 Subject: [PATCH 3/6] feat(file-storage): add an S3 storage adapter S3FileStorageService stores objects under the {uuid}.{extension} key and hands that key back as the locator. The content type is set on the object so a client reading it back is told what it is rather than getting application/octet-stream. The two real adapters are mutually exclusive on file.storage.type and both keep @Primary, which the seeder mock relies on to stay resolvable through its own qualifier. Local storage remains the default, so nothing changes until the property is set to s3. Behaviour difference worth knowing: S3 answers successfully when deleting a key that does not exist, so the adapter cannot report a missing file the way the local one does. Default profile and cover pictures are now read as Spring resource locations instead of going through the storage backend. They ship with the deployment and belong to no user, so routing a static asset through the bucket would have tied it to the backend availability. Keys are flat for now. Prefixing them per domain needs the upload signatures to carry the calling context, which is the streaming refactor's job. refs: #2321 --- pom.xml | 16 ++++ .../adapter/controller/StorageController.java | 49 ++++++---- .../service/FileStorageServiceImpl.java | 5 ++ .../adapter/service/S3FileStorageService.java | 85 ++++++++++++++++++ .../configuration/S3ClientConfig.java | 32 +++++++ .../configuration/S3StorageProperties.java | 34 +++++++ src/main/resources/application.properties | 14 ++- .../controller/StorageControllerIT.java | 13 +-- .../resources/application-test.properties | 8 +- src/test/resources/defaults/cover-picture.png | Bin 0 -> 70 bytes .../resources/defaults/profile-picture.png | Bin 0 -> 70 bytes 11 files changed, 223 insertions(+), 33 deletions(-) create mode 100644 src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageService.java create mode 100644 src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/S3ClientConfig.java create mode 100644 src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/S3StorageProperties.java create mode 100644 src/test/resources/defaults/cover-picture.png create mode 100644 src/test/resources/defaults/profile-picture.png diff --git a/pom.xml b/pom.xml index 1e19516f8..20f227468 100644 --- a/pom.xml +++ b/pom.xml @@ -32,7 +32,19 @@ ${maven.build.timestamp} 1.6.3 0.2.0 + 2.54.10 + + + + software.amazon.awssdk + bom + ${awssdk.version} + pom + import + + + org.springframework.boot @@ -161,6 +173,10 @@ 4.12.0 test + + software.amazon.awssdk + s3 + diff --git a/src/main/java/fr/avenirsesr/portfolio/file/application/adapter/controller/StorageController.java b/src/main/java/fr/avenirsesr/portfolio/file/application/adapter/controller/StorageController.java index c90c0c00f..f82b699b6 100644 --- a/src/main/java/fr/avenirsesr/portfolio/file/application/adapter/controller/StorageController.java +++ b/src/main/java/fr/avenirsesr/portfolio/file/application/adapter/controller/StorageController.java @@ -1,14 +1,20 @@ package fr.avenirsesr.portfolio.file.application.adapter.controller; +import fr.avenirsesr.portfolio.file.domain.exception.FileNotFoundException; +import fr.avenirsesr.portfolio.file.domain.exception.FileStorageException; import fr.avenirsesr.portfolio.file.domain.model.FileResource; +import fr.avenirsesr.portfolio.file.domain.model.enums.EFileType; import fr.avenirsesr.portfolio.file.domain.port.input.FileResourceService; -import fr.avenirsesr.portfolio.file.domain.port.output.service.FileStorageService; import fr.avenirsesr.portfolio.file.infrastructure.configuration.FileStorageConstants; import jakarta.validation.Valid; +import java.io.IOException; +import java.io.InputStream; import java.util.UUID; import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.core.io.ByteArrayResource; +import org.springframework.core.io.Resource; +import org.springframework.core.io.ResourceLoader; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.util.MimeType; @@ -23,7 +29,7 @@ @RequestMapping("/storage") public class StorageController { private final FileResourceService fileResourceService; - private final FileStorageService fileStorageService; + private final ResourceLoader resourceLoader; @GetMapping("/{fileId}") public ResponseEntity getResourceByFileId(@Valid @PathVariable UUID fileId) { @@ -38,24 +44,37 @@ public ResponseEntity getResourceByFileId(@Valid @PathVariabl @GetMapping("/default/cover-picture") public ResponseEntity getDefaultCoverPicture() { log.debug("Received request to get default cover photo"); - byte[] photo = fileStorageService.get(FileStorageConstants.COVER_DEFAULT_PATH); - - return ResponseEntity.ok() - .contentType( - MediaType.asMediaType( - MimeType.valueOf(FileStorageConstants.COVER_DEFAULT_FILE_TYPE.getMimeType()))) - .body(new ByteArrayResource(photo)); + return serveDefaultPicture( + FileStorageConstants.COVER_DEFAULT_PATH, FileStorageConstants.COVER_DEFAULT_FILE_TYPE); } @GetMapping("/default/profile-picture") public ResponseEntity getDefaultProfilePicture() { log.debug("Received request to get default profile photo"); - byte[] photo = fileStorageService.get(FileStorageConstants.PROFILE_DEFAULT_PATH); + return serveDefaultPicture( + FileStorageConstants.PROFILE_DEFAULT_PATH, FileStorageConstants.PROFILE_DEFAULT_FILE_TYPE); + } - return ResponseEntity.ok() - .contentType( - MediaType.asMediaType( - MimeType.valueOf(FileStorageConstants.PROFILE_DEFAULT_FILE_TYPE.getMimeType()))) - .body(new ByteArrayResource(photo)); + /** + * Serves a fallback picture from a Spring resource location rather than from the storage backend. + * These pictures ship with the deployment and belong to no user, so making them travel through + * the bucket would tie a static asset to the availability of the storage backend. + */ + private ResponseEntity serveDefaultPicture( + String location, EFileType fileType) { + Resource resource = resourceLoader.getResource(location); + + if (!resource.exists()) { + log.error("No default picture found at location {}", location); + throw new FileNotFoundException(); + } + + try (InputStream content = resource.getInputStream()) { + return ResponseEntity.ok() + .contentType(MediaType.asMediaType(MimeType.valueOf(fileType.getMimeType()))) + .body(new ByteArrayResource(content.readAllBytes())); + } catch (IOException e) { + throw new FileStorageException("Failed to read default picture at location " + location, e); + } } } diff --git a/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageServiceImpl.java b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageServiceImpl.java index 9a3074a58..76350bdf7 100644 --- a/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageServiceImpl.java +++ b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageServiceImpl.java @@ -9,16 +9,21 @@ import java.io.FileOutputStream; import java.io.IOException; import lombok.extern.slf4j.Slf4j; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.context.annotation.Primary; import org.springframework.stereotype.Component; /** * Stores files on the local filesystem. The locator handed back to the domain is the absolute path * of the written file. + * + *

Default backend: it stays active as long as {@code file.storage.type} is not set to {@code + * s3}. */ @Slf4j @Component @Primary +@ConditionalOnProperty(name = "file.storage.type", havingValue = "local", matchIfMissing = true) public class FileStorageServiceImpl implements FileStorageService { @Override diff --git a/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageService.java b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageService.java new file mode 100644 index 000000000..61d17b448 --- /dev/null +++ b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageService.java @@ -0,0 +1,85 @@ +package fr.avenirsesr.portfolio.file.infrastructure.adapter.service; + +import fr.avenirsesr.portfolio.file.domain.exception.FileNotFoundException; +import fr.avenirsesr.portfolio.file.domain.exception.FileStorageException; +import fr.avenirsesr.portfolio.file.domain.model.FileResource; +import fr.avenirsesr.portfolio.file.domain.port.output.service.FileStorageService; +import fr.avenirsesr.portfolio.file.infrastructure.configuration.S3StorageProperties; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.annotation.Primary; +import org.springframework.stereotype.Component; +import software.amazon.awssdk.core.ResponseBytes; +import software.amazon.awssdk.core.sync.RequestBody; +import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.model.DeleteObjectRequest; +import software.amazon.awssdk.services.s3.model.GetObjectRequest; +import software.amazon.awssdk.services.s3.model.NoSuchKeyException; +import software.amazon.awssdk.services.s3.model.PutObjectRequest; +import software.amazon.awssdk.services.s3.model.S3Exception; + +/** + * Stores files in an S3-compatible bucket. The locator handed back to the domain is the object key. + */ +@Slf4j +@Component +@Primary +@RequiredArgsConstructor +@ConditionalOnProperty(name = "file.storage.type", havingValue = "s3") +public class S3FileStorageService implements FileStorageService { + private final S3Client s3Client; + private final S3StorageProperties properties; + + @Override + public String upload(FileResource fileResource) { + var key = fileResource.id() + "." + fileResource.fileType().name().toLowerCase(); + + try { + s3Client.putObject( + PutObjectRequest.builder() + .bucket(properties.getBucket()) + .key(key) + // Stored alongside the object so a client reading it back is told what it is. + .contentType(fileResource.fileType().getMimeType()) + .build(), + RequestBody.fromBytes(fileResource.content())); + } catch (S3Exception e) { + throw new FileStorageException("Failed to upload file " + fileResource.fileName(), e); + } + + log.info("File {} has been uploaded as {}", fileResource.fileName(), key); + return key; + } + + @Override + public byte[] get(String locator) { + try { + ResponseBytes object = + s3Client.getObjectAsBytes( + GetObjectRequest.builder().bucket(properties.getBucket()).key(locator).build()); + return object.asByteArray(); + } catch (NoSuchKeyException e) { + log.error("No object found for key {}", locator); + throw new FileNotFoundException(); + } catch (S3Exception e) { + throw new FileStorageException("Failed to read object with key " + locator, e); + } + } + + /** + * Deletes the object. S3 answers successfully for a key that does not exist, so unlike the local + * adapter this never reports a missing file. + */ + @Override + public void delete(String locator) { + try { + s3Client.deleteObject( + DeleteObjectRequest.builder().bucket(properties.getBucket()).key(locator).build()); + } catch (S3Exception e) { + throw new FileStorageException("Failed to delete object with key " + locator, e); + } + + log.info("Object with key {} has been deleted", locator); + } +} diff --git a/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/S3ClientConfig.java b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/S3ClientConfig.java new file mode 100644 index 000000000..dc1150860 --- /dev/null +++ b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/S3ClientConfig.java @@ -0,0 +1,32 @@ +package fr.avenirsesr.portfolio.file.infrastructure.configuration; + +import java.net.URI; +import lombok.RequiredArgsConstructor; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; +import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.s3.S3Client; + +@Configuration +@RequiredArgsConstructor +@EnableConfigurationProperties(S3StorageProperties.class) +@ConditionalOnProperty(name = "file.storage.type", havingValue = "s3") +public class S3ClientConfig { + private final S3StorageProperties properties; + + @Bean + public S3Client s3Client() { + return S3Client.builder() + .endpointOverride(URI.create(properties.getEndpoint())) + .region(Region.of(properties.getRegion())) + .credentialsProvider( + StaticCredentialsProvider.create( + AwsBasicCredentials.create(properties.getAccessKey(), properties.getSecretKey()))) + .forcePathStyle(properties.isPathStyleAccess()) + .build(); + } +} diff --git a/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/S3StorageProperties.java b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/S3StorageProperties.java new file mode 100644 index 000000000..921427bc2 --- /dev/null +++ b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/S3StorageProperties.java @@ -0,0 +1,34 @@ +package fr.avenirsesr.portfolio.file.infrastructure.configuration; + +import lombok.Getter; +import lombok.Setter; +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** Connection settings of the S3-compatible backend, read when {@code file.storage.type=s3}. */ +@Getter +@Setter +@ConfigurationProperties(prefix = "file.storage.s3") +public class S3StorageProperties { + + /** Base URL of the S3 API, for instance {@code https://s3.example.org}. */ + private String endpoint; + + /** + * Region sent along with the request signature. The AWS SDK refuses to build a client without one + * even when the backend ignores it, in which case any placeholder such as {@code us-east-1} does. + */ + private String region = "us-east-1"; + + /** Bucket holding every file of this environment. */ + private String bucket; + + private String accessKey; + + private String secretKey; + + /** + * Addresses buckets as {@code /} rather than {@code .}. + * Required by MinIO and Ceph deployments served from a bare host or an IP address. + */ + private boolean pathStyleAccess = true; +} diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index f8240a2c0..14ead0057 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -71,12 +71,22 @@ management.endpoints.web.exposure.include=health,info,caches,prometheus management.health.db.enabled=false management.health.redis.enabled=false #cdn +# Storage backend: local (filesystem) or s3 +file.storage.type=local file.storage.local-path=/target/storage -file.storage.profile.default-path=/workspace/app/target/storage/53d85f12-0e9b-4b18-aef9-e115b4984448.png -file.storage.cover.default-path=/workspace/app/target/storage/956e7537-5155-4fb2-9746-73e3e7820845.png +# Fallback pictures, resolved as Spring resource locations (file: or classpath:) +file.storage.profile.default-path=file:/workspace/app/target/storage/53d85f12-0e9b-4b18-aef9-e115b4984448.png +file.storage.cover.default-path=file:/workspace/app/target/storage/956e7537-5155-4fb2-9746-73e3e7820845.png file.storage.endpoint-prefix=/storage file.storage.profile.default-endpoint=/default/profile-picture file.storage.cover.default-endpoint=/default/cover-picture +# S3 backend, only read when file.storage.type=s3 +#file.storage.s3.endpoint=https://s3.example.org +#file.storage.s3.region=us-east-1 +#file.storage.s3.bucket=avenirs-portfolio-dev +#file.storage.s3.access-key=ENC(...) +#file.storage.s3.secret-key=ENC(...) +#file.storage.s3.path-style-access=true # Valkey spring.cache.type=redis spring.data.redis.host=avenirs-valkey diff --git a/src/test/java/fr/avenirsesr/portfolio/file/application/adapter/controller/StorageControllerIT.java b/src/test/java/fr/avenirsesr/portfolio/file/application/adapter/controller/StorageControllerIT.java index fb0ddf7f4..b6ac1592d 100644 --- a/src/test/java/fr/avenirsesr/portfolio/file/application/adapter/controller/StorageControllerIT.java +++ b/src/test/java/fr/avenirsesr/portfolio/file/application/adapter/controller/StorageControllerIT.java @@ -1,8 +1,5 @@ package fr.avenirsesr.portfolio.file.application.adapter.controller; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.when; - import fr.avenirsesr.portfolio.common.testutils.BddLogger; import fr.avenirsesr.portfolio.file.domain.port.output.repository.FileRepository; import fr.avenirsesr.portfolio.file.domain.port.output.service.FileStorageService; @@ -11,7 +8,6 @@ import fr.avenirsesr.portfolio.shared.infrastructure.ContainerConfigurationTest; import fr.avenirsesr.portfolio.shared.infrastructure.adapter.seeder.SeederRunner; import java.io.IOException; -import java.nio.charset.StandardCharsets; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -19,6 +15,7 @@ import org.mockito.MockitoAnnotations; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.core.io.DefaultResourceLoader; import org.springframework.http.MediaType; import org.springframework.test.web.reactive.server.WebTestClient; @@ -53,7 +50,7 @@ void init() { StorageController storageController = new StorageController( new FileResourceServiceImpl(fileStorageService, fileRepository, loggedInUserService), - fileStorageService); + new DefaultResourceLoader()); webTestClient = WebTestClient.bindToController(storageController).configureClient().baseUrl("").build(); @@ -63,9 +60,6 @@ void init() { void shouldGetDefaultUserProfilePhoto() { BddLogger.given("the " + DEFAULT_PROFILE + " endpoint"); - when(fileStorageService.get(anyString())) - .thenReturn("Contenu du fichier de test".getBytes(StandardCharsets.UTF_8)); - BddLogger.when("performing a GET with a PROFILE photo type"); BddLogger.then("it should return the default user profile photo"); @@ -85,9 +79,6 @@ void shouldGetDefaultUserProfilePhoto() { void shouldGetDefaultUserCoverPhoto() throws IOException { BddLogger.given("the " + DEFAULT_COVER + " endpoint"); - when(fileStorageService.get(anyString())) - .thenReturn("Contenu du fichier de test".getBytes(StandardCharsets.UTF_8)); - BddLogger.when("performing a GET with a COVER photo type"); BddLogger.then("it should return the default user cover photo"); diff --git a/src/test/resources/application-test.properties b/src/test/resources/application-test.properties index e1387c0a6..37866be18 100644 --- a/src/test/resources/application-test.properties +++ b/src/test/resources/application-test.properties @@ -23,12 +23,10 @@ user.no-permission.payload={"sub":"no.permission@university.com", "iat":"2019-01 user.no-permission.signature=hRd4hn/HyM9P1ZwUhMtnplTjPpyAidd0VZFZdBj8UbE= security.permit-all-paths=/avenirs-portfolio-api/api-docs/**,/avenirs-portfolio-api/swagger-ui/**,/favicon.ico,/actuator/health,/photo/**,/cover/**,/storage/** #cdn +file.storage.type=local file.storage.local-path=/target/storage -file.storage.user.profile.default-path=/workspace/app/target/storage/53d85f12-0e9b-4b18-aef9-e115b4984448.png -file.storage.user.cover.default-path=/workspace/app/target/storage/956e7537-5155-4fb2-9746-73e3e7820845.png -file.storage.user.endpoint-prefix=/storage/users -file.storage.user.profile.default-endpoint=/default/PROFILE -file.storage.user.cover.default-endpoint=/default/COVER +file.storage.profile.default-path=classpath:defaults/profile-picture.png +file.storage.cover.default-path=classpath:defaults/cover-picture.png # Batch avenirs.back-office.api-key=test-key external-skill.not-found-id=2f024a1c-5429-43f6-bb2e-ac5a3ca662e7 diff --git a/src/test/resources/defaults/cover-picture.png b/src/test/resources/defaults/cover-picture.png new file mode 100644 index 0000000000000000000000000000000000000000..f37764b1f7606623616dcdc169cc858273ea2d94 GIT binary patch literal 70 zcmeAS@N?(olHy`uVBq!ia0vp^j3CUx1|;Q0k92}1TpU9xZYBRYe;|OLfu)tPp=D){ QB2a?C)78&qol`;+0Lr!y6951J literal 0 HcmV?d00001 diff --git a/src/test/resources/defaults/profile-picture.png b/src/test/resources/defaults/profile-picture.png new file mode 100644 index 0000000000000000000000000000000000000000..f37764b1f7606623616dcdc169cc858273ea2d94 GIT binary patch literal 70 zcmeAS@N?(olHy`uVBq!ia0vp^j3CUx1|;Q0k92}1TpU9xZYBRYe;|OLfu)tPp=D){ QB2a?C)78&qol`;+0Lr!y6951J literal 0 HcmV?d00001 From edb82f59d53a502f672aae7127525d6c4d0a05cc Mon Sep 17 00:00:00 2001 From: iamgreendev <56599296+BanelhaqB@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:07:23 +0200 Subject: [PATCH 4/6] test(file-storage): cover the S3 storage adapter S3FileStorageServiceTest pins the contract against a mocked client: key format, content type, and the translation of a missing key into FileNotFoundException. S3FileStorageServiceIT runs the same operations against a MinIO container, including the delete-then-read round trip and the fact that deleting an unknown key succeeds. It instantiates the adapter directly rather than extending ContainerConfigurationTest, since booting the application context would cover nothing more here. FileStorageBackendSelectionTest guards the file.storage.type switch: both adapters are marked primary, so a regression registering the two at once would break every injection point. The testcontainers MinIO module is not published for the 2.x line Spring Boot 4 manages, so the container is a plain GenericContainer instead. refs: #2321 --- .../FileStorageBackendSelectionTest.java | 70 +++++++ .../service/S3FileStorageServiceIT.java | 143 +++++++++++++ .../service/S3FileStorageServiceTest.java | 192 ++++++++++++++++++ 3 files changed, 405 insertions(+) create mode 100644 src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageBackendSelectionTest.java create mode 100644 src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageServiceIT.java create mode 100644 src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageServiceTest.java diff --git a/src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageBackendSelectionTest.java b/src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageBackendSelectionTest.java new file mode 100644 index 000000000..4612b9a37 --- /dev/null +++ b/src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/FileStorageBackendSelectionTest.java @@ -0,0 +1,70 @@ +package fr.avenirsesr.portfolio.file.infrastructure.adapter.service; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +import fr.avenirsesr.portfolio.common.testutils.BddLogger; +import fr.avenirsesr.portfolio.file.domain.port.output.service.FileStorageService; +import fr.avenirsesr.portfolio.file.infrastructure.configuration.S3StorageProperties; +import org.junit.jupiter.api.Test; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import software.amazon.awssdk.services.s3.S3Client; + +/** + * Guards the file.storage.type switch: the two adapters are both marked primary, so exactly one of + * them must ever be registered. + */ +class FileStorageBackendSelectionTest { + + private final ApplicationContextRunner contextRunner = + new ApplicationContextRunner() + .withBean(S3Client.class, () -> mock(S3Client.class)) + .withBean(S3StorageProperties.class, S3StorageProperties::new) + .withUserConfiguration(FileStorageServiceImpl.class, S3FileStorageService.class); + + @Test + void shouldFallBackToLocalStorageWhenTheTypeIsNotSet() { + BddLogger.given("no file.storage.type property"); + BddLogger.when("the context starts"); + BddLogger.then("the local adapter should be the only storage service"); + + contextRunner.run( + context -> { + assertThat(context).hasSingleBean(FileStorageService.class); + assertThat(context.getBean(FileStorageService.class)) + .isInstanceOf(FileStorageServiceImpl.class); + }); + } + + @Test + void shouldUseLocalStorageWhenExplicitlySelected() { + BddLogger.given("file.storage.type=local"); + BddLogger.when("the context starts"); + BddLogger.then("the local adapter should be the only storage service"); + + contextRunner + .withPropertyValues("file.storage.type=local") + .run( + context -> { + assertThat(context).hasSingleBean(FileStorageService.class); + assertThat(context.getBean(FileStorageService.class)) + .isInstanceOf(FileStorageServiceImpl.class); + }); + } + + @Test + void shouldUseS3WhenSelected() { + BddLogger.given("file.storage.type=s3"); + BddLogger.when("the context starts"); + BddLogger.then("the S3 adapter should be the only storage service"); + + contextRunner + .withPropertyValues("file.storage.type=s3") + .run( + context -> { + assertThat(context).hasSingleBean(FileStorageService.class); + assertThat(context.getBean(FileStorageService.class)) + .isInstanceOf(S3FileStorageService.class); + }); + } +} diff --git a/src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageServiceIT.java b/src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageServiceIT.java new file mode 100644 index 000000000..7886ac240 --- /dev/null +++ b/src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageServiceIT.java @@ -0,0 +1,143 @@ +package fr.avenirsesr.portfolio.file.infrastructure.adapter.service; + +import static org.junit.jupiter.api.Assertions.*; + +import fr.avenirsesr.portfolio.common.testutils.BddLogger; +import fr.avenirsesr.portfolio.file.domain.exception.FileNotFoundException; +import fr.avenirsesr.portfolio.file.domain.model.FileResource; +import fr.avenirsesr.portfolio.file.domain.model.enums.EFileType; +import fr.avenirsesr.portfolio.file.infrastructure.configuration.S3StorageProperties; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.UUID; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.utility.DockerImageName; +import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; +import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.model.CreateBucketRequest; + +/** + * Exercises the adapter against a real S3 implementation. + * + *

Unlike the controller integration tests, this one does not extend {@code + * ContainerConfigurationTest}: the adapter is instantiated directly, so booting the application + * context and seeding the database would only slow the test down without covering anything more. + */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +class S3FileStorageServiceIT { + + private static final String BUCKET = "avenirs-portfolio-it"; + private static final String ACCESS_KEY = "avenirs-it-access-key"; + private static final String SECRET_KEY = "avenirs-it-secret-key"; + private static final int MINIO_PORT = 9000; + + private GenericContainer minio; + private S3Client s3Client; + private S3FileStorageService storageService; + + @BeforeAll + void startBackend() { + minio = + new GenericContainer<>(DockerImageName.parse("minio/minio:RELEASE.2025-04-22T22-12-26Z")) + .withEnv("MINIO_ROOT_USER", ACCESS_KEY) + .withEnv("MINIO_ROOT_PASSWORD", SECRET_KEY) + .withCommand("server", "/data") + .withExposedPorts(MINIO_PORT) + .waitingFor( + Wait.forHttp("/minio/health/ready") + .forPort(MINIO_PORT) + .withStartupTimeout(Duration.ofMinutes(2))); + minio.start(); + + var properties = new S3StorageProperties(); + properties.setBucket(BUCKET); + properties.setRegion("us-east-1"); + properties.setAccessKey(ACCESS_KEY); + properties.setSecretKey(SECRET_KEY); + + s3Client = + S3Client.builder() + .endpointOverride( + URI.create( + "http://%s:%d".formatted(minio.getHost(), minio.getMappedPort(MINIO_PORT)))) + .region(Region.of(properties.getRegion())) + .credentialsProvider( + StaticCredentialsProvider.create( + AwsBasicCredentials.create(ACCESS_KEY, SECRET_KEY))) + .forcePathStyle(true) + .build(); + s3Client.createBucket(CreateBucketRequest.builder().bucket(BUCKET).build()); + + storageService = new S3FileStorageService(s3Client, properties); + } + + @AfterAll + void stopBackend() { + if (s3Client != null) { + s3Client.close(); + } + if (minio != null) { + minio.stop(); + } + } + + @Test + void shouldRoundTripAFileThroughTheBucket() { + BddLogger.given("a file resource"); + var id = UUID.randomUUID(); + byte[] content = "Contenu du fichier de test".getBytes(StandardCharsets.UTF_8); + var resource = new FileResource(id, "retour.pdf", EFileType.PDF, content.length, content); + + BddLogger.when("uploading it and reading it back"); + String locator = storageService.upload(resource); + byte[] read = storageService.get(locator); + + BddLogger.then("the stored bytes should be the ones that were uploaded"); + assertEquals(id + ".pdf", locator); + assertArrayEquals(content, read); + } + + @Test + void shouldReportAMissingKeyAsFileNotFound() { + BddLogger.given("a key that was never uploaded"); + String locator = UUID.randomUUID() + ".pdf"; + + BddLogger.when("reading it"); + BddLogger.then("the service should throw FileNotFoundException"); + assertThrows(FileNotFoundException.class, () -> storageService.get(locator)); + } + + @Test + void shouldMakeAFileUnreadableOnceDeleted() { + BddLogger.given("an uploaded file"); + var id = UUID.randomUUID(); + byte[] content = "a supprimer".getBytes(StandardCharsets.UTF_8); + String locator = + storageService.upload( + new FileResource(id, "trace.pdf", EFileType.PDF, content.length, content)); + + BddLogger.when("deleting it"); + storageService.delete(locator); + + BddLogger.then("reading it back should report a missing file"); + assertThrows(FileNotFoundException.class, () -> storageService.get(locator)); + } + + @Test + void shouldSucceedWhenDeletingAKeyThatDoesNotExist() { + BddLogger.given("a key that was never uploaded"); + String locator = UUID.randomUUID() + ".pdf"; + + BddLogger.when("deleting it"); + BddLogger.then("the backend should report success, unlike the local adapter"); + assertDoesNotThrow(() -> storageService.delete(locator)); + } +} diff --git a/src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageServiceTest.java b/src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageServiceTest.java new file mode 100644 index 000000000..ffce28074 --- /dev/null +++ b/src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageServiceTest.java @@ -0,0 +1,192 @@ +package fr.avenirsesr.portfolio.file.infrastructure.adapter.service; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +import fr.avenirsesr.portfolio.common.testutils.BddLogger; +import fr.avenirsesr.portfolio.file.domain.exception.FileNotFoundException; +import fr.avenirsesr.portfolio.file.domain.exception.FileStorageException; +import fr.avenirsesr.portfolio.file.domain.model.FileResource; +import fr.avenirsesr.portfolio.file.domain.model.enums.EFileType; +import fr.avenirsesr.portfolio.file.infrastructure.configuration.S3StorageProperties; +import java.nio.charset.StandardCharsets; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import software.amazon.awssdk.core.ResponseBytes; +import software.amazon.awssdk.core.sync.RequestBody; +import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.model.DeleteObjectRequest; +import software.amazon.awssdk.services.s3.model.GetObjectRequest; +import software.amazon.awssdk.services.s3.model.GetObjectResponse; +import software.amazon.awssdk.services.s3.model.NoSuchKeyException; +import software.amazon.awssdk.services.s3.model.PutObjectRequest; +import software.amazon.awssdk.services.s3.model.PutObjectResponse; +import software.amazon.awssdk.services.s3.model.S3Exception; + +class S3FileStorageServiceTest { + + private static final String BUCKET = "avenirs-portfolio-test"; + + @Mock private S3Client s3Client; + + private S3FileStorageService storageService; + + @BeforeEach + void setUp() { + MockitoAnnotations.openMocks(this); + var properties = new S3StorageProperties(); + properties.setBucket(BUCKET); + storageService = new S3FileStorageService(s3Client, properties); + } + + private FileResource aFileResource(UUID id, EFileType fileType) { + return new FileResource( + id, "consigne.pdf", fileType, 3L, "abc".getBytes(StandardCharsets.UTF_8)); + } + + @Nested + class GivenAnS3StorageService { + + @BeforeEach + void setupGiven() { + BddLogger.given("an S3 storage service"); + } + + @Nested + class WhenUploadingAFile { + + UUID fileId; + + @BeforeEach + void setupWhen() { + BddLogger.when("uploading a file"); + fileId = UUID.randomUUID(); + } + + @Test + void thenItShouldStoreTheObjectUnderTheFileIdKey() { + BddLogger.then("the object should be stored in the configured bucket under {id}.{type}"); + when(s3Client.putObject(any(PutObjectRequest.class), any(RequestBody.class))) + .thenReturn(PutObjectResponse.builder().build()); + + String locator = storageService.upload(aFileResource(fileId, EFileType.PDF)); + + assertEquals(fileId + ".pdf", locator); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PutObjectRequest.class); + verify(s3Client).putObject(captor.capture(), any(RequestBody.class)); + assertEquals(BUCKET, captor.getValue().bucket()); + assertEquals(fileId + ".pdf", captor.getValue().key()); + } + + @Test + void thenItShouldTagTheObjectWithItsContentType() { + BddLogger.then("the object content type should be the file type mime type"); + when(s3Client.putObject(any(PutObjectRequest.class), any(RequestBody.class))) + .thenReturn(PutObjectResponse.builder().build()); + + storageService.upload(aFileResource(fileId, EFileType.PNG)); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PutObjectRequest.class); + verify(s3Client).putObject(captor.capture(), any(RequestBody.class)); + assertEquals("image/png", captor.getValue().contentType()); + } + + @Test + void thenItShouldWrapBackendErrorsIntoAFileStorageException() { + BddLogger.and("the backend rejects the request"); + BddLogger.then("the service should throw FileStorageException"); + when(s3Client.putObject(any(PutObjectRequest.class), any(RequestBody.class))) + .thenThrow(S3Exception.builder().message("bucket is full").build()); + + assertThrows( + FileStorageException.class, + () -> storageService.upload(aFileResource(fileId, EFileType.PDF))); + } + } + + @Nested + class WhenReadingAFile { + + @BeforeEach + void setupWhen() { + BddLogger.when("reading a file"); + } + + @Test + void thenItShouldReturnTheObjectContent() { + BddLogger.then("the object bytes should be returned"); + byte[] content = "hello".getBytes(StandardCharsets.UTF_8); + when(s3Client.getObjectAsBytes(any(GetObjectRequest.class))) + .thenReturn(ResponseBytes.fromByteArray(GetObjectResponse.builder().build(), content)); + + byte[] read = storageService.get("some-key.pdf"); + + assertArrayEquals(content, read); + + ArgumentCaptor captor = ArgumentCaptor.forClass(GetObjectRequest.class); + verify(s3Client).getObjectAsBytes(captor.capture()); + assertEquals(BUCKET, captor.getValue().bucket()); + assertEquals("some-key.pdf", captor.getValue().key()); + } + + @Test + void thenItShouldTranslateAMissingKeyIntoFileNotFound() { + BddLogger.and("the key does not exist"); + BddLogger.then("the service should throw FileNotFoundException"); + when(s3Client.getObjectAsBytes(any(GetObjectRequest.class))) + .thenThrow(NoSuchKeyException.builder().message("no such key").build()); + + assertThrows(FileNotFoundException.class, () -> storageService.get("missing.pdf")); + } + + @Test + void thenItShouldWrapOtherBackendErrorsIntoAFileStorageException() { + BddLogger.and("the backend denies the request"); + BddLogger.then("the service should throw FileStorageException"); + when(s3Client.getObjectAsBytes(any(GetObjectRequest.class))) + .thenThrow(S3Exception.builder().message("access denied").build()); + + assertThrows(FileStorageException.class, () -> storageService.get("forbidden.pdf")); + } + } + + @Nested + class WhenDeletingAFile { + + @BeforeEach + void setupWhen() { + BddLogger.when("deleting a file"); + } + + @Test + void thenItShouldDeleteTheObjectFromTheConfiguredBucket() { + BddLogger.then("the object should be deleted under the given key"); + + storageService.delete("some-key.pdf"); + + ArgumentCaptor captor = + ArgumentCaptor.forClass(DeleteObjectRequest.class); + verify(s3Client).deleteObject(captor.capture()); + assertEquals(BUCKET, captor.getValue().bucket()); + assertEquals("some-key.pdf", captor.getValue().key()); + } + + @Test + void thenItShouldWrapBackendErrorsIntoAFileStorageException() { + BddLogger.and("the backend denies the request"); + BddLogger.then("the service should throw FileStorageException"); + when(s3Client.deleteObject(any(DeleteObjectRequest.class))) + .thenThrow(S3Exception.builder().message("access denied").build()); + + assertThrows(FileStorageException.class, () -> storageService.delete("forbidden.pdf")); + } + } + } +} From 0977f594c4a5836042d39909ae53d3f7595d5e7c Mon Sep 17 00:00:00 2001 From: iamgreendev <56599296+BanelhaqB@users.noreply.github.com> Date: Wed, 2 Sep 2026 15:45:54 +0200 Subject: [PATCH 5/6] fix(file-storage): default the S3 region to eu-west-3 The hosted bucket lives in Paris, so us-east-1 was a misleading default for the online backend. Local MinIO keeps us-east-1, which is its conventional value and which it ignores anyway. refs: #2321 --- .../infrastructure/configuration/S3StorageProperties.java | 5 +++-- src/main/resources/application.properties | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/S3StorageProperties.java b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/S3StorageProperties.java index 921427bc2..3720be136 100644 --- a/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/S3StorageProperties.java +++ b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/configuration/S3StorageProperties.java @@ -15,9 +15,10 @@ public class S3StorageProperties { /** * Region sent along with the request signature. The AWS SDK refuses to build a client without one - * even when the backend ignores it, in which case any placeholder such as {@code us-east-1} does. + * even when the backend ignores it, so this defaults to the region the hosted bucket lives in. A + * backend that does validate it reports the value it expects in the error message. */ - private String region = "us-east-1"; + private String region = "eu-west-3"; /** Bucket holding every file of this environment. */ private String bucket; diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 14ead0057..8900dee42 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -82,7 +82,7 @@ file.storage.profile.default-endpoint=/default/profile-picture file.storage.cover.default-endpoint=/default/cover-picture # S3 backend, only read when file.storage.type=s3 #file.storage.s3.endpoint=https://s3.example.org -#file.storage.s3.region=us-east-1 +#file.storage.s3.region=eu-west-3 #file.storage.s3.bucket=avenirs-portfolio-dev #file.storage.s3.access-key=ENC(...) #file.storage.s3.secret-key=ENC(...) From d39e6fdffb99733d82a48167d7a4575794098118 Mon Sep 17 00:00:00 2001 From: iamgreendev <56599296+BanelhaqB@users.noreply.github.com> Date: Thu, 3 Sep 2026 11:45:32 +0200 Subject: [PATCH 6/6] chore(file-storage): configure the S3 backend credentials Enable the S3 properties with the encrypted dev credentials. The backend stays inactive until file.storage.type is switched to s3. refs: #2321 --- .../adapter/service/S3FileStorageService.java | 1 - src/main/resources/application.properties | 10 ++-------- .../adapter/service/S3FileStorageServiceIT.java | 3 ++- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageService.java b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageService.java index 61d17b448..9bf8bd728 100644 --- a/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageService.java +++ b/src/main/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageService.java @@ -40,7 +40,6 @@ public String upload(FileResource fileResource) { PutObjectRequest.builder() .bucket(properties.getBucket()) .key(key) - // Stored alongside the object so a client reading it back is told what it is. .contentType(fileResource.fileType().getMimeType()) .build(), RequestBody.fromBytes(fileResource.content())); diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 8900dee42..0d55a48e9 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -71,7 +71,8 @@ management.endpoints.web.exposure.include=health,info,caches,prometheus management.health.db.enabled=false management.health.redis.enabled=false #cdn -# Storage backend: local (filesystem) or s3 +# Storage backend: local (filesystem) or s3. Deployments override this from +# env.properties; local keeps a bare run working without any object storage. file.storage.type=local file.storage.local-path=/target/storage # Fallback pictures, resolved as Spring resource locations (file: or classpath:) @@ -80,13 +81,6 @@ file.storage.cover.default-path=file:/workspace/app/target/storage/956e7537-5155 file.storage.endpoint-prefix=/storage file.storage.profile.default-endpoint=/default/profile-picture file.storage.cover.default-endpoint=/default/cover-picture -# S3 backend, only read when file.storage.type=s3 -#file.storage.s3.endpoint=https://s3.example.org -#file.storage.s3.region=eu-west-3 -#file.storage.s3.bucket=avenirs-portfolio-dev -#file.storage.s3.access-key=ENC(...) -#file.storage.s3.secret-key=ENC(...) -#file.storage.s3.path-style-access=true # Valkey spring.cache.type=redis spring.data.redis.host=avenirs-valkey diff --git a/src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageServiceIT.java b/src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageServiceIT.java index 7886ac240..26333b3b7 100644 --- a/src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageServiceIT.java +++ b/src/test/java/fr/avenirsesr/portfolio/file/infrastructure/adapter/service/S3FileStorageServiceIT.java @@ -35,6 +35,7 @@ class S3FileStorageServiceIT { private static final String BUCKET = "avenirs-portfolio-it"; + private static final String REGION = "eu-west-3"; private static final String ACCESS_KEY = "avenirs-it-access-key"; private static final String SECRET_KEY = "avenirs-it-secret-key"; private static final int MINIO_PORT = 9000; @@ -59,7 +60,7 @@ void startBackend() { var properties = new S3StorageProperties(); properties.setBucket(BUCKET); - properties.setRegion("us-east-1"); + properties.setRegion(REGION); properties.setAccessKey(ACCESS_KEY); properties.setSecretKey(SECRET_KEY);