feat: 건강 검진 결과 수정 기능 구현 - #56
Merged
Merged
Conversation
📝 WalkthroughWalkthroughPR은 건강 검진 결과 수정 기능을 구현하며, 기존 검진 기록 수정, 임베딩 업데이트, 식단 추천 기능 연동을 포함합니다. 요청 타입을 WriteHealthReportRequest로 통합하고, UpdateHealthReportCommand와 UpdateHealthReportService를 추가하여 업데이트 워크플로우를 처리합니다. Changes
Sequence DiagramsequenceDiagram
actor Client
participant Controller as HealthReportController
participant Service as UpdateHealthReportService
participant MemberPort as MemberPersistencePort
participant ReportPort as HealthReportPersistencePort
participant HealthScoreUseCase
participant EmbeddingService as SaveHealthReportEmbeddingUseCase
participant MealUseCase as CreateRecommendedMealUseCase
Client->>Controller: PUT /{healthReportId} with WriteHealthReportRequest
Controller->>Service: updateHealthReport(UpdateHealthReportCommand)
Service->>MemberPort: getMember(memberId)
MemberPort-->>Service: Member
Service->>ReportPort: getHealthReport(memberId, healthReportId)
ReportPort-->>Service: HealthReport
Service->>ReportPort: checkDateConflict(memberId, newDate)
ReportPort-->>Service: conflictStatus
Service->>Service: healthReport.update(command)
Service->>ReportPort: saveHealthReport(healthReport)
ReportPort-->>Service: saved
Service->>HealthScoreUseCase: updateHealthScore(member)
HealthScoreUseCase-->>Service: completed
Service->>EmbeddingService: embeddingAndSave(text, member, report)
Note over EmbeddingService: async execution
EmbeddingService-->>Service: scheduled
Service->>MealUseCase: createRecommendedMeal(...)
MealUseCase-->>Service: created
Service-->>Controller: void
Controller-->>Client: 200 OK with HEALTH_REPORT_UPDATED
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 4
🧹 Nitpick comments (1)
src/main/java/org/sopt/carena/healthreport/application/dto/command/UpdateHealthReportCommand.java (1)
5-5: 애플리케이션 계층이 웹 요청 DTO를 직접 알지 않게 분리해 주세요.
application.dto.command가adapter.in.web.request.WriteHealthReportRequest를 import하면 인바운드 어댑터 의존성이 안쪽 계층으로 새어 들어옵니다. 생성 커맨드도 같은 패턴이라, 매핑은 컨트롤러나 전용 매퍼에서 처리하고 커맨드는 웹 타입을 모르게 두는 편이 경계 유지에 안전합니다.Also applies to: 30-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/carena/healthreport/application/dto/command/UpdateHealthReportCommand.java` at line 5, UpdateHealthReportCommand currently imports the inbound web DTO WriteHealthReportRequest which leaks adapter concerns into the application layer; remove that import and make UpdateHealthReportCommand use plain fields or domain/value types (e.g., Long userId, LocalDate reportDate, String content, etc.) and provide a constructor/factory that accepts those primitives/domain types only; then move the mapping from WriteHealthReportRequest to UpdateHealthReportCommand into the controller or a dedicated mapper (e.g., HealthReportController or HealthReportMapper) so controllers translate WriteHealthReportRequest -> UpdateHealthReportCommand and the application layer (UpdateHealthReportCommand, its fields and any related service methods) has no reference to adapter.in.web.request types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/sopt/carena/healthreport/adapter/in/web/controller/HealthReportController.java`:
- Around line 72-75: Change the healthReportId path variable from String to a
numeric type so Spring performs validation: in HealthReportController
updateHealthReport and getEntireHealthReport change the parameter from
`@PathVariable`(name = "healthReportId") final String healthReportId to
`@PathVariable`(name = "healthReportId") final long healthReportId and remove
Long.parseLong(...) usages (call UpdateHealthReportCommand.of(memberId,
healthReportId, request) directly). Also update the corresponding method
signatures in the HealthReportApiDocs interface to accept a long healthReportId
so both controller and API docs are consistent.
In
`@src/main/java/org/sopt/carena/healthreport/adapter/out/persistence/mapper/HealthReportMapper.java`:
- Around line 36-38: The mapper HealthReportMapper.toEntity sets
.id(domain.getId()) which creates a detached entity that when passed into
healthReportRepository.save(...) (from HealthReportPersistenceAdapter) risks an
incomplete merge because BaseEntity.createdAt and other persistent fields are
omitted; fix by either (A) changing UpdateHealthReportService to fetch the
managed HealthReportEntity, apply domain-level changes onto that managed entity
and then save the managed instance (do not construct a new entity with an ID),
or (B) extend HealthReportMapper.toEntity to copy all persistent fields from the
domain/DTO into the entity—including BaseEntity.createdAt (and any other
audit/persistent fields)—so the saved entity is a full snapshot and not a
partially populated detached entity; pick one approach and update references to
HealthReportMapper.toEntity and the save call in HealthReportPersistenceAdapter
accordingly.
In
`@src/main/java/org/sopt/carena/healthreport/application/service/CreateHealthReportService.java`:
- Around line 53-54: The submitted task discards the Future so exceptions from
saveHealthReportEmbeddingUseCase.embeddingAndSave can be swallowed; change the
submitted Runnable to catch Throwable around embeddingAndSave (or use a
CompletableFuture/whenComplete) and log any exception (including
non-EmbeddingFailedException) so failures are observable; update the code that
calls virtualExecutorService.submit(...) to either store and handle the returned
Future or wrap the call to
saveHealthReportEmbeddingUseCase.embeddingAndSave(...) in a try/catch that logs
the error (include member, healthReport id/context) before rethrowing or
handling, referencing virtualExecutorService.submit and
saveHealthReportEmbeddingUseCase.embeddingAndSave.
In
`@src/main/java/org/sopt/carena/healthreport/application/service/SaveHealthReportEmbeddingService.java`:
- Around line 26-45: The retry currently only triggers for
EmbeddingFailedException, but embeddingAndSave calls embeddingPort.embed(...)
and healthReportEmbeddingPersistencePort.saveHealthReportEmbedding(...) which
may throw other runtime exceptions; either wrap those external exceptions in
EmbeddingFailedException (catch exceptions around embeddingPort.embed(...) and
saveHealthReportEmbedding(...) and rethrow new EmbeddingFailedException(cause))
so `@Retryable`(retryFor = EmbeddingFailedException.class) will apply, or broaden
the `@Retryable` on embeddingAndSave to include the specific persistence/port
exception types (or RuntimeException) so failures during save also retry; update
embeddingAndSave in SaveHealthReportEmbeddingService accordingly.
---
Nitpick comments:
In
`@src/main/java/org/sopt/carena/healthreport/application/dto/command/UpdateHealthReportCommand.java`:
- Line 5: UpdateHealthReportCommand currently imports the inbound web DTO
WriteHealthReportRequest which leaks adapter concerns into the application
layer; remove that import and make UpdateHealthReportCommand use plain fields or
domain/value types (e.g., Long userId, LocalDate reportDate, String content,
etc.) and provide a constructor/factory that accepts those primitives/domain
types only; then move the mapping from WriteHealthReportRequest to
UpdateHealthReportCommand into the controller or a dedicated mapper (e.g.,
HealthReportController or HealthReportMapper) so controllers translate
WriteHealthReportRequest -> UpdateHealthReportCommand and the application layer
(UpdateHealthReportCommand, its fields and any related service methods) has no
reference to adapter.in.web.request types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a25142cf-8187-4794-b4bc-d6fbe5a06025
📒 Files selected for processing (19)
src/main/java/org/sopt/carena/global/common/BaseEntity.javasrc/main/java/org/sopt/carena/healthreport/adapter/in/web/code/SuccessCode.javasrc/main/java/org/sopt/carena/healthreport/adapter/in/web/controller/HealthReportApiDocs.javasrc/main/java/org/sopt/carena/healthreport/adapter/in/web/controller/HealthReportController.javasrc/main/java/org/sopt/carena/healthreport/adapter/in/web/request/WriteHealthReportRequest.javasrc/main/java/org/sopt/carena/healthreport/adapter/out/persistence/HealthReportEmbeddingPersistenceAdapter.javasrc/main/java/org/sopt/carena/healthreport/adapter/out/persistence/entity/HealthReportEmbeddingEntity.javasrc/main/java/org/sopt/carena/healthreport/adapter/out/persistence/entity/HealthReportEntity.javasrc/main/java/org/sopt/carena/healthreport/adapter/out/persistence/mapper/HealthReportEmbeddingMapper.javasrc/main/java/org/sopt/carena/healthreport/adapter/out/persistence/mapper/HealthReportMapper.javasrc/main/java/org/sopt/carena/healthreport/application/dto/command/CreateHealthReportCommand.javasrc/main/java/org/sopt/carena/healthreport/application/dto/command/UpdateHealthReportCommand.javasrc/main/java/org/sopt/carena/healthreport/application/port/in/SaveHealthReportEmbeddingUseCase.javasrc/main/java/org/sopt/carena/healthreport/application/port/in/UpdateHealthReportUseCase.javasrc/main/java/org/sopt/carena/healthreport/application/service/CreateHealthReportService.javasrc/main/java/org/sopt/carena/healthreport/application/service/SaveHealthReportEmbeddingService.javasrc/main/java/org/sopt/carena/healthreport/application/service/UpdateHealthReportService.javasrc/main/java/org/sopt/carena/healthreport/domain/HealthReport.javasrc/main/java/org/sopt/carena/healthreport/domain/HealthReportEmbedding.java
Comment on lines
+72
to
+75
| @PathVariable(name = "healthReportId") final String healthReportId, | ||
| @Valid @RequestBody final WriteHealthReportRequest request | ||
| ) { | ||
| updateHealthReportUseCase.updateHealthReport(UpdateHealthReportCommand.of(memberId, Long.parseLong(healthReportId), request)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify manual parsing of healthReportId in controller/api-docs signatures.
rg -n -C2 '@PathVariable\(name = "healthReportId"\) final String|Long\.parseLong\(healthReportId\)|updateHealthReport\(long memberId, String healthReportId' src/main/javaRepository: team-carena/carena-server
Length of output: 3485
🏁 Script executed:
# Check entire HealthReportController for all methods using healthReportId
cat -n src/main/java/org/sopt/carena/healthreport/adapter/in/web/controller/HealthReportController.java | head -120Repository: team-carena/carena-server
Length of output: 7046
🏁 Script executed:
# Check HealthReportApiDocs interface signatures
cat -n src/main/java/org/sopt/carena/healthreport/adapter/in/web/controller/HealthReportApiDocs.javaRepository: team-carena/carena-server
Length of output: 5414
🏁 Script executed:
# Search for all Long.parseLong usages in health report module
rg -n 'Long\.parseLong' src/main/java/org/sopt/carena/healthreport/Repository: team-carena/carena-server
Length of output: 499
healthReportId는 String 파싱 대신 숫자 타입으로 바로 바인딩하세요.
updateHealthReport와 getEntireHealthReport 두 메서드 모두에서 Long.parseLong()으로 수동 파싱하고 있습니다. 잘못된 값이 들어오면 NumberFormatException으로 500 에러가 발생하므로, @PathVariable long healthReportId로 받아서 Spring이 타입 검증을 맡기세요. 이에 맞춰 HealthReportApiDocs 인터페이스의 두 메서드 시그니처도 함께 수정하세요.
🔧 제안 수정안
`@PutMapping`(path = "/{healthReportId}")
public ResponseEntity<SuccessResponse<Void>> updateHealthReport(
`@AuthenticationPrincipal` final long memberId,
- `@PathVariable`(name = "healthReportId") final String healthReportId,
+ `@PathVariable`(name = "healthReportId") final long healthReportId,
`@Valid` `@RequestBody` final WriteHealthReportRequest request
) {
- updateHealthReportUseCase.updateHealthReport(UpdateHealthReportCommand.of(memberId, Long.parseLong(healthReportId), request));
+ updateHealthReportUseCase.updateHealthReport(UpdateHealthReportCommand.of(memberId, healthReportId, request)); `@GetMapping`(path = "/{healthReportId}")
public ResponseEntity<SuccessResponse<EntireHealthReportView>> getEntireHealthReport(
`@AuthenticationPrincipal` final long memberId,
- `@PathVariable`(name = "healthReportId") final String healthReportId
+ `@PathVariable`(name = "healthReportId") final long healthReportId
) {
- return ResponseEntity.status(SuccessCode.HEALTH_REPORT_FOUND.getStatus())
- .body(ApiResponse.success(SuccessCode.HEALTH_REPORT_FOUND,
- getEntireHealthReportUseCase.getEntireHealthReport(memberId, Long.parseLong(healthReportId))));
+ return ResponseEntity.status(SuccessCode.HEALTH_REPORT_FOUND.getStatus())
+ .body(ApiResponse.success(SuccessCode.HEALTH_REPORT_FOUND,
+ getEntireHealthReportUseCase.getEntireHealthReport(memberId, healthReportId))); `@Operation`(summary = "건강 검진 결과 수정", description = "건강 검진 결과를 수정합니다.")
-ResponseEntity<SuccessResponse<Void>> updateHealthReport(long memberId, String healthReportId, `@Valid` WriteHealthReportRequest request);
+ResponseEntity<SuccessResponse<Void>> updateHealthReport(long memberId, long healthReportId, `@Valid` WriteHealthReportRequest request);
`@Operation`(summary = "건강 검진 결과 중 내용 전체 조회", description = "식별자에 해당하는 건강 검진 결과의 전체 내용을 조회합니다.")
-ResponseEntity<SuccessResponse<EntireHealthReportView>> getEntireHealthReport(long memberId, String healthReportId);
+ResponseEntity<SuccessResponse<EntireHealthReportView>> getEntireHealthReport(long memberId, long healthReportId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/sopt/carena/healthreport/adapter/in/web/controller/HealthReportController.java`
around lines 72 - 75, Change the healthReportId path variable from String to a
numeric type so Spring performs validation: in HealthReportController
updateHealthReport and getEntireHealthReport change the parameter from
`@PathVariable`(name = "healthReportId") final String healthReportId to
`@PathVariable`(name = "healthReportId") final long healthReportId and remove
Long.parseLong(...) usages (call UpdateHealthReportCommand.of(memberId,
healthReportId, request) directly). Also update the corresponding method
signatures in the HealthReportApiDocs interface to accept a long healthReportId
so both controller and API docs are consistent.
Comment on lines
36
to
+38
| public static HealthReportEntity toEntity(final HealthReport domain) { | ||
| return HealthReportEntity.builder() | ||
| .id(domain.getId()) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# HealthReportMapper.toEntity(...) 호출 위치 확인
rg -n -C3 --type=java 'HealthReportMapper\.toEntity\(' -g '*HealthReport*.java'
# HealthReport 저장/병합 구현 확인
rg -n -C5 --type=java 'implements HealthReportPersistencePort|EntityManager\.merge\(|saveAndFlush\s*\(|save\s*\(' -g '*HealthReport*.java'
# 업데이트 서비스에서 저장까지 이어지는 흐름 확인
rg -n -C8 --type=java 'class UpdateHealthReportService|updateHealthReport\s*\(|healthReportPersistencePort\.' -g '*HealthReport*.java'Repository: team-carena/carena-server
Length of output: 46016
🏁 Script executed:
cat -n src/main/java/org/sopt/carena/healthreport/adapter/out/persistence/mapper/HealthReportMapper.javaRepository: team-carena/carena-server
Length of output: 4185
🏁 Script executed:
cat -n src/main/java/org/sopt/carena/healthreport/adapter/out/persistence/entity/HealthReportEntity.java | head -50Repository: team-carena/carena-server
Length of output: 1885
🏁 Script executed:
cat -n src/main/java/org/sopt/carena/global/common/BaseEntity.javaRepository: team-carena/carena-server
Length of output: 746
ID를 포함한 detached 엔티티 저장이 불완전한 merge 위험을 만듭니다.
toEntity() 메서드에서 .id(domain.getId())를 설정하면서 업데이트 경로가 detached HealthReportEntity를 save()에 전달하게 됩니다. Spring Data JPA는 ID가 있는 detached 엔티티를 merge하는데, 이 매퍼는 BaseEntity의 createdAt 같은 감사 필드를 싣지 않아 불완전한 엔티티 스냅샷을 만듭니다.
@Column(updatable = false)가 SQL UPDATE를 막더라도, 패턴 자체가 문제입니다. 향후 BaseEntity나 HealthReportEntity에 새 필드가 추가되면 같은 위험이 노출될 수 있습니다.
수정 경로가 조회된 managed 엔티티를 직접 갱신하거나, 최소한 매퍼가 모든 persistent 상태를 보존하도록 개선하세요:
- 옵션 1:
UpdateHealthReportService에서 조회한HealthReport를 도메인 레벨에서만 갱신하고, persistence adapter에서는 그 상태를 유지한 채 관리 엔티티를 업데이트 - 옵션 2: 매퍼에서 모든
BaseEntity필드(현재createdAt)를 포함하도록 확장
확인된 코드 위치
src/main/java/org/sopt/carena/healthreport/adapter/out/persistence/mapper/HealthReportMapper.java
- 라인 38: .id(domain.getId())
- 라인 64: .id(domain.getId())
src/main/java/org/sopt/carena/healthreport/adapter/out/persistence/HealthReportPersistenceAdapter.java
- 라인 33: healthReportRepository.save(HealthReportMapper.toEntity(...))
src/main/java/org/sopt/carena/global/common/BaseEntity.java
- `@CreatedDate` createdAt 필드는 매퍼에서 누락됨
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/sopt/carena/healthreport/adapter/out/persistence/mapper/HealthReportMapper.java`
around lines 36 - 38, The mapper HealthReportMapper.toEntity sets
.id(domain.getId()) which creates a detached entity that when passed into
healthReportRepository.save(...) (from HealthReportPersistenceAdapter) risks an
incomplete merge because BaseEntity.createdAt and other persistent fields are
omitted; fix by either (A) changing UpdateHealthReportService to fetch the
managed HealthReportEntity, apply domain-level changes onto that managed entity
and then save the managed instance (do not construct a new entity with an ID),
or (B) extend HealthReportMapper.toEntity to copy all persistent fields from the
domain/DTO into the entity—including BaseEntity.createdAt (and any other
audit/persistent fields)—so the saved entity is a full snapshot and not a
partially populated detached entity; pick one approach and update references to
HealthReportMapper.toEntity and the save call in HealthReportPersistenceAdapter
accordingly.
Comment on lines
+53
to
+54
| virtualExecutorService.submit( | ||
| () -> saveHealthReportEmbeddingUseCase.embeddingAndSave(embeddingText, member, healthReport)); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Java ExecutorService submit(Runnable) exception handling when the returned Future is ignored, compared with execute(Runnable)
💡 Result:
execute(Runnable): ifrun()throws an unchecked exception (RuntimeException/Error), it is treated as an uncaught exception on the worker thread, so the thread’sUncaughtExceptionHandler(or the JVM default handler) can see/log it, and the pool will typically replace the dying worker thread. (stackoverflow.com)submit(Runnable): the executor wraps the task in aFuture(typically aFutureTask). Any exception thrown byrun()is captured and stored in thatFutureand is only rethrown (wrapped inExecutionException) if you later callFuture.get(). The thread’sUncaughtExceptionHandleris not notified. (stackoverflow.com)- If you ignore the returned
Futurefromsubmit(Runnable): the exception is effectively silenced/lost (unless you add your own reporting, e.g., wrap theRunnablewithtry/catch+ logging, or overrideThreadPoolExecutor.afterExecuteand extract the exception from theFuture). (community.sonarsource.com)
Citations:
- 1: https://stackoverflow.com/questions/1838923/why-is-uncaughtexceptionhandler-not-called-by-executorservice?utm_source=openai
- 2: https://stackoverflow.com/questions/1838923/why-is-uncaughtexceptionhandler-not-called-by-executorservice?utm_source=openai
- 3: https://community.sonarsource.com/t/java-always-handle-future-returned-by-executorservice-submit/20186?utm_source=openai
🏁 Script executed:
cd /tmp || cd .
find . -type f -name "CreateHealthReportService.java" 2>/dev/null | head -5Repository: team-carena/carena-server
Length of output: 51
🏁 Script executed:
find . -type f -name "SaveHealthReportEmbeddingUseCase.java" 2>/dev/null | head -5Repository: team-carena/carena-server
Length of output: 171
🏁 Script executed:
find . -type f -name "*SaveHealthReportEmbedding*" 2>/dev/null | head -10Repository: team-carena/carena-server
Length of output: 274
🏁 Script executed:
wc -l src/main/java/org/sopt/carena/healthreport/application/service/CreateHealthReportService.javaRepository: team-carena/carena-server
Length of output: 165
🏁 Script executed:
cat -n src/main/java/org/sopt/carena/healthreport/application/service/CreateHealthReportService.java | sed -n '40,70p'Repository: team-carena/carena-server
Length of output: 821
🏁 Script executed:
cat -n src/main/java/org/sopt/carena/healthreport/application/port/in/SaveHealthReportEmbeddingUseCase.javaRepository: team-carena/carena-server
Length of output: 446
🏁 Script executed:
cat -n src/main/java/org/sopt/carena/healthreport/application/service/SaveHealthReportEmbeddingService.javaRepository: team-carena/carena-server
Length of output: 2606
비동기 예외가 관찰되지 않습니다.
Line 53에서 submit()의 반환값을 버리고 있어서, embeddingAndSave()가 EmbeddingFailedException 이외의 예외로 실패하면 예외가 Future 안에 갇힌 후 호출부에서는 전혀 관측되지 않습니다. 건강검진 리포트 임베딩 동기화는 비즈니스 흐름의 핵심 후처리이므로, 명시적 로깅으로 모든 예외를 드러내는 것이 안전합니다.
제안 수정안
- virtualExecutorService.submit(
- () -> saveHealthReportEmbeddingUseCase.embeddingAndSave(embeddingText, member, healthReport));
+ virtualExecutorService.execute(() -> {
+ try {
+ saveHealthReportEmbeddingUseCase.embeddingAndSave(embeddingText, member, healthReport);
+ } catch (Exception e) {
+ log.error("건강검진 리포트 임베딩 저장 비동기 작업 실패: memberId={}, reportId={}",
+ member.getId(), healthReport.getId(), e);
+ }
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/sopt/carena/healthreport/application/service/CreateHealthReportService.java`
around lines 53 - 54, The submitted task discards the Future so exceptions from
saveHealthReportEmbeddingUseCase.embeddingAndSave can be swallowed; change the
submitted Runnable to catch Throwable around embeddingAndSave (or use a
CompletableFuture/whenComplete) and log any exception (including
non-EmbeddingFailedException) so failures are observable; update the code that
calls virtualExecutorService.submit(...) to either store and handle the returned
Future or wrap the call to
saveHealthReportEmbeddingUseCase.embeddingAndSave(...) in a try/catch that logs
the error (include member, healthReport id/context) before rethrowing or
handling, referencing virtualExecutorService.submit and
saveHealthReportEmbeddingUseCase.embeddingAndSave.
Comment on lines
+26
to
+45
| @Retryable( | ||
| retryFor = EmbeddingFailedException.class, | ||
| maxAttempts = 3, | ||
| backoff = @Backoff(delay = 5000, multiplier = 2), | ||
| recover = "recoverEmbedding" | ||
| ) | ||
| public void embeddingAndSave(final String embeddingText, final Member member, final HealthReport healthReport) { | ||
| // 임베딩 호출 | ||
| float[] embedding = embeddingPort.embed(embeddingText).vector(); | ||
|
|
||
| // 임베딩 결과 도메인 생성 | ||
| HealthReportEmbedding healthReportEmbedding = HealthReportEmbedding.builder() | ||
| .embeddingText(embeddingText) | ||
| .embedding(embedding) | ||
| .memberId(member.getId()) | ||
| .healthReportId(healthReport.getId()) | ||
| .build(); | ||
|
|
||
| // 임베딩 결과 저장 | ||
| healthReportEmbeddingPersistencePort.saveHealthReportEmbedding(healthReportEmbedding); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Spring Retry, when a method annotated with @retryable(retryFor = EmbeddingFailedException.class) throws a different RuntimeException, does it retry or invoke @recover?
💡 Result:
No.
With @Retryable(retryFor = EmbeddingFailedException.class), Spring Retry only retries when the thrown exception is an EmbeddingFailedException (or a subtype). If the method throws a different RuntimeException, it is treated as not retryable and is thrown to the caller immediately—Spring Retry will not retry and will not invoke @Recover (since recovery is for an exhausted retry). [1]
Also, @Recover methods are selected based on the exception type they declare (a matching Throwable subtype) and the original method signature/return type. [2]
Sources:
[1] Spring Retry README (exception classification / “exception is thrown … immediately, if the exception is not retryable”) (github.com)
[2] Spring Retry @Recover Javadoc (docs.spring.io)
Citations:
- 1: https://github.com/spring-projects/spring-retry
- 2: https://docs.spring.io/spring-retry/docs/api/current/org/springframework/retry/annotation/Recover.html?utm_source=openai
저장 단계 실패는 재시도/복구 경로에 진입하지 않습니다.
Line 26에서 @Retryable(retryFor = EmbeddingFailedException.class)로 제한한 상태라, Line 34의 임베딩 호출이나 Line 45의 saveHealthReportEmbedding(...)가 다른 런타임 예외를 던지면 첫 실패 후 즉시 호출자에게 전달되고 @Recover도 실행되지 않습니다. 비동기 처리이므로 외부 포트 예외를 EmbeddingFailedException으로 변환하거나, 재시도 대상을 저장 예외까지 포함하도록 확대해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/sopt/carena/healthreport/application/service/SaveHealthReportEmbeddingService.java`
around lines 26 - 45, The retry currently only triggers for
EmbeddingFailedException, but embeddingAndSave calls embeddingPort.embed(...)
and healthReportEmbeddingPersistencePort.saveHealthReportEmbedding(...) which
may throw other runtime exceptions; either wrap those external exceptions in
EmbeddingFailedException (catch exceptions around embeddingPort.embed(...) and
saveHealthReportEmbedding(...) and rethrow new EmbeddingFailedException(cause))
so `@Retryable`(retryFor = EmbeddingFailedException.class) will apply, or broaden
the `@Retryable` on embeddingAndSave to include the specific persistence/port
exception types (or RuntimeException) so failures during save also retry; update
embeddingAndSave in SaveHealthReportEmbeddingService accordingly.
byunhm02
approved these changes
Mar 11, 2026
byunhm02
left a comment
byunhm02
left a comment
Contributor
There was a problem hiding this comment.
깔끔하게 잘 구현하셨네용~~수고하셨습니다~!!!!!!!
This was referenced Mar 23, 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
2 participants
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.
🚀 Related issue
closes #55
#️⃣ Summary
🎯 Work Description
👍 동작 확인
💬 To Reviewers
requestbody와 형태가 동일하고 추후 변경도 동시에 적용되는 상황이라WriteHealthReportRequest로 명칭을 변경하고 두 메서드에서 재사용하도록 구현하였습니다.Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항