Skip to content
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.

Sign up for GitHub

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

[feat/CK-237] 골룸 참여 시 발생하는 동시성 이슈를 해결한다 #199

Merged
merged 2 commits into from
Dec 2, 2023
Merged

[feat/CK-237] 골룸 참여 시 발생하는 동시성 이슈를 해결한다 #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a241b85
refactor: 골룸 참여 시 발생하는 동시성 이슈를 비관적 락으로 해결
miseongk Nov 17, 2023
71eeab5
test: 테스트 코드 수정
miseongk Nov 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions backend/kirikiri/src/main/java/co/kirikiri/persistence/goalroom/GoalRoomQueryRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

public interface GoalRoomQueryRepository {

Optional<GoalRoom> findGoalRoomByIdWithPessimisticLock(Long goalRoomId);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final 붙여주세요

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵


Optional<GoalRoom> findByIdWithRoadmapContent(final Long goalRoomId);

Optional<GoalRoom> findByIdWithContentAndTodos(final Long goalRoomId);
Expand Down
11 changes: 11 additions & 0 deletions .../kirikiri/src/main/java/co/kirikiri/persistence/goalroom/GoalRoomQueryRepositoryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import co.kirikiri.persistence.goalroom.dto.RoadmapGoalRoomsOrderType;
import com.querydsl.core.types.OrderSpecifier;
import com.querydsl.core.types.dsl.BooleanExpression;
import jakarta.persistence.LockModeType;
import java.time.LocalDate;
import java.util.List;
import java.util.Optional;
Expand All @@ -27,6 +28,16 @@ public GoalRoomQueryRepositoryImpl() {
super(GoalRoom.class);
}

@Override
public Optional<GoalRoom> findGoalRoomByIdWithPessimisticLock(final Long goalRoomId) {
return Optional.ofNullable(selectFrom(goalRoom)
.innerJoin(goalRoom.goalRoomPendingMembers.values, goalRoomPendingMember)
.fetchJoin()
.where(goalRoom.id.eq(goalRoomId))
.setLockMode(LockModeType.PESSIMISTIC_WRITE)
.fetchOne());
}

@Override
public Optional<GoalRoom> findByIdWithRoadmapContent(final Long goalRoomId) {
return Optional.ofNullable(selectFrom(goalRoom)
Expand Down
5 changes: 2 additions & 3 deletions backend/kirikiri/src/main/java/co/kirikiri/persistence/goalroom/GoalRoomRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
package co.kirikiri.persistence.goalroom;

import co.kirikiri.domain.goalroom.GoalRoom;
import org.springframework.data.jpa.repository.JpaRepository;
import java.time.LocalDate;
import java.util.List;
import java.util.Optional;
import org.springframework.data.jpa.repository.JpaRepository;

public interface GoalRoomRepository extends JpaRepository<GoalRoom, Long>, GoalRoomQueryRepository {

@Override

Optional<GoalRoom> findById(final Long goalRoomId);

List<GoalRoom> findAllByEndDate(final LocalDate endDate);
Expand Down
11 changes: 8 additions & 3 deletions backend/kirikiri/src/main/java/co/kirikiri/service/goalroom/GoalRoomCreateService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,12 @@ private Member findMemberByIdentifier(final String memberIdentifier) {

public void join(final String identifier, final Long goalRoomId) {
final Member member = findMemberByIdentifier(identifier);
final GoalRoom goalRoom = findGoalRoomById(goalRoomId);
final GoalRoom goalRoom = findGoalRoomByIdWithPessimisticLock(goalRoomId);
goalRoom.join(member);
}

private GoalRoom findGoalRoomById(final Long goalRoomId) {
return goalRoomRepository.findById(goalRoomId)
private GoalRoom findGoalRoomByIdWithPessimisticLock(final Long goalRoomId) {
return goalRoomRepository.findGoalRoomByIdWithPessimisticLock(goalRoomId)
.orElseThrow(() -> new NotFoundException("존재하지 않는 골룸입니다. goalRoomId = " + goalRoomId));
}

Expand All @@ -139,6 +139,11 @@ public Long addGoalRoomTodo(final Long goalRoomId, final String identifier,
return goalRoom.findLastGoalRoomTodo().getId();
}

private GoalRoom findGoalRoomById(final Long goalRoomId) {
return goalRoomRepository.findById(goalRoomId)
.orElseThrow(() -> new NotFoundException("존재하지 않는 골룸입니다. goalRoomId = " + goalRoomId));
}

private void checkGoalRoomCompleted(final GoalRoom goalRoom) {
if (goalRoom.isCompleted()) {
throw new BadRequestException("이미 종료된 골룸입니다.");
Expand Down
39 changes: 4 additions & 35 deletions backend/kirikiri/src/test/java/co/kirikiri/integration/GoalRoomCreateIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,14 @@
import io.restassured.common.mapper.TypeRef;
import io.restassured.response.ExtractableResponse;
import io.restassured.response.Response;
import org.junit.jupiter.api.Test;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.mock.web.MockMultipartFile;
import java.io.IOException;
import java.time.temporal.ChronoUnit;
import java.util.Collections;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.mock.web.MockMultipartFile;

class GoalRoomCreateIntegrationTest extends InitIntegrationTest {

Expand Down Expand Up @@ -290,37 +290,6 @@ class GoalRoomCreateIntegrationTest extends InitIntegrationTest {
);
}

@Test
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 테스트는 왜 없어진 건가요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

골룸과 골룸 펜딩 멤버를 조인하게 되면서, goalRoom.join 메서드 내부에서 모집 중이지 않은 골룸에 대해 참여하려는 경우가 절대 없어요. 그 전에 서비스에서 골룸이 존재하지 않습니다 예외가 터지기 때문에요! (Join 하기 때문에 펜딩 멤버 자체가 없으면 골룸도 조회가 안되더라구요)
그래서 해당 검증(validateStatus)을 지울까 고민했는데, 비즈니스 로직이 명시적이면 좋겠다 싶어서 그냥 놔두었어요.
대신에 해당 테스트는 발생할 수 없는 경우라서 삭제했어요.

void 모집_중이지_않은_골룸에_참가_요청을_보내면_예외가_발생한다() throws IOException {
//given
final Long 기본_로드맵_아이디 = 로드맵_생성(기본_로드맵_생성_요청, 기본_로그인_토큰);
final RoadmapResponse 로드맵_응답 = 로드맵을_아이디로_조회하고_응답객체를_반환한다(기본_로드맵_아이디);

final List<GoalRoomRoadmapNodeRequest> 골룸_노드_별_기간_요청 = List.of(
new GoalRoomRoadmapNodeRequest(로드맵_응답.content().nodes().get(0).id(), 정상적인_골룸_노드_인증_횟수, 오늘, 십일_후));
final GoalRoomCreateRequest 골룸_생성_요청 = new GoalRoomCreateRequest(기본_로드맵_아이디, 정상적인_골룸_이름, 정상적인_골룸_제한_인원,
골룸_노드_별_기간_요청);
final Long 골룸_아이디 = 골룸을_생성하고_아이디를_반환한다(골룸_생성_요청, 기본_로그인_토큰);
골룸을_시작한다(기본_로그인_토큰, 골룸_아이디);

final MemberJoinRequest 팔로워_회원_가입_요청 = new MemberJoinRequest("identifier2", "paswword2@",
"follower", GenderType.FEMALE, DEFAULT_EMAIL);
final LoginRequest 팔로워_로그인_요청 = new LoginRequest(팔로워_회원_가입_요청.identifier(), 팔로워_회원_가입_요청.password());
회원가입(팔로워_회원_가입_요청);
final String 팔로워_액세스_토큰 = String.format(BEARER_TOKEN_FORMAT, 로그인(팔로워_로그인_요청).accessToken());

//when
final ExtractableResponse<Response> 참가_요청에_대한_응답 = 골룸_참가_요청(골룸_아이디, 팔로워_액세스_토큰);

//then
final String 예외_메시지 = 참가_요청에_대한_응답.asString();

assertAll(
() -> assertThat(참가_요청에_대한_응답.statusCode()).isEqualTo(HttpStatus.BAD_REQUEST.value()),
() -> assertThat(예외_메시지).contains("모집 중이지 않은 골룸에는 참여할 수 없습니다.")
);
}

@Test
void 인증_피드_등록을_요청한다() throws IOException {
//given
Expand Down
8 changes: 4 additions & 4 deletions backend/kirikiri/src/test/java/co/kirikiri/service/GoalRoomCreateServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ static void setUp() {

when(memberRepository.findByIdentifier(any()))
.thenReturn(Optional.of(follower));
when(goalRoomRepository.findById(anyLong()))
when(goalRoomRepository.findGoalRoomByIdWithPessimisticLock(anyLong()))
.thenReturn(Optional.of(goalRoom));

//when
Expand Down Expand Up @@ -282,7 +282,7 @@ static void setUp() {

when(memberRepository.findByIdentifier(any()))
.thenReturn(Optional.of(follower));
when(goalRoomRepository.findById(anyLong()))
when(goalRoomRepository.findGoalRoomByIdWithPessimisticLock(anyLong()))
.thenReturn(Optional.empty());

//when, then
Expand All @@ -304,7 +304,7 @@ static void setUp() {

when(memberRepository.findByIdentifier(any()))
.thenReturn(Optional.of(follower));
when(goalRoomRepository.findById(anyLong()))
when(goalRoomRepository.findGoalRoomByIdWithPessimisticLock(anyLong()))
.thenReturn(Optional.of(goalRoom));

//when, then
Expand All @@ -326,7 +326,7 @@ static void setUp() {

when(memberRepository.findByIdentifier(any()))
.thenReturn(Optional.of(follower));
when(goalRoomRepository.findById(anyLong()))
when(goalRoomRepository.findGoalRoomByIdWithPessimisticLock(anyLong()))
.thenReturn(Optional.of(goalRoom));

//when, then
Expand Down
Loading