Merged
Conversation
Closed
Contributor
📝 WalkthroughWalkthrough빌드 설정 변경(Gradle 플러그인·의존성 조정), DTO 확장( Changes
Sequence Diagram(s)sequenceDiagram
participant Kafka as Kafka Topic
participant Consumer as PaymentConsumer
participant Service as PaymentService
participant Repo as PaymentRepository/DB
Kafka->>Consumer: withdraw result (includes orderId)
Consumer->>Service: parse & decide success/failure
alt success
Service->>Repo: completePayment (update ledger/status)
Repo-->>Service: ack
Service-->>Consumer: success handled
else failure
Consumer->>Repo: find Payment by orderId
Repo-->>Consumer: payment (usedPoints, status)
alt status == PENDING
Consumer->>Service: compensatePayment(usedPoints)
Service->>Repo: persist refund & set ledger FAILED
Repo-->>Service: ack
Service-->>Consumer: compensation done
else
Consumer->>Consumer: log warning (duplicate/invalid)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![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: 1
🤖 Fix all issues with AI agents
In `@cloud-services/msa-point-service/build.gradle`:
- Line 3: 현재 build.gradle의 Spring Boot plugin declaration id
'org.springframework.boot' version '3.5.10'가 Spring Cloud 2025.1.0과 호환되지 않습니다;
고치려면 팀 결정에 따라 하나를 선택해 적용하세요: (옵션 A) 업그레이드: 변경 대상은 build.gradle의 plugin
declaration (id 'org.springframework.boot' version ...)을 Spring Boot 4.0.x로 올리고,
이후 Spring Cloud 2025.1.0과의 호환성을 위해 모든 관련 의존성 및 BOM 설정(예: spring-cloud-bom
entries)과 프로젝트의 Java target/compatibility를 점검하세요; 또는 (옵션 B) 다운그레이드: Spring Cloud
BOM/version declarations를 2025.0.x로 낮춰 현재 build.gradle의 Spring Boot 3.5.10과
일치시키고, pom.xml이나 다른 모듈의 spring-cloud 버전 선언(혹은 dependencyManagement)도 동일하게 맞추어
테스트하세요.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cloud-services/msa-point-service/build.gradle`:
- Around line 33-35: Update the springCloudVersion declaration in the ext block
so it uses the newer patch release 2025.0.1 instead of 2025.0.0; locate the ext
{ ... } block containing the set('springCloudVersion', ...) entry and change its
value to "2025.0.1" to pick up the latest stable Spring Cloud 2025 patch.
🧹 Nitpick comments (1)
cloud-services/msa-point-service/build.gradle (1)
38-58: Jackson 의존성 명시 제거 추천Spring Boot 3.5.10에서
spring-boot-starter-web은spring-boot-starter-json을 통해jackson-databind와jackson-datatype-jsr310을 이미 포함하고 있습니다. 명시적인 버전 고정이 필요하지 않다면 이 두 의존성을 제거해도 문제없습니다. 빌드 설정을 더 간결하게 유지하기 위해 제거를 권장합니다.♻️ 제거 예시
- implementation 'com.fasterxml.jackson.core:jackson-databind' - implementation 'com.fasterxml.jackson.datatype:jackson-datatype-jsr310'
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@cloud-services/msa-point-service/src/main/java/com/techsemina/msa/pointservice/service/PaymentConsumer.java`:
- Around line 47-50: The consumer's duplicate-prevention check in
PaymentConsumer is inconsistent with compensatePayment(): change the guard in
the consumer (the block that currently checks
"FAILED".equals(payment.getStatus())) to the same logic used by
compensatePayment() — skip processing unless payment.getStatus() == PENDING
(i.e., if status is not PENDING, log "이미 처리된 환불 건입니다." and return) so messages
for COMPLETED or FAILED don't trigger unintended state changes; update the check
in PaymentConsumer to explicitly mirror compensatePayment()'s condition.
- Around line 42-61: The consumer duplicates state changes on the same Payment
entity causing conflicting entity instances and potential optimistic locking
issues: remove the redundant status change either by having
paymentService.compensatePayment(...) perform the full update (including setting
status to "FAILED" and saving) and stop setting status/save in the consumer, or
conversely remove status changes from compensatePayment(...) and let the
consumer alone call paymentRepository.findByOrderId(...), call
paymentService.compensatePayment(...) (which only handles refund logic), then
set payment.setStatus("FAILED") and save once; ensure you use the same entity
instance (avoid calling findByOrderId twice for the same orderId) and keep
transaction boundaries consistent (annotate the service or consumer with
`@Transactional` as appropriate).
In
`@cloud-services/msa-point-service/src/main/java/com/techsemina/msa/pointservice/service/PaymentService.java`:
- Line 58: The kafka topic name is hardcoded in PaymentService at the
kafkaTemplate.send("core-withdraw-request", cashMessage) call; change this to
use a configurable property or constant instead: add a configuration property
(e.g., kafka.topic.withdraw-request in application.yml) and inject it into
PaymentService (via `@Value` or a `@ConfigurationProperties-backed` bean) then
replace the literal in the kafkaTemplate.send(...) invocation with the injected
property (e.g., withdrawRequestTopic) so the topic can be managed per
environment and is no longer a string literal.
🧹 Nitpick comments (3)
cloud-services/msa-point-service/src/main/java/com/techsemina/msa/pointservice/service/PaymentService.java (2)
56-62: 비동기 Kafka 전송 시 에러 처리 콜백 추가를 권장합니다.
.get()제거로 트랜잭션 시간이 단축된 것은 좋지만, 현재 fire-and-forget 방식은 메시지 전송 실패를 감지할 수 없습니다. 전송이 실패해도 Payment는PENDING상태로 남아 orphan 레코드가 될 수 있어요.
CompletableFuture콜백을 추가하여 최소한 실패 로깅은 하는 것을 권장합니다:🔧 비동기 전송 + 에러 콜백 적용 예시
try { // ❌ .get()으로 기다리지 말기! 트랜잭션 길어짐. 비동기로 보내기 - kafkaTemplate.send("core-withdraw-request", cashMessage); + kafkaTemplate.send("core-withdraw-request", cashMessage) + .whenComplete((result, ex) -> { + if (ex != null) { + log.error("❌ Kafka 전송 실패 (orderId={}): {}", + request.getOrderId(), ex.getMessage()); + // 필요시 별도 재시도 로직이나 알림 처리 + } + }); } catch (Exception e) {
13-13: 사용되지 않는 import가 남아있습니다.
.get(5, TimeUnit.SECONDS)제거 후TimeUnit이 더 이상 사용되지 않습니다. IDE나 빌드 도구에서 자동 정리해 주세요.-import java.util.concurrent.TimeUnit;cloud-services/msa-point-service/src/main/java/com/techsemina/msa/pointservice/service/PaymentConsumer.java (1)
25-27: Kafka 토픽명 하드코딩 및 throws Exception 시그니처 개선 필요.
"core-withdraw-result"토픽명도 외부 설정으로 관리하면 좋습니다.throws Exception은 너무 포괄적입니다.JsonProcessingException등 실제 발생 가능한 예외만 명시하거나, try-catch로 처리하는 것이 좋습니다.🔧 개선 예시
- `@KafkaListener`(topics = "core-withdraw-result", groupId = "point-service-group") + `@KafkaListener`(topics = "${kafka.topic.withdraw-result}", groupId = "${spring.kafka.consumer.group-id}") `@Transactional` - public void consumeWithdrawResult(String message) throws Exception { + public void consumeWithdrawResult(String message) throws JsonProcessingException {
application.yml:kafka: topic: withdraw-result: core-withdraw-result
Comment on lines
+42
to
+61
| // (1) 장부(DB)에서 주문 조회 (orderId로 찾기!) | ||
| Payment payment = paymentRepository.findByOrderId(result.getOrderId()) | ||
| .orElseThrow(() -> new RuntimeException("주문 정보를 찾을 수 없습니다.")); | ||
|
|
||
| // (2) 이미 처리된 건인지 확인 (중복 방지) | ||
| if ("FAILED".equals(payment.getStatus())) { | ||
| log.info("이미 처리된 환불 건입니다."); | ||
| return; | ||
| } | ||
| // (3) 실제 사용했던 포인트 조회 | ||
| Long usedPoint = payment.getPointAmount(); | ||
|
|
||
| // (4) 포인트 환불 | ||
| paymentService.compensatePayment(payment.getOrderId()); | ||
|
|
||
| } catch (Exception e) { | ||
| log.error("❌ 메시지 처리 중 에러", e); | ||
| // (5) 장부 상태 업데이트 (FAILED) | ||
| payment.setStatus("FAILED"); | ||
| paymentRepository.save(payment); // @Transactional 있으면 자동 저장됨 (Dirty Checking) | ||
|
|
||
| log.info("✅ 포인트 {}점 환불 완료.", usedPoint); |
Contributor
There was a problem hiding this comment.
중복 상태 업데이트 및 트랜잭션 범위 문제가 있습니다.
compensatePayment() 메서드와 이 컨슈머에서 동일한 Payment 레코드에 대해 상태를 중복 설정하고 있습니다:
- Line 55:
compensatePayment()가 내부에서payment.setStatus("FAILED")수행 - Line 58-59: 컨슈머에서 다시
payment.setStatus("FAILED")후save()호출
두 메서드가 각각 findByOrderId()를 호출하므로 서로 다른 엔티티 인스턴스를 수정하게 되어, Dirty Checking이 제대로 동작하지 않거나 optimistic locking 예외가 발생할 수 있습니다.
🔧 수정 방안: 역할 분리 명확화
// (4) 포인트 환불
paymentService.compensatePayment(payment.getOrderId());
- // (5) 장부 상태 업데이트 (FAILED)
- payment.setStatus("FAILED");
- paymentRepository.save(payment); // `@Transactional` 있으면 자동 저장됨 (Dirty Checking)
+ // compensatePayment 내부에서 이미 FAILED 상태로 변경 & 저장됨
+ // 중복 업데이트 제거
log.info("✅ 포인트 {}점 환불 완료.", usedPoint);또는 compensatePayment()에서 상태 변경을 제거하고 컨슈머에서만 처리하는 방식도 가능합니다.
🤖 Prompt for AI Agents
In
`@cloud-services/msa-point-service/src/main/java/com/techsemina/msa/pointservice/service/PaymentConsumer.java`
around lines 42 - 61, The consumer duplicates state changes on the same Payment
entity causing conflicting entity instances and potential optimistic locking
issues: remove the redundant status change either by having
paymentService.compensatePayment(...) perform the full update (including setting
status to "FAILED" and saving) and stop setting status/save in the consumer, or
conversely remove status changes from compensatePayment(...) and let the
consumer alone call paymentRepository.findByOrderId(...), call
paymentService.compensatePayment(...) (which only handles refund logic), then
set payment.setStatus("FAILED") and save once; ensure you use the same entity
instance (avoid calling findByOrderId twice for the same orderId) and keep
transaction boundaries consistent (annotate the service or consumer with
`@Transactional` as appropriate).
Comment on lines
+47
to
50
| if ("FAILED".equals(payment.getStatus())) { | ||
| log.info("이미 처리된 환불 건입니다."); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
중복 방지 조건이 compensatePayment()와 일치하지 않습니다.
- 컨슈머:
FAILED상태만 스킵 compensatePayment():PENDING이 아니면 모두 스킵 (COMPLETED, FAILED 포함)
만약 이미 COMPLETED된 주문에 대해 실패 메시지가 늦게 도착하면:
- 컨슈머는 COMPLETED를 FAILED가 아니므로 통과
compensatePayment()는 PENDING 아니므로 스킵- 하지만 아래 코드에서 상태를 FAILED로 변경 (의도치 않은 상태 변경)
가드 조건을 통일하세요:
🔧 가드 조건 통일
// (2) 이미 처리된 건인지 확인 (중복 방지)
- if ("FAILED".equals(payment.getStatus())) {
- log.info("이미 처리된 환불 건입니다.");
+ if (!"PENDING".equals(payment.getStatus())) {
+ log.info("이미 처리된 건입니다. (상태: {})", payment.getStatus());
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ("FAILED".equals(payment.getStatus())) { | |
| log.info("이미 처리된 환불 건입니다."); | |
| return; | |
| } | |
| if (!"PENDING".equals(payment.getStatus())) { | |
| log.info("이미 처리된 건입니다. (상태: {})", payment.getStatus()); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In
`@cloud-services/msa-point-service/src/main/java/com/techsemina/msa/pointservice/service/PaymentConsumer.java`
around lines 47 - 50, The consumer's duplicate-prevention check in
PaymentConsumer is inconsistent with compensatePayment(): change the guard in
the consumer (the block that currently checks
"FAILED".equals(payment.getStatus())) to the same logic used by
compensatePayment() — skip processing unless payment.getStatus() == PENDING
(i.e., if status is not PENDING, log "이미 처리된 환불 건입니다." and return) so messages
for COMPLETED or FAILED don't trigger unintended state changes; update the check
in PaymentConsumer to explicitly mirror compensatePayment()'s condition.
| try { | ||
| kafkaTemplate.send("core-withdraw-request", cashMessage).get(5, TimeUnit.SECONDS); | ||
| // ❌ .get()으로 기다리지 말기! 트랜잭션 길어짐. 비동기로 보내기 | ||
| kafkaTemplate.send("core-withdraw-request", cashMessage); |
Contributor
There was a problem hiding this comment.
Kafka 토픽명이 여전히 하드코딩되어 있습니다.
PR 목적 중 하나가 Kafka 하드코딩 수정인데, "core-withdraw-request" 토픽명이 아직 문자열 리터럴로 남아있습니다. 설정 파일이나 상수로 관리하면 환경별 배포와 유지보수가 편해집니다.
🔧 토픽명 외부화 예시
application.yml:
kafka:
topic:
withdraw-request: core-withdraw-request서비스 클래스:
+ `@Value`("${kafka.topic.withdraw-request}")
+ private String withdrawRequestTopic;
// ...
- kafkaTemplate.send("core-withdraw-request", cashMessage);
+ kafkaTemplate.send(withdrawRequestTopic, cashMessage);🤖 Prompt for AI Agents
In
`@cloud-services/msa-point-service/src/main/java/com/techsemina/msa/pointservice/service/PaymentService.java`
at line 58, The kafka topic name is hardcoded in PaymentService at the
kafkaTemplate.send("core-withdraw-request", cashMessage) call; change this to
use a configurable property or constant instead: add a configuration property
(e.g., kafka.topic.withdraw-request in application.yml) and inject it into
PaymentService (via `@Value` or a `@ConfigurationProperties-backed` bean) then
replace the literal in the kafkaTemplate.send(...) invocation with the injected
property (e.g., withdrawRequestTopic) so the topic can be managed per
environment and is no longer a string literal.
This was referenced Feb 4, 2026
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
1 participant
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.
Bug: SpringBoot version 4 -> 3.5.10
Summary by CodeRabbit
New Features
Bug Fixes
Chores