[Feat/#17] 파일 업로드 URL 발급/삭제 도메인·API 추가 - #22
Conversation
| * S3 연동 전 임시 엔드포인트. presigned-url 발급 응답의 uploadUrl이 이 경로를 가리킨다. | ||
| * S3로 전환하면 이 메서드와 LocalFileStorageClient의 로컬 구현을 함께 제거한다. | ||
| */ | ||
| @PutMapping("/local-upload/**") |
There was a problem hiding this comment.
임시 엔드포인트는 추후 삭제 편의를 위해 별도 컨트롤러 클래스를 만들어서 분리하는 건 어떤가요? AdminLocalFileUploadController 정도가 적당하겠네요!
There was a problem hiding this comment.
추천 감사합니다! 원래는 한 메서드에 넣은 후 주석으로 명시해서 추후에 수정하는 방식으로 진행하려고했는데, 말씀해주신 방식이 더 좋을 것 같네요, 수정하도록 하겠습니다!
| ); | ||
| File saved = fileRepository.save(file); | ||
|
|
||
| return new FileUploadUrlIssueResult(saved.getId(), uploadUrl.url(), fileKey, uploadUrl.expiresAt()); |
There was a problem hiding this comment.
정적 팩토리 메서드로 객체 생성 통일하면 좋을 거 같아요~
There was a problem hiding this comment.
컨벤션 문서 상 "record(도메인 객체·Command·Request/Response): 표준 생성자를 쓴다." 라고 나와있어 위와 같이 짰는데, "단 타입 변환이 끼면 from(...)/toCommand()를 둔다" - 이 부분 확인했습니다!
혹시 coding-style.md에는 값 여러개를 받아 조합하는 경우 of()를 쓴다고 나와있고, 현재 이 로직은 File과 UploadUrl 두 개를 합쳐서 만드는 것인데 이 경우 from()과 of() 중에 어떤 걸 사용하면 될까요?
| UploadUrl uploadUrl = fileStorageClient.issuePresignedUrl(fileKey, command.contentType()); | ||
|
|
||
| File file = File.of( | ||
| null, |
There was a problem hiding this comment.
객체를 생성할 때 id를 null로 집어넣어서 생성한 후에 Repository에 save하는 것보다는 File.create()를 새로 만들어서 id를 받지 않는 정적 팩토리 메서드를 만드는 게 어떨까요?
id에 null을 집어넣는 맥락이 불필요한 코드같습니다.
There was a problem hiding this comment.
넵 추천해주신 대로 create() 와 of() 구분하여 정적 팩토리 메서드 추가했습니다!
|
|
||
| import org.springframework.boot.context.properties.ConfigurationProperties; | ||
|
|
||
| @ConfigurationProperties(prefix = "file.storage.local") |
There was a problem hiding this comment.
로컬같은 경우는 운영 코드에 들어갈 필요가 없고 작동 테스트용이기 때문에 운영 환경에서는 코드가 동작하지 않도록 아래 어노테이션을 추가해도 좋을 거 같습니다.
@Profile("!prod")
There was a problem hiding this comment.
file:
storage:
type: ${FILE_STORAGE_TYPE:local}
local:
base-path: ${LOCAL_STORAGE_BASE_PATH:./local-storage}
base-url: ${LOCAL_STORAGE_BASE_URL:http://localhost:8080}
upload-url-expiry-seconds: ${LOCAL_STORAGE_UPLOAD_URL_EXPIRY_SECONDS:600}
s3:
bucket: ${AWS_S3_BUCKET:}
region: ${AWS_REGION:}
upload-url-expiry-seconds: ${S3_UPLOAD_URL_EXPIRY_SECONDS:600}
현재 application-infrastructure-client.yml에서 위와 같이 type을 지정해서 사용하고 있습니다. 추후에 s3로 변경 시 storage_type을 s3로 변경할 예정이고,
작동 테스트가 아닌 운영에서 코드가 동작하는 문제는 @ConditionalOnProperty로 막고있어 문제가 생기진 않을 것 같습니다!
@profile("prod")를 넣을 시 선택 기준이 두 개가 될 수 있는데 혹시 추가해서 안전장치를 하나 더 두는 방향으로 진행하는 게 좋을까요?
2 participants
#️⃣연관된 이슈
🎯 해결하려는 문제가 무엇인가요?
공지·행사·아카이빙 등 다른 도메인이 이미지·첨부파일을 참조할 수 있으려면, 파일을 업로드하고 메타데이터(
file_id,file_key등)를 관리하는 기능이 먼저 필요하다.❓ 왜 해결해야 하나요?
지금은 파일을 업로드/보관/삭제할 방법이 없어서 공지 첨부파일 등 파일을 참조하는 후속 기능을 시작할 수 없다.
⭐ 어떻게 해결했나요?
FileStorageClient포트로 추상화. 로컬 디스크 구현체(LocalFileStorageClient)와 S3 구현체(S3FileStorageClient)를 함께 추가했고,file.storage.type(local/s3, 기본값local) 설정값으로 어느 쪽을 띄울지 고른다(@ConditionalOnProperty)POST /v1/admin/files/presigned-url(발급),DELETE /v1/admin/files/{fileId}(삭제)FileCategory)별 확장자·최대 용량 정책을FileUploadPolicy로 검증🧩 이 PR의 한계 & 트레이드오프
file.storage.type=local)다. 로컬 구현체와 임시 업로드 엔드포인트(PUT /v1/admin/files/local-upload/**)는 S3를 상시 사용하기로 확정되면LocalFileStorageClient와 함께 제거할 예정 — 두 곳 다 클래스/메서드 주석에 "S3 연동 시 함께 제거" 표시를 남겨뒀다DefaultCredentialsProvider)을 쓴다. 로컬 수동 테스트 때는 환경변수로 임시 액세스 키를 공급했고, 실제 배포는 EC2 인스턴스 프로필(IAM 역할)로 자격증명을 받을 예정이라 이후에도 액세스 키를 코드/설정에 넣을 일이 없다FileStorageClient.write(...)는 S3 구현체에서UnsupportedOperationException을 던진다 — S3는 클라이언트가 presigned URL로 직접 업로드하므로 서버가 파일 바이트를 받을 일이 없다. 인터페이스를 쪼개는 대신 이 방식으로 막기로 했다(아래 "검토한 대안과 선택 이유" 참고)ADMINrole만 확인하고, 카테고리별 소관 부서 검증은 하지 않음⛓️ 기존 기능에 미치는 영향
admin-api모듈에 생기는 첫 컨트롤러라build.gradle.kts에 의존성 추가bootstrap의application.yaml에infrastructure-client설정 import 추가,.env.example에 스토리지 관련 환경변수 추가(local/s3 두 섹션)infrastructure:client에 AWS SDK v2(software.amazon.awssdk:s3) 의존성 추가,gradle/libs.versions.toml에 좌표 등록🔀 Edge Case & 실패 시나리오
fileId/fileKey를 조회·삭제하면FILE_NOT_FOUND(404)UNSUPPORTED_FILE_EXTENSION(400)FILE_SIZE_EXCEEDED(400)fileKey가files/{uuid}.ext형태로 슬래시를 포함해서{fileKey}경로 변수 대신/**와일드카드로 받고 URI에서 직접 파싱file.storage.type=s3인 상태에서 로컬 전용 엔드포인트(/local-upload/**)를 호출하면write(...)의UnsupportedOperationException으로 500이 난다 — 그 엔드포인트 자체가 로컬 전용 임시 기능이라 의도된 동작📋 검토한 대안과 선택 이유
/v1/admin/**가 이미ADMINrole로 막혀 있어 서비스 로직에 별도 코드 없이 role 검증만으로 충분하다고 판단. 부서 단위 검증이 필요한지는 아래 리뷰 포인트로 남김FileStorageClient.write(...)를 S3 구현체가 지원 못 하는 문제: 인터페이스를 로컬 전용/공통으로 쪼개는 방식도 검토했으나, 로컬 구현체 자체가 임시(S3 확정 시 통째로 제거 예정)라 지금 쪼개봐야 제거 시점에 그 분리도 같이 없어진다.UnsupportedOperationException+ 사유 주석으로 막는 쪽을 택했고, 이 패턴을docs/conventions/coding-style.md2-12절에 컨벤션으로 남겼다@Profile대신@ConditionalOnProperty를 택함 — 로컬/운영처럼 배포 환경을 나누는 게 아니라 같은 환경 안에서 설정값(file.storage.type)으로 고르는 것이므로💬 리뷰 포인트
[r]삭제 권한을ADMINrole만 확인하도록 구현했는데, 카테고리별 소관 부서까지 검증하는 방식(DepartmentAccessChecker신설)이 필요한지[c]카테고리별 확장자/용량 정책을 하드코딩한 것이 적절한지[c]files테이블에 업로드 완료 여부를 나타내는status컬럼이 없어서, presigned-url만 발급받고 실제로 업로드하지 않은 "orphan" 레코드를 지금은 정리할 방법이 없다. 이번 PR 스코프에서status컬럼 + 정리 배치까지 추가할지, 후속 이슈로 미룰지 논의 필요[c]S3를 상시 사용하기로 확정되는 시점에 로컬 구현체·임시 업로드 엔드포인트를 이 브랜치에서 바로 제거할지, 별도 이슈로 뺄지