Skip to content

refactor: ArchUnit rule 기반 패키지 구조 재구성#112

Merged
seongwop merged 9 commits into
developfrom
feature/delivery-service
May 29, 2026
Merged

refactor: ArchUnit rule 기반 패키지 구조 재구성#112
seongwop merged 9 commits into
developfrom
feature/delivery-service

Conversation

@seongwop

Copy link
Copy Markdown
Contributor

📝 작업 내용

이번 PR에서 작업한 내용을 간략히 설명해 주세요. (마이크로서비스 명시)

  • [Delivery Service] ArchUnit rule 기반 패키지 구조 재구성

🛠️ 테스트 내역

  • Postman 테스트 완료
  • 단위 테스트 작성 및 통과

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request restructures the package layout of the delivery-service module by flattening and moving packages such as clients, controllers, DTOs, entities, repositories, and services to root-level packages. Feedback on these changes highlights several improvement opportunities: ensuring idempotency in the Kafka consumer to prevent duplicate messages from triggering incorrect failure events, maintaining package consistency by also moving the domain.enums package and separating Feign Client DTOs from Presentation DTOs, and removing an unnecessary flush() call in DeliveryService to optimize JPA's dirty checking mechanism.

@@ -1,10 +1,10 @@
package com.msa.delivery_service.infrastructure.kafka;
package com.msa.delivery_service.message;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

[High] / 패키지 구조 변경 과정에서 Kafka Consumer의 멱등성(Idempotency) 처리를 점검한 결과, 중복 메시지 수신 시 existsByOrderId 검증에 의해 DUPLICATE_ORDER_DELIVERY 예외가 발생하고, 이 예외가 catch 블록에서 처리되어 delivery.create.failed 토픽으로 실패 이벤트가 발행되는 문제가 있습니다. 이는 이미 성공적으로 처리된 배송 건에 대해 잘못된 실패/보상 처리를 유발할 수 있습니다. / DUPLICATE_ORDER_DELIVERY 예외가 발생한 경우, 이미 배송이 생성된 상태이므로 실패 이벤트를 발행하지 않고 기존 배송 정보를 조회하여 성공(delivery.create.succeed)으로 응답하거나 무시하도록 예외 처리를 세분화해야 합니다.

References
  1. Consumers should be idempotent where possible. (link)

@@ -1,4 +1,4 @@
package com.msa.delivery_service.domain.entity;
package com.msa.delivery_service.entity;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

[Medium] / 패키지 구조를 평탄화(Flattening)하여 domain.entity를 entity로 이동하였으나, domain.enums 패키지는 그대로 유지되어 패키지 구조의 일관성이 결여되어 있습니다. / domain.enums 패키지 내의 Enum 클래스들도 root 패키지 하위의 enums 또는 entity 패키지 등으로 이동하여 패키지 구조의 일관성을 유지하는 것이 좋습니다.

References
  1. Naming and package convention consistency (link)

Comment on lines +3 to +4
import com.msa.delivery_service.dto.HubResponse;
import com.msa.delivery_service.dto.HubRouteResponse;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

[Medium] / 외부 API 통신을 위한 Feign Client DTO(HubResponse, HubRouteResponse)와 Presentation 레이어의 DTO(DeliveryRequest, DeliveryResponse 등)가 모두 하나의 flat한 dto 패키지에 혼재되어 있어, 레이어 간 경계가 모호해지고 의존성 관리가 어려워질 수 있습니다. / 외부 Feign Client 전용 DTO는 client 패키지 하위(예: client.hub.dto)에 두고, Presentation 레이어 DTO와 분리하여 관리하는 것이 아키텍처 일관성 및 유지보수성 측면에서 좋습니다.

References
  1. Naming and package convention consistency (link)

Comment on lines 138 to 140
// MASTER, HUB_MANAGER: 대표 배송 상태 변경 가능
// DELIVERY_MANAGER: 업체 배송 담당자인 경우만 변경 가능
// 그 외 권한: 접근 불가
switch (role) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

[Medium] / updateDeliveryStatus 메서드 내에서 엔티티의 상태 변경은 트랜잭션 커밋 시점에 JPA의 더티 체킹(Dirty Checking)에 의해 자동으로 데이터베이스에 반영(Flush)됩니다. 따라서 메서드 끝(156라인)에서 명시적으로 flush()를 호출하는 것은 불필요한 데이터베이스 쓰기 작업을 유발하여 성능에 악영향을 줄 수 있습니다. / 불필요한 deliveryRepository.flush() 호출을 제거하여 JPA의 영속성 컨텍스트 메커니즘을 효율적으로 활용하도록 수정하는 것이 좋습니다.

@seongwop seongwop merged commit d1abc88 into develop May 29, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants