-
Notifications
You must be signed in to change notification settings - Fork 0
#170 [Feat] 마이페이지 기능 구현 #173
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
Conversation
- 욕설, 비속어 검증 아호코라식 알고리즘 사용 - 검증 테스트 추가
- SignupRequest에 nickname 필드 추가 - 기존 UserInformation 필드의 nickname을 name으로 변경 - 닉네임 검증 API 추가 및 테스트
- 취득, 취득 예정 정보 수정 API 추가 - 취득, 취득 예정 생성 API 수정
- 욕설, 비속어 검증 아호코라식 알고리즘 사용 - 검증 테스트 추가
- SignupRequest에 nickname 필드 추가 - 기존 UserInformation 필드의 nickname을 name으로 변경 - 닉네임 검증 API 추가 및 테스트
- 취득, 취득 예정 정보 수정 API 추가 - 취득, 취득 예정 생성 API 수정
- 사용자 정보 변경에서 사용하는 닉네임 검증 메서드와 단순 닉네임 검증 메서드 분리
|
Caution Review failedThe pull request is closed. Walkthrough마이페이지·개인정보·닉네임 검증 및 취득 예정/완료 관련 API와 도메인 모델을 확장하고, Aho-Corasick 기반 비속어 필터(NoProfanity)와 관련 검증기·예외·테스트를 추가했습니다. 또한 인증 화이트리스트 및 추천(랭킹) 로직을 도입했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as UserController
participant Service as UserService
participant Filter as ProfanityFilter
participant Repo as UserRepository
participant Response
Client->>Controller: GET /api/v1/user/validation?keyword=nick
Controller->>Service: validateNickname(<userId?>, nickname)
rect rgba(230,230,250,0.6)
Note over Service,Filter: 닉네임 유효성 검사
Service->>Service: validateKeyword(nickname) (빈값/길이 체크)
Service->>Filter: containsProfanity(nickname)
Filter-->>Service: boolean
end
alt contains profanity
Service-->>Controller: throw InvalidNicknameException(NICKNAME_CONTAINS_PROFANITY)
Controller-->>Response: 400 Bad Request
else passed profanity
Service->>Repo: existsByNickname(nickname)
Repo-->>Service: boolean
alt exists == true
Service-->>Controller: throw InvalidNicknameException(NICKNAME_DUPLICATE)
Controller-->>Response: 400 Bad Request
else
Controller-->>Response: 200 SUCCESS
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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 (5)
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: 15
Fix all issues with AI Agents 🤖
In
@src/main/java/org/sopt/certi_server/domain/acquisition/controller/AcquisitionController.java:
- Around line 28-38: The @Parameter annotation is incorrectly applied to the
@RequestBody parameter in addAcquisition; remove the @Parameter(...) on the
CreateAcquisitionRequest parameter and document the request body either by
annotating the parameter with
io.swagger.v3.oas.annotations.parameters.RequestBody (e.g.,
@RequestBody(description="certification payload", required=true)) or by adding
@Schema annotations inside CreateAcquisitionRequest; update the addAcquisition
method signature to use the proper request-body annotation and/or ensure
CreateAcquisitionRequest fields carry @Schema metadata.
In
@src/main/java/org/sopt/certi_server/domain/acquisition/dto/request/CreateAcquisitionRequest.java:
- Around line 3-5: Remove the unused jakarta.validation imports for @NotBlank
and @NotEmpty from CreateAcquisitionRequest and keep only @NotNull for
Long-typed fields; update the import list to remove
jakarta.validation.constraints.NotBlank and
jakarta.validation.constraints.NotEmpty and ensure any field-level annotations
in the CreateAcquisitionRequest DTO use @NotNull for Long properties (no
@NotBlank/@NotEmpty).
In
@src/main/java/org/sopt/certi_server/domain/acquisition/entity/Acquisition.java:
- Around line 56-62: The Acquisition constructor currently accepts a grade but
always sets acquisitionDate = LocalDate.now(), preventing creation of
acquisitions with specific dates and causing inconsistency with
changeAcquisition; modify the constructor (public Acquisition(Certification
certification, CardType cardType, User user, SmallCardType smallCardType, String
grade)) to accept a LocalDate acquisitionDate parameter and assign it to the
acquisitionDate field, or add an overloaded constructor that takes
acquisitionDate while keeping the existing one delegating to the new constructor
with LocalDate.now() so callers can opt to supply an explicit date.
In
@src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java:
- Around line 94-103: Replace the raw String request body with a validated
request DTO: create a record/class named UpdateUniversityRequest with a field
universityName annotated with @NotBlank, then change the controller method
updateUniversity to accept @Valid @RequestBody UpdateUniversityRequest request
(retain @AuthenticationPrincipal Long userId), log request.universityName(), and
pass request.universityName() to userService.changeUniversity; ensure validation
is enabled (e.g., add @Valid) and adjust imports accordingly.
- Around line 105-113: Wrap the raw String request into a DTO: create a record
UpdateMajorRequest with a @NotBlank String majorName, annotate the controller
parameter with @RequestBody @Valid UpdateMajorRequest (import
javax.validation.Valid) in the UserController.updateMajor method and pass
request.majorName() into userService.changeMajor; ensure validation annotations
are present on the DTO and that controller uses the DTO instead of @RequestBody
String majorName.
In
@src/main/java/org/sopt/certi_server/domain/user/dto/request/SignupRequest.java:
- Line 17: SignupRequest's nickname field was made required via the
@NotEmpty/@Size/@NoProfanity annotations which will break clients that don't
send it; either ensure all clients (web/mobile/legacy) are updated to include
nickname in signup payloads or add a backward-compatibility change server-side
(e.g., relax @NotEmpty on SignupRequest.nickname, accept null in the service
layer and assign a default or return a clear validation error with migration
guidance, or introduce a new API version for required-nickname). Update client
teams and/or implement one of these server-side strategies referencing the
SignupRequest class and its nickname field to restore compatibility before
deploying.
In @src/main/java/org/sopt/certi_server/domain/user/service/UserService.java:
- Around line 176-191: In validateKeyword, avoid NPE and remove redundant
isEmpty(): first check for null or blank (e.g., if (nickname == null ||
nickname.isBlank()) throw InvalidNicknameException(ErrorCode.NICKNAME_EMPTY));
then keep the length check (nickname.length() > 7) and the profanity check using
profanityFilter.containsProfanity(nickname); this ensures null-safety and
eliminates the duplicate isEmpty() condition while preserving existing error
codes and behavior.
In
@src/main/java/org/sopt/certi_server/domain/userprecertification/dto/request/CreateUserPreCertificationRequest.java:
- Around line 13-15: The testDate field in CreateUserPreCertificationRequest is
declared as LocalDateTime but uses a date-only JSON pattern ("yyyy.MM.dd"),
causing parsing errors; change the field type to java.time.LocalDate (matching
PatchPreCertificationRequest) and update any usages/validation to accept
LocalDate, or alternatively keep LocalDateTime and change the @JsonFormat
pattern to include time (e.g., "yyyy.MM.dd HH:mm:ss"); update the DTO field
(testDate) and any consumers/serializers accordingly to ensure consistent types
across CreateUserPreCertificationRequest and PatchPreCertificationRequest.
In
@src/main/java/org/sopt/certi_server/domain/userprecertification/dto/request/PatchPreCertificationRequest.java:
- Around line 10-12: In PatchPreCertificationRequest the field testDate is
declared as LocalDateTime but annotated with a date-only pattern "yyyy.MM.dd",
which can cause parse errors; either change the field type to LocalDate and keep
the @JsonFormat pattern as-is, or update the @JsonFormat pattern to include time
components (e.g., hours/minutes/seconds) and ensure clients send full datetime
strings; modify the testDate declaration and its annotation accordingly in the
PatchPreCertificationRequest class to match the chosen option.
In
@src/main/java/org/sopt/certi_server/domain/userprecertification/entity/UserPreCertification.java:
- Around line 44-45: The testDate field in UserPreCertification currently uses
LocalDateTime which lacks timezone info; change the field type to
java.time.Instant (update the field declaration named testDate in class
UserPreCertification), adjust any getters/setters/constructors and places that
construct or parse this value to use Instant (or convert using
ZonedDateTime/OffsetDateTime as needed), and ensure the JPA mapping
(@Column/name = "test_date") and DB column use a timezone-aware type (e.g.,
TIMESTAMP WITH TIME ZONE) or add appropriate columnDefinition/attribute
converter so instants are persisted as UTC.
In
@src/main/java/org/sopt/certi_server/domain/userprecertification/service/UserPreCertificationService.java:
- Line 93: The equality check uses reference comparison (upc.getUser() != user)
which can fail for different entity instances; replace it with a null-safe
ID-based comparison or equals-based check — e.g., compare upc.getUser().getId()
to user.getId() (or use !Objects.equals(upc.getUser(), user) only if equals is
properly implemented) and throw the same
ForbiddenException(ErrorCode.ACCESS_DENIED) when IDs differ; ensure you guard
against null user or null id to avoid NPEs.
In @src/main/java/org/sopt/certi_server/global/error/code/ErrorCode.java:
- Line 43: The CERTIFICATION_NOT_FOUND entry in the ErrorCode enum contains a
leading space in its code string (" E404009") which can break error matching;
open the ErrorCode enum and update the CERTIFICATION_NOT_FOUND constant to use
the trimmed code value ("E404009") so the stored code has no leading/trailing
whitespace, and run tests or search usages of ErrorCode to ensure matching still
works.
In @src/main/java/org/sopt/certi_server/global/valid/ProfanityFilter.java:
- Around line 15-24: The ProfanityFilter currently hardcodes badWords inside the
init() method; move that list to external configuration and load it at startup
so additions don't require code changes: add a configuration property (e.g., a
List<String> profanity.words via @ConfigurationProperties or @Value or read from
resources/profanity-words.txt), inject that List into the ProfanityFilter class,
and change init() to build the Trie using the injected List<String> (refer to
ProfanityFilter.init() and the trie field) instead of List.of(...); ensure
null/empty checks and appropriate logging if the configured list is missing.
In @src/main/java/org/sopt/certi_server/global/valid/ProfanityValidator.java:
- Around line 19-22: The isValid method in ProfanityValidator should treat null
as valid to avoid NPE from ProfanityFilter.containsProfanity; modify
ProfanityValidator.isValid to first check if value is null and return true in
that case, otherwise call filter.containsProfanity(value) and return the
negation; rely on @NotNull for requiring non-null values elsewhere.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/sopt/certi_server/domain/acquisition/controller/AcquisitionController.java (1)
61-70: Operation description 오타 수정 필요Line 62의 description이 "상세 조회합니다"로 되어 있으나, DELETE API이므로 "삭제합니다"가 맞습니다. 복사-붙여넣기 오류로 보입니다.
🔎 제안된 수정
@DeleteMapping(value = "/{acquisitionId}") -@Operation(summary = "취득한 자격증 삭제 API", description = "취득한 자격증을 상세 조회합니다") +@Operation(summary = "취득한 자격증 삭제 API", description = "취득한 자격증을 삭제합니다") public ResponseEntity<SuccessResponse<Void>> deleteAcquisition(
🧹 Nitpick comments (24)
src/main/java/org/sopt/certi_server/global/annotation/NoProfanity.java (1)
9-17: LGTM! Jakarta Bean Validation 커스텀 어노테이션이 올바르게 구현되었습니다.표준 제약 조건 어노테이션 구조를 따르고 있습니다.
메서드 파라미터에서도 직접 검증이 필요한 경우를 대비해
ElementType.PARAMETER를 추가하는 것을 고려해볼 수 있습니다:🔎 선택적 개선안
-@Target({ElementType.FIELD}) +@Target({ElementType.FIELD, ElementType.PARAMETER})src/main/java/org/sopt/certi_server/domain/user/entity/User.java (1)
87-92: changeUser 메서드의 부분 업데이트 지원 여부를 검토하세요.현재
changeUser메서드는 모든 필드를 한 번에 업데이트합니다. 일부 필드만 변경이 필요한 경우, null이 전달되면 기존 값을 덮어쓸 수 있습니다.서비스 레이어에서 호출 시 항상 모든 값을 전달하는지 확인이 필요합니다. 부분 업데이트가 필요하다면 null 체크 로직을 고려해볼 수 있습니다:
🔎 부분 업데이트 지원 예시 (필요시)
public void changeUser(String name, String nickname, String email, LocalDate birthDate){ - this.name = name; - this.nickname = nickname; - this.email = email; - this.birthDate = birthDate; + if (name != null) this.name = name; + if (nickname != null) this.nickname = nickname; + if (email != null) this.email = email; + if (birthDate != null) this.birthDate = birthDate; }src/test/java/org/sopt/certi_server/domain/user/dto/request/UpdateUserRequestTest.java (2)
14-14: 사용하지 않는 import 제거
org.junit.jupiter.api.Assertions.*는 테스트 코드에서 사용되지 않습니다. AssertJ의assertThat만 사용되고 있으므로 해당 import를 제거하는 것이 좋습니다.
22-41: 테스트 커버리지 확장 권장현재 하나의 비속어 케이스만 테스트하고 있습니다. 다음과 같은 추가 테스트 케이스를 고려해보세요:
- 여러 비속어가 포함된 경우
- 비속어가 문자열의 시작/중간/끝에 위치한 경우
- 경계값 테스트 (길이 제한과 함께)
참고:
UserServiceTest.java에 이미 포괄적인 닉네임 검증 테스트가 있으므로, 여기서는 DTO 레벨의 validation 동작만 확인하는 것으로도 충분할 수 있습니다.src/main/java/org/sopt/certi_server/domain/userprecertification/entity/UserPreCertification.java (1)
69-72: 업데이트 메서드에 입력 검증을 추가하세요.
changeUserPreCertification메서드가null값을 검증하지 않습니다.location과testDate가null인 경우를 처리하는 로직을 추가하거나, 메서드 파라미터에@NonNull애노테이션을 추가하는 것을 권장합니다.🔎 제안하는 수정안
-public void changeUserPreCertification(Location location, LocalDateTime testDate){ +public void changeUserPreCertification(Location location, LocalDateTime testDate){ + if (location == null || testDate == null) { + throw new IllegalArgumentException("location and testDate must not be null"); + } this.location = location; this.testDate = testDate; }src/main/java/org/sopt/certi_server/domain/user/dto/response/GetMyPageInfoResponse.java (1)
17-28: 팩토리 메서드에 null 검증을 추가하는 것을 고려하세요.
from메서드가user와jobResponse파라미터에 대한null검증을 수행하지 않습니다.user.getNickname()또는user.getEmail()호출 시 NPE가 발생할 수 있습니다. 방어적 검증 추가를 권장합니다.🔎 제안하는 수정안
public static GetMyPageInfoResponse from( User user, GetJobResponse jobResponse, int upCount, int acCount, int fCount ){ + if (user == null) { + throw new IllegalArgumentException("user must not be null"); + } return GetMyPageInfoResponse.builder() .nickname(user.getNickname()) .email(user.getEmail()) .jobResponse(jobResponse) .upCount(upCount) .acCount(acCount) .fCount(fCount) .build(); }src/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUserRequest.java (1)
1-11: 사용되지 않는 import를 제거하는 것이 좋습니다.Lines 4와 8에 사용되지 않는 import가 포함되어 있습니다:
jakarta.validation.Valid: 현재 코드에서 사용되지 않음lombok.Getter: record는 암묵적으로 getter를 제공하므로 불필요🔎 제안하는 수정사항
package org.sopt.certi_server.domain.user.dto.request; import com.fasterxml.jackson.annotation.JsonFormat; -import jakarta.validation.Valid; import jakarta.validation.constraints.Email; import jakarta.validation.constraints.NotEmpty; import jakarta.validation.constraints.Past; -import lombok.Getter; import org.sopt.certi_server.global.annotation.NoProfanity; import java.time.LocalDate;src/main/java/org/sopt/certi_server/global/valid/ProfanityValidator.java (1)
1-10: 사용되지 않는 import를 제거하는 것이 좋습니다.Line 8의
org.springframework.beans.factory.annotation.Autowiredimport가 사용되지 않습니다. 이 클래스는@RequiredArgsConstructor를 통해 의존성 주입을 처리하고 있습니다.🔎 제안하는 수정사항
package org.sopt.certi_server.global.valid; import jakarta.validation.ConstraintValidator; import jakarta.validation.ConstraintValidatorContext; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.sopt.certi_server.global.annotation.NoProfanity; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component;src/main/java/org/sopt/certi_server/domain/acquisition/dto/request/PatchAcquisitionRequest.java (2)
8-15: 전체적인 구조는 적절하나 grade 필드 검증 추가를 권장합니다.Record 구조와 acquisitionDate의 검증은 적절합니다. 다만 grade 필드에 대한 검증이 없어 빈 문자열이나 과도하게 긴 값이 허용될 수 있습니다.
14-14: grade 필드에 유효성 검증 추가를 고려하세요.grade 필드가 선택적(optional)이더라도, 값이 제공될 경우 빈 문자열이나 과도하게 긴 값을 방지하기 위한 검증이 필요합니다.
🔎 제안하는 수정 사항
+import jakarta.validation.constraints.Size; + public record PatchAcquisitionRequest( @NotNull(message = "취득 날짜 정보를 입력해주세요") @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy.MM.dd", timezone = "Asia/Seoul") LocalDate acquisitionDate, + @Size(max = 50, message = "등급은 최대 50자까지 입력 가능합니다") String grade ) { }src/main/java/org/sopt/certi_server/domain/user/dto/response/OAuthUserInformation.java (1)
11-11: 검증 메시지와 필드명 불일치필드명이
nickname에서name으로 변경되었지만, 검증 메시지는 여전히 "사용자 닉네임 정보는 필수입니다"로 되어 있습니다. 의미적 일관성을 위해 메시지도 함께 수정하는 것이 좋습니다.🔎 제안하는 수정 사항
- @NotEmpty(message = "사용자 닉네임 정보는 필수입니다.") String name, + @NotEmpty(message = "사용자 이름 정보는 필수입니다.") String name,src/main/java/org/sopt/certi_server/domain/acquisition/dto/response/GetAcquisitionListResponse.java (1)
6-9: 오타 및 네이밍 불일치
- Line 8: 파라미터명에 오타가 있습니다:
priorCertificaitonResponseList→priorCertificationResponseList- 필드명
acquisitionListDetailResponses가 실제 타입GetAcquisitionResponse와 맞지 않습니다.🔎 제안하는 수정 사항
public record GetAcquisitionListResponse( - List<GetAcquisitionResponse> acquisitionListDetailResponses + List<GetAcquisitionResponse> acquisitionResponses ) { - public static GetAcquisitionListResponse of(List<GetAcquisitionResponse> priorCertificaitonResponseList) { - return new GetAcquisitionListResponse(priorCertificaitonResponseList); + public static GetAcquisitionListResponse of(List<GetAcquisitionResponse> acquisitionResponseList) { + return new GetAcquisitionListResponse(acquisitionResponseList); } }src/main/java/org/sopt/certi_server/domain/userprecertification/dto/response/PreCertificationSimple.java (1)
15-18: testDate 필드에 @jsonformat 누락
nearestTestDate(Line 13)에는@JsonFormat이 적용되어 있지만, 새로 추가된testDate에는 없습니다. API 응답에서 날짜 형식이 일관되지 않을 수 있습니다.🔎 제안하는 수정 사항
int iconIndex, String city, String state, - LocalDateTime testDate + @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy.MM.dd", timezone = "Asia/Seoul") + LocalDateTime testDate ) {src/main/java/org/sopt/certi_server/domain/acquisition/service/AcquisitionService.java (2)
52-62: 중복 데이터베이스 쿼리동일한
findFirstByUserOrderByCreatedTimeDesc(user)쿼리가 두 번 호출됩니다. 한 번의 조회로 결과를 재사용하면 성능을 개선할 수 있습니다.🔎 제안하는 수정 사항
- CardType cardType = acquisitionRepository.findFirstByUserOrderByCreatedTimeDesc(user) + var lastAcquisition = acquisitionRepository.findFirstByUserOrderByCreatedTimeDesc(user); + + CardType cardType = lastAcquisition .map(acquisition -> { int index = acquisition.getCardType().getIndex(); return issueNextCardType(index); }).orElseGet(CardType::issueRandomCardType); - SmallCardType smallCardType = acquisitionRepository.findFirstByUserOrderByCreatedTimeDesc(user) + SmallCardType smallCardType = lastAcquisition .map(acquisition -> { int index = acquisition.getSmallCardType().getIndex(); return issueNextSmallCardType(index); }).orElseGet(SmallCardType::issueRandomSmallCardType);
104-130: 권한 검증 로직 중복
patchAcquisition(Lines 122-124)과deleteAcquisition(Lines 109-111)에서 동일한 권한 검증 패턴이 반복됩니다. 헬퍼 메서드로 추출하면 유지보수성이 향상됩니다.🔎 제안하는 수정 사항
private void validateAcquisitionOwnership(User user, Acquisition acquisition) { if (!Objects.equals(acquisition.getUser().getId(), user.getId())) { throw new ForbiddenException(ErrorCode.ACCESS_DENIED); } }이후 두 메서드에서:
validateAcquisitionOwnership(findUser, findAcquisition);src/test/java/org/sopt/certi_server/domain/user/service/UserServiceTest.java (3)
76-77: 테스트 데이터 의존성 취약
findById(1L)및findByName(...)으로 기존 데이터베이스 레코드에 의존하고 있습니다. 테스트 환경에 따라 해당 데이터가 없으면 테스트가 실패할 수 있습니다.테스트 데이터를 직접 생성하거나
@Sql어노테이션으로 테스트 데이터를 로드하는 것이 더 안정적입니다.
50-52: 미사용 Repository 주입
certificationCommentRepository와certificationCommentLikeRepository가 주입되었지만 현재 테스트에서 사용되지 않습니다. 향후 테스트 추가 예정이 아니라면 제거를 고려해 주세요.
158-173: 테스트 네이밍 명확성
@DisplayName에서 "[성공]"이 사용되었지만, 실제로는 예외가 발생해야 하는 시나리오입니다. "[실패]" 또는 "[예외]"로 변경하면 테스트 의도가 더 명확해집니다.🔎 제안 예시
- @DisplayName("[성공] 닉네임에 욕설이 포함되어 있다.") + @DisplayName("[예외] 닉네임에 욕설이 포함되면 InvalidNicknameException 발생")src/main/java/org/sopt/certi_server/domain/acquisition/entity/Acquisition.java (1)
49-50: grade 필드에 대한 검증을 추가하세요.
grade필드가 nullable하며 형식에 대한 검증이 없습니다. 잘못된 값이나 빈 문자열이 저장될 수 있습니다.🔎 검증 로직 추가 제안
Entity에 Bean Validation 어노테이션을 추가하거나, 비즈니스 로직에서 검증하는 것을 고려하세요:
+@Column(name = "grade", length = 20) -@Column(name = "grade") private String grade;또는 서비스 레이어에서 grade 값의 유효성을 검증하세요.
src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java (1)
100-100: 로깅 일관성을 개선하세요.
updateUniversity메서드에만 로그 문이 있고 다른 메서드들(updateMajor,putPersonalInformation등)에는 없습니다.src/main/java/org/sopt/certi_server/domain/userprecertification/controller/UserPreCertificationController.java (2)
30-30: 검증 어노테이션 사용의 일관성을 개선하세요.
getPreCertificationListData(Line 30)에서는@NotNull검증이 제거되었지만,deletePreCertification(Line 60)에서는 여전히 사용되고 있습니다.Spring Security의
@AuthenticationPrincipal은 인증되지 않은 경우 이미 예외를 발생시키므로@NotNull은 중복 검증입니다. 일관성을 위해 모두 제거하거나 모두 유지하는 것이 좋습니다.🔎 일관성을 위한 수정 제안
@DeleteMapping(value = "/{certificationId}") @Operation(summary = "취득예정 자격증 삭제 API", description = "취득예정 자격증을 삭제합니다") public ResponseEntity<SuccessResponse<Void>> deletePreCertification( - @AuthenticationPrincipal @NotNull(message = "인증되지 않은 사용자입니다.") Long userId, + @AuthenticationPrincipal Long userId, @Parameter(description = "certification Id", example = "1") @PathVariable Long certificationId ) {Also applies to: 60-60
46-55: 경로 변수 명명의 일관성을 개선하세요.PATCH 엔드포인트는
/{userPreCertificationId}를 사용하고 DELETE 엔드포인트는/{certificationId}를 사용합니다. 두 엔드포인트 모두 UserPreCertification 엔티티를 다루지만, 참조 방식이 불일치합니다:
- PATCH는 UserPreCertification 엔티티의 ID를 직접 사용
- DELETE는 Certification 엔티티의 ID를 사용하여 간접적으로 UserPreCertification을 조회
DELETE도
/{userPreCertificationId}로 변경하고, 서비스 메서드를userPreCertificationId를 직접 받도록 리팩토링하면 API 설계가 더욱 명확하고 일관성 있어집니다.Also applies to: 57-66
src/main/java/org/sopt/certi_server/domain/user/service/UserService.java (2)
157-165: 검증 순서 개선 권장현재 중복 검사 후
validateKeyword를 호출하고 있습니다.nickname이null인 경우 Line 160의Objects.equals는 정상 동작하지만, Line 164에서validateKeyword호출 시 NPE가 발생합니다.
validateKeyword를 먼저 호출하여 null/blank 검사를 수행한 후 중복 검사를 진행하는 것이 더 안전합니다. 이렇게 하면 불필요한 DB 조회도 방지할 수 있습니다.🔎 제안된 수정
public void validateNickname(final Long userId, String nickname) { + validateKeyword(nickname); User user = getUser(userId); if(!Objects.equals(user.getNickname(), nickname) && userRepository.existsByNickname(nickname)) { throw new InvalidNicknameException(ErrorCode.NICKNAME_DUPLICATE); } - - validateKeyword(nickname); }
167-174: 동일한 검증 순서 문제위의 오버로드된 메서드와 동일한 문제가 있습니다.
validateKeyword를 먼저 호출하여 null/blank 검사를 수행한 후 중복 검사를 진행하세요.🔎 제안된 수정
public void validateNickname(String nickname) { + validateKeyword(nickname); if(userRepository.existsByNickname(nickname)) { throw new InvalidNicknameException(ErrorCode.NICKNAME_DUPLICATE); } - - validateKeyword(nickname); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
build.gradlesrc/main/java/org/sopt/certi_server/domain/acquisition/controller/AcquisitionController.javasrc/main/java/org/sopt/certi_server/domain/acquisition/dto/request/CreateAcquisitionRequest.javasrc/main/java/org/sopt/certi_server/domain/acquisition/dto/request/PatchAcquisitionRequest.javasrc/main/java/org/sopt/certi_server/domain/acquisition/dto/response/GetAcquisitionDetailResponse.javasrc/main/java/org/sopt/certi_server/domain/acquisition/dto/response/GetAcquisitionListDetailResponse.javasrc/main/java/org/sopt/certi_server/domain/acquisition/dto/response/GetAcquisitionListResponse.javasrc/main/java/org/sopt/certi_server/domain/acquisition/dto/response/GetAcquisitionResponse.javasrc/main/java/org/sopt/certi_server/domain/acquisition/entity/Acquisition.javasrc/main/java/org/sopt/certi_server/domain/acquisition/service/AcquisitionService.javasrc/main/java/org/sopt/certi_server/domain/favorite/repository/FavoriteRepository.javasrc/main/java/org/sopt/certi_server/domain/user/controller/UserController.javasrc/main/java/org/sopt/certi_server/domain/user/dto/request/SignupRequest.javasrc/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUserRequest.javasrc/main/java/org/sopt/certi_server/domain/user/dto/response/GetMyPageInfoResponse.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/PersonalInformationResponse.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/User.javasrc/main/java/org/sopt/certi_server/domain/user/repository/UserRepository.javasrc/main/java/org/sopt/certi_server/domain/user/service/AuthService.javasrc/main/java/org/sopt/certi_server/domain/user/service/KakaoService.javasrc/main/java/org/sopt/certi_server/domain/user/service/UserService.javasrc/main/java/org/sopt/certi_server/domain/userprecertification/controller/UserPreCertificationController.javasrc/main/java/org/sopt/certi_server/domain/userprecertification/dto/request/CreateUserPreCertificationRequest.javasrc/main/java/org/sopt/certi_server/domain/userprecertification/dto/request/PatchPreCertificationRequest.javasrc/main/java/org/sopt/certi_server/domain/userprecertification/dto/response/PreCertificationSimple.javasrc/main/java/org/sopt/certi_server/domain/userprecertification/entity/Location.javasrc/main/java/org/sopt/certi_server/domain/userprecertification/entity/UserPreCertification.javasrc/main/java/org/sopt/certi_server/domain/userprecertification/repository/UserPreCertificationRepository.javasrc/main/java/org/sopt/certi_server/domain/userprecertification/service/UserPreCertificationService.javasrc/main/java/org/sopt/certi_server/global/annotation/NoProfanity.javasrc/main/java/org/sopt/certi_server/global/config/SecurityConfig.javasrc/main/java/org/sopt/certi_server/global/error/code/ErrorCode.javasrc/main/java/org/sopt/certi_server/global/error/exception/InvalidNicknameException.javasrc/main/java/org/sopt/certi_server/global/filter/JwtAuthenticationFilter.javasrc/main/java/org/sopt/certi_server/global/valid/ProfanityFilter.javasrc/main/java/org/sopt/certi_server/global/valid/ProfanityValidator.javasrc/test/java/org/sopt/certi_server/domain/user/dto/request/UpdateUserRequestTest.javasrc/test/java/org/sopt/certi_server/domain/user/service/UserServiceTest.java
💤 Files with no reviewable changes (1)
- src/main/java/org/sopt/certi_server/domain/acquisition/dto/response/GetAcquisitionListDetailResponse.java
🧰 Additional context used
🧬 Code graph analysis (15)
src/main/java/org/sopt/certi_server/global/filter/JwtAuthenticationFilter.java (2)
src/main/java/org/sopt/certi_server/global/jwt/core/JwtValidator.java (1)
Component(13-50)src/main/java/org/sopt/certi_server/global/jwt/core/JwtExtractor.java (1)
JwtExtractor(13-54)
src/main/java/org/sopt/certi_server/domain/user/repository/UserRepository.java (2)
src/main/java/org/sopt/certi_server/domain/comment/repository/CertificationCommentLikeRepository.java (1)
CertificationCommentLikeRepository(14-45)src/main/java/org/sopt/certi_server/domain/acquisition/repository/AcquisitionRepository.java (2)
Repository(14-44)existsByUserAndCertification(31-31)
src/main/java/org/sopt/certi_server/domain/favorite/repository/FavoriteRepository.java (2)
src/main/java/org/sopt/certi_server/domain/activity/repository/ActivityRepository.java (1)
ActivityRepository(10-17)src/main/java/org/sopt/certi_server/domain/acquisition/repository/AcquisitionRepository.java (1)
countByUser(33-33)
src/main/java/org/sopt/certi_server/domain/user/dto/response/UserInformation.java (2)
src/main/java/org/sopt/certi_server/global/client/KakaoApiFeignClient.java (1)
getInformation(11-12)src/main/java/org/sopt/certi_server/domain/user/dto/response/kakao/KakaoOAuthResponse.java (1)
KakaoOAuthResponse(6-16)
src/main/java/org/sopt/certi_server/domain/acquisition/dto/response/GetAcquisitionListResponse.java (1)
src/main/java/org/sopt/certi_server/domain/acquisition/dto/response/GetAcquisitionListDetailResponse.java (2)
GetAcquisitionListDetailResponse(12-34)from(23-33)
src/main/java/org/sopt/certi_server/domain/user/dto/response/PersonalInformationResponse.java (5)
src/main/java/org/sopt/certi_server/domain/user/dto/response/SignUpResponse.java (1)
SignUpResponse(9-32)src/main/java/org/sopt/certi_server/domain/user/dto/response/AuthResponse.java (1)
AuthResponse(5-21)src/main/java/org/sopt/certi_server/domain/user/dto/response/CareerDetailResponse.java (1)
CareerDetailResponse(9-30)src/main/java/org/sopt/certi_server/domain/certification/dto/response/CertificationDetailResponse.java (1)
CertificationDetailResponse(10-43)src/main/java/org/sopt/certi_server/domain/comment/dto/response/CertificationCommentResponse.java (1)
CertificationCommentResponse(8-42)
src/main/java/org/sopt/certi_server/global/valid/ProfanityValidator.java (1)
src/main/java/org/sopt/certi_server/global/valid/ProfanityFilter.java (1)
Component(9-31)
src/main/java/org/sopt/certi_server/domain/user/dto/response/GetUserResponse.java (2)
src/main/java/org/sopt/certi_server/domain/user/dto/response/AuthResponse.java (1)
AuthResponse(5-21)src/main/java/org/sopt/certi_server/domain/user/dto/response/SignUpResponse.java (1)
SignUpResponse(9-32)
src/main/java/org/sopt/certi_server/domain/user/dto/response/OAuthUserInformation.java (1)
src/main/java/org/sopt/certi_server/domain/user/dto/response/AuthResponse.java (1)
AuthResponse(5-21)
src/main/java/org/sopt/certi_server/global/error/exception/InvalidNicknameException.java (6)
src/main/java/org/sopt/certi_server/global/error/exception/InvalidValueException.java (2)
InvalidValueException(5-9)InvalidValueException(6-8)src/main/java/org/sopt/certi_server/global/error/exception/BusinessException.java (2)
BusinessException(7-15)BusinessException(11-14)src/main/java/org/sopt/certi_server/global/error/exception/UnauthorizedException.java (1)
UnauthorizedException(5-14)src/main/java/org/sopt/certi_server/global/error/exception/ForbiddenException.java (2)
ForbiddenException(5-9)ForbiddenException(6-8)src/main/java/org/sopt/certi_server/global/error/exception/NotFoundException.java (1)
NotFoundException(5-9)src/main/java/org/sopt/certi_server/global/error/exception/BadRequestException.java (2)
BadRequestException(5-13)BadRequestException(10-12)
src/test/java/org/sopt/certi_server/domain/user/service/UserServiceTest.java (2)
src/main/java/org/sopt/certi_server/global/error/exception/InvalidNicknameException.java (1)
InvalidNicknameException(5-9)src/test/java/org/sopt/certi_server/domain/user/dto/request/UpdateUserRequestTest.java (1)
SpringBootTest(16-42)
src/main/java/org/sopt/certi_server/global/valid/ProfanityFilter.java (1)
src/main/java/org/sopt/certi_server/global/valid/ProfanityValidator.java (1)
Component(11-23)
src/main/java/org/sopt/certi_server/domain/user/service/UserService.java (2)
src/main/java/org/sopt/certi_server/global/error/exception/InvalidNicknameException.java (1)
InvalidNicknameException(5-9)src/main/java/org/sopt/certi_server/global/error/exception/NotFoundException.java (1)
NotFoundException(5-9)
src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java (1)
src/main/java/org/sopt/certi_server/global/filter/JwtAuthenticationFilter.java (1)
Slf4j(22-80)
src/test/java/org/sopt/certi_server/domain/user/dto/request/UpdateUserRequestTest.java (1)
src/test/java/org/sopt/certi_server/domain/user/service/UserServiceTest.java (1)
SpringBootTest(38-243)
⏰ 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 (38)
src/main/java/org/sopt/certi_server/domain/userprecertification/entity/Location.java (1)
9-17: LGTM! 임베더블 값 객체로 적절하게 구현되었습니다.JPA
@Embeddable패턴을 올바르게 사용하고 있습니다. 선택적으로, 캡슐화 강화를 위해@NoArgsConstructor(access = AccessLevel.PROTECTED)를 고려해볼 수 있습니다.src/main/java/org/sopt/certi_server/global/error/code/ErrorCode.java (1)
20-23: LGTM! 닉네임 검증 에러 코드가 적절하게 추가되었습니다.닉네임 검증 요구사항(길이, 공백, 중복, 비속어)에 맞는 에러 코드들이 일관된 네이밍과 코드 체계로 정의되었습니다.
src/main/java/org/sopt/certi_server/domain/user/entity/User.java (3)
42-46: LGTM! nickname 유니크 제약조건과 name 필드 추가가 적절합니다.
nickname에unique = true제약조건을 추가하여 DB 레벨에서 중복을 방지하고,name필드를 분리하여 실명과 닉네임을 구분합니다.
61-62: LGTM! birthDate 필드 추가가 적절합니다.생년월일을
LocalDate타입으로 저장하는 것은 날짜 처리에 적합한 선택입니다.
94-100: LGTM! 대학교/전공 변경 메서드가 적절하게 추가되었습니다.연관 엔티티 변경을 위한 메서드가 명확하게 정의되어 있습니다.
src/main/java/org/sopt/certi_server/domain/favorite/repository/FavoriteRepository.java (1)
32-33: LGTM! countByUser 메서드가 일관된 패턴으로 추가되었습니다.
AcquisitionRepository,ActivityRepository와 동일한 시그니처를 사용하여 코드베이스 전체의 일관성을 유지하고 있습니다.build.gradle (1)
80-81: LGTM! 아호코라식 의존성 추가가 적절합니다.비속어 필터링을 위한 효율적인 다중 패턴 매칭 알고리즘 구현에 적합한 라이브러리이며, 0.6.3은 Maven Central에서 게시된 최신 버전입니다.
src/main/java/org/sopt/certi_server/domain/user/service/KakaoService.java (1)
57-57: 로깅 변경 승인카카오 프로필의
nickname→name필드명 변경에 맞춰 로깅 코드가 올바르게 업데이트되었습니다. 이는KakaoUserInformationResponse의 DTO 리팩토링과 일치합니다.src/main/java/org/sopt/certi_server/domain/user/service/AuthService.java (1)
129-130: OAuth 실명과 사용자 닉네임 분리 구현 승인User 엔티티가
name(line 45-46)과nickname(line 42-43) 필드를 모두 지원하며, 두 필드 모두@Column어노테이션으로 적절히 정의되어 있습니다. Builder 생성자(line 72-85)도 두 파라미터를 모두 수용하고,changeUser()메서드(line 87-92)에서도 둘 다 업데이트됩니다.AuthService의
convertDtoToEntity()메서드에서 OAuth 제공자의 실명(request.userInformation().name())과 사용자가 선택한 닉네임(request.nickname())을 명확하게 분리하여 설정하는 것은 올바른 구현입니다. 또한PersonalInformationResponse와SignUpResponse등 응답 DTO에서 두 필드가 일관되게 사용되고 있으며,UserService에서 닉네임 중복 검증(line 160)도 제대로 처리되고 있습니다.src/main/java/org/sopt/certi_server/global/filter/JwtAuthenticationFilter.java (1)
37-38: 인증 제외 목록에 validation 엔드포인트가 추가되었습니다.닉네임 검증 API를 JWT 인증 없이 호출할 수 있도록 설정한 것은 적절해 보입니다. 다만 해당 엔드포인트가 악용되지 않도록 rate limiting이나 입력 검증이 구현되어 있는지 확인하세요.
src/main/java/org/sopt/certi_server/domain/user/dto/response/GetUserResponse.java (1)
9-9: 필드명 변경이 올바르게 적용되었습니다.
name에서nickname으로의 필드명 변경이from메서드와 일관성 있게 적용되었습니다. User 엔티티의getNickname()메서드 호출도 정확합니다.Also applies to: 16-16
src/main/java/org/sopt/certi_server/domain/userprecertification/entity/UserPreCertification.java (1)
47-48: Location 클래스가@Embeddable애노테이션으로 올바르게 정의되어 있습니다.@Embedded로 사용되는 것이 적절합니다.src/main/java/org/sopt/certi_server/domain/user/repository/UserRepository.java (1)
16-16: 닉네임 중복 확인 메서드가 올바르게 구현되었습니다.Spring Data JPA 명명 규칙을 따른 정확한 구현입니다. nickname 필드에
unique = true속성이 설정되어 있어 데이터베이스에서 자동으로 유니크 인덱스가 생성되므로,existsByNickname()쿼리의 성능이 이미 최적화되어 있습니다.src/main/java/org/sopt/certi_server/domain/userprecertification/repository/UserPreCertificationRepository.java (1)
45-45: LGTM!마이페이지 기능을 위한 사용자별 취득 예정 개수 조회 메서드가 적절하게 추가되었습니다. Spring Data JPA의 명명 규칙을 따르고 있어 자동으로 쿼리가 생성됩니다.
src/main/java/org/sopt/certi_server/domain/user/dto/response/UserInformation.java (1)
6-19: LGTM!
KakaoUserInformationResponse의 변경사항과 일관되게 필드명이nickname에서name으로 변경되었고, 매핑 로직도 올바르게 업데이트되었습니다.src/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUserRequest.java (1)
13-24: LGTM!개인정보 수정을 위한 DTO가 적절한 검증 어노테이션과 함께 잘 구성되었습니다:
- 비속어 검증 (
@NoProfanity)- 이메일 형식 검증 (
- 닉네임 공백 방지 (
@NotEmpty)- 생년월일 과거 날짜 검증 (
@Past)모든 검증 메시지도 한국어로 일관되게 작성되었습니다.
src/main/java/org/sopt/certi_server/domain/user/dto/response/kakao/KakaoUserInformationResponse.java (1)
20-23: 필드명 변경이 모든 매핑 로직에 일관되게 적용되었습니다.
nickname에서name으로의 필드명 변경이OAuthUserInformation.from()과UserInformation.from()매퍼, 그리고KakaoService에서 모두 올바르게 적용되었습니다. 기존.nickname()호출이 남아있지 않으므로 추가 수정이 필요하지 않습니다.src/main/java/org/sopt/certi_server/global/error/exception/InvalidNicknameException.java (1)
5-8: LGTM! 코드베이스의 기존 예외 패턴과 일관성 있게 구현되었습니다.InvalidNicknameException은 다른 예외 클래스들(NotFoundException, ForbiddenException 등)과 동일한 패턴을 따르고 있어 일관성이 유지됩니다.
src/main/java/org/sopt/certi_server/domain/user/dto/response/PersonalInformationResponse.java (1)
8-24: LGTM! Response DTO가 올바르게 구현되었습니다.Record 구조와 factory 메서드가 적절하며, 코드베이스의 다른 Response DTO들(AuthResponse, SignUpResponse 등)과 일관된 패턴을 따릅니다. User 엔티티의 모든 필드가 올바르게 매핑되었습니다.
src/main/java/org/sopt/certi_server/domain/acquisition/dto/response/GetAcquisitionDetailResponse.java (1)
23-23: API 응답 필드명 변경에 대한 추가 검증이 필요합니다.현재 코드를 확인한 결과,
acquisitionDate필드가GetAcquisitionDetailResponse에서 일관되게 사용되고 있으며acquisition.getAcquisitionDate()로 제대로 매핑되어 있습니다. 그러나 다음 항목들을 확인하시기 바랍니다:
- API 문서가
acquisitionDate필드로 업데이트되었는지 확인 (현재 공식 API 문서를 찾을 수 없음)- 이 변경이 실제로 API의 breaking change인지, 그리고 클라이언트에 전달되었는지 확인
- 필요시 마이그레이션 전략 또는 versioning 전략 검토
src/main/java/org/sopt/certi_server/global/valid/ProfanityFilter.java (1)
26-30: 스레드 안전성 검증 완료 - 추가 조치 불필요합니다.Ahocorasick 0.6.3 라이브러리의
Trie는 빌드 완료 후 동시 읽기 작업에 대해 스레드 안전합니다. 현재 코드는 이를 올바르게 구현하고 있습니다:
@PostConstruct초기화에서 Trie를 단 한 번 빌드@Component싱글톤으로 인스턴스 공유containsMatch()읽기 전용 연산만 동시 호출이는 Ahocorasick 공식 가이드에서 권장하는 패턴과 동일합니다.
라인 28의 정규화 로직
[^가-힣a-zA-Z0-9]은 특수문자를 제거하여 우회 시도(예: f-uck)를 방지하는 의도된 설계입니다.Likely an incorrect or invalid review comment.
src/test/java/org/sopt/certi_server/domain/user/service/UserServiceTest.java (1)
111-122: 마이페이지 조회 테스트 구현 확인마이페이지 홈 API에 대한 테스트가 주요 필드들(
nickname,jobResponse, 각종 카운트)을 검증하고 있습니다.src/main/java/org/sopt/certi_server/domain/userprecertification/service/UserPreCertificationService.java (2)
85-102: 적절한 권한 검증이 구현되었습니다.
patchPreCertification메서드가 사용자 소유권을 검증하고NotFoundException과ForbiddenException을 적절히 사용하고 있습니다. 다만 Line 93의 참조 비교 이슈를 수정해야 합니다.
43-72: Request 객체 기반 리팩터링이 잘 되었습니다.
CreateUserPreCertificationRequest를 사용하도록 메서드를 리팩터링하여city,state,testDate등의 추가 정보를 받을 수 있게 된 점이 좋습니다. Location 객체 생성과 함께 깔끔하게 구현되었으며, 모든 필드에@NotNull,@NotBlank등의 검증 어노테이션이 적절하게 적용되어 있어 입력값 검증도 철저합니다.src/main/java/org/sopt/certi_server/domain/acquisition/dto/response/GetAcquisitionResponse.java (1)
13-35: Response DTO 구조 변경이 적절합니다.새로운 필드들(
acquisitionDate,grade,description)이 추가되고from()메서드가 올바르게 업데이트되었습니다.@JsonFormat어노테이션으로 날짜 형식도 명확히 지정되어 있습니다.src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java (3)
37-45: 마이페이지 홈 API가 잘 구현되었습니다.엔드포인트가 명확하고 적절한 응답 타입을 반환합니다.
85-92: 닉네임 검증 엔드포인트가 적절히 구현되었습니다.JWT 필터 제외 목록에 포함되어 인증 없이 호출 가능한 점이 확인됩니다 (relevant code snippets 참조).
47-64: 개인정보 조회/수정 API가 잘 구현되었습니다.조회와 수정 엔드포인트가 RESTful하게 설계되었습니다.
UpdateUserRequest는 적절한 검증 어노테이션으로 잘 구성되어 있습니다.name과nickName에@NoProfanity, 이메일에@NotEmpty, 생년월일에@Past등 각 필드에 맞는 검증이 적용되어 있으며, 검증에 대한 테스트 커버리지도 있습니다.src/main/java/org/sopt/certi_server/domain/userprecertification/controller/UserPreCertificationController.java (2)
35-44: Request body 기반 API로 개선되었습니다.
CreateUserPreCertificationRequest를 사용하여 추가 정보(city, state, testDate)를 받을 수 있게 되었습니다. API 설계가 더 명확해졌습니다.
46-55: 취득 예정 수정 API가 잘 구현되었습니다.
PatchPreCertificationRequest를 통해 Location과 testDate를 업데이트할 수 있으며, 사용자 인증도 적절히 처리됩니다.src/main/java/org/sopt/certi_server/domain/user/service/UserService.java (5)
120-136: LGTM!여러 리포지토리에서 데이터를 집계하여 마이페이지 정보를 구성하는 로직이 적절합니다. 다만, Lines 121-122의 불필요한 빈 줄은 제거하는 것이 좋습니다.
138-141: LGTM!간단하고 명확한 구현입니다.
143-155: LGTM!닉네임 검증 후 사용자 정보 업데이트가 트랜잭션 내에서 적절하게 처리됩니다.
194-202: LGTM!대학교 변경 로직이 적절하게 구현되었습니다.
204-212: LGTM!학과 변경 로직이 적절하게 구현되었습니다.
src/main/java/org/sopt/certi_server/domain/acquisition/controller/AcquisitionController.java (3)
40-48: LGTM!명시적인 path value 추가가 적절합니다.
50-59: LGTM!취득 자격증 수정 API가 RESTful 패턴에 맞게 PATCH 메서드로 적절히 구현되었습니다.
72-81: LGTM!리스트 조회 API의 응답 타입 변경이 적절합니다.
src/main/java/org/sopt/certi_server/domain/acquisition/controller/AcquisitionController.java
Show resolved
Hide resolved
...main/java/org/sopt/certi_server/domain/acquisition/dto/request/CreateAcquisitionRequest.java
Outdated
Show resolved
Hide resolved
| public Acquisition(Certification certification, CardType cardType, User user, SmallCardType smallCardType, String grade) { | ||
| this.certification = certification; | ||
| this.smallCardType = smallCardType; | ||
| this.cardType = cardType; | ||
| this.user = user; | ||
| this.acquisitionDate = LocalDate.now(); | ||
| } |
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
생성자 설계의 일관성을 개선하세요.
생성자가 grade 파라미터는 받지만 acquisitionDate는 받지 않고 LocalDate.now()로 하드코딩하고 있습니다. 이는 다음과 같은 문제를 야기할 수 있습니다:
- 과거 날짜로 취득 정보를 생성해야 하는 경우 대응이 불가능합니다
changeAcquisition메서드에서는acquisitionDate를 변경할 수 있지만, 생성 시점에는 지정할 수 없어 일관성이 떨어집니다
🔎 생성자에 acquisitionDate 파라미터 추가
@Builder
-public Acquisition(Certification certification, CardType cardType, User user, SmallCardType smallCardType, String grade) {
+public Acquisition(Certification certification, CardType cardType, User user, SmallCardType smallCardType, String grade, LocalDate acquisitionDate) {
this.certification = certification;
this.smallCardType = smallCardType;
this.cardType = cardType;
this.user = user;
- this.acquisitionDate = LocalDate.now();
+ this.grade = grade;
+ this.acquisitionDate = acquisitionDate != null ? acquisitionDate : LocalDate.now();
}🤖 Prompt for AI Agents
In
@src/main/java/org/sopt/certi_server/domain/acquisition/entity/Acquisition.java
around lines 56-62, The Acquisition constructor currently accepts a grade but
always sets acquisitionDate = LocalDate.now(), preventing creation of
acquisitions with specific dates and causing inconsistency with
changeAcquisition; modify the constructor (public Acquisition(Certification
certification, CardType cardType, User user, SmallCardType smallCardType, String
grade)) to accept a LocalDate acquisitionDate parameter and assign it to the
acquisitionDate field, or add an overloaded constructor that takes
acquisitionDate while keeping the existing one delegating to the new constructor
with LocalDate.now() so callers can opt to supply an explicit date.
| @NotEmpty(message = "학년 정보는 필수입니다.") String grade, | ||
| @NotEmpty(message = "계열 정보는 필수입니다.") String track, | ||
| @NotEmpty(message = "전공 정보는 필수입니다.") String major, | ||
| @NotEmpty(message = "닉네임 정보는 필수입니다.") @Size(min = 1, max = 7, message = "닉네임은 최소 1자, 최대 7자까지 입력 해야 합니다.") @NoProfanity String nickname, |
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:
# First, examine the full SignupRequest.java file to understand the field definitions
cat -n src/main/java/org/sopt/certi_server/domain/user/dto/request/SignupRequest.javaRepository: cerdeuk/CERTI-BE
Length of output: 1189
🏁 Script executed:
# Search for all SignupRequest usages in the codebase
rg "SignupRequest" --type=java -n -B 2 -A 2Repository: cerdeuk/CERTI-BE
Length of output: 4124
🏁 Script executed:
# Look for test cases related to signup
fd -e "java" | xargs grep -l "SignupRequest\|signup" | head -20Repository: cerdeuk/CERTI-BE
Length of output: 368
🏁 Script executed:
# Search for test files related to signup
find . -type f -name "*Test*.java" | xargs grep -l "signup\|Signup\|SignupRequest" 2>/dev/null | head -20Repository: cerdeuk/CERTI-BE
Length of output: 42
🏁 Script executed:
# Check the convertDtoToEntity method to see how nickname is handled
sed -n '122,150p' src/main/java/org/sopt/certi_server/domain/user/service/AuthService.javaRepository: cerdeuk/CERTI-BE
Length of output: 1190
🏁 Script executed:
# Search for any old signup endpoints or alternative signup methods
rg "signup|register" --type=java -i -l | head -20Repository: cerdeuk/CERTI-BE
Length of output: 1183
회원가입 API의 새로운 필수 필드 추가로 인한 클라이언트 호환성 확인 필요
nickname 필드의 검증 로직은 적절합니다 (@NotEmpty, @Size(min=1, max=7), @NoProfanity). 백엔드 구현도 올바르게 되어 있습니다 (필드 유효성 검사, 서비스 계층에서 User 엔티티로 정상 변환).
그러나 nickname 필드가 필수 필드(@NotEmpty)로 추가되었으므로, 이 변경사항을 반영하지 않은 클라이언트는 회원가입 실패로 인한 오류를 받게 됩니다. 배포 전에 다음을 반드시 확인하세요:
- 모든 클라이언트(웹, 모바일 등)에서 회원가입 요청 시 nickname 필드를 포함하도록 업데이트되었는지 확인
- 레거시 클라이언트 버전의 호환성 전략 수립 (필요 시 API 버전 관리 또는 단계적 배포 계획)
🤖 Prompt for AI Agents
In
@src/main/java/org/sopt/certi_server/domain/user/dto/request/SignupRequest.java
around line 17, SignupRequest's nickname field was made required via the
@NotEmpty/@Size/@NoProfanity annotations which will break clients that don't
send it; either ensure all clients (web/mobile/legacy) are updated to include
nickname in signup payloads or add a backward-compatibility change server-side
(e.g., relax @NotEmpty on SignupRequest.nickname, accept null in the service
layer and assign a default or return a clear validation error with migration
guidance, or introduce a new API version for required-nickname). Update client
teams and/or implement one of these server-side strategies referencing the
SignupRequest class and its nickname field to restore compatibility before
deploying.
| private void validateKeyword(String nickname) { | ||
| // 공백 검사 | ||
| if (nickname.isEmpty() || nickname.isBlank()){ | ||
| throw new InvalidNicknameException(ErrorCode.NICKNAME_EMPTY); | ||
| } | ||
|
|
||
| // 길이 검사 | ||
| if(nickname.length() > 7){ | ||
| throw new InvalidNicknameException(ErrorCode.NICKNAME_TOO_LONG); | ||
| } | ||
|
|
||
| // 욕설 검사 | ||
| if (profanityFilter.containsProfanity(nickname)){ | ||
| throw new InvalidNicknameException(ErrorCode.NICKNAME_CONTAINS_PROFANITY); | ||
| } | ||
| } |
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.
Null 안전성 및 중복 검사 제거 필요
두 가지 개선 사항이 있습니다:
- NPE 위험:
nickname이null인 경우 Line 178에서NullPointerException이 발생합니다. - 중복 조건:
isBlank()는 빈 문자열에 대해서도true를 반환하므로isEmpty()검사는 불필요합니다.
🔎 제안된 수정
private void validateKeyword(String nickname) {
// 공백 검사
- if (nickname.isEmpty() || nickname.isBlank()){
+ if (nickname == null || nickname.isBlank()){
throw new InvalidNicknameException(ErrorCode.NICKNAME_EMPTY);
}🤖 Prompt for AI Agents
In @src/main/java/org/sopt/certi_server/domain/user/service/UserService.java
around lines 176-191, In validateKeyword, avoid NPE and remove redundant
isEmpty(): first check for null or blank (e.g., if (nickname == null ||
nickname.isBlank()) throw InvalidNicknameException(ErrorCode.NICKNAME_EMPTY));
then keep the length check (nickname.length() > 7) and the profanity check using
profanityFilter.containsProfanity(nickname); this ensures null-safety and
eliminates the duplicate isEmpty() condition while preserving existing error
codes and behavior.
| () -> new NotFoundException(ErrorCode.PRECERTIFICATION_NOT_FOUND) | ||
| ); | ||
|
|
||
| if(upc.getUser() != user) throw new ForbiddenException(ErrorCode.ACCESS_DENIED); |
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 엔티티를 다르다고 판단할 수 있습니다.
🔎 ID 기반 비교로 수정
-if(upc.getUser() != user) throw new ForbiddenException(ErrorCode.ACCESS_DENIED);
+if(!upc.getUser().getId().equals(user.getId())) throw new ForbiddenException(ErrorCode.ACCESS_DENIED);🤖 Prompt for AI Agents
In
@src/main/java/org/sopt/certi_server/domain/userprecertification/service/UserPreCertificationService.java
around line 93, The equality check uses reference comparison (upc.getUser() !=
user) which can fail for different entity instances; replace it with a null-safe
ID-based comparison or equals-based check — e.g., compare upc.getUser().getId()
to user.getId() (or use !Objects.equals(upc.getUser(), user) only if equals is
properly implemented) and throw the same
ForbiddenException(ErrorCode.ACCESS_DENIED) when IDs differ; ensure you guard
against null user or null id to avoid NPEs.
| "/actuator/**", | ||
| "/api/v1/user/validation" |
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:
cat -n src/main/java/org/sopt/certi_server/global/config/SecurityConfig.java | head -60Repository: cerdeuk/CERTI-BE
Length of output: 2739
🏁 Script executed:
# Search for rate limiting implementations
rg -n "RateLimiter|@RateLimit|Bucket4j|throttle|Resilience4j" --type=java -iRepository: cerdeuk/CERTI-BE
Length of output: 42
🏁 Script executed:
# Search for the endpoint implementation
rg -n "/api/v1/user/validation" --type=javaRepository: cerdeuk/CERTI-BE
Length of output: 290
🏁 Script executed:
# Search for interceptor or filter configurations
rg -n "addInterceptors|Filter|Interceptor" --type=javaRepository: cerdeuk/CERTI-BE
Length of output: 4108
🏁 Script executed:
# Check for any Spring Cloud related rate limiting (Spring Cloud Gateway, etc.)
fd -e yml -e yaml | xargs rg -l "rate.limit|throttle|resilience4j" 2>/dev/null || echo "No config files found with rate limiting"Repository: cerdeuk/CERTI-BE
Length of output: 100
🏁 Script executed:
# Find the user validation endpoint implementation
rg -n "user/validation" --type=java -A 10 -B 2Repository: cerdeuk/CERTI-BE
Length of output: 2954
🏁 Script executed:
# Find UserController or similar
fd -e java | xargs rg -l "RequestMapping.*user|RestController.*user" | head -5Repository: cerdeuk/CERTI-BE
Length of output: 138
🏁 Script executed:
# Check for any rate limiting or request throttling annotations
rg -n "@RequestMapping|@GetMapping|@PostMapping" --type=java -A 5 | grep -A 5 "validation"Repository: cerdeuk/CERTI-BE
Length of output: 860
🏁 Script executed:
# Get the full UserController implementation for the validation endpoint
cat -n src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java | sed -n '80,100p'Repository: cerdeuk/CERTI-BE
Length of output: 1112
🏁 Script executed:
# Check UserService.validateNickname method to see what it does
rg -n "validateNickname" --type=java -A 15Repository: cerdeuk/CERTI-BE
Length of output: 14154
🏁 Script executed:
# Check if there are any input validation annotations or constraints
rg -n "@Valid|@Size|@Length|@Pattern" --type=java | grep -i userRepository: cerdeuk/CERTI-BE
Length of output: 1656
인증 없는 닉네임 검증 엔드포인트에 Rate Limiting 및 접근 제어 구현 필요
닉네임 검증 엔드포인트(/api/v1/user/validation)가 인증 없이 완전히 개방되어 있으며, 현재 다음과 같은 보안 위험이 존재합니다:
- 닉네임 열거 공격: 엔드포인트가 닉네임 존재 여부에 따라 서로 다른 예외를 반환하므로(NICKNAME_DUPLICATE vs 검증 오류), 공격자가 기존 닉네임을 체계적으로 수집할 수 있습니다.
- 서비스 남용: Rate limiting이 구현되지 않아 무제한 대량 요청으로 인한 서비스 거부 공격이 가능합니다.
- 입력 검증 부재: 컨트롤러 레벨에서
@Valid,@Size등의 검증 애너테이션이 없어 서비스 로직에만 의존하고 있습니다.
Spring의 Bucket4j, Resilience4j RateLimiter, 또는 커스텀 인터셉터를 사용하여 엔드포인트별 요청 제한을 구현하고, 닉네임 존재 여부 응답을 표준화(예: "검증 결과를 반환하되 구체적 사유는 숨김")하여 열거 공격을 완화해야 합니다.
src/main/java/org/sopt/certi_server/global/error/code/ErrorCode.java
Outdated
Show resolved
Hide resolved
| @PostConstruct | ||
| public void init(){ | ||
| List<String> badWords = List.of("병신", "시발", "개새끼", "fuck"); | ||
|
|
||
| this.trie = Trie.builder() | ||
| .addKeywords(badWords) | ||
| .ignoreCase() | ||
| .ignoreOverlaps() | ||
| .build(); | ||
| } |
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
비속어 목록을 외부 설정으로 분리하세요.
비속어 목록이 코드에 하드코딩되어 있어 유지보수가 어렵습니다. 새로운 비속어를 추가하거나 제거할 때마다 코드를 수정하고 재배포해야 합니다. 외부 설정 파일(예: YAML, properties) 또는 데이터베이스로 분리하는 것을 권장합니다.
🔎 제안하는 개선 방안
방안 1: application.yml에서 비속어 목록 로드
+import org.springframework.beans.factory.annotation.Value;
+
@Component
public class ProfanityFilter {
private Trie trie;
+
+ @Value("${profanity.words}")
+ private List<String> badWords;
@PostConstruct
public void init(){
- List<String> badWords = List.of("병신", "시발", "개새끼", "fuck");
-
this.trie = Trie.builder()
.addKeywords(badWords)
.ignoreCase()
.ignoreOverlaps()
.build();
}그리고 application.yml에 추가:
profanity:
words:
- 병신
- 시발
- 개새끼
- fuck방안 2: 별도의 텍스트 파일에서 로드
resources/profanity-words.txt 파일을 생성하고 초기화 시 읽어오는 방식도 고려할 수 있습니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @src/main/java/org/sopt/certi_server/global/valid/ProfanityFilter.java around
lines 15-24, The ProfanityFilter currently hardcodes badWords inside the init()
method; move that list to external configuration and load it at startup so
additions don't require code changes: add a configuration property (e.g., a
List<String> profanity.words via @ConfigurationProperties or @Value or read from
resources/profanity-words.txt), inject that List into the ProfanityFilter class,
and change init() to build the Trie using the injected List<String> (refer to
ProfanityFilter.init() and the trie field) instead of List.of(...); ensure
null/empty checks and appropriate logging if the configured list is missing.
| @Override | ||
| public boolean isValid(String value, ConstraintValidatorContext context) { | ||
| return !filter.containsProfanity(value); | ||
| } |
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.
null 값 처리를 추가해야 합니다.
현재 구현은 value가 null일 경우 ProfanityFilter.containsProfanity()에서 NullPointerException이 발생할 수 있습니다. ProfanityFilter의 containsProfanity 메서드는 내부적으로 keyword.replaceAll()을 호출하는데, null 값에 대한 처리가 없습니다.
일반적인 validator 관례에 따라 null 값은 유효한 것으로 간주하고, null 체크가 필요한 경우 별도로 @NotNull 어노테이션을 사용합니다.
🔎 제안하는 수정사항
@Override
public boolean isValid(String value, ConstraintValidatorContext context) {
+ if (value == null) {
+ return true;
+ }
return !filter.containsProfanity(value);
}📝 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.
| @Override | |
| public boolean isValid(String value, ConstraintValidatorContext context) { | |
| return !filter.containsProfanity(value); | |
| } | |
| @Override | |
| public boolean isValid(String value, ConstraintValidatorContext context) { | |
| if (value == null) { | |
| return true; | |
| } | |
| return !filter.containsProfanity(value); | |
| } |
🤖 Prompt for AI Agents
In @src/main/java/org/sopt/certi_server/global/valid/ProfanityValidator.java
around lines 19-22, The isValid method in ProfanityValidator should treat null
as valid to avoid NPE from ProfanityFilter.containsProfanity; modify
ProfanityValidator.isValid to first check if value is null and return true in
that case, otherwise call filter.containsProfanity(value) and return the
negation; rely on @NotNull for requiring non-null values elsewhere.
Fix/#171 카테고리 API 수정
- 욕설, 비속어 검증 아호코라식 알고리즘 사용 - 검증 테스트 추가
- SignupRequest에 nickname 필드 추가 - 기존 UserInformation 필드의 nickname을 name으로 변경 - 닉네임 검증 API 추가 및 테스트
- 취득, 취득 예정 정보 수정 API 추가 - 취득, 취득 예정 생성 API 수정
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/userprecertification/dto/request/PatchPreCertificationRequest.java (1)
9-17: 이전 파싱 오류가 수정되었습니다.이전 리뷰에서 지적된
LocalDateTime과 날짜 전용 패턴 불일치 문제가 해결되었습니다. 현재 패턴"yyyy.MM.dd HH:mm:ss"는 시간 정보를 포함하므로LocalDateTime과 호환됩니다.다만, 시험 일정에 시간 정보가 실제로 필요한지 확인해보세요. 만약 날짜만 의미가 있고 시간은 불필요하다면,
LocalDate를 사용하는 것이 API 사용자에게 더 간단하고 직관적입니다.🔎 시간 정보가 불필요한 경우 권장하는 수정
-import java.time.LocalDateTime; +import java.time.LocalDate; public record PatchPreCertificationRequest( @NotNull(message = "날짜 정보를 입력해주세요") - @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy.MM.dd HH:mm:ss", timezone = "Asia/Seoul") - LocalDateTime testDate, + @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy.MM.dd", timezone = "Asia/Seoul") + LocalDate testDate, @NotBlank(message = "시험 장소(시/도)를 입력해주세요") String city, @NotBlank(message = "시험 장소(시/군/구)를 입력해주세요") String state ) { }src/main/java/org/sopt/certi_server/domain/userprecertification/dto/request/CreateUserPreCertificationRequest.java (1)
9-17: 이전 파싱 오류가 수정되었으며, DTO 간 일관성이 유지되고 있습니다.
PatchPreCertificationRequest와 동일하게 파싱 오류가 수정되었습니다. 현재 패턴"yyyy.MM.dd HH:mm:ss"는LocalDateTime과 호환되며, 두 DTO 간 일관성이 잘 유지되고 있습니다.시험 일정에 시간 정보가 실제로 필요한지 확인하시고, 불필요하다면
PatchPreCertificationRequest와 함께LocalDate로 변경하는 것을 고려해보세요. 생성과 수정 API에서 동일한 타입을 사용하는 것이 중요합니다.🔎 시간 정보가 불필요한 경우 권장하는 수정
-import java.time.LocalDateTime; +import java.time.LocalDate; public record CreateUserPreCertificationRequest( @NotNull(message = "자격증 id 정보는 필수입니다.") Long certificationId, @NotBlank(message = "시험 장소(시/도)를 입력해주세요") String city, @NotBlank(message = "시험 장소(시/군/구)를 입력해주세요") String state, @NotNull(message = "시험 일정을 입력해주세요") - @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy.MM.dd HH:mm:ss", timezone = "Asia/Seoul") - LocalDateTime testDate + @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy.MM.dd", timezone = "Asia/Seoul") + LocalDate testDate ) { }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/org/sopt/certi_server/domain/userprecertification/dto/request/CreateUserPreCertificationRequest.javasrc/main/java/org/sopt/certi_server/domain/userprecertification/dto/request/PatchPreCertificationRequest.java
- 욕설, 비속어 검증 아호코라식 알고리즘 사용 - 검증 테스트 추가
- SignupRequest에 nickname 필드 추가 - 기존 UserInformation 필드의 nickname을 name으로 변경 - 닉네임 검증 API 추가 및 테스트
- 사용자 정보 변경에서 사용하는 닉네임 검증 메서드와 단순 닉네임 검증 메서드 분리
# Conflicts: # src/main/java/org/sopt/certi_server/domain/favorite/repository/FavoriteRepository.java # src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java # src/main/java/org/sopt/certi_server/domain/user/entity/User.java # src/main/java/org/sopt/certi_server/domain/user/service/UserService.java # src/main/java/org/sopt/certi_server/domain/userprecertification/entity/UserPreCertification.java # src/test/java/org/sopt/certi_server/domain/user/service/UserServiceTest.java
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 (4)
src/main/java/org/sopt/certi_server/domain/certification/dto/response/CertificationScoreDto.java (1)
33-44:toString()메서드에description필드가 누락되었습니다.새로 추가된
description필드가toString()메서드에 포함되어 있지 않습니다. 디버깅 및 로깅 시 일관성을 위해 추가하는 것이 좋습니다.🔎 수정 제안
@Override public String toString() { return "CertificationScoreDto{" + "certificationId=" + certificationId + ", certificationName='" + certificationName + '\'' + ", certificationType='" + certificationType + '\'' + ", testType='" + testType + '\'' + ", tags=" + tags + ", recommendationScore=" + recommendationScore + ", isFavorite=" + isFavorite + + ", description='" + description + '\'' + '}'; }src/main/java/org/sopt/certi_server/domain/certification/service/CertificationService.java (1)
205-224: 사용하지 않는 변수user가 있습니다.Line 206에서
user변수를 가져오지만 이후에 사용하지 않습니다.getUserJob(userId)가 내부적으로 이미 사용자를 조회하므로 중복됩니다.🔎 수정 제안
public List<CertificationRankResponse> getCertificationJob(final Long userId){ - User user = userService.getUser(userId); List<String> jobList = userService.getUserJob(userId).jobList(); if (jobList.isEmpty()) { throw new NotFoundException(ErrorCode.JOB_NOT_FOUND); }src/main/java/org/sopt/certi_server/domain/certification/controller/CertificationController.java (1)
85-102: 반환 타입에 와일드카드 대신 구체적인 타입 사용을 권장합니다.
SuccessResponse<?>대신SuccessResponse<List<CertificationRankResponse>>를 사용하면 Swagger 문서화가 개선되고 타입 안전성이 향상됩니다. 다른 엔드포인트들(예:getCertification,recommendCertification)도 구체적인 타입을 사용하고 있습니다.🔎 수정 제안
@GetMapping("/job") @Operation(summary = "직무별 자격증 조회 API", description = "3순위 직무별 자격증을 조회합니다") - public ResponseEntity<SuccessResponse<?>> getTop3ByJob( + public ResponseEntity<SuccessResponse<List<CertificationRankResponse>>> getTop3ByJob( @AuthenticationPrincipal Long userId ){ List<CertificationRankResponse> certificationRankResponseList = certificationService.getCertificationJob(userId); return ResponseEntity.ok(SuccessResponse.of(SuccessCode.SUCCESS_FETCH, certificationRankResponseList)); } @GetMapping("/track") @Operation(summary = "계열별 자격증 조회 API", description = "3순위 계열별 자격증을 조회합니다") - public ResponseEntity<SuccessResponse<?>> getTop3ByTrack( + public ResponseEntity<SuccessResponse<List<CertificationRankResponse>>> getTop3ByTrack( @AuthenticationPrincipal Long userId ) { List<CertificationRankResponse> certificationRankResponseList = certificationService.getCertificationTrack(userId); return ResponseEntity.ok(SuccessResponse.of(SuccessCode.SUCCESS_FETCH, certificationRankResponseList)); - }src/main/java/org/sopt/certi_server/domain/favorite/repository/FavoriteRepository.java (1)
38-50:select distinct와group by가 중복되어 있습니다.
GROUP BY c를 사용하면 이미 각 Certification별로 그룹화되므로select distinct c는 불필요합니다. 일부 JPA 구현체에서 예상치 못한 동작을 유발할 수 있습니다.🔎 수정 제안
@Query(""" - select distinct c + select c from Favorite f join f.certification c join CertificationJob cj on cj.certification = c where cj.job.id = :jobId group by c order by count(f) desc, max(cj.weight) desc """) List<Certification> findTopByJobOrderByFavoriteCount( @Param("jobId") Long jobId, Pageable pageable );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/org/sopt/certi_server/domain/certification/controller/CertificationController.javasrc/main/java/org/sopt/certi_server/domain/certification/dto/response/CertificationRankResponse.javasrc/main/java/org/sopt/certi_server/domain/certification/dto/response/CertificationScoreDto.javasrc/main/java/org/sopt/certi_server/domain/certification/dto/response/CertificationSimple.javasrc/main/java/org/sopt/certi_server/domain/certification/service/CertificationService.javasrc/main/java/org/sopt/certi_server/domain/favorite/repository/FavoriteRepository.javasrc/main/java/org/sopt/certi_server/domain/user/controller/UserController.javasrc/main/java/org/sopt/certi_server/domain/userprecertification/entity/UserPreCertification.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/org/sopt/certi_server/domain/userprecertification/entity/UserPreCertification.java
- src/main/java/org/sopt/certi_server/domain/user/controller/UserController.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). (1)
- GitHub Check: deploy-cd
🔇 Additional comments (4)
src/main/java/org/sopt/certi_server/domain/certification/dto/response/CertificationSimple.java (1)
19-19: LGTM!
description필드 추가가 적절하게 구현되었습니다. 다른 필드들과 동일한 패턴을 따르고 있으며,CertificationScoreDto등 다른 DTO의 변경사항과도 일관성이 있습니다.Also applies to: 33-33
src/main/java/org/sopt/certi_server/domain/certification/dto/response/CertificationRankResponse.java (1)
5-19: LGTM!
CertificationRankResponse레코드가 잘 구현되었습니다.Certification엔티티로부터 직접 생성할 수 있는 편의 생성자가 있고,certificationType에 대한 null 처리도CertificationSimple과 동일한 패턴을 따르고 있습니다.src/main/java/org/sopt/certi_server/domain/certification/service/CertificationService.java (1)
226-239: LGTM!
getCertificationTrack메서드가 적절하게 구현되었습니다. 사용자의 트랙을 가져와 해당 트랙의 상위 자격증을 조회하는 로직이 명확합니다.src/main/java/org/sopt/certi_server/domain/favorite/repository/FavoriteRepository.java (1)
36-36: LGTM!
countByUser메서드와findTopCertificationsByTrack쿼리가 적절하게 구현되었습니다. 트랙별 상위 자격증 조회 쿼리가 명확하고 올바르게 작성되어 있습니다.Also applies to: 52-64
- 단순 plain text 요청 방지
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: 6
🤖 Fix all issues with AI agents
In
@src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java:
- Around line 108-116: Add bean validation to the UpdateMajorRequest DTO (e.g.,
annotate the majorName field with javax.validation constraints such as
@NotBlank/@Size as appropriate) and then enable validation in the controller by
annotating the request parameter in the updateMajor method with @Valid on the
UpdateMajorRequest argument; ensure the method signature remains public
ResponseEntity<SuccessResponse<Void>> updateMajor(@AuthenticationPrincipal Long
userId, @Valid @RequestBody UpdateMajorRequest request) so validation runs
before calling userService.changeMajor(userId, request.majorName()).
- Around line 60-68: The putPersonalInformation endpoint is missing request
validation: annotate the @RequestBody UpdateUserRequest parameter with @Valid so
Spring triggers JSR-303 validation for UpdateUserRequest's constraints; update
the method signature in UserController.putPersonalInformation to add @Valid to
the request parameter and ensure javax/ jakarta.validation is available on the
classpath.
- Around line 98-106: The controller method updateUniversity is missing @Valid
on the @RequestBody, so UpdateUniversityRequest validation won't run; add bean
validation annotations (e.g., @NotBlank/@Size) to the fields/record in
UpdateUniversityRequest and then annotate the controller parameter as @Valid
@RequestBody UpdateUniversityRequest request in updateUniversity, ensuring your
validation provider (javax.validation / spring-boot-starter-validation) is on
the classpath so constraint violations result in a 400 response.
- Around line 89-96: Add request-level validation for the nickname parameter in
UserController.validateNickname: annotate the controller class with @Validated
and change the method parameter to use javax.validation.constraints.@NotBlank
(e.g. @RequestParam("keyword") @NotBlank String nickname) so empty or blank
values are rejected automatically; ensure the @NotBlank import is added and that
any validation exceptions are handled by your global exception handler (or add
an explicit null/blank check and return a 400 Bad Request before calling
userService.validateNickname(nickname) if you prefer manual validation).
In
@src/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateMajorRequest.java:
- Line 3: Add the same not-empty validation used in SignupRequest to the
UpdateMajorRequest record by annotating the majorName component with @NotEmpty
and adding the corresponding import (matching the project's validation package,
e.g., jakarta.validation.constraints.NotEmpty) so the record rejects null/blank
majors; update the record declaration for UpdateMajorRequest to include the
annotation on the majorName component.
In
@src/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUniversityRequest.java:
- Line 3: The UpdateUniversityRequest record's universityName component lacks
validation, allowing null/empty values; add the same @NotEmpty validation used
in SignupRequest to the universityName record component (annotate the record
parameter) and import the corresponding validation annotation (e.g.,
jakarta.validation.constraints.NotEmpty or
javax.validation.constraints.NotEmpty) so the request is validated consistently.
🧹 Nitpick comments (2)
src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java (2)
5-6: 사용하지 않는 import 문을 제거하세요.
@NotEmpty와@Size애노테이션을 import했지만 컨트롤러 코드에서 사용되지 않습니다.♻️ 제안하는 수정사항
-import jakarta.validation.constraints.NotEmpty; -import jakarta.validation.constraints.Size; import lombok.RequiredArgsConstructor;
11-11: 중복된 import 문을 제거하세요.
UpdateUserRequest는 9번 라인의 wildcard import(org.sopt.certi_server.domain.user.dto.request.*)에 이미 포함되어 있습니다.♻️ 제안하는 수정사항
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;
📜 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/UpdateMajorRequest.javasrc/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUniversityRequest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUniversityRequest.java (3)
src/main/java/org/sopt/certi_server/domain/user/dto/request/SignupRequest.java (1)
SignupRequest(10-18)src/main/java/org/sopt/certi_server/domain/user/entity/University.java (1)
University(8-25)src/main/java/org/sopt/certi_server/domain/user/dto/response/GetUniversityListResponse.java (1)
GetUniversityListResponse(5-11)
⏰ 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
| @PutMapping(value = "/pinfo") | ||
| @Operation(summary = "개인정보 수정 API", description = "개인정보를 수정합니다.") | ||
| public ResponseEntity<SuccessResponse<Void>> putPersonalInformation( | ||
| @AuthenticationPrincipal Long userId, | ||
| @RequestBody UpdateUserRequest request | ||
| ){ | ||
| userService.updateUserInformation(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.
🛠️ Refactor suggestion | 🟠 Major
@Valid 애노테이션을 추가하여 요청 검증을 활성화하세요.
@RequestBody에 @Valid 애노테이션이 없으면 UpdateUserRequest의 필드 검증이 실행되지 않습니다. Spring의 자동 검증을 활성화하려면 @Valid를 추가해야 합니다.
✨ 제안하는 수정사항
+import jakarta.validation.Valid;
+
@PutMapping(value = "/pinfo")
@Operation(summary = "개인정보 수정 API", description = "개인정보를 수정합니다.")
public ResponseEntity<SuccessResponse<Void>> putPersonalInformation(
@AuthenticationPrincipal Long userId,
- @RequestBody UpdateUserRequest request
+ @RequestBody @Valid UpdateUserRequest request
){🤖 Prompt for AI Agents
In
@src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java
around lines 60 - 68, The putPersonalInformation endpoint is missing request
validation: annotate the @RequestBody UpdateUserRequest parameter with @Valid so
Spring triggers JSR-303 validation for UpdateUserRequest's constraints; update
the method signature in UserController.putPersonalInformation to add @Valid to
the request parameter and ensure javax/ jakarta.validation is available on the
classpath.
| @GetMapping(value = "/validation") | ||
| @Operation(summary = "닉네임 검증 API", description = "닉네임이 중복이거나 욕설이 포함되어 있는지 검사합니다.") | ||
| public ResponseEntity<SuccessResponse<Void>> validateNickname( | ||
| @RequestParam(value = "keyword") String nickname | ||
| ){ | ||
| userService.validateNickname(nickname); | ||
| return ResponseEntity.ok(SuccessResponse.of(SuccessCode.SUCCESS_FETCH)); | ||
| } |
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
닉네임 파라미터에 유효성 검증을 추가하세요.
닉네임 검증 API임에도 불구하고 nickname 파라미터 자체에 대한 검증이 없습니다. 빈 문자열이나 null이 전달될 경우 서비스 레이어에서 예상치 못한 동작이 발생할 수 있습니다.
✨ 제안하는 수정사항
+import jakarta.validation.constraints.NotBlank;
+import org.springframework.validation.annotation.Validated;
+
@RestController
+@Validated
@RequiredArgsConstructor
@RequestMapping("/api/v1/user")
...
@GetMapping(value = "/validation")
@Operation(summary = "닉네임 검증 API", description = "닉네임이 중복이거나 욕설이 포함되어 있는지 검사합니다.")
public ResponseEntity<SuccessResponse<Void>> validateNickname(
- @RequestParam(value = "keyword") String nickname
+ @RequestParam(value = "keyword") @NotBlank(message = "닉네임은 필수입니다") String nickname
){Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
@src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java
around lines 89 - 96, Add request-level validation for the nickname parameter in
UserController.validateNickname: annotate the controller class with @Validated
and change the method parameter to use javax.validation.constraints.@NotBlank
(e.g. @RequestParam("keyword") @NotBlank String nickname) so empty or blank
values are rejected automatically; ensure the @NotBlank import is added and that
any validation exceptions are handled by your global exception handler (or add
an explicit null/blank check and return a 400 Bad Request before calling
userService.validateNickname(nickname) if you prefer manual validation).
| @PutMapping(value = "/university") | ||
| @Operation(summary = "대학교 변경 API", description = "대학교 정보를 변경합니다.") | ||
| public ResponseEntity<SuccessResponse<Void>> updateUniversity( | ||
| @AuthenticationPrincipal Long userId, | ||
| @RequestBody UpdateUniversityRequest request | ||
| ){ | ||
| userService.changeUniversity(userId, request.universityName()); | ||
| 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
@Valid 애노테이션을 추가하여 요청 검증을 활성화하세요.
@RequestBody에 @Valid 애노테이션이 없으면 UpdateUniversityRequest의 필드 검증이 실행되지 않습니다. 다만, 해당 DTO에 검증 애노테이션이 누락되어 있으므로 먼저 DTO에 검증을 추가한 후 컨트롤러에 @Valid를 적용해야 합니다.
✨ 제안하는 수정사항
+import jakarta.validation.Valid;
+
@PutMapping(value = "/university")
@Operation(summary = "대학교 변경 API", description = "대학교 정보를 변경합니다.")
public ResponseEntity<SuccessResponse<Void>> updateUniversity(
@AuthenticationPrincipal Long userId,
- @RequestBody UpdateUniversityRequest request
+ @RequestBody @Valid UpdateUniversityRequest request
){🤖 Prompt for AI Agents
In
@src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java
around lines 98 - 106, The controller method updateUniversity is missing @Valid
on the @RequestBody, so UpdateUniversityRequest validation won't run; add bean
validation annotations (e.g., @NotBlank/@Size) to the fields/record in
UpdateUniversityRequest and then annotate the controller parameter as @Valid
@RequestBody UpdateUniversityRequest request in updateUniversity, ensuring your
validation provider (javax.validation / spring-boot-starter-validation) is on
the classpath so constraint violations result in a 400 response.
| @PutMapping(value = "/major") | ||
| @Operation(summary = "학과 변경 API", description = "학과 정보를 변경합니다.") | ||
| public ResponseEntity<SuccessResponse<Void>> updateMajor( | ||
| @AuthenticationPrincipal Long userId, | ||
| @RequestBody UpdateMajorRequest request | ||
| ){ | ||
| userService.changeMajor(userId, request.majorName()); | ||
| 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
@Valid 애노테이션을 추가하여 요청 검증을 활성화하세요.
@RequestBody에 @Valid 애노테이션이 없으면 UpdateMajorRequest의 필드 검증이 실행되지 않습니다. 다만, 해당 DTO에 검증 애노테이션이 누락되어 있으므로 먼저 DTO에 검증을 추가한 후 컨트롤러에 @Valid를 적용해야 합니다.
✨ 제안하는 수정사항
+import jakarta.validation.Valid;
+
@PutMapping(value = "/major")
@Operation(summary = "학과 변경 API", description = "학과 정보를 변경합니다.")
public ResponseEntity<SuccessResponse<Void>> updateMajor(
@AuthenticationPrincipal Long userId,
- @RequestBody UpdateMajorRequest request
+ @RequestBody @Valid UpdateMajorRequest request
){Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
@src/main/java/org/sopt/certi_server/domain/user/controller/UserController.java
around lines 108 - 116, Add bean validation to the UpdateMajorRequest DTO (e.g.,
annotate the majorName field with javax.validation constraints such as
@NotBlank/@Size as appropriate) and then enable validation in the controller by
annotating the request parameter in the updateMajor method with @Valid on the
UpdateMajorRequest argument; ensure the method signature remains public
ResponseEntity<SuccessResponse<Void>> updateMajor(@AuthenticationPrincipal Long
userId, @Valid @RequestBody UpdateMajorRequest request) so validation runs
before calling userService.changeMajor(userId, request.majorName()).
| @@ -0,0 +1,4 @@ | |||
| package org.sopt.certi_server.domain.user.dto.request; | |||
|
|
|||
| public record UpdateMajorRequest(String majorName) { | |||
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.
필수 유효성 검증 애노테이션이 누락되었습니다.
majorName 필드에 유효성 검증이 없어 null이나 빈 문자열이 전달될 수 있습니다. SignupRequest에서 major 필드에 @NotEmpty를 사용하는 것과 일관되게 동일한 검증을 추가해야 합니다.
✨ 제안하는 수정사항
+import jakarta.validation.constraints.NotEmpty;
+
-public record UpdateMajorRequest(String majorName) {
+public record UpdateMajorRequest(
+ @NotEmpty(message = "학과 이름은 필수입니다")
+ String majorName
+) {
}🤖 Prompt for AI Agents
In
@src/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateMajorRequest.java
at line 3, Add the same not-empty validation used in SignupRequest to the
UpdateMajorRequest record by annotating the majorName component with @NotEmpty
and adding the corresponding import (matching the project's validation package,
e.g., jakarta.validation.constraints.NotEmpty) so the record rejects null/blank
majors; update the record declaration for UpdateMajorRequest to include the
annotation on the majorName component.
| @@ -0,0 +1,4 @@ | |||
| package org.sopt.certi_server.domain.user.dto.request; | |||
|
|
|||
| public record UpdateUniversityRequest(String universityName) { | |||
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.
필수 유효성 검증 애노테이션이 누락되었습니다.
universityName 필드에 유효성 검증이 없어 null이나 빈 문자열이 전달될 수 있습니다. 관련 코드의 SignupRequest에서는 university 필드에 @NotEmpty를 사용하고 있으므로, 일관성을 위해 동일한 검증을 추가해야 합니다.
✨ 제안하는 수정사항
+import jakarta.validation.constraints.NotEmpty;
+
-public record UpdateUniversityRequest(String universityName) {
+public record UpdateUniversityRequest(
+ @NotEmpty(message = "대학교 이름은 필수입니다")
+ String universityName
+) {
}📝 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 record UpdateUniversityRequest(String universityName) { | |
| import jakarta.validation.constraints.NotEmpty; | |
| public record UpdateUniversityRequest( | |
| @NotEmpty(message = "대학교 이름은 필수입니다") | |
| String universityName | |
| ) { | |
| } |
🤖 Prompt for AI Agents
In
@src/main/java/org/sopt/certi_server/domain/user/dto/request/UpdateUniversityRequest.java
at line 3, The UpdateUniversityRequest record's universityName component lacks
validation, allowing null/empty values; add the same @NotEmpty validation used
in SignupRequest to the universityName record component (annotate the record
parameter) and import the corresponding validation annotation (e.g.,
jakarta.validation.constraints.NotEmpty or
javax.validation.constraints.NotEmpty) so the request is validated consistently.
- 불필요한 임포트 제거 - 잘못된 공백 제거 - 생성자 일관성 유지 - 잘못된 swagger 어노테이션 제거
📣 Related Issue
📝 Summary
🙏 Question & PR point
📬 Postman
Summary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.