[fix/#133-register-property] 매물 등록 로직 수정#134
Open
HUIJAEKO wants to merge 2 commits into
Open
Conversation
📝 WalkthroughWalkthroughRabbitMQ 기반 이벤트 드리븐 처리 추가: AMQP 의존성 추가, 로컬 docker-compose에 RabbitMQ 서비스 추가, 교환/큐/바인딩 및 메시지 컨버터 설정, 이벤트 DTO/발행자/중계자/소비자 및 비동기 오케스트레이터 도입으로 매물 등록 후 비동기 점수측정 흐름을 구현함. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant PropertyService as PropertyService
participant EventPublisher as ApplicationEventPublisher
participant PropertyEventRelay as PropertyEventRelay
participant RabbitMQ as RabbitMQ
participant PropertyCreatedConsumer as Consumer
participant PropertyAsyncOrchestrator as Orchestrator
participant ExplanationServices as ExplanationServices
Client->>PropertyService: register(request)
PropertyService->>PropertyService: DB에 매물 저장
PropertyService->>EventPublisher: publishEvent(PropertyCreatedEvent)
PropertyService-->>Client: return propertyId
EventPublisher->>PropertyEventRelay: on(PropertyCreatedEvent)
PropertyEventRelay->>RabbitMQ: convertAndSend(exchange, routingKey, event)
RabbitMQ->>PropertyCreatedConsumer: 메시지 전달
PropertyCreatedConsumer->>PropertyAsyncOrchestrator: process(propertyId)
Orchestrator->>ExplanationServices: evaluateAndDescribe(propertyId) (5개 병렬 호출)
ExplanationServices-->>Orchestrator: 점수/설명 반환
Orchestrator->>Orchestrator: 모든 작업 완료 대기 및 처리
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/main/java/com/example/Dingle/global/config/RabbitConfig.java (1)
27-30: Dead Letter Queue(DLQ) 설정을 고려해 보세요.현재 큐 설정은 기본적이며 동작합니다. 프로덕션 환경에서는 메시지 처리 실패 시 재시도 및 DLQ를 통한 실패 메시지 추적을 위해 추가 설정을 고려해 보세요.
💡 DLQ 설정 예시
`@Bean` public Queue propertyCreatedQueue() { return QueueBuilder.durable(QUEUE) .withArgument("x-dead-letter-exchange", EXCHANGE) .withArgument("x-dead-letter-routing-key", "property.created.dlq") .build(); } `@Bean` public Queue deadLetterQueue() { return QueueBuilder.durable("property.created.dlq").build(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/example/Dingle/global/config/RabbitConfig.java` around lines 27 - 30, Add a Dead Letter Queue (DLQ) configuration to the existing propertyCreatedQueue bean so failed messages are routed for inspection: update propertyCreatedQueue() (which currently returns QueueBuilder.durable(QUEUE).build()) to add arguments "x-dead-letter-exchange" and "x-dead-letter-routing-key" pointing to your DLX/route (use EXCHANGE and a DLQ routing key like "property.created.dlq"), and add a new bean deadLetterQueue() that builds the durable DLQ (e.g., "property.created.dlq"); ensure any bindings for the DLQ route are created so messages routed to the DLQ are consumable.src/main/java/com/example/Dingle/property/controller/PropertyController.java (1)
5-5: 사용되지 않는 import 문이 있습니다.
PropertyCreatedEvent,ApplicationEventPublisher,LocalDateimport가 컨트롤러에서 사용되지 않습니다. AI 요약에 따르면 이벤트 발행은PropertyService에서 처리됩니다. 이 import들은 리팩토링 과정에서 남은 것으로 보입니다.♻️ 사용되지 않는 import 제거
import com.example.Dingle.global.dto.ResponseDTO; import com.example.Dingle.property.dto.PropertyCompareDTO; -import com.example.Dingle.property.dto.PropertyCreatedEvent; import com.example.Dingle.property.dto.PropertyListDTO; import com.example.Dingle.property.dto.PropertyRegisterRequestDTO; import com.example.Dingle.property.service.PropertyListService; import com.example.Dingle.property.service.PropertyService; import com.example.Dingle.user.dto.CustomUserDetails; import io.swagger.v3.oas.annotations.Operation; import lombok.RequiredArgsConstructor; -import org.springframework.context.ApplicationEventPublisher; import org.springframework.http.ResponseEntity; import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.web.bind.annotation.*; -import java.time.LocalDate; import java.util.List;Also applies to: 13-13, 18-18
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/example/Dingle/property/controller/PropertyController.java` at line 5, Remove the leftover unused imports and any related unused injections from the controller: delete the import lines for PropertyCreatedEvent, ApplicationEventPublisher, and LocalDate, and if the PropertyController class contains an ApplicationEventPublisher field or constructor parameter, remove that field/parameter and its assignment; keep PropertyService usage intact since event publishing moved to PropertyService. Ensure no other references to those symbols remain in PropertyController after removal.src/main/java/com/example/Dingle/property/service/PropertyAsyncOrchestrator.java (3)
55-56: 예외 처리 시 원인 체인이 손실될 수 있습니다.
Exception을 광범위하게 catch하면InterruptedException의 인터럽트 상태가 복원되지 않습니다. 또한 첫 번째 실패 이후 나머지 태스크의 예외 정보가 손실됩니다.♻️ 인터럽트 처리 개선
- } catch (Exception e) { - throw new RuntimeException("Property async processing failed", e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException("Property async processing interrupted", e); + } catch (ExecutionException e) { + throw new RuntimeException("Property async processing failed", e.getCause()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/example/Dingle/property/service/PropertyAsyncOrchestrator.java` around lines 55 - 56, In PropertyAsyncOrchestrator, avoid catching Exception broadly: change the try/catch so InterruptedException is caught separately, call Thread.currentThread().interrupt() and rethrow (or wrap) to preserve interrupt status; for other failures collect task exceptions instead of discarding them—when aggregating results from your async tasks (e.g., the CompletableFuture/ExecutorFlow used in this method), capture the primary exception and call addSuppressed(...) for remaining task exceptions or wrap them in a CompletionException so none are lost before throwing the RuntimeException("Property async processing failed", e); ensure the catch block references InterruptedException and the general Exception handler uses suppressed exceptions from the individual futures.
20-21: ExecutorService를 메서드 호출마다 생성합니다.
process()호출마다 새로운ExecutorService를 생성하고 종료합니다. Virtual thread executor는 가볍지만, 이 패턴은 여전히 불필요한 오버헤드를 발생시킵니다.♻️ 싱글톤 ExecutorService 사용 고려
`@Service` `@RequiredArgsConstructor` public class PropertyAsyncOrchestrator { + private final ExecutorService executor = Executors.newVirtualThreadPerTaskExecutor(); + private final EnvironmentExplanationService environmentExplanationService; // ... other services public void process(Long propertyId) { - ExecutorService executor = Executors.newVirtualThreadPerTaskExecutor(); - try { // ... submit tasks - } finally { - executor.close(); - } + } catch (Exception e) { + throw new RuntimeException("Property async processing failed", e); + } } }또는 Spring의
@Async와TaskExecutor빈을 활용하는 것도 고려해 보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/example/Dingle/property/service/PropertyAsyncOrchestrator.java` around lines 20 - 21, The process() method in PropertyAsyncOrchestrator creates a new ExecutorService via Executors.newVirtualThreadPerTaskExecutor() on every call; change this to use a shared/singleton executor instead of per-call creation: add a private final ExecutorService field in PropertyAsyncOrchestrator initialized once (or accept/inject a TaskExecutor/@Async-managed executor) and reuse it inside process(Long propertyId); ensure proper lifecycle management by shutting down the executor on application stop (or rely on Spring bean lifecycle) and remove per-call executor creation and shutdown from process().
24-53:CompletableFuture를 사용하면 더 간결하게 작성할 수 있습니다.현재 구현은 동작하지만,
CompletableFuture.allOf()를 사용하면 코드가 더 간결해지고 예외 처리도 개선됩니다.♻️ CompletableFuture 활용 예시
public void process(Long propertyId) { CompletableFuture<?>[] futures = { CompletableFuture.runAsync(() -> environmentExplanationService.evaluateAndDescribe(propertyId)), CompletableFuture.runAsync(() -> convenienceExplanationService.evaluateAndDescribe(propertyId)), CompletableFuture.runAsync(() -> accessibilityExplanationService.evaluateAndDescribe(propertyId)), CompletableFuture.runAsync(() -> noiseExplanationService.evaluateAndDescribe(propertyId)), CompletableFuture.runAsync(() -> safetyExplanationService.evaluateAndDescribe(propertyId)) }; try { CompletableFuture.allOf(futures).join(); } catch (CompletionException e) { throw new RuntimeException("Property async processing failed", e.getCause()); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/example/Dingle/property/service/PropertyAsyncOrchestrator.java` around lines 24 - 53, The current PropertyAsyncOrchestrator submits five callables to an Executor and blocks on each Future.get(); replace this with CompletableFuture to simplify and improve exception handling: create CompletableFuture.runAsync tasks for environmentExplanationService.evaluateAndDescribe(propertyId), convenienceExplanationService.evaluateAndDescribe(propertyId), accessibilityExplanationService.evaluateAndDescribe(propertyId), noiseExplanationService.evaluateAndDescribe(propertyId), and safetyExplanationService.evaluateAndDescribe(propertyId) (optionally passing the existing executor), collect them into an array and use CompletableFuture.allOf(...).join(); wrap join in try/catch to translate CompletionException into a runtime exception with the cause so failures are propagated cleanly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-compose.local.yml`:
- Around line 37-38: The docker-compose references a named volume rabbitmq-data
in the rabbitmq service but does not declare it at the top-level; add a
top-level volumes: section that declares rabbitmq-data (e.g., include a volumes
block with rabbitmq-data:) so the named volume is explicitly defined; update the
docker-compose file to include this volumes entry to match the referenced
rabbitmq-data volume.
---
Nitpick comments:
In `@src/main/java/com/example/Dingle/global/config/RabbitConfig.java`:
- Around line 27-30: Add a Dead Letter Queue (DLQ) configuration to the existing
propertyCreatedQueue bean so failed messages are routed for inspection: update
propertyCreatedQueue() (which currently returns
QueueBuilder.durable(QUEUE).build()) to add arguments "x-dead-letter-exchange"
and "x-dead-letter-routing-key" pointing to your DLX/route (use EXCHANGE and a
DLQ routing key like "property.created.dlq"), and add a new bean
deadLetterQueue() that builds the durable DLQ (e.g., "property.created.dlq");
ensure any bindings for the DLQ route are created so messages routed to the DLQ
are consumable.
In
`@src/main/java/com/example/Dingle/property/controller/PropertyController.java`:
- Line 5: Remove the leftover unused imports and any related unused injections
from the controller: delete the import lines for PropertyCreatedEvent,
ApplicationEventPublisher, and LocalDate, and if the PropertyController class
contains an ApplicationEventPublisher field or constructor parameter, remove
that field/parameter and its assignment; keep PropertyService usage intact since
event publishing moved to PropertyService. Ensure no other references to those
symbols remain in PropertyController after removal.
In
`@src/main/java/com/example/Dingle/property/service/PropertyAsyncOrchestrator.java`:
- Around line 55-56: In PropertyAsyncOrchestrator, avoid catching Exception
broadly: change the try/catch so InterruptedException is caught separately, call
Thread.currentThread().interrupt() and rethrow (or wrap) to preserve interrupt
status; for other failures collect task exceptions instead of discarding
them—when aggregating results from your async tasks (e.g., the
CompletableFuture/ExecutorFlow used in this method), capture the primary
exception and call addSuppressed(...) for remaining task exceptions or wrap them
in a CompletionException so none are lost before throwing the
RuntimeException("Property async processing failed", e); ensure the catch block
references InterruptedException and the general Exception handler uses
suppressed exceptions from the individual futures.
- Around line 20-21: The process() method in PropertyAsyncOrchestrator creates a
new ExecutorService via Executors.newVirtualThreadPerTaskExecutor() on every
call; change this to use a shared/singleton executor instead of per-call
creation: add a private final ExecutorService field in PropertyAsyncOrchestrator
initialized once (or accept/inject a TaskExecutor/@Async-managed executor) and
reuse it inside process(Long propertyId); ensure proper lifecycle management by
shutting down the executor on application stop (or rely on Spring bean
lifecycle) and remove per-call executor creation and shutdown from process().
- Around line 24-53: The current PropertyAsyncOrchestrator submits five
callables to an Executor and blocks on each Future.get(); replace this with
CompletableFuture to simplify and improve exception handling: create
CompletableFuture.runAsync tasks for
environmentExplanationService.evaluateAndDescribe(propertyId),
convenienceExplanationService.evaluateAndDescribe(propertyId),
accessibilityExplanationService.evaluateAndDescribe(propertyId),
noiseExplanationService.evaluateAndDescribe(propertyId), and
safetyExplanationService.evaluateAndDescribe(propertyId) (optionally passing the
existing executor), collect them into an array and use
CompletableFuture.allOf(...).join(); wrap join in try/catch to translate
CompletionException into a runtime exception with the cause so failures are
propagated cleanly.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
build.gradledocker-compose.local.ymlsrc/main/java/com/example/Dingle/global/config/RabbitConfig.javasrc/main/java/com/example/Dingle/property/controller/PropertyController.javasrc/main/java/com/example/Dingle/property/dto/PropertyCreatedEvent.javasrc/main/java/com/example/Dingle/property/service/PropertyAsyncOrchestrator.javasrc/main/java/com/example/Dingle/property/service/PropertyService.javasrc/main/java/com/example/Dingle/property/util/PropertyCreatedConsumer.javasrc/main/java/com/example/Dingle/property/util/PropertyEventRelay.java
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docker-compose.local.yml (1)
29-29:rabbitmq:3-management대신 고정 태그 또는 다이제스트를 사용하여 재현성 확보
3-management는 가변 태그로서, 로컬 환경에서 패치 업데이트에 따라 다른 이미지가 풀될 수 있습니다. 재현 가능한 배포를 위해 다음 중 하나를 권장합니다:
- 패치 레벨 태그:
rabbitmq:3.13.1-management- 태그 + 다이제스트 (최상):
rabbitmq:3.13.1-management@sha256:abc...다이제스트 방식이 정확한 이미지 콘텐츠를 보장하므로 가장 안정적입니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.local.yml` at line 29, Replace the mutable image tag "rabbitmq:3-management" with a fixed patch-level tag or an image digest to ensure reproducible pulls; update the "image: rabbitmq:3-management" entry to something like "rabbitmq:3.13.1-management" or, preferably, "rabbitmq:3.13.1-management@sha256:<actual-digest>" so the exact image content is pinned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-compose.local.yml`:
- Around line 31-36: Change the port bindings from "5672:5672" and "15672:15672"
to bind only to localhost by using "127.0.0.1:5672:5672" and
"127.0.0.1:15672:15672", and stop hardcoding credentials by replacing
RABBITMQ_DEFAULT_USER: guest and RABBITMQ_DEFAULT_PASS: guest with environment
variable references like RABBITMQ_DEFAULT_USER: ${RABBITMQ_DEFAULT_USER} and
RABBITMQ_DEFAULT_PASS: ${RABBITMQ_DEFAULT_PASS}; then add the actual values to
your .env (e.g. RABBITMQ_DEFAULT_USER=guest, RABBITMQ_DEFAULT_PASS=guest) and
ensure the .env is excluded from version control.
---
Nitpick comments:
In `@docker-compose.local.yml`:
- Line 29: Replace the mutable image tag "rabbitmq:3-management" with a fixed
patch-level tag or an image digest to ensure reproducible pulls; update the
"image: rabbitmq:3-management" entry to something like
"rabbitmq:3.13.1-management" or, preferably,
"rabbitmq:3.13.1-management@sha256:<actual-digest>" so the exact image content
is pinned.
Comment on lines
+31
to
+36
| ports: | ||
| - "5672:5672" # AMQP | ||
| - "15672:15672" # Management UI | ||
| environment: | ||
| RABBITMQ_DEFAULT_USER: guest | ||
| RABBITMQ_DEFAULT_PASS: guest |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1. 파일 존재 확인
if [ -f "docker-compose.local.yml" ]; then
echo "[✓] docker-compose.local.yml 파일 발견"
# 2. 파일 크기 확인
wc -l docker-compose.local.yml
# 3. RabbitMQ 섹션 확인 (28-40 라인 범위)
echo -e "\n[RabbitMQ 섹션 확인]"
sed -n '28,40p' docker-compose.local.yml
# 4. 포트 바인딩 패턴 확인
echo -e "\n[포트 바인딩 패턴]"
grep -n "5672\|15672" docker-compose.local.yml || echo "RabbitMQ 포트 미발견"
# 5. RABBITMQ 환경변수 확인
echo -e "\n[RABBITMQ 환경변수]"
grep -n "RABBITMQ_DEFAULT" docker-compose.local.yml || echo "RABBITMQ 환경변수 미발견"
# 6. .env 파일 확인
echo -e "\n[.env 파일 확인]"
if [ -f ".env" ] || [ -f ".env.local" ]; then
echo ".env 파일 발견"
[ -f ".env" ] && head -10 .env
[ -f ".env.local" ] && head -10 .env.local
else
echo ".env 파일 미발견"
fi
else
echo "[✗] docker-compose.local.yml 파일 미발견"
echo "사용 가능한 compose 파일:"
find . -name "docker-compose*.yml" -o -name "docker-compose*.yaml"
fiRepository: D-ingle/backend
Length of output: 669
RabbitMQ 자격증명과 포트 바인딩 설정 개선 필요
Lines 32-33의 포트가 모든 인터페이스(0.0.0.0)에 바인딩되어 있고, Lines 35-36의 guest/guest 자격증명이 하드코딩되어 있습니다. 로컬 개발환경에서도 다음을 권장합니다:
- 포트 바인딩을
127.0.0.1로 제한하여 로컬호스트 접근만 허용 - 자격증명을
.env파일로 분리하여 버전 제어 대상에서 제외
권장 수정안
ports:
- - "5672:5672" # AMQP
- - "15672:15672" # Management UI
+ - "127.0.0.1:5672:5672" # AMQP (로컬 접근만)
+ - "127.0.0.1:15672:15672" # Management UI (로컬 접근만)
environment:
- RABBITMQ_DEFAULT_USER: guest
- RABBITMQ_DEFAULT_PASS: guest
+ RABBITMQ_DEFAULT_USER: ${RABBITMQ_DEFAULT_USER:-guest}
+ RABBITMQ_DEFAULT_PASS: ${RABBITMQ_DEFAULT_PASS:-guest}.env 파일에 추가:
RABBITMQ_DEFAULT_USER=guest
RABBITMQ_DEFAULT_PASS=guest
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-compose.local.yml` around lines 31 - 36, Change the port bindings from
"5672:5672" and "15672:15672" to bind only to localhost by using
"127.0.0.1:5672:5672" and "127.0.0.1:15672:15672", and stop hardcoding
credentials by replacing RABBITMQ_DEFAULT_USER: guest and RABBITMQ_DEFAULT_PASS:
guest with environment variable references like RABBITMQ_DEFAULT_USER:
${RABBITMQ_DEFAULT_USER} and RABBITMQ_DEFAULT_PASS: ${RABBITMQ_DEFAULT_PASS};
then add the actual values to your .env (e.g. RABBITMQ_DEFAULT_USER=guest,
RABBITMQ_DEFAULT_PASS=guest) and ensure the .env is excluded from version
control.
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.
🔍️ 이 PR을 통해 해결하려는 문제가 무엇인가요?
✨ 이 PR에서 핵심적으로 변경된 사항은 무엇일까요?
🙏 Reviewer 분들이 이런 부분을 신경써서 봐 주시면 좋겠어요
🩺 이 PR에서 테스트 혹은 검증이 필요한 부분이 있을까요?
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
릴리스 노트