-
Notifications
You must be signed in to change notification settings - Fork 1
[#10] feat: User 관련 길이 제한을 도메인으로 이동 #70
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
Walkthrough사용자명/닉네임 최대 길이를 User 도메인 상수로 중앙화했습니다. 기존 하드코딩 값(20)을 User.MAX_USERNAME_LENGTH(50), User.MAX_NICKNAME_LENGTH(20)로 치환했고, 관련 검증 메시지와 테스트 기대값을 일관되게 업데이트했습니다. 공개 API 시그니처 변화는 없습니다. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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
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. 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (7)
src/main/java/com/nexters/teamace/user/domain/User.java (1)
12-13: 후속 개선(옵션): Username/Nickname 값을 VO로 캡슐화상수 중앙화는 훌륭합니다. 다음 단계로 Username/Nickname을 값 객체(Value Object)로 두고 생성 시 불변식(공백 불허, 최대 길이)을 내부에서 강제하면, Command/Request 등 여러 계층에서의 중복 검증을 더 줄일 수 있습니다.
src/main/java/com/nexters/teamace/user/application/CreateUserCommand.java (1)
10-26: 경계값(50자/20자) 허용 테스트 추가 제안현재 테스트에서 51자, 21자 초과 시 예외를 검증하고 있으나, 정확히 최대값(50자·20자)이 허용되는지는 확인되지 않습니다. 회귀 방지를 위해 아래 위치에 다음과 같은 경계값 통과 케이스를 추가해 보세요.
– src/test/java/com/nexters/teamace/user/application/CreateUserCommandTest.java
– src/test/java/com/nexters/teamace/auth/application/SignupCommandTest.java
– src/test/java/com/nexters/teamace/auth/application/LoginCommandTest.java제안 예시:
@Test void usernameExactlyMaxLength_shouldSucceed() { String maxUsername = "a".repeat(User.MAX_USERNAME_LENGTH); assertThatNoException() .isThrownBy(() -> new CreateUserCommand(maxUsername, "ValidNick")); } @Test void nicknameExactlyMaxLength_shouldSucceed() { String maxNickname = "a".repeat(User.MAX_NICKNAME_LENGTH); assertThatNoException() .isThrownBy(() -> new CreateUserCommand("ValidUser", maxNickname)); }[optional_refactors_recommended]
src/test/java/com/nexters/teamace/auth/application/SignupCommandTest.java (1)
54-66: 중첩 클래스 이름의 20 → 50 불일치 정정 및 메시지 상수 사용 제안DisplayName은 50자로 바뀌었으나, 클래스 이름이 여전히 20자를 가리킵니다. 또한 에러 메시지는 상수를 사용하면 정책 변경 시 테스트 수정 비용이 줄어듭니다.
다음 변경을 권장합니다:
- class Context_when_creating_with_username_longer_than_20_characters { + class Context_when_creating_with_username_longer_than_50_characters { @@ - thenThrownBy(() -> new SignupCommand(longUsername, "Valid User")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Username must be between 1 and 50 characters"); + thenThrownBy(() -> new SignupCommand(longUsername, "Valid User")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage(USERNAME_SIZE);파일 상단에 다음 정적 임포트를 추가하세요(선택):
import static com.nexters.teamace.common.exception.ValidationErrorMessage.USERNAME_SIZE;src/test/java/com/nexters/teamace/auth/application/LoginCommandTest.java (1)
53-66: 중첩 클래스 이름의 20 → 50 불일치 정정 및 메시지 상수 사용 제안DisplayName은 50자 기준인데, 클래스 이름은 20자를 참조합니다. 에러 메시지 문자열도 상수 사용이 유지보수에 유리합니다.
권장 변경:
- class Context_when_creating_with_username_longer_than_20_characters { + class Context_when_creating_with_username_longer_than_50_characters { @@ - thenThrownBy(() -> new LoginCommand(longUsername)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Username must be between 1 and 50 characters"); + thenThrownBy(() -> new LoginCommand(longUsername)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage(USERNAME_SIZE);파일 상단에 다음 정적 임포트를 추가하세요(선택):
import static com.nexters.teamace.common.exception.ValidationErrorMessage.USERNAME_SIZE;src/test/java/com/nexters/teamace/user/application/CreateUserCommandTest.java (2)
69-71: 테스트 컨텍스트 클래스명에서 20 → 50으로 일관성 정리 필요표시명(DisplayName)은 50자로 바뀌었지만, 클래스명은 여전히
..._longer_than_20_characters입니다. 가독성과 추후 검색/일관성을 위해 50으로 맞추는 걸 제안합니다.아래처럼 변경 제안합니다:
-class Context_when_creating_with_username_longer_than_20_characters { +class Context_when_creating_with_username_longer_than_50_characters {
75-79: 경계값(정확히 50자) 통과 케이스 테스트 추가 제안51자 초과 실패 케이스는 잘 검증합니다. 추가로 정확히 50자(허용 최대치)와 1자(허용 최소치) 통과 케이스를 포함하면 회귀 방지와 명세 보강에 도움이 됩니다.
예시:
@Test @DisplayName("정확히 50자 사용자명은 통과한다") void it_allows_username_with_exactly_50_characters() { final String username50 = "a".repeat(50); final CreateUserCommand command = new CreateUserCommand(username50, "Valid User"); then(command.username()).isEqualTo(username50); } @Test @DisplayName("정확히 1자 사용자명은 통과한다") void it_allows_username_with_exactly_1_character() { final CreateUserCommand command = new CreateUserCommand("a", "Valid User"); then(command.username()).isEqualTo("a"); }원하시면 PR에 바로 반영 가능한 패치 생성까지 도와드릴게요.
src/main/java/com/nexters/teamace/common/exception/ValidationErrorMessage.java (1)
3-4: common.exception → user.domain 의존 추가는 결합도를 높입니다 (선택적 개선 제안)
ValidationErrorMessage가User도메인에 직접 의존하면서 공통 레이어가 도메인 레이어를 바라보는 구조가 되었습니다. 규모가 커지면 순환 의존이나 공통 모듈의 재사용성 저하로 이어질 수 있습니다.가능한 대안:
- 메시지를 Key 기반으로 관리하고(예:
{validation.username.size}), Bean Validation의{min},{max}플레이스홀더를 활용하여 값은 애노테이션에서 주입.- 도메인에
UserConstraints같은 전용 상수 클래스를 두고, 메시지 클래스는 그 상수 클래스를 참조하지 않도록(키/템플릿만 보유) 역할 분리.- 혹은 constraints 전용의 “shared” 모듈(공통 상수만 제공)을 분리해
common.exception과user양쪽에서 참조.참고로 본 변경 자체는 동작에는 문제 없고, 유지보수 관점에서의 결합도만 짚어드립니다.
Also applies to: 14-18
📜 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 sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
src/main/java/com/nexters/teamace/auth/application/LoginCommand.java(1 hunks)src/main/java/com/nexters/teamace/auth/application/SignupCommand.java(1 hunks)src/main/java/com/nexters/teamace/auth/presentation/SignupRequest.java(1 hunks)src/main/java/com/nexters/teamace/common/exception/ValidationErrorMessage.java(2 hunks)src/main/java/com/nexters/teamace/user/application/CreateUserCommand.java(2 hunks)src/main/java/com/nexters/teamace/user/domain/User.java(1 hunks)src/test/java/com/nexters/teamace/auth/application/LoginCommandTest.java(1 hunks)src/test/java/com/nexters/teamace/auth/application/SignupCommandTest.java(1 hunks)src/test/java/com/nexters/teamace/user/application/CreateUserCommandTest.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/com/nexters/teamace/auth/presentation/SignupRequest.java (1)
src/main/java/com/nexters/teamace/common/exception/ValidationErrorMessage.java (1)
ValidationErrorMessage(5-40)
src/test/java/com/nexters/teamace/user/application/CreateUserCommandTest.java (2)
src/test/java/com/nexters/teamace/auth/application/LoginCommandTest.java (1)
DisplayName(10-66)src/test/java/com/nexters/teamace/auth/application/SignupCommandTest.java (1)
DisplayName(10-108)
src/test/java/com/nexters/teamace/auth/application/LoginCommandTest.java (2)
src/test/java/com/nexters/teamace/auth/application/SignupCommandTest.java (1)
DisplayName(10-108)src/test/java/com/nexters/teamace/user/application/CreateUserCommandTest.java (1)
DisplayName(10-136)
🔇 Additional comments (7)
src/main/java/com/nexters/teamace/user/domain/User.java (2)
12-13: 도메인으로 길이 상수 중앙화: 좋습니다하드코딩 제거 및 도메인에 정책을 명시적으로 둔 점이 명확하고 재사용성도 좋아졌습니다.
12-13: DB 제약 및 검증 애너테이션 일관성 확인 완료
애플리케이션 최대 길이(MAX_USERNAME_LENGTH=50, MAX_NICKNAME_LENGTH=20)를 하회하는 DB 컬럼 제약(varchar(20) 등)이나
@Column(length=20),@Size(max=20)설정이 레포 전반에 존재하지 않습니다.
– Flyway 마이그레이션(V1__Create_user_schema.sql)의 username/nickname은 VARCHAR(255)
– JPA 엔티티에 별도@Column(length=…)미지정(기본 255)
– 프레젠테이션(@SiZe), 비즈니스 로직(IllegalArgumentException) 모두 상수 기반으로 최신화됨따라서 추가 조치가 필요 없습니다.
src/main/java/com/nexters/teamace/user/application/CreateUserCommand.java (1)
14-16: 하드코딩 제거 및 도메인 상수 사용 OK길이 검증이 User 도메인 상수로 일원화되어 유지보수성이 좋아졌습니다.
Also applies to: 22-24
src/main/java/com/nexters/teamace/auth/application/LoginCommand.java (1)
13-15: 도메인 상수 사용으로 검증 일원화: 좋습니다하드코딩 제거로 정책 변경 시 파급 범위가 줄었습니다. hasText 검증 이후 length 확인 순서도 안전합니다.
src/main/java/com/nexters/teamace/auth/application/SignupCommand.java (1)
5-5: 도메인 상수로 길이 검증 중앙화한 점 좋습니다하드코딩(20) 제거하고
User.MAX_USERNAME_LENGTH/User.MAX_NICKNAME_LENGTH로 치환되어 일관성과 변경 용이성이 좋아졌습니다. 예외 메시지도 중앙 메시지에 위임되어 유지보수에 유리합니다.Also applies to: 13-21
src/main/java/com/nexters/teamace/auth/presentation/SignupRequest.java (2)
10-20: LGTM프레젠테이션 레이어의 검증도 도메인 상수를 사용하도록 통일되어 일관성이 좋아졌습니다.
10-19: @SiZe 애노테이션의 max 값은 이미 컴파일타임 상수로 정의되어 있습니다.
User.MAX_USERNAME_LENGTH = 50User.MAX_NICKNAME_LENGTH = 20따라서 현재 코드에 컴파일 에러는 발생하지 않습니다.
추가로, 필요에 따라 메시지 템플릿에서{min}/{max}플레이스홀더를 활용하는 방법도 고려해볼 수 있습니다.
| @DisplayName("50자보다 긴 사용자명으로 생성할 때") | ||
| class Context_when_creating_with_username_longer_than_20_characters { | ||
|
|
||
| @Test | ||
| @DisplayName("IllegalArgumentException을 던진다") | ||
| void it_throws_IllegalArgumentException() { | ||
| final String longUsername = "a".repeat(21); | ||
| final String longUsername = "a".repeat(51); | ||
|
|
||
| thenThrownBy(() -> new CreateUserCommand(longUsername, "Valid User")) | ||
| .isInstanceOf(IllegalArgumentException.class) | ||
| .hasMessage("Username must be between 1 and 20 characters"); | ||
| .hasMessage("Username must be between 1 and 50 characters"); |
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.
💡 Verification agent
🧩 Analysis chain
레거시 명칭/문구 잔존 여부 점검 스크립트
다른 테스트의 컨텍스트 클래스명이나 메시지에 ..._20_characters가 남아 있을 수 있습니다. 아래 스크립트로 점검해 보세요. Username 관련해서는 더 이상 "20"이 남지 않는 것이 기대 결과입니다.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "컨텍스트 클래스명에 'longer_than_20_characters' 잔존 검색"
rg -nP -C2 'longer_than_20_characters' --type=java
echo
echo "Username 관련 'between 1 and 20 characters' 메시지 잔존 검색"
rg -nP -C2 'Username must be between 1 and 20 characters' --type=javaLength of output: 3055
테스트 컨텍스트 클래스명 ‘…_20_characters’ → ‘…_50_characters’로 일관성 있게 수정 필요
아래 테스트 파일들에서 컨텍스트 클래스명이 아직 …_longer_than_20_characters로 남아 있으니, 새로 적용된 50자 제한에 맞춰 모두 변경해주세요.
메시지 문자열(Username must be between 1 and 20 characters)은 더 이상 남아 있지 않아 정상입니다.
- src/test/java/com/nexters/teamace/user/application/CreateUserCommandTest.java:70
class Context_when_creating_with_username_longer_than_20_characters - src/test/java/com/nexters/teamace/auth/application/SignupCommandTest.java:55
class Context_when_creating_with_username_longer_than_20_characters - src/test/java/com/nexters/teamace/auth/application/LoginCommandTest.java:54
class Context_when_creating_with_username_longer_than_20_characters
🤖 Prompt for AI Agents
In src/test/java/com/nexters/teamace/user/application/CreateUserCommandTest.java
around lines 69-79 (and similarly in
src/test/java/com/nexters/teamace/auth/application/SignupCommandTest.java around
line 55 and
src/test/java/com/nexters/teamace/auth/application/LoginCommandTest.java around
line 54), the inner test context class name still reads
Context_when_creating_with_username_longer_than_20_characters; rename these to
Context_when_creating_with_username_longer_than_50_characters (and update any
references/imports in those files) so the class names reflect the new
50-character username limit and remain consistent across tests.
pythonstrup
left a 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.
고생하셨습니다
| if (username.length() > User.MAX_USERNAME_LENGTH) { | ||
| throw new IllegalArgumentException(USERNAME_SIZE); |
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.
매직 넘버 제거한 거 아주 좋습니다👍👍👍👍
📌 관련 이슈
#10
📝 작업 개요
실제 deviceCode가 길어서 username 길이 제한 상향 조정
✅ 작업 사항
💭 고민한 점들(필수 X)
하드코딩 되어있어서 하나로 뺐는데 어디 둘까 하다가 일단 도메인으로 뺐어욤
Summary by CodeRabbit