feat: [ALT-214] 회원가입 약관 동의 API 구현#84
Conversation
📝 WalkthroughWalkthrough발행된 약관을 조회하는 공개 API를 추가하고, 가입(정규·소셜) 시 클라이언트가 제출한 약관 타입 집합을 검증해 필수 약관 누락 시 예외를 던지며, 동의한 약관을 UserTermsAgreement로 저장합니다. 변경 사항약관 동의 기능 통합
Sequence DiagramsequenceDiagram
participant Client
participant Controller
participant Validator
participant UseCase
participant Repo
participant CreateTx
Client->>Controller: POST /signup (agreedTermsTypes)
Controller->>Validator: validateAndResolve(agreedTermsTypes)
Validator->>Repo: findLatestPublishedPerType()
Repo-->>Validator: List~Terms~
alt missing required
Validator-->>Controller: CustomException REQUIRED_TERMS_NOT_AGREED
Controller-->>Client: 400
else all good
Validator-->>CreateTx: List~Terms~
CreateTx->>Repo: saveAll(UserTermsAgreement)
CreateTx-->>Controller: GenerateTokenResponseDto
Controller-->>Client: 200
end
예상 코드 리뷰 난이도🎯 3 (Moderate) | ⏱️ ~25 minutes 관련 PR
제안 검토자
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/general/terms/controller/TermsPublicController.java`:
- Around line 22-23: The `@Resource`(name=...) on the private final field
getPublishedTermsList in TermsPublicController should be replaced with a Spring
`@Qualifier` to ensure name-based injection works with constructor injection;
remove the `@Resource` annotation and apply `@Qualifier`("getPublishedTermsList") to
the injection point (either the generated constructor parameter or an explicit
constructor) while keeping the field as private final and using constructor
injection (via `@RequiredArgsConstructor` or an explicit constructor) so Spring
will use the named bean for GetPublishedTermsListUseCase.
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/general/user/dto/CreateUserRequestDto.java`:
- Around line 63-65: CreateUserRequestDto's agreedTermsIds only checks the list
itself with `@NotNull` but allows null elements, which can cause
TermsAgreementValidator.findById(null) to throw; add element-level Bean
Validation so each list element is non-null (e.g., apply `@NotNull` to the generic
type of agreedTermsIds and consider `@NotEmpty` on the collection) and adjust
imports/tests accordingly to ensure requests like [1, null, 3] are rejected
before reaching TermsAgreementValidator.
In
`@src/main/java/com/dreamteam/alter/application/terms/service/TermsAgreementValidator.java`:
- Line 20: The validateAndResolve method in TermsAgreementValidator performs
multiple DB reads without a transaction, which can cause non-repeatable reads;
fix it by annotating the method validateAndResolve with `@Transactional`(readOnly
= true) (or apply the same annotation at class level if appropriate), ensure the
org.springframework.transaction.annotation.Transactional import is present, and
run tests to verify behavior remains unchanged.
- Around line 31-35: The current resolver in TermsAgreementValidator uses
termsQueryRepository.findById(id) which only applies notDeleted() and can return
DRAFT/unpublished Terms; update the resolution to only accept published terms by
adding/using a repository method that enforces status = PUBLISHED (e.g.,
TermsQueryRepository.findPublishedById or a modified findById that filters
status), and throw the same CustomException(ErrorCode.TERMS_NOT_FOUND, "약관 ID: "
+ id) when the published-term lookup returns empty so unpublished IDs cannot be
resolved or persisted as UserTermsAgreement.
- Around line 31-35: TermsAgreementValidator currently maps agreedTermsIds and
calls termsQueryRepository.findById for each id causing N+1 queries; change this
to a single bulk fetch by adding/using a repository method like
termsQueryRepository.findAllByIds(List<Long> ids) to retrieve all Term entities
at once, then validate that all requested ids were returned (compare sizes or
check missing ids) and throw the existing CustomException
(ErrorCode.TERMS_NOT_FOUND) for any missing id(s); update the logic in the
method that references agreedTermsIds and remove the per-id findById stream
usage to use the bulk result instead.
In
`@src/main/java/com/dreamteam/alter/application/terms/usecase/GetPublishedTermsList.java`:
- Line 3: GetPublishedTermsList currently depends on adapter DTO
PublishedTermsItemResponseDto which violates hexagonal boundaries; change the
use case and its execute() to return a domain List<Terms> (or appropriate domain
DTO/entity) instead of PublishedTermsItemResponseDto, remove the direct import
of PublishedTermsItemResponseDto from GetPublishedTermsList and
GetPublishedTermsListUseCase, and move the mapping from domain Terms ->
PublishedTermsItemResponseDto into the controller layer (controller should call
GetPublishedTermsList.execute(), map the returned List<Terms> to
PublishedTermsItemResponseDto, and return that to the client).
In
`@src/main/java/com/dreamteam/alter/domain/terms/entity/UserTermsAgreement.java`:
- Around line 4-20: UserTermsAgreement and other domain classes currently import
jakarta.persistence and Spring auditing annotations (e.g.,
AuditingEntityListener, `@CreatedDate`) which couples the domain to
infrastructure; remove all JPA/Spring-specific annotations and imports from the
domain class (UserTermsAgreement) and make it a pure POJO/value object
representing domain state and behavior, then create a separate persistence
representation (e.g., UserTermsAgreementEntity) in the
persistence/infrastructure layer that contains the jakarta.persistence
annotations, AuditingEntityListener use, and mapping fields; add mapping code
(mapper/constructor) between UserTermsAgreement and UserTermsAgreementEntity and
update repositories/services to persist the entity instead of the domain object
so the domain package has zero infrastructure dependencies.
In
`@src/main/java/com/dreamteam/alter/domain/terms/port/inbound/GetPublishedTermsListUseCase.java`:
- Around line 3-9: The GetPublishedTermsListUseCase inbound port currently
returns adapter DTO PublishedTermsItemResponseDto (import at top) which couples
the domain layer to the adapter; change the port to return a domain-level type
(e.g., List<Terms> or a new domain/port-specific response record) instead of
PublishedTermsItemResponseDto, update the interface signature of
GetPublishedTermsListUseCase.execute() accordingly, and move the
PublishedTermsItemResponseDto mapping into the adapter/controller so adapters
convert domain Terms → PublishedTermsItemResponseDto.
In
`@src/main/java/com/dreamteam/alter/domain/terms/port/outbound/UserTermsAgreementRepository.java`:
- Around line 1-7: The domain port interfaces (e.g.,
UserTermsAgreementRepository, ReportRepository, TermsRepository) currently
extend Spring Data's JpaRepository which leaks infrastructure into the domain;
change each port to a pure contract by removing JpaRepository and declaring only
the domain-needed methods (e.g., save(UserTermsAgreement),
Optional<UserTermsAgreement> findById(Long), existsByUserId(...)); then create
corresponding adapter implementations under adapter/outbound that extend
JpaRepository (or wrap a JpaRepository) to fulfill the port contracts, wiring
conversions/mapping as needed so the domain layer has zero Spring Data
dependencies.
In
`@src/test/java/com/dreamteam/alter/application/user/usecase/CreateUserTests.java`:
- Around line 110-111: The verifications using anyList() for
cacheRepository.deleteAll and createUserTx.process are too loose; update them to
assert the actual lists passed (e.g., expected cache keys and terms) by
capturing or matching arguments: replace
then(cacheRepository).should().deleteAll(anyList()) with a stronger check using
an ArgumentCaptor or Mockito.argThat to compare the list contents to the
expected list, and similarly replace
then(createUserTx).should(never()).process(any(), any(), any(), anyBoolean(),
anyBoolean(), anyList()) with a matcher/captor that verifies the exact last
argument list (or uses eq(expectedTerms)) so the test fails on regressions;
apply the same change for the other occurrences referencing createUserTx.process
and cacheRepository.deleteAll in this test class.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3b084980-b748-4d4a-a38d-34dd315385f1
📒 Files selected for processing (20)
src/main/java/com/dreamteam/alter/adapter/inbound/general/terms/controller/TermsPublicController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/terms/controller/TermsPublicControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/terms/dto/PublishedTermsItemResponseDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/user/dto/CreateUserRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/user/dto/CreateUserWithSocialRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/outbound/terms/persistence/TermsQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/application/terms/service/TermsAgreementValidator.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/GetPublishedTermsList.javasrc/main/java/com/dreamteam/alter/application/user/usecase/CreateUser.javasrc/main/java/com/dreamteam/alter/application/user/usecase/CreateUserTx.javasrc/main/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocial.javasrc/main/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocialTx.javasrc/main/java/com/dreamteam/alter/common/exception/ErrorCode.javasrc/main/java/com/dreamteam/alter/domain/terms/entity/UserTermsAgreement.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/GetPublishedTermsListUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsQueryRepository.javasrc/main/java/com/dreamteam/alter/domain/terms/port/outbound/UserTermsAgreementRepository.javasrc/test/java/com/dreamteam/alter/application/terms/service/TermsAgreementValidatorTests.javasrc/test/java/com/dreamteam/alter/application/user/usecase/CreateUserTests.javasrc/test/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocialTests.java
There was a problem hiding this comment.
♻️ Duplicate comments (3)
src/main/java/com/dreamteam/alter/application/terms/service/TermsAgreementValidator.java (1)
20-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
@Transactional(readOnly = true)누락
validateAndResolve()는 DB 읽기 전용 작업임에도 트랜잭션 어노테이션이 없습니다. Spring은readOnly속성을 통해 기저 데이터 접근 레이어를 최적화하며, Hibernate는 세션 FlushMode를MANUAL로 설정하고 더티 체크를 비활성화합니다. 또한 읽기 전용 트랜잭션은 읽기-쓰기 메서드는 기본 데이터 소스를,@Transactional(readOnly = true)메서드는 읽기 전용 데이터 소스를 사용하도록 라우팅이 가능합니다.🛡️ 수정 제안
+import org.springframework.transaction.annotation.Transactional; `@Component`("termsAgreementValidator") `@RequiredArgsConstructor` public class TermsAgreementValidator { private final TermsQueryRepository termsQueryRepository; + `@Transactional`(readOnly = true) public List<Terms> validateAndResolve(Set<TermsType> agreedTermsTypes) {As per coding guidelines, "Read-only operations use
@Transactional(readOnly = true)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/application/terms/service/TermsAgreementValidator.java` around lines 20 - 34, The validateAndResolve method in TermsAgreementValidator performs only read operations but lacks a transactional annotation; annotate the validateAndResolve(Set<TermsType> agreedTermsTypes) method (or the containing class) with `@Transactional`(readOnly = true) so Spring/Hibernate can optimize reads (e.g., set FlushMode.MANUAL and route to read-only datasource); ensure the annotation is imported from org.springframework.transaction.annotation.Transactional and applied to the method signature or class-level as appropriate.src/main/java/com/dreamteam/alter/adapter/inbound/general/terms/controller/TermsPublicController.java (1)
22-23: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
@Resource(name = ...)이름 기반 주입이 여전히 동작하지 않습니다.
@RequiredArgsConstructor가 생성하는 생성자 파라미터에@Resource의name힌트가 전달되지 않으므로 지정한 빈 이름이 무시됩니다. 현재는 구현체가 하나뿐이라 타입 기반으로 우연히 동작하지만, 구현체가 추가되면NoUniqueBeanDefinitionException이 발생합니다.
@Qualifier로 교체하세요.♻️ 수정 방안
+import org.springframework.beans.factory.annotation.Qualifier; ... - `@Resource`(name = "getPublishedTermsList") + `@Qualifier`("getPublishedTermsList") private final GetPublishedTermsListUseCase getPublishedTermsList;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dreamteam/alter/adapter/inbound/general/terms/controller/TermsPublicController.java` around lines 22 - 23, The `@Resource`(name = "getPublishedTermsList") on the final GetPublishedTermsListUseCase field in TermsPublicController won't pass the name into the Lombok-generated constructor; replace it with Spring's `@Qualifier` to preserve the bean name for constructor injection: remove `@Resource` and annotate the GetPublishedTermsListUseCase injection point with `@Qualifier`("getPublishedTermsList") so the `@RequiredArgsConstructor-created` constructor receives the qualifier and the correct bean is selected at runtime (apply the same change for any other `@Resource` usages that need name-based injection).src/test/java/com/dreamteam/alter/application/user/usecase/CreateUserTests.java (1)
192-204: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
anyList()기반 검증이 회귀를 놓칠 수 있습니다.성공 케이스에서
createUserTx.process의agreedTerms인자 및cacheRepository.deleteAll의 키 목록을anyList()로 검증하면 잘못된 인자가 전달되어도 테스트가 통과됩니다.termsAgreementValidator.validateAndResolve의 반환값이 이미List.of(termsMock1, termsMock2)로 명확히 스텁되어 있으므로eq(...)로 강화할 수 있습니다.♻️ 강화된 검증 예시
- then(createUserTx).should().process(eq(request), eq("01012345678"), any(), eq(true), eq(false), anyList()); - then(cacheRepository).should().deleteAll(anyList()); + then(createUserTx).should().process(eq(request), eq("01012345678"), any(), eq(true), eq(false), eq(List.of(termsMock1, termsMock2))); + then(cacheRepository).should().deleteAll( + eq(List.of("SIGNUP:PENDING:signup-session-id", "SIGNUP:CONTACT:01012345678")) + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dreamteam/alter/application/user/usecase/CreateUserTests.java` around lines 192 - 204, Replace the loose anyList() verifications with exact equality checks: assert that createUserTx.process(...) receives the resolved agreed terms by using eq(List.of(termsMock1, termsMock2)) for the agreedTerms parameter (call site: createUserTx.process) and assert cacheRepository.deleteAll(...) is called with eq(List.of(termsMock1, termsMock2)) (call site: cacheRepository.deleteAll); keep the existing stubbing of termsAgreementValidator.validateAndResolve(List) as-is and update the two then(...).should() verifications to use eq(...) matching the stubbed list.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/general/terms/controller/TermsPublicController.java`:
- Around line 22-23: The `@Resource`(name = "getPublishedTermsList") on the final
GetPublishedTermsListUseCase field in TermsPublicController won't pass the name
into the Lombok-generated constructor; replace it with Spring's `@Qualifier` to
preserve the bean name for constructor injection: remove `@Resource` and annotate
the GetPublishedTermsListUseCase injection point with
`@Qualifier`("getPublishedTermsList") so the `@RequiredArgsConstructor-created`
constructor receives the qualifier and the correct bean is selected at runtime
(apply the same change for any other `@Resource` usages that need name-based
injection).
In
`@src/main/java/com/dreamteam/alter/application/terms/service/TermsAgreementValidator.java`:
- Around line 20-34: The validateAndResolve method in TermsAgreementValidator
performs only read operations but lacks a transactional annotation; annotate the
validateAndResolve(Set<TermsType> agreedTermsTypes) method (or the containing
class) with `@Transactional`(readOnly = true) so Spring/Hibernate can optimize
reads (e.g., set FlushMode.MANUAL and route to read-only datasource); ensure the
annotation is imported from
org.springframework.transaction.annotation.Transactional and applied to the
method signature or class-level as appropriate.
In
`@src/test/java/com/dreamteam/alter/application/user/usecase/CreateUserTests.java`:
- Around line 192-204: Replace the loose anyList() verifications with exact
equality checks: assert that createUserTx.process(...) receives the resolved
agreed terms by using eq(List.of(termsMock1, termsMock2)) for the agreedTerms
parameter (call site: createUserTx.process) and assert
cacheRepository.deleteAll(...) is called with eq(List.of(termsMock1,
termsMock2)) (call site: cacheRepository.deleteAll); keep the existing stubbing
of termsAgreementValidator.validateAndResolve(List) as-is and update the two
then(...).should() verifications to use eq(...) matching the stubbed list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ce10aaf7-d562-4776-a484-970a397d7057
📒 Files selected for processing (15)
src/main/java/com/dreamteam/alter/adapter/inbound/general/terms/controller/TermsPublicController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/user/dto/CreateUserRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/user/dto/CreateUserWithSocialRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/outbound/terms/persistence/TermsQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/application/terms/service/TermsAgreementValidator.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/GetPublishedTermsList.javasrc/main/java/com/dreamteam/alter/application/user/usecase/CreateUser.javasrc/main/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocial.javasrc/main/java/com/dreamteam/alter/common/exception/ErrorCode.javasrc/main/java/com/dreamteam/alter/domain/terms/entity/UserTermsAgreement.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/GetPublishedTermsListUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/port/outbound/TermsQueryRepository.javasrc/test/java/com/dreamteam/alter/application/terms/service/TermsAgreementValidatorTests.javasrc/test/java/com/dreamteam/alter/application/user/usecase/CreateUserTests.javasrc/test/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocialTests.java
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/dreamteam/alter/application/terms/service/TermsAgreementValidator.java`:
- Around line 22-35: TermsAgreementValidator.validateAndResolve currently calls
agreedTermsTypes.contains(...) and can NPE if agreedTermsTypes is null; add an
explicit null guard at the start of validateAndResolve (e.g., if
(agreedTermsTypes == null) throw new
CustomException(ErrorCode.REQUIRED_TERMS_NOT_AGREED);) so that a domain-level
CustomException is thrown instead of a NullPointerException before any uses of
agreedTermsTypes in the required-check and the final filter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f61519e8-482c-495b-a4d3-eb2e6d0e6bed
📒 Files selected for processing (6)
src/main/java/com/dreamteam/alter/adapter/outbound/terms/persistence/UserTermsAgreementJpaRepository.javasrc/main/java/com/dreamteam/alter/adapter/outbound/terms/persistence/UserTermsAgreementRepositoryImpl.javasrc/main/java/com/dreamteam/alter/application/terms/service/TermsAgreementValidator.javasrc/main/java/com/dreamteam/alter/domain/terms/port/outbound/UserTermsAgreementRepository.javasrc/test/java/com/dreamteam/alter/application/user/usecase/CreateUserTests.javasrc/test/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocialTests.java
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/general/terms/dto/PublishedTermsItemResponseDto.java`:
- Line 47: PublishedTermsItemResponseDto currently builds version with
".version("v" + result.version())" which yields "vnull" if result.version() is
null; change this to null-safe handling by checking result.version() first
(e.g., if result.version() == null throw an IllegalStateException or return a
clear default/empty value) and only prepend "v" when a non-null version exists;
update the builder usage in PublishedTermsItemResponseDto to use that
null-checked value (referencing result.version() and the .version(...) builder
call).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f457d222-3d44-4ee5-a6a2-34d241e7bf4f
📒 Files selected for processing (4)
src/main/java/com/dreamteam/alter/adapter/inbound/general/terms/dto/PublishedTermsItemResponseDto.javasrc/main/java/com/dreamteam/alter/application/terms/usecase/GetPublishedTermsList.javasrc/main/java/com/dreamteam/alter/domain/terms/port/inbound/GetPublishedTermsListUseCase.javasrc/main/java/com/dreamteam/alter/domain/terms/result/GetPublishedTermsListResult.java
관련 문서
https://www.notion.so/BE-API-35986553162880c8aba5dcb1db372d6a?v=3288655316288071b53d000c67f996d3&source=copy_link
Summary by CodeRabbit
New Features
Bug Fixes / Validation
Tests