[hotfix] 회원가입 / 닉네임 변경 / 투표하기 api 수정#224
Conversation
Walkthrough투표 API 응답이 항목별 리스트(퍼센트, isVoted 포함)로 확장되어 서비스·포트·리포지토리·컨트롤러·테스트가 갱신되었고, 회원가입 요청에서 닉네임 null 검증 및 nickname_updated_at을 null로 저장하도록 변경되었으며 OpenAPI 서버 구성이 프로필 기반으로 동적 설정되도록 변경됨. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as VoteCommandController
participant Service as VoteService
participant CmdPort as VoteCommandPort
participant QryPort as VoteQueryPort
Client->>Controller: POST /votes/{voteId} (vote command)
Controller->>Service: vote(command)
activate Service
Service->>CmdPort: perform vote/cancel and update counts
CmdPort-->>Service: ack
Service->>QryPort: findVoteItemsByVoteId(voteId, userId)
QryPort-->>Service: List<VoteItemQueryDto>
Service->>Service: calculate percentages, build VoteItemDto list
Service-->>Controller: VoteResult(postId, roomId, voteItems)
deactivate Service
Controller-->>Client: VoteResponse(postId, roomId, voteItems)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Test Results384 tests 384 ✅ 31s ⏱️ Results for commit 00803c3. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (11)
src/main/java/konkuk/thip/user/application/service/UserSignupService.java (1)
30-31: 닉네임 변경 가능 시점 반영(null 주입) 자체는 의도에 부합. 다만 downstream NPE 리스크 점검 필요회원가입 직후 닉네임 변경을 허용하기 위해 nicknameUpdatedAt에 null을 주입하는 변경은 PR 목적(#222)에 부합합니다. 다만 다음을 반드시 확인해주세요.
- 닉네임 변경 주기 계산 로직에서
user.getNicknameUpdatedAt()을 바로 연산(plusMonths, between 등)하는 코드가 있으면 NPE가 발생합니다. null일 때 “즉시 변경 가능”으로 안전 처리하는 가드가 필요합니다.- 관련 단위/통합 테스트(회원가입 직후 즉시 변경, 6개월 경과 전/후 케이스)가 존재하는지 확인 바랍니다.
선택 제안: 서비스에서 null을 직접 넘기기보다는, 도메인 팩토리(예:
User.forSignup(...)또는User.withoutId(...).withoutNicknameUpdatedAt()형태)로 의도를 명확히 하면 가독성과 안정성이 좋아집니다.src/main/java/konkuk/thip/config/OpenApiConfig.java (1)
53-64: 프로필 스위치 로직은 목적에 부합. 선택적으로 Spring Profile 기반으로 단순화 가능현재
server.profile스위치로 서버 리스트를 동적으로 구성하는 의도는 명확합니다. 향후 유지보수를 위해 Spring Profile(spring.profiles.active) 기반으로 전환하면 외부 설정과의 일관성이 높아집니다. 예를 들어Environment를 주입받아env.acceptsProfiles("prod")식으로 분기할 수 있습니다. 필요 시 리팩터링 제안 드립니다.src/main/java/konkuk/thip/vote/application/port/in/dto/VoteResult.java (2)
14-18: isVoted는 원시 boolean 사용 권장 (null 삼중 상태 제거)인증된 사용자 컨텍스트(@userid)에서만 사용하는 필드라면 null 허용(Boolean)보다는 원시형(boolean)으로 명확히 하는 편이 안전합니다.
- Boolean isVoted + boolean isVoted @@ - public static VoteItemDto of(Long voteItemId, String itemName, int percentage, Boolean isVoted) { + public static VoteItemDto of(Long voteItemId, String itemName, int percentage, boolean isVoted) { return new VoteItemDto(voteItemId, itemName, percentage, isVoted); }참고: 만약 비로그인 케이스까지 공용으로 쓸 예정이라면(Boolean이 필요한 경우), 해당 의도를 JavaDoc으로 명시해 주세요.
13-13: percentage 정밀도/반올림 규칙 명확화 필요현재 int 타입은 소수점 반올림 규칙을 숨깁니다. 퍼센트 산출 시:
- 0표(total=0)인 경우 0% 처리가 안전한지
- 반올림/내림 기준(예: 소수 1자리 반올림, Bankers rounding 등)
을 명확히 해야 합니다. 서비스 레벨 계산 로직이 이를 보장하는지 확인 부탁드립니다. 필요시 double/BigDecimal(+ RoundingMode)로 전환을 고려해 주세요.src/main/java/konkuk/thip/vote/application/port/out/VoteQueryPort.java (1)
17-17: findVoteItemsByVoteId 반환 순서 계약 정의UI 일관성을 위해 반환 리스트의 정렬 기준을 포트 수준에서 명시하는 것이 좋습니다(예: 참여수 내림차순, 동률 시 id 오름차순). 구현/테스트가 해당 계약을 따르는지 확인 부탁드립니다. 간단한 JavaDoc 추가도 추천합니다.
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepository.java (1)
18-18: 메서드 네이밍/계약 일관성동일 목적의 복수 메서드가 서로 다른 네이밍 패턴을 가집니다.
- mapVoteItemsByVoteIds(Set, Long) vs findVoteItemsByVoteId(Long, Long)
컨벤션 통일(예: 모두 find…로 시작하고, 그룹핑/매핑은 리턴타입/JavaDoc으로 설명) 또는 JavaDoc 보강을 권장합니다.
src/main/java/konkuk/thip/vote/adapter/in/web/VoteCommandController.java (1)
59-60: 응답 필드(postId)와 경로 변수(voteId) 명칭 불일치 — 계약 확인 필요API 경로는 voteId를 사용하지만 응답은 postId를 노출합니다. 클라이언트 입장에서 혼란(식별자 동등성 의문)이 생길 수 있습니다. 다음 중 하나로 정리하는 것을 권장합니다.
- 응답 필드명을 voteId로 통일
- 혹은 OpenAPI/Schema 설명에 “postId == voteId”를 명확히 표기
현재 계약/문서/프런트 기대치와의 정합성 확인 부탁드립니다.
src/main/java/konkuk/thip/vote/application/service/VoteService.java (1)
43-60: 퍼센트 계산의 0건 처리와 정합성 확인 권장.
VoteItem.calculatePercentages(counts)가 총합 0인 경우(아직 아무도 투표하지 않은 경우) 0으로 안전하게 처리되는지 확인이 필요합니다. 서비스에서 예외가 발생하지 않도록 보장되어야 합니다.- Repository에서 “모든 항목”을 반환한다는 전제가 중요합니다. 0표 항목이 누락되면 퍼센트와 isVoted가 불완전해집니다.
확인을 위해 아래 항목을 점검해 주세요:
calculatePercentages()구현이 합계 0을 처리하는지.findVoteItemsByVoteId()가 0표 항목 포함, 고정된 정렬로 모든 항목을 반환하는지(ORDER BY 보장).필요하다면 테스트 보강도 가능합니다(예: 모든 항목 count=0인 경우 퍼센트 0 반환 케이스).
원하시면 위 두 케이스를 검증하는 단위/통합 테스트 템플릿을 작성해 드립니다.
src/main/java/konkuk/thip/vote/adapter/in/web/response/VoteResponse.java (1)
13-15: 팩토리 오버로드 추가로 매핑 중복 제거 제안.현재 컨트롤러에서
postId/roomId/voteItems를 직접 꺼내어 전달하고 있는데, 이전 형태의of(VoteResult)팩토리를 오버로드로 제공하면 매핑 중복을 줄이고 추후 필드 확장 시 변경 범위를 축소할 수 있습니다.추가 제안 코드:
public record VoteResponse( Long postId, Long roomId, List<VoteResult.VoteItemDto> voteItems ) { - public static VoteResponse of(Long postId, Long roomId, List<VoteResult.VoteItemDto> voteItems) { - return new VoteResponse(postId, roomId, voteItems); - } + public static VoteResponse of(Long postId, Long roomId, List<VoteResult.VoteItemDto> voteItems) { + return new VoteResponse(postId, roomId, voteItems); + } + + // 편의 팩토리 오버로드 + public static VoteResponse of(VoteResult result) { + return new VoteResponse(result.postId(), result.roomId(), result.voteItems()); + } }src/test/java/konkuk/thip/vote/application/service/VoteServiceTest.java (2)
41-46: 모의(stub) 범위 축소로 테스트 신뢰도 향상 제안.
any(), any()로 스터빙하면 잘못된 인자로 호출돼도 테스트가 통과할 수 있습니다.eq(voteId)로 좁히거나, 호출 검증을 추가해 정확한 파라미터로 질의되는지 보장하는 것을 권장합니다. 또한 다항목 반환/정렬/0표 케이스 등도 별도 테스트로 커버되면 좋습니다.- when(voteQueryPort.findVoteItemsByVoteId(any(), any())) + when(voteQueryPort.findVoteItemsByVoteId(eq(voteId), any())) .thenReturn(List.of( new VoteItemQueryDto(voteId, voteItemId, itemName, count, isVoted) ));추가로, 다음 검증도 고려해 주세요.
verify(voteQueryPort).findVoteItemsByVoteId(eq(voteId), anyLong());
58-67: 응답 형태 검증 타당성 확인.단일 항목 케이스에 대해
voteItemId,isVoted를 검증하는 것은 새로운 응답 스펙을 잘 반영합니다. 퍼센트/합계 0 케이스에 대한 보강 테스트를 추가하면 더 견고해집니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/main/java/konkuk/thip/config/OpenApiConfig.java(1 hunks)src/main/java/konkuk/thip/user/adapter/in/web/request/UserSignupRequest.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntity.java(1 hunks)src/main/java/konkuk/thip/user/application/service/UserSignupService.java(1 hunks)src/main/java/konkuk/thip/vote/adapter/in/web/VoteCommandController.java(2 hunks)src/main/java/konkuk/thip/vote/adapter/in/web/response/VoteResponse.java(1 hunks)src/main/java/konkuk/thip/vote/adapter/out/persistence/VoteQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepository.java(1 hunks)src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java(1 hunks)src/main/java/konkuk/thip/vote/application/port/in/dto/VoteResult.java(1 hunks)src/main/java/konkuk/thip/vote/application/port/out/VoteQueryPort.java(1 hunks)src/main/java/konkuk/thip/vote/application/service/VoteService.java(2 hunks)src/test/java/konkuk/thip/vote/adapter/in/web/VoteApiTest.java(4 hunks)src/test/java/konkuk/thip/vote/application/service/VoteServiceTest.java(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-14T14:19:38.796Z
Learnt from: buzz0331
PR: THIP-TextHip/THIP-Server#75
File: src/main/java/konkuk/thip/vote/adapter/out/persistence/VoteQueryRepositoryImpl.java:50-83
Timestamp: 2025-07-14T14:19:38.796Z
Learning: Vote와 VoteItem 엔티티는 자주 함께 사용되므로, N+1 문제를 방지하기 위해 양방향 매핑과 fetch join을 고려하는 것이 좋습니다. 특히 기록장 조회 API 등에서도 함께 사용될 가능성이 높습니다.
Applied to files:
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.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: build
🔇 Additional comments (5)
src/main/java/konkuk/thip/user/adapter/in/web/request/UserSignupRequest.java (1)
16-16: @notblank 추가 적절합니다닉네임에 대한 null/blank 차단이 명확해졌습니다. 이후 Pattern/Size 검증과도 잘 어울립니다.
src/main/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntity.java (1)
27-27: nullable=true 명시 자체는 OKJPA 기본값이 nullable=true이긴 하지만, 의도를 남기는 차원에서 명시적으로 표기한 것은 가독성 측면에서 도움이 됩니다.
src/main/java/konkuk/thip/vote/application/service/VoteService.java (1)
30-30: 트랜잭션 경계 설정 적절합니다.투표/취소 + 카운트 업데이트 + 결과 조회가 하나의 트랜잭션으로 묶여 원자성이 보장됩니다. 기본 FlushMode(AUTO) 하에서는 조회 직전 flush가 수행되어 최신 카운트가 반영됩니다.
src/test/java/konkuk/thip/vote/adapter/in/web/VoteApiTest.java (1)
94-99: 응답 스키마 검증 강화 LGTM.초기 투표 성공 케이스에서
postId/roomId/voteItems[0]를 직접 검증하는 변경은 새 응답 포맷을 잘 커버합니다.src/test/java/konkuk/thip/vote/application/service/VoteServiceTest.java (1)
36-39: 서비스 의존성 주입 변경 LGTM.새 포트(
VoteQueryPort) 주입과 Service 생성자 갱신이 테스트에 반영되어 일관성이 유지됩니다.
|
|
||
| @Value("${server.http-url}") private String httpUrl; | ||
|
|
||
| @Value(("${server.profile}")) private String profile; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
server.profile 미설정 시 구동 실패 가능성 — 기본값 지정 권장
운영 환경에서 항상 server.profile이 주입된다는 보장이 없다면 애플리케이션 부팅 시점에 실패할 수 있습니다. 안전한 기본값(local)을 지정하는 것을 권장합니다.
- @Value(("${server.profile}")) private String profile;
+ @Value("${server.profile:local}") private String profile;아직 server.profile가 어떻게 사용·설정되는지 확인이 필요해 보입니다. 코드 및 설정 파일 전반에서 해당 프로퍼티가 어디에 선언/사용되는지 찾아보겠습니다.
🏁 Script executed:
#!/bin/bash
# repository 루트에서 server.profile 프로퍼티 전체 검색
rg "server.profile" -nLength of output: 135
서버 프로필 기본값 지정 제안
server.profile 프로퍼티가 없을 경우 애플리케이션 구동 시점에
Could not resolve placeholder 'server.profile' 예외가 발생할 수 있습니다.
안전하게 기본값(local)을 지정해 주세요.
- 대상 파일:
src/main/java/konkuk/thip/config/OpenApiConfig.java(라인 49)
- @Value("${server.profile}") private String profile;
+ @Value("${server.profile:local}") private String profile;📝 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.
| @Value(("${server.profile}")) private String profile; | |
| @Value("${server.profile:local}") private String profile; |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/config/OpenApiConfig.java around line 49, the
@Value injection for server.profile currently fails if the property is missing;
change the annotation to provide a default by using the Spring property default
syntax so it falls back to "local" (e.g. replace the current @Value(...) with
@Value("${server.profile:local}") to avoid Could not resolve placeholder
errors).
| @Schema(description = "사용자 닉네임", example = "홍길동_123") | ||
| @NotBlank(message = "닉네임은 필수입니다.") | ||
| @Pattern(regexp = "[가-힣a-zA-Z0-9]+", message = "닉네임은 한글, 영어, 숫자로만 구성되어야 합니다.(공백불가)") | ||
| @Size(max = 10, message = "닉네임은 최대 10자 입니다.") | ||
| String nickname |
There was a problem hiding this comment.
예시와 정규식 간 불일치: 예시(홍길동_123)에 포함된 밑줄(_)이 현재 정규식에 의해 허용되지 않습니다
- 현재 정규식
[가-힣a-zA-Z0-9]+는_를 허용하지 않습니다. - 스키마 예시
홍길동_123을 유지하려면_를 허용하도록 정규식을 수정해야 합니다. 반대로 정규식을 유지하려면 예시에서_를 제거해야 합니다.
대안 1(권장: 밑줄 허용)
- @Pattern(regexp = "[가-힣a-zA-Z0-9]+", message = "닉네임은 한글, 영어, 숫자로만 구성되어야 합니다.(공백불가)")
+ @Pattern(regexp = "^[가-힣a-zA-Z0-9_]+$", message = "닉네임은 한글, 영어, 숫자, 밑줄(_)만 허용됩니다.(공백불가)")대안 2(예시 수정: 밑줄 제거)
- @Schema(description = "사용자 닉네임", example = "홍길동_123")
+ @Schema(description = "사용자 닉네임", example = "홍길동123")🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/user/adapter/in/web/request/UserSignupRequest.java
around lines 15 to 19, the @Pattern regexp doesn't allow underscore but the
@Schema example contains one (홍길동_123); update the validation to make example
and regex consistent. Preferred fix: modify the regexp to include underscore
(e.g. add '_' to the character class) and update the validation message to
mention underscores are allowed; alternative: change the example to remove the
underscore if you want to disallow it. Ensure the @Pattern, @Size, and @Schema
example all match the chosen rule.
There was a problem hiding this comment.
@buzz0331 코래 말처럼 스웨거 schema의 example 을 수정해야할 것 같습니다!!
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| @Column(name = "nickname_updated_at", nullable = true) // 회원가입 시에는 null | ||
| private LocalDate nicknameUpdatedAt; // 날짜 형식으로 저장 (예: "2023-10-01") | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
DB 스키마(DDL) 일치 여부 확인 필요: NOT NULL → NULL 변경에 대한 마이그레이션
JPA 매핑에서 nickname_updated_at을 nullable=true로 변경한 것은 서비스 변경과 일치합니다. 다만 운영/개발 DB에도 동일한 스키마 변경(ALTER TABLE … DROP NOT NULL 등)이 반영되어야 런타임 INSERT 시 에러가 발생하지 않습니다. 마이그레이션 스크립트 존재 여부를 확인해주세요.
다음 스크립트로 마이그레이션/DDL 흔적을 빠르게 점검할 수 있습니다:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "후보 마이그레이션 디렉터리 탐색:"
fd -t d -HI '(db|database).*(migration|migrations)|migration|migrations' || true
echo
echo "nickname_updated_at 관련 스키마/변경 검색:"
rg -n -A3 -B3 -i 'nickname_updated_at|alter table.*users|drop not null|nullable' || true
echo
echo "Liquibase/Hibernate 설정 파일 내 컬럼 정의 검색(XML/YAML 등):"
rg -n -A3 -B3 -i 'nickname_updated_at|nullable' --glob '*.{xml,yml,yaml,sql}' || trueLength of output: 41636
DB 스키마(DDL) 마이그레이션 스크립트 확인 및 추가 필요
UserJpaEntity에서 nickname_updated_at을 nullable=true로 변경하셨지만,
검색 결과 프로젝트 내 마이그레이션 디렉터리나 관련 SQL 스크립트가 발견되지 않았습니다.
운영·개발 DB에도 아래 ALTER 문을 반영하는 마이그레이션 스크립트를 반드시 추가해 주세요.
파일 위치
- src/main/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntity.java (27–29라인)
예시 SQL (프로젝트에 맞게 조정)
-- MySQL
ALTER TABLE users
MODIFY COLUMN nickname_updated_at DATE NULL;
-- PostgreSQL
ALTER TABLE users
ALTER COLUMN nickname_updated_at DROP NOT NULL;마이그레이션 도구(Flyway, Liquibase 등) 디렉터리에 버전 관리되는 스크립트로 포함하시고,
배포 전 스테이징 환경에서 스키마 변경을 검증해 주세요.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntity.java around
lines 27–29, you made nickname_updated_at nullable but did not add a DB
migration; add a versioned migration script (in the project's
Flyway/Liquibase/SQL migrations directory) that updates the users table to make
nickname_updated_at NULL (include the correct DDL for your target DB engine,
e.g., MySQL or PostgreSQL), commit it to version control alongside the code
change, and validate the schema change in staging before deployment.
There was a problem hiding this comment.
오케이 DB 레벨에서도 alter sql 문을 통해서 설정을 변경해야겟네
There was a problem hiding this comment.
@seongjunnoh 네, 맞습니다!
JPA 엔티티 변경과 함께 데이터베이스 스키마도 동기화해야 합니다. 다음과 같이 진행하시면 됩니다:
MySQL의 경우:
ALTER TABLE users MODIFY COLUMN nickname_updated_at DATE NULL;PostgreSQL의 경우:
ALTER TABLE users ALTER COLUMN nickname_updated_at DROP NOT NULL;배포 시 안전을 위해 스테이징 환경에서 먼저 테스트해보시고, 가능하다면 마이그레이션 스크립트로 버전 관리하시는 것을 권장합니다.
🧠 Learnings used
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
There was a problem hiding this comment.
오케이 DB 레벨에서도 alter sql 문을 통해서 설정을 변경해야겟네
수정해두었습니다!!
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java
Show resolved
Hide resolved
| @Override | ||
| public List<VoteItemQueryDto> findVoteItemsByVoteId(Long voteId, Long userId) { | ||
| return voteJpaRepository.findVoteItemsByVoteId(voteId, userId); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
투표 항목 정렬 보장 필요(클라이언트 응답 일관성 관점).
현재 Repository가 반환하는 List<VoteItemQueryDto>의 정렬이 보장되지 않으면, API 응답 내 항목 순서가 호출 시점마다 달라질 수 있습니다. 서비스 단의 퍼센트 계산은 동일 리스트 기반이라 즉각적인 불일치는 없지만, 클라이언트 UI/스냅샷 테스트에서 순서가 바뀌는 것은 불안정성을 유발합니다. Repository 쿼리에서 명시적으로 ORDER BY(비즈니스 기준: 노출 순서/생성일/ID 등)를 보장하거나, 어댑터에서 최소한의 정렬을 적용하는 것을 권장합니다.
다음 스크립트로 Repository 구현에서 ORDER BY가 보장되는지 확인해 주세요.
정렬이 보장되지 않는 경우, 임시 방편으로 어댑터에서 ID 기준 정렬을 적용할 수 있습니다(비즈니스 정렬 기준이 따로 있다면 해당 기준을 사용).
- public List<VoteItemQueryDto> findVoteItemsByVoteId(Long voteId, Long userId) {
- return voteJpaRepository.findVoteItemsByVoteId(voteId, userId);
- }
+ public List<VoteItemQueryDto> findVoteItemsByVoteId(Long voteId, Long userId) {
+ return voteJpaRepository.findVoteItemsByVoteId(voteId, userId)
+ .stream()
+ .sorted(java.util.Comparator.comparing(VoteItemQueryDto::voteItemId))
+ .toList();
+ }참고: 위 변경 시 import가 필요합니다.
import java.util.Comparator;🏁 Script executed:
#!/bin/bash
# findVoteItemsByVoteId 쿼리 정의 및 ORDER BY 여부 확인
rg -n "findVoteItemsByVoteId\\(" -A 10
rg -n "ORDER\\s+BY|order\\s+by" -A 2Length of output: 6109
투표 항목 정렬 보장 필요
VoteQueryRepositoryImpl의 findVoteItemsByVoteId 쿼리에 ORDER BY가 없어 반환 순서가 불안정합니다. 클라이언트 UI나 스냅샷 테스트에서 리스트 순서가 매번 달라지지 않도록, 아래 중 하나를 적용해주세요:
- Repository 레벨에서 QueryDSL
.orderBy(...)추가return jpaQueryFactory .select(new QVoteItemQueryDto(...)) .from(voteItem) … .where(...)
-
.orderBy(voteItem.voteItemId.asc()) // 비즈니스 기준 필드 사용 .fetch();
- 어댑터 레벨에서 반환 리스트에 최소 정렬 적용
public List<VoteItemQueryDto> findVoteItemsByVoteId(Long voteId, Long userId) { -
return voteJpaRepository.findVoteItemsByVoteId(voteId, userId);
-
return voteJpaRepository.findVoteItemsByVoteId(voteId, userId) -
.stream() -
.sorted(Comparator.comparing(VoteItemQueryDto::voteItemId)) -
.toList(); }+ import java.util.Comparator;
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/vote/adapter/out/persistence/VoteQueryPersistenceAdapter.java
around lines 41 to 44, the adapter currently returns
voteJpaRepository.findVoteItemsByVoteId(voteId, userId) but the underlying query
has no ORDER BY so list order is unstable; fix by either adding an
.orderBy(voteItem.voteItemId.asc()) to the QueryDSL query in
VoteQueryRepositoryImpl, or (if changing repository is not possible) sort the
returned list in this adapter before returning, e.g. call
result.sort(Comparator.comparing(VoteItemQueryDto::getVoteItemId)) and add
import java.util.Comparator; to ensure deterministic ordering.
| public static VoteResult of(Long postId, Long roomId, List<VoteItemDto> voteItems) { | ||
| return new VoteResult(postId, roomId, voteItems); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
List 불변성 보장 필요 (외부 변경으로부터 VoteResult 보호)
record 자체는 불변이지만, 생성자에 전달된 List는 외부에서 변경될 수 있습니다. 응답 DTO를 안전하게 유지하려면 방어적 복사를 권장합니다.
아래처럼 불변 리스트로 감싸주세요.
- public static VoteResult of(Long postId, Long roomId, List<VoteItemDto> voteItems) {
- return new VoteResult(postId, roomId, voteItems);
+ public static VoteResult of(Long postId, Long roomId, List<VoteItemDto> voteItems) {
+ return new VoteResult(postId, roomId, List.copyOf(voteItems));
}📝 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 static VoteResult of(Long postId, Long roomId, List<VoteItemDto> voteItems) { | |
| return new VoteResult(postId, roomId, voteItems); | |
| } | |
| public static VoteResult of(Long postId, Long roomId, List<VoteItemDto> voteItems) { | |
| return new VoteResult(postId, roomId, List.copyOf(voteItems)); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/vote/application/port/in/dto/VoteResult.java around
lines 21 to 23, the factory method passes the mutable List voteItems directly
into the record allowing external mutation; change it to pass an immutable
defensive copy (e.g., List.copyOf(voteItems) or Collections.unmodifiableList(new
ArrayList<>(voteItems))) so the VoteResult holds an unmodifiable list and is
protected from external changes.
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.data.voteItems[?(@.voteItemId == %s)].isVoted", item2.getVoteItemId()).value(true)) | ||
| .andExpect(jsonPath("$.data.voteItems[?(@.voteItemId == %s)].isVoted", item1.getVoteItemId()).value(false)); |
There was a problem hiding this comment.
JsonPath 필터 결과는 배열입니다. 단일 값 비교 시 인덱싱이 필요합니다.
[?(@.voteItemId == %s)].isVoted는 배열을 반환합니다. 현재 .value(true)는 단일 boolean을 기대하므로 실패 가능성이 있습니다. 첫 번째 매칭 항목만 검사하려면 [0] 인덱스를 추가하세요(또는 Matchers.contains/hasItem를 사용).
- .andExpect(jsonPath("$.data.voteItems[?(@.voteItemId == %s)].isVoted", item2.getVoteItemId()).value(true))
- .andExpect(jsonPath("$.data.voteItems[?(@.voteItemId == %s)].isVoted", item1.getVoteItemId()).value(false));
+ .andExpect(jsonPath("$.data.voteItems[?(@.voteItemId == %s)].isVoted[0]", item2.getVoteItemId()).value(true))
+ .andExpect(jsonPath("$.data.voteItems[?(@.voteItemId == %s)].isVoted[0]", item1.getVoteItemId()).value(false));대안: import static org.hamcrest.Matchers.hasItem; 후
.andExpect(jsonPath("$.data.voteItems[?(@.voteItemId == %s)].isVoted", item2.getVoteItemId()).value(hasItem(true)))🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/vote/adapter/in/web/VoteApiTest.java around lines
127 to 129, the JsonPath filter expression returns an array so asserting a
single boolean causes failures; either index the first match by appending [0] to
the path (e.g. ...[?(@.voteItemId == %s)].isVoted[0]) or change the assertion to
expect a collection using a Hamcrest matcher (import static
org.hamcrest.Matchers.hasItem and assert value(hasItem(true)) / hasItem(false))
so the test checks the result correctly.
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.data.voteItems[?(@.voteItemId == %s)].isVoted", item.getVoteItemId()).value(false)); | ||
|
|
There was a problem hiding this comment.
JsonPath 필터 결과 인덱싱 누락 동일 이슈.
여기도 배열 비교 이슈가 동일하게 발생할 수 있습니다. [0] 인덱스를 추가하는 것을 권장합니다.
- .andExpect(jsonPath("$.data.voteItems[?(@.voteItemId == %s)].isVoted", item.getVoteItemId()).value(false));
+ .andExpect(jsonPath("$.data.voteItems[?(@.voteItemId == %s)].isVoted[0]", item.getVoteItemId()).value(false));📝 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.
| .andExpect(status().isOk()) | |
| .andExpect(jsonPath("$.data.voteItems[?(@.voteItemId == %s)].isVoted", item.getVoteItemId()).value(false)); | |
| .andExpect(status().isOk()) | |
| .andExpect(jsonPath("$.data.voteItems[?(@.voteItemId == %s)].isVoted[0]", item.getVoteItemId()).value(false)); |
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/vote/adapter/in/web/VoteApiTest.java around lines
174 to 176, the JsonPath assertion uses a filter that returns an array which can
cause indexing ambiguity; update the JsonPath to select the first matched
element by appending [0] before accessing .isVoted (e.g. use
voteItems[?(@.voteItemId == %s)][0].isVoted) so the assertion targets a single
value and avoid array comparison issues.
seongjunnoh
left a comment
seongjunnoh
left a comment
There was a problem hiding this comment.
고생하셨습니다!! 리뷰 몇개 남겼는데 확인 부탁드릴게요!!
| List<Server> serverList = switch (profile) { | ||
| case "prod" -> List.of(new Server().url(httpsUrl).description("HTTPS 배포 서버")); | ||
| case "dev" -> List.of( | ||
| new Server().url(httpUrl).description("HTTP 개발 서버"), | ||
| new Server().url(httpsUrl).description("HTTPS 배포 서버"), | ||
| new Server().url("http://localhost:8080").description("로컬 개발 서버") | ||
| ); | ||
| default -> List.of( | ||
| new Server().url("http://localhost:8080").description("로컬 개발 서버") | ||
| ); | ||
| }; |
| @Column(name = "nickname_updated_at", nullable = true) // 회원가입 시에는 null | ||
| private LocalDate nicknameUpdatedAt; // 날짜 형식으로 저장 (예: "2023-10-01") | ||
|
|
There was a problem hiding this comment.
오케이 DB 레벨에서도 alter sql 문을 통해서 설정을 변경해야겟네
| Alias alias = Alias.from(command.aliasName()); | ||
| User user = User.withoutId( | ||
| command.nickname(), LocalDate.now(), USER.getType(), command.oauth2Id(), alias | ||
| command.nickname(), null, USER.getType(), command.oauth2Id(), alias |
There was a problem hiding this comment.
p3 : withoutId 메서드 내부에서 null을 바로 설정하는건 어떤가요??
| @Schema(description = "사용자 닉네임", example = "홍길동_123") | ||
| @NotBlank(message = "닉네임은 필수입니다.") | ||
| @Pattern(regexp = "[가-힣a-zA-Z0-9]+", message = "닉네임은 한글, 영어, 숫자로만 구성되어야 합니다.(공백불가)") | ||
| @Size(max = 10, message = "닉네임은 최대 10자 입니다.") | ||
| String nickname |
There was a problem hiding this comment.
@buzz0331 코래 말처럼 스웨거 schema의 example 을 수정해야할 것 같습니다!!
| String aliasName, | ||
|
|
||
| @Schema(description = "사용자 닉네임", example = "홍길동_123") | ||
| @NotBlank(message = "닉네임은 필수입니다.") |
There was a problem hiding this comment.
p1 : 이렇게 수정되면 UserSignupControllerTest 의 signup_nickname_blank 메서드 코드도 수정되어야 할 것 같습니다!
(현재 테스트 코드에서는 에러 발생함)
// 현재 테스트 코드
@Test
@DisplayName("[닉네임]값이 공백일 경우, 400 error가 발생한다.")
void signup_nickname_blank() throws Exception {
//given: nickname blank
UserSignupRequest request = new UserSignupRequest(
"문학가",
""
);
//when //then
String testToken = jwtUtil.createSignupToken("kakao_12345678");
mockMvc.perform(post("/users/signup")
.header("Authorization", "Bearer " + testToken) //헤더 추가
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(request)))
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.code").value(API_INVALID_PARAM.getCode()))
.andExpect(jsonPath("$.message", containsString("닉네임은 한글, 영어, 숫자로만 구성되어야 합니다.(공백불가)")));
}
// 수정되어야 할 테스트 코드
@Test
@DisplayName("[닉네임]값이 공백일 경우, 400 error가 발생한다.")
void signup_nickname_blank() throws Exception {
//given: nickname blank
UserSignupRequest request = new UserSignupRequest(
"문학가",
""
);
//when //then
String testToken = jwtUtil.createSignupToken("kakao_12345678");
mockMvc.perform(post("/users/signup")
.header("Authorization", "Bearer " + testToken) //헤더 추가
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(request)))
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.code").value(API_INVALID_PARAM.getCode()))
.andExpect(jsonPath("$.message", anyOf(
containsString("닉네임은 필수입니다."), // @NotBlank
containsString("닉네임은 한글, 영어, 숫자로만 구성되어야 합니다.(공백불가)") // @Pattern
)));
}@pattern와 @notblank 모두 빈 문자열을 허용하지 않으므로, nickname에 "" 이 들어오면 둘 중 어떤 예외가 먼저 발생할 지 모르므로 테스트 코드를 위와 같이 수정해야 테스트 코드의 실패가 발생하지 않을 것 같습니다!!
추가로 제가 정확한 상황을 전달 못드린 거 같아서 다시 전달드리자면,
어제 발생한 이슈는
- FE에서 nickname 이 아니라 nickName 으로 잘못 요청을 보냄
- 따라서 BE에서는 nickname에 해당하는 값을 null로 인식
- 그런데 @pattern 은 "" 은 걸러내지만, null은 걸러내지 못하므로 bean validation 을 문제없이 통과
- 하지만 jpa 레벨에서 nickname은 not null 이므로 UserJpaEntity 를 save 하는 과정에서 DB에러(= 500 에러) 가 발생한 것으로 추정
입니다!!
음 그런데 nickname 으로 "" 공백이 들어올 경우, 현재는 어떤 예외가 먼저 발생하는지 강제하지 못하므로 FE에서의 예외처리 로직도 같이 영향을 받을 것 같은데, 이를 강제하는게 좋을까요?? @buzz0331 @hd0rable
There was a problem hiding this comment.
이에 대해서 코드로 null과 blank를 명시적으로 분리해보았는데 코드로 확인 부탁드릴게여!! (테스트 코드 보시면 될듯!)
There was a problem hiding this comment.
좋습니다!! 이러면 FE에서도 예외 메시지에 대한 혼동이 없을 것 같네요!!
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java
Show resolved
Hide resolved
|
@seongjunnoh 리뷰 모두 반영했습니다! 확인부탁드릴게요~ |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/main/java/konkuk/thip/user/domain/User.java (1)
61-65: 잠재적 NPE: 새 닉네임이 null일 경우 isBlank()에서 NPE 발생업스트림에서 null을 절대 주지 않는 계약이라도 방어적 코드를 권장합니다. 공통 에러 코드로 매핑하여 도메인 일관성을 유지하는 편이 안전합니다.
- private void validateCanUpdateNickname(String nickname) { - if(nickname.isBlank()) { // 빈칸 불가 + private void validateCanUpdateNickname(String nickname) { + if (nickname == null || nickname.isBlank()) { // null/빈칸 불가 throw new InvalidStateException(ErrorCode.USER_NICKNAME_CANNOT_BE_BLANK); }
♻️ Duplicate comments (1)
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java (1)
118-119: Q타입 지역 변수 그림자 선언 제거 권장 (클래스 필드 재사용)클래스 필드에 이미 동일한 Q타입이 선언되어 있는데, 동일 이름의 지역 변수를 재선언하고 있습니다. 가독성을 해치고 유지보수에 불리합니다. 클래스 필드를 그대로 사용하세요.
- QVoteItemJpaEntity voteItem = QVoteItemJpaEntity.voteItemJpaEntity; - QVoteParticipantJpaEntity voteParticipant = QVoteParticipantJpaEntity.voteParticipantJpaEntity;추가로, 본 메서드와
mapVoteItemsByVoteIds에 동일한 선택절이 반복됩니다. 공통 private 메서드로 추출하면 중복을 줄일 수 있습니다(선택).
🧹 Nitpick comments (3)
src/main/java/konkuk/thip/user/adapter/in/web/request/UserSignupRequest.java (1)
18-18: 정규식 앵커 명시로 가독성/명시성 개선 제안Bean Validation은 기본적으로 전체 매칭을 사용하지만, 의도를 코드로 명확히 드러내기 위해 ^, $ 앵커를 추가하는 것을 권장합니다.
- @Pattern(regexp = "[가-힣a-z0-9]+", message = "닉네임은 한글, 영어 소문자, 숫자로만 구성되어야 합니다.(공백불가)") + @Pattern(regexp = "^[가-힣a-z0-9]+$", message = "닉네임은 한글, 영어 소문자, 숫자로만 구성되어야 합니다.(공백불가)")src/test/java/konkuk/thip/user/adapter/in/web/UserSignupControllerTest.java (1)
131-149: null 케이스 커버리지 추가 훌륭함 + 공백문자-only/대문자 케이스도 보강 제안null 입력에 대한 400 검증이 추가되어 회귀 방지에 도움됩니다. 추가로 다음 두 케이스도 보강하면 좋겠습니다.
- 공백 문자만(" ")으로 구성된 닉네임 → 400
- 정책상 소문자만 허용이 의도라면, 대문자 포함("TestUser") → 400
참고 스니펫:
@Test @DisplayName("[닉네임]값이 공백 문자만으로 구성된 경우, 400 error가 발생한다.") void signup_nickname_whitespace_only() throws Exception { UserSignupRequest request = new UserSignupRequest("문학가", " "); String testToken = jwtUtil.createSignupToken("kakao_12345678"); mockMvc.perform(post("/users/signup") .header("Authorization", "Bearer " + testToken) .contentType(MediaType.APPLICATION_JSON) .content(objectMapper.writeValueAsString(request))) .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.code").value(API_INVALID_PARAM.getCode())) .andExpect(jsonPath("$.message", containsString("닉네임은 한글, 영어 소문자, 숫자로만 구성되어야 합니다.(공백불가)"))); } @Test @DisplayName("[닉네임]값에 영어 대문자가 포함된 경우, 400 error가 발생한다. (정책: 소문자만 허용 시)") void signup_nickname_uppercase_should_fail_when_lowercase_only_policy() throws Exception { UserSignupRequest request = new UserSignupRequest("문학가", "TestUser"); String testToken = jwtUtil.createSignupToken("kakao_12345678"); mockMvc.perform(post("/users/signup") .header("Authorization", "Bearer " + testToken) .contentType(MediaType.APPLICATION_JSON) .content(objectMapper.writeValueAsString(request))) .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.code").value(API_INVALID_PARAM.getCode())) .andExpect(jsonPath("$.message", containsString("닉네임은 한글, 영어 소문자, 숫자로만 구성되어야 합니다.(공백불가)"))); }src/main/java/konkuk/thip/user/domain/User.java (1)
52-59: 닉네임 정책 일관성 확보 제안(도메인/표현 계층 간)현재 허용문자/패턴 검증은 표현 계층(@pattern)에서만 수행되고, 도메인에서는 길이/공백/주기만 검사합니다. 정책 변경 시 불일치 위험이 있습니다. 정규식/최대길이를 상수나 Value Object(예: Nickname)로 추출해 도메인에서 1차 보장하거나, 공용 Validator를 두고 컨트롤러/도메인에서 동일 로직을 재사용하는 방식을 검토해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/main/java/konkuk/thip/user/adapter/in/web/request/UserSignupRequest.java(2 hunks)src/main/java/konkuk/thip/user/application/service/UserSignupService.java(1 hunks)src/main/java/konkuk/thip/user/domain/User.java(1 hunks)src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java(1 hunks)src/test/java/konkuk/thip/user/adapter/in/web/UserSignupControllerTest.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-14T14:19:38.796Z
Learnt from: buzz0331
PR: THIP-TextHip/THIP-Server#75
File: src/main/java/konkuk/thip/vote/adapter/out/persistence/VoteQueryRepositoryImpl.java:50-83
Timestamp: 2025-07-14T14:19:38.796Z
Learning: Vote와 VoteItem 엔티티는 자주 함께 사용되므로, N+1 문제를 방지하기 위해 양방향 매핑과 fetch join을 고려하는 것이 좋습니다. 특히 기록장 조회 API 등에서도 함께 사용될 가능성이 높습니다.
Applied to files:
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.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: build
🔇 Additional comments (7)
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java (1)
117-139: voteId 파라미터와 postId 필드 사용 불일치 — 의도 확인 요청메서드명/파라미터는
voteId인데, 필터는voteItem.voteJpaEntity.postId에 걸고 있습니다. 도메인 설계상 Vote의 식별자가postId라면 OK지만, 만약 별도의voteId가 있다면 혼동을 유발할 수 있습니다. 인터페이스/호출부 전반에서 일관성이 맞는지 확인 부탁드립니다. 필요 시 파라미터명 또는 where절 대상을 정리해 주세요.src/main/java/konkuk/thip/user/adapter/in/web/request/UserSignupRequest.java (2)
17-18: null/blank 분리 검증 적용 OK닉네임 null 케이스는 @NotNull, 빈 문자열/공백은 @pattern으로 분리해 처리하는 의도가 명확합니다. 예외 메시지도 목적에 부합합니다.
16-18: 대문자 미허용으로 API 규칙 변경 — 요구사항/호환성 확인 필요이 PR의 목표는 null/blank 검증 보완인데, 정규식이
[가-힣a-z0-9]+로 바뀌면서 영어 대문자가 더 이상 허용되지 않습니다. 기존 클라이언트/데이터에서 대문자 닉네임을 사용 중이라면 갑작스런 400이 발생할 수 있습니다. 의도된 정책이라면 그대로 가되, 테스트에 대문자 실패 케이스를 추가해 주세요. 의도치 않은 변경이라면 아래처럼 되돌리는 것을 권장합니다.- @Schema(description = "사용자 닉네임", example = "thip") - @NotNull(message = "닉네임은 필수입니다.") - @Pattern(regexp = "[가-힣a-z0-9]+", message = "닉네임은 한글, 영어 소문자, 숫자로만 구성되어야 합니다.(공백불가)") + @Schema(description = "사용자 닉네임", example = "thip") + @NotNull(message = "닉네임은 필수입니다.") + @Pattern(regexp = "^[가-힣a-zA-Z0-9]+$", message = "닉네임은 한글, 영어 대소문자, 숫자로만 구성되어야 합니다.(공백불가)")src/test/java/konkuk/thip/user/adapter/in/web/UserSignupControllerTest.java (2)
128-129: 메시지 기대값 갱신 적절blank 케이스가 @pattern에 의해 걸리도록 테스트 기대 메시지를 맞춘 점 합리적입니다.
168-168: 메시지 기대값 갱신 적절허용 문자 정책 변경을 반영한 실패 메시지 검증이 일관적입니다.
src/main/java/konkuk/thip/user/application/service/UserSignupService.java (1)
30-31: 닉네임 변경 시점 정책 반영 OK
User.withoutId호출에서 날짜 인자를 제거하고 도메인에서nicknameUpdatedAt = null을 설정하도록 위임한 구조가 명확하고 테스트 용이성도 좋아졌습니다.src/main/java/konkuk/thip/user/domain/User.java (1)
29-34: 회원가입 시 nicknameUpdatedAt=null 주입 변경 타당초기 가입 직후 닉네임 변경 허용 요구사항을 충족합니다. 아래의 검증 로직과도 자연스럽게 이어집니다.
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java (2)
118-119: 로컬 Q타입 재선언(그림자 변수) 제거 제안 — 클래스 필드 사용으로 단순화클래스 필드로 이미
voteItem,voteParticipant가 선언되어 있는데, 메서드 내에서 동일 이름으로 재선언하여 그림자 변수가 발생했습니다. 가독성과 유지보수성 저하 요인이므로 재선언을 제거하는 편이 좋습니다. (이전 리뷰에서도 동일 지적이 있었습니다.)적용 diff:
- QVoteItemJpaEntity voteItem = QVoteItemJpaEntity.voteItemJpaEntity; - QVoteParticipantJpaEntity voteParticipant = QVoteParticipantJpaEntity.voteParticipantJpaEntity;
96-110: 중복된 DTO 프로젝션(select new QVoteItemQueryDto(...)) 추출로 중복 제거두 메서드에서 동일한 프로젝션이 반복됩니다. 공통 private 메서드로 추출하면 유지보수성이 개선됩니다.
예시(파일 외부 변경 코드 스니펫):
// 파일 내 private 메서드로 추가 private QVoteItemQueryDto voteItemDtoProjection(Long userId) { return new QVoteItemQueryDto( voteItem.voteJpaEntity.postId, voteItem.voteItemId, voteItem.itemName, voteItem.count, JPAExpressions .selectOne() .from(voteParticipant) .where( voteParticipant.voteItemJpaEntity.eq(voteItem), voteParticipant.userJpaEntity.userId.eq(userId) ) .exists() ); } // 사용 예: // .select(voteItemDtoProjection(userId))Also applies to: 121-136
🧹 Nitpick comments (3)
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java (3)
69-88: findTopParticipationVotesByRoom N+1 쿼리 패턴 — 일괄 조회로 개선 권장topVotes를 구한 뒤 각 vote마다 voteItems를 별도 쿼리로 조회하고 있어 N+1이 발생합니다. 과거 러닝(동일 도메인)에서도 fetch join/일괄 조회로의 변경을 권장했습니다.
개선 옵션:
- 옵션 A(권장): topVoteIds를 모아 한 번의 IN 쿼리로 모든 아이템을 조회하고, Java에서 voteId 기준으로 그룹핑.
- 옵션 B: 엔티티에 @batchsize를 적용하거나, Hibernate 기본 배치 페치 사이즈 도입.
- 옵션 C: fetch join은 컬렉션 + limit 조합에서 제약이 있으므로 신중히 적용.
예시(개념 스니펫):
List<Long> topVoteIds = topVotes.stream().map(VoteJpaEntity::getPostId).toList(); List<VoteItemJpaEntity> items = jpaQueryFactory .select(voteItem) .from(voteItem) .where(voteItem.voteJpaEntity.postId.in(topVoteIds)) .orderBy(voteItem.voteJpaEntity.postId.asc(), voteItem.count.desc(), voteItem.voteItemId.asc()) .fetch(); // 이후 Map<Long, List<VoteItemJpaEntity>>로 그룹핑
31-43: findVotesByRoom 결과 순서 비결정적 — orderBy 추가 권장정렬 조건이 없어 호출마다 반환 순서가 달라질 수 있습니다. UI/테스트 안정성을 위해 결정적 정렬을 권장합니다. 도메인 의도에 따라 예를 들어 다음과 같이 고려할 수 있습니다:
- 최신 우선: createdAt.desc(), postId.desc()
- 페이지 순서: page.asc(), postId.asc()
96-113: mapVoteItemsByVoteIds 정렬 기준 누락 — 응답 일관성 위해 orderBy 추가 권장단건 조회 메서드(findVoteItemsByVoteId)는 정렬을 적용하지만, 다건 조회(mapVoteItemsByVoteIds)는 정렬이 없어 클라이언트가 섞인 항목을 받을 수 있습니다. 다음과 같은 결정적 정렬 적용을 권합니다:
- voteId asc, count desc, voteItemId asc
예시(개념 스니펫):
.from(voteItem) .where(voteItem.voteJpaEntity.postId.in(voteIds)) .orderBy( voteItem.voteJpaEntity.postId.asc(), voteItem.count.desc(), voteItem.voteItemId.asc() )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-14T14:19:38.796Z
Learnt from: buzz0331
PR: THIP-TextHip/THIP-Server#75
File: src/main/java/konkuk/thip/vote/adapter/out/persistence/VoteQueryRepositoryImpl.java:50-83
Timestamp: 2025-07-14T14:19:38.796Z
Learning: Vote와 VoteItem 엔티티는 자주 함께 사용되므로, N+1 문제를 방지하기 위해 양방향 매핑과 fetch join을 고려하는 것이 좋습니다. 특히 기록장 조회 API 등에서도 함께 사용될 가능성이 높습니다.
Applied to files:
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java
🔇 Additional comments (3)
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java (3)
121-139: 정렬 기준 및 exists 서브쿼리 구성 적절 — LGTM
- 정렬 조건을 메인 쿼리 단계에 두고, 서브쿼리에는 조건만 두신 점이 바른 사용법입니다.
orderBy(voteItem.count.desc(), voteItem.voteItemId.asc())로 결정적 순서를 보장하는 것도 좋습니다.
137-138: 검증 완료: 용어 일치함
Vote는 PostJpaEntity를 상속하여postId를 식별자로 사용하며, 도메인에서는 이를voteId라 지칭합니다.
따라서findVoteItemsByVoteId(Long voteId, …)가.where(voteJpaEntity.postId.eq(voteId))로 필터링하는 것은 의도대로 동작하며, 명칭도 도메인 관점에서 일관적입니다.Likely an incorrect or invalid review comment.
127-135: userId null 안전성 검증 필요
VoteQueryRepositoryImpl의 서브쿼리에서voteParticipant.userJpaEntity.userId.eq(userId)호출 시
userId가 null이면 QueryDSL 구현체에 따라 NPE 또는 잘못된 SQL(항상 false)이 발생할 수 있습니다. 현재VoteService.vote()진입부에서roomParticipantValidator.validateUserIsRoomMember(command.roomId(), command.userId());로 검증하고 있으나, 요청이 들어오는 Controller/DTO 단계에서
userId가 null로 넘어올 가능성을 확인해야 합니다. 보장이 없다면 다음 중 한 가지를 적용해주세요:
- Controller 또는 API 입력 DTO에
@NotNull어노테이션 추가 및Objects.requireNonNull(command.userId())사전 검증- QueryDSL 조건 빌더를 null-safe하게 변경
BooleanExpression userEq = (userId != null) ? voteParticipant.userJpaEntity.userId.eq(userId) : Expressions.FALSE; // 그리고 .where(..., userEq) 적용roomParticipantValidator.validateUserIsRoomMember내부에서 null 검증이 이미 이뤄진다면, 해당 로직 위치와 동작을 주석으로 명시
| @NotNull(message = "닉네임은 필수입니다.") | ||
| @Pattern(regexp = "[가-힣a-z0-9]+", message = "닉네임은 한글, 영어 소문자, 숫자로만 구성되어야 합니다.(공백불가)") |
There was a problem hiding this comment.
오호 이렇게 notnull + pattern 의 조합이면 [null 체크 + 공백 체크 + 패턴 체크] 가 전부 되겠네요!! 좋습니다!!
@coderabbitai 내가 작성한 내용에 틀린 부분이 없지?
There was a problem hiding this comment.
@seongjunnoh 맞습니다! 분석이 정확합니다.
@NotNull + @Pattern(regexp = "[가-힣a-z0-9]+") 조합으로:
- null 체크:
@NotNull이 null 값을 방지 - 공백 체크: 패턴의
+수량자가 최소 1개 이상의 문자를 요구하므로 빈 문자열 방지 - 패턴 체크: 한글, 영어 소문자, 숫자만 허용하고 공백 등 다른 문자는 거부
이전의 @NotBlank는 null, 빈 문자열, 공백 문자열을 모두 체크했는데, 새로운 조합도 동일한 효과를 내면서 더 엄격한 문자 패턴 검증까지 수행합니다. 아주 좋은 개선이네요!
🧠 Learnings used
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#180
File: src/main/java/konkuk/thip/room/application/service/AttendanceCheckCreateService.java:22-40
Timestamp: 2025-08-14T09:15:31.332Z
Learning: seongjunnoh는 기술적 완벽성보다 실제 비즈니스 시나리오와 사용자 행동 패턴을 우선 고려하며, 발생 가능성이 낮은 엣지 케이스보다는 실용적인 구현을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
2 participants
#️⃣ 연관된 이슈
closes #205
closes #222
closes #219
📝 작업 내용
다음의 api를 수정했습니다.
📸 스크린샷
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit