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.

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

[BE] 방 매칭 실패 시 매칭 실패 원인을 전달하는 기능 구현(#562) #575

Merged
merged 9 commits into from
Oct 12, 2024
32 changes: 32 additions & 0 deletions backend/src/main/java/corea/matchresult/domain/FailedMatching.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package corea.matchresult.domain;

import corea.exception.ExceptionType;
import jakarta.persistence.*;
import lombok.AccessLevel;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Entity
@AllArgsConstructor
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Getter
public class FailedMatching {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

private long roomId;

@Enumerated(EnumType.STRING)
private MatchingFailedReason reason;

public FailedMatching(long roomId, ExceptionType exceptionType) {
this(null, roomId, MatchingFailedReason.from(exceptionType));
}

public String getMatchingFailedReason() {
return reason.getMessage();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package corea.matchresult.domain;

import corea.exception.CoreaException;
import corea.exception.ExceptionType;
import corea.matchresult.view.MatchingFailedMessageClassifier;

import java.util.Arrays;

public enum MatchingFailedReason {

ROOM_NOT_FOUND(ExceptionType.ROOM_NOT_FOUND),
ROOM_STATUS_INVALID(ExceptionType.ROOM_STATUS_INVALID),

PARTICIPANT_SIZE_LACK(ExceptionType.PARTICIPANT_SIZE_LACK),
PARTICIPANT_SIZE_LACK_DUE_TO_PULL_REQUEST(ExceptionType.PARTICIPANT_SIZE_LACK_DUE_TO_PULL_REQUEST),

AUTOMATIC_MATCHING_NOT_FOUND(ExceptionType.AUTOMATIC_MATCHING_NOT_FOUND),
;

private final ExceptionType exceptionType;

MatchingFailedReason(ExceptionType exceptionType) {
this.exceptionType = exceptionType;
}

public static MatchingFailedReason from(ExceptionType exceptionType) {
return Arrays.stream(values())
.filter(reason -> reason.isTypeMatching(exceptionType))
.findAny()
.orElseThrow(() -> new CoreaException(ExceptionType.NOT_FOUND_ERROR));
Copy link
Contributor

Choose a reason for hiding this comment

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

여기서 NOT_FOUND_ERROR 를 던지면 다음 흐름은 어떻게 되나요?

UNKNOWN_ERROR 와 같은 ENUM 값을 만들고
하는건 어떨까요?

( 매칭을 실패했는데 해당 REASON 이 없어서
FAIL 이나, FAILED_REASON 이 없어서 예외 발생할 거 같은 우려 )

Copy link
Contributor

Choose a reason for hiding this comment

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

UNKNOWN_ERROR 와 같은 ENUM 값을 만들고
하는건 어떨까요?

좋네요~ 수정했습니다.

}

private boolean isTypeMatching(ExceptionType exceptionType) {
return this.exceptionType == exceptionType;
}

public String getMessage() {
return MatchingFailedMessageClassifier.classifyFailureMessage(exceptionType);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package corea.matchresult.repository;

import corea.matchresult.domain.FailedMatching;
import org.springframework.data.jpa.repository.JpaRepository;

import java.util.Optional;

public interface FailedMatchingRepository extends JpaRepository<FailedMatching, Long> {

Optional<FailedMatching> findByRoomId(long roomId);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package corea.matchresult.view;

import corea.exception.ExceptionType;

import java.util.HashMap;
import java.util.Map;

public class MatchingFailedMessageClassifier {
Copy link
Contributor

Choose a reason for hiding this comment

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

이 클래스가 분리된 이유가 뭔가요? ㅇ.ㅇ

Copy link
Contributor

Choose a reason for hiding this comment

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

각 실패 원인에 대한 메시지 관리를 따로 하고 싶어서 분리했어요!

MatchingFailedReason에서 메시지까지 같이 가지고 있으면 너무 코드가 길어지더라고요.
그리고 레벨 1때 배운 내용들 생각하면 클라이언트 출력용 메시지는 따로 관리하는 것이 적절한 역할 분리라고 생각이 들었습니다.

애쉬는 너무 과한 분리라고 생각이 드신건가요?? 🧐

Copy link
Contributor

Choose a reason for hiding this comment

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

상태마다 할당되는 메세지가 각각 하나이다 보니 그냥 enum에서 메세지까지 같이 관리해도 되지 않을까... 싶더라고용.

MatchingFailedReason이 추가될 때마다 MatchingFailedMessageClassifier까지 함께 수정되어야 한다는 점에서
관리 포인트가 늘어날 것 같다는 생각도 듭니다.

그래도 다른 분들 괜찮으시면 수정할 필요는 없을듯요~~ 👍👍

Copy link
Contributor

Choose a reason for hiding this comment

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

코드적인 면으로는 클린코드 일거 같은데
관리적인 면에서 귀찮을거 같긴 하네요.

추가로, ENUM 이 늘어날 때 메시지를 실수로 추가하지 못할 우려가 생길꺼 같은데
switch - case 구문을 써서 넣는게 좋을거 같아용 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

MatchingFailedReason에서 관리하도록 수정했고,
switch-case는 사용하지 않고 그냥 매개변수로 갖도록 수정했어요!~


private static final String ROOM_NOT_FOUND_MESSAGE = "기존에 존재하던 방이 방장의 삭제로 인해 더 이상 유효하지 않아 매칭을 진행할 수 없습니다.";
private static final String ROOM_STATUS_INVALID_MESSAGE = "방이 이미 매칭 중이거나, 매칭이 완료되어 더 이상 매칭을 진행할 수 없는 상태입니다.";
private static final String PARTICIPANT_SIZE_LACK_MESSAGE = "방의 최소 참여 인원보다 참가자가 부족하여 매칭을 시작할 수 없습니다. 더 많은 참가자가 필요합니다.";
private static final String DUE_TO_PULL_REQUEST_MESSAGE = "참가자의 수는 최소 인원을 충족하였지만, 일부 참가자가 pull request를 제출하지 않아 매칭을 진행하기에 참가자가 부족합니다.";
private static final String MATCHING_NOT_FOUND_MESSAGE = "해당 방에 대해 예약된 자동 매칭 시간이 존재하지 않거나 설정되지 않아 매칭을 진행할 수 없습니다.";
private static final String DEFAULT_MATCHING_FAILED_MESSAGED = "매칭 과정에서 오류가 발생하여 매칭을 완료할 수 없습니다. 문제가 지속될 경우 관리자에게 문의하세요.";

private static final Map<ExceptionType, String> MESSAGE_STORE = new HashMap<>();

static {
MESSAGE_STORE.put(ExceptionType.ROOM_NOT_FOUND, ROOM_NOT_FOUND_MESSAGE);
MESSAGE_STORE.put(ExceptionType.ROOM_STATUS_INVALID, ROOM_STATUS_INVALID_MESSAGE);

MESSAGE_STORE.put(ExceptionType.PARTICIPANT_SIZE_LACK, PARTICIPANT_SIZE_LACK_MESSAGE);
MESSAGE_STORE.put(ExceptionType.PARTICIPANT_SIZE_LACK_DUE_TO_PULL_REQUEST, DUE_TO_PULL_REQUEST_MESSAGE);

MESSAGE_STORE.put(ExceptionType.AUTOMATIC_MATCHING_NOT_FOUND, MATCHING_NOT_FOUND_MESSAGE);
}

public static String classifyFailureMessage(ExceptionType exceptionType) {
if (MESSAGE_STORE.containsKey(exceptionType)) {
return MESSAGE_STORE.get(exceptionType);
}
return DEFAULT_MATCHING_FAILED_MESSAGED;
}
}
54 changes: 52 additions & 2 deletions backend/src/main/java/corea/room/dto/RoomResponse.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package corea.room.dto;

import corea.matchresult.domain.FailedMatching;
import corea.member.domain.MemberRole;
import corea.participation.domain.Participation;
import corea.participation.domain.ParticipationStatus;
import corea.room.domain.Room;
import io.swagger.v3.oas.annotations.media.Schema;
Expand Down Expand Up @@ -52,9 +54,14 @@ public record RoomResponse(@Schema(description = "방 아이디", example = "1")
MemberRole memberRole,

@Schema(description = "방 상태", example = "OPEN")
String roomStatus
String roomStatus,

@Schema(description = "매칭 실패 원인 메세지 제공", example = "참여 인원이 부족하여 매칭을 진행할 수 없습니다.")
String message
) {

private static final String DEFAULT_MESSAGE = "";

public static RoomResponse from(Room room) {
return RoomResponse.of(room, MemberRole.BOTH, ParticipationStatus.NOT_PARTICIPATED);
}
Expand All @@ -76,7 +83,50 @@ public static RoomResponse of(Room room, MemberRole role, ParticipationStatus pa
room.getReviewDeadline(),
participationStatus,
role,
room.getRoomStatus()
room.getRoomStatus(),
DEFAULT_MESSAGE
);
}

public static RoomResponse of(Room room, Participation participation) {
Copy link
Contributor

Choose a reason for hiding this comment

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

당장은 이렇게 구분하는게 최선일 거 같긴 하네요.

return new RoomResponse(
room.getId(),
room.getTitle(),
room.getContent(),
room.getManagerName(),
room.getRepositoryLink(),
room.getThumbnailLink(),
room.getMatchingSize(),
room.getKeyword(),
room.getCurrentParticipantsSize(),
room.getLimitedParticipantsSize(),
room.getRecruitmentDeadline(),
room.getReviewDeadline(),
participation.getStatus(),
participation.getMemberRole(),
room.getRoomStatus(),
DEFAULT_MESSAGE
);
}

public static RoomResponse of(Room room, Participation participation, FailedMatching failedMatching) {
return new RoomResponse(
room.getId(),
room.getTitle(),
room.getContent(),
room.getManagerName(),
room.getRepositoryLink(),
room.getThumbnailLink(),
room.getMatchingSize(),
room.getKeyword(),
room.getCurrentParticipantsSize(),
room.getLimitedParticipantsSize(),
room.getRecruitmentDeadline(),
room.getReviewDeadline(),
participation.getStatus(),
participation.getMemberRole(),
room.getRoomStatus(),
failedMatching.getMatchingFailedReason()
);
}
}
23 changes: 15 additions & 8 deletions backend/src/main/java/corea/room/service/RoomService.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

import corea.exception.CoreaException;
import corea.exception.ExceptionType;
import corea.matchresult.repository.FailedMatchingRepository;
import corea.matchresult.repository.MatchResultRepository;
import corea.member.domain.Member;
import corea.member.domain.MemberRole;
import corea.member.repository.MemberRepository;
import corea.participation.domain.Participation;
import corea.participation.domain.ParticipationStatus;
import corea.participation.repository.ParticipationRepository;
import corea.room.domain.Room;
import corea.room.domain.RoomClassification;
Expand All @@ -28,8 +30,6 @@
import java.util.Collections;
import java.util.List;

import static corea.participation.domain.ParticipationStatus.*;

@Slf4j
@Service
@RequiredArgsConstructor
Expand All @@ -45,6 +45,7 @@ public class RoomService {
private final MemberRepository memberRepository;
private final MatchResultRepository matchResultRepository;
private final ParticipationRepository participationRepository;
private final FailedMatchingRepository failedMatchingRepository;
private final AutomaticMatchingRepository automaticMatchingRepository;
private final AutomaticUpdateRepository automaticUpdateRepository;

Expand All @@ -61,7 +62,7 @@ public RoomResponse create(long memberId, RoomCreateRequest request) {
automaticMatchingRepository.save(new AutomaticMatching(room.getId(), request.recruitmentDeadline()));
automaticUpdateRepository.save(new AutomaticUpdate(room.getId(), request.reviewDeadline()));

return RoomResponse.of(room, participation.getMemberRole(), MANAGER);
return RoomResponse.of(room, participation.getMemberRole(), ParticipationStatus.MANAGER);
}

private void validateDeadLine(LocalDateTime recruitmentDeadline, LocalDateTime reviewDeadline) {
Expand All @@ -83,8 +84,14 @@ public RoomResponse findOne(long roomId, long memberId) {
Room room = getRoom(roomId);

return participationRepository.findByRoomIdAndMemberId(roomId, memberId)
.map(participation -> RoomResponse.of(room, participation.getMemberRole(), participation.getStatus()))
.orElseGet(() -> RoomResponse.of(room, MemberRole.NONE, NOT_PARTICIPATED));
.map(participation -> createRoomResponseWithParticipation(room, participation))
.orElseGet(() -> RoomResponse.of(room, MemberRole.NONE, ParticipationStatus.NOT_PARTICIPATED));
}

private RoomResponse createRoomResponseWithParticipation(Room room, Participation participation) {
return failedMatchingRepository.findByRoomId(room.getId())
.map(failedMatching -> RoomResponse.of(room, participation, failedMatching))
.orElseGet(() -> RoomResponse.of(room, participation));
}

public RoomResponses findParticipatedRooms(long memberId) {
Expand All @@ -94,7 +101,7 @@ public RoomResponses findParticipatedRooms(long memberId) {
.toList();

List<Room> rooms = roomRepository.findAllByIdInOrderByReviewDeadlineAsc(roomIds);
return RoomResponses.of(rooms, MemberRole.NONE, PARTICIPATED, true, 0);
return RoomResponses.of(rooms, MemberRole.NONE, ParticipationStatus.PARTICIPATED, true, 0);
}

public RoomResponses findRoomsWithRoomStatus(long memberId, int pageNumber, String expression, RoomStatus roomStatus) {
Expand All @@ -107,10 +114,10 @@ private RoomResponses getRoomResponses(long memberId, int pageNumber, RoomClassi

if (classification.isAll()) {
Page<Room> roomsWithPage = roomRepository.findAllByMemberAndStatus(memberId, status, pageRequest);
return RoomResponses.of(roomsWithPage, MemberRole.NONE, NOT_PARTICIPATED, pageNumber);
return RoomResponses.of(roomsWithPage, MemberRole.NONE, ParticipationStatus.NOT_PARTICIPATED, pageNumber);
}
Page<Room> roomsWithPage = roomRepository.findAllByMemberAndClassificationAndStatus(memberId, classification, status, pageRequest);
return RoomResponses.of(roomsWithPage, MemberRole.NONE, NOT_PARTICIPATED, pageNumber);
return RoomResponses.of(roomsWithPage, MemberRole.NONE, ParticipationStatus.NOT_PARTICIPATED, pageNumber);
}

@Transactional
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import corea.matching.domain.PullRequestInfo;
import corea.matching.service.MatchingService;
import corea.matching.service.PullRequestProvider;
import corea.matchresult.domain.FailedMatching;
import corea.matchresult.repository.FailedMatchingRepository;
import corea.room.domain.Room;
import corea.room.repository.RoomRepository;
import corea.scheduler.domain.AutomaticMatching;
Expand All @@ -25,6 +27,7 @@ public class AutomaticMatchingExecutor {
private final MatchingService matchingService;
private final PullRequestProvider pullRequestProvider;
private final RoomRepository roomRepository;
private final FailedMatchingRepository failedMatchingRepository;
private final AutomaticMatchingRepository automaticMatchingRepository;

@Async
Expand All @@ -39,7 +42,7 @@ public void execute(long roomId) {
});
} catch (CoreaException e) {
log.warn("매칭 실행 중 에러 발생: {}", e.getMessage(), e);
updateRoomStatusToFail(roomId);
recordMatchingFailure(roomId, e.getExceptionType());
}
}

Expand All @@ -53,16 +56,26 @@ private void startMatching(long roomId) {
automaticMatching.updateStatusToDone();
}

private void updateRoomStatusToFail(long roomId) {
private void recordMatchingFailure(long roomId, ExceptionType exceptionType) {
//TODO: 위와 동일
TransactionTemplate template = new TransactionTemplate(transactionManager);
template.execute(status -> {
Room room = getRoom(roomId);
room.updateStatusToFail();
updateRoomStatusToFail(roomId);
saveFailedMatching(roomId, exceptionType);
return null;
});
}

private void updateRoomStatusToFail(long roomId) {
Room room = getRoom(roomId);
room.updateStatusToFail();
}

private void saveFailedMatching(long roomId, ExceptionType exceptionType) {
FailedMatching failedMatching = new FailedMatching(roomId, exceptionType);
failedMatchingRepository.save(failedMatching);
}

private Room getRoom(long roomId) {
return roomRepository.findById(roomId)
.orElseThrow(() -> new CoreaException(ExceptionType.ROOM_NOT_FOUND));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package corea.matchresult.domain;

import corea.exception.CoreaException;
import corea.exception.ExceptionType;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

class MatchingFailedReasonTest {

@ParameterizedTest
@CsvSource(value = {"ROOM_NOT_FOUND, ROOM_NOT_FOUND", "AUTOMATIC_MATCHING_NOT_FOUND, AUTOMATIC_MATCHING_NOT_FOUND"})
Copy link
Contributor

Choose a reason for hiding this comment

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

ROOM_NOT_FOUND 중복으로 들어가용

Copy link
Contributor

Choose a reason for hiding this comment

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

(ExceptionType exceptionType, MatchingFailedReason expected)

하나는 ExceptionType이고, 하나는 MatchingFailedReason의 값이라 중복으로 들어가는게 맞습니다~

@DisplayName("ExceptionType을 통해 매칭이 실패한 이유를 찾을 수 있다.")
void from(ExceptionType exceptionType, MatchingFailedReason expected) {
MatchingFailedReason actual = MatchingFailedReason.from(exceptionType);

assertThat(actual).isEqualTo(expected);
}

@Test
@DisplayName("ExceptionType을 통해 매칭 실패 이유를 찾을 수 없다면 예외가 발생한다.")
void notFound() {
assertThatThrownBy(() -> MatchingFailedReason.from(ExceptionType.ALREADY_COMPLETED_FEEDBACK))
.isInstanceOf(CoreaException.class);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package corea.matchresult.view;

import corea.exception.ExceptionType;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;

class MatchingFailedMessageClassifierTest {

@Test
@DisplayName("매칭 진행 중 발생한 예외를 통해 예외 반환 메세지를 얻을 수 있다.")
void classifyFailureMessage() {
String message = MatchingFailedMessageClassifier.classifyFailureMessage(ExceptionType.PARTICIPANT_SIZE_LACK_DUE_TO_PULL_REQUEST);

assertThat(message).isEqualTo("참가자의 수는 최소 인원을 충족하였지만, 일부 참가자가 pull request를 제출하지 않아 매칭을 진행하기에 참가자가 부족합니다.");
}

@Test
@DisplayName("지정되어 있는 예외가 아닐 경우 기본 메세지를 얻을 수 있다.")
void getDefaultMatchingFailedMessage() {
String message = MatchingFailedMessageClassifier.classifyFailureMessage(ExceptionType.ALREADY_COMPLETED_FEEDBACK);

assertThat(message).isEqualTo("매칭 과정에서 오류가 발생하여 매칭을 완료할 수 없습니다. 문제가 지속될 경우 관리자에게 문의하세요.");
}
}
Loading
Loading