-
Notifications
You must be signed in to change notification settings - Fork 0
[FIX/#110] 회원탈퇴 중복 요청 예외 처리 적용 #112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,7 @@ | |
| import org.sopt.routee.member.internal.service.validator.ProfileImageFileNameValidator; | ||
| import org.sopt.routee.util.TimeZoneUtils; | ||
| import org.springframework.context.ApplicationEventPublisher; | ||
| import org.springframework.orm.ObjectOptimisticLockingFailureException; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
| import org.springframework.transaction.support.TransactionTemplate; | ||
|
|
@@ -109,17 +110,21 @@ private void validateRequiredAgreements(AgreementCommand agreement) { | |
| } | ||
|
|
||
| public void withdraw(long memberId, String accessTokenHash, String refreshTokenHash) { | ||
| transactionTemplate.executeWithoutResult(status -> { | ||
| Member member = memberRepository.findById(memberId) | ||
| .orElseThrow(MemberNotFoundException::new); | ||
| try { | ||
| transactionTemplate.executeWithoutResult(status -> { | ||
| Member member = memberRepository.findById(memberId) | ||
| .orElseThrow(MemberNotFoundException::new); | ||
|
|
||
| memberAgreementRepository.deleteByMember_Id(memberId); | ||
| memberRepository.delete(member); | ||
| memberAgreementRepository.deleteByMember_Id(memberId); | ||
| memberRepository.delete(member); | ||
|
|
||
| activityUseCase.deleteForMemberWithdrawal(memberId); | ||
| activityUseCase.deleteForMemberWithdrawal(memberId); | ||
| }); | ||
| } catch (ObjectOptimisticLockingFailureException e) { | ||
| throw new MemberNotFoundException(); | ||
| } | ||
|
|
||
| applicationEventPublisher.publishEvent(new MemberWithdrawnEvent(memberId, accessTokenHash, refreshTokenHash)); | ||
| }); | ||
| applicationEventPublisher.publishEvent(new MemberWithdrawnEvent(memberId, accessTokenHash, refreshTokenHash)); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: sed -n '1,180p' routee-member/src/main/java/org/sopt/routee/member/internal/service/MemberService.java
printf '\n--- listener ---\n'
sed -n '1,140p' routee-auth/src/main/java/org/sopt/routee/auth/internal/listener/AuthMemberEventListener.java
printf '\n--- event/config references ---\n'
rg -n --glob '*.{java,kt,yml,yaml,properties,gradle}' \
'MemberWithdrawnEvent|ApplicationModuleListener|TransactionalEventListener|EventPublicationRegistry|spring\.modulith|`@EnableAsync`|AsyncConfigurer|TaskExecutor|`@Async`' \
routee-member routee-auth routee-app build.gradle* settings.gradle* gradle.propertiesRepository: Team-Routee/Routee-Server Length of output: 11294 🏁 Script executed: printf '%s\n' '--- module listener ---'
sed -n '1,100p' routee-member/src/main/java/org/sopt/routee/member/internal/listener/MemberActivityEventListener.java
printf '%s\n' '--- build files ---'
find . -maxdepth 3 -type f \( -name 'build.gradle' -o -name 'build.gradle.kts' -o -name 'settings.gradle' -o -name 'settings.gradle.kts' \) -print
rg -n --glob 'build.gradle*' --glob 'settings.gradle*' \
'modulith|spring-modulith-events|jdbc|jpa|transaction|scheduling' .
printf '%s\n' '--- event and listener usages ---'
sed -n '1,80p' routee-member/src/main/java/org/sopt/routee/member/api/event/MemberWithdrawnEvent.java
rg -n --glob '*.java' 'MemberWithdrawnEvent|AuthMemberEventListener|revokeTokens' routee-*Repository: Team-Routee/Routee-Server Length of output: 5404 Broken Authentication (CWE-613): Insufficient Session Expiration Reachability: External · Exploitability: Difficult 회원 삭제와 토큰 무효화 사이에 내구성 있는 이벤트 경계를 유지하세요.
📍 Affects 2 files
🤖 Prompt for AI AgentsSource: MCP tools |
||
|
|
||
| Thread.startVirtualThread(() -> deleteMemberImages(memberId)); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| package org.sopt.routee.member.internal.service; | ||
|
|
||
| import static org.assertj.core.api.Assertions.*; | ||
| import static org.mockito.ArgumentMatchers.*; | ||
| import static org.mockito.Mockito.*; | ||
|
|
||
| import java.util.Optional; | ||
| import java.util.function.Consumer; | ||
|
|
||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.DisplayName; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.extension.ExtendWith; | ||
| import org.mockito.ArgumentCaptor; | ||
| import org.mockito.Mock; | ||
| import org.mockito.junit.jupiter.MockitoExtension; | ||
| import org.sopt.routee.activity.api.usecase.ActivityUseCase; | ||
| import org.sopt.routee.external.api.port.FileDeletePort; | ||
| import org.sopt.routee.external.api.port.FileImageAccessUrlPort; | ||
| import org.sopt.routee.external.api.port.FileUploadPresignPort; | ||
| import org.sopt.routee.external.api.port.OidcVerifyPort; | ||
| import org.sopt.routee.member.api.event.MemberWithdrawnEvent; | ||
| import org.sopt.routee.member.internal.entity.Member; | ||
| import org.sopt.routee.member.internal.exception.MemberNotFoundException; | ||
| import org.sopt.routee.member.internal.repository.MemberAgreementRepository; | ||
| import org.sopt.routee.member.internal.repository.MemberRepository; | ||
| import org.sopt.routee.member.internal.service.validator.ProfileImageFileNameValidator; | ||
| import org.springframework.context.ApplicationEventPublisher; | ||
| import org.springframework.orm.ObjectOptimisticLockingFailureException; | ||
| import org.springframework.transaction.TransactionStatus; | ||
| import org.springframework.transaction.support.TransactionTemplate; | ||
|
|
||
| @ExtendWith(MockitoExtension.class) | ||
| class MemberServiceTest { | ||
|
|
||
| private static final long MEMBER_ID = 1L; | ||
| private static final String ACCESS_TOKEN_HASH = "access-hash"; | ||
| private static final String REFRESH_TOKEN_HASH = "refresh-hash"; | ||
|
|
||
| @Mock | ||
| private OidcVerifyPort oidcVerifyPort; | ||
|
|
||
| @Mock | ||
| private ActivityUseCase activityUseCase; | ||
|
|
||
| @Mock | ||
| private MemberRepository memberRepository; | ||
|
|
||
| @Mock | ||
| private MemberAgreementRepository memberAgreementRepository; | ||
|
|
||
| @Mock | ||
| private ApplicationEventPublisher applicationEventPublisher; | ||
|
|
||
| @Mock | ||
| private FileUploadPresignPort fileUploadPresignPort; | ||
|
|
||
| @Mock | ||
| private FileImageAccessUrlPort fileImageAccessUrlPort; | ||
|
|
||
| @Mock | ||
| private FileDeletePort fileDeletePort; | ||
|
|
||
| @Mock | ||
| private ProfileImageFileNameValidator profileImageFileNameValidator; | ||
|
|
||
| @Mock | ||
| private TransactionTemplate transactionTemplate; | ||
|
|
||
| private MemberService memberService; | ||
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| memberService = new MemberService( | ||
| oidcVerifyPort, | ||
| activityUseCase, | ||
| memberRepository, | ||
| memberAgreementRepository, | ||
| applicationEventPublisher, | ||
| fileUploadPresignPort, | ||
| fileImageAccessUrlPort, | ||
| fileDeletePort, | ||
| profileImageFileNameValidator, | ||
| transactionTemplate | ||
| ); | ||
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| private void stubTransactionTemplateToRunCallback() { | ||
| doAnswer(invocation -> { | ||
| Consumer<TransactionStatus> callback = invocation.getArgument(0); | ||
| callback.accept(mock(TransactionStatus.class)); | ||
| return null; | ||
| }).when(transactionTemplate).executeWithoutResult(any(Consumer.class)); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("withdraw: 정상적으로 회원을 탈퇴시키고 토큰 무효화 이벤트를 발행한다") | ||
| void withdraw_success_publishesMemberWithdrawnEvent() { | ||
| // given | ||
| stubTransactionTemplateToRunCallback(); | ||
| Member member = mock(Member.class); | ||
| when(memberRepository.findById(MEMBER_ID)).thenReturn(Optional.of(member)); | ||
|
|
||
| // when | ||
| memberService.withdraw(MEMBER_ID, ACCESS_TOKEN_HASH, REFRESH_TOKEN_HASH); | ||
|
|
||
| // then | ||
| verify(memberAgreementRepository).deleteByMember_Id(MEMBER_ID); | ||
| verify(memberRepository).delete(member); | ||
| verify(activityUseCase).deleteForMemberWithdrawal(MEMBER_ID); | ||
|
|
||
| ArgumentCaptor<MemberWithdrawnEvent> eventCaptor = ArgumentCaptor.forClass(MemberWithdrawnEvent.class); | ||
| verify(applicationEventPublisher).publishEvent(eventCaptor.capture()); | ||
| assertThat(eventCaptor.getValue()) | ||
| .extracting( | ||
| MemberWithdrawnEvent::memberId, | ||
| MemberWithdrawnEvent::accessTokenHash, | ||
| MemberWithdrawnEvent::refreshTokenHash) | ||
| .containsExactly(MEMBER_ID, ACCESS_TOKEN_HASH, REFRESH_TOKEN_HASH); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("withdraw: 동시 탈퇴 요청으로 낙관적 락 예외가 발생하면 회원 없음 예외로 변환한다") | ||
| void withdraw_concurrentWithdrawal_throwsMemberNotFoundException() { | ||
| // given | ||
| stubTransactionTemplateToRunCallback(); | ||
| Member member = mock(Member.class); | ||
| when(memberRepository.findById(MEMBER_ID)).thenReturn(Optional.of(member)); | ||
| doThrow(new ObjectOptimisticLockingFailureException(Member.class, MEMBER_ID)) | ||
| .when(memberAgreementRepository).deleteByMember_Id(MEMBER_ID); | ||
|
Comment on lines
+130
to
+131
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🤖 get_repo_knowledge executed:
Length of output: 9319 🏁 Script executed: #!/bin/bash
set -e
file='routee-member/src/test/java/org/sopt/routee/member/internal/service/MemberServiceTest.java'
printf '%s\n' '--- changed test context ---'
sed -n '1,220p' "$file"
printf '%s\n' '--- relevant production symbols ---'
rg -n -C 5 'stubTransactionTemplateToRunCallback|TransactionTemplate|executeWithoutResult|deleteByMember_Id|ObjectOptimisticLockingFailureException|MemberService' routee-member/src/main routee-member/src/testRepository: Team-Routee/Routee-Server Length of output: 31510 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- member persistence definitions ---'
rg -n -C 5 '`@Version`|class Member|class MemberAgreement|interface MemberAgreementRepository|deleteByMember_Id' routee-member/src/main/java
printf '%s\n' '--- integration-test and database patterns ---'
rg -l 'DataJpaTest|SpringBootTest|TransactionTemplate|ObjectOptimisticLockingFailureException|`@Version`|Concurrent' --glob '*Test.java' --glob '*.gradle' .
printf '%s\n' '--- module test/build configuration ---'
fd -i -t f '(build.gradle|build.gradle.kts|gradle.properties|application.*|.*Test.*)' routee-member routee-app | sort | head -80Repository: Team-Routee/Routee-Server Length of output: 14774 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- version and entity mappings ---'
rg -n -C 8 '`@MappedSuperclass`|`@Version`|class BaseEntity|extends BaseEntity|`@OneToOne`|`@ManyToOne`' --glob '*.java' routee-common routee-member
printf '%s\n' '--- module and application test dependencies/configuration ---'
sed -n '1,220p' routee-member/build.gradle
sed -n '1,220p' routee-app/build.gradle
sed -n '1,220p' routee-app/src/main/resources/application.yml
printf '%s\n' '--- exact transaction API usage ---'
sed -n '105,135p' routee-member/src/main/java/org/sopt/routee/member/internal/service/MemberService.javaRepository: Team-Routee/Routee-Server Length of output: 11483 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- all optimistic-locking/version configuration ---'
rg -n -i '`@version`|optimistic|stale|lockmode|lock-mode|hibernate\.jdbc|hibernate\.event' --glob '*.java' --glob '*.yml' --glob '*.yaml' --glob '*.properties' --glob '*.gradle' .
printf '%s\n' '--- complete entity base and withdrawal mappings ---'
cat -n routee-common/src/main/java/org/sopt/routee/entity/BaseEntity.java
cat -n routee-member/src/main/java/org/sopt/routee/member/internal/entity/Member.java | sed -n '1,130p'
cat -n routee-member/src/main/java/org/sopt/routee/member/internal/entity/MemberAgreement.java | sed -n '1,100p'Repository: Team-Routee/Routee-Server Length of output: 6605 🌐 Web query:
💡 Result: In Jakarta Persistence, optimistic locking is primarily implemented using the Citations:
트랜잭션 종료 단계의 예외 경로를 검증하세요. 현재
🤖 Prompt for AI AgentsSource: MCP tools |
||
|
|
||
| // when | ||
| Throwable thrown = catchThrowable( | ||
| () -> memberService.withdraw(MEMBER_ID, ACCESS_TOKEN_HASH, REFRESH_TOKEN_HASH)); | ||
|
|
||
| // then | ||
| assertThat(thrown).isInstanceOf(MemberNotFoundException.class); | ||
| verify(activityUseCase, never()).deleteForMemberWithdrawal(anyLong()); | ||
| verify(applicationEventPublisher, never()).publishEvent(any()); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
원인 예외를 보존하세요.
new MemberNotFoundException()은 원래ObjectOptimisticLockingFailureException의 cause와 stack trace를 버립니다. 운영 환경에서 실제 충돌 위치를 확인하기 어렵습니다. 원인 예외를 받는 생성자를 추가하고 변환 시 전달하세요.수정 예시
🧰 Tools
🪛 PMD (7.26.0)
[Medium] 124-124: PreserveStackTrace (Best Practices): Thrown exception does not preserve the stack trace of exception 'e' on all code paths
(PreserveStackTrace (Best Practices))
🤖 Prompt for AI Agents
Source: Linters/SAST tools