feat: admin presignUrl 조회 API - #144 #151
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAWS S3 프리사인 URL 발급 API와 관련 S3 구성·서비스 및 DTO가 추가되고, 예약 생성 흐름에 reservationName이 도입되며 여러 리졸버 패키지가 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin
participant API as AdminController
participant Svc as S3Service
participant Presigner as S3Presigner
Admin->>API: POST /api/admin/images/url\nS3PreSignedUrlRequest(eventId, mediaInfoRequests)
API->>Svc: getS3PreSignedUrls(eventId, mediaInfoRequests)
loop for each mediaInfo
Svc->>Presigner: presign PutObjectRequest(bucket, key=events/{eventId}/{mediaType}/{uuid}, expiry=10m)
Presigner-->>Svc: pre-signed URL
end
Svc-->>API: S3PreSignedUrlResponse(preSignedUrlInfoList)
API-->>Admin: 200 OK + pre-signed URLs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
src/main/java/com/permitseoul/permitserver/domain/admin/base/core/domain/MediaType.java (1)
3-7: 선택 사항: 코드 스타일 개선enum 정의에서 마지막 상수 뒤의 trailing comma(5번 라인)와 닫는 중괄호 앞의 빈 줄(6번 라인)을 제거하여 코드 스타일을 간결하게 만드는 것을 고려하세요.
public enum MediaType { IMAGE, - VIDEO, - + VIDEO }src/main/java/com/permitseoul/permitserver/domain/admin/base/api/dto/res/S3PreSignedUrlResponse.java (1)
5-20: 권장: 중복된 팩토리 메서드 제거Record는 이미 표준 생성자를 제공하므로
of()정적 팩토리 메서드가 불필요합니다. Record의 간결함을 유지하기 위해 제거하는 것을 권장합니다.public record S3PreSignedUrlResponse( List<PreSignedUrlInfo> preSignedUrlInfoList -) { - public static S3PreSignedUrlResponse of(final List<PreSignedUrlInfo> preSignedUrlInfoList) { - return new S3PreSignedUrlResponse(preSignedUrlInfoList); - } +) { } public record PreSignedUrlInfo( String preSignedUrl, String mediaName - ) { - public static PreSignedUrlInfo of(final String preSignedUrl, final String mediaName) { - return new PreSignedUrlInfo(preSignedUrl, mediaName); - } - } + ) { } }만약 팩토리 메서드를 유지하려면, 유효성 검사나 변환 로직 등의 추가 기능을 제공할 때만 사용하세요.
build.gradle (1)
82-89: 의존성 버전 업데이트 권장
build.gradle(82-89)의 AWS SDK BOM(2.21.0)을 2.35.0으로, Spring Cloud Dependencies(2023.0.0)를 Maven Central 최신 릴리스(2025.1.0-M3) 또는 안정화된 최신 버전으로 업데이트 검토하세요. 보안 취약점은 발견되지 않았습니다.src/main/java/com/permitseoul/permitserver/domain/admin/util/aws/S3Service.java (5)
31-37: 람다 표현식을 간결하게 리팩토링하세요.불필요한 중괄호와 return 문을 제거하여 가독성을 향상시킬 수 있습니다.
다음 diff를 적용하세요:
final List<S3PreSignedUrlResponse.PreSignedUrlInfo> preSignedUrlInfo = mediaInfoRequests.stream() - .map(mediaInfoRequest -> { - return S3PreSignedUrlResponse.PreSignedUrlInfo.of( - generatePreSignedUrl(eventId, eventType, mediaInfoRequest.mediaType()), mediaInfoRequest.mediaName() - ); - } - ).toList(); + .map(mediaInfoRequest -> S3PreSignedUrlResponse.PreSignedUrlInfo.of( + generatePreSignedUrl(eventId, eventType, mediaInfoRequest.mediaType()), + mediaInfoRequest.mediaName() + )) + .toList();
49-49: URL 경로에 enum.toString() 사용 시 대소문자 일관성 문제가 발생할 수 있습니다.enum의 toString()은 대소문자가 섞인 이름을 반환할 수 있으며, URL 경로는 일반적으로 소문자를 사용하는 것이 표준입니다. EventType과 MediaType enum의 이름이 대문자로 정의된 경우 (예:
IMAGE,VIDEO), URL에 대문자가 포함되어 일관성이 떨어질 수 있습니다.다음 diff를 적용하여 소문자로 변환하세요:
- key.append("events/").append(eventType.toString()).append("/").append(eventId).append("/").append(mediaType.toString()).append("/").append(fileName); + key.append("events/").append(eventType.toString().toLowerCase()).append("/").append(eventId).append("/").append(mediaType.toString().toLowerCase()).append("/").append(fileName);
53-53: String.valueOf() 호출이 불필요합니다.StringBuilder를 String으로 변환할 때 String.valueOf()를 사용하는 것보다 toString()을 직접 호출하는 것이 더 명확합니다.
다음 diff를 적용하세요:
final PutObjectRequest putObjectRequest = PutObjectRequest.builder() .bucket(awsS3Properties.bucket()) - .key(String.valueOf(key)) + .key(key.toString()) .build();
42-62: S3 작업에 대한 예외 처리 및 검증을 추가하세요.현재 구현에는 다음과 같은 잠재적 문제가 있습니다:
- S3 presigner 작업이 실패할 경우 예외가 전파되어 사용자에게 내부 오류가 노출될 수 있습니다.
- bucket 속성이 구성되지 않은 경우에 대한 검증이 없습니다.
- 디버깅이나 감사를 위한 로깅이 없습니다.
에러 처리, 검증 로직, 로깅을 추가하는 것을 권장합니다.
다음 사항을 확인하세요:
- AWS S3 Properties가 올바르게 구성되었는지 확인
- S3 presigner 초기화가 성공적으로 이루어지는지 확인
- 운영 환경에서 에러 시나리오를 테스트했는지 확인
예시 개선 사항:
private String generatePreSignedUrl(final long eventId, final EventType eventType, final MediaType mediaType) { try { final String fileName = generateFileName(); final String key = String.format("events/%s/%d/%s/%s", eventType.toString().toLowerCase(), eventId, mediaType.toString().toLowerCase(), fileName); final PutObjectRequest putObjectRequest = PutObjectRequest.builder() .bucket(awsS3Properties.bucket()) .key(key) .build(); final PutObjectPresignRequest preSignRequest = PutObjectPresignRequest.builder() .signatureDuration(Duration.ofMinutes(EXPIRE_TIME)) .putObjectRequest(putObjectRequest) .build(); final URL preSignedUrl = s3Presigner.presignPutObject(preSignRequest).url(); log.debug("Generated pre-signed URL for eventId={}, eventType={}, mediaType={}", eventId, eventType, mediaType); return preSignedUrl.toString(); } catch (S3Exception e) { log.error("Failed to generate pre-signed URL for eventId={}, eventType={}, mediaType={}", eventId, eventType, mediaType, e); throw new CustomS3Exception("Pre-signed URL 생성에 실패했습니다.", e); } }
26-26: 운영 및 보안 고려사항을 검토하세요.다음 사항들을 고려해보시기 바랍니다:
만료 시간 구성: 10분의 만료 시간이 모든 사용 사례에 적합한지 확인하세요. 대용량 파일 업로드의 경우 더 긴 시간이 필요할 수 있습니다. 구성 가능한 값으로 변경하는 것을 권장합니다.
요청 제한: 하나의 요청에서 생성할 수 있는 pre-signed URL 수에 제한이 없습니다. 대량 요청으로 인한 리소스 고갈을 방지하기 위해 mediaInfoRequests의 크기에 제한을 두는 것을 고려하세요.
로깅 및 모니터링: Pre-signed URL 생성에 대한 감사 로그를 추가하여 사용 패턴을 추적하고 보안 문제를 감지할 수 있도록 하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
build.gradle(2 hunks)src/main/java/com/permitseoul/permitserver/PermitServerApplication.java(2 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/base/api/controller/AdminController.java(3 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/base/api/dto/req/AdminValidateRequest.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/base/api/dto/req/S3PreSignedUrlRequest.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/base/api/dto/res/S3PreSignedUrlResponse.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/base/core/domain/MediaType.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/util/aws/AwsS3Properties.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/util/aws/S3Service.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/util/aws/config/S3Config.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/auth/api/controller/AuthController.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/coupon/api/controller/CouponController.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/event/api/controller/EventController.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/eventtimetable/timetable/api/controller/TimetableController.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/eventtimetable/userlike/api/controller/TimetableLikeController.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/payment/api/controller/PaymentController.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/reservation/api/controller/ReservationController.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/ticket/api/controller/TicketController.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/user/api/controller/UserController.java(1 hunks)src/main/java/com/permitseoul/permitserver/global/aop/resolver/event/EventIdPathVariable.java(1 hunks)src/main/java/com/permitseoul/permitserver/global/aop/resolver/event/EventIdPathVariableResolver.java(1 hunks)src/main/java/com/permitseoul/permitserver/global/aop/resolver/timetableblock/TimetableBlockIdPathVariable.java(1 hunks)src/main/java/com/permitseoul/permitserver/global/aop/resolver/timetableblock/TimetableBlockPathVariableResolver.java(1 hunks)src/main/java/com/permitseoul/permitserver/global/aop/resolver/user/UserIdHeader.java(1 hunks)src/main/java/com/permitseoul/permitserver/global/aop/resolver/user/UserIdHeaderResolver.java(1 hunks)src/main/java/com/permitseoul/permitserver/global/config/WebConfig.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/permitseoul/permitserver/domain/admin/base/api/controller/AdminController.java (1)
src/main/java/com/permitseoul/permitserver/global/response/ApiResponseUtil.java (1)
ApiResponseUtil(7-29)
🔇 Additional comments (22)
src/main/java/com/permitseoul/permitserver/global/aop/resolver/user/UserIdHeaderResolver.java (1)
1-1: 패키지 리팩토링이 올바르게 적용되었습니다.리졸버 클래스를
global.aop.resolver네임스페이스로 이동하는 것은 코드 구조를 개선하는 좋은 변경입니다.src/main/java/com/permitseoul/permitserver/domain/admin/base/api/dto/req/AdminValidateRequest.java (1)
1-1: DTO 패키지 구조 개선이 적절합니다.요청 DTO를
dto.req서브패키지로 분리하는 것은 일반적인 패턴입니다.src/main/java/com/permitseoul/permitserver/domain/eventtimetable/timetable/api/controller/TimetableController.java (1)
4-6: 임포트 경로가 올바르게 업데이트되었습니다.새로운 리졸버 패키지 경로로의 마이그레이션이 정확합니다.
src/main/java/com/permitseoul/permitserver/global/aop/resolver/timetableblock/TimetableBlockIdPathVariable.java (1)
1-1: 패키지 이동이 올바르게 적용되었습니다.어노테이션 정의는 변경 없이 패키지만 이동되었습니다.
src/main/java/com/permitseoul/permitserver/domain/auth/api/controller/AuthController.java (1)
9-9: 임포트 경로 업데이트가 정확합니다.
UserIdHeader어노테이션의 새로운 패키지 경로가 올바르게 반영되었습니다.src/main/java/com/permitseoul/permitserver/domain/ticket/api/controller/TicketController.java (1)
5-6: 임포트 경로 업데이트 및 불필요한 임포트 제거가 잘 되었습니다.리졸버 패키지 경로가 올바르게 업데이트되었으며, 사용하지 않는
Param임포트를 제거한 것도 좋은 개선입니다.src/main/java/com/permitseoul/permitserver/global/aop/resolver/user/UserIdHeader.java (1)
1-1: 패키지 이동이 올바르게 적용되었습니다.어노테이션 정의는 그대로 유지하면서 패키지만 이동되었습니다.
src/main/java/com/permitseoul/permitserver/global/config/WebConfig.java (1)
3-5: 이전 패키지 경로 참조가 모두 제거되었습니다.
코드베이스 전체에서com.permitseoul.permitserver.global.resolver경로에 대한 참조가 발견되지 않습니다.src/main/java/com/permitseoul/permitserver/domain/eventtimetable/userlike/api/controller/TimetableLikeController.java (1)
4-5: LGTM! 패키지 리팩토링이 올바르게 적용되었습니다.리졸버 어노테이션들의 임포트 경로가
global.aop.resolver패키지로 일관되게 변경되었습니다. 로직 변경 없이 패키지 구조 개선만 이루어졌습니다.src/main/java/com/permitseoul/permitserver/domain/coupon/api/controller/CouponController.java (1)
5-5: LGTM! 패키지 리팩토링이 올바르게 적용되었습니다.
EventIdPathVariable의 임포트 경로가 새로운global.aop.resolver패키지 구조로 정확히 업데이트되었습니다.src/main/java/com/permitseoul/permitserver/domain/reservation/api/controller/ReservationController.java (1)
12-12: LGTM! 패키지 리팩토링이 올바르게 적용되었습니다.
UserIdHeader의 임포트 경로가 새로운 패키지 구조로 정확히 업데이트되었습니다.src/main/java/com/permitseoul/permitserver/global/aop/resolver/timetableblock/TimetableBlockPathVariableResolver.java (1)
1-1: LGTM! 리졸버 클래스의 패키지 이동이 올바르게 적용되었습니다.리졸버 클래스가
global.aop.resolver하위 패키지로 이동하여 전체 프로젝트의 패키지 구조 개선이 일관되게 반영되었습니다.src/main/java/com/permitseoul/permitserver/domain/payment/api/controller/PaymentController.java (1)
11-11: LGTM! 패키지 리팩토링이 올바르게 적용되었습니다.
UserIdHeader의 임포트 경로가 새로운 패키지 구조로 정확히 업데이트되었습니다.src/main/java/com/permitseoul/permitserver/domain/event/api/controller/EventController.java (1)
5-5: LGTM! 패키지 리팩토링이 올바르게 적용되었습니다.
EventIdPathVariable의 임포트 경로가 새로운global.aop.resolver패키지 구조로 정확히 업데이트되었습니다.src/main/java/com/permitseoul/permitserver/PermitServerApplication.java (1)
6-6: LGTM! AWS S3 설정 프로퍼티가 올바르게 추가되었습니다.
AwsS3Properties가@EnableConfigurationProperties에 추가되어 admin presignUrl API 기능을 위한 AWS S3 설정이 활성화되었습니다. 이는 PR의 목적(#144 - admin presignUrl 조회 API)과 일치합니다.
application.yml또는application.properties에 필요한 AWS S3 관련 설정(aws.s3.*)이 올바르게 구성되어 있는지 확인하세요.Also applies to: 28-29
src/main/java/com/permitseoul/permitserver/domain/user/api/controller/UserController.java (1)
6-6: LGTM! 패키지 리팩토링이 올바르게 적용되었습니다.
UserIdHeader의 임포트 경로가 새로운 패키지 구조로 정확히 업데이트되었습니다.src/main/java/com/permitseoul/permitserver/global/aop/resolver/event/EventIdPathVariable.java (1)
1-11: LGTM!패키지 리팩토링이 적절하게 수행되었습니다. resolver 관련 클래스들을
global.aop.resolver하위로 이동시켜 코드 구조를 개선했습니다.src/main/java/com/permitseoul/permitserver/global/aop/resolver/event/EventIdPathVariableResolver.java (1)
1-42: LGTM!패키지 리팩토링이 일관되게 적용되었습니다. 구현 로직의 변경 없이 구조만 개선되었습니다.
src/main/java/com/permitseoul/permitserver/domain/admin/util/aws/config/S3Config.java (1)
13-47: LGTM!AWS S3 클라이언트 설정이 올바르게 구성되었습니다.
S3Client와S3Presigner빈이 적절하게 정의되어 있으며, AWS SDK v2의 권장 패턴을 따르고 있습니다.단,
AwsS3Properties에서 자격 증명 처리에 대한 보안 개선이 필요한 점은 별도로 지적되었습니다.src/main/java/com/permitseoul/permitserver/domain/admin/base/api/controller/AdminController.java (1)
34-40: 보안 설정 적절함기존 SecurityConfig에서
/api/admin/**경로에 대해 ADMIN 권한을 요구하도록 설정되어 있어 추가 확인 불필요합니다.src/main/java/com/permitseoul/permitserver/domain/admin/base/api/dto/req/S3PreSignedUrlRequest.java (1)
23-28: LGTM!중첩된 레코드 구조가 명확하고 검증 로직이 적절합니다.
src/main/java/com/permitseoul/permitserver/domain/admin/util/aws/S3Service.java (1)
64-66: LGTM!UUID를 사용한 파일명 생성은 충돌을 방지하고 고유성을 보장하는 적절한 방법입니다.
| //s3 | ||
| implementation("software.amazon.awssdk:bom:2.21.0") | ||
| implementation("software.amazon.awssdk:s3:2.21.0") | ||
| } |
There was a problem hiding this comment.
치명적: BOM을 implementation으로 추가하면 안 됩니다
82번 라인에서 AWS SDK BOM을 implementation 의존성으로 추가했는데, 이는 잘못된 사용법입니다. BOM(Bill of Materials)은 dependencyManagement 블록에서만 선언해야 합니다.
다음과 같이 수정하세요:
//s3
- implementation("software.amazon.awssdk:bom:2.21.0")
implementation("software.amazon.awssdk:s3:2.21.0")
}
dependencyManagement {
imports {
mavenBom "org.springframework.cloud:spring-cloud-dependencies:2023.0.0"
+ mavenBom "software.amazon.awssdk:bom:2.21.0"
}
}BOM을 dependencyManagement에 추가하면 S3 의존성의 버전이 자동으로 관리됩니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //s3 | |
| implementation("software.amazon.awssdk:bom:2.21.0") | |
| implementation("software.amazon.awssdk:s3:2.21.0") | |
| } | |
| //s3 | |
| implementation("software.amazon.awssdk:s3:2.21.0") | |
| } | |
| dependencyManagement { | |
| imports { | |
| mavenBom "org.springframework.cloud:spring-cloud-dependencies:2023.0.0" | |
| mavenBom "software.amazon.awssdk:bom:2.21.0" | |
| } | |
| } |
🤖 Prompt for AI Agents
In build.gradle around lines 81 to 84, the AWS SDK BOM was added as an
implementation dependency which is incorrect; remove the BOM from implementation
and instead declare the BOM in the dependencyManagement block (or use Gradle's
platform/dependencyManagement mechanism) so that the S3 dependency can be listed
without an explicit version and will inherit the BOM-managed version; keep
implementation("software.amazon.awssdk:s3") (without the BOM line) and ensure
the BOM entry is placed in dependencyManagement so versions are centralized and
managed.
...n/java/com/permitseoul/permitserver/domain/admin/base/api/dto/req/S3PreSignedUrlRequest.java
Outdated
Show resolved
Hide resolved
| public record AwsS3Properties ( | ||
| String accessKey, | ||
| String secretKey, | ||
| String bucket, | ||
| String region | ||
| ){ } |
There was a problem hiding this comment.
🧩 Analysis chain
보안: 민감한 자격 증명을 안전하게 처리하세요
AWS 자격 증명(accessKey, secretKey)을 일반 문자열로 저장하면 로그나 스택 트레이스에 노출될 위험이 있습니다. 다음 방법을 고려하세요:
- Spring Cloud Config의 암호화 기능 사용
- AWS Secrets Manager 또는 Parameter Store 활용
- 환경 변수 사용 시 로깅에서 제외되도록 설정
또한, 로그에 이 properties 객체가 출력되지 않도록 toString() 메서드를 오버라이드하거나 로깅 설정을 확인하세요.
다음 스크립트를 실행하여 이 properties가 로그에 노출될 가능성을 확인하세요:
🏁 Script executed:
#!/bin/bash
# Description: Check if AwsS3Properties might be logged anywhere
# Search for logging statements that might expose the properties
rg -nP --type=java -C3 'log.*awsS3Properties|logger.*awsS3Properties'
# Search for toString usage that might leak credentials
ast-grep --pattern $'class AwsS3Properties {
$$$
toString() {
$$$
}
$$$
}'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Print top of AwsS3Properties.java to confirm annotations
file=$(fd AwsS3Properties.java | head -1)
sed -n '1,20p' "$file"
# Search for logging statements that reference AwsS3Properties
rg -nP --type=java 'log\.(info|debug|warn|error).*\bAwsS3Properties\b'Length of output: 315
보안: AWS 자격 증명 평문 보관 위험
src/main/java/com/permitseoul/permitserver/domain/admin/util/aws/AwsS3Properties.java: accessKey와 secretKey가 record 필드로 평문 유지되어 기본 toString()에 노출될 수 있습니다. Spring Cloud Config 암호화, AWS Secrets Manager/Parameter Store 적용 또는 record toString() 재정의를 통해 민감 정보 제외를 구현하세요.
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/admin/util/aws/AwsS3Properties.java
(lines 6-11), accessKey and secretKey are stored as record fields and will be
exposed by the record's default toString(); fix by removing plaintext credential
storage or securing retrieval and by preventing accidental exposure: move
credentials to a secure store (Spring Cloud Config with encryption or AWS
Secrets Manager/Parameter Store) and load them at runtime instead of hardcoding;
if you must keep a properties holder, override the record's toString() to redact
accessKey/secretKey (e.g., show only masked or omitted values) and avoid logging
the object or its generated getters, and add annotations (e.g., @JsonIgnore)
where applicable to prevent serialization of sensitive fields.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
src/main/java/com/permitseoul/permitserver/domain/reservation/core/domain/entity/ReservationEntity.java (1)
22-23: 데이터베이스 컬럼 제약조건 추가를 고려하세요.
reservationName필드에nullable = false가 설정되어 있어 좋습니다. 다만 데이터 무결성을 위해length제약조건 추가를 권장합니다.예시:
- @Column(name = "reservation_name", nullable = false) + @Column(name = "reservation_name", nullable = false, length = 500) private String reservationName;src/main/java/com/permitseoul/permitserver/domain/admin/util/aws/S3Service.java (4)
6-6: 사용하지 않는 import를 제거하세요.
EventType이 코드에서 사용되지 않습니다.다음 diff를 적용하세요:
-import com.permitseoul.permitserver.domain.event.core.domain.EventType;
26-26: 만료 시간을 설정 파일로 외부화하는 것을 고려하세요.현재 pre-signed URL 만료 시간이 10분으로 하드코딩되어 있습니다. 향후 변경이나 환경별 설정이 필요할 경우 유연성이 떨어집니다.
AwsS3Properties에 만료 시간 속성을 추가하는 것을 권장합니다:
AwsS3Properties에 다음을 추가:private long presignedUrlExpirationMinutes = 10;그런 다음 이 필드를 사용하도록 수정:
- private static final long EXPIRE_TIME = 10; // pre-signed url 유효기간(10분) + // S3Service.java에서 상수 제거하고 awsS3Properties.presignedUrlExpirationMinutes() 사용
30-36: 람다 표현식을 간소화하세요.불필요한 중괄호와 return 문을 제거하여 코드를 더 간결하게 만들 수 있습니다.
다음 diff를 적용하세요:
final List<S3PreSignedUrlResponse.PreSignedUrlInfo> preSignedUrlInfo = mediaInfoRequests.stream() - .map(mediaInfoRequest -> { - return S3PreSignedUrlResponse.PreSignedUrlInfo.of( - generatePreSignedUrl(eventId, mediaInfoRequest.mediaType()), mediaInfoRequest.mediaName() - ); - } - ).toList(); + .map(mediaInfoRequest -> S3PreSignedUrlResponse.PreSignedUrlInfo.of( + generatePreSignedUrl(eventId, mediaInfoRequest.mediaType()), + mediaInfoRequest.mediaName() + )).toList();
49-59: Aws SDK 예외 전용 핸들러 추가 검토GlobalExceptionHandler의
@ExceptionHandler(Exception.class)가 모든 예외를 처리하지만,SdkException/S3Exception전용 핸들러를 추가하면 AWS 오류에 대해 더 구체적인 에러 메시지를 제공할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/com/permitseoul/permitserver/domain/admin/base/api/controller/AdminController.java(3 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/base/api/dto/req/S3PreSignedUrlRequest.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/event/api/dto/req/AdminEventWithTicketCreateRequest.java(0 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/event/api/service/AdminEventService.java(0 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/util/aws/S3Service.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/event/core/component/EventRetriever.java(0 hunks)src/main/java/com/permitseoul/permitserver/domain/reservation/api/service/ReservationService.java(6 hunks)src/main/java/com/permitseoul/permitserver/domain/reservation/core/component/ReservationAndReservationTicketFacade.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/reservation/core/component/ReservationSaver.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/reservation/core/domain/Reservation.java(2 hunks)src/main/java/com/permitseoul/permitserver/domain/reservation/core/domain/entity/ReservationEntity.java(3 hunks)
💤 Files with no reviewable changes (3)
- src/main/java/com/permitseoul/permitserver/domain/admin/event/api/dto/req/AdminEventWithTicketCreateRequest.java
- src/main/java/com/permitseoul/permitserver/domain/event/core/component/EventRetriever.java
- src/main/java/com/permitseoul/permitserver/domain/admin/event/api/service/AdminEventService.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/permitseoul/permitserver/domain/admin/base/api/controller/AdminController.java (1)
src/main/java/com/permitseoul/permitserver/global/response/ApiResponseUtil.java (1)
ApiResponseUtil(7-29)
🔇 Additional comments (7)
src/main/java/com/permitseoul/permitserver/domain/reservation/core/component/ReservationAndReservationTicketFacade.java (1)
25-32: 파라미터 추가가 올바르게 적용되었습니다.
reservationName파라미터가 첫 번째 인자로 추가되어reservationSaver.saveReservation에 올바르게 전달되고 있습니다.src/main/java/com/permitseoul/permitserver/domain/reservation/core/domain/Reservation.java (1)
14-14: 도메인 모델 필드 추가가 올바르게 적용되었습니다.
reservationName필드가 추가되고fromEntity메서드에서 올바르게 매핑되고 있습니다.src/main/java/com/permitseoul/permitserver/domain/reservation/api/service/ReservationService.java (4)
59-63: 상수 정의가 적절합니다.예약 이름 생성에 사용되는 상수들이 명확하게 정의되어 있습니다.
77-94: 변수 추출 및 검증 로직 개선이 올바르게 적용되었습니다.
eventName과coupon변수를 미리 추출하여 가독성이 향상되었고, 검증 메서드의 반환값을 적절히 활용하고 있습니다.
118-128:buildReservationName호출이 올바르게 적용되었습니다.예약 이름을 별도 메서드로 생성한 후 파사드에 전달하는 로직이 적절합니다.
270-272: 검증 메서드의 반환 타입 변경이 적절합니다.
validateExistEventById가 이벤트 이름을 반환하도록 변경되어 중복 조회를 방지하고 있습니다.src/main/java/com/permitseoul/permitserver/domain/admin/base/api/dto/req/S3PreSignedUrlRequest.java (1)
11-25: LGTM! 검증 로직이 잘 구현되었습니다.DTO 구조와 검증 어노테이션이 적절하게 적용되었습니다. 이전 리뷰에서 지적된 필드명 불일치 문제도 해결되었습니다.
...main/java/com/permitseoul/permitserver/domain/admin/base/api/controller/AdminController.java
Show resolved
Hide resolved
| final String fileName = generateFileName(); //uuid | ||
| final StringBuilder key = new StringBuilder(); | ||
|
|
||
| //경로 : events/{eventType}/{eventId}/{mediaType}/{filename} | ||
| key.append("events/").append(eventId).append("/").append(mediaType.toString()).append("/").append(fileName); |
There was a problem hiding this comment.
S3 키 생성 로직에 파일 확장자를 추가하고 주석을 수정하세요.
여러 가지 개선이 필요합니다:
- 주석 불일치: 주석에
{eventType}이라고 되어 있지만 실제 코드는{eventId}만 사용합니다. - 파일 확장자 누락: UUID만 사용하면 S3에 저장된 파일의 타입을 식별하기 어렵습니다. MediaType에 따라 적절한 확장자를 추가해야 합니다.
- Enum toString() 사용:
mediaType.toString()은 대문자(IMAGE, VIDEO)를 반환하므로 소문자 경로 규칙과 맞지 않을 수 있습니다.
다음 diff를 적용하여 개선하세요:
final String fileName = generateFileName(); //uuid
- final StringBuilder key = new StringBuilder();
-
- //경로 : events/{eventType}/{eventId}/{mediaType}/{filename}
- key.append("events/").append(eventId).append("/").append(mediaType.toString()).append("/").append(fileName);
+
+ // 경로: events/{eventId}/{mediaType}/{filename}.{extension}
+ final String extension = getFileExtension(mediaType);
+ final String key = String.format("events/%d/%s/%s.%s",
+ eventId,
+ mediaType.name().toLowerCase(),
+ fileName,
+ extension
+ );그리고 확장자를 반환하는 헬퍼 메서드를 추가하세요:
private String getFileExtension(final MediaType mediaType) {
return switch (mediaType) {
case IMAGE -> "jpg";
case VIDEO -> "mp4";
};
}🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/admin/util/aws/S3Service.java
around lines 43 to 47, update the S3 key generation and comment to match actual
path components (remove the incorrect {eventType} in the comment), append a
proper file extension based on MediaType, and avoid mediaType.toString() by
using a lowercased name; specifically: change the comment to indicate path
events/{eventId}/{mediaType}/{filename}.{ext}, build the key using
mediaType.name().toLowerCase() and append "." + getFileExtension(mediaType) to
the generated filename, and add the provided private getFileExtension(MediaType)
helper method to return "jpg" for IMAGE and "mp4" for VIDEO.
| private String buildReservationName(final String eventName, | ||
| final List<TicketTypeEntity> ticketTypeEntities, | ||
| final List<ReservationInfoRequest.TicketTypeInfo> ticketTypeInfos, | ||
| final Coupon coupon) { | ||
| final StringBuilder reservationName = new StringBuilder(); | ||
|
|
||
| // 티켓 타입 아이디에 따른 구매 개수 map | ||
| final Map<Long, Integer> ticketTypeCountMap = ticketTypeInfos.stream() | ||
| .collect(Collectors.toMap( | ||
| ReservationInfoRequest.TicketTypeInfo::id, | ||
| ReservationInfoRequest.TicketTypeInfo::count | ||
| )); | ||
|
|
||
| // 티켓 이름 + 수량 | ||
| final List<String> ticketNameWithCountList = ticketTypeEntities.stream() | ||
| .map(ticket -> { | ||
| int count = ticketTypeCountMap.getOrDefault(ticket.getTicketTypeId(), 0); | ||
| return ticket.getTicketTypeName() + TICKET_TYPE_BUY_COUNT + count; | ||
| }) | ||
| .toList(); | ||
|
|
||
| if (ticketNameWithCountList.isEmpty()) { | ||
| return eventName; | ||
| } | ||
| if (coupon != null) { | ||
| return String.valueOf(reservationName.append(START_BRACKET).append(eventName).append(END_BRACKET).append(String.join(SLASH, ticketNameWithCountList)).append(WITH_COUPON)); | ||
| } | ||
|
|
||
| return String.valueOf(reservationName.append(START_BRACKET).append(eventName).append(END_BRACKET).append(String.join(SLASH, ticketNameWithCountList))); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
문자열 생성 로직을 개선하세요.
buildReservationName 메서드에 다음 개선사항을 적용하세요:
- 불필요한
String.valueOf()제거: Lines 192-193, 195에서StringBuilder에String.valueOf()를 사용하는 것은 불필요합니다.toString()을 사용하세요. - 긴 라인 분리: Lines 192, 195의 메서드 체이닝이 너무 길어 가독성이 떨어집니다.
- 조기 반환 조건 재검토: Line 188에서
ticketNameWithCountList가 비어있을 때eventName만 반환하는데, 이 경우가 실제로 발생 가능한지 검증이 필요합니다.
다음과 같이 리팩토링하세요:
- private String buildReservationName(final String eventName,
- final List<TicketTypeEntity> ticketTypeEntities,
- final List<ReservationInfoRequest.TicketTypeInfo> ticketTypeInfos,
- final Coupon coupon) {
- final StringBuilder reservationName = new StringBuilder();
-
- // 티켓 타입 아이디에 따른 구매 개수 map
- final Map<Long, Integer> ticketTypeCountMap = ticketTypeInfos.stream()
- .collect(Collectors.toMap(
- ReservationInfoRequest.TicketTypeInfo::id,
- ReservationInfoRequest.TicketTypeInfo::count
- ));
-
- // 티켓 이름 + 수량
- final List<String> ticketNameWithCountList = ticketTypeEntities.stream()
- .map(ticket -> {
- int count = ticketTypeCountMap.getOrDefault(ticket.getTicketTypeId(), 0);
- return ticket.getTicketTypeName() + TICKET_TYPE_BUY_COUNT + count;
- })
- .toList();
-
- if (ticketNameWithCountList.isEmpty()) {
- return eventName;
- }
- if (coupon != null) {
- return String.valueOf(reservationName.append(START_BRACKET).append(eventName).append(END_BRACKET).append(String.join(SLASH, ticketNameWithCountList)).append(WITH_COUPON));
- }
-
- return String.valueOf(reservationName.append(START_BRACKET).append(eventName).append(END_BRACKET).append(String.join(SLASH, ticketNameWithCountList)));
- }
+ private String buildReservationName(final String eventName,
+ final List<TicketTypeEntity> ticketTypeEntities,
+ final List<ReservationInfoRequest.TicketTypeInfo> ticketTypeInfos,
+ final Coupon coupon) {
+ // 티켓 타입 아이디에 따른 구매 개수 map
+ final Map<Long, Integer> ticketTypeCountMap = ticketTypeInfos.stream()
+ .collect(Collectors.toMap(
+ ReservationInfoRequest.TicketTypeInfo::id,
+ ReservationInfoRequest.TicketTypeInfo::count
+ ));
+
+ // 티켓 이름 + 수량
+ final List<String> ticketNameWithCountList = ticketTypeEntities.stream()
+ .map(ticket -> {
+ int count = ticketTypeCountMap.getOrDefault(ticket.getTicketTypeId(), 0);
+ return ticket.getTicketTypeName() + TICKET_TYPE_BUY_COUNT + count;
+ })
+ .toList();
+
+ if (ticketNameWithCountList.isEmpty()) {
+ return eventName;
+ }
+
+ final StringBuilder reservationName = new StringBuilder();
+ reservationName.append(START_BRACKET)
+ .append(eventName)
+ .append(END_BRACKET)
+ .append(String.join(SLASH, ticketNameWithCountList));
+
+ if (coupon != null) {
+ reservationName.append(WITH_COUPON);
+ }
+
+ return reservationName.toString();
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private String buildReservationName(final String eventName, | |
| final List<TicketTypeEntity> ticketTypeEntities, | |
| final List<ReservationInfoRequest.TicketTypeInfo> ticketTypeInfos, | |
| final Coupon coupon) { | |
| final StringBuilder reservationName = new StringBuilder(); | |
| // 티켓 타입 아이디에 따른 구매 개수 map | |
| final Map<Long, Integer> ticketTypeCountMap = ticketTypeInfos.stream() | |
| .collect(Collectors.toMap( | |
| ReservationInfoRequest.TicketTypeInfo::id, | |
| ReservationInfoRequest.TicketTypeInfo::count | |
| )); | |
| // 티켓 이름 + 수량 | |
| final List<String> ticketNameWithCountList = ticketTypeEntities.stream() | |
| .map(ticket -> { | |
| int count = ticketTypeCountMap.getOrDefault(ticket.getTicketTypeId(), 0); | |
| return ticket.getTicketTypeName() + TICKET_TYPE_BUY_COUNT + count; | |
| }) | |
| .toList(); | |
| if (ticketNameWithCountList.isEmpty()) { | |
| return eventName; | |
| } | |
| if (coupon != null) { | |
| return String.valueOf(reservationName.append(START_BRACKET).append(eventName).append(END_BRACKET).append(String.join(SLASH, ticketNameWithCountList)).append(WITH_COUPON)); | |
| } | |
| return String.valueOf(reservationName.append(START_BRACKET).append(eventName).append(END_BRACKET).append(String.join(SLASH, ticketNameWithCountList))); | |
| } | |
| private String buildReservationName(final String eventName, | |
| final List<TicketTypeEntity> ticketTypeEntities, | |
| final List<ReservationInfoRequest.TicketTypeInfo> ticketTypeInfos, | |
| final Coupon coupon) { | |
| // 티켓 타입 아이디에 따른 구매 개수 map | |
| final Map<Long, Integer> ticketTypeCountMap = ticketTypeInfos.stream() | |
| .collect(Collectors.toMap( | |
| ReservationInfoRequest.TicketTypeInfo::id, | |
| ReservationInfoRequest.TicketTypeInfo::count | |
| )); | |
| // 티켓 이름 + 수량 | |
| final List<String> ticketNameWithCountList = ticketTypeEntities.stream() | |
| .map(ticket -> { | |
| int count = ticketTypeCountMap.getOrDefault(ticket.getTicketTypeId(), 0); | |
| return ticket.getTicketTypeName() + TICKET_TYPE_BUY_COUNT + count; | |
| }) | |
| .toList(); | |
| if (ticketNameWithCountList.isEmpty()) { | |
| return eventName; | |
| } | |
| final StringBuilder reservationName = new StringBuilder(); | |
| reservationName.append(START_BRACKET) | |
| .append(eventName) | |
| .append(END_BRACKET) | |
| .append(String.join(SLASH, ticketNameWithCountList)); | |
| if (coupon != null) { | |
| reservationName.append(WITH_COUPON); | |
| } | |
| return reservationName.toString(); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/reservation/api/service/ReservationService.java
around lines 167 to 196, simplify and clean up the reservation name
construction: remove unnecessary String.valueOf() and call toString() on the
StringBuilder when returning; split the long method-chaining by building the
StringBuilder in a few clear steps (append START_BRACKET, eventName,
END_BRACKET, then append joined ticket names, and conditionally append
WITH_COUPON) instead of one long chained expression to improve readability; and
re-evaluate the early return when ticketNameWithCountList is empty — either keep
the early return but add a short comment explaining why an empty list can occur
or replace it with an explicit validation/assertion that this case should not
happen and handle accordingly.
...in/java/com/permitseoul/permitserver/domain/reservation/core/component/ReservationSaver.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/permitseoul/permitserver/domain/reservation/api/service/ReservationService.java (1)
167-204: 이전 리뷰 제안이 아직 적용되지 않았습니다.이전 리뷰에서 제안된 다음 개선사항들이 여전히 미적용 상태입니다:
- 불필요한
String.valueOf()제거: Lines 194, 203에서StringBuilder에 대해String.valueOf()를 사용하는 대신toString()을 직접 호출해야 합니다.- 긴 메서드 체이닝 분리: Lines 195-199, 203의 체이닝이 너무 길어 가독성이 떨어집니다. 단계별로 분리하여 작성하는 것이 좋습니다.
- 조기 반환 조건 재검토: Line 189에서
ticketNameWithCountList가 비어있을 때eventName만 반환하는데, 이 경우가 실제로 발생 가능한지 검증이 필요합니다.이전 리뷰의 제안된 리팩토링을 적용해 주세요.
🧹 Nitpick comments (1)
src/main/java/com/permitseoul/permitserver/domain/admin/event/core/component/AdminEventRetriever.java (1)
37-40: 성능 최적화를 위해existsById()사용을 권장합니다.현재 구현은 전체 엔티티를 조회한 후 존재 여부만 확인하고 있습니다.
existsById()를 사용하면 COUNT 쿼리만 실행되어 성능이 향상됩니다.다음 diff를 적용하여 성능을 최적화하세요:
@Transactional(readOnly = true) public void validateEventExist(final long eventId) { - eventRepository.findById(eventId).orElseThrow(AdminEventNotFoundException::new); + if (!eventRepository.existsById(eventId)) { + throw new AdminEventNotFoundException(); + } }참고: 메서드 이름을
validateEventExists로 변경하면 영어 문법상 더 정확합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/permitseoul/permitserver/domain/admin/event/core/component/AdminEventRetriever.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/util/aws/S3Service.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/reservation/api/service/ReservationService.java(7 hunks)src/main/java/com/permitseoul/permitserver/domain/reservation/core/component/ReservationSaver.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/permitseoul/permitserver/domain/admin/util/aws/S3Service.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/permitseoul/permitserver/domain/admin/event/core/component/AdminEventRetriever.java (1)
src/main/java/com/permitseoul/permitserver/domain/admin/event/core/exception/AdminEventNotFoundException.java (1)
AdminEventNotFoundException(3-4)
🔇 Additional comments (6)
src/main/java/com/permitseoul/permitserver/domain/reservation/core/component/ReservationSaver.java (1)
16-24: 변경 사항이 올바르게 적용되었습니다.이전 리뷰에서 지적된 중복 저장 호출 문제가 해결되었습니다. 이제 단일
save호출을 통해 엔티티를 저장하고 즉시 도메인 객체로 변환하여 반환하므로 불필요한 데이터베이스 호출이 제거되었습니다. 또한reservationName매개변수가 올바르게 추가되어 예약 이름을 포함한 예약 생성 플로우가 완성되었습니다.src/main/java/com/permitseoul/permitserver/domain/reservation/api/service/ReservationService.java (5)
59-65: 상수 정의가 적절합니다.예약 이름 형식화를 위한 상수들이 명확하게 정의되어 있습니다.
MAX_RESERVATION_VALID_TIME상수로 하드코딩된 값을 대체한 것도 좋은 개선입니다.
78-95: 변경된 로직이 올바르게 구현되었습니다.이벤트 이름과 쿠폰 정보를 사전에 가져오는 흐름이 명확하며, 이후
buildReservationName에서 사용하기 위한 준비 작업이 적절하게 수행되고 있습니다.
119-122: 예약 이름 생성 및 전달이 올바릅니다.
buildReservationName을 통해 생성된 예약 이름을 facade에 전달하는 흐름이 적절하게 구현되었습니다.
149-149: 응답 데이터 변경이 올바릅니다.이벤트 이름 대신 예약 이름을 사용하도록 변경된 것이 새로운 기능 요구사항과 일치합니다.
278-280: 메서드 시그니처 변경이 적절합니다.이벤트 존재 여부를 검증하면서 동시에 이벤트 이름을 반환하도록 변경된 것이 효율적입니다. 이는
buildReservationName에서 필요한 데이터를 제공하기 위한 합리적인 변경입니다.
🔥Pull requests
⛳️ 작업한 브랜치
👷 작업한 내용
feat: admin presignUrl 조회 API
🚨 참고 사항