-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feat/#29/매칭 취소,매칭 성공 로직 완성 #32
base: dev
Are you sure you want to change the base?
The head ref may contain hidden characters: "feat/#29/\uB9E4\uCE6D-\uCDE8\uC18C,\uB9E4\uCE6D-\uC131\uACF5-\uB85C\uC9C1-\uC644\uC131"
Changes from all commits
3469ba7
021e85c
959419d
1cd28fe
67e5985
ad3faba
61af188
871b065
6b6b5c0
05dc9dc
51de6b4
00bb278
c34a82c
c454d92
d5c389d
541fc94
e4be019
115a3ff
69c3573
e13ae21
4bf3e15
f534c13
005163e
4ef71d2
c98f161
3de9118
d6b4817
440d91a
5acae17
6df1e8c
6424e86
cd4c568
f3e97b4
a2d7000
93ee46b
e9faac6
242b7e2
f9aeea4
56dc901
2b31cdb
8e212b7
25b6d01
c79b747
4c2ce2c
a1e1171
8f3c4c8
3c36c2c
a3f8abb
b0af0fe
952d355
d3a3b29
12cf905
b20683f
342227e
6886e2d
6b999ac
2d7f2ae
e09ad24
857ae25
c0996ea
d93eb15
06e76bc
eb59131
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 |
---|---|---|
|
@@ -37,3 +37,5 @@ out/ | |
.vscode/ | ||
|
||
src/main/resources/.env | ||
|
||
docker-compose.yml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package com.gachtaxi.domain.matching.aop; | ||
|
||
import java.lang.annotation.ElementType; | ||
import java.lang.annotation.Retention; | ||
import java.lang.annotation.RetentionPolicy; | ||
import java.lang.annotation.Target; | ||
|
||
@Retention(RetentionPolicy.RUNTIME) | ||
@Target(ElementType.METHOD) | ||
public @interface SseSubscribeRequired { | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
package com.gachtaxi.domain.matching.aop; | ||
|
||
import com.gachtaxi.domain.matching.common.controller.ResponseMessage; | ||
import com.gachtaxi.domain.matching.common.exception.ControllerNotHasCurrentMemberIdException; | ||
import com.gachtaxi.domain.matching.common.service.AutoMatchingService; | ||
import com.gachtaxi.global.auth.jwt.annotation.CurrentMemberId; | ||
import com.gachtaxi.global.common.response.ApiResponse; | ||
import java.lang.reflect.Parameter; | ||
import lombok.RequiredArgsConstructor; | ||
import org.aspectj.lang.ProceedingJoinPoint; | ||
import org.aspectj.lang.annotation.Around; | ||
import org.aspectj.lang.annotation.Aspect; | ||
import org.aspectj.lang.reflect.MethodSignature; | ||
import org.springframework.http.HttpStatus; | ||
import org.springframework.stereotype.Component; | ||
|
||
@Aspect | ||
@Component | ||
@RequiredArgsConstructor | ||
public class SseSubscribeRequiredAop { | ||
|
||
private final AutoMatchingService autoMatchingService; | ||
|
||
@Around("@annotation(com.gachtaxi.domain.matching.aop.SseSubscribeRequired)") | ||
public Object checkSseSubscribe(ProceedingJoinPoint proceedingJoinPoint) throws Throwable{ | ||
Long memberId = null; | ||
MethodSignature signature = (MethodSignature) proceedingJoinPoint.getSignature(); | ||
Parameter[] parameters = signature.getMethod().getParameters(); | ||
|
||
for (int i = 0; i < parameters.length; i++) { | ||
Parameter parameter = parameters[i]; | ||
if (parameter.getType().equals(Long.class) && parameter.isAnnotationPresent( | ||
CurrentMemberId.class)) { | ||
memberId = (Long) proceedingJoinPoint.getArgs()[i]; | ||
break; | ||
} | ||
} | ||
|
||
if (memberId == null) { | ||
throw new ControllerNotHasCurrentMemberIdException(); | ||
} | ||
|
||
if (!this.autoMatchingService.isSseSubscribed(memberId)) { | ||
return ApiResponse.response( | ||
HttpStatus.BAD_REQUEST, | ||
ResponseMessage.NOT_SUBSCRIBED_SSE.getMessage() | ||
); | ||
} | ||
|
||
return proceedingJoinPoint.proceed(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,14 @@ | ||
package com.gachtaxi.domain.matching.common.controller; | ||
|
||
import com.gachtaxi.domain.matching.aop.SseSubscribeRequired; | ||
import com.gachtaxi.domain.matching.common.dto.request.AutoMatchingCancelledRequest; | ||
import com.gachtaxi.domain.matching.common.dto.request.AutoMatchingPostRequest; | ||
import com.gachtaxi.domain.matching.common.dto.response.AutoMatchingPostResponse; | ||
import com.gachtaxi.domain.matching.common.service.AutoMatchingService; | ||
import com.gachtaxi.global.auth.jwt.annotation.CurrentMemberId; | ||
import com.gachtaxi.global.common.response.ApiResponse; | ||
import lombok.RequiredArgsConstructor; | ||
import lombok.extern.slf4j.Slf4j; | ||
import org.springframework.http.HttpStatus; | ||
import org.springframework.http.MediaType; | ||
import org.springframework.web.bind.annotation.GetMapping; | ||
|
@@ -15,6 +19,7 @@ | |
import org.springframework.web.bind.annotation.RestController; | ||
import org.springframework.web.servlet.mvc.method.annotation.SseEmitter; | ||
|
||
@Slf4j | ||
@RestController | ||
@RequiredArgsConstructor | ||
@RequestMapping("/api/matching/auto") | ||
|
@@ -23,31 +28,33 @@ public class AutoMatchingController { | |
private final AutoMatchingService autoMatchingService; | ||
|
||
@GetMapping(value = "/subscribe", produces = MediaType.TEXT_EVENT_STREAM_VALUE) | ||
public SseEmitter subscribeSse(@RequestParam Long memberId) { | ||
// TODO: 인가 로직 완성되면 해당 멤버의 아이디를 가져오도록 변경 | ||
// Long memberId = 1L; | ||
|
||
public SseEmitter subscribeSse(@CurrentMemberId Long memberId) { | ||
return this.autoMatchingService.handleSubscribe(memberId); | ||
} | ||
|
||
@PostMapping("/request") | ||
@SseSubscribeRequired | ||
public ApiResponse<AutoMatchingPostResponse> requestMatching( | ||
@RequestParam Long memberId, | ||
@CurrentMemberId Long memberId, | ||
@RequestBody AutoMatchingPostRequest autoMatchingPostRequest | ||
) { | ||
// TODO: 인가 로직 완성되면 해당 멤버의 아이디를 가져오도록 변경 | ||
// Long memberId = 1L; | ||
if (!this.autoMatchingService.isSseSubscribed(memberId)) { | ||
return ApiResponse.response( | ||
HttpStatus.BAD_REQUEST, | ||
ResponseMessage.NOT_SUBSCRIBED_SSE.getMessage() | ||
); | ||
} | ||
|
||
return ApiResponse.response( | ||
HttpStatus.OK, | ||
ResponseMessage.AUTO_MATCHING_REQUEST_ACCEPTED.getMessage(), | ||
this.autoMatchingService.handlerAutoRequestMatching(memberId, autoMatchingPostRequest) | ||
Comment on lines
42
to
44
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. HttpStatus나 ResponseMessage는 정적 경로 추가하면 좋을 거 같아요! |
||
); | ||
} | ||
|
||
@PostMapping("/cancel") | ||
@SseSubscribeRequired | ||
public ApiResponse<AutoMatchingPostResponse> cancelMatching( | ||
@CurrentMemberId Long memberId, | ||
@RequestBody AutoMatchingCancelledRequest autoMatchingCancelledRequest | ||
) { | ||
return ApiResponse.response( | ||
HttpStatus.OK, | ||
ResponseMessage.AUTO_MATCHING_REQUEST_CANCELLED.getMessage(), | ||
this.autoMatchingService.handlerAutoCancelMatching(memberId, autoMatchingCancelledRequest) | ||
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. response 메서드 내에서 서비스의 반환값을 넣어주는 코드가 길어지니 가독성이 약간 아쉬운 것 같아요!
위 처럼 명시적으로 나눠지면 보기가 더 편할 것 같아요! |
||
); | ||
Comment on lines
+54
to
+58
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. 여기두요! |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package com.gachtaxi.domain.matching.common.dto.request; | ||
|
||
public record AutoMatchingCancelledRequest( | ||
Long roomId | ||
) { | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
package com.gachtaxi.domain.matching.common.entity; | ||
|
||
import com.gachtaxi.domain.matching.common.entity.enums.MatchingRoomStatus; | ||
import com.gachtaxi.domain.matching.event.dto.kafka_topic.MatchRoomCreatedEvent; | ||
import com.gachtaxi.domain.members.entity.Members; | ||
import com.gachtaxi.global.common.entity.BaseEntity; | ||
import jakarta.persistence.CascadeType; | ||
|
@@ -18,10 +19,11 @@ | |
import lombok.Builder; | ||
import lombok.Getter; | ||
import lombok.NoArgsConstructor; | ||
import lombok.Setter; | ||
|
||
@Entity | ||
@Table(name = "matching_room") | ||
@Builder | ||
@Builder(access = AccessLevel.PRIVATE) | ||
@NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
@AllArgsConstructor(access = AccessLevel.PRIVATE) | ||
public class MatchingRoom extends BaseEntity { | ||
|
@@ -38,6 +40,8 @@ public class MatchingRoom extends BaseEntity { | |
private List<MemberMatchingRoomChargingInfo> memberMatchingRoomChargingInfo; | ||
|
||
@ManyToOne(cascade = CascadeType.PERSIST, optional = false) | ||
@Getter | ||
@Setter | ||
private Members roomMaster; | ||
|
||
@Column(name = "title", nullable = false) | ||
|
@@ -56,7 +60,35 @@ public class MatchingRoom extends BaseEntity { | |
@Enumerated(EnumType.STRING) | ||
private MatchingRoomStatus matchingRoomStatus; | ||
|
||
public boolean isActiveMatchingRoom() { | ||
public boolean isActive() { | ||
return this.matchingRoomStatus == MatchingRoomStatus.ACTIVE; | ||
} | ||
|
||
public void changeRoomMaster(Members members) { | ||
this.setRoomMaster(members); | ||
} | ||
|
||
public void cancelMatchingRoom() { | ||
this.matchingRoomStatus = MatchingRoomStatus.CANCELLED; | ||
} | ||
|
||
public void completeMatchingRoom() { | ||
this.matchingRoomStatus = MatchingRoomStatus.COMPLETE; | ||
} | ||
|
||
public boolean isFull(int size) { | ||
return size == totalCharge; | ||
} | ||
|
||
public static MatchingRoom activeOf(MatchRoomCreatedEvent matchRoomCreatedEvent, Members members, Route route) { | ||
return MatchingRoom.builder() | ||
.capacity(matchRoomCreatedEvent.maxCapacity()) | ||
.roomMaster(members) | ||
.title(matchRoomCreatedEvent.title()) | ||
.description(matchRoomCreatedEvent.description()) | ||
.route(route) | ||
.totalCharge(matchRoomCreatedEvent.expectedTotalCharge()) | ||
.matchingRoomStatus(MatchingRoomStatus.ACTIVE) | ||
.build(); | ||
} | ||
Comment on lines
+83
to
+93
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. activeOf 메서드로 MatchRoomCreatedEvent, 태그, 멤버 등의 다양한 데이터를 사용해서 활성화된 방 매칭 부분에 책임 관리에 대한 부분은 저도 서비스 알고리즘을 현재 설계해보면서 고민이 많은 부분인데, public static List<MatchingRoomTagInfo> initializeTags(MatchRoomCreatedEvent event) {
return event.getTags().stream()
.map(tag -> MatchingRoomTagInfo.of(tag))
.collect(Collectors.toList());
} 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. 이 메서드는 "새로운 매칭방 생성" 이라는 역할만을 가지고 있는 메서드입니다. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
package com.gachtaxi.domain.matching.common.entity.enums; | ||
|
||
public enum MatchingRoomStatus { | ||
PROCESS, COMPLETE, CANCELED, ACTIVE | ||
COMPLETE, CANCELLED, ACTIVE | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
package com.gachtaxi.domain.matching.common.entity.enums; | ||
|
||
public enum PaymentStatus { | ||
PAYED, NOT_PAYED, FAILED | ||
PAYED, NOT_PAYED, FAILED, LEFT | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package com.gachtaxi.domain.matching.common.exception; | ||
|
||
import static com.gachtaxi.domain.matching.common.exception.ErrorMessage.CONTROLLER_NOT_HAS_CURRENT_MEMBER_ID; | ||
import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR; | ||
|
||
import com.gachtaxi.global.common.exception.BaseException; | ||
|
||
public class ControllerNotHasCurrentMemberIdException extends BaseException { | ||
|
||
public ControllerNotHasCurrentMemberIdException() { | ||
super(INTERNAL_SERVER_ERROR, CONTROLLER_NOT_HAS_CURRENT_MEMBER_ID.getMessage()); | ||
} | ||
} |
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.
SseSubscribeRequired 어노테이션을 활용해서 AOP 방식으로 체크를 해주는 로직은 한번도 보지못하였는데 코드도 클린해지고 중복 처리에도 용이한거 같습니다 많이 배웠습니다
제가 이해한 바가 맞다면 해당 어노테이션은 SSE 구독 여부를 확인하기 위한 어노테이션으로 사용자가 SSE에 구독되어 있는지 확인해주는 걸로 이해했는데 맞을까요 ??
또한 저희가 컨트롤러 수준에서 구독 여부만 확인해주면 되지만, 이 로직을 추가하면 테스트의 복잡성이 증가할 수도 있겠다는 생각이 들었는데, 기존 방식에서 AOP 방식을 도입한 주영님의 의견도 궁금합니다
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.
SSE 구독이 되어있지 않으면 서비스 로직 자체가 실행되면 안된다고 판단해서 컨트롤러 수준에서 구독이 안되어있으면 튕겨내는 식으로 구현했습니다. 이때 구독 여부 확인 로직 자체는 단위 테스트가 필요 없다고 생각합니다.
단위 테스트에서는 비즈니스 로직만을 검증하므로 서비스 로직 단위 테스트시에는 구독이 무조건 되어있다고 가정하고 테스트를 진행할 것이기 때문입니다.(구독이 안되어있으면 서비스 로직 자체가 실행 안되므로)
통합 테스트에서는 구독 여부 확인도 테스트해야하지만, 구독이 안되어있으면 서비스 로직 자체가 실행이 안되도록 설계했으므로
구독 x -> 컨트롤러에서 튕겨나가는지 검증(서비스 로직이 실행 안되는걸 검증)
구독 o -> 서비스 로직이 정상적으로 실행되는지 검증
즉, SSE 구독 여부 확인과 서비스 로직을 완전히 분리해 놓은 설계가 되므로 테스트의 복잡성 또한 낮아질 것으로 생각됩니다.