Conversation
- password 컬럼 nullable 허용 (소셜 전용 계정은 비밀번호 없음) - createWithSocial() 정적 팩토리 메서드 추가
- CreateUserWithSocialUseCase 인바운드 포트 추가 - CreateUserWithSocial 유스케이스 구현 - Redis 회원가입 세션 검증 - 닉네임·연락처·소셜ID 중복 확인 - 이메일 인증 세션 선택적 처리 - 소셜 인증 후 User 및 UserSocial 생성
- CreateUserWithSocialRequestDto 추가 (소셜 회원가입 요청 DTO) - POST /signup-social 엔드포인트 구현 - UserPublicControllerSpec에 Swagger 문서 추가
- 세션 미존재, 닉네임/연락처/소셜ID 중복 예외 케이스 - 정상 소셜 회원가입 시나리오 - 이메일 인증 세션 포함 시나리오
- 소셜 회원가입 시 별도 이메일 인증 없이 소셜 계정에서 제공하는 이메일을 User.email에 바로 저장하도록 변경 - socialAuthInfo.getEmail()로 이메일을 취득하고 중복 확인 후 저장 - 소셜 계정에 이메일이 없는 경우 null로 저장 (기존 nullable 정책 유지)
- UnlinkSocialAccountUseCase 추가 (DELETE /app/users/social/unlink) - 비밀번호 없는 사용자가 마지막 소셜 계정 해제 시 차단 (A016) - UserSocial hard delete: UserSocialRepository command port + UserSocialJpaRepository 신규 추가 - UserSocialQueryRepository에 findByUserIdAndSocialProvider, countByUserId 추가 - ErrorCode A015(SOCIAL_ACCOUNT_NOT_LINKED), A016(SOCIAL_UNLINK_NOT_ALLOWED), A017(INVALID_CURRENT_PASSWORD) 추가
- UpdatePasswordUseCase 추가 (PUT /app/users/me/password) - 비밀번호 있는 사용자: currentPassword 검증 후 변경 (A017) - 소셜 전용 사용자(password=null): currentPassword 없이 바로 신규 설정
📝 WalkthroughWalkthrough소셜 계정 회원가입, 비밀번호 변경, 소셜 계정 해제 기능을 추가합니다. 새로운 REST 엔드포인트, 요청 DTO, 애플리케이션 유스케이스, 저장소 인터페이스 및 구현체, 도메인 포트를 도입하고 포괄적인 단위 테스트로 검증합니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/general/user/controller/UserSocialController.java`:
- Around line 38-40: The DELETE handler unlinkSocialAccount in
UserSocialController currently accepts a request body
(UnlinkSocialAccountRequestDto), which can be dropped by gateways/proxies;
change the API to accept the provider as a query param instead: replace the
`@RequestBody` UnlinkSocialAccountRequestDto parameter with a `@RequestParam`
SocialProvider provider in the unlinkSocialAccount method signature and update
any call sites and service-layer calls that read from
UnlinkSocialAccountRequestDto to use the provider parameter (and remove or adapt
the DTO) so the controller no longer depends on a DELETE body.
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/general/user/dto/CreateUserWithSocialRequestDto.java`:
- Around line 30-39: CreateUserWithSocialRequestDto currently allows both
oauthToken and authorizationCode to be empty; add platform-specific
required-field validation by implementing a custom bean validation on the DTO
(e.g., an `@AssertTrue` boolean method or a custom ConstraintValidator). Inside
the validator method (in CreateUserWithSocialRequestDto) check platformType
(PlatformType) and enforce the required combination: for WEB require
authorizationCode non-empty, for NATIVE require oauthToken non-null (and
validate its inner fields), and for any other PlatformType enforce the
appropriate rule or a clear failure message; return false (or raise a constraint
violation) with a helpful message referencing oauthToken/authorizationCode when
the rule fails. Ensure validation annotations remain on
oauthToken/authorizationCode and that the error message is meaningful for
clients.
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/general/user/dto/UpdatePasswordRequestDto.java`:
- Around line 14-22: The DTO lacks validation preventing the new password from
matching the current one; update the validation so UpdatePasswordRequestDto
enforces newPassword != currentPassword (or add the check in the UpdatePassword
handling logic) by adding a cross-field validation: implement a custom
constraint or an `@AssertTrue` method on UpdatePasswordRequestDto that compares
currentPassword and newPassword and fails when they are equal, and ensure the
constraint/message is used by the controller/service that invokes UpdatePassword
so identical passwords are rejected.
In
`@src/main/java/com/dreamteam/alter/adapter/outbound/user/persistence/UserSocialQueryRepositoryImpl.java`:
- Around line 22-30: The current implementation in
UserSocialQueryRepositoryImpl::findByUserIdAndSocialProvider uses
queryFactory.selectFrom(QUserSocial.userSocial)...fetchOne(), which will throw
at runtime if multiple rows exist because (user_id, social_provider) is not
constrained as unique; either add a DB/entity-level unique constraint for
(user_id, social_provider) on the UserSocial entity (e.g., via
`@Table`(uniqueConstraints=...)) to guarantee single result, or change the
repository query to a defensive read (e.g., use fetchFirst() or use
fetch().stream().findFirst()) so the method returns the first match without
throwing; update the code in the findByUserIdAndSocialProvider method
accordingly and ensure tests cover duplicate-row behavior.
In
`@src/main/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocial.java`:
- Around line 24-25: The use case CreateUserWithSocial currently depends on
Spring's StringRedisTemplate and performs Redis deletions before DB commit;
replace the direct dependency by introducing an outbound port interface
SignupSessionPort (e.g., method deleteSignupSession(String key) and any
read/write methods used) and inject that port into CreateUserWithSocial instead
of StringRedisTemplate, update all calls (lines referencing redisTemplate ops)
to call signupSessionPort methods, remove any Spring Data imports, and ensure
Redis cleanup runs only after successful DB commit by registering a transaction
synchronization (e.g., TransactionSynchronizationManager.registerSynchronization
with an afterCommit callback that calls
signupSessionPort.deleteSignupSession(sessionKey)) or using an after-commit
transactional event; update CreateUserWithSocial to use these symbols and remove
direct infra coupling.
- Around line 100-106: The Redis deletions (redisTemplate.delete(sessionIdKey)
and redisTemplate.delete(CONTACT_INDEX_KEY_PREFIX + contact)) are happening
before the DB work (authService.generateAuthorization and
authLogRepository.save) and must be moved to run only after a successful commit;
change this by registering an after-commit hook with
TransactionSynchronizationManager (e.g.,
TransactionSynchronizationManager.registerSynchronization) that performs the two
redisTemplate.delete calls in its afterCommit() method so the Redis cleanup only
executes once the transaction committing the authorization and auth log
succeeds.
In
`@src/main/java/com/dreamteam/alter/application/user/usecase/UnlinkSocialAccount.java`:
- Around line 33-40: The current check (userSocialQueryRepository.countByUserId)
followed by userSocialRepository.delete is racy; replace with an atomic
conditional deletion or a pessimistic lock in UnlinkSocialAccount. Implement and
call a repository operation like
userSocialRepository.deleteIfMoreThanOneForUser(userSocial.getId(),
user.getId()) that performs a single SQL conditional DELETE (e.g., DELETE ...
WHERE id = ? AND (SELECT COUNT(*) FROM user_social WHERE user_id = ?) > 1) and
throws CustomException(ErrorCode.SOCIAL_UNLINK_NOT_ALLOWED) if the delete
affected 0 rows, or alternatively load the user's socials with a
PESSIMISTIC_WRITE lock, re-check the count, and then delete to ensure the
check+delete is atomic. Ensure to reference and update calls to
userSocialQueryRepository.countByUserId and userSocialRepository.delete in
UnlinkSocialAccount accordingly.
In `@src/main/java/com/dreamteam/alter/domain/user/entity/User.java`:
- Around line 34-35: LoginWithPassword is calling
passwordEncoder.matches(request.getPassword(), user.getPassword()) without
null-checking user.getPassword(), causing NPE for social-only users; update the
login flow (LoginWithPassword.java) to first check if user.getPassword() == null
and treat that as invalid login, following the pattern from UpdatePassword.java
(i.e., replace the direct matches call with a guard like: if (user.getPassword()
== null || !passwordEncoder.matches(...)) throw INVALID_LOGIN_INFO), referencing
the User.password field and the passwordEncoder usage to locate where to add the
check.
In
`@src/main/java/com/dreamteam/alter/domain/user/port/inbound/CreateUserWithSocialUseCase.java`:
- Around line 3-7: The domain port CreateUserWithSocialUseCase currently depends
on adapter DTOs (CreateUserWithSocialRequestDto, GenerateTokenResponseDto);
replace those with domain-level request/response types (e.g.,
CreateUserWithSocialCommand or CreateUserWithSocialRequest and
DomainGenerateTokenResponse) defined under the domain package, update the
CreateUserWithSocialUseCase.execute signature to use these domain types, and
move mapping responsibilities to the inbound adapter (convert
CreateUserWithSocialRequestDto → domain request and domain response →
GenerateTokenResponseDto in the adapter/controller). Ensure no imports from
adapter.inbound.* remain in the domain port.
In
`@src/main/java/com/dreamteam/alter/domain/user/port/inbound/UnlinkSocialAccountUseCase.java`:
- Around line 3-7: The domain inbound port UnlinkSocialAccountUseCase currently
depends on the adapter DTO UnlinkSocialAccountRequestDto, breaking layer
boundaries; change the signature of UnlinkSocialAccountUseCase.execute(AppActor,
...) to accept a domain-owned command or primitives (e.g., create a domain model
like UnlinkSocialAccountCommand in com.dreamteam.alter.domain.user.command or
use simple params) instead of UnlinkSocialAccountRequestDto, remove the import
of adapter.inbound DTO from the domain interface, and update the inbound adapter
to map UnlinkSocialAccountRequestDto → UnlinkSocialAccountCommand before calling
UnlinkSocialAccountUseCase.execute.
In
`@src/main/java/com/dreamteam/alter/domain/user/port/inbound/UpdatePasswordUseCase.java`:
- Around line 3-7: The domain port UpdatePasswordUseCase currently depends on an
adapter DTO (UpdatePasswordRequestDto); remove this infrastructure dependency by
introducing a domain-level request/command object (e.g., UpdatePasswordCommand
or UpdatePasswordRequest) in the domain package and change the method signature
of UpdatePasswordUseCase.execute(AppActor, UpdatePasswordRequestDto) to use that
domain object instead; then perform mapping from UpdatePasswordRequestDto to the
new domain request inside the inbound adapter layer (where
CreateUserWithSocialUseCase was handled), ensuring UpdatePasswordUseCase, its
implementations, and any domain services only import the new domain request
class and not the adapter DTO.
In
`@src/test/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocialTests.java`:
- Around line 93-124: Replace the four near-identical helpers
(createSocialAuthInfo, createSocialAuthInfoForDuplicateSocialId,
createSocialAuthInfoForDuplicateEmail, createSocialAuthInfoWithoutEmail) with a
single parameterized factory that returns a mocked SocialAuthInfo; e.g., a
method like createSocialAuthInfo(String socialId, `@Nullable` String email,
`@Nullable` String refreshToken, SocialProvider provider) that mocks
SocialAuthInfo and conditionally stubs getProvider(), getSocialId(), getEmail(),
and getRefreshToken() only when non-null (use SocialProvider.KAKAO as default
where callers expect it); update tests to call this new helper and remove the
four old methods so maintenance and variations are centralized around one
function.
- Around line 132-286: Rename the test methods to follow the
action_condition_expectedResult naming convention instead of the current
fails_when... and succeeds_with... forms: update methods like
fails_whenSignupSessionNotFound, fails_whenNicknameDuplicated,
fails_whenContactDuplicated, fails_whenSocialIdAlreadyRegistered,
fails_whenSocialEmailAlreadyExists, succeeds_withValidSocialSignup, and
succeeds_withNoEmailFromSocialAccount to names that start with the action (e.g.,
createUserWithSocial), then the condition, then the expected result (e.g.,
createUserWithSocial_signupSessionNotFound_throwsSignUpSessionNotExist); keep or
adjust `@DisplayName` as needed and update any direct references to these method
names in the test class (e.g., in CreateUserWithSocialTests) so tests compile
and run.
- Around line 182-225: The tests for SOCIAL_ID_DUPLICATED and EMAIL_DUPLICATED
need to also assert that the signup session key is cleaned up from Redis after
the failure; after invoking createUserWithSocial.execute(request) add
verification that the Redis deletion call for "SIGNUP:PENDING:signup-session-id"
was invoked (match the same Redis collaborator used in the test setup — e.g.,
verify(redisTemplate).delete("SIGNUP:PENDING:signup-session-id") or
verify(yourRedisClient).remove(...) depending on implementation) in both failing
tests, and keep the existing assertion that userRepository.save is never called.
In
`@src/test/java/com/dreamteam/alter/application/user/usecase/UnlinkSocialAccountTests.java`:
- Around line 83-97: The two tests execute_withLinkedSocial_succeeds and
execute_withSingleSocial_succeeds duplicate the same observable behavior;
consolidate or differentiate them: either remove one and keep a single test that
asserts unlinkSocialAccount.execute(actor, request) does not throw and that
userSocialRepository.delete(userSocial) is invoked, or update
execute_withSingleSocial_succeeds to set up the "single social" precondition and
add an extra verification (e.g., verify that userRepository.countSocialsByUserId
or equivalent is not called/was called as intended) to demonstrate the
special-case behavior; locate the test methods and the calls to
unlinkSocialAccount.execute and userSocialRepository.delete to implement the
change.
In
`@src/test/java/com/dreamteam/alter/application/user/usecase/UpdatePasswordTests.java`:
- Around line 75-79: Remove the fragile verification that all interactions on
the mocked user are exhausted in these negative tests: delete the
then(user).shouldHaveNoMoreInteractions() assertions (seen at the end of the
failing tests) and keep only the essential assertions that the use case throws
CustomException with ErrorCode.INVALID_CURRENT_PASSWORD and that no
updatePasswordRepository.save/update method was called; this avoids failing the
test due to the legitimate user.getPassword() call inside updatePassword.execute
while still verifying that password update logic was not invoked.
🪄 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: 95919fb6-7db0-4216-92e1-8b234d3a12a1
📒 Files selected for processing (25)
src/main/java/com/dreamteam/alter/adapter/inbound/general/user/controller/UserPublicController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/user/controller/UserPublicControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/user/controller/UserSelfController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/user/controller/UserSelfControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/user/controller/UserSocialController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/user/controller/UserSocialControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/user/dto/CreateUserWithSocialRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/user/dto/UnlinkSocialAccountRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/user/dto/UpdatePasswordRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/outbound/user/persistence/UserSocialJpaRepository.javasrc/main/java/com/dreamteam/alter/adapter/outbound/user/persistence/UserSocialQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/adapter/outbound/user/persistence/UserSocialRepositoryImpl.javasrc/main/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocial.javasrc/main/java/com/dreamteam/alter/application/user/usecase/UnlinkSocialAccount.javasrc/main/java/com/dreamteam/alter/application/user/usecase/UpdatePassword.javasrc/main/java/com/dreamteam/alter/common/exception/ErrorCode.javasrc/main/java/com/dreamteam/alter/domain/user/entity/User.javasrc/main/java/com/dreamteam/alter/domain/user/port/inbound/CreateUserWithSocialUseCase.javasrc/main/java/com/dreamteam/alter/domain/user/port/inbound/UnlinkSocialAccountUseCase.javasrc/main/java/com/dreamteam/alter/domain/user/port/inbound/UpdatePasswordUseCase.javasrc/main/java/com/dreamteam/alter/domain/user/port/outbound/UserSocialQueryRepository.javasrc/main/java/com/dreamteam/alter/domain/user/port/outbound/UserSocialRepository.javasrc/test/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocialTests.javasrc/test/java/com/dreamteam/alter/application/user/usecase/UnlinkSocialAccountTests.javasrc/test/java/com/dreamteam/alter/application/user/usecase/UpdatePasswordTests.java
Comment on lines
+38
to
+40
| @DeleteMapping("/unlink") | ||
| public ResponseEntity<CommonApiResponse<Void>> unlinkSocialAccount( | ||
| @Valid @RequestBody UnlinkSocialAccountRequestDto request |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the UserSocialController.java file
find . -type f -name "UserSocialController.java" | head -5Repository: alter-app/alter-backend
Length of output: 168
🏁 Script executed:
# Read the UserSocialController.java file to see the implementation
cat -n ./src/main/java/com/dreamteam/alter/adapter/inbound/general/user/controller/UserSocialController.javaRepository: alter-app/alter-backend
Length of output: 2310
🏁 Script executed:
# Check the UnlinkSocialAccountRequestDto structure
find . -type f -name "UnlinkSocialAccountRequestDto.java" | head -1 | xargs cat -nRepository: alter-app/alter-backend
Length of output: 758
🏁 Script executed:
# Search for other DELETE endpoints in the codebase to see the pattern
rg -t java "@DeleteMapping" -A 3 | head -30Repository: alter-app/alter-backend
Length of output: 3594
🏁 Script executed:
# Let's check if there are any tests or documentation about the unlink endpoint
rg -t java "unlinkSocialAccount" -B 2 -A 5Repository: alter-app/alter-backend
Length of output: 10251
🏁 Script executed:
# Check the UserSocialControllerSpec interface for Swagger documentation
cat -n ./src/main/java/com/dreamteam/alter/adapter/inbound/general/user/controller/UserSocialControllerSpec.javaRepository: alter-app/alter-backend
Length of output: 3574
DELETE 요청본문은 API Gateway 및 프록시 호환성 문제를 유발합니다.
@DeleteMapping("/unlink")에서 @RequestBody를 사용하면 일부 API Gateway, 프록시, 클라이언트에서 요청본문이 유실될 수 있습니다. HTTP 표준 관례상 DELETE는 본문을 포함하지 않도록 설계되어 있으며, 많은 중간 계층이 DELETE 본문을 무시하거나 제거합니다. provider를 쿼리 파라미터로 변경하는 것을 권장합니다:
권장 변경
`@DeleteMapping`("/unlink")
public ResponseEntity<CommonApiResponse<Void>> unlinkSocialAccount(
`@RequestParam` SocialProvider provider
) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/general/user/controller/UserSocialController.java`
around lines 38 - 40, The DELETE handler unlinkSocialAccount in
UserSocialController currently accepts a request body
(UnlinkSocialAccountRequestDto), which can be dropped by gateways/proxies;
change the API to accept the provider as a query param instead: replace the
`@RequestBody` UnlinkSocialAccountRequestDto parameter with a `@RequestParam`
SocialProvider provider in the unlinkSocialAccount method signature and update
any call sites and service-layer calls that read from
UnlinkSocialAccountRequestDto to use the provider parameter (and remove or adapt
the DTO) so the controller no longer depends on a DELETE body.
Comment on lines
+30
to
+39
| @Valid | ||
| @Schema(description = "OAuth 토큰") | ||
| private OauthLoginTokenDto oauthToken; | ||
|
|
||
| @Schema(description = "OAuth 인가 코드", example = "authorizationCode") | ||
| private String authorizationCode; | ||
|
|
||
| @NotNull | ||
| @Schema(description = "플랫폼 타입", example = "WEB / NATIVE") | ||
| private PlatformType platformType; |
There was a problem hiding this comment.
플랫폼별 필수 입력 조합 검증이 빠져 있습니다.
Line 30~39 기준으로 oauthToken과 authorizationCode가 동시에 비어도 요청이 통과합니다. 이 경우 소셜 인증 단계에서 모호한 실패(또는 내부 예외)로 이어질 수 있으니, platformType별 필수 필드 검증을 명시적으로 추가해주세요.
제안 수정안 (유스케이스 방어 검증)
@@
+if (request.getPlatformType() == PlatformType.NATIVE && request.getOauthToken() == null) {
+ throw new CustomException(ErrorCode.INVALID_PARAMETER);
+}
+
+if (request.getPlatformType() == PlatformType.WEB &&
+ (request.getAuthorizationCode() == null || request.getAuthorizationCode().isBlank())) {
+ throw new CustomException(ErrorCode.INVALID_PARAMETER);
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/general/user/dto/CreateUserWithSocialRequestDto.java`
around lines 30 - 39, CreateUserWithSocialRequestDto currently allows both
oauthToken and authorizationCode to be empty; add platform-specific
required-field validation by implementing a custom bean validation on the DTO
(e.g., an `@AssertTrue` boolean method or a custom ConstraintValidator). Inside
the validator method (in CreateUserWithSocialRequestDto) check platformType
(PlatformType) and enforce the required combination: for WEB require
authorizationCode non-empty, for NATIVE require oauthToken non-null (and
validate its inner fields), and for any other PlatformType enforce the
appropriate rule or a clear failure message; return false (or raise a constraint
violation) with a helpful message referencing oauthToken/authorizationCode when
the rule fails. Ensure validation annotations remain on
oauthToken/authorizationCode and that the error message is meaningful for
clients.
Comment on lines
+14
to
+22
| public class UpdatePasswordRequestDto { | ||
|
|
||
| @Schema(description = "현재 비밀번호 (비밀번호가 설정된 사용자는 필수, 소셜 전용 사용자는 생략 가능)", example = "currentPass1!") | ||
| private String currentPassword; | ||
|
|
||
| @NotBlank | ||
| @Size(min = 8, max = 16) | ||
| @Schema(description = "새 비밀번호 (8~16자, 영문·숫자·특수문자 각 1개 이상)", example = "newPass1!") | ||
| private String newPassword; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
새 비밀번호가 현재 비밀번호와 동일한지 검증 누락
UpdatePassword.java의 구현을 보면, newPassword가 currentPassword와 동일한 경우에 대한 검증이 없습니다. 보안 모범 사례상 비밀번호 변경 시 이전 비밀번호와 동일한 값 설정을 방지하는 것이 권장됩니다.
♻️ UpdatePassword.java에서 동일 비밀번호 검증 추가 제안
if (!PasswordValidator.isValid(request.getNewPassword())) {
throw new CustomException(ErrorCode.INVALID_PASSWORD_FORMAT);
}
+if (ObjectUtils.isNotEmpty(user.getPassword()) &&
+ passwordEncoder.matches(request.getNewPassword(), user.getPassword())) {
+ throw new CustomException(ErrorCode.SAME_AS_CURRENT_PASSWORD); // 새 에러 코드 필요
+}
user.updatePassword(passwordEncoder.encode(request.getNewPassword()));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/general/user/dto/UpdatePasswordRequestDto.java`
around lines 14 - 22, The DTO lacks validation preventing the new password from
matching the current one; update the validation so UpdatePasswordRequestDto
enforces newPassword != currentPassword (or add the check in the UpdatePassword
handling logic) by adding a cross-field validation: implement a custom
constraint or an `@AssertTrue` method on UpdatePasswordRequestDto that compares
currentPassword and newPassword and fails when they are equal, and ensure the
constraint/message is used by the controller/service that invokes UpdatePassword
so identical passwords are rejected.
Comment on lines
+22
to
+30
| public Optional<UserSocial> findByUserIdAndSocialProvider(Long userId, SocialProvider socialProvider) { | ||
| QUserSocial qUserSocial = QUserSocial.userSocial; | ||
|
|
||
| UserSocial userSocial = queryFactory.selectFrom(qUserSocial) | ||
| .where( | ||
| qUserSocial.user.id.eq(userId), | ||
| qUserSocial.socialProvider.eq(socialProvider) | ||
| ) | ||
| .fetchOne(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[1] 문제 구간의 fetchOne 사용 확인"
rg -n --type java 'findByUserIdAndSocialProvider|fetchOne\\(' src/main/java/com/dreamteam/alter/adapter/outbound/user/persistence/UserSocialQueryRepositoryImpl.java
echo
echo "[2] UserSocial 엔티티의 유니크 제약 정의 확인"
rg -n --type java '@Table|uniqueConstraints|social_provider|user_id' src/main/java/com/dreamteam/alter/domain/user/entity/UserSocial.java
echo
echo "[3] SQL 마이그레이션에서 user_socials 복합 유니크 인덱스 존재 여부 확인"
rg -n --iglob '*.sql' 'user_socials|unique|social_provider|user_id'Repository: alter-app/alter-backend
Length of output: 242
🏁 Script executed:
# First, verify the file exists and check its content around lines 22-30
head -35 src/main/java/com/dreamteam/alter/adapter/outbound/user/persistence/UserSocialQueryRepositoryImpl.java | tail -20Repository: alter-app/alter-backend
Length of output: 712
🏁 Script executed:
# Check if UserSocial entity exists and examine its structure
find . -name "UserSocial.java" -type fRepository: alter-app/alter-backend
Length of output: 137
🏁 Script executed:
# Search for SQL migrations related to user_socials table
find . -name "*.sql" -type f -exec grep -l "user_social" {} \;Repository: alter-app/alter-backend
Length of output: 49
🏁 Script executed:
# Examine the UserSocial entity for unique constraints
cat src/main/java/com/dreamteam/alter/domain/user/entity/UserSocial.javaRepository: alter-app/alter-backend
Length of output: 1769
🏁 Script executed:
# Search more broadly for migration files
find . -type f \( -name "*.sql" -o -name "*migration*" -o -name "*V[0-9]*" \) | head -20Repository: alter-app/alter-backend
Length of output: 49
🏁 Script executed:
# Also check for any Flyway or Liquibase migration directories
find . -type d \( -name "db" -o -name "migration" -o -name "flyway" \) 2>/dev/nullRepository: alter-app/alter-backend
Length of output: 49
🏁 Script executed:
# Search more thoroughly for SQL files in common migration locations
find . -type f -name "*.sql" 2>/dev/null | head -30Repository: alter-app/alter-backend
Length of output: 49
🏁 Script executed:
# Also check the root and common db paths
ls -la db/ 2>/dev/null || ls -la src/main/resources/db/ 2>/dev/null || echo "No standard db directories found"Repository: alter-app/alter-backend
Length of output: 99
🏁 Script executed:
# Search for any references to unique constraints in SQL files
rg -i "unique|constraint" --type sqlRepository: alter-app/alter-backend
Length of output: 215
🏁 Script executed:
# Check if there are Flyway migrations in src/main/resources
find src/main/resources -name "*.sql" 2>/dev/null | head -20Repository: alter-app/alter-backend
Length of output: 49
🏁 Script executed:
# Look for any other potential constraint definitions - check if JPA has uniqueConstraints
rg "uniqueConstraints" --type javaRepository: alter-app/alter-backend
Length of output: 443
🏁 Script executed:
# Check for any recent changes or documentation about user_socials constraints
rg -i "user.social|social.provider" --type java -A 3 -B 3 | grep -i "unique\|constraint" | head -20Repository: alter-app/alter-backend
Length of output: 228
🏁 Script executed:
# Verify if multiple UserSocial records could exist with same user_id and social_provider
# by checking insert/create operations
rg "UserSocial.create|\.builder\(\)" --type java -A 5 | head -50Repository: alter-app/alter-backend
Length of output: 4570
fetchOne() 사용으로 중복 데이터 시 런타임 실패 위험이 있습니다.
UserSocial 엔티티에 (user_id, social_provider) 복합 유니크 제약이 없어, fetchOne()이 다건 데이터를 반환하면 예외가 발생합니다. 현재 엔티티는 social_id에만 unique = true가 적용되어 있고, @Table 어노테이션에 uniqueConstraints가 없습니다. 유사하게 정의된 UserFavoritePosting처럼 uniqueConstraints = @UniqueConstraint(columnNames = {"user_id", "social_provider"})를 추가하거나, 방어 로직으로 fetchFirst() 또는 fetch().stream().findFirst()로 변경하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/dreamteam/alter/adapter/outbound/user/persistence/UserSocialQueryRepositoryImpl.java`
around lines 22 - 30, The current implementation in
UserSocialQueryRepositoryImpl::findByUserIdAndSocialProvider uses
queryFactory.selectFrom(QUserSocial.userSocial)...fetchOne(), which will throw
at runtime if multiple rows exist because (user_id, social_provider) is not
constrained as unique; either add a DB/entity-level unique constraint for
(user_id, social_provider) on the UserSocial entity (e.g., via
`@Table`(uniqueConstraints=...)) to guarantee single result, or change the
repository query to a defensive read (e.g., use fetchFirst() or use
fetch().stream().findFirst()) so the method returns the first match without
throwing; update the code in the findByUserIdAndSocialProvider method
accordingly and ensure tests cover duplicate-row behavior.
Comment on lines
+24
to
+25
| import org.springframework.data.redis.core.StringRedisTemplate; | ||
| import org.springframework.stereotype.Service; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n src/main/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocial.javaRepository: alter-app/alter-backend
Length of output: 6315
애플리케이션 계층에서 StringRedisTemplate 직접 의존을 제거하고 아웃바운드 포트로 추상화해야 합니다.
Line 24, Line 42: 유스케이스가 Spring Data Redis 인프라에 직접 결합되어 있습니다. SignupSessionPort 같은 아웃바운드 포트로 추상화하여 애플리케이션 계층의 순수성을 유지해 주세요.
또한 Line 101-102의 Redis 삭제 작업이 DB 트랜잭션 커밋 전에 수행되므로, DB 저장 실패 시 Redis와 DB 상태가 불일치할 수 있습니다. 트랜잭션 동기화 메커니즘(TransactionSynchronization)을 통해 DB 커밋 후에 Redis 정리가 이루어지도록 개선이 필요합니다.
코딩 가이드라인: "No direct infrastructure dependencies (no Spring Data, no HTTP clients)."
Also applies to: 42-43, 113-114, 120-121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocial.java`
around lines 24 - 25, The use case CreateUserWithSocial currently depends on
Spring's StringRedisTemplate and performs Redis deletions before DB commit;
replace the direct dependency by introducing an outbound port interface
SignupSessionPort (e.g., method deleteSignupSession(String key) and any
read/write methods used) and inject that port into CreateUserWithSocial instead
of StringRedisTemplate, update all calls (lines referencing redisTemplate ops)
to call signupSessionPort methods, remove any Spring Data imports, and ensure
Redis cleanup runs only after successful DB commit by registering a transaction
synchronization (e.g., TransactionSynchronizationManager.registerSynchronization
with an afterCommit callback that calls
signupSessionPort.deleteSignupSession(sessionKey)) or using an after-commit
transactional event; update CreateUserWithSocial to use these symbols and remove
direct infra coupling.
Comment on lines
+93
to
+124
| private SocialAuthInfo createSocialAuthInfo() { | ||
| SocialAuthInfo authInfo = mock(SocialAuthInfo.class); | ||
| given(authInfo.getProvider()).willReturn(SocialProvider.KAKAO); | ||
| given(authInfo.getSocialId()).willReturn("kakao-social-id"); | ||
| given(authInfo.getEmail()).willReturn("social@example.com"); | ||
| given(authInfo.getRefreshToken()).willReturn("kakao-refresh-token"); | ||
| return authInfo; | ||
| } | ||
|
|
||
| private SocialAuthInfo createSocialAuthInfoForDuplicateSocialId() { | ||
| SocialAuthInfo authInfo = mock(SocialAuthInfo.class); | ||
| given(authInfo.getProvider()).willReturn(SocialProvider.KAKAO); | ||
| given(authInfo.getSocialId()).willReturn("kakao-social-id"); | ||
| return authInfo; | ||
| } | ||
|
|
||
| private SocialAuthInfo createSocialAuthInfoForDuplicateEmail() { | ||
| SocialAuthInfo authInfo = mock(SocialAuthInfo.class); | ||
| given(authInfo.getProvider()).willReturn(SocialProvider.KAKAO); | ||
| given(authInfo.getSocialId()).willReturn("kakao-social-id"); | ||
| given(authInfo.getEmail()).willReturn("social@example.com"); | ||
| return authInfo; | ||
| } | ||
|
|
||
| private SocialAuthInfo createSocialAuthInfoWithoutEmail() { | ||
| SocialAuthInfo authInfo = mock(SocialAuthInfo.class); | ||
| given(authInfo.getProvider()).willReturn(SocialProvider.KAKAO); | ||
| given(authInfo.getSocialId()).willReturn("kakao-social-id"); | ||
| given(authInfo.getEmail()).willReturn(null); | ||
| given(authInfo.getRefreshToken()).willReturn("kakao-refresh-token"); | ||
| return authInfo; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
SocialAuthInfo 테스트 헬퍼 중복을 하나로 합치면 유지보수가 쉬워집니다.
createSocialAuthInfo* 4개가 거의 동일한 mocking 코드를 반복합니다. 파라미터 기반 헬퍼 1개로 통합하는 편이 변경 내성이 높습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocialTests.java`
around lines 93 - 124, Replace the four near-identical helpers
(createSocialAuthInfo, createSocialAuthInfoForDuplicateSocialId,
createSocialAuthInfoForDuplicateEmail, createSocialAuthInfoWithoutEmail) with a
single parameterized factory that returns a mocked SocialAuthInfo; e.g., a
method like createSocialAuthInfo(String socialId, `@Nullable` String email,
`@Nullable` String refreshToken, SocialProvider provider) that mocks
SocialAuthInfo and conditionally stubs getProvider(), getSocialId(), getEmail(),
and getRefreshToken() only when non-null (use SocialProvider.KAKAO as default
where callers expect it); update tests to call this new helper and remove the
four old methods so maintenance and variations are centralized around one
function.
Comment on lines
+132
to
+286
| void fails_whenSignupSessionNotFound() { | ||
| // given | ||
| given(valueOperations.get("SIGNUP:PENDING:signup-session-id")).willReturn(null); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> createUserWithSocial.execute(request)) | ||
| .isInstanceOf(CustomException.class) | ||
| .satisfies(ex -> assertThat(((CustomException) ex).getErrorCode()) | ||
| .isEqualTo(ErrorCode.SIGNUP_SESSION_NOT_EXIST)); | ||
|
|
||
| then(userRepository).should(never()).save(any()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("닉네임이 중복될 경우 NICKNAME_DUPLICATED 예외 발생 및 Redis 세션 삭제") | ||
| void fails_whenNicknameDuplicated() { | ||
| // given | ||
| given(valueOperations.get("SIGNUP:PENDING:signup-session-id")).willReturn("01012345678"); | ||
| given(userQueryRepository.findByNickname("유땡땡")).willReturn(Optional.of(mock(User.class))); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> createUserWithSocial.execute(request)) | ||
| .isInstanceOf(CustomException.class) | ||
| .satisfies(ex -> assertThat(((CustomException) ex).getErrorCode()) | ||
| .isEqualTo(ErrorCode.NICKNAME_DUPLICATED)); | ||
|
|
||
| then(redisTemplate).should().delete("SIGNUP:PENDING:signup-session-id"); | ||
| then(redisTemplate).should().delete("SIGNUP:CONTACT:01012345678"); | ||
| then(userRepository).should(never()).save(any()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("연락처가 중복될 경우 USER_CONTACT_DUPLICATED 예외 발생 및 Redis 세션 삭제") | ||
| void fails_whenContactDuplicated() { | ||
| // given | ||
| given(valueOperations.get("SIGNUP:PENDING:signup-session-id")).willReturn("01012345678"); | ||
| given(userQueryRepository.findByNickname("유땡땡")).willReturn(Optional.empty()); | ||
| given(userQueryRepository.findByContact("01012345678")).willReturn(Optional.of(mock(User.class))); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> createUserWithSocial.execute(request)) | ||
| .isInstanceOf(CustomException.class) | ||
| .satisfies(ex -> assertThat(((CustomException) ex).getErrorCode()) | ||
| .isEqualTo(ErrorCode.USER_CONTACT_DUPLICATED)); | ||
|
|
||
| then(redisTemplate).should().delete("SIGNUP:PENDING:signup-session-id"); | ||
| then(redisTemplate).should().delete("SIGNUP:CONTACT:01012345678"); | ||
| then(userRepository).should(never()).save(any()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("이미 등록된 소셜 계정일 경우 SOCIAL_ID_DUPLICATED 예외 발생") | ||
| void fails_whenSocialIdAlreadyRegistered() { | ||
| // given | ||
| given(valueOperations.get("SIGNUP:PENDING:signup-session-id")).willReturn("01012345678"); | ||
| given(userQueryRepository.findByNickname("유땡땡")).willReturn(Optional.empty()); | ||
| given(userQueryRepository.findByContact("01012345678")).willReturn(Optional.empty()); | ||
|
|
||
| SocialAuthInfo authInfo = createSocialAuthInfoForDuplicateSocialId(); | ||
| given(socialAuthenticationManager.authenticate(any())).willReturn(authInfo); | ||
| given(userSocialQueryRepository.existsBySocialProviderAndSocialId(SocialProvider.KAKAO, "kakao-social-id")) | ||
| .willReturn(true); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> createUserWithSocial.execute(request)) | ||
| .isInstanceOf(CustomException.class) | ||
| .satisfies(ex -> assertThat(((CustomException) ex).getErrorCode()) | ||
| .isEqualTo(ErrorCode.SOCIAL_ID_DUPLICATED)); | ||
|
|
||
| then(userRepository).should(never()).save(any()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("소셜 계정 이메일이 이미 가입된 경우 EMAIL_DUPLICATED 예외 발생") | ||
| void fails_whenSocialEmailAlreadyExists() { | ||
| // given | ||
| given(valueOperations.get("SIGNUP:PENDING:signup-session-id")).willReturn("01012345678"); | ||
| given(userQueryRepository.findByNickname("유땡땡")).willReturn(Optional.empty()); | ||
| given(userQueryRepository.findByContact("01012345678")).willReturn(Optional.empty()); | ||
|
|
||
| SocialAuthInfo authInfo = createSocialAuthInfoForDuplicateEmail(); | ||
| given(socialAuthenticationManager.authenticate(any())).willReturn(authInfo); | ||
| given(userSocialQueryRepository.existsBySocialProviderAndSocialId(SocialProvider.KAKAO, "kakao-social-id")) | ||
| .willReturn(false); | ||
| given(userQueryRepository.findByEmail("social@example.com")).willReturn(Optional.of(mock(User.class))); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> createUserWithSocial.execute(request)) | ||
| .isInstanceOf(CustomException.class) | ||
| .satisfies(ex -> assertThat(((CustomException) ex).getErrorCode()) | ||
| .isEqualTo(ErrorCode.EMAIL_DUPLICATED)); | ||
|
|
||
| then(userRepository).should(never()).save(any()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("유효한 입력으로 소셜 회원가입 성공 - 소셜 계정 이메일 자동 저장") | ||
| void succeeds_withValidSocialSignup() { | ||
| // given | ||
| given(valueOperations.get("SIGNUP:PENDING:signup-session-id")).willReturn("01012345678"); | ||
| given(userQueryRepository.findByNickname("유땡땡")).willReturn(Optional.empty()); | ||
| given(userQueryRepository.findByContact("01012345678")).willReturn(Optional.empty()); | ||
|
|
||
| SocialAuthInfo authInfo = createSocialAuthInfo(); | ||
| given(socialAuthenticationManager.authenticate(any())).willReturn(authInfo); | ||
| given(userSocialQueryRepository.existsBySocialProviderAndSocialId(SocialProvider.KAKAO, "kakao-social-id")) | ||
| .willReturn(false); | ||
| given(userQueryRepository.findByEmail("social@example.com")).willReturn(Optional.empty()); | ||
|
|
||
| User savedUser = mock(User.class); | ||
| given(userRepository.save(any(User.class))).willReturn(savedUser); | ||
|
|
||
| Authorization authorization = mock(Authorization.class); | ||
| given(authService.generateAuthorization(eq(savedUser), eq(TokenScope.APP))).willReturn(authorization); | ||
|
|
||
| // when | ||
| GenerateTokenResponseDto result = createUserWithSocial.execute(request); | ||
|
|
||
| // then | ||
| assertThat(result).isNotNull(); | ||
| then(userRepository).should().save(any(User.class)); | ||
| then(userQueryRepository).should().findByEmail("social@example.com"); | ||
| then(authService).should().generateAuthorization(savedUser, TokenScope.APP); | ||
| then(authLogRepository).should().save(any()); | ||
| then(redisTemplate).should().delete("SIGNUP:PENDING:signup-session-id"); | ||
| then(redisTemplate).should().delete("SIGNUP:CONTACT:01012345678"); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("소셜 계정에 이메일이 없을 경우 이메일 없이 회원가입 성공") | ||
| void succeeds_withNoEmailFromSocialAccount() { | ||
| // given | ||
| given(valueOperations.get("SIGNUP:PENDING:signup-session-id")).willReturn("01012345678"); | ||
| given(userQueryRepository.findByNickname("유땡땡")).willReturn(Optional.empty()); | ||
| given(userQueryRepository.findByContact("01012345678")).willReturn(Optional.empty()); | ||
|
|
||
| SocialAuthInfo authInfo = createSocialAuthInfoWithoutEmail(); | ||
| given(socialAuthenticationManager.authenticate(any())).willReturn(authInfo); | ||
| given(userSocialQueryRepository.existsBySocialProviderAndSocialId(SocialProvider.KAKAO, "kakao-social-id")) | ||
| .willReturn(false); | ||
|
|
||
| User savedUser = mock(User.class); | ||
| given(userRepository.save(any(User.class))).willReturn(savedUser); | ||
|
|
||
| Authorization authorization = mock(Authorization.class); | ||
| given(authService.generateAuthorization(eq(savedUser), eq(TokenScope.APP))).willReturn(authorization); | ||
|
|
||
| // when | ||
| GenerateTokenResponseDto result = createUserWithSocial.execute(request); | ||
|
|
||
| // then | ||
| assertThat(result).isNotNull(); | ||
| then(userRepository).should().save(any(User.class)); | ||
| then(userQueryRepository).should(never()).findByEmail(any()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
테스트 메서드명을 action_condition_expectedResult 패턴으로 통일해 주세요.
fails_when..., succeeds_with... 형태는 의미 전달은 되지만, 저장소 규칙의 명시 패턴과는 다릅니다. 일관된 네이밍으로 변경하는 것이 좋습니다.
수정 예시
-void fails_whenSignupSessionNotFound() {
+void execute_signupSessionMissing_throwsSignupSessionNotExist() {
-void succeeds_withNoEmailFromSocialAccount() {
+void execute_socialEmailMissing_createsUserWithoutEmail() {As per coding guidelines, src/test/**: Test methods use descriptive names: action_condition_expectedResult.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocialTests.java`
around lines 132 - 286, Rename the test methods to follow the
action_condition_expectedResult naming convention instead of the current
fails_when... and succeeds_with... forms: update methods like
fails_whenSignupSessionNotFound, fails_whenNicknameDuplicated,
fails_whenContactDuplicated, fails_whenSocialIdAlreadyRegistered,
fails_whenSocialEmailAlreadyExists, succeeds_withValidSocialSignup, and
succeeds_withNoEmailFromSocialAccount to names that start with the action (e.g.,
createUserWithSocial), then the condition, then the expected result (e.g.,
createUserWithSocial_signupSessionNotFound_throwsSignUpSessionNotExist); keep or
adjust `@DisplayName` as needed and update any direct references to these method
names in the test class (e.g., in CreateUserWithSocialTests) so tests compile
and run.
Comment on lines
+182
to
+225
| @Test | ||
| @DisplayName("이미 등록된 소셜 계정일 경우 SOCIAL_ID_DUPLICATED 예외 발생") | ||
| void fails_whenSocialIdAlreadyRegistered() { | ||
| // given | ||
| given(valueOperations.get("SIGNUP:PENDING:signup-session-id")).willReturn("01012345678"); | ||
| given(userQueryRepository.findByNickname("유땡땡")).willReturn(Optional.empty()); | ||
| given(userQueryRepository.findByContact("01012345678")).willReturn(Optional.empty()); | ||
|
|
||
| SocialAuthInfo authInfo = createSocialAuthInfoForDuplicateSocialId(); | ||
| given(socialAuthenticationManager.authenticate(any())).willReturn(authInfo); | ||
| given(userSocialQueryRepository.existsBySocialProviderAndSocialId(SocialProvider.KAKAO, "kakao-social-id")) | ||
| .willReturn(true); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> createUserWithSocial.execute(request)) | ||
| .isInstanceOf(CustomException.class) | ||
| .satisfies(ex -> assertThat(((CustomException) ex).getErrorCode()) | ||
| .isEqualTo(ErrorCode.SOCIAL_ID_DUPLICATED)); | ||
|
|
||
| then(userRepository).should(never()).save(any()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("소셜 계정 이메일이 이미 가입된 경우 EMAIL_DUPLICATED 예외 발생") | ||
| void fails_whenSocialEmailAlreadyExists() { | ||
| // given | ||
| given(valueOperations.get("SIGNUP:PENDING:signup-session-id")).willReturn("01012345678"); | ||
| given(userQueryRepository.findByNickname("유땡땡")).willReturn(Optional.empty()); | ||
| given(userQueryRepository.findByContact("01012345678")).willReturn(Optional.empty()); | ||
|
|
||
| SocialAuthInfo authInfo = createSocialAuthInfoForDuplicateEmail(); | ||
| given(socialAuthenticationManager.authenticate(any())).willReturn(authInfo); | ||
| given(userSocialQueryRepository.existsBySocialProviderAndSocialId(SocialProvider.KAKAO, "kakao-social-id")) | ||
| .willReturn(false); | ||
| given(userQueryRepository.findByEmail("social@example.com")).willReturn(Optional.of(mock(User.class))); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> createUserWithSocial.execute(request)) | ||
| .isInstanceOf(CustomException.class) | ||
| .satisfies(ex -> assertThat(((CustomException) ex).getErrorCode()) | ||
| .isEqualTo(ErrorCode.EMAIL_DUPLICATED)); | ||
|
|
||
| then(userRepository).should(never()).save(any()); | ||
| } |
There was a problem hiding this comment.
SOCIAL_ID_DUPLICATED/EMAIL_DUPLICATED 실패 경로의 Redis 정리 검증이 빠져 있습니다.
현재 두 테스트는 예외와 save 미호출만 검증하고, 세션 키 삭제 여부는 검증하지 않습니다. 이 상태에선 해당 실패 경로의 세션 정리 회귀를 잡지 못합니다.
수정 제안 (검증 추가)
@@
void fails_whenSocialIdAlreadyRegistered() {
@@
then(userRepository).should(never()).save(any());
+ then(redisTemplate).should().delete("SIGNUP:PENDING:signup-session-id");
+ then(redisTemplate).should().delete("SIGNUP:CONTACT:01012345678");
}
@@
void fails_whenSocialEmailAlreadyExists() {
@@
then(userRepository).should(never()).save(any());
+ then(redisTemplate).should().delete("SIGNUP:PENDING:signup-session-id");
+ then(redisTemplate).should().delete("SIGNUP:CONTACT:01012345678");
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/dreamteam/alter/application/user/usecase/CreateUserWithSocialTests.java`
around lines 182 - 225, The tests for SOCIAL_ID_DUPLICATED and EMAIL_DUPLICATED
need to also assert that the signup session key is cleaned up from Redis after
the failure; after invoking createUserWithSocial.execute(request) add
verification that the Redis deletion call for "SIGNUP:PENDING:signup-session-id"
was invoked (match the same Redis collaborator used in the test setup — e.g.,
verify(redisTemplate).delete("SIGNUP:PENDING:signup-session-id") or
verify(yourRedisClient).remove(...) depending on implementation) in both failing
tests, and keep the existing assertion that userRepository.save is never called.
Comment on lines
+83
to
+97
| @Test | ||
| @DisplayName("연동된 소셜 계정을 정상 해제한다") | ||
| void execute_withLinkedSocial_succeeds() { | ||
| // when & then | ||
| assertThatNoException().isThrownBy(() -> unlinkSocialAccount.execute(actor, request)); | ||
| then(userSocialRepository).should().delete(userSocial); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("소셜 계정이 1개뿐이어도 해제 가능하다 (제약은 비밀번호 없는 사용자에만 적용)") | ||
| void execute_withSingleSocial_succeeds() { | ||
| // when & then | ||
| assertThatNoException().isThrownBy(() -> unlinkSocialAccount.execute(actor, request)); | ||
| then(userSocialRepository).should().delete(userSocial); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
동일 시나리오 테스트가 중복됩니다.
Line [83]-Line [89]와 Line [91]-Line [97]는 현재 관찰 가능한 행위가 동일합니다. 하나로 합치거나, 두 번째 케이스에 별도 전제/검증(예: count 조회 미호출 검증)을 추가해 의도를 분리해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/dreamteam/alter/application/user/usecase/UnlinkSocialAccountTests.java`
around lines 83 - 97, The two tests execute_withLinkedSocial_succeeds and
execute_withSingleSocial_succeeds duplicate the same observable behavior;
consolidate or differentiate them: either remove one and keep a single test that
asserts unlinkSocialAccount.execute(actor, request) does not throw and that
userSocialRepository.delete(userSocial) is invoked, or update
execute_withSingleSocial_succeeds to set up the "single social" precondition and
add an extra verification (e.g., verify that userRepository.countSocialsByUserId
or equivalent is not called/was called as intended) to demonstrate the
special-case behavior; locate the test methods and the calls to
unlinkSocialAccount.execute and userSocialRepository.delete to implement the
change.
Comment on lines
+75
to
+79
| assertThatThrownBy(() -> updatePassword.execute(actor, request)) | ||
| .isInstanceOf(CustomException.class) | ||
| .hasFieldOrPropertyWithValue("errorCode", ErrorCode.INVALID_CURRENT_PASSWORD); | ||
| then(user).shouldHaveNoMoreInteractions(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /root/repo && find . -type f -name "UpdatePasswordTests.java" 2>/dev/null | head -5Repository: alter-app/alter-backend
Length of output: 127
🏁 Script executed:
cd /root/repo && git ls-files | grep -i "UpdatePasswordTests" | head -5Repository: alter-app/alter-backend
Length of output: 127
🏁 Script executed:
git ls-files | grep -i "UpdatePasswordTests"Repository: alter-app/alter-backend
Length of output: 150
🏁 Script executed:
find . -type f -name "*UpdatePasswordTests*" 2>/dev/nullRepository: alter-app/alter-backend
Length of output: 152
🏁 Script executed:
wc -l src/test/java/com/dreamteam/alter/application/user/usecase/UpdatePasswordTests.javaRepository: alter-app/alter-backend
Length of output: 154
🏁 Script executed:
cat -n src/test/java/com/dreamteam/alter/application/user/usecase/UpdatePasswordTests.javaRepository: alter-app/alter-backend
Length of output: 6872
🏁 Script executed:
find . -type f -name "UpdatePassword.java" | grep -v TestRepository: alter-app/alter-backend
Length of output: 147
🏁 Script executed:
git ls-files | grep -i "UpdatePassword" | grep -v TestRepository: alter-app/alter-backend
Length of output: 328
🏁 Script executed:
cat -n src/main/java/com/dreamteam/alter/application/user/usecase/UpdatePassword.javaRepository: alter-app/alter-backend
Length of output: 1987
shouldHaveNoMoreInteractions() 제거 필요 - 테스트 실패 위험
라인 78, 91, 105, 141에서 then(user).shouldHaveNoMoreInteractions()를 사용하면 SUT 내부의 user.getPassword() 호출(라인 27)이 미검증 상호작용으로 남아 테스트가 실패합니다. 이 예외 케이스들에서는 핵심 검증(비밀번호 업데이트 미호출)만 필요합니다.
✅ 제안 수정
- then(user).shouldHaveNoMoreInteractions();
+ then(user).should(org.mockito.Mockito.never()).updatePassword(anyString());📝 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
| assertThatThrownBy(() -> updatePassword.execute(actor, request)) | |
| .isInstanceOf(CustomException.class) | |
| .hasFieldOrPropertyWithValue("errorCode", ErrorCode.INVALID_CURRENT_PASSWORD); | |
| then(user).shouldHaveNoMoreInteractions(); | |
| } | |
| assertThatThrownBy(() -> updatePassword.execute(actor, request)) | |
| .isInstanceOf(CustomException.class) | |
| .hasFieldOrPropertyWithValue("errorCode", ErrorCode.INVALID_CURRENT_PASSWORD); | |
| then(user).should(org.mockito.Mockito.never()).updatePassword(anyString()); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/dreamteam/alter/application/user/usecase/UpdatePasswordTests.java`
around lines 75 - 79, Remove the fragile verification that all interactions on
the mocked user are exhausted in these negative tests: delete the
then(user).shouldHaveNoMoreInteractions() assertions (seen at the end of the
failing tests) and keep only the essential assertions that the use case throws
CustomException with ErrorCode.INVALID_CURRENT_PASSWORD and that no
updatePasswordRepository.save/update method was called; this avoids failing the
test due to the legitimate user.getPassword() call inside updatePassword.execute
while still verifying that password update logic was not invoked.
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.
관련 문서
https://www.notion.so/BE-33d86553162880019459d27e67fbd09d?source=copy_link
Summary by CodeRabbit
릴리스 노트
New Features
Bug Fixes & Improvements