Skip to content
This repository was archived by the owner on Oct 24, 2024. It is now read-only.
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.flab.makedel.dto.StoreDTO;
import com.flab.makedel.service.StoreService;
import com.google.firebase.messaging.FirebaseMessagingException;
import java.io.IOException;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
Expand Down Expand Up @@ -83,7 +84,7 @@ public ResponseEntity<Void> openMyStore(@PathVariable long storeId,
@PostMapping("/{storeId}/orders/{orderId}/approve")
@LoginCheck(userLevel = UserLevel.OWNER)
public void approveOrder(@PathVariable long orderId, @PathVariable long storeId,
@CurrentUserId String ownerId) throws FirebaseMessagingException {
@CurrentUserId String ownerId) throws IOException {
storeService.validateMyStore(storeId, ownerId);
storeService.approveOrder(orderId);
}
Expand Down
9 changes: 6 additions & 3 deletions src/main/java/com/flab/makedel/service/PushService.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public void init() throws IOException {
}

public void sendMessageToStandbyRidersInSameArea(String address, PushMessageDTO pushMessage)
Copy link
Member

Choose a reason for hiding this comment

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

다시 보니 이 클래스의 이름은 PushService인데 메소드 이름에 StandbyRidersInSameArea가 붙어있습니다. 순수하게 푸시 로직만 들어있는게 아니라 라이더 관련 책임도 들어있네요. 그러면 두 책임간의 결합도가 높은게 아닐까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

도메인 관계를 잘못 설정하고 있었네요. RiderService가 Pushservice에 의존하여 메세지를 보내게 변경하였습니다.

throws FirebaseMessagingException {
throws IOException {
Set<String> tokenSet = deliveryDAO.selectStandbyRiderTokenList(address);
List<Message> messages = tokenSet.stream().map(token -> Message.builder()
.putData("title", pushMessage.getTitle())
Expand All @@ -57,8 +57,11 @@ public void sendMessageToStandbyRidersInSameArea(String address, PushMessageDTO
.setToken(token)
.build())
.collect(Collectors.toList());

FirebaseMessaging.getInstance().sendAll(messages);
try {
FirebaseMessaging.getInstance().sendAll(messages);
} catch (FirebaseMessagingException e) {
throw new IOException(e);
Copy link
Member

Choose a reason for hiding this comment

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

여기서 checked exception을 사용하신 이유가 있으실까요?

Copy link
Collaborator Author

@tjdrnr0557 tjdrnr0557 Nov 4, 2020

Choose a reason for hiding this comment

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

checked exception은 catch로 잡든 throw로 던지든 반드시 예외처리를 해줘야합니다. 예외처리를 강제로 시키고 이 소스 코드를 같이 개발하는 개발자들에게 이 예외는 꼭처리해야한다고 명확하게 알려주기 위해 checked exception을 사용했습니다. 또한 다시 ioexception으로 감싸서 던져서 사용자가 푸쉬메세지를 보내는데 실패한다면 사용자에게도 알려줄 수 있도록 다시 throw하였습니다.

}
}

}
3 changes: 2 additions & 1 deletion src/main/java/com/flab/makedel/service/StoreService.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.flab.makedel.mapper.OrderMapper;
import com.flab.makedel.mapper.StoreMapper;
import com.google.firebase.messaging.FirebaseMessagingException;
import java.io.IOException;
import java.time.LocalDateTime;
import java.util.List;
import lombok.RequiredArgsConstructor;
Expand Down Expand Up @@ -69,7 +70,7 @@ public void validateMyStore(long storeId, String ownerId) throws HttpClientError
}

@Transactional
public void approveOrder(long orderId) throws FirebaseMessagingException {
public void approveOrder(long orderId) throws IOException {
orderMapper.approveOrder(orderId, OrderStatus.APPROVED_ORDER);
OrderReceiptDTO orderReceipt = orderMapper.selectOrderReceipt(orderId);
deliveryService.registerStandbyOrderWhenOrderApprove(orderId, orderReceipt);
Expand Down