-
Notifications
You must be signed in to change notification settings - Fork 54
[#70] 배달 매칭 서비스 #69
base: rider_info_service
Are you sure you want to change the base?
[#70] 배달 매칭 서비스 #69
Conversation
[#46] 주문시 쿠폰 적용
Coupon에 대한 테스트코드 작성
@@ -129,7 +133,12 @@ public void updateMail(HttpSession session, @RequestBody UpdateMailRequest reque | |||
riderInfoService.changeMail(id, request.getPassword(), request.getUpdateMail()); | |||
} | |||
|
|||
|
|||
@PostMapping("delivery/accept") | |||
@LoginCheck(type = UserType.RIDER) |
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.
모습을 보니 떠오른건데 type보다는 level이 어떨까요?
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.
level도 생각해보았는데 뭔가 접근의 계층이 있어야 할것같은 느낌이라서요. 제가 만든 로그인은 3가지의 전혀 다른 타입이라서 일단 타입으로 해두었습니다.
src/main/java/com/delfood/dao/deliveery/MultiThreadDeliveryDao.java
Outdated
Show resolved
Hide resolved
|
||
@Repository(value = "multiThreadDeliveryDao") | ||
@ThreadSafe | ||
public class MultiThreadDeliveryDao implements DeliveryDao{ |
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.
저장소가 로컬저장소이므로 MultiThread
보단 LocalMemoryRiderMatcher
등 적절한 이름을 지어주는게 좋을 것 같습니다
src/main/java/com/delfood/dao/deliveery/MultiThreadDeliveryDao.java
Outdated
Show resolved
Hide resolved
if (riders.containsKey(riderInfo.getId())) { | ||
riders.replace(riderInfo.getId(), riderInfo); | ||
} else { | ||
riders.put(riderInfo.getId(), riderInfo); |
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.
여기 또한 두 단계가 atomic하지 않습니다
- 없으면
- 삽입한다
*/ | ||
@Override | ||
public OrderStatus getOrderStatus(Long orderId) { | ||
OrderStatus status = orders.get(orderId); |
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.
이 때에는 order에 없지만
OrderStatus status = orders.get(orderId); | ||
if (Objects.isNull(status)) { | ||
status = orderService.getOrderStatus(orderId); | ||
setOrderStatus(orderId, status); |
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.
이 때는 order에 있으면 어떻게 될까요? 중복으로 put을 하게 되지 않을까요?
@@ -293,4 +294,9 @@ public void orderApprove(Long orderId, long minute) { | |||
pushService.sendMessageToMember(messageInfo, memberId); | |||
} | |||
|
|||
@Transactional(readOnly = true) |
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.
쿼리가 1개뿐인데 Transactional을 붙여준 이유가 있을까요?
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.
생각해보니 읽는 쪽에서는 트랜잭션을 걸 필요가 없었네요. 쿼리도 하나구요. 착각한거같습니다 ㅎㅎ
private static final long SCHEDULE_DELETE_DELIVERY_RIDER_SECOND = 300000; | ||
|
||
@Autowired | ||
@Qualifier("multiThreadDeliveryDao") |
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.
이럴땐 @Resource
를 사용해줘도 좋습니다.
* @return | ||
*/ | ||
@Override | ||
public List<DeliveryRiderDTO> toList() { |
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.
메소드의 동작을 유추하기에는 getRiderList가 더 나을 것 같습니다
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.
그때 얘기한 computeIfAbsent 쪽만 적용하시면 될 것 같네요~
- 배달 대기명단에 올리기 - 배달 대기명단 삭제 - 내 배달 현황 - 내가 배달한 물품들 (페이징 포함)
* @param session 현제 세션 | ||
* @return | ||
*/ | ||
@GetMapping("delivery") |
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.
만약 배달중인 라이더의 정보를 조회할 요구사항이 생기면 {riderId}
와 겹치지 않을까요?
SELECT odr.id orderId, odr.delivery_cost deliveryCost, odr.order_status orderStatus | ||
FROM ORDER odr INNER JOIN PAYMENT pay ON (odr.id = pay.order_id) | ||
WHERE odr.rider_id = #{riderId} | ||
AND start_time >= #{today} |
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.
메소드명은 today인데 날짜를 파라미터르 받고있네요. 이럴 때는 쿼리에 확실하게 오늘 날짜만 조회하도록 넣어두는것이 좋습니다~
No description provided.