-
Notifications
You must be signed in to change notification settings - Fork 0
#177 [feat] API 추가 #185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
#177 [feat] API 추가 #185
Conversation
- PreSigned URL 조회 API, - 사용자 이미지 수정 API, - 광고성 정보 수신 동의 조회 및 토글 API 추가
WalkthroughS3 통합 및 presigned URL 생성, 사용자 프로필 이미지 업데이트, 마케팅 동의 조회/토글 API를 추가하고 관련 DTO·엔티티를 확장하며 빌드 의존성과 S3 서비스 클래스를 도입했습니다. 기존 기능들은 유지됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UserController
participant UserService
participant S3Service
participant UserRepository
participant S3
Client->>UserController: GET /presigned-url?userId=123
UserController->>UserService: getPreSignedURL(userId)
UserService->>S3Service: getPreSignedUrlForUpload(uuid-key)
S3Service->>S3: Generate presigned PUT URL
S3-->>S3Service: Presigned URL
UserService->>S3Service: getPublicKey(uuid-key)
S3Service-->>UserService: Public URL
UserService-->>UserController: GetPreSignedURLResponse(preSignedURL, publicURL)
UserController-->>Client: SuccessResponse<GetPreSignedURLResponse>
sequenceDiagram
participant Client
participant UserController
participant UserService
participant UserRepository
Client->>UserController: GET /marketing-agreement?userId=123
UserController->>UserService: getMarketingAgree(userId)
UserService->>UserRepository: findById(userId)
UserRepository-->>UserService: User
UserService-->>UserController: MarketingResponse(isAdAgreed)
UserController-->>Client: SuccessResponse<MarketingResponse>
Client->>UserController: PATCH /marketing-agreement?userId=123
UserController->>UserService: toggleMarketingAgree(userId)
UserService->>UserRepository: findById(userId)
UserRepository-->>UserService: User
UserService->>UserService: user.updateMarketingAgree()
UserService->>UserRepository: save(user)
UserService-->>UserController: Void
UserController-->>Client: SuccessResponse<Void>
sequenceDiagram
participant Client
participant UserController
participant UserService
participant UserRepository
Client->>UserController: PATCH /profile-image (PatchUserProfileImageRequest)
UserController->>UserService: updateUserProfileImage(userId, request)
UserService->>UserRepository: findById(userId)
UserRepository-->>UserService: User
UserService->>UserService: user.updateProfileImage(request.publicURL)
UserService->>UserRepository: save(user)
UserService-->>UserController: Void
UserController-->>Client: SuccessResponse<Void>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
✏️ Tip: You can disable this entire section by setting 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @build.gradle:
- Around line 26-30: The dependencyManagement entry pins Spring Cloud AWS to
"io.awspring.cloud:spring-cloud-aws-dependencies:3.3.0", which is incompatible
with the project's Spring Boot 3.5.3; update the BOM version to one that
officially supports Spring Boot 3.5.x or alternatively downgrade Spring Boot to
a supported 3.4.x release. Locate the mavenBom declaration in
dependencyManagement and either change the version string
"io.awspring.cloud:spring-cloud-aws-dependencies:3.3.0" to the compatible Spring
Cloud AWS BOM version for Boot 3.5.3 or align the project's Spring Boot version
to 3.4.x so the existing BOM remains valid, and verify against the Spring Cloud
AWS compatibility matrix after making the change.
In
@src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java:
- Around line 151-158: The current patchMarketingAgree controller uses a toggle
(userService.toggleMarketingAgree) which is vulnerable to race conditions;
change the API to accept an explicit boolean (e.g., PUT or PATCH with a request
body/param like marketingAgree) and call a new service method
userService.updateMarketingAgree(userId, marketingAgree) instead of toggle.
Implement a transactional service method updateMarketingAgree(final Long userId,
boolean marketingAgree) that loads the User (e.g., getUser(userId)) and calls
user.setMarketingAgree(marketingAgree) to persist the explicit value. Ensure the
controller signature and request mapping reflect the explicit boolean input and
remove reliance on toggle logic.
- Around line 132-140: The patchUserProfileImage controller method lacks
request-body validation and the PatchUserProfileImageRequest DTO has no
constraints; add @Valid to the controller parameter on patchUserProfileImage
(i.e., change @RequestBody PatchUserProfileImageRequest request to @RequestBody
@Valid PatchUserProfileImageRequest request) and annotate the
PatchUserProfileImageRequest.publicURL field with validation annotations such as
@NotBlank and @Pattern(regexp="^https?://", message="유효한 URL 형식이어야 합니다") so
malformed or empty URLs are rejected by Spring validation.
In @src/main/java/org/sopt/certi_server/domain/user/entity/User.java:
- Around line 106-108: Replace the toggle method updateMarketingAgree with an
explicit setter setMarketingAgree(Boolean agree) on the User entity: implement
setMarketingAgree to accept a Boolean, validate/null-handle the parameter (e.g.,
treat null as false or throw IllegalArgumentException depending on your policy)
and assign it to the marketingAgree field, and update callers to use
setMarketingAgree(...) instead of toggling; reference: method
updateMarketingAgree, field marketingAgree, class User.
In @src/main/java/org/sopt/certi_server/domain/user/service/UserService.java:
- Around line 227-232: The toggleMarketingAgree method in UserService uses a
toggle (user.updateMarketingAgree()) which is vulnerable to race conditions;
change to explicit value setting: add or use a setter on the User entity (e.g.,
setMarketingAgree(boolean)) and update the service method to accept the desired
boolean (or take the value from the controller) and call
user.setMarketingAgree(desiredValue) inside the existing @Transactional
toggleMarketingAgree (rename to setMarketingAgree or accept a parameter),
ensuring the controller supplies the explicit value and the change is persisted
within the transaction instead of flipping state in-place.
In @src/main/java/org/sopt/certi_server/global/s3/S3Service.java:
- Around line 30-32: The getPublicKey method hardcodes the AWS region string
"ap-northeast-2"; add a region field (e.g., private String region) injected via
@Value("${aws.s3.region}") or from your config class and replace the hardcoded
region in getPublicKey with that region variable so the URL becomes formatted
using bucketName and region; ensure the new region field is declared in
S3Service and wired by Spring.
🧹 Nitpick comments (16)
build.gradle (2)
61-62: 주석 처리된 코드를 삭제하세요.주석 처리된 의존성들은 삭제하는 것이 좋습니다. Git 히스토리에 이전 코드가 보존되므로, 주석으로 남겨두면 코드베이스가 어수선해집니다.
♻️ 제안하는 수정
- //S3의존성 추가 -// implementation("software.amazon.awssdk:bom:2.21.0") -// implementation("software.amazon.awssdk:s3:2.21.0") - + //S3의존성 추가 implementation 'io.awspring.cloud:spring-cloud-aws-starter-s3'
92-95: 불필요한 빈 줄을 제거하세요.파일 끝의 연속된 빈 줄들은 불필요합니다. 한 줄의 공백만으로도 충분합니다.
♻️ 제안하는 수정
implementation 'org.ahocorasick:ahocorasick:0.6.3' } - - - - // Querydsl 빌드 옵션 설정src/main/java/org/sopt/certi_server/domain/user/dto/response/MarketingResponse.java (2)
6-9: 불필요한 세미콜론을 제거하세요.Line 8의 메서드 종료 중괄호 뒤의 세미콜론은 불필요합니다.
♻️ 제안하는 수정
public static MarketingResponse of(boolean isAdAgreed){ return new MarketingResponse(isAdAgreed); - }; + } }
3-5: 필드 네이밍 컨벤션을 고려하세요.Record 필드는 "is" 접두사 없이
adAgreed로 명명하는 것이 더 일반적입니다. Record는 자동으로 접근자를 생성하므로 getter 네이밍 규칙을 따를 필요가 없습니다.♻️ 제안하는 수정 (선택사항)
public record MarketingResponse( - boolean isAdAgreed + boolean adAgreed ) { - public static MarketingResponse of(boolean isAdAgreed){ - return new MarketingResponse(isAdAgreed); + public static MarketingResponse of(boolean adAgreed){ + return new MarketingResponse(adAgreed); } }src/main/java/org/sopt/certi_server/domain/user/dto/response/GetMyPageInfoResponse.java (1)
11-11: 필드명을 Java 네이밍 컨벤션에 맞게 수정하세요.Line 11의
profileImageURL은 Java 네이밍 컨벤션에 맞지 않습니다. Line 23에서 호출하는user.getProfileImageUrl()과 일관성을 유지하기 위해profileImageUrl(Url을 한 단어로 처리)로 변경해야 합니다. 또한 코드베이스 내 다른 DTO인UserInformation도profileImageUrl패턴을 사용하고 있습니다.♻️ 제안하는 수정
@Builder public record GetMyPageInfoResponse( String nickname, - String profileImageURL, + String profileImageUrl, String email, GetJobResponse jobResponse, int upCount, int acCount, int fCount ) { public static GetMyPageInfoResponse from( User user, GetJobResponse jobResponse, int upCount, int acCount, int fCount ){ return GetMyPageInfoResponse.builder() .nickname(user.getNickname()) - .profileImageURL(user.getProfileImageUrl()) + .profileImageUrl(user.getProfileImageUrl()) .email(user.getEmail()) .jobResponse(jobResponse) .upCount(upCount)Also applies to: 23-23
src/main/java/org/sopt/certi_server/domain/user/dto/request/PatchUserProfileImageRequest.java (2)
3-6: 필드명을 Java 네이밍 컨벤션에 맞게 수정하세요.
publicURL은 Java 네이밍 컨벤션을 위반합니다.publicUrl로 변경하여 camelCase 규칙을 따라야 합니다.♻️ 제안하는 수정
public record PatchUserProfileImageRequest( - String publicURL + String publicUrl ) { }
3-6: 입력 검증 추가를 고려하세요.프로필 이미지 업데이트를 위한 request DTO에 검증 어노테이션이 없습니다. 코드베이스의 다른 request DTO들(
UpdateUserRequest,PatchAcquisitionRequest등)은@NotNull,@NotEmpty,@NotBlank등의 검증을 사용합니다.publicUrl필드에 적절한 검증을 추가하면 데이터 무결성과 에러 처리가 개선됩니다.♻️ 제안하는 수정
package org.sopt.certi_server.domain.user.dto.request; +import jakarta.validation.constraints.NotBlank; + public record PatchUserProfileImageRequest( + @NotBlank(message = "프로필 이미지 URL은 필수입니다") String publicUrl ) { }src/main/java/org/sopt/certi_server/domain/user/dto/response/GetPreSignedURLResponse.java (1)
3-17: 필드명을 Java 네이밍 컨벤션에 맞게 수정하세요.
preSignedURL과publicURL필드명이 Java 네이밍 컨벤션을 위반합니다.preSignedUrl과publicUrl로 변경하여 camelCase 규칙을 따라야 합니다.♻️ 제안하는 수정
public record GetPreSignedURLResponse( - String preSignedURL, - String publicURL + String preSignedUrl, + String publicUrl ) { public static GetPreSignedURLResponse of( - String preSignedURL, - String publicURL + String preSignedUrl, + String publicUrl ){ return new GetPreSignedURLResponse( - preSignedURL, - publicURL + preSignedUrl, + publicUrl ); } }src/main/java/org/sopt/certi_server/global/s3/S3Service.java (3)
24-28: 입력 파라미터 검증 추가를 고려하세요.
key파라미터에 대한 검증이 없습니다. 악의적인 경로 탐색(path traversal) 공격이나 예상치 못한 키 형식을 방지하기 위해 입력 검증을 추가하는 것이 좋습니다.♻️ 제안하는 검증 로직
public String getPreSignedUrlForUpload(String key) { + if (key == null || key.isBlank()) { + throw new IllegalArgumentException("S3 key cannot be null or blank"); + } + if (key.contains("..") || key.startsWith("/")) { + throw new IllegalArgumentException("Invalid S3 key format"); + } // 10분 동안 유효한 업로드용 URL 생성 URL preSignedUrl = s3Template.createSignedPutURL(bucketName, key, Duration.ofMinutes(10)); return preSignedUrl.toString(); }
30-32: 메서드 이름이 혼란스럽습니다.메서드 이름이
getPublicKey이지만 실제로는 S3 객체의 public URL을 반환합니다.getPublicUrl또는buildPublicUrl과 같은 이름이 더 명확합니다.♻️ 제안하는 수정
-public String getPublicKey(String key){ +public String getPublicUrl(String key){ return String.format("https://%s.s3.ap-northeast-2.amazonaws.com/%s", bucketName, key); }그리고 UserService에서도 호출 부분을 수정:
-String publicKey = s3Service.getPublicKey(key); +String publicKey = s3Service.getPublicUrl(key);
37-41: 사용하지 않는 메서드를 제거하거나 향후 사용 계획을 명시하세요.
getPreSignedUrlForDownload메서드는 코드베이스에서 호출되지 않습니다. 향후 사용 예정이 없다면 제거하거나, 사용 예정이 있다면 TODO 주석을 추가하는 것을 권장합니다.src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java (1)
124-130: Rate limiting 추가를 고려하세요.Presigned URL 생성 엔드포인트는 무제한 S3 키 생성을 허용할 수 있으므로, 남용 방지를 위해 rate limiting을 추가하는 것이 좋습니다.
src/main/java/org/sopt/certi_server/domain/user/entity/User.java (3)
64-65: Boolean 타입 대신 boolean 사용을 고려하세요.
marketingAgree필드가Boolean타입으로 null 값을 가질 수 있습니다. 광고성 정보 수신 동의는 일반적으로 명확한 true/false 값을 가져야 하므로,boolean타입으로 변경하고 기본값을 설정하는 것이 좋습니다.♻️ 제안하는 수정
-@Column(name = "marketing_agree") -private Boolean marketingAgree; +@Column(name = "marketing_agree", nullable = false) +private boolean marketingAgree = false;그리고 Builder 시그니처도 수정:
-public User(Long id, University university, String track, String grade, MajorImpl major, String nickname, String name, String email, - String profileImageUrl, SocialType socialType, Long socialId, Boolean marketingAgree) { +public User(Long id, University university, String track, String grade, MajorImpl major, String nickname, String name, String email, + String profileImageUrl, SocialType socialType, Long socialId, boolean marketingAgree) {
110-112: 파라미터 이름이 혼란스럽습니다.메서드가
publicKey를 받지만 실제로는 URL을 저장합니다. 파라미터 이름을profileImageUrl로 변경하는 것이 더 명확합니다.♻️ 제안하는 수정
-public void updateProfileImage(String publicKey) { - this.profileImageUrl = publicKey; +public void updateProfileImage(String profileImageUrl) { + this.profileImageUrl = profileImageUrl; }
110-112: 입력 검증 추가를 고려하세요.
profileImageUrl에 대한 검증 없이 값을 설정합니다. null 체크나 URL 형식 검증을 추가하는 것이 좋습니다.♻️ 제안하는 검증 로직
public void updateProfileImage(String publicKey) { + if (publicKey == null || publicKey.isBlank()) { + throw new IllegalArgumentException("Profile image URL cannot be null or blank"); + } this.profileImageUrl = publicKey; }src/main/java/org/sopt/certi_server/domain/user/service/UserService.java (1)
234-240: 업로드되지 않은 객체에 대한 정리 전략을 고려하세요.Presigned URL을 생성하지만 사용자가 실제로 업로드하지 않는 경우 S3에 객체가 생성되지 않습니다. 하지만 여러 번 URL을 요청하면 많은 키가 생성될 수 있으므로, S3 라이프사이클 정책을 통해 오래된 미완료 업로드를 정리하는 것이 좋습니다.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
build.gradlesrc/main/java/org/sopt/certi_server/domain/user/controller/UserController.javasrc/main/java/org/sopt/certi_server/domain/user/dto/request/PatchUserProfileImageRequest.javasrc/main/java/org/sopt/certi_server/domain/user/dto/response/GetMyPageInfoResponse.javasrc/main/java/org/sopt/certi_server/domain/user/dto/response/GetPreSignedURLResponse.javasrc/main/java/org/sopt/certi_server/domain/user/dto/response/MarketingResponse.javasrc/main/java/org/sopt/certi_server/domain/user/entity/User.javasrc/main/java/org/sopt/certi_server/domain/user/service/UserService.javasrc/main/java/org/sopt/certi_server/global/s3/S3Service.java
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/org/sopt/certi_server/domain/user/dto/request/PatchUserProfileImageRequest.java (8)
src/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUserRequest.java (1)
UpdateUserRequest(13-26)src/main/java/org/sopt/certi_server/domain/userprecertification/dto/request/PatchPreCertificationRequest.java (1)
PatchPreCertificationRequest(9-17)src/main/java/org/sopt/certi_server/domain/acquisition/dto/request/PatchAcquisitionRequest.java (1)
PatchAcquisitionRequest(8-16)src/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateMajorRequest.java (1)
UpdateMajorRequest(3-4)src/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUniversityRequest.java (1)
UpdateUniversityRequest(3-4)src/main/java/org/sopt/certi_server/domain/userprecertification/dto/request/UserPreCertificationRequest.java (1)
UserPreCertificationRequest(5-6)src/main/java/org/sopt/certi_server/domain/certification/dto/request/CertificationCreateRequest.java (1)
CertificationCreateRequest(10-27)src/main/java/org/sopt/certi_server/domain/user/dto/request/CreateCareerRequest.java (1)
CreateCareerRequest(9-24)
src/main/java/org/sopt/certi_server/domain/user/dto/response/GetMyPageInfoResponse.java (1)
src/main/java/org/sopt/certi_server/domain/user/dto/response/UserInformation.java (1)
UserInformation(6-20)
src/main/java/org/sopt/certi_server/global/s3/S3Service.java (1)
src/main/java/org/sopt/certi_server/domain/user/service/UserService.java (1)
Service(35-247)
🔇 Additional comments (2)
src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java (1)
143-149: LGTM!광고성 정보 수신 동의 조회 API가 올바르게 구현되었습니다.
src/main/java/org/sopt/certi_server/domain/user/service/UserService.java (1)
221-225: LGTM!광고성 정보 수신 동의 조회 메서드가 올바르게 구현되었습니다.
| dependencyManagement { | ||
| imports { | ||
| mavenBom "io.awspring.cloud:spring-cloud-aws-dependencies:3.3.0" | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Spring Cloud AWS 3.3.0 Spring Boot compatibility
💡 Result:
Spring Cloud AWS 3.3.0 is published as compatible with Spring Boot 3.4 (and with the Spring Cloud 2024.0 release train). [1][2]
Sources:
- Spring Cloud AWS 3.3.0 release notes. [1]
- Spring Cloud AWS project compatibility page. [2]
🏁 Script executed:
# Find and read the build.gradle file to see the Spring Boot version
find . -name "build.gradle" -type f | head -5Repository: cerdeuk/CERTI-BE
Length of output: 74
🏁 Script executed:
cat -n ./build.gradleRepository: cerdeuk/CERTI-BE
Length of output: 4033
Spring Cloud AWS 버전과 Spring Boot의 호환성 불일치를 해결하세요.
Spring Cloud AWS 3.3.0은 공식적으로 Spring Boot 3.4와의 호환성을 지원합니다. 그러나 현재 프로젝트는 Spring Boot 3.5.3을 사용 중이므로 지원되는 버전 범위를 벗어나 있습니다. Spring Cloud AWS 3.3.0을 Spring Boot 3.5.3과 함께 사용하면 예상치 못한 호환성 문제가 발생할 수 있습니다. Spring Cloud AWS를 Spring Boot 3.5.3과 호환되는 버전으로 업그레이드하거나 Spring Boot를 3.4.x로 다운그레이드하세요.
🤖 Prompt for AI Agents
In @build.gradle around lines 26 - 30, The dependencyManagement entry pins
Spring Cloud AWS to "io.awspring.cloud:spring-cloud-aws-dependencies:3.3.0",
which is incompatible with the project's Spring Boot 3.5.3; update the BOM
version to one that officially supports Spring Boot 3.5.x or alternatively
downgrade Spring Boot to a supported 3.4.x release. Locate the mavenBom
declaration in dependencyManagement and either change the version string
"io.awspring.cloud:spring-cloud-aws-dependencies:3.3.0" to the compatible Spring
Cloud AWS BOM version for Boot 3.5.3 or align the project's Spring Boot version
to 3.4.x so the existing BOM remains valid, and verify against the Spring Cloud
AWS compatibility matrix after making the change.
| @PatchMapping(value = "/profile-image") | ||
| @Operation(summary = "프로필 이미지 수정 API", description = "업로드 된 이미지로 프로필 이미지를 변경합니다.") | ||
| public ResponseEntity<SuccessResponse<Void>> patchUserProfileImage( | ||
| @AuthenticationPrincipal Long userId, | ||
| @RequestBody PatchUserProfileImageRequest request | ||
| ){ | ||
| userService.updateUserProfileImage(userId, request); | ||
| return ResponseEntity.ok(SuccessResponse.of(SuccessCode.SUCCESS_UPDATE)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
fd -type f -name "*PatchUserProfileImageRequest*"Repository: cerdeuk/CERTI-BE
Length of output: 230
🏁 Script executed:
fd PatchUserProfileImageRequestRepository: cerdeuk/CERTI-BE
Length of output: 153
🏁 Script executed:
cat -n src/main/java/org/sopt/certi_server/domain/user/dto/request/PatchUserProfileImageRequest.javaRepository: cerdeuk/CERTI-BE
Length of output: 232
🏁 Script executed:
sed -n '130,142p' src/main/java/org/sopt/certi_server/domain/user/controller/UserController.javaRepository: cerdeuk/CERTI-BE
Length of output: 539
@Valid 어노테이션 추가 및 요청 본문 검증을 구현하세요.
@RequestBody 파라미터에 @Valid 어노테이션이 없으며, PatchUserProfileImageRequest DTO의 publicURL 필드에도 검증 로직이 전혀 없습니다. 다음과 같이 수정하세요:
-
컨트롤러에
@Valid어노테이션 추가:@RequestBody @Valid PatchUserProfileImageRequest request
-
DTO에 검증 어노테이션 추가:
public record PatchUserProfileImageRequest( @NotBlank(message = "프로필 이미지 URL은 필수입니다") @Pattern(regexp = "^https?://", message = "유효한 URL 형식이어야 합니다") String publicURL ) {}
악의적이거나 잘못된 URL이 저장되는 것을 방지해야 합니다.
🤖 Prompt for AI Agents
In
@src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java
around lines 132 - 140, The patchUserProfileImage controller method lacks
request-body validation and the PatchUserProfileImageRequest DTO has no
constraints; add @Valid to the controller parameter on patchUserProfileImage
(i.e., change @RequestBody PatchUserProfileImageRequest request to @RequestBody
@Valid PatchUserProfileImageRequest request) and annotate the
PatchUserProfileImageRequest.publicURL field with validation annotations such as
@NotBlank and @Pattern(regexp="^https?://", message="유효한 URL 형식이어야 합니다") so
malformed or empty URLs are rejected by Spring validation.
| @Transactional | ||
| public void toggleMarketingAgree(final Long userId) { | ||
| User user = getUser(userId); | ||
|
|
||
| user.updateMarketingAgree(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
토글 패턴의 동시성 문제가 있습니다.
User 엔티티와 컨트롤러에서 언급한 것처럼, 토글 방식은 동시 요청 시 문제를 일으킬 수 있습니다. 명시적인 값 설정 방식으로 변경하세요.
🤖 Prompt for AI Agents
In @src/main/java/org/sopt/certi_server/domain/user/service/UserService.java
around lines 227 - 232, The toggleMarketingAgree method in UserService uses a
toggle (user.updateMarketingAgree()) which is vulnerable to race conditions;
change to explicit value setting: add or use a setter on the User entity (e.g.,
setMarketingAgree(boolean)) and update the service method to accept the desired
boolean (or take the value from the controller) and call
user.setMarketingAgree(desiredValue) inside the existing @Transactional
toggleMarketingAgree (rename to setMarketingAgree or accept a parameter),
ensuring the controller supplies the explicit value and the change is persisted
within the transaction instead of flipping state in-place.
src/main/java/org/sopt/certi_server/domain/user/service/UserService.java
Show resolved
Hide resolved
| public String getPublicKey(String key){ | ||
| return String.format("https://%s.s3.ap-northeast-2.amazonaws.com/%s", bucketName, key); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
하드코딩된 리전을 설정으로 분리하세요.
ap-northeast-2 리전이 하드코딩되어 있습니다. 환경에 따라 리전이 변경될 수 있으므로 설정값으로 관리하는 것이 좋습니다.
♻️ 제안하는 수정
@Value 어노테이션으로 리전을 주입받도록 변경:
@Value("${spring.cloud.aws.s3.bucket}")
private String bucketName;
+
+@Value("${spring.cloud.aws.region.static}")
+private String region;
public String getPublicKey(String key){
- return String.format("https://%s.s3.ap-northeast-2.amazonaws.com/%s", bucketName, key);
+ return String.format("https://%s.s3.%s.amazonaws.com/%s", bucketName, region, key);
}🤖 Prompt for AI Agents
In @src/main/java/org/sopt/certi_server/global/s3/S3Service.java around lines 30
- 32, The getPublicKey method hardcodes the AWS region string "ap-northeast-2";
add a region field (e.g., private String region) injected via
@Value("${aws.s3.region}") or from your config class and replace the hardcoded
region in getPublicKey with that region variable so the URL becomes formatted
using bucketName and region; ensure the new region field is declared in
S3Service and wired by Spring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
build.gradle (2)
61-62: 주석 처리된 의존성 제거 권장주석 처리된 이전 AWS SDK 의존성들은 Git 히스토리에 보존되므로 완전히 제거하는 것이 좋습니다. 주석으로 남겨두면 실제 사용 중인 의존성에 대한 혼란을 야기할 수 있습니다.
♻️ 제안된 수정사항
- //S3의존성 추가 -// implementation("software.amazon.awssdk:bom:2.21.0") -// implementation("software.amazon.awssdk:s3:2.21.0") - + //S3 의존성 추가
92-95: 파일 끝의 불필요한 빈 줄 제거파일 끝에 여러 개의 빈 줄이 추가되었습니다. 코드 가독성을 위해 제거하는 것을 고려하세요.
♻️ 제안된 수정사항
// ahocorasick(아호코라식) 의존성 추가 implementation 'org.ahocorasick:ahocorasick:0.6.3' } - - - - // Querydsl 빌드 옵션 설정 def generated = 'src/main/generated'
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
build.gradle
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
build.gradle (2)
64-64: Spring Cloud AWS S3 스타터로의 전환 승인AWS SDK v2를 Spring Cloud AWS S3 스타터로 교체한 것은 적절한 변경입니다. Spring Cloud 추상화를 사용하면 Spring Boot와의 통합이 개선되고 설정이 단순해집니다. 버전은 BOM에서 관리되므로 일관성이 유지됩니다.
26-30: 버전 호환성 확인 완료 - 우려사항 없음Spring Cloud AWS 3.4.0은 Spring Boot 3.5.x를 대상으로 설계되었으므로 Spring Boot 3.5.3과 완전히 호환됩니다. 공식 호환성 매트릭스에 따르면 Spring Cloud AWS 3.4.x는 Spring Cloud 2025.0.x → Spring Boot 3.5.x로 매핑되며, 이 조합은 현재 권장되는 구성입니다. 버전 선택이 올바릅니다.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/org/sopt/certi_server/domain/user/entity/User.java (1)
75-89: 빌더로 생성 시marketingAgree기본값 보장이 필요합니다.
현재 빌더 인자로marketingAgree가null로 들어오면 그대로 저장됩니다. 토글/응답에서 의미가 애매해지고 NPE 위험도 커집니다.제안 수정안(널 방지)
@@ - this.marketingAgree = marketingAgree; + this.marketingAgree = (marketingAgree != null) ? marketingAgree : Boolean.FALSE;src/main/java/org/sopt/certi_server/domain/user/service/UserService.java (1)
145-158:publicURL이 선택값이면, PUT 요청에서 누락 시 기존 프로필 이미지가 null로 덮일 수 있어요.
이 API가 “전체 교체(필수 필드 모두 필요)”인지, “부분 수정(누락은 유지)”인지 정책을 명확히 하고 그에 맞게 검증(@NotNull) 또는 merge 로직을 두는 게 안전합니다.
🤖 Fix all issues with AI agents
In
@src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java:
- Around line 124-130: The getPreSignedURL controller currently implies
upload-only behavior; update the API to explicitly distinguish upload vs
download by adding a request parameter or separate endpoints and wiring it
through to the service. Modify the method signature for getPreSignedURL to
accept a @RequestParam like "operation" (or "mode") and map values (e.g.,
"upload" | "download") to call the appropriate service method (e.g.,
userService.getPreSignedURL(userId, operation) or separate
userService.getUploadPreSignedURL / getDownloadPreSignedURL), and update the
Operation summary/description to document both modes and expected behavior for
each response variant (GetPreSignedURLResponse for upload/download). Ensure
validation of the param and return the same SuccessResponse wrapping the correct
service result.
- Around line 141-148: The endpoint patchMarketingAgree currently toggles state
via userService.toggleMarketingAgree(userId), which is non-idempotent and
vulnerable to retries; change the API to accept the desired boolean state (e.g.,
a request body or request param like "marketingAgree": true/false) and update
the controller to call an idempotent service method (e.g.,
userService.setMarketingAgree(userId, marketingAgree)) instead of
toggleMarketingAgree; adjust the method signature to accept the boolean (and
validate it) and update the service to persist the explicit state so repeated
requests are safe.
In @src/main/java/org/sopt/certi_server/domain/user/entity/User.java:
- Around line 64-66: The marketingAgree field is nullable (private Boolean
marketingAgree) which can cause NPEs in flows like updateMarketingAgree(); make
it non-nullable and default to false by annotating/setting it as nullable=false
and providing a default/fallback value (e.g., initialize marketingAgree = false
or enforce non-null in the JPA column mapping), and update
updateMarketingAgree() to rely on the non-null primitive/Boolean with safe
assignment so reads/toggles cannot encounter null.
- Around line 107-109: In User.updateMarketingAgree the field marketingAgree can
be null causing an NPE when negating; make the toggle null-safe by checking
this.marketingAgree for null and initializing it to a defined boolean (e.g.,
true or false) before flipping, e.g., if marketingAgree is null set it to true
(or false per domain rule) otherwise set it to !marketingAgree; update the
method in the User class to perform this null check and assignment so calling
updateMarketingAgree never throws an NPE.
- Around line 111-113: The parameter name in User.updateProfileImage(String
publicKey) is misleading; rename the parameter to profileImageUrl to match its
usage (assigning to this.profileImageUrl) and to be consistent with
changeUser(String profileImageUrl); update the method signature and any callers
to use the new parameter name.
- Around line 91-97: The changeUser(...) method currently overwrites the
existing profileImageUrl with a null value when the client omits publicURL; to
fix, prevent null from clobbering the stored image by adding a null-check either
in the entity or service: in User.changeUser(...) only assign
this.profileImageUrl = profileImageUrl if profileImageUrl != null, or
alternatively validate UpdateUserRequest.publicURL as @NotNull and enforce in
UserService.updateUserInformation(...) before calling changeUser(...); update
tests accordingly.
In @src/main/java/org/sopt/certi_server/domain/user/service/UserService.java:
- Around line 222-234: toggleMarketingAgree is non-idempotent because it blindly
flips state via User.updateMarketingAgree(); change it to an idempotent setter:
modify the service method signature from toggleMarketingAgree(Long userId) to
setMarketingAgree(Long userId, boolean agree) (or add an overload), fetch User
via getUser(userId) and call a new User.setMarketingAgree(boolean agree) method
instead of updateMarketingAgree(), keep @Transactional, and update the
controller to send the desired boolean; ensure User.setMarketingAgree only
writes when the value differs to avoid unnecessary updates.
- Around line 235-241: UserService.getPreSignedURL currently generates a key
without validating file type and calls S3Service methods that omit Content-Type
in the presign and build a hardcoded public URL; fix by (1) adding a file
type/extension or MIME validation in UserService.getPreSignedURL and include the
validated extension in the generated key (or accept a validated contentType
parameter), (2) update S3Service.getPreSignedUrlForUpload / createSignedPutURL
to accept metadata (Content-Type) and pass it into the
s3Template.createSignedPutURL call so the Content-Type is part of the signature,
and (3) replace getPublicKey’s hardcoded URL assembly with a provider-based URL
generation using the S3 client or configured endpoint/utility (derive the bucket
and endpoint/region from configuration or the S3 client instead of string
concatenation) so region/endpoint changes or CloudFront won’t break.
🧹 Nitpick comments (4)
src/main/java/org/sopt/certi_server/domain/acquisition/dto/response/GetAcquisitionResponse.java (1)
13-36:certificationType매핑 NPE/지연로딩 가능성만 확인해주세요.
acquisition.getCertification().getCertificationType()가 null 가능하거나, LAZY 로딩인데 컨트롤러까지 오는 과정에서 세션이 닫히면 NPE/LazyInitializationException리스크가 있습니다. (특히 목록 조회에서 fetch join 미사용 시)src/main/java/org/sopt/certi_server/domain/userprecertification/dto/response/PreCertificationSimple.java (1)
9-34:certificationType도 도메인 null 불가 전제인지 명확히 해두면 좋아요.
upc.getCertification().getCertificationType()가 nullable이면 현재 생성 시점에 바로 NPE가 납니다. nullable 가능성이 있으면 기본값/예외를 의도적으로 처리하는 쪽이 안전합니다.src/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUserRequest.java (1)
13-26:publicURL입력값 검증/컨벤션을 한 번만 정리해두면 좋아요.
- 외부 URL을 그대로 저장해도 되는 정책인지(아니라면 버킷/프리픽스/https 스킴 등 검증 필요)
- 네이밍을
publicUrl로 맞추고, 필요하면@JsonProperty("publicURL")로 하위호환 유지하는 방식도 고려 가능src/main/java/org/sopt/certi_server/domain/user/service/UserService.java (1)
243-247: 프로필 이미지 URL은 최소한 “우리 버킷/프리픽스/https” 검증을 추천합니다.
임의의publicURL저장을 허용하면 클라이언트가 외부 리소스를 그대로 신뢰/표시하게 되어 보안/피싱/정책 위반 이슈가 생길 수 있습니다.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/org/sopt/certi_server/domain/acquisition/dto/response/GetAcquisitionResponse.javasrc/main/java/org/sopt/certi_server/domain/user/controller/UserController.javasrc/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUserRequest.javasrc/main/java/org/sopt/certi_server/domain/user/dto/response/PersonalInformationResponse.javasrc/main/java/org/sopt/certi_server/domain/user/entity/User.javasrc/main/java/org/sopt/certi_server/domain/user/service/UserService.javasrc/main/java/org/sopt/certi_server/domain/userprecertification/dto/response/PreCertificationSimple.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/org/sopt/certi_server/domain/user/service/UserService.java (1)
src/main/java/org/sopt/certi_server/global/s3/S3Service.java (1)
Service(11-42)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy-cd
🔇 Additional comments (2)
src/main/java/org/sopt/certi_server/domain/user/dto/response/PersonalInformationResponse.java (2)
13-15: 응답 필드명이profileImageURL(URL 대문자)로 고정되는데, 기존 컨벤션과 일치하는지 확인이 필요합니다.
Jackson 기본 정책이면 JSON 키가 그대로profileImageURL로 나갑니다. 다른 응답/요청이profileImageUrl을 쓰는 컨벤션이면 클라이언트 호환성 이슈가 날 수 있어요.
17-24:from(User user)매핑은 단순/명확해서 좋습니다. 다만 “URL vs key” 의미만 일관성 확인이 필요합니다.
여기서는user.getProfileImageUrl()을 그대로 내려주므로, 엔티티에 key가 저장되는 구조라면 API 계약이 어긋납니다.
| @GetMapping(value = "/presigned-url") | ||
| @Operation(summary = "presigned URL 반환 API", description = "Presigned URL을 반환합니다. 해당 URL을 통해 이미지를 업로드 할 수 있습니다.") | ||
| public ResponseEntity<SuccessResponse<GetPreSignedURLResponse>> getPreSignedURL( | ||
| @AuthenticationPrincipal Long userId | ||
| ){ | ||
| return ResponseEntity.ok(SuccessResponse.of(SuccessCode.SUCCESS_FETCH, userService.getPreSignedURL(userId))); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presigned URL API가 “업/다운로드 모두” 요구사항을 충족하는지 확인 필요해요.
현재 컨트롤러 설명/흐름상 업로드 용도로만 보입니다. 다운로드 presigned도 요구사항이면 별도 엔드포인트/쿼리 파라미터 등으로 구분이 필요합니다.
🤖 Prompt for AI Agents
In
@src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java
around lines 124 - 130, The getPreSignedURL controller currently implies
upload-only behavior; update the API to explicitly distinguish upload vs
download by adding a request parameter or separate endpoints and wiring it
through to the service. Modify the method signature for getPreSignedURL to
accept a @RequestParam like "operation" (or "mode") and map values (e.g.,
"upload" | "download") to call the appropriate service method (e.g.,
userService.getPreSignedURL(userId, operation) or separate
userService.getUploadPreSignedURL / getDownloadPreSignedURL), and update the
Operation summary/description to document both modes and expected behavior for
each response variant (GetPreSignedURLResponse for upload/download). Ensure
validation of the param and return the same SuccessResponse wrapping the correct
service result.
| @PatchMapping(value = "/marketing-agreement") | ||
| @Operation(summary = "광고성 수신 정보 동의 토글 API", description = "광고성 수신 정보 동의를 토글식으로 변경합니다.") | ||
| public ResponseEntity<SuccessResponse<Void>> patchMarketingAgree( | ||
| @AuthenticationPrincipal Long userId | ||
| ){ | ||
| userService.toggleMarketingAgree(userId); | ||
| return ResponseEntity.ok(SuccessResponse.of(SuccessCode.SUCCESS_UPDATE)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
마케팅 동의 “토글”은 재시도에 취약(비멱등)해서 운영 리스크가 있어요.
네트워크 재시도/중복 요청이 나면 의도와 반대로 다시 토글될 수 있어서, PATCH/PUT로 “원하는 상태(boolean)”를 받는 형태가 보통 더 안전합니다.
제안(개념) 변경 예시
- @PatchMapping(value = "/marketing-agreement")
- public ResponseEntity<SuccessResponse<Void>> patchMarketingAgree(@AuthenticationPrincipal Long userId){
- userService.toggleMarketingAgree(userId);
- return ResponseEntity.ok(SuccessResponse.of(SuccessCode.SUCCESS_UPDATE));
- }
+ @PutMapping(value = "/marketing-agreement")
+ public ResponseEntity<SuccessResponse<Void>> putMarketingAgree(
+ @AuthenticationPrincipal Long userId,
+ @RequestBody MarketingAgreementRequest request
+ ){
+ userService.setMarketingAgree(userId, request.agree());
+ return ResponseEntity.ok(SuccessResponse.of(SuccessCode.SUCCESS_UPDATE));
+ }🤖 Prompt for AI Agents
In
@src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java
around lines 141 - 148, The endpoint patchMarketingAgree currently toggles state
via userService.toggleMarketingAgree(userId), which is non-idempotent and
vulnerable to retries; change the API to accept the desired boolean state (e.g.,
a request body or request param like "marketingAgree": true/false) and update
the controller to call an idempotent service method (e.g.,
userService.setMarketingAgree(userId, marketingAgree)) instead of
toggleMarketingAgree; adjust the method signature to accept the boolean (and
validate it) and update the service to persist the explicit state so repeated
requests are safe.
| @Column(name = "marketing_agree") | ||
| private Boolean marketingAgree; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
marketingAgree 널 가능 상태로 두면 토글/조회 흐름에서 런타임 오류 위험이 큽니다.
Boolean로 추가되어 DB/엔티티 기본값이 null일 수 있습니다. 아래 updateMarketingAgree()가 null에서 NPE가 날 수 있어요(동일 이슈의 근본 원인). nullable=false + 기본값(false)(또는 할당 시 null-coalescing)로 정리하는 걸 권장합니다.
🤖 Prompt for AI Agents
In @src/main/java/org/sopt/certi_server/domain/user/entity/User.java around
lines 64 - 66, The marketingAgree field is nullable (private Boolean
marketingAgree) which can cause NPEs in flows like updateMarketingAgree(); make
it non-nullable and default to false by annotating/setting it as nullable=false
and providing a default/fallback value (e.g., initialize marketingAgree = false
or enforce non-null in the JPA column mapping), and update
updateMarketingAgree() to rely on the non-null primitive/Boolean with safe
assignment so reads/toggles cannot encounter null.
| public void changeUser(String name, String nickname, String email, LocalDate birthDate, String profileImageUrl){ | ||
| this.name = name; | ||
| this.nickname = nickname; | ||
| this.email = email; | ||
| this.birthDate = birthDate; | ||
| this.profileImageUrl = profileImageUrl; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -n --type=java -C3 '\.changeUser\s*\('Repository: cerdeuk/CERTI-BE
Length of output: 780
🏁 Script executed:
cat -n src/main/java/org/sopt/certi_server/domain/user/service/UserService.java | sed -n '140,170p'Repository: cerdeuk/CERTI-BE
Length of output: 1289
🏁 Script executed:
fd -t f 'UpdateUserRequest' --type f | head -5Repository: cerdeuk/CERTI-BE
Length of output: 229
🏁 Script executed:
cat -n src/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUserRequest.javaRepository: cerdeuk/CERTI-BE
Length of output: 1095
🏁 Script executed:
rg -n --type=java 'updateUserInformation' | head -20Repository: cerdeuk/CERTI-BE
Length of output: 525
🏁 Script executed:
cat -n src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java | sed -n '55,75p'Repository: cerdeuk/CERTI-BE
Length of output: 1145
changeUser() 메서드에서 null인 profileImageUrl이 기존 이미지를 삭제합니다.
UpdateUserRequest의 publicURL은 @NotNull 검증이 없어 클라이언트가 null을 전송하면, UserService.updateUserInformation() → changeUser() 호출 시 기존 이미지가 null로 덮어씌워집니다. 서비스나 엔티티 단계에서 null-check를 추가하거나, DTO 레벨에서 publicURL을 필수값으로 정의하기를 권장합니다.
🤖 Prompt for AI Agents
In @src/main/java/org/sopt/certi_server/domain/user/entity/User.java around
lines 91 - 97, The changeUser(...) method currently overwrites the existing
profileImageUrl with a null value when the client omits publicURL; to fix,
prevent null from clobbering the stored image by adding a null-check either in
the entity or service: in User.changeUser(...) only assign this.profileImageUrl
= profileImageUrl if profileImageUrl != null, or alternatively validate
UpdateUserRequest.publicURL as @NotNull and enforce in
UserService.updateUserInformation(...) before calling changeUser(...); update
tests accordingly.
| public void updateMarketingAgree() { | ||
| this.marketingAgree = !this.marketingAgree; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updateMarketingAgree()는 marketingAgree == null이면 NPE로 터집니다. (필수 수정)
제안 수정안(널 안전 토글)
@@
public void updateMarketingAgree() {
- this.marketingAgree = !this.marketingAgree;
+ this.marketingAgree = !Boolean.TRUE.equals(this.marketingAgree);
}📝 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.
| public void updateMarketingAgree() { | |
| this.marketingAgree = !this.marketingAgree; | |
| } | |
| public void updateMarketingAgree() { | |
| this.marketingAgree = !Boolean.TRUE.equals(this.marketingAgree); | |
| } |
🤖 Prompt for AI Agents
In @src/main/java/org/sopt/certi_server/domain/user/entity/User.java around
lines 107 - 109, In User.updateMarketingAgree the field marketingAgree can be
null causing an NPE when negating; make the toggle null-safe by checking
this.marketingAgree for null and initializing it to a defined boolean (e.g.,
true or false) before flipping, e.g., if marketingAgree is null set it to true
(or false per domain rule) otherwise set it to !marketingAgree; update the
method in the User class to perform this null check and assignment so calling
updateMarketingAgree never throws an NPE.
| public void updateProfileImage(String publicKey) { | ||
| this.profileImageUrl = publicKey; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for updateProfileImage and profileImageUrl usage patterns
rg -n --type=java -C3 '\bupdateProfileImage\b|\bprofileImageUrl\b|getProfileImageUrl\s*\(' src/Repository: cerdeuk/CERTI-BE
Length of output: 12510
🏁 Script executed:
#!/bin/bash
# Find the PatchUserProfileImageRequest DTO
rg -n --type=java 'class PatchUserProfileImageRequest|record PatchUserProfileImageRequest' -A 10 src/Repository: cerdeuk/CERTI-BE
Length of output: 518
메서드 파라미터명을 profileImageUrl로 변경하세요.
updateProfileImage(String publicKey) 메서드가 실제로는 request.publicURL()을 받아서 profileImageUrl 필드에 저장하는데, 파라미터명이 publicKey로 되어 있어 의미가 불일치합니다. 같은 클래스의 changeUser(String profileImageUrl) 메서드처럼 파라미터명을 profileImageUrl로 통일하면 코드의 일관성과 가독성이 향상됩니다.
🤖 Prompt for AI Agents
In @src/main/java/org/sopt/certi_server/domain/user/entity/User.java around
lines 111 - 113, The parameter name in User.updateProfileImage(String publicKey)
is misleading; rename the parameter to profileImageUrl to match its usage
(assigning to this.profileImageUrl) and to be consistent with changeUser(String
profileImageUrl); update the method signature and any callers to use the new
parameter name.
| public MarketingResponse getMarketingAgree(final Long userId){ | ||
| User user = getUser(userId); | ||
|
|
||
| return MarketingResponse.of(user.getMarketingAgree()); | ||
| } | ||
|
|
||
| @Transactional | ||
| public void toggleMarketingAgree(final Long userId) { | ||
| User user = getUser(userId); | ||
|
|
||
| user.updateMarketingAgree(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
toggleMarketingAgree()는 비멱등이라(재시도 시 상태 재토글) 서비스 레벨에서도 보호가 필요해요.
컨트롤러만이 아니라 서비스도 “원하는 상태로 set”하는 형태가 운영 리스크가 낮습니다.
🤖 Prompt for AI Agents
In @src/main/java/org/sopt/certi_server/domain/user/service/UserService.java
around lines 222 - 234, toggleMarketingAgree is non-idempotent because it
blindly flips state via User.updateMarketingAgree(); change it to an idempotent
setter: modify the service method signature from toggleMarketingAgree(Long
userId) to setMarketingAgree(Long userId, boolean agree) (or add an overload),
fetch User via getUser(userId) and call a new User.setMarketingAgree(boolean
agree) method instead of updateMarketingAgree(), keep @Transactional, and update
the controller to send the desired boolean; ensure User.setMarketingAgree only
writes when the value differs to avoid unnecessary updates.
| public GetPreSignedURLResponse getPreSignedURL(final Long userId) { | ||
| String key = "profiles/user-" + userId + "/" + UUID.randomUUID(); | ||
| String publicKey = s3Service.getPublicKey(key); | ||
| String preSignedURL = s3Service.getPreSignedUrlForUpload(key); | ||
|
|
||
| return GetPreSignedURLResponse.of(preSignedURL, publicKey); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find S3Service implementation
find . -type f -name "*.java" | xargs grep -l "class S3Service\|interface S3Service" | head -5Repository: cerdeuk/CERTI-BE
Length of output: 122
🏁 Script executed:
#!/bin/bash
# Check project dependencies
find . -type f \( -name "pom.xml" -o -name "build.gradle" \) | head -5Repository: cerdeuk/CERTI-BE
Length of output: 74
🏁 Script executed:
#!/bin/bash
# Find S3 configuration files
find . -type f \( -name "*.properties" -o -name "*.yml" -o -name "*.yaml" \) | xargs grep -l "s3\|aws" 2>/dev/null | head -10Repository: cerdeuk/CERTI-BE
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Examine S3Service implementation
cat -n ./src/main/java/org/sopt/certi_server/global/s3/S3Service.javaRepository: cerdeuk/CERTI-BE
Length of output: 1625
🏁 Script executed:
#!/bin/bash
# Check build.gradle for dependencies
cat -n ./build.gradleRepository: cerdeuk/CERTI-BE
Length of output: 4033
🏁 Script executed:
#!/bin/bash
# Find application configuration files
find . -type f \( -name "application.yml" -o -name "application.yaml" -o -name "application*.properties" \) | head -10Repository: cerdeuk/CERTI-BE
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Find application configuration files
find . -type f \( -name "application*.yml" -o -name "application*.properties" -o -name "*.yml" -o -name "*.properties" \) | grep -E "(application|application-)" | head -20Repository: cerdeuk/CERTI-BE
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check UserService implementation around the getPreSignedURL method
cat -n ./src/main/java/org/sopt/certi_server/domain/user/service/UserService.java | sed -n '230,250p'Repository: cerdeuk/CERTI-BE
Length of output: 845
🏁 Script executed:
#!/bin/bash
# Search for S3 bucket or AWS configuration classes
find . -type f -name "*.java" | xargs grep -l "bucketName\|S3.*Config\|AWS.*Config" | head -10Repository: cerdeuk/CERTI-BE
Length of output: 122
🌐 Web query:
Spring Cloud AWS 3.4.0 S3Template createSignedPutURL Content-Type header presigned URL
💡 Result:
Short answer: In Spring Cloud AWS 3.4.0 S3Template.createSignedPutURL the Content-Type must be provided as part of the PutObject request metadata for it to be enforced by S3 — if the presigner/SDK does not include Content-Type in the signed headers the client’s browser-supplied Content-Type will cause signature mismatch. This is an SDK-level behavior (AWS SDK v2 presigner) that many projects have reported: the presigned PUT URL will only enforce content-type if Content-Type is included in the signed headers when the URL is created. (docs.awspring.io)
What to do
- Pass the desired content type via the S3Template overload that accepts metadata (ObjectMetadata.contentType) when creating the signed PUT URL so the presigner can include it. (S3Template exposes createSignedPutURL with metadata/contentType in the API history.) (docs.awspring.io)
- If you still get SignatureDoesNotMatch from browsers, it’s likely the presigner didn’t include content-type in X-Amz-SignedHeaders (an SDK behavior/bug). Workarounds:
- Use a presigned POST (form-based upload) where content-type can be specified as a condition.
- Ensure the client sets exactly the same Content-Type header you used when signing (including casing/charset).
- If necessary, generate the presigned request server-side using the underlying AWS SDK presigner and explicitly include Content-Type in the PutObjectRequest so it appears in X-Amz-SignedHeaders. (github.com)
If you want, tell me your exact S3Template call and client upload code (fetch/axios/browser) and I’ll give a minimal reproducible example and the exact code change to make it work.
Citations:
- 1: https://docs.awspring.io/spring-cloud-aws/docs/3.4.0/reference/html/index.html?utm_source=openai
- 2: https://docs.awspring.io/spring-cloud-aws/docs/3.0.1/apidocs/io/awspring/cloud/s3/S3Template.html?utm_source=openai
- 3: Unable to use presigned upload URLs to upload from JS frontend boto/boto3#3708
🏁 Script executed:
#!/bin/bash
# Search for application configuration in src/main/resources
find ./src/main/resources -type f \( -name "*.yml" -o -name "*.yaml" -o -name "*.properties" \) 2>/dev/null | head -20Repository: cerdeuk/CERTI-BE
Length of output: 95
🏁 Script executed:
#!/bin/bash
# Check for any bucket policy or S3 configuration documentation
find . -type f \( -name "*.md" -o -name "*.txt" \) | xargs grep -l "bucket\|S3\|AWS" 2>/dev/null | head -5Repository: cerdeuk/CERTI-BE
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Search more broadly for application config files
find . -path ./node_modules -prune -o -type f -name "application*.yml" -print 2>/dev/null
find . -path ./node_modules -prune -o -type f -name "application*.properties" -print 2>/dev/nullRepository: cerdeuk/CERTI-BE
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check common Spring Boot config locations
ls -la ./src/main/resources/ 2>/dev/nullRepository: cerdeuk/CERTI-BE
Length of output: 231
🏁 Script executed:
#!/bin/bash
# Search for AWS or bucket configuration anywhere in Java files
rg "spring\.cloud\.aws\.s3\.bucket|bucket.*=" --type java | head -20Repository: cerdeuk/CERTI-BE
Length of output: 164
🏁 Script executed:
#!/bin/bash
# Look for S3 bucket policy or ACL configuration
rg "public.*read|GetObject|PutObject|bucket.*policy" --type java | head -20Repository: cerdeuk/CERTI-BE
Length of output: 42
Presigned URL/공개 URL 생성에서 Content-Type 서명 미포함 및 URL 하드코딩 문제 확인됨
검증 결과 다음 이슈들이 확인되었습니다:
-
Content-Type 미포함:
S3Service.getPreSignedUrlForUpload()에서s3Template.createSignedPutURL()을 호출할 때 Content-Type 메타데이터를 전달하지 않음. Spring Cloud AWS 3.4.0에서는 Content-Type을 서명에 포함시키려면 메타데이터 파라미터로 전달해야 하는데, 이를 누락하면 브라우저에서 Content-Type 헤더 추가 시 서명 불일치 오류 발생 가능. -
하드코딩된 공개 URL:
getPublicKey()에서"https://{bucketName}.s3.ap-northeast-2.amazonaws.com/{key}"형식으로 고정 조립 → 리전 변경, 엔드포인트 변경, CloudFront 도입 시 깨질 가능성 높음. -
파일 타입 제약 없음:
UserService.getPreSignedURL()에서 생성되는 key("profiles/user-{userId}/{UUID}")에 확장자 또는 MIME 타입 검증 없음.
개선 방안:
createSignedPutURL()호출 시 Content-Type 메타데이터 파라미터 추가- 공개 URL 생성을 설정/템플릿 기반으로 변경 또는 S3 SDK 유틸 활용
- 업로드 전 파일 타입/확장자 검증 추가
- S3 버킷 정책 및 공개 접근 권한 재확인
🤖 Prompt for AI Agents
In @src/main/java/org/sopt/certi_server/domain/user/service/UserService.java
around lines 235 - 241, UserService.getPreSignedURL currently generates a key
without validating file type and calls S3Service methods that omit Content-Type
in the presign and build a hardcoded public URL; fix by (1) adding a file
type/extension or MIME validation in UserService.getPreSignedURL and include the
validated extension in the generated key (or accept a validated contentType
parameter), (2) update S3Service.getPreSignedUrlForUpload / createSignedPutURL
to accept metadata (Content-Type) and pass it into the
s3Template.createSignedPutURL call so the Content-Type is part of the signature,
and (3) replace getPublicKey’s hardcoded URL assembly with a provider-based URL
generation using the S3 client or configured endpoint/utility (derive the bucket
and endpoint/region from configuration or the S3 client instead of string
concatenation) so region/endpoint changes or CloudFront won’t break.
- 유저 프로필 사진 조회 추가 - 생년월일 조회 추가 - 사용자 이름, 닉네임 둘다 조회할 수 있게, 이력서, 홈화면에서 선택적으로 사용 가능 - 카카오 information에서 이름 못가져오던 오류 수정
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
@src/main/java/org/sopt/certi_server/domain/user/controller/AuthController.java:
- Around line 70-75: The processSignup method is missing the @Valid annotation
on its SignupRequest parameter so Bean Validation constraints in SignupRequest
are not applied; update the method signature in AuthController.processSignup to
annotate the request parameter with @Valid (i.e., use @Valid @RequestBody
SignupRequest request) and add the corresponding javax.validation.Valid import
if missing so request-level validation behaves consistently with the other
endpoints.
In @src/main/java/org/sopt/certi_server/domain/user/entity/Career.java:
- Around line 26-27: The Career entity maps the field `private String name` to
the DB column `nickname` via `@Column(name = "nickname")`, causing a semantic
mismatch; pick one fix and apply it project-wide: either rename the field in the
Career class (e.g., `jobTitle` or `careerName`) and update the `@Column` to the
matching column name, or change the annotation to `@Column(name = "name")` to
match the field; after changing, update all usages (e.g., CareerService,
CareerDetailResponse, DTOs, mappers, repositories, and any tests) to reference
the new field name/column mapping so compilation and mapping remain consistent.
In
@src/main/java/org/sopt/certi_server/domain/userprecertification/dto/response/PreCertificationSimple.java:
- Around line 27-28: PreCertificationSimple currently calls
upc.getCertification().getCertificationType().getKoreanName() without null
protection which can NPE if Certification.certificationType is null; update the
mapping in PreCertificationSimple (where you build the DTO from upc) to mirror
CertificationSimple by guarding certificationType (e.g., use a null-safe
expression: upc.getCertification().getCertificationType() != null ?
upc.getCertification().getCertificationType().getKoreanName() : null) or
alternatively enforce not-null at the entity/DB level (add @Column(nullable =
false) / validation) and adjust usages accordingly.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/org/sopt/certi_server/domain/activity/controller/ActivityController.javasrc/main/java/org/sopt/certi_server/domain/user/controller/AuthController.javasrc/main/java/org/sopt/certi_server/domain/user/dto/response/GetUserResponse.javasrc/main/java/org/sopt/certi_server/domain/user/dto/response/OAuthUserInformation.javasrc/main/java/org/sopt/certi_server/domain/user/dto/response/UserInformation.javasrc/main/java/org/sopt/certi_server/domain/user/dto/response/kakao/KakaoUserInformationResponse.javasrc/main/java/org/sopt/certi_server/domain/user/entity/Career.javasrc/main/java/org/sopt/certi_server/domain/user/service/KakaoService.javasrc/main/java/org/sopt/certi_server/domain/userprecertification/dto/response/PreCertificationSimple.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/org/sopt/certi_server/domain/user/dto/response/GetUserResponse.java (5)
src/main/java/org/sopt/certi_server/domain/user/dto/response/PersonalInformationResponse.java (1)
Builder(8-24)src/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUserRequest.java (1)
UpdateUserRequest(13-26)src/main/java/org/sopt/certi_server/domain/user/dto/response/GetMyPageInfoResponse.java (1)
Builder(8-29)src/main/java/org/sopt/certi_server/domain/user/entity/User.java (1)
Builder(71-85)src/main/java/org/sopt/certi_server/domain/user/dto/response/SignUpResponse.java (1)
SignUpResponse(9-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy-cd
🔇 Additional comments (8)
src/main/java/org/sopt/certi_server/domain/activity/controller/ActivityController.java (2)
27-34: LGTM!
SuccessResponse에<Void>제네릭 타입을 명시적으로 지정하여 타입 안정성을 향상시킨 좋은 변경입니다. 데이터 페이로드가 없는 생성 작업에 적합합니다.
47-56: LGTM!
createActivity와 동일하게SuccessResponse<Void>타입을 명시하여 일관성을 유지했습니다. 삭제 작업의 반환 타입으로 적절합니다.src/main/java/org/sopt/certi_server/domain/user/service/KakaoService.java (1)
57-58: LGTM - KakaoProfile 필드 변경에 맞춰 수정됨.
nickname()사용이KakaoUserInformationResponse의 필드명 변경과 일관성 있게 적용되었습니다.다만, 닉네임과 이메일을 로그에 직접 출력하는 것은 개인정보 노출 위험이 있으므로, 프로덕션 환경에서는 로그 레벨을
DEBUG로 변경하거나 마스킹 처리를 고려해 주세요.src/main/java/org/sopt/certi_server/domain/user/dto/response/UserInformation.java (1)
11-19: LGTM - 카카오 프로필 매핑 변경 적용됨.
KakaoProfile의nickname()필드에서 값을 가져오도록 일관성 있게 수정되었습니다.src/main/java/org/sopt/certi_server/domain/user/dto/response/OAuthUserInformation.java (1)
14-24: LGTM - 일관된 nickname 매핑 적용.
OAuthUserInformation에서도KakaoProfile.nickname()을 사용하도록 변경되어 다른 DTO들과 일관성이 유지됩니다.src/main/java/org/sopt/certi_server/domain/user/dto/response/GetUserResponse.java (1)
10-28: LGTM - 사용자 프로필 정보 확장.PR 목표에 맞게
name,profileImage,birthDate필드가 추가되었고,User엔티티에서 올바르게 매핑되고 있습니다.
profileImage와birthDate가 null일 수 있는 경우, API 응답에서 null이 그대로 노출될 수 있습니다. 프론트엔드에서 null 처리가 되어 있다면 문제없지만, 필요 시 기본값 또는 Optional 처리를 고려해 주세요.src/main/java/org/sopt/certi_server/domain/user/dto/response/kakao/KakaoUserInformationResponse.java (1)
19-25: 카카오 API 필드명 변경 확인됨카카오 공식 API 문서에 따르면,
KakaoProfile의 필드명 변경은 정확합니다. 카카오 로그인 API(/v2/user/me)와 카카오톡 프로필 API 모두nickname필드를 사용하며,name필드는 없습니다. 변경 사항이 카카오 API 응답 구조와 일치합니다.src/main/java/org/sopt/certi_server/domain/userprecertification/dto/response/PreCertificationSimple.java (1)
12-13: LGTM!새로운 필드
certificationType과description이 record에 적절하게 추가되었습니다. AI 요약에 따르면GetAcquisitionResponse에서도 동일한 패턴이 사용되어 DTO 간 일관성이 유지됩니다.
| public ResponseEntity<SuccessResponse<SignUpResponse>> processSignup( | ||
| @RequestHeader("Authorization") @NotEmpty(message = "임시 토큰이 누락되었습니다.") String authorization, | ||
| @Valid @RequestBody SignupRequest request | ||
| @RequestBody SignupRequest request | ||
| ) { | ||
| return ResponseEntity.ok(SuccessResponse.of(SuccessCode.SUCCESS_CREATE, authService.register(authorization, request))); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Valid 어노테이션 누락으로 요청 본문 유효성 검사가 비활성화됨
SignupRequest에서 @Valid 어노테이션이 제거되어 DTO 내 Bean Validation 제약 조건(@NotNull, @NotBlank 등)이 적용되지 않습니다. 동일 컨트롤러의 다른 엔드포인트(라인 44, 58)는 @Valid @RequestBody 패턴을 사용하고 있어 의도치 않은 변경으로 보입니다.
🔧 수정 제안
public ResponseEntity<SuccessResponse<SignUpResponse>> processSignup(
@RequestHeader("Authorization") @NotEmpty(message = "임시 토큰이 누락되었습니다.") String authorization,
- @RequestBody SignupRequest request
+ @Valid @RequestBody SignupRequest request
) {🤖 Prompt for AI Agents
In
@src/main/java/org/sopt/certi_server/domain/user/controller/AuthController.java
around lines 70 - 75, The processSignup method is missing the @Valid annotation
on its SignupRequest parameter so Bean Validation constraints in SignupRequest
are not applied; update the method signature in AuthController.processSignup to
annotate the request parameter with @Valid (i.e., use @Valid @RequestBody
SignupRequest request) and add the corresponding javax.validation.Valid import
if missing so request-level validation behaves consistently with the other
endpoints.
src/main/java/org/sopt/certi_server/domain/user/entity/Career.java
Outdated
Show resolved
Hide resolved
| upc.getCertification().getCertificationType().getKoreanName(), | ||
| upc.getCertification().getDescription(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find the Certification entity file
fd -t f -e "java" | xargs rg -l "class Certification" | head -5Repository: cerdeuk/CERTI-BE
Length of output: 536
🏁 Script executed:
# Also find the PreCertificationSimple file and examine it
fd -t f "PreCertificationSimple.java"Repository: cerdeuk/CERTI-BE
Length of output: 164
🏁 Script executed:
# Search for certificationType field definition across the codebase
rg -n "certificationType" --type java -B 2 -A 2 | head -100Repository: cerdeuk/CERTI-BE
Length of output: 12216
🏁 Script executed:
# Read the Certification entity file around the certificationType field
sed -n '38,50p' src/main/java/org/sopt/certi_server/domain/certification/entity/Certification.javaRepository: cerdeuk/CERTI-BE
Length of output: 335
🏁 Script executed:
# Also check the full field declarations with context
sed -n '35,85p' src/main/java/org/sopt/certi_server/domain/certification/entity/Certification.javaRepository: cerdeuk/CERTI-BE
Length of output: 1711
🏁 Script executed:
# Check PreCertificationSimple.java to see the from() method
cat -n src/main/java/org/sopt/certi_server/domain/userprecertification/dto/response/PreCertificationSimple.javaRepository: cerdeuk/CERTI-BE
Length of output: 1773
Line 27에서 getCertificationType() null 체크 필요
Certification 엔티티의 certificationType 필드는 @NotNull 어노테이션이 없고 데이터베이스 레벨에서도 NOT NULL 제약이 없어 null을 반환할 수 있습니다. 라인 27의 getCertificationType().getKoreanName() 호출 시 NPE가 발생할 수 있습니다.
다른 유사한 클래스인 CertificationSimple에서는 certificationType != null ? 로 null 체크를 수행하고 있으므로, 여기서도 동일하게 null 체크를 추가하거나 엔티티에 @Column(nullable=false) 제약을 추가해야 합니다.
🤖 Prompt for AI Agents
In
@src/main/java/org/sopt/certi_server/domain/userprecertification/dto/response/PreCertificationSimple.java
around lines 27 - 28, PreCertificationSimple currently calls
upc.getCertification().getCertificationType().getKoreanName() without null
protection which can NPE if Certification.certificationType is null; update the
mapping in PreCertificationSimple (where you build the DTO from upc) to mirror
CertificationSimple by guarding certificationType (e.g., use a null-safe
expression: upc.getCertification().getCertificationType() != null ?
upc.getCertification().getCertificationType().getKoreanName() : null) or
alternatively enforce not-null at the entity/DB level (add @Column(nullable =
false) / validation) and adjust usages accordingly.
- 사용자 정보 수정 API LocalDate 형식 스웨거 미반영 오류 해결 - 개인정보 수정 Dto에 검증 누락 해결
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java (2)
8-14: 중복된 import 문이 존재합니다.Lines 8-9에서 와일드카드 import (
request.*,response.*)를 사용하고 있어 lines 10-14의 개별 import 문은 불필요합니다.♻️ 제안하는 수정
import jakarta.validation.Valid; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.sopt.certi_server.domain.user.dto.request.*; import org.sopt.certi_server.domain.user.dto.response.*; -import org.sopt.certi_server.domain.user.dto.request.UpdateUserRequest; -import org.sopt.certi_server.domain.user.dto.response.GetJobResponse; -import org.sopt.certi_server.domain.user.dto.response.GetMyPageInfoResponse; -import org.sopt.certi_server.domain.user.dto.response.GetUserResponse; -import org.sopt.certi_server.domain.user.dto.response.PersonalInformationResponse; import org.sopt.certi_server.domain.user.service.UserService;
142-149: 토글 방식의 API 설계 검토 필요토글 패턴은 다음과 같은 문제가 있을 수 있습니다:
- 멱등성 부재: 동일 요청을 여러 번 보내면 결과가 달라집니다.
- 동시성 문제: 동시 요청 시 예상치 못한 상태가 될 수 있습니다.
- 클라이언트 의도 불명확: 클라이언트가 원하는 최종 상태를 명시하지 못합니다.
명시적으로 상태를 전달받는 방식을 고려해 보세요:
♻️ 제안하는 대안
- @PatchMapping(value = "/marketing-agreement") - @Operation(summary = "광고성 수신 정보 동의 토글 API", description = "광고성 수신 정보 동의를 토글식으로 변경합니다.") - public ResponseEntity<SuccessResponse<Void>> patchMarketingAgree( - @AuthenticationPrincipal Long userId - ){ - userService.toggleMarketingAgree(userId); - return ResponseEntity.ok(SuccessResponse.of(SuccessCode.SUCCESS_UPDATE)); - } + @PatchMapping(value = "/marketing-agreement") + @Operation(summary = "광고성 수신 정보 동의 수정 API", description = "광고성 수신 정보 동의 여부를 수정합니다.") + public ResponseEntity<SuccessResponse<Void>> patchMarketingAgree( + @AuthenticationPrincipal Long userId, + @Valid @RequestBody UpdateMarketingRequest request + ){ + userService.updateMarketingAgree(userId, request.isAgreed()); + return ResponseEntity.ok(SuccessResponse.of(SuccessCode.SUCCESS_UPDATE)); + }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/org/sopt/certi_server/domain/user/controller/UserController.javasrc/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUserRequest.javasrc/main/java/org/sopt/certi_server/domain/userprecertification/controller/UserPreCertificationController.java
💤 Files with no reviewable changes (1)
- src/main/java/org/sopt/certi_server/domain/userprecertification/controller/UserPreCertificationController.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUserRequest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (2)
src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java (2)
61-67: @Valid 추가 확인 - 일관성 검토 필요
@Valid어노테이션 추가는 적절합니다. 다만, 같은 컨트롤러 내 다른@RequestBody파라미터들(lines 82, 101, 111)에는@Valid가 없는데, 해당 DTO들에도 validation constraint가 있다면 동일하게 적용하는 것이 좋습니다.
125-131: LGTM!Presigned URL 조회 API가 기존 컨트롤러 패턴에 맞게 잘 구현되었습니다.
- nickname -> name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/main/java/org/sopt/certi_server/domain/activity/dto/request/CreateActivityRequest.java`:
- Around line 11-17: The class CreateActivityRequest uses the `@Schema` annotation
but the import is missing; add the import statement for
io.swagger.v3.oas.annotations.media.Schema to the CreateActivityRequest file so
the annotations on fields like startAt and endAt resolve and the code compiles.
Ensure the import is added alongside the other imports in the
CreateActivityRequest class where `@NotNull` and `@JsonFormat` are used.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/org/sopt/certi_server/domain/activity/dto/request/CreateActivityRequest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/org/sopt/certi_server/domain/activity/dto/request/CreateActivityRequest.java (3)
src/main/java/org/sopt/certi_server/domain/user/dto/request/CreateCareerRequest.java (1)
CreateCareerRequest(9-24)src/main/java/org/sopt/certi_server/domain/activity/service/ActivityService.java (1)
ActivityService(19-66)src/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUserRequest.java (1)
UpdateUserRequest(13-26)
🪛 GitHub Actions: CD
src/main/java/org/sopt/certi_server/domain/activity/dto/request/CreateActivityRequest.java
[error] 12-12: Cannot find symbol: class Schema. Annotation @Schema used for '활동 시작일' is not resolved (likely missing import or dependency).
[error] 17-17: Cannot find symbol: class Schema. Annotation @Schema used for '활동 종료일' is not resolved (likely missing import or dependency).
🪛 GitHub Actions: CI
src/main/java/org/sopt/certi_server/domain/activity/dto/request/CreateActivityRequest.java
[error] 12-12: cannot find symbol: class Schema. Ensure proper import for io.swagger.v3.oas.annotations.media.Schema or add the necessary dependency.
[error] 17-17: cannot find symbol: class Schema. Ensure proper import for io.swagger.v3.oas.annotations.media.Schema or add the necessary dependency.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
📣 Related Issue
📝 Summary
🙏 Question & PR point
📬 Postman
Summary by CodeRabbit
신기능
개선사항
✏️ Tip: You can customize this high-level summary in your review settings.