Open
Conversation
📝 WalkthroughWalkthrough프로그램 도메인 엔티티, 값 객체(Title, Description, Price, Duration), 서비스, 컨트롤러, 저장소를 추가하여 작가가 프로그램을 생성할 수 있는 기능을 구현합니다. 사용자 ID 추출을 위한 어노테이션 및 인자 해석기도 새로 추가됩니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as ProgramController
participant Resolver as UserIdArgumentResolver
participant Service as ProgramService
participant Repository as ProgramRepository
participant Database as DB
Client->>Controller: POST /reservation/program<br/>(ProgramCreateRequest + X-Passport header)
activate Controller
Controller->>Resolver: resolve `@UserId` parameter<br/>(extract userId from X-Passport)
activate Resolver
Resolver->>Resolver: parse & validate Passport header
Resolver-->>Controller: return photographerId (Long)
deactivate Resolver
Controller->>Service: createProgram(photographerId,<br/>ProgramCreateRequest)
activate Service
Service->>Service: Program.create(...)<br/>(validate fields, build domain entity)
Service->>Repository: save(program)
activate Repository
Repository->>Database: INSERT into program table
Database-->>Repository: return saved Program
deactivate Repository
Service-->>Controller: return ProgramResponse(id)
deactivate Service
Controller-->>Client: 201 Created<br/>ResultResponse<ProgramResponse>
deactivate Controller
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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
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 |
Test Results (reservation)237 tests 233 ✅ 8s ⏱️ For more details on these failures, see this check. Results for commit 171d3cd. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In
`@reservation/src/main/java/net/catsnap/CatsnapReservation/program/domain/Program.java`:
- Around line 140-142: The delete() method sets deletedAt using
LocalDateTime.now(), which makes tests time-dependent; modify Program.delete to
accept a Clock parameter (e.g., delete(Clock clock)) and set deletedAt =
LocalDateTime.now(clock), and add a no-arg overload delete() that delegates to
delete(Clock.systemDefaultZone()) so existing callers remain unchanged; update
references to Program.delete in tests to pass a fixed Clock to allow
deterministic unit tests.
- Around line 164-179: The equals/hashCode use only the id field which breaks
collections when id is null; update Program.equals(Object) to treat transient
entities specially by returning this == o when either id is null (i.e., if id ==
null then fall back to instance equality) and otherwise compare ids, and update
Program.hashCode() to return System.identityHashCode(this) (or another stable
identity-based value) when id is null and Objects.hash(id) when id is non-null
so the hashCode remains stable before/after persistence.
In
`@reservation/src/main/java/net/catsnap/CatsnapReservation/program/domain/vo/Title.java`:
- Around line 26-35: The validate method in Title contains redundant length
check: after failing value.isBlank(), value.length() < MIN_LENGTH is always
false because MIN_LENGTH == 1; remove the redundant condition and simplify the
range check in Title.validate to only test value.length() > MAX_LENGTH (or test
value.length() < MIN_LENGTH when MIN_LENGTH > 1), keeping the null/isBlank check
and throwing DomainException with the same message; update any error message
logic to compute current length via value.length() and keep references to
validate, MIN_LENGTH and MAX_LENGTH intact.
In
`@reservation/src/main/java/net/catsnap/CatsnapReservation/program/infrastructure/repository/ProgramRepository.java`:
- Around line 14-22: The findById(Long id) declaration in ProgramRepository
duplicates JpaRepository's method; remove this redundant method declaration from
ProgramRepository and keep the Javadoc elsewhere if you want to document
behavior. If you need a special query that ignores deletion flags, add a clearly
named repository method (e.g., findByIdIgnoringDeleted or use
ProgramSpecification with a method like findOne(Specification<Program> spec))
and implement/compose it instead of shadowing JpaRepository::findById.
In
`@reservation/src/main/java/net/catsnap/CatsnapReservation/program/infrastructure/repository/ProgramSpecification.java`:
- Around line 21-23: Replace hard-coded field name strings in
ProgramSpecification methods (e.g., isActive(), and the methods using
"photographerId") with type-safe references: use the generated JPA metamodel
(Program_.deletedAt, Program_.photographerId) or introduce constants on the
Program entity (e.g., Program.Fields.DELETED_AT /
Program.Fields.PHOTOGRAPHER_ID) and use root.get(Program_.deletedAt) or
root.get(Program.Fields.DELETED_AT) instead of root.get("deletedAt"); ensure the
metamodel is generated (annotation processing enabled) or the new constants are
defined and imported before updating the other methods that currently use string
literals.
In
`@reservation/src/test/java/net/catsnap/CatsnapReservation/program/application/ProgramServiceTest.java`:
- Around line 122-168: Update the tests in ProgramServiceTest that currently
only assert the exception type (the three tests calling
programService.createProgram) to also assert the exception message or error code
for stronger validation; catch or use assertion helpers (e.g., AssertJ's
assertThatThrownBy) to verify DomainException.getMessage() or a specific
errorCode property on the thrown DomainException matches the expected text/code
for empty title, negative price, and zero duration respectively so each test
asserts both type and precise failure reason.
In
`@reservation/src/test/java/net/catsnap/CatsnapReservation/program/domain/ProgramTest.java`:
- Around line 127-137: The test 동일한_ID를_가진_프로그램은_같다() currently compares
program1 to itself and doesn't verify ID-based equality; update the test to
create two distinct Program instances via createDefaultProgram(), assign the
same ID to both (use the Program class's setter or reflection to set the private
id field if no setter exists), then assert that program1.equals(program2)
(assertThat(program1).isEqualTo(program2)); reference the ProgramTest method
동일한_ID를_가진_프로그램은_같다() and helper createDefaultProgram() when locating where to
change the test and use Program#id (or private id field) as the unique
identifier to set.
In
`@reservation/src/test/java/net/catsnap/CatsnapReservation/program/infrastructure/repository/ProgramSpecificationTest.java`:
- Around line 17-62: These tests only assert that
ProgramSpecification.isActive(), isDeleted(), and belongsTo(...) return non-null
Specifications; update them to verify actual predicate behavior by converting to
`@DataJpaTest` integration tests: persist sample Program entities (active/deleted
and with different owner IDs) via the repository, then call
repository.findAll(ProgramSpecification.isActive()),
findAll(ProgramSpecification.isDeleted()),
findAll(ProgramSpecification.belongsTo(1L)), and combinations like
isActive().and(belongsTo(1L)) and assert the returned lists contain the expected
entities; this exercises the real Criteria/Predicate logic rather than only null
checks.
In
`@reservation/src/test/java/net/catsnap/CatsnapReservation/shared/presentation/web/resolver/UserIdArgumentResolverTest.java`:
- Around line 72-137: Add a new test in the resolveArgument_테스트 nested class to
cover an expired Passport: create a Passport with an expired expiredAt (e.g.,
Instant.now().minus(...) so expiredAt < now), sign it using
passportHandler.sign(...) and mock NativeWebRequest/HttpServletRequest to return
that signed token for PassportHandler.PASSPORT_KEY, then call
resolver.resolveArgument(...) and assert it throws PresentationException (and
optionally assert the appropriate PresentationErrorCode for expired passport).
Ensure you reference the existing symbols Passport, passportHandler.sign,
PassportHandler.PASSPORT_KEY, resolver.resolveArgument, and
PresentationException when implementing the test.
Comment on lines
+140
to
+142
| public void delete() { | ||
| this.deletedAt = LocalDateTime.now(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
delete() 메서드의 테스트 용이성 개선을 권장합니다.
LocalDateTime.now()를 직접 호출하면 단위 테스트에서 시간을 제어하기 어렵습니다. Clock을 주입하거나 삭제 시간을 파라미터로 받는 방식을 고려해 보세요.
♻️ Clock 사용 예시
// 엔티티에 Clock 필드 추가 또는 메서드 파라미터로 전달
public void delete(Clock clock) {
this.deletedAt = LocalDateTime.now(clock);
}
// 기본 메서드 오버로드 (기존 호출 유지)
public void delete() {
delete(Clock.systemDefaultZone());
}🤖 Prompt for AI Agents
In
`@reservation/src/main/java/net/catsnap/CatsnapReservation/program/domain/Program.java`
around lines 140 - 142, The delete() method sets deletedAt using
LocalDateTime.now(), which makes tests time-dependent; modify Program.delete to
accept a Clock parameter (e.g., delete(Clock clock)) and set deletedAt =
LocalDateTime.now(clock), and add a no-arg overload delete() that delegates to
delete(Clock.systemDefaultZone()) so existing callers remain unchanged; update
references to Program.delete in tests to pass a fixed Clock to allow
deterministic unit tests.
Comment on lines
+164
to
+179
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) { | ||
| return true; | ||
| } | ||
| if (o == null || getClass() != o.getClass()) { | ||
| return false; | ||
| } | ||
| Program program = (Program) o; | ||
| return Objects.equals(id, program.id); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(id); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
JPA 엔티티의 equals()/hashCode() 구현 시 주의사항
현재 구현은 id만으로 동등성을 판단합니다. 이 방식은 일반적이지만, 영속화되지 않은 엔티티(id == null)들이 HashSet이나 HashMap에 추가된 후 영속화되면 hashCode가 변경되어 컬렉션에서 해당 엔티티를 찾을 수 없게 될 수 있습니다.
현재 사용 패턴에서 이 문제가 발생하지 않는다면 괜찮지만, 향후 영속화 전 엔티티를 컬렉션에 담는 경우가 생긴다면 주의가 필요합니다.
♻️ 대안: id가 null일 때 인스턴스 비교 사용
`@Override`
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
Program program = (Program) o;
- return Objects.equals(id, program.id);
+ return id != null && Objects.equals(id, program.id);
}
`@Override`
public int hashCode() {
- return Objects.hash(id);
+ return getClass().hashCode();
}📝 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.
Suggested change
| @Override | |
| public boolean equals(Object o) { | |
| if (this == o) { | |
| return true; | |
| } | |
| if (o == null || getClass() != o.getClass()) { | |
| return false; | |
| } | |
| Program program = (Program) o; | |
| return Objects.equals(id, program.id); | |
| } | |
| @Override | |
| public int hashCode() { | |
| return Objects.hash(id); | |
| } | |
| `@Override` | |
| public boolean equals(Object o) { | |
| if (this == o) { | |
| return true; | |
| } | |
| if (o == null || getClass() != o.getClass()) { | |
| return false; | |
| } | |
| Program program = (Program) o; | |
| return id != null && Objects.equals(id, program.id); | |
| } | |
| `@Override` | |
| public int hashCode() { | |
| return getClass().hashCode(); | |
| } |
🤖 Prompt for AI Agents
In
`@reservation/src/main/java/net/catsnap/CatsnapReservation/program/domain/Program.java`
around lines 164 - 179, The equals/hashCode use only the id field which breaks
collections when id is null; update Program.equals(Object) to treat transient
entities specially by returning this == o when either id is null (i.e., if id ==
null then fall back to instance equality) and otherwise compare ids, and update
Program.hashCode() to return System.identityHashCode(this) (or another stable
identity-based value) when id is null and Objects.hash(id) when id is non-null
so the hashCode remains stable before/after persistence.
Comment on lines
+26
to
+35
| private void validate(String value) { | ||
| if (value == null || value.isBlank()) { | ||
| throw new DomainException(DomainErrorCode.DOMAIN_CONSTRAINT_VIOLATION, "프로그램 제목은 필수입니다."); | ||
| } | ||
| if (value.length() < MIN_LENGTH || value.length() > MAX_LENGTH) { | ||
| String message = String.format("프로그램 제목은 %d자 이상 %d자 이하여야 합니다. 현재: %d자", | ||
| MIN_LENGTH, MAX_LENGTH, value.length()); | ||
| throw new DomainException(DomainErrorCode.DOMAIN_CONSTRAINT_VIOLATION, message); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
중복 검증 로직 제거 가능
value.isBlank() 검사를 통과한 후에는 value.length() >= 1이 보장됩니다. MIN_LENGTH = 1이므로 value.length() < MIN_LENGTH 조건은 항상 false입니다.
♻️ 제안 수정
private void validate(String value) {
if (value == null || value.isBlank()) {
throw new DomainException(DomainErrorCode.DOMAIN_CONSTRAINT_VIOLATION, "프로그램 제목은 필수입니다.");
}
- if (value.length() < MIN_LENGTH || value.length() > MAX_LENGTH) {
- String message = String.format("프로그램 제목은 %d자 이상 %d자 이하여야 합니다. 현재: %d자",
- MIN_LENGTH, MAX_LENGTH, value.length());
+ if (value.length() > MAX_LENGTH) {
+ String message = String.format("프로그램 제목은 %d자 이하여야 합니다. 현재: %d자",
+ MAX_LENGTH, value.length());
throw new DomainException(DomainErrorCode.DOMAIN_CONSTRAINT_VIOLATION, message);
}
}📝 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.
Suggested change
| private void validate(String value) { | |
| if (value == null || value.isBlank()) { | |
| throw new DomainException(DomainErrorCode.DOMAIN_CONSTRAINT_VIOLATION, "프로그램 제목은 필수입니다."); | |
| } | |
| if (value.length() < MIN_LENGTH || value.length() > MAX_LENGTH) { | |
| String message = String.format("프로그램 제목은 %d자 이상 %d자 이하여야 합니다. 현재: %d자", | |
| MIN_LENGTH, MAX_LENGTH, value.length()); | |
| throw new DomainException(DomainErrorCode.DOMAIN_CONSTRAINT_VIOLATION, message); | |
| } | |
| } | |
| private void validate(String value) { | |
| if (value == null || value.isBlank()) { | |
| throw new DomainException(DomainErrorCode.DOMAIN_CONSTRAINT_VIOLATION, "프로그램 제목은 필수입니다."); | |
| } | |
| if (value.length() > MAX_LENGTH) { | |
| String message = String.format("프로그램 제목은 %d자 이하여야 합니다. 현재: %d자", | |
| MAX_LENGTH, value.length()); | |
| throw new DomainException(DomainErrorCode.DOMAIN_CONSTRAINT_VIOLATION, message); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@reservation/src/main/java/net/catsnap/CatsnapReservation/program/domain/vo/Title.java`
around lines 26 - 35, The validate method in Title contains redundant length
check: after failing value.isBlank(), value.length() < MIN_LENGTH is always
false because MIN_LENGTH == 1; remove the redundant condition and simplify the
range check in Title.validate to only test value.length() > MAX_LENGTH (or test
value.length() < MIN_LENGTH when MIN_LENGTH > 1), keeping the null/isBlank check
and throwing DomainException with the same message; update any error message
logic to compute current length via value.length() and keep references to
validate, MIN_LENGTH and MAX_LENGTH intact.
Comment on lines
+14
to
+22
| /** | ||
| * 프로그램 ID로 조회 (삭제 여부 상관없이) | ||
| * <p> | ||
| * 리뷰 조회 등 삭제된 프로그램도 보여야 하는 경우 사용 | ||
| * | ||
| * @param id 프로그램 ID | ||
| * @return 프로그램 (존재하지 않으면 empty) | ||
| */ | ||
| Optional<Program> findById(Long id); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
findById 메서드 선언이 중복됩니다.
JpaRepository<Program, Long>가 이미 동일한 시그니처의 findById(Long id) 메서드를 제공하므로, 이 메서드 선언은 불필요합니다. Javadoc으로 삭제 여부와 무관하게 조회한다는 의도를 명시한 것은 좋지만, 실제 동작은 상위 인터페이스와 동일합니다.
삭제된 프로그램을 제외하고 조회하는 로직이 필요하다면 ProgramSpecification을 활용하는 별도 메서드를 추가하는 것이 더 명확합니다.
🤖 Prompt for AI Agents
In
`@reservation/src/main/java/net/catsnap/CatsnapReservation/program/infrastructure/repository/ProgramRepository.java`
around lines 14 - 22, The findById(Long id) declaration in ProgramRepository
duplicates JpaRepository's method; remove this redundant method declaration from
ProgramRepository and keep the Javadoc elsewhere if you want to document
behavior. If you need a special query that ignores deletion flags, add a clearly
named repository method (e.g., findByIdIgnoringDeleted or use
ProgramSpecification with a method like findOne(Specification<Program> spec))
and implement/compose it instead of shadowing JpaRepository::findById.
Comment on lines
+21
to
+23
| public static Specification<Program> isActive() { | ||
| return (root, query, cb) -> cb.isNull(root.get("deletedAt")); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
문자열 리터럴 대신 상수 또는 JPA Metamodel 사용을 권장합니다.
필드명을 문자열 리터럴("deletedAt", "photographerId")로 직접 사용하면, 엔티티 필드명이 변경될 경우 컴파일 타임에 오류를 감지할 수 없습니다. JPA Metamodel(예: Program_) 또는 상수를 사용하면 타입 안전성을 확보할 수 있습니다.
♻️ 상수 사용 예시
public class ProgramSpecification {
+ private static final String DELETED_AT = "deletedAt";
+ private static final String PHOTOGRAPHER_ID = "photographerId";
private ProgramSpecification() {
}
public static Specification<Program> isActive() {
- return (root, query, cb) -> cb.isNull(root.get("deletedAt"));
+ return (root, query, cb) -> cb.isNull(root.get(DELETED_AT));
}
public static Specification<Program> isDeleted() {
- return (root, query, cb) -> cb.isNotNull(root.get("deletedAt"));
+ return (root, query, cb) -> cb.isNotNull(root.get(DELETED_AT));
}
public static Specification<Program> belongsTo(Long photographerId) {
- return (root, query, cb) -> cb.equal(root.get("photographerId"), photographerId);
+ return (root, query, cb) -> cb.equal(root.get(PHOTOGRAPHER_ID), photographerId);
}
}Also applies to: 28-29, 37-38
🤖 Prompt for AI Agents
In
`@reservation/src/main/java/net/catsnap/CatsnapReservation/program/infrastructure/repository/ProgramSpecification.java`
around lines 21 - 23, Replace hard-coded field name strings in
ProgramSpecification methods (e.g., isActive(), and the methods using
"photographerId") with type-safe references: use the generated JPA metamodel
(Program_.deletedAt, Program_.photographerId) or introduce constants on the
Program entity (e.g., Program.Fields.DELETED_AT /
Program.Fields.PHOTOGRAPHER_ID) and use root.get(Program_.deletedAt) or
root.get(Program.Fields.DELETED_AT) instead of root.get("deletedAt"); ensure the
metamodel is generated (annotation processing enabled) or the new constants are
defined and imported before updating the other methods that currently use string
literals.
Comment on lines
+122
to
+168
| @Test | ||
| void 빈_제목으로_생성_시_예외가_발생한다() { | ||
| // given | ||
| Long photographerId = 1L; | ||
| ProgramCreateRequest request = new ProgramCreateRequest( | ||
| "", | ||
| "설명", | ||
| 100000L, | ||
| 60 | ||
| ); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> programService.createProgram(photographerId, request)) | ||
| .isInstanceOf(DomainException.class); | ||
| } | ||
|
|
||
| @Test | ||
| void 음수_가격으로_생성_시_예외가_발생한다() { | ||
| // given | ||
| Long photographerId = 1L; | ||
| ProgramCreateRequest request = new ProgramCreateRequest( | ||
| "테스트", | ||
| "설명", | ||
| -1L, | ||
| 60 | ||
| ); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> programService.createProgram(photographerId, request)) | ||
| .isInstanceOf(DomainException.class); | ||
| } | ||
|
|
||
| @Test | ||
| void 영분_소요시간으로_생성_시_예외가_발생한다() { | ||
| // given | ||
| Long photographerId = 1L; | ||
| ProgramCreateRequest request = new ProgramCreateRequest( | ||
| "테스트", | ||
| "설명", | ||
| 100000L, | ||
| 0 | ||
| ); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> programService.createProgram(photographerId, request)) | ||
| .isInstanceOf(DomainException.class); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
예외 메시지 검증 추가 고려
예외 테스트에서 DomainException 타입만 검증하고 있습니다. 예외 메시지나 에러 코드도 함께 검증하면 테스트의 명확성이 높아집니다.
♻️ 제안하는 개선
// when & then
assertThatThrownBy(() -> programService.createProgram(photographerId, request))
- .isInstanceOf(DomainException.class);
+ .isInstanceOf(DomainException.class)
+ .hasMessageContaining("제목");🤖 Prompt for AI Agents
In
`@reservation/src/test/java/net/catsnap/CatsnapReservation/program/application/ProgramServiceTest.java`
around lines 122 - 168, Update the tests in ProgramServiceTest that currently
only assert the exception type (the three tests calling
programService.createProgram) to also assert the exception message or error code
for stronger validation; catch or use assertion helpers (e.g., AssertJ's
assertThatThrownBy) to verify DomainException.getMessage() or a specific
errorCode property on the thrown DomainException matches the expected text/code
for empty title, negative price, and zero duration respectively so each test
asserts both type and precise failure reason.
Comment on lines
+127
to
+137
| @Test | ||
| void 동일한_ID를_가진_프로그램은_같다() { | ||
| // given | ||
| Program program1 = createDefaultProgram(); | ||
| Program program2 = createDefaultProgram(); | ||
|
|
||
| // then | ||
| // ID가 null이므로 equals는 ID 기반으로 동작 | ||
| // 실제 DB 저장 후에는 ID가 할당되어 비교 가능 | ||
| assertThat(program1).isEqualTo(program1); | ||
| } |
There was a problem hiding this comment.
동등성 테스트가 실제 목적을 검증하지 않습니다.
테스트 이름은 "동일한 ID를 가진 프로그램은 같다"이지만, 실제로는 program1을 자기 자신과 비교(assertThat(program1).isEqualTo(program1))하고 있어 동등성 로직을 검증하지 못합니다.
ID 기반 동등성을 테스트하려면 리플렉션으로 ID를 설정하거나, 별도의 테스트 픽스처를 사용해야 합니다.
🐛 테스트 개선 제안
`@Test`
void 동일한_ID를_가진_프로그램은_같다() {
// given
- Program program1 = createDefaultProgram();
- Program program2 = createDefaultProgram();
+ Program program = createDefaultProgram();
// then
- // ID가 null이므로 equals는 ID 기반으로 동작
- // 실제 DB 저장 후에는 ID가 할당되어 비교 가능
- assertThat(program1).isEqualTo(program1);
+ // ID가 null인 경우의 동등성 동작을 명시적으로 문서화
+ assertThat(program).isEqualTo(program); // 동일 인스턴스
+ // 참고: ID 기반 동등성은 통합 테스트에서 DB 저장 후 검증 필요
}🤖 Prompt for AI Agents
In
`@reservation/src/test/java/net/catsnap/CatsnapReservation/program/domain/ProgramTest.java`
around lines 127 - 137, The test 동일한_ID를_가진_프로그램은_같다() currently compares
program1 to itself and doesn't verify ID-based equality; update the test to
create two distinct Program instances via createDefaultProgram(), assign the
same ID to both (use the Program class's setter or reflection to set the private
id field if no setter exists), then assert that program1.equals(program2)
(assertThat(program1).isEqualTo(program2)); reference the ProgramTest method
동일한_ID를_가진_프로그램은_같다() and helper createDefaultProgram() when locating where to
change the test and use Program#id (or private id field) as the unique
identifier to set.
Comment on lines
+17
to
+62
| @Test | ||
| void isActive_Specification이_생성된다() { | ||
| // when | ||
| Specification<Program> spec = ProgramSpecification.isActive(); | ||
|
|
||
| // then | ||
| assertThat(spec).isNotNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void isDeleted_Specification이_생성된다() { | ||
| // when | ||
| Specification<Program> spec = ProgramSpecification.isDeleted(); | ||
|
|
||
| // then | ||
| assertThat(spec).isNotNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void belongsTo_Specification이_생성된다() { | ||
| // when | ||
| Specification<Program> spec = ProgramSpecification.belongsTo(1L); | ||
|
|
||
| // then | ||
| assertThat(spec).isNotNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void Specification을_조합할_수_있다() { | ||
| // when | ||
| Specification<Program> spec = ProgramSpecification.isActive() | ||
| .and(ProgramSpecification.belongsTo(1L)); | ||
|
|
||
| // then | ||
| assertThat(spec).isNotNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void or_조건으로_Specification을_조합할_수_있다() { | ||
| // when | ||
| Specification<Program> spec = ProgramSpecification.belongsTo(1L) | ||
| .or(ProgramSpecification.belongsTo(2L)); | ||
|
|
||
| // then | ||
| assertThat(spec).isNotNull(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Specification 테스트가 객체 생성만 검증합니다.
현재 테스트들은 Specification 객체가 null이 아닌지만 확인하고 있어서, 실제 필터링 로직(isNull, isNotNull, equal 조건)이 올바르게 동작하는지는 검증하지 않습니다.
실제 predicate 동작을 검증하려면 @DataJpaTest를 활용한 통합 테스트가 필요합니다. 현재 테스트는 smoke test 수준으로, 추후 통합 테스트 추가를 권장합니다.
🤖 Prompt for AI Agents
In
`@reservation/src/test/java/net/catsnap/CatsnapReservation/program/infrastructure/repository/ProgramSpecificationTest.java`
around lines 17 - 62, These tests only assert that
ProgramSpecification.isActive(), isDeleted(), and belongsTo(...) return non-null
Specifications; update them to verify actual predicate behavior by converting to
`@DataJpaTest` integration tests: persist sample Program entities (active/deleted
and with different owner IDs) via the repository, then call
repository.findAll(ProgramSpecification.isActive()),
findAll(ProgramSpecification.isDeleted()),
findAll(ProgramSpecification.belongsTo(1L)), and combinations like
isActive().and(belongsTo(1L)) and assert the returned lists contain the expected
entities; this exercises the real Criteria/Predicate logic rather than only null
checks.
Comment on lines
+72
to
+137
| @Nested | ||
| class resolveArgument_테스트 { | ||
|
|
||
| @Test | ||
| void 유효한_Passport에서_userId를_추출한다() throws Exception { | ||
| // given | ||
| Long expectedUserId = 123L; | ||
| String signedPassport = createSignedPassport(expectedUserId, CatsnapAuthority.PHOTOGRAPHER); | ||
|
|
||
| NativeWebRequest webRequest = mock(NativeWebRequest.class); | ||
| HttpServletRequest httpRequest = mock(HttpServletRequest.class); | ||
| when(webRequest.getNativeRequest(HttpServletRequest.class)).thenReturn(httpRequest); | ||
| when(httpRequest.getHeader(PassportHandler.PASSPORT_KEY)).thenReturn(signedPassport); | ||
|
|
||
| // when | ||
| Object result = resolver.resolveArgument(null, null, webRequest, null); | ||
|
|
||
| // then | ||
| assertThat(result).isEqualTo(expectedUserId); | ||
| } | ||
|
|
||
| @Test | ||
| void Passport_헤더가_없으면_UNAUTHORIZED_예외가_발생한다() { | ||
| // given | ||
| NativeWebRequest webRequest = mock(NativeWebRequest.class); | ||
| HttpServletRequest httpRequest = mock(HttpServletRequest.class); | ||
| when(webRequest.getNativeRequest(HttpServletRequest.class)).thenReturn(httpRequest); | ||
| when(httpRequest.getHeader(PassportHandler.PASSPORT_KEY)).thenReturn(null); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> resolver.resolveArgument(null, null, webRequest, null)) | ||
| .isInstanceOf(PresentationException.class) | ||
| .extracting("resultCode") | ||
| .isEqualTo(PresentationErrorCode.UNAUTHORIZED); | ||
| } | ||
|
|
||
| @Test | ||
| void 빈_Passport_헤더면_UNAUTHORIZED_예외가_발생한다() { | ||
| // given | ||
| NativeWebRequest webRequest = mock(NativeWebRequest.class); | ||
| HttpServletRequest httpRequest = mock(HttpServletRequest.class); | ||
| when(webRequest.getNativeRequest(HttpServletRequest.class)).thenReturn(httpRequest); | ||
| when(httpRequest.getHeader(PassportHandler.PASSPORT_KEY)).thenReturn(""); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> resolver.resolveArgument(null, null, webRequest, null)) | ||
| .isInstanceOf(PresentationException.class) | ||
| .extracting("resultCode") | ||
| .isEqualTo(PresentationErrorCode.UNAUTHORIZED); | ||
| } | ||
|
|
||
| @Test | ||
| void 유효하지_않은_Passport면_INVALID_PASSPORT_예외가_발생한다() { | ||
| // given | ||
| NativeWebRequest webRequest = mock(NativeWebRequest.class); | ||
| HttpServletRequest httpRequest = mock(HttpServletRequest.class); | ||
| when(webRequest.getNativeRequest(HttpServletRequest.class)).thenReturn(httpRequest); | ||
| when(httpRequest.getHeader(PassportHandler.PASSPORT_KEY)).thenReturn("invalid-passport"); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> resolver.resolveArgument(null, null, webRequest, null)) | ||
| .isInstanceOf(PresentationException.class) | ||
| .extracting("resultCode") | ||
| .isEqualTo(PresentationErrorCode.INVALID_PASSPORT); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
만료된 Passport 테스트 케이스 추가 고려
현재 테스트가 유효한 Passport, 누락된 헤더, 빈 헤더, 잘못된 형식의 Passport를 커버하고 있습니다. 만료된 Passport에 대한 테스트도 추가하면 더 완전한 커버리지를 갖출 수 있습니다.
♻️ 만료된 Passport 테스트 추가 제안
`@Test`
void 만료된_Passport면_예외가_발생한다() {
// given
Instant past = Instant.now().minus(1, ChronoUnit.HOURS);
Instant expiredAt = past.plus(30, ChronoUnit.MINUTES); // 30분 전 만료
Passport expiredPassport = new Passport((byte) 1, 123L, CatsnapAuthority.PHOTOGRAPHER, past, expiredAt);
String signedPassport = passportHandler.sign(expiredPassport);
NativeWebRequest webRequest = mock(NativeWebRequest.class);
HttpServletRequest httpRequest = mock(HttpServletRequest.class);
when(webRequest.getNativeRequest(HttpServletRequest.class)).thenReturn(httpRequest);
when(httpRequest.getHeader(PassportHandler.PASSPORT_KEY)).thenReturn(signedPassport);
// when & then
assertThatThrownBy(() -> resolver.resolveArgument(null, null, webRequest, null))
.isInstanceOf(PresentationException.class);
}🤖 Prompt for AI Agents
In
`@reservation/src/test/java/net/catsnap/CatsnapReservation/shared/presentation/web/resolver/UserIdArgumentResolverTest.java`
around lines 72 - 137, Add a new test in the resolveArgument_테스트 nested class to
cover an expired Passport: create a Passport with an expired expiredAt (e.g.,
Instant.now().minus(...) so expiredAt < now), sign it using
passportHandler.sign(...) and mock NativeWebRequest/HttpServletRequest to return
that signed token for PassportHandler.PASSPORT_KEY, then call
resolver.resolveArgument(...) and assert it throws PresentationException (and
optionally assert the appropriate PresentationErrorCode for expired passport).
Ensure you reference the existing symbols Passport, passportHandler.sign,
PassportHandler.PASSPORT_KEY, resolver.resolveArgument, and
PresentationException when implementing the test.
Contributor
There was a problem hiding this comment.
Pull request overview
작가(Photographer)가 자신의 촬영 프로그램을 생성할 수 있는 API를 추가하고, Passport 기반 인증 사용자 ID 주입(@userid) 및 관련 테스트/아키텍처 검증을 도입합니다.
Changes:
- Program 생성 유스케이스(도메인/서비스/리포지토리/컨트롤러) 및 요청/응답 DTO 추가
- Passport 헤더에서 userId를 추출해 컨트롤러 파라미터로 주입하는
@UserId+ ArgumentResolver 추가 및 MVC 설정에 등록 - Program/VO/Converter/Controller/Service 관련 테스트 및 컨트롤러 인증 아키텍처(ArchUnit) 테스트 추가
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| reservation/build.gradle | Validation starter 및 ArchUnit 테스트 의존성 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/presentation/ProgramController.java | 프로그램 생성 REST API 엔드포인트 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/application/ProgramService.java | 프로그램 생성 유스케이스 구현 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/application/dto/request/ProgramCreateRequest.java | 프로그램 생성 요청 DTO(+ validation) 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/application/dto/response/ProgramResponse.java | 프로그램 생성 응답 DTO 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/domain/Program.java | Program 애그리거트 및 소프트 삭제/수정 로직 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/domain/vo/Title.java | 제목 VO 및 제약 검증 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/domain/vo/Price.java | 가격 VO 및 제약 검증/무료 여부 로직 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/domain/vo/Duration.java | 소요시간 VO 및 표현/계산 로직 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/domain/vo/Description.java | 설명 VO 및 제약/empty 판정 로직 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/infrastructure/repository/ProgramRepository.java | Program JPA Repository 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/infrastructure/repository/ProgramSpecification.java | Program 조회용 Specification 유틸 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/infrastructure/converter/TitleConverter.java | Title VO JPA 컨버터 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/infrastructure/converter/PriceConverter.java | Price VO JPA 컨버터 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/infrastructure/converter/DurationConverter.java | Duration VO JPA 컨버터 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/program/infrastructure/converter/DescriptionConverter.java | Description VO JPA 컨버터 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/shared/presentation/web/resolver/UserId.java | 컨트롤러 파라미터 userId 주입용 어노테이션 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/shared/presentation/web/resolver/UserIdArgumentResolver.java | Passport 헤더에서 userId 추출하는 ArgumentResolver 추가 |
| reservation/src/main/java/net/catsnap/CatsnapReservation/shared/presentation/web/config/WebMvcConfig.java | ArgumentResolver 등록 로직 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/program/presentation/ProgramControllerTest.java | ProgramController 생성 API/검증/인가 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/program/application/ProgramServiceTest.java | ProgramService 생성 유스케이스 통합 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/program/domain/ProgramTest.java | Program 도메인 동작 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/program/domain/vo/TitleTest.java | Title VO 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/program/domain/vo/PriceTest.java | Price VO 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/program/domain/vo/DurationTest.java | Duration VO 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/program/domain/vo/DescriptionTest.java | Description VO 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/program/infrastructure/converter/TitleConverterTest.java | TitleConverter 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/program/infrastructure/converter/PriceConverterTest.java | PriceConverter 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/program/infrastructure/converter/DurationConverterTest.java | DurationConverter 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/program/infrastructure/converter/DescriptionConverterTest.java | DescriptionConverter 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/program/infrastructure/repository/ProgramSpecificationTest.java | ProgramSpecification 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/shared/presentation/web/resolver/UserIdArgumentResolverTest.java | UserIdArgumentResolver 단위 테스트 추가 |
| reservation/src/test/java/net/catsnap/CatsnapReservation/architecture/ControllerAuthenticationArchitectureTest.java | 모든 컨트롤러 메서드 인증 어노테이션 강제 ArchUnit 테스트 추가 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+65
to
+69
| @Column(nullable = false, updatable = false) | ||
| private LocalDateTime createdAt; | ||
|
|
||
| @LastModifiedDate | ||
| @Column(nullable = false) |
There was a problem hiding this comment.
createdAt/updatedAt에 @CreatedDate/@LastModifiedDate를 사용하면서 컬럼을 nullable = false로 지정했는데, reservation 모듈에서는 @EnableJpaAuditing 설정을 찾을 수 없습니다. 이 상태면 엔티티 저장 시 auditing 값이 채워지지 않아 insert 시점에 NOT NULL 제약 위반으로 실패할 수 있습니다. reservation 애플리케이션에 JPA Auditing 활성화(@EnableJpaAuditing) 설정을 추가하거나, 해당 컬럼을 nullable로 두고 직접 값을 세팅하는 방식으로 수정이 필요합니다.
Suggested change
| @Column(nullable = false, updatable = false) | |
| private LocalDateTime createdAt; | |
| @LastModifiedDate | |
| @Column(nullable = false) | |
| @Column(updatable = false) | |
| private LocalDateTime createdAt; | |
| @LastModifiedDate | |
| @Column |
Comment on lines
+40
to
+44
| * @return {@link UserId} 어노테이션이 있으면 true | ||
| */ | ||
| @Override | ||
| public boolean supportsParameter(MethodParameter parameter) { | ||
| return parameter.hasParameterAnnotation(UserId.class); |
There was a problem hiding this comment.
supportsParameter가 @UserId 존재 여부만 확인하고 파라미터 타입을 검증하지 않습니다. 실수로 @UserId를 Long/long 외 타입에 붙이면 런타임에 바인딩 오류(500)로 이어질 수 있어, Long.class/long.class 여부도 함께 체크하도록 수정하는 편이 안전합니다.
Suggested change
| * @return {@link UserId} 어노테이션이 있으면 true | |
| */ | |
| @Override | |
| public boolean supportsParameter(MethodParameter parameter) { | |
| return parameter.hasParameterAnnotation(UserId.class); | |
| * @return {@link UserId} 어노테이션이 있고 파라미터 타입이 Long/long이면 true | |
| */ | |
| @Override | |
| public boolean supportsParameter(MethodParameter parameter) { | |
| return parameter.hasParameterAnnotation(UserId.class) | |
| && (parameter.getParameterType() == Long.class || parameter.getParameterType() == long.class); |
Comment on lines
+16
to
+28
| * <h3>사용 예시</h3> | ||
| * <pre>{@code | ||
| * @RestController | ||
| * public class ProgramController { | ||
| * | ||
| * @LoginPhotographer | ||
| * @PostMapping("/programs") | ||
| * public ProgramResponse createProgram( | ||
| * @UserId Long photographerId, // Passport에서 userId 추출 | ||
| * @RequestBody ProgramCreateRequest request | ||
| * ) { | ||
| * return programService.createProgram(photographerId, request); | ||
| * } |
There was a problem hiding this comment.
Javadoc 예시가 현재 API 형태와 불일치합니다(예: @PostMapping("/programs"), 반환 타입이 ProgramResponse로만 표기). 실제 구현은 /reservation/program 경로이며 ResultResponse<ProgramResponse>로 래핑해 반환하므로, 예시를 현재 컨트롤러 시그니처/경로와 맞게 갱신하는 것이 혼동을 줄입니다.
Comment on lines
+11
to
+22
| public interface ProgramRepository extends JpaRepository<Program, Long>, | ||
| JpaSpecificationExecutor<Program> { | ||
|
|
||
| /** | ||
| * 프로그램 ID로 조회 (삭제 여부 상관없이) | ||
| * <p> | ||
| * 리뷰 조회 등 삭제된 프로그램도 보여야 하는 경우 사용 | ||
| * | ||
| * @param id 프로그램 ID | ||
| * @return 프로그램 (존재하지 않으면 empty) | ||
| */ | ||
| Optional<Program> findById(Long id); |
There was a problem hiding this comment.
JpaRepository가 이미 findById(Long)를 제공하는데 동일 시그니처를 다시 선언하고 있습니다. 동작 변화 없이 중복만 생기므로 메서드 선언/주석을 제거하고 기본 제공 메서드를 사용하는 편이 유지보수에 유리합니다.
Comment on lines
+17
to
+24
| @Test | ||
| void isActive_Specification이_생성된다() { | ||
| // when | ||
| Specification<Program> spec = ProgramSpecification.isActive(); | ||
|
|
||
| // then | ||
| assertThat(spec).isNotNull(); | ||
| } |
There was a problem hiding this comment.
현재 테스트들이 Specification이 null이 아닌지만 검증하고 있어, 실제로 deletedAt/photographerId 조건이 올바르게 구성되는지(예: isNull/isNotNull/equal) 확인하지 못합니다. 인메모리 DB + repository로 실제 조회 결과를 검증하거나, Criteria API를 통해 생성된 predicate를 검증하는 식으로 의미 있는 assertion을 추가하는 것이 좋습니다.
Comment on lines
+128
to
+135
| void 동일한_ID를_가진_프로그램은_같다() { | ||
| // given | ||
| Program program1 = createDefaultProgram(); | ||
| Program program2 = createDefaultProgram(); | ||
|
|
||
| // then | ||
| // ID가 null이므로 equals는 ID 기반으로 동작 | ||
| // 실제 DB 저장 후에는 ID가 할당되어 비교 가능 |
There was a problem hiding this comment.
테스트 이름은 "동일한 ID를 가진 프로그램은 같다"인데, 실제 assertion은 program1이 자기 자신과 같은지만 확인합니다. 이 테스트는 의도한 행위를 검증하지 못하므로, DB에 저장해 ID가 부여된 두 엔티티를 비교하는 형태로 바꾸거나(통합 테스트로 이동), 최소한 테스트 명/설명을 현재 assertion에 맞게 수정해 주세요.
Suggested change
| void 동일한_ID를_가진_프로그램은_같다() { | |
| // given | |
| Program program1 = createDefaultProgram(); | |
| Program program2 = createDefaultProgram(); | |
| // then | |
| // ID가 null이므로 equals는 ID 기반으로 동작 | |
| // 실제 DB 저장 후에는 ID가 할당되어 비교 가능 | |
| void 프로그램은_자기자신과_같다() { | |
| // given | |
| Program program1 = createDefaultProgram(); | |
| Program program2 = createDefaultProgram(); | |
| // then | |
| // equals의 기본 성질(반사성)을 검증한다. | |
| // ID 기반 동등성은 DB 저장 후 ID가 할당되는 통합 테스트에서 검증한다. |
| * | ||
| * @param id 프로그램 ID | ||
| * @return 프로그램 (존재하지 않으면 empty) | ||
| */ |
There was a problem hiding this comment.
This method overrides CrudRepository<Program,Long>.findById; it is advisable to add an Override annotation.
Suggested change
| */ | |
| */ | |
| @Override |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 관련 이슈
✨ PR 세부 내용
작가가 program을 만드는 api 개발했습니다.
Summary by CodeRabbit
릴리스 노트
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.