[FEATURE] [Notification] AI-Slack 이벤트 연동 및 Slack API 개선 #30 - #46
Closed
250ghghghgh wants to merge 26 commits into
Closed
250ghghghgh wants to merge 26 commits into
250ghghghgh wants to merge 26 commits into
Conversation
…rvice/infrastructure/messaging/dto/ShipmentCreatedEvent.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
250ghghghgh
requested review from
Jin4041,
afterrest,
Copilot,
kim-jun-won,
soo96 and
zlonce
April 6, 2026 19:33
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRabbitMQ로 수신된 ShipmentCreated 이벤트 처리 흐름이 재구성되고 Notification 오케스트레이션·검색·권한 검증·인프라 설정이 확장되었습니다. Listener/Handler(리슨→레디스 idempotency 검사→NotificationOrchestratorService)로 이벤트를 전달해 OrderInternalClient로 주문조회, AiAppService로 AI 로그 생성/검색, SlackAppService로 메시지 전송을 수행하며 전송 결과에 따라 AiLog 상태를 갱신합니다. 또한 페이징 검색용 SearchCommand/QueryDSL 구현, 역할 기반 검증 헤더(X-User-Id/X-User-Role) 도입, Swagger/OpenFeign/Redis/Eureka 설정, 관련 DTO/레포지토리/컨트롤러 시그니처 변경 및 테스트 추가/수정이 포함됩니다. Sequence Diagram(s)sequenceDiagram
participant Listener as ShipmentCreatedListener
participant Handler as ShipmentCreatedHandler
participant Orchestrator as NotificationOrchestratorService
participant OrderClient as OrderInternalClient
participant AiService as AiAppService
participant SlackService as SlackAppService
Listener->>Handler: onShipmentCreated(event)
Handler->>Handler: Redis idempotency check (SETNX, TTL 24h)
alt not duplicate
Handler->>Orchestrator: handleShipmentCreated(event)
Orchestrator->>OrderClient: getOrderReadModel(orderId)
OrderClient-->>Orchestrator: OrderReadModelResponse / exception -> null
Orchestrator->>AiService: generateAiLog(GenerateDeadlineCommand)
AiService-->>Orchestrator: AiLogResult (aiId, finalDeadlineAt)
Orchestrator->>SlackService: sendSlackMessage(SendSlackMessageCommand)
alt Slack send success
SlackService-->>Orchestrator: success
Orchestrator->>AiService: markSlackSendSuccess(aiId)
else Slack send failure
SlackService-->>Orchestrator: error
Orchestrator->>AiService: markSlackSendFail(aiId)
Orchestrator->>Handler: rethrow -> consumer retry
end
else duplicate
Handler->>Handler: log duplicate & ignore
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Notification 서비스에서 shipment.created 이벤트를 기반으로 AI 발송 시한 산출 → Slack 자동 알림 발송까지 이어지는 오케스트레이션 흐름을 추가하고, Slack/AI 조회 API에 페이징·검색·요청 검증 및 서비스 디스커버리/Feign 연동을 보강합니다.
Changes:
- RabbitMQ
ShipmentCreatedEvent컨슈밍 및NotificationOrchestratorService를 통한 AI+Slack 연동 플로우 구현 - Slack/AI 외부 API에 검색/페이징, 요청 DTO 검증(@Valid), 역할 기반 접근 제한 로직 추가
- Feign(Eureka) 연동, WebClient 빈 충돌 방지, AuditorAware 헤더 기반 처리, Swagger 설정 및 로컬 실행 설정 정비
Reviewed changes
Copilot reviewed 42 out of 44 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| notification-service/src/test/java/com/shipflow/notificationservice/SlackAppServiceTest.java | Slack AppService 시그니처 변경(userId/userRole) 및 페이징 조회 테스트 반영 |
| notification-service/src/test/java/com/shipflow/notificationservice/NotificationOrchestratorServiceTest.java | shipment.created 이벤트 처리 오케스트레이션 테스트 신규 추가 |
| notification-service/src/test/java/com/shipflow/notificationservice/AiAppServiceTest.java | AI 로그 조회 API 시그니처 변경 반영 |
| notification-service/src/main/resources/application.yaml | Eureka/Swagger/DB/RabbitMQ 기본값 및 app name 정비 |
| notification-service/src/main/java/com/shipflow/notificationservice/presentation/slack/external/SlackController.java | Slack API에 헤더 기반 사용자정보, 검색/페이징 파라미터, @Valid 적용 |
| notification-service/src/main/java/com/shipflow/notificationservice/presentation/slack/dto/response/SlackMessageResponse.java | 리스트 변환 헬퍼 제거(페이지 응답으로 전환) |
| notification-service/src/main/java/com/shipflow/notificationservice/presentation/slack/dto/request/UpdateSlackMessageRequest.java | 메시지 검증(@NotBlank/@SiZe) 및 커맨드에 userId/userRole 포함 |
| notification-service/src/main/java/com/shipflow/notificationservice/presentation/slack/dto/request/SendSlackMessageRequest.java | Slack ID/메시지/타입 검증 및 커맨드에 userId/userRole 포함 |
| notification-service/src/main/java/com/shipflow/notificationservice/presentation/common/BasePageRequest.java | null-safe 페이지 파라미터 및 기본 정렬 Pageable 생성 추가 |
| notification-service/src/main/java/com/shipflow/notificationservice/presentation/ai/external/AiController.java | AI 조회/검색 페이징 및 debug 생성 API(헤더 기반) 추가 |
| notification-service/src/main/java/com/shipflow/notificationservice/presentation/ai/dto/request/GenerateDeadlineRequest.java | AI 생성 요청 스키마 확장 및 커맨드 변환 로직 변경 |
| notification-service/src/main/java/com/shipflow/notificationservice/NotificationserviceApplication.java | Feign 활성화 추가 |
| notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/persistence/slack/SlackMessageRepositoryImpl.java | QueryDSL 기반 Slack 메시지 검색/페이징 구현 |
| notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/persistence/slack/SlackMessageJpaRepository.java | 기존 전체조회 메서드 제거(검색/페이징으로 대체) |
| notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/persistence/ai/AiLogRepositoryImpl.java | QueryDSL 기반 AI 로그 검색/페이징 구현 |
| notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/persistence/ai/AiLogJpaRepository.java | 기존 전체조회 메서드 제거(검색/페이징으로 대체) |
| notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/messaging/dto/ShipmentCreatedEvent.java | SagaEvent 상속 및 경유지(routes), slackId 필드 등 이벤트 모델 확장 |
| notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/messaging/consumer/ShipmentCreatedListener.java | RabbitListener로 이벤트 수신 → Handler 위임 |
| notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/messaging/consumer/ShipmentCreatedHandler.java | AbstractSagaHandler 기반 처리 핸들러 추가 |
| notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/messaging/consumer/ShipmentCreatedConsumer.java | 기존 단순 로깅 Consumer 제거 |
| notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/client/order/OrderReadModelResponse.java | Order read-model 응답 DTO 신규 추가 |
| notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/client/order/OrderInternalClient.java | orderservice FeignClient 신규 추가 |
| notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/client/ai/GeminiApiClient.java | WebClient 빈 충돌 방지(qualifier) 및 생성자 주입 명시 |
| notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/client/ai/config/GeminiApiConfig.java | WebClient 빈 이름을 geminiWebClient로 명시 |
| notification-service/src/main/java/com/shipflow/notificationservice/domain/slack/SlackMessage.java | Slack 메시지 수정 메서드 위치/정리 |
| notification-service/src/main/java/com/shipflow/notificationservice/domain/slack/repository/SlackMessageRepository.java | 목록 조회를 search(Pageable)로 변경 |
| notification-service/src/main/java/com/shipflow/notificationservice/domain/slack/exception/SlackErrorCode.java | 접근 거부 에러코드 추가 |
| notification-service/src/main/java/com/shipflow/notificationservice/domain/ai/repository/AiLogRepository.java | 목록 조회를 search(Pageable)로 변경 |
| notification-service/src/main/java/com/shipflow/notificationservice/domain/ai/exception/AiErrorCode.java | 접근 거부 에러코드 추가 |
| notification-service/src/main/java/com/shipflow/notificationservice/domain/ai/AiLog.java | 포맷/공백 정리 |
| notification-service/src/main/java/com/shipflow/notificationservice/config/SwaggerConfig.java | OpenAPI 기본 정보 구성 추가 |
| notification-service/src/main/java/com/shipflow/notificationservice/config/JPAConfig.java | AuditorAware를 X-User-Id 헤더 기반으로 변경 |
| notification-service/src/main/java/com/shipflow/notificationservice/application/slack/SlackAppService.java | 역할 검증 추가 및 목록 조회를 페이징/검색으로 전환 |
| notification-service/src/main/java/com/shipflow/notificationservice/application/slack/dto/command/UpdateSlackMessageCommand.java | userId/userRole 포함 |
| notification-service/src/main/java/com/shipflow/notificationservice/application/slack/dto/command/SendSlackMessageCommand.java | userId/userRole 포함 |
| notification-service/src/main/java/com/shipflow/notificationservice/application/slack/dto/command/SearchSlackMessageCommand.java | 검색 조건 커맨드 신규 추가 |
| notification-service/src/main/java/com/shipflow/notificationservice/application/NotificationOrchestratorService.java | 이벤트 기반 AI 생성 + Slack 발송 오케스트레이션 구현 |
| notification-service/src/main/java/com/shipflow/notificationservice/application/ai/dto/command/SearchAiLogCommand.java | AI 로그 검색 조건 커맨드 신규 추가 |
| notification-service/src/main/java/com/shipflow/notificationservice/application/ai/dto/command/GenerateDeadlineCommand.java | 이벤트/주문 컨텍스트 확장(주문번호/수량 등) |
| notification-service/src/main/java/com/shipflow/notificationservice/application/ai/AiAppService.java | AI 로그 조회/검색 API 변경 및 입력 검증 강화 |
| notification-service/build.gradle | Feign/Eureka/Swagger 의존성 추가 |
| keycloak/shipflow-export.json | Keycloak export 갱신(사용자/키/설정 포함) |
| docker-compose.yml | notificationservice 환경변수 및 포트/헬스체크 포맷 정리 |
| common/src/main/java/com/shipflow/common/domain/BaseEntity.java | 포맷/공백 정리 |
Comments suppressed due to low confidence (1)
notification-service/src/main/java/com/shipflow/notificationservice/application/slack/SlackAppService.java:56
sendSlackMessage()에서 Slack 전송 실패(BusinessException)를 catch 후 예외를 전파하지 않아, 호출자는 실패 여부를 예외로 감지할 수 없습니다(오케스트레이터에서 성공 처리로 이어질 수 있음). 실패를 호출자에게 전파하거나, 메서드 계약을 '항상 성공 응답 + sendStatus로 실패 전달'로 고정하고 이를 사용하는 쪽에서 sendStatus 기반으로 처리하도록 정리해 주세요.
try {
SlackSendInfo result = slackSender.sendMessage(
command.receiverSlackId(),
command.message()
);
slackMessage.markSuccess(result.slackTs(), result.slackChannelId());
} catch (BusinessException e) {
slackMessage.markFail();
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
48
to
52
| SlackMessageResponse.from( | ||
| slackAppService.sendSlackMessage(request.toCommand()) | ||
| slackAppService.sendSlackMessage( | ||
| request.toCommand(UUID.fromString(userId), userRole) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
X-User-Id를 UUID.fromString(userId)로 바로 파싱하고 있어, 헤더가 누락/오염된 경우 IllegalArgumentException으로 500이 발생합니다. 헤더 UUID 형식 검증(예: @Validated + 커스텀 validator)이나 예외를 400으로 매핑하는 처리가 필요합니다.
Comment on lines
54
to
57
| return ApiResponse.ok( | ||
| AiLogResponse.from( | ||
| aiAppService.generateAiLog(request.toCommand()) | ||
| aiAppService.generateAiLog(request.toCommand(UUID.fromString(userId))) | ||
| ) |
There was a problem hiding this comment.
X-User-Id를 UUID.fromString(userId)로 바로 파싱해 잘못된 값이 들어오면 500으로 터질 수 있습니다. 입력 검증을 추가하거나(예: UUID 타입으로 바인딩/검증), 파싱 실패를 400으로 변환하는 예외 처리를 추가해 주세요.
Comment on lines
+13
to
+23
| @NotNull UUID orderId, | ||
| @NotNull UUID relatedShipmentId, | ||
| @NotNull UUID shipmentManagerId, | ||
| @NotNull String receiverSlackId, | ||
| @NotNull UUID productId, | ||
| @NotNull String product, | ||
| @NotNull Integer quantity, | ||
| @NotNull UUID departureHubId, | ||
| @NotNull String fromHub, | ||
| @NotNull UUID arrivalHubId, | ||
| @NotNull String toHub, |
There was a problem hiding this comment.
GenerateDeadlineRequest에서 receiverSlackId, product, fromHub, toHub 등이 @NotNull만 적용되어 빈 문자열이 통과할 수 있습니다. 기존 의도대로라면 @NotBlank(필요 시 Slack ID는 @Pattern)로 강화해 잘못된 입력이 AI 호출까지 전달되지 않게 해주세요.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
notification-service/src/main/java/com/shipflow/notificationservice/domain/slack/repository/SlackMessageRepository.java (1)
9-18: 🛠️ Refactor suggestion | 🟠 Major도메인 저장소 계약이 애플리케이션 DTO에 역의존하고 있습니다.
SlackMessageRepository가SearchSlackMessageCommand를 직접 받기 시작하면서 domain 레이어가 application 레이어에 묶였습니다. 여기에Pageable까지 함께 노출돼서 저장소 계약이 프레임워크/유스케이스 세부사항을 그대로 안게 됩니다. 검색 조건은 domain용 criteria로 분리하고, 애플리케이션 서비스에서 command와 pageable을 그 타입으로 매핑하는 쪽이 계층 경계를 지키기 쉽습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/domain/slack/repository/SlackMessageRepository.java` around lines 9 - 18, SlackMessageRepository currently depends on the application DTO SearchSlackMessageCommand and exposes framework type Pageable, creating a reverse dependency and leaking application/framework concerns into the domain; change the repository contract to accept a domain-level search criteria (e.g., a new SlackMessageSearchCriteria or SlackMessageQuery object) and return a Page<SlackMessage> (or another domain-friendly paginated result) without importing application DTOs or framework types in the domain package; then update the application layer to map SearchSlackMessageCommand and Pageable into SlackMessageSearchCriteria and call SlackMessageRepository.search(criteria) so the domain (SlackMessageRepository, save, findByIdAndDeletedAtIsNull, search) no longer depends on application DTOs or Pageable.notification-service/src/main/java/com/shipflow/notificationservice/domain/ai/repository/AiLogRepository.java (1)
9-18: 🛠️ Refactor suggestion | 🟠 Major도메인 저장소가 Application DTO에 의존하고 있어 레이어 경계가 깨집니다.
AiLogRepository는 도메인 계층 포트인데SearchAiLogCommand(application 패키지)를 직접 받으면 의존 방향이 역전됩니다. 검색 조건 타입을 도메인/인프라 공용 쿼리 모델로 분리해 주세요.🧩 제안 diff
-import com.shipflow.notificationservice.application.ai.dto.command.SearchAiLogCommand; +import com.shipflow.notificationservice.domain.ai.repository.query.AiLogSearchCriteria; ... -Page<AiLog> search(SearchAiLogCommand command, Pageable pageable); +Page<AiLog> search(AiLogSearchCriteria criteria, Pageable pageable);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/domain/ai/repository/AiLogRepository.java` around lines 9 - 18, AiLogRepository currently depends on the application DTO SearchAiLogCommand which breaks layer boundaries; introduce a domain-level query model (e.g., AiLogSearchCriteria or AiLogQuery) in the domain package and change the repository signature Page<AiLog> search(SearchAiLogCommand command, Pageable pageable) to use that domain query type instead, then update infrastructure/implementation classes to map from SearchAiLogCommand (application layer) to the new domain query before calling AiLogRepository.search, ensuring all references to SearchAiLogCommand are removed from the domain/repository API.notification-service/src/main/java/com/shipflow/notificationservice/application/slack/SlackAppService.java (1)
48-56:⚠️ Potential issue | 🟡 Minor
BusinessException외의 예외 발생 시 메시지 상태가 불명확해집니다.
catch블록이BusinessException만 처리합니다. 네트워크 타임아웃이나 기타 런타임 예외가 발생하면markFail()이 호출되지 않고 예외가 전파되어 트랜잭션이 롤백됩니다. 이 경우 메시지가 저장되지 않거나 상태가 불명확해질 수 있습니다.🔧 모든 예외 처리 제안
try { SlackSendInfo result = slackSender.sendMessage( command.receiverSlackId(), command.message() ); slackMessage.markSuccess(result.slackTs(), result.slackChannelId()); - } catch (BusinessException e) { + } catch (Exception e) { slackMessage.markFail(); + if (e instanceof BusinessException) { + throw e; + } + throw new BusinessException(SlackErrorCode.SLACK_SEND_FAILED); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/application/slack/SlackAppService.java` around lines 48 - 56, The current try/catch only handles BusinessException so other exceptions (e.g., network timeouts, runtime errors) leave slackMessage state unset; update SlackAppService to add a broad catch (e.g., catch Exception) around slackSender.sendMessage that calls slackMessage.markFail() for any non-BusinessException error (in addition to the existing BusinessException branch) and then rethrow or propagate the exception so the failure is recorded; reference slackSender.sendMessage(...), slackMessage.markFail(), slackMessage.markSuccess(...), and the existing BusinessException catch when making the change.notification-service/src/main/java/com/shipflow/notificationservice/application/ai/AiAppService.java (1)
58-64:⚠️ Potential issue | 🟠 Major트랜잭션 롤백으로 인해 실패 상태가 저장되지 않습니다.
catch블록에서aiLog.markFail()을 호출하지만, 예외가 다시 던져지면 전체@Transactional이 롤백되어 실패 상태가 DB에 저장되지 않습니다. AI 호출 실패 이력을 추적하려면 별도 트랜잭션이 필요합니다.🔧 해결 방안
- 별도 서비스로 분리:
markFail을REQUIRES_NEW전파 속성을 가진 별도 메서드로 분리- 또는 TransactionTemplate 사용: 실패 마킹을 새로운 트랜잭션에서 처리
// 예시: 별도 컴포넌트에서 처리 `@Service` public class AiLogStatusUpdater { `@Transactional`(propagation = Propagation.REQUIRES_NEW) public void markFail(UUID aiLogId) { AiLog aiLog = aiLogRepository.findById(aiLogId) .orElseThrow(...); aiLog.markFail(); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/application/ai/AiAppService.java` around lines 58 - 64, The current catch blocks in AiAppService call aiLog.markFail() but rethrow exceptions causing the outer `@Transactional` to roll back and the failure state not to persist; extract the failure-update into a separate component or method that executes in a new transaction (e.g., create AiLogStatusUpdater.markFail(UUID aiLogId) annotated with `@Transactional`(propagation = Propagation.REQUIRES_NEW) or invoke TransactionTemplate.execute with PROPAGATION_REQUIRES_NEW) and have the catch blocks call that new method (passing aiLog id rather than a possibly detached aiLog entity) so the failure state is saved even when the surrounding transaction rolls back.
🧹 Nitpick comments (18)
notification-service/src/main/java/com/shipflow/notificationservice/application/ai/dto/command/GenerateDeadlineCommand.java (1)
11-18: 현재 이벤트 입력보다 커맨드 시그니처가 더 넓습니다.관련 호출부를 보면
ordererId는SYSTEM_USER_ID로 대체되고,supplierCompanyId/receiverCompanyId는null로 넘어갑니다. 지금 형태는 실제 필수 입력을 표현하지 못하므로, 직접 API 호출용 커맨드와shipment.created오케스트레이션용 커맨드를 분리하거나, 이벤트 경로에서 없는 값은 별도 보강 객체로 분리하는 편이 이후 검증/확장에 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/application/ai/dto/command/GenerateDeadlineCommand.java` around lines 11 - 18, GenerateDeadlineCommand currently exposes a wider signature than the events that produce it (fields ordererId, supplierCompanyId, receiverCompanyId are not provided by the shipment.created path and are passed as SYSTEM_USER_ID/null), so narrow or separate the API-facing and event-facing representations: either split GenerateDeadlineCommand into two distinct DTOs (e.g., ApiGenerateDeadlineCommand for direct API calls and EventGenerateDeadlineCommand for orchestration) or move optional/absent fields (ordererId, supplierCompanyId, receiverCompanyId) into a separate optional metadata/augmentation object used only by callers that can supply them; update code paths that construct GenerateDeadlineCommand to use the correct DTO or attach the augmentation object and adjust any validation accordingly.notification-service/src/main/java/com/shipflow/notificationservice/domain/slack/SlackMessage.java (1)
85-90: 인라인 변경 이력 주석은 제거해도 좋겠습니다.Line [85]의
// userId 파라미터 제거는 커밋 이력으로 충분히 추적 가능해서, 코드 본문에서는 제거하는 편이 가독성에 유리합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/domain/slack/SlackMessage.java` around lines 85 - 90, Remove the inline history comment from the SlackMessage.updateMessage method: locate the method named updateMessage(String newMessage) in class SlackMessage and delete the trailing inline comment "// userId 파라미터 제거" so the method contains only the validation and assignment logic (preserve the null/blank check and BusinessException usage).notification-service/src/main/java/com/shipflow/notificationservice/config/SwaggerConfig.java (1)
12-19: OpenAPI에 인증 헤더 스키마도 함께 선언하는 것을 권장합니다.현재 메타 정보는 충분하지만, 이번 PR에서 사용하는
X-User-Id,X-User-Role를 스키마에 반영하면 Swagger에서 호출 재현성이 크게 좋아집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/config/SwaggerConfig.java` around lines 12 - 19, The OpenAPI bean (openAPI()) currently only sets Info metadata; add header security schema declarations for the custom headers `X-User-Id` and `X-User-Role` by adding a Components().addSecuritySchemes(...) with two ApiKey schemas (in: header, type: apiKey) and then attach a global SecurityRequirement referencing those scheme names to the returned OpenAPI object so Swagger UI shows and sends these headers when trying endpoints; update the OpenAPI construction in the openAPI() method to include these Components and SecurityRequirement entries.notification-service/src/test/java/com/shipflow/notificationservice/AiAppServiceTest.java (1)
154-174: 권한 검증 회귀 방지를 위해 거부 케이스 테스트를 추가해주세요.Line [171]에서
userRole인자를 추가한 것은 좋습니다. 다만 현재는 성공 케이스만 있어 권한 검증(MASTER외) 회귀를 잡기 어렵습니다.테스트 보강 예시
`@Test` void get_success() { @@ assertThat(result.requestStatus()).isEqualTo(AiRequestStatus.SUCCESS); } + +@Test +void get_fail_forbidden_when_not_master() { + UUID id = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + String userRole = "MANAGER"; + + assertThatThrownBy(() -> aiAppService.getAiLog(userId, userRole, id)) + .isInstanceOf(BusinessException.class); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/test/java/com/shipflow/notificationservice/AiAppServiceTest.java` around lines 154 - 174, Add a new negative test in AiAppServiceTest (next to get_success()) that verifies permission checks by calling aiAppService.getAiLog with the same id/userId but a non-MASTER userRole (e.g., "USER") after stubbing aiLogRepository.findByIdAndDeletedAtIsNull(id) to return the AiLog; assert that the call fails with the expected access-denied behavior (throwing the service's authorization exception, e.g., AccessDeniedException or the domain-specific exception) rather than returning a success result so permission regression is caught.notification-service/src/main/java/com/shipflow/notificationservice/application/ai/dto/command/SearchAiLogCommand.java (1)
17-20: 기간 필터 역전(createdAtFrom > createdAtTo) 가드를 추가하는 것이 좋습니다.검색 조건 불변식을 커맨드 생성 시점에 막아 두면, 하위 쿼리 계층이 단순해지고 잘못된 요청을 빠르게 차단할 수 있습니다.
✅ 제안 diff
public record SearchAiLogCommand( UUID userId, String userRole, UUID shipmentManagerId, AiRequestType requestType, AiRequestStatus requestStatus, LocalDate workDate, LocalDateTime createdAtFrom, LocalDateTime createdAtTo ) { + public SearchAiLogCommand { + if (createdAtFrom != null && createdAtTo != null && createdAtFrom.isAfter(createdAtTo)) { + throw new IllegalArgumentException("createdAtFrom must be before or equal to createdAtTo"); + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/application/ai/dto/command/SearchAiLogCommand.java` around lines 17 - 20, Add a guard in the SearchAiLogCommand constructor to validate the createdAt range: inside the constructor of class SearchAiLogCommand, check if both createdAtFrom and createdAtTo are non-null and if createdAtFrom.isAfter(createdAtTo) then throw an IllegalArgumentException (or similar) with a clear message; this enforces the invariant at object creation and prevents downstream query errors when using createdAtFrom/createdAtTo.notification-service/src/main/java/com/shipflow/notificationservice/application/slack/dto/command/UpdateSlackMessageCommand.java (1)
6-7:userRole를 문자열로 두기보다 enum/값 객체로 고정하는 것을 권장합니다.권한 값 오타/임의 문자열 유입을 컴파일 타임에 막을 수 있어, 인가 로직 안정성이 올라갑니다.
♻️ 제안 diff
public record UpdateSlackMessageCommand( UUID userId, - String userRole, + UserRole userRole, UUID slackId, String message ) { }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/application/slack/dto/command/UpdateSlackMessageCommand.java` around lines 6 - 7, Replace the loose String userRole in UpdateSlackMessageCommand with a strongly-typed enum/value object to prevent invalid role strings; add a UserRole enum (or reuse an existing one) and change the UpdateSlackMessageCommand field signature, constructor, getters, and any builders/factories from String userRole to UserRole userRole, and update call sites that construct or read this command (parsers, mappers, tests) to convert incoming strings to UserRole (e.g., via UserRole.valueOf or a safe from(String) factory) so compilation enforces valid roles.notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/client/order/OrderInternalClient.java (1)
13-13: Feign 클라이언트에서@PathVariable의 명시적 바인딩을 고려하세요.현재 형태는 작동하지만(Spring Boot 3.5.13은 자동으로 파라미터 메타데이터를 활성화함), 명시적 이름 바인딩이 코드 가독성과 유지보수성을 높입니다.
🔧 제안 diff
-OrderReadModelResponse getOrderReadModel(`@PathVariable` UUID orderId); +OrderReadModelResponse getOrderReadModel(`@PathVariable`("orderId") UUID orderId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/client/order/OrderInternalClient.java` at line 13, The Feign client method getOrderReadModel currently uses `@PathVariable` without an explicit name; update the OrderInternalClient interface by annotating the method parameter with a named path variable (e.g., `@PathVariable`("orderId") UUID orderId) so the parameter binding is explicit and stable across frameworks and refactors; ensure the method signature in OrderInternalClient reflects this named binding.notification-service/src/test/java/com/shipflow/notificationservice/SlackAppServiceTest.java (1)
59-66: 권한 검증 회귀를 막는 케이스가 없습니다.이번 변경의 핵심이
userId/userRole추가인데, 이 파일은 전부MASTER경로만 검증합니다.USER나 권한 없는 호출에서BusinessException이 발생하고slackMessageRepository/slackSender가 호출되지 않는 케이스를 최소 한두 개는 추가해 두는 편이 안전합니다.Also applies to: 154-171, 204-228, 257-262, 361-383
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/test/java/com/shipflow/notificationservice/SlackAppServiceTest.java` around lines 59 - 66, Add negative permission test cases in SlackAppServiceTest to prevent regression: create at least one SendSlackMessageCommand instance using a non-MASTER role (e.g., "USER" and an explicitly unauthorized role), invoke the same service method used in existing tests, assert that a BusinessException is thrown, and verify slackMessageRepository and slackSender are never called (use your mocks' verifyZeroInteractions/verifyNoMoreInteractions or equivalent). Apply the same pattern to the other test blocks referenced (around lines 154-171, 204-228, 257-262, 361-383) so each positive MASTER-path test has corresponding unauthorized-role test coverage.notification-service/src/test/java/com/shipflow/notificationservice/NotificationOrchestratorServiceTest.java (2)
71-82: 성공 케이스 검증이 너무 느슨합니다.
sendSlackMessage(any())와generateAiLog(any())만 확인하면 수신자,relatedShipmentId,relatedAiLogId,messageType, 생성된 메시지 본문이 잘못 매핑돼도 테스트가 통과합니다.ArgumentCaptor로 오케스트레이션의 핵심 필드를 잡아 두는 편이 회귀 방지에 훨씬 도움이 됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/test/java/com/shipflow/notificationservice/NotificationOrchestratorServiceTest.java` around lines 71 - 82, The test's verifications are too loose—replace the generic verify(...).sendSlackMessage(any()) and generateAiLog(any()) checks with ArgumentCaptor usage to capture the actual objects passed from notificationOrchestratorService.handleShipmentCreated(event) (capture the Slack message DTO sent via slackAppService.sendSlackMessage and the AI request/log object passed to aiAppService.generateAiLog), then assert their core fields (recipient/targets, relatedShipmentId, relatedAiLogId, messageType, and the generated message body/content) match expected values derived from the test fixtures; keep existing mocks for orderInternalClient.getOrderReadModel and aiLogRepository but add assertions on the captured arguments to prevent regressions in mapping/field population.
106-139:slackId == null경로의 기대 동작을 더 명확히 고정해 주세요.지금은 예외 유무만 확인해서
UNKNOWN치환, 전송 스킵, 잘못된 값으로 발송 시도 모두 통과합니다. 수신자 누락은 알림 유실과 직결되니,SlackAppService호출 여부나 전달된receiverSlackId를 명시적으로 검증해 두는 편이 안전합니다. 만약 실제 정책이UNKNOWN치환이라면, 이번 PR에서 강화한 Slack ID 검증 규칙과도 다시 맞춰보는 게 좋겠습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/test/java/com/shipflow/notificationservice/NotificationOrchestratorServiceTest.java` around lines 106 - 139, The test currently only asserts no exception for handleShipmentCreated but doesn't verify Slack delivery behavior; update the test method handleShipmentCreated_nullSlackId to explicitly verify SlackAppService interactions: after stubbing orderInternalClient.getOrderReadModel(...) and invoking notificationOrchestratorService.handleShipmentCreated(event), add a Mockito verify on the SlackAppService (e.g., verify(slackAppService,...)) to assert either that the send method was never called when missing receivers OR that it was called with receiverSlackId equal to "UNKNOWN" (choose the one matching current policy); reference notificationOrchestratorService.handleShipmentCreated and the SlackAppService send method when adding the verify to make the expectation explicit.notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/persistence/ai/AiLogRepositoryImpl.java (1)
54-60:Pageable의 sort가 현재 무시됩니다.조회는 항상
createdAt DESC로 고정돼서 API가 넘긴 정렬 파라미터가 반영되지 않습니다. 정렬을 고정할 의도라면 API 계약을 단순화하고, 아니면pageable.getSort()를 QueryDSL로 매핑해 주세요. 추가로 같은createdAt묶음에서 페이지 경계가 흔들리지 않도록idtie-breaker도 두는 편이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/persistence/ai/AiLogRepositoryImpl.java` around lines 54 - 60, The query always forces aiLog.createdAt.desc(), ignoring the Pageable sort; update AiLogRepositoryImpl where queryFactory builds the query (using aiLog and builder) to map pageable.getSort() into QueryDSL OrderSpecifier(s) instead of hardcoding createdAt desc, and append a deterministic tie-breaker (aiLog.id.asc() or desc matching createdAt direction) so pagination boundaries are stable; fall back to createdAt.desc() if pageable.getSort() is empty. Ensure the mapping handles multiple sort orders and uses the same aiLog field names when constructing OrderSpecifier instances.notification-service/src/main/java/com/shipflow/notificationservice/presentation/ai/external/AiController.java (2)
50-52: 역할 검증이 컨트롤러와 서비스에서 중복됩니다.디버그 엔드포인트에서 컨트롤러 레벨 역할 검증을 수행하지만,
AiAppService의 다른 메서드들은 서비스 레벨에서 검증합니다. 디버그 목적의 조기 실패라면 괜찮지만, 일관성을 위해 서비스 레이어에 검증을 위임하거나 주석으로 의도를 명시하는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/presentation/ai/external/AiController.java` around lines 50 - 52, The controller currently duplicates role validation by checking userRole and throwing BusinessException(AiErrorCode.FORBIDDEN_AI_ACCESS) while AiAppService also enforces role checks; remove the controller-level check in AiController (the if ("MASTER".equals(userRole)) block) and delegate authorization entirely to AiAppService methods so validation is centralized, or if this check is intentionally an early debug guard, add a clear comment above the userRole check explaining it's a debug-only fast-fail and keep it; ensure any callers now rely on AiAppService for enforcement and update/cover with unit tests for AiAppService authorization behavior.
56-56:UUID.fromString()파싱 실패 시 예외 처리가 필요합니다.
X-User-Id헤더 값이 유효한 UUID 형식이 아닐 경우IllegalArgumentException이 발생합니다. 게이트웨이에서 검증된 값만 전달된다면 문제없지만, 방어적 프로그래밍을 위해 예외 처리나 글로벌 예외 핸들러에서 처리하는 것을 권장합니다.🛡️ 방어적 처리 예시
글로벌 예외 핸들러에서
IllegalArgumentException을 처리하거나, 컨트롤러에서 명시적으로 변환:private UUID parseUserId(String userId) { try { return UUID.fromString(userId); } catch (IllegalArgumentException e) { throw new BusinessException(CommonErrorCode.INVALID_USER_ID); } }Also applies to: 70-70, 95-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/presentation/ai/external/AiController.java` at line 56, The call UUID.fromString(userId) in AiController (used inside request.toCommand(UUID.fromString(userId)) and the other occurrences at the same controller) can throw IllegalArgumentException for malformed X-User-Id; add defensive parsing by extracting a helper like parseUserId(String userId) that tries UUID.fromString(...) and on failure throws a controlled BusinessException (e.g., CommonErrorCode.INVALID_USER_ID) or returns a validated UUID, then replace direct UUID.fromString(...) uses in aiAppService.generateAiLog(...) and the other two locations with parseUserId(userId); alternatively ensure a global exception handler maps IllegalArgumentException to the same BusinessException.notification-service/src/main/java/com/shipflow/notificationservice/presentation/ai/dto/request/GenerateDeadlineRequest.java (2)
16-23: String 필드에@NotBlank사용을 권장합니다.
receiverSlackId,product,fromHub,toHub필드가@NotNull만 사용하고 있어 빈 문자열("")이 허용됩니다.AiAppService.validateCommand()에서 blank 체크를 수행하지만, 요청 단계에서 조기 실패를 위해@NotBlank사용을 권장합니다.♻️ `@NotBlank` 적용 제안
- `@NotNull` String receiverSlackId, + `@NotBlank` String receiverSlackId, `@NotNull` UUID productId, - `@NotNull` String product, + `@NotBlank` String product, `@NotNull` Integer quantity, `@NotNull` UUID departureHubId, - `@NotNull` String fromHub, + `@NotBlank` String fromHub, `@NotNull` UUID arrivalHubId, - `@NotNull` String toHub, + `@NotBlank` String toHub,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/presentation/ai/dto/request/GenerateDeadlineRequest.java` around lines 16 - 23, In GenerateDeadlineRequest, replace or augment `@NotNull` on the String fields receiverSlackId, product, fromHub, and toHub with `@NotBlank` so empty strings are rejected at request validation time; update the imports to include javax.validation.constraints.NotBlank and keep UUID/Integer fields with `@NotNull`, and ensure this complements AiAppService.validateCommand() by failing fast on blank inputs.
36-37: null 값 전달이 의도된 것인지 확인이 필요합니다.
supplierCompanyId,receiverCompanyId,workDate에null이 명시적으로 전달되고 있습니다. 현재 코드 흐름에서 이 값들이 사용되지 않아 문제가 없지만, 향후 이 필드들이 필요해지면 수정이 필요합니다. 의도된 설계라면 코드 주석으로 명시하는 것이 좋습니다.Also applies to: 50-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/presentation/ai/dto/request/GenerateDeadlineRequest.java` around lines 36 - 37, The three arguments supplierCompanyId, receiverCompanyId, and workDate are being passed as literal nulls into the GenerateDeadlineRequest construction (see GenerateDeadlineRequest constructor/builder usage); confirm intent and either (a) supply the correct values from the call site, (b) make the fields optional in GenerateDeadlineRequest (e.g., Optional or nullable annotations) and add validation where used, or (c) if nulls are intentional, add an inline comment or Javadoc at the call site and in the GenerateDeadlineRequest class documenting why these fields may be null and the expected behavior when they are absent so future changes won’t accidentally break assumptions.notification-service/src/main/java/com/shipflow/notificationservice/application/NotificationOrchestratorService.java (1)
106-114: 예외를 삼키면 디버깅이 어려워질 수 있습니다.
getOrderReadModel에서 모든 예외를 catch하고null을 반환합니다. 이는 회복력 패턴이지만, 네트워크 오류와 실제 데이터 부재(404)를 구분할 수 없어 문제 진단이 어려워집니다. 최소한 로그를 남기는 것을 권장합니다.🔧 로깅 추가 제안
private OrderReadModelResponse getOrderReadModel(UUID orderId) { if (orderId == null) return null; try { return orderInternalClient.getOrderReadModel(orderId); } catch (Exception e) { + log.warn("주문 정보 조회 실패 - orderId: {}, error: {}", orderId, e.getMessage()); return null; } }Lombok
@Slf4j어노테이션 추가가 필요합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/application/NotificationOrchestratorService.java` around lines 106 - 114, getOrderReadModel currently swallows all exceptions and returns null; add logging so failures can be diagnosed: annotate NotificationOrchestratorService with Lombok `@Slf4j`, and in getOrderReadModel's catch block call log.warn or log.error with a clear message including the orderId and the caught Exception (e) and any relevant context from orderInternalClient.getOrderReadModel, then continue returning null; this preserves behavior but emits useful diagnostic information.notification-service/src/main/java/com/shipflow/notificationservice/application/ai/AiAppService.java (1)
67-74: 단건 조회에서userId파라미터가 사용되지 않습니다.
getAiLog(UUID userId, String userRole, UUID aiId)메서드에서userId파라미터를 받지만 실제로 사용하지 않습니다. 향후 감사 로깅이나 사용자별 필터링을 위해 의도적으로 추가한 것이라면 주석으로 명시하거나, 불필요하다면 제거하는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/application/ai/AiAppService.java` around lines 67 - 74, getAiLog currently accepts a userId but never uses it; either incorporate it into the method for auditing/authorization or remove it to avoid dead API surface. If you intended to record the caller, update getAiLog(UUID userId, String userRole, UUID aiId) to log an audit entry (e.g., via process/audit logger) including userId and aiId after validateMasterRole and before returning AiLogResult.from(aiLog), referencing getAiLog, validateMasterRole, aiLogRepository and AiLogResult; otherwise remove the unused userId parameter from getAiLog and update all callers to match the new signature.notification-service/src/main/java/com/shipflow/notificationservice/presentation/slack/external/SlackController.java (1)
83-83:Pageable과BasePageRequest사용이 일관되지 않습니다.
SlackController는Pageable을 직접 사용하고,AiController는BasePageRequest를 사용합니다. 두 방식 모두 동작하지만, 프로젝트 전체의 일관성을 위해 하나로 통일하는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/presentation/slack/external/SlackController.java` at line 83, The SlackController currently accepts a Spring Pageable parameter (see the method signature containing "Pageable pageable") while other controllers like AiController use the project's BasePageRequest; make them consistent by replacing the Pageable parameter with BasePageRequest in SlackController's endpoint signature and adapt the method to convert BasePageRequest into a Pageable (e.g., call an existing toPageable()/toPageableRequest() helper or construct a PageRequest from page/size/sort fields) before passing it to service/repository calls; update any usages inside SlackController methods that reference "pageable" to use the converted Pageable variable and ensure import of BasePageRequest.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@notification-service/src/main/java/com/shipflow/notificationservice/application/slack/SlackAppService.java`:
- Around line 124-128: The validation in validateCreateRole currently checks for
"DELIVERY_MANAGER" which doesn't exist in Keycloak; remove "DELIVERY_MANAGER"
from the allowed roles set in validateCreateRole so it only contains "MASTER",
"HUB_MANAGER", "COMPANY_MANAGER", and keep throwing
BusinessException(SlackErrorCode.FORBIDDEN_SLACK_ACCESS) for other values to
preserve behavior; update any related tests or callers that assumed
DELIVERY_MANAGER was valid.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/config/JPAConfig.java`:
- Around line 43-54: The current logic silently replaces missing/invalid
X-User-Id with SYSTEM_UUID; change it so SYSTEM_UUID is returned only when there
is no request context (attrs == null), and for cases where a request exists but
the header is missing/blank or UUID.fromString fails, do not fall back
silently—throw a clear exception (e.g., IllegalArgumentException) or return an
explicit empty/failed result instead; update the try/catch around
RequestContextHolder.getRequestAttributes(), the code that reads
getRequest().getHeader("X-User-Id"), and the UUID.fromString parsing so parsing
errors are not caught and replaced with SYSTEM_UUID, and remove the broad catch
that unconditionally returns SYSTEM_UUID.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/client/ai/config/GeminiApiConfig.java`:
- Around line 11-13: The geminiWebClient bean in GeminiApiConfig lacks
connection and response timeouts which can lead to resource hang when the
external Gemini API is slow; update geminiWebClient() to build a Reactor Netty
HttpClient with a connect timeout (ChannelOption.CONNECT_TIMEOUT_MILLIS) and a
response/read timeout (e.g., HttpClient#responseTimeout or doOnConnected with
ReadTimeoutHandler/WriteTimeoutHandler), plug that HttpClient into
WebClient.builder() via a ReactorClientHttpConnector, and choose sensible
durations (e.g., a few seconds) so all external calls from geminiWebClient have
explicit timeouts.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/messaging/consumer/ShipmentCreatedHandler.java`:
- Around line 18-20: Add an idempotency guard around
ShipmentCreatedHandler.process: before calling
notificationOrchestratorService.handleShipmentCreated(event) check a durable
store/table for SagaEvent.eventId to see if this eventId was already processed
and return early if so; if not, call handleShipmentCreated and then persist a
“processed” record for that eventId (or update saga state) after successful
completion so future redeliveries are skipped. Use the unique symbols
ShipmentCreatedHandler.process,
notificationOrchestratorService.handleShipmentCreated, AbstractSagaHandler flow,
and SagaEvent.eventId to locate code and ensure the check-and-record is
atomic/transactional with any state changes (or write the processed marker after
success) to prevent duplicate Slack sends and AiLog updates on redelivery.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/messaging/dto/ShipmentCreatedEvent.java`:
- Around line 17-20: The Lombok-generated `@AllArgsConstructor` bypasses
SagaEvent's protected SagaEvent(String) initializer so ShipmentCreatedEvent
instances created directly (e.g., in NotificationOrchestratorServiceTest and
OrderMessageFlowTest) end up with null parent metadata; remove
`@AllArgsConstructor` and `@Builder` from ShipmentCreatedEvent and keep/add
`@NoArgsConstructor` and `@JsonIgnoreProperties`(ignoreUnknown = true) so Jackson
can deserialize correctly, and if this DTO must also be produced within the app
add an explicit constructor in ShipmentCreatedEvent that calls
super("shipment.created") to initialize eventId/eventType/occurredAt.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/persistence/slack/SlackMessageRepositoryImpl.java`:
- Around line 53-58: The query in SlackMessageRepositoryImpl uses
queryFactory.selectFrom(slackMessage).where(builder).offset(...).limit(...).fetch()
without orderBy, causing unstable pagination; add a deterministic sort before
fetch by either applying pageable.getSort() mapped to QueryDSL orderSpecifiers
or at minimum an explicit stable order (e.g., slackMessage.createdAt desc/asc)
via query.orderBy(...). Implement or call a helper to convert Pageable/Sort to
QueryDSL OrderSpecifier(s) and apply it to the JPAQuery prior to fetch() so
pages are stable and deterministic.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/presentation/slack/dto/request/SendSlackMessageRequest.java`:
- Around line 14-15: The `@Pattern` on SendSlackMessageRequest currently restricts
Slack IDs to prefixes U or C; update the validation regex on the
SendSlackMessageRequest class (the `@Pattern` annotation) to accept all Slack ID
types supported by SlackApiClient by changing the pattern to include U, C, D,
and G (e.g., "^[UCDG][A-Z0-9]+$") so DM (D...) and private channel (G...) IDs
are validated correctly; ensure the validation message remains appropriate for
the broader set.
---
Outside diff comments:
In
`@notification-service/src/main/java/com/shipflow/notificationservice/application/ai/AiAppService.java`:
- Around line 58-64: The current catch blocks in AiAppService call
aiLog.markFail() but rethrow exceptions causing the outer `@Transactional` to roll
back and the failure state not to persist; extract the failure-update into a
separate component or method that executes in a new transaction (e.g., create
AiLogStatusUpdater.markFail(UUID aiLogId) annotated with
`@Transactional`(propagation = Propagation.REQUIRES_NEW) or invoke
TransactionTemplate.execute with PROPAGATION_REQUIRES_NEW) and have the catch
blocks call that new method (passing aiLog id rather than a possibly detached
aiLog entity) so the failure state is saved even when the surrounding
transaction rolls back.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/application/slack/SlackAppService.java`:
- Around line 48-56: The current try/catch only handles BusinessException so
other exceptions (e.g., network timeouts, runtime errors) leave slackMessage
state unset; update SlackAppService to add a broad catch (e.g., catch Exception)
around slackSender.sendMessage that calls slackMessage.markFail() for any
non-BusinessException error (in addition to the existing BusinessException
branch) and then rethrow or propagate the exception so the failure is recorded;
reference slackSender.sendMessage(...), slackMessage.markFail(),
slackMessage.markSuccess(...), and the existing BusinessException catch when
making the change.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/domain/ai/repository/AiLogRepository.java`:
- Around line 9-18: AiLogRepository currently depends on the application DTO
SearchAiLogCommand which breaks layer boundaries; introduce a domain-level query
model (e.g., AiLogSearchCriteria or AiLogQuery) in the domain package and change
the repository signature Page<AiLog> search(SearchAiLogCommand command, Pageable
pageable) to use that domain query type instead, then update
infrastructure/implementation classes to map from SearchAiLogCommand
(application layer) to the new domain query before calling
AiLogRepository.search, ensuring all references to SearchAiLogCommand are
removed from the domain/repository API.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/domain/slack/repository/SlackMessageRepository.java`:
- Around line 9-18: SlackMessageRepository currently depends on the application
DTO SearchSlackMessageCommand and exposes framework type Pageable, creating a
reverse dependency and leaking application/framework concerns into the domain;
change the repository contract to accept a domain-level search criteria (e.g., a
new SlackMessageSearchCriteria or SlackMessageQuery object) and return a
Page<SlackMessage> (or another domain-friendly paginated result) without
importing application DTOs or framework types in the domain package; then update
the application layer to map SearchSlackMessageCommand and Pageable into
SlackMessageSearchCriteria and call SlackMessageRepository.search(criteria) so
the domain (SlackMessageRepository, save, findByIdAndDeletedAtIsNull, search) no
longer depends on application DTOs or Pageable.
---
Nitpick comments:
In
`@notification-service/src/main/java/com/shipflow/notificationservice/application/ai/AiAppService.java`:
- Around line 67-74: getAiLog currently accepts a userId but never uses it;
either incorporate it into the method for auditing/authorization or remove it to
avoid dead API surface. If you intended to record the caller, update
getAiLog(UUID userId, String userRole, UUID aiId) to log an audit entry (e.g.,
via process/audit logger) including userId and aiId after validateMasterRole and
before returning AiLogResult.from(aiLog), referencing getAiLog,
validateMasterRole, aiLogRepository and AiLogResult; otherwise remove the unused
userId parameter from getAiLog and update all callers to match the new
signature.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/application/ai/dto/command/GenerateDeadlineCommand.java`:
- Around line 11-18: GenerateDeadlineCommand currently exposes a wider signature
than the events that produce it (fields ordererId, supplierCompanyId,
receiverCompanyId are not provided by the shipment.created path and are passed
as SYSTEM_USER_ID/null), so narrow or separate the API-facing and event-facing
representations: either split GenerateDeadlineCommand into two distinct DTOs
(e.g., ApiGenerateDeadlineCommand for direct API calls and
EventGenerateDeadlineCommand for orchestration) or move optional/absent fields
(ordererId, supplierCompanyId, receiverCompanyId) into a separate optional
metadata/augmentation object used only by callers that can supply them; update
code paths that construct GenerateDeadlineCommand to use the correct DTO or
attach the augmentation object and adjust any validation accordingly.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/application/ai/dto/command/SearchAiLogCommand.java`:
- Around line 17-20: Add a guard in the SearchAiLogCommand constructor to
validate the createdAt range: inside the constructor of class
SearchAiLogCommand, check if both createdAtFrom and createdAtTo are non-null and
if createdAtFrom.isAfter(createdAtTo) then throw an IllegalArgumentException (or
similar) with a clear message; this enforces the invariant at object creation
and prevents downstream query errors when using createdAtFrom/createdAtTo.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/application/NotificationOrchestratorService.java`:
- Around line 106-114: getOrderReadModel currently swallows all exceptions and
returns null; add logging so failures can be diagnosed: annotate
NotificationOrchestratorService with Lombok `@Slf4j`, and in getOrderReadModel's
catch block call log.warn or log.error with a clear message including the
orderId and the caught Exception (e) and any relevant context from
orderInternalClient.getOrderReadModel, then continue returning null; this
preserves behavior but emits useful diagnostic information.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/application/slack/dto/command/UpdateSlackMessageCommand.java`:
- Around line 6-7: Replace the loose String userRole in
UpdateSlackMessageCommand with a strongly-typed enum/value object to prevent
invalid role strings; add a UserRole enum (or reuse an existing one) and change
the UpdateSlackMessageCommand field signature, constructor, getters, and any
builders/factories from String userRole to UserRole userRole, and update call
sites that construct or read this command (parsers, mappers, tests) to convert
incoming strings to UserRole (e.g., via UserRole.valueOf or a safe from(String)
factory) so compilation enforces valid roles.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/config/SwaggerConfig.java`:
- Around line 12-19: The OpenAPI bean (openAPI()) currently only sets Info
metadata; add header security schema declarations for the custom headers
`X-User-Id` and `X-User-Role` by adding a Components().addSecuritySchemes(...)
with two ApiKey schemas (in: header, type: apiKey) and then attach a global
SecurityRequirement referencing those scheme names to the returned OpenAPI
object so Swagger UI shows and sends these headers when trying endpoints; update
the OpenAPI construction in the openAPI() method to include these Components and
SecurityRequirement entries.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/domain/slack/SlackMessage.java`:
- Around line 85-90: Remove the inline history comment from the
SlackMessage.updateMessage method: locate the method named updateMessage(String
newMessage) in class SlackMessage and delete the trailing inline comment "//
userId 파라미터 제거" so the method contains only the validation and assignment logic
(preserve the null/blank check and BusinessException usage).
In
`@notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/client/order/OrderInternalClient.java`:
- Line 13: The Feign client method getOrderReadModel currently uses
`@PathVariable` without an explicit name; update the OrderInternalClient interface
by annotating the method parameter with a named path variable (e.g.,
`@PathVariable`("orderId") UUID orderId) so the parameter binding is explicit and
stable across frameworks and refactors; ensure the method signature in
OrderInternalClient reflects this named binding.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/persistence/ai/AiLogRepositoryImpl.java`:
- Around line 54-60: The query always forces aiLog.createdAt.desc(), ignoring
the Pageable sort; update AiLogRepositoryImpl where queryFactory builds the
query (using aiLog and builder) to map pageable.getSort() into QueryDSL
OrderSpecifier(s) instead of hardcoding createdAt desc, and append a
deterministic tie-breaker (aiLog.id.asc() or desc matching createdAt direction)
so pagination boundaries are stable; fall back to createdAt.desc() if
pageable.getSort() is empty. Ensure the mapping handles multiple sort orders and
uses the same aiLog field names when constructing OrderSpecifier instances.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/presentation/ai/dto/request/GenerateDeadlineRequest.java`:
- Around line 16-23: In GenerateDeadlineRequest, replace or augment `@NotNull` on
the String fields receiverSlackId, product, fromHub, and toHub with `@NotBlank` so
empty strings are rejected at request validation time; update the imports to
include javax.validation.constraints.NotBlank and keep UUID/Integer fields with
`@NotNull`, and ensure this complements AiAppService.validateCommand() by failing
fast on blank inputs.
- Around line 36-37: The three arguments supplierCompanyId, receiverCompanyId,
and workDate are being passed as literal nulls into the GenerateDeadlineRequest
construction (see GenerateDeadlineRequest constructor/builder usage); confirm
intent and either (a) supply the correct values from the call site, (b) make the
fields optional in GenerateDeadlineRequest (e.g., Optional or nullable
annotations) and add validation where used, or (c) if nulls are intentional, add
an inline comment or Javadoc at the call site and in the GenerateDeadlineRequest
class documenting why these fields may be null and the expected behavior when
they are absent so future changes won’t accidentally break assumptions.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/presentation/ai/external/AiController.java`:
- Around line 50-52: The controller currently duplicates role validation by
checking userRole and throwing
BusinessException(AiErrorCode.FORBIDDEN_AI_ACCESS) while AiAppService also
enforces role checks; remove the controller-level check in AiController (the if
("MASTER".equals(userRole)) block) and delegate authorization entirely to
AiAppService methods so validation is centralized, or if this check is
intentionally an early debug guard, add a clear comment above the userRole check
explaining it's a debug-only fast-fail and keep it; ensure any callers now rely
on AiAppService for enforcement and update/cover with unit tests for
AiAppService authorization behavior.
- Line 56: The call UUID.fromString(userId) in AiController (used inside
request.toCommand(UUID.fromString(userId)) and the other occurrences at the same
controller) can throw IllegalArgumentException for malformed X-User-Id; add
defensive parsing by extracting a helper like parseUserId(String userId) that
tries UUID.fromString(...) and on failure throws a controlled BusinessException
(e.g., CommonErrorCode.INVALID_USER_ID) or returns a validated UUID, then
replace direct UUID.fromString(...) uses in aiAppService.generateAiLog(...) and
the other two locations with parseUserId(userId); alternatively ensure a global
exception handler maps IllegalArgumentException to the same BusinessException.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/presentation/slack/external/SlackController.java`:
- Line 83: The SlackController currently accepts a Spring Pageable parameter
(see the method signature containing "Pageable pageable") while other
controllers like AiController use the project's BasePageRequest; make them
consistent by replacing the Pageable parameter with BasePageRequest in
SlackController's endpoint signature and adapt the method to convert
BasePageRequest into a Pageable (e.g., call an existing
toPageable()/toPageableRequest() helper or construct a PageRequest from
page/size/sort fields) before passing it to service/repository calls; update any
usages inside SlackController methods that reference "pageable" to use the
converted Pageable variable and ensure import of BasePageRequest.
In
`@notification-service/src/test/java/com/shipflow/notificationservice/AiAppServiceTest.java`:
- Around line 154-174: Add a new negative test in AiAppServiceTest (next to
get_success()) that verifies permission checks by calling aiAppService.getAiLog
with the same id/userId but a non-MASTER userRole (e.g., "USER") after stubbing
aiLogRepository.findByIdAndDeletedAtIsNull(id) to return the AiLog; assert that
the call fails with the expected access-denied behavior (throwing the service's
authorization exception, e.g., AccessDeniedException or the domain-specific
exception) rather than returning a success result so permission regression is
caught.
In
`@notification-service/src/test/java/com/shipflow/notificationservice/NotificationOrchestratorServiceTest.java`:
- Around line 71-82: The test's verifications are too loose—replace the generic
verify(...).sendSlackMessage(any()) and generateAiLog(any()) checks with
ArgumentCaptor usage to capture the actual objects passed from
notificationOrchestratorService.handleShipmentCreated(event) (capture the Slack
message DTO sent via slackAppService.sendSlackMessage and the AI request/log
object passed to aiAppService.generateAiLog), then assert their core fields
(recipient/targets, relatedShipmentId, relatedAiLogId, messageType, and the
generated message body/content) match expected values derived from the test
fixtures; keep existing mocks for orderInternalClient.getOrderReadModel and
aiLogRepository but add assertions on the captured arguments to prevent
regressions in mapping/field population.
- Around line 106-139: The test currently only asserts no exception for
handleShipmentCreated but doesn't verify Slack delivery behavior; update the
test method handleShipmentCreated_nullSlackId to explicitly verify
SlackAppService interactions: after stubbing
orderInternalClient.getOrderReadModel(...) and invoking
notificationOrchestratorService.handleShipmentCreated(event), add a Mockito
verify on the SlackAppService (e.g., verify(slackAppService,...)) to assert
either that the send method was never called when missing receivers OR that it
was called with receiverSlackId equal to "UNKNOWN" (choose the one matching
current policy); reference notificationOrchestratorService.handleShipmentCreated
and the SlackAppService send method when adding the verify to make the
expectation explicit.
In
`@notification-service/src/test/java/com/shipflow/notificationservice/SlackAppServiceTest.java`:
- Around line 59-66: Add negative permission test cases in SlackAppServiceTest
to prevent regression: create at least one SendSlackMessageCommand instance
using a non-MASTER role (e.g., "USER" and an explicitly unauthorized role),
invoke the same service method used in existing tests, assert that a
BusinessException is thrown, and verify slackMessageRepository and slackSender
are never called (use your mocks'
verifyZeroInteractions/verifyNoMoreInteractions or equivalent). Apply the same
pattern to the other test blocks referenced (around lines 154-171, 204-228,
257-262, 361-383) so each positive MASTER-path test has corresponding
unauthorized-role test coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ca6aca3b-735d-4a50-a639-0c8abe9ff5f5
📒 Files selected for processing (44)
common/src/main/java/com/shipflow/common/domain/BaseEntity.javadocker-compose.ymlkeycloak/shipflow-export.jsonnotification-service/build.gradlenotification-service/src/main/java/com/shipflow/notificationservice/NotificationserviceApplication.javanotification-service/src/main/java/com/shipflow/notificationservice/application/NotificationOrchestratorService.javanotification-service/src/main/java/com/shipflow/notificationservice/application/ai/AiAppService.javanotification-service/src/main/java/com/shipflow/notificationservice/application/ai/dto/command/GenerateDeadlineCommand.javanotification-service/src/main/java/com/shipflow/notificationservice/application/ai/dto/command/SearchAiLogCommand.javanotification-service/src/main/java/com/shipflow/notificationservice/application/slack/SlackAppService.javanotification-service/src/main/java/com/shipflow/notificationservice/application/slack/dto/command/SearchSlackMessageCommand.javanotification-service/src/main/java/com/shipflow/notificationservice/application/slack/dto/command/SendSlackMessageCommand.javanotification-service/src/main/java/com/shipflow/notificationservice/application/slack/dto/command/UpdateSlackMessageCommand.javanotification-service/src/main/java/com/shipflow/notificationservice/config/JPAConfig.javanotification-service/src/main/java/com/shipflow/notificationservice/config/SwaggerConfig.javanotification-service/src/main/java/com/shipflow/notificationservice/domain/ai/AiLog.javanotification-service/src/main/java/com/shipflow/notificationservice/domain/ai/exception/AiErrorCode.javanotification-service/src/main/java/com/shipflow/notificationservice/domain/ai/repository/AiLogRepository.javanotification-service/src/main/java/com/shipflow/notificationservice/domain/slack/SlackMessage.javanotification-service/src/main/java/com/shipflow/notificationservice/domain/slack/exception/SlackErrorCode.javanotification-service/src/main/java/com/shipflow/notificationservice/domain/slack/repository/SlackMessageRepository.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/client/ai/GeminiApiClient.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/client/ai/config/GeminiApiConfig.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/client/order/OrderInternalClient.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/client/order/OrderReadModelResponse.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/messaging/consumer/ShipmentCreatedConsumer.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/messaging/consumer/ShipmentCreatedHandler.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/messaging/consumer/ShipmentCreatedListener.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/messaging/dto/ShipmentCreatedEvent.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/persistence/ai/AiLogJpaRepository.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/persistence/ai/AiLogRepositoryImpl.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/persistence/slack/SlackMessageJpaRepository.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/persistence/slack/SlackMessageRepositoryImpl.javanotification-service/src/main/java/com/shipflow/notificationservice/presentation/ai/dto/request/GenerateDeadlineRequest.javanotification-service/src/main/java/com/shipflow/notificationservice/presentation/ai/external/AiController.javanotification-service/src/main/java/com/shipflow/notificationservice/presentation/common/BasePageRequest.javanotification-service/src/main/java/com/shipflow/notificationservice/presentation/slack/dto/request/SendSlackMessageRequest.javanotification-service/src/main/java/com/shipflow/notificationservice/presentation/slack/dto/request/UpdateSlackMessageRequest.javanotification-service/src/main/java/com/shipflow/notificationservice/presentation/slack/dto/response/SlackMessageResponse.javanotification-service/src/main/java/com/shipflow/notificationservice/presentation/slack/external/SlackController.javanotification-service/src/main/resources/application.yamlnotification-service/src/test/java/com/shipflow/notificationservice/AiAppServiceTest.javanotification-service/src/test/java/com/shipflow/notificationservice/NotificationOrchestratorServiceTest.javanotification-service/src/test/java/com/shipflow/notificationservice/SlackAppServiceTest.java
💤 Files with no reviewable changes (3)
- notification-service/src/main/java/com/shipflow/notificationservice/presentation/slack/dto/response/SlackMessageResponse.java
- notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/messaging/consumer/ShipmentCreatedConsumer.java
- notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/persistence/ai/AiLogJpaRepository.java
…ID 정규식 확장, 페이지 정렬 추가
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
notification-service/src/main/java/com/shipflow/notificationservice/application/slack/SlackAppService.java (1)
62-68: 미사용userId파라미터 확인 필요
userId파라미터가 선언되었지만 메서드 본문에서 사용되지 않습니다. API 일관성을 위해 의도적으로 추가한 것이라면 주석을 남겨주시고, 그렇지 않다면 제거를 고려해주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/application/slack/SlackAppService.java` around lines 62 - 68, The getSlackMessage method in SlackAppService declares a UUID userId parameter that is never used; either remove the unused userId parameter from the method signature (and update all callers) or, if it was intentionally added for future authorization/auditing, document its purpose with a clarifying comment and/or use it in validateMasterRole or additional checks (e.g., validate ownership or audit logging) before fetching via slackMessageRepository.findByIdAndDeletedAtIsNull; update SlackAppService.getSlackMessage and any references accordingly to keep the API consistent.notification-service/src/main/java/com/shipflow/notificationservice/config/RedisConfig.java (1)
12-19: Spring Boot의StringRedisTemplate자동 구성과 빈 충돌 가능성이 있습니다.Spring Boot는
StringRedisTemplate(extendsRedisTemplate<String, String>)을 자동 구성합니다. 현재 커스텀 빈과 타입이 동일하여ShipmentCreatedHandler에서@Qualifier없이 주입 시NoUniqueBeanDefinitionException이 발생할 수 있습니다.권장 해결 방안:
- 이 커스텀 빈에
@Primary추가- 또는 Spring Boot의
StringRedisTemplate을 직접 사용 (중복 정의 제거)♻️ 옵션 1: `@Primary` 추가
+import org.springframework.context.annotation.Primary; + `@Configuration` public class RedisConfig { `@Bean` + `@Primary` public RedisTemplate<String, String> redisTemplate(RedisConnectionFactory connectionFactory) {♻️ 옵션 2: StringRedisTemplate 직접 사용하도록 Handler 수정
ShipmentCreatedHandler.java에서:-private final RedisTemplate<String, String> redisTemplate; +private final StringRedisTemplate redisTemplate;이 경우
RedisConfig클래스는 삭제 가능합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notification-service/src/main/java/com/shipflow/notificationservice/config/RedisConfig.java` around lines 12 - 19, The custom RedisTemplate bean in RedisConfig (method redisTemplate) conflicts with Spring Boot's auto-configured StringRedisTemplate, causing NoUniqueBeanDefinitionException when injecting into ShipmentCreatedHandler; fix by either marking your custom bean as the primary RedisTemplate with `@Primary` on the redisTemplate bean method (so it wins injection) or remove this custom bean and change ShipmentCreatedHandler to inject StringRedisTemplate directly (or add `@Qualifier` there) — update RedisConfig.redisTemplate or ShipmentCreatedHandler accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@notification-service/src/main/java/com/shipflow/notificationservice/application/slack/SlackAppService.java`:
- Around line 124-128: In validateCreateRole, the allowed-role set in Set.of
mistakenly contains "SHIPMENT_MANAGER " with a trailing space which prevents
matching the UserRole value; remove the trailing space so the set contains
"SHIPMENT_MANAGER" (i.e., update the Set.of call in validateCreateRole) so users
with that role no longer trigger the
BusinessException(SlackErrorCode.FORBIDDEN_SLACK_ACCESS).
In
`@notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/messaging/consumer/ShipmentCreatedHandler.java`:
- Around line 49-54: The current catch block in ShipmentCreatedHandler loses the
original exception if redisTemplate.delete(key) throws; wrap the delete call in
its own try/catch and, if it fails, attach that deletion exception to the
original exception (e.g., addSuppressed or initCause) before rethrowing the
original exception from the catch that surrounds
notificationOrchestratorService.handleShipmentCreated(event), so the original
business exception is preserved while still surfacing any Redis deletion error.
---
Nitpick comments:
In
`@notification-service/src/main/java/com/shipflow/notificationservice/application/slack/SlackAppService.java`:
- Around line 62-68: The getSlackMessage method in SlackAppService declares a
UUID userId parameter that is never used; either remove the unused userId
parameter from the method signature (and update all callers) or, if it was
intentionally added for future authorization/auditing, document its purpose with
a clarifying comment and/or use it in validateMasterRole or additional checks
(e.g., validate ownership or audit logging) before fetching via
slackMessageRepository.findByIdAndDeletedAtIsNull; update
SlackAppService.getSlackMessage and any references accordingly to keep the API
consistent.
In
`@notification-service/src/main/java/com/shipflow/notificationservice/config/RedisConfig.java`:
- Around line 12-19: The custom RedisTemplate bean in RedisConfig (method
redisTemplate) conflicts with Spring Boot's auto-configured StringRedisTemplate,
causing NoUniqueBeanDefinitionException when injecting into
ShipmentCreatedHandler; fix by either marking your custom bean as the primary
RedisTemplate with `@Primary` on the redisTemplate bean method (so it wins
injection) or remove this custom bean and change ShipmentCreatedHandler to
inject StringRedisTemplate directly (or add `@Qualifier` there) — update
RedisConfig.redisTemplate or ShipmentCreatedHandler accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0303653e-b536-439e-800f-7470a1ab9781
📒 Files selected for processing (10)
notification-service/build.gradlenotification-service/src/main/java/com/shipflow/notificationservice/application/slack/SlackAppService.javanotification-service/src/main/java/com/shipflow/notificationservice/config/JPAConfig.javanotification-service/src/main/java/com/shipflow/notificationservice/config/RedisConfig.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/client/ai/config/GeminiApiConfig.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/messaging/consumer/ShipmentCreatedHandler.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/messaging/dto/ShipmentCreatedEvent.javanotification-service/src/main/java/com/shipflow/notificationservice/infrastructure/persistence/slack/SlackMessageRepositoryImpl.javanotification-service/src/main/java/com/shipflow/notificationservice/presentation/slack/dto/request/SendSlackMessageRequest.javanotification-service/src/main/resources/application.yaml
✅ Files skipped from review due to trivial changes (1)
- notification-service/build.gradle
🚧 Files skipped from review as they are similar to previous changes (6)
- notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/client/ai/config/GeminiApiConfig.java
- notification-service/src/main/java/com/shipflow/notificationservice/presentation/slack/dto/request/SendSlackMessageRequest.java
- notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/persistence/slack/SlackMessageRepositoryImpl.java
- notification-service/src/main/java/com/shipflow/notificationservice/config/JPAConfig.java
- notification-service/src/main/java/com/shipflow/notificationservice/infrastructure/messaging/dto/ShipmentCreatedEvent.java
- notification-service/src/main/resources/application.yaml
…rvice/application/slack/SlackAppService.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 PR 제목
[FEATURE] [Notification] AI-Slack 이벤트 연동 및 Slack API 개선 #30
✨ 작업 내용
Notification 서비스의 AI/Slack 핵심 로직 구현 및 Gateway 연동, 트랜잭션 경계 개선
🔍 상세 내용
ShipmentCreatedEvent) 수신 후 AI 발송 시한 계산 및 Slack 알림 발송 플로우 구현NotificationOrchestratorService클래스 레벨@Transactional제거 → 외부 API 호출(Gemini, Slack) 중 DB 커넥션 점유 방지GeminiApiConfigWebClient 빈 이름geminiWebClient로 명시 → 빈 충돌 방지JPAConfigAuditorAware를X-User-Id헤더 기반으로 수정 →createdBy,updatedBy자동 처리OrderInternalClientFeignClient nameorderservice로 수정 (Eureka 등록명 일치)🔗 관련 이슈
Closes #30
docker-compose.ymlnotificationservice에 RabbitMQ 환경변수(RABBITMQ_HOST,RABBITMQ_PORT,RABBITMQ_USERNAME,RABBITMQ_PASSWORD) 명시적 추가로 수정 했습니다. 확인 부탁드립니다.✅ 체크리스트
Summary by CodeRabbit
새로운 기능
개선 사항
테스트
잡무(Chore)