Conversation
SseEmitterService 내부에 하드코딩되어 있던 MongoTemplate 의존성을 제거하고, SseDeliveryFailedEvent 이벤트를 발행하도록 변경했습니다. 발행된 이벤트는 WebhookLogService의 이벤트 리스너가 처리하여 sseDeliveryStatus 상태를 갱신합니다. 이를 통해 각 서비스가 독립적인 책임을 가지도록 아키텍처를 개선했습니다.
리뷰 피드백 반영: - SseEmitterService에서 SSE 전송 실패 시 이벤트가 정상 발행되는지 검증 (SseEmitterServiceTest) - WebhookLogService에서 해당 이벤트를 수신하여 MongoDB에 반영하는지 검증 (WebhookLogServiceTest) 이벤트 기반 구조 변경(MongoTemplate 제거)에 대한 실제 동작을 단위 테스트로 확인합니다.
MockResponseScheduler 내부에 존재하던 HTTP 헤더 검증 및 Content-Type 변환 로직을 HttpHeaderSanitizer로 추출하여 단일 책임 원칙(SRP)을 준수하도록 개선했습니다. 새로운 클래스에 대한 단위 테스트를 추가했습니다.
Extract log capacity enforcement and payload processing from WebhookService to reduce tight coupling and adhere to Single Responsibility Principle.
수동으로 ObjectMapper를 생성할 경우 Spring Boot의 전역 Jackson 설정이 반영되지 않는 문제를 해결하기 위해 의존성 주입 방식으로 변경했습니다.
updateCountersAndEnforceCap 메서드의 반환값이 WebhookService에서 사용되지 않으므로 반환 타입을 void로 변경했습니다.
표준 Java 컨벤션에 맞추어 임포트 구문을 정렬했습니다.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough웹훅 payload 처리와 로그 cap 집행을 전용 컴포넌트로 분리하고, mock 응답 헤더 sanitization을 공통화했습니다. SSE 전달 실패는 이벤트로 발행한 뒤 비동기 로그 상태 갱신으로 처리합니다. Changes웹훅 처리 흐름
작업공간 무시 규칙
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SseEmitterService
participant ApplicationEventPublisher
participant WebhookLogService
participant MongoDB
SseEmitterService->>ApplicationEventPublisher: SSE 실패 이벤트 발행
ApplicationEventPublisher->>WebhookLogService: 실패 이벤트 전달
WebhookLogService->>MongoDB: 실패 상태 및 오류 메시지 갱신
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 4
🧹 Nitpick comments (8)
FH_backend/src/main/java/com/flashhook/domain/webhook/util/HttpHeaderSanitizer.java (1)
27-33: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
toLowerCase()에Locale.ROOT를 명시하세요.기본 로케일 의존은 터키어 로케일(
tr-TR)에서I→ı변환으로 allowlist 매칭과 content-type 비교가 어긋날 수 있습니다.♻️ 제안
- if (ALLOWED_HEADERS.contains(k.toLowerCase())) { + if (ALLOWED_HEADERS.contains(k.toLowerCase(Locale.ROOT))) { String sanitizedValue = v.replaceAll( "[\\x00-\\x1F\\x7F]", "" ); if ("content-type".equalsIgnoreCase(k)) { - String lowerValue = sanitizedValue.toLowerCase(); + String lowerValue = sanitizedValue.toLowerCase(Locale.ROOT);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FH_backend/src/main/java/com/flashhook/domain/webhook/util/HttpHeaderSanitizer.java` around lines 27 - 33, Update the lowercase conversions in HttpHeaderSanitizer to use Locale.ROOT, including the allowlist key normalization and sanitized Content-Type value normalization, so matching remains locale-independent.FH_backend/src/test/java/com/flashhook/domain/webhook/service/SseEmitterServiceTest.java (1)
41-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value빈
@BeforeEach메서드를 제거하세요.♻️ 제안
- `@BeforeEach` - void setUp() { - } -
org.junit.jupiter.api.BeforeEachimport도 함께 제거해야 합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FH_backend/src/test/java/com/flashhook/domain/webhook/service/SseEmitterServiceTest.java` around lines 41 - 43, Remove the empty setUp() method annotated with `@BeforeEach` from SseEmitterServiceTest and remove the now-unused org.junit.jupiter.api.BeforeEach import.FH_backend/src/test/java/com/flashhook/domain/webhook/util/HttpHeaderSanitizerTest.java (1)
12-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win엣지 케이스 테스트 보강을 권장합니다.
현재 5개 케이스는 정상 경로를 잘 덮지만,
sanitize(null), 값이null인 엔트리, 그리고 파싱 불가능한 content-type(예:application/json; charset=) 케이스가 빠져 있습니다. 특히 마지막 케이스는HttpHeaderSanitizer.javaLine 64에서 지적한 예외 경로를 고정하는 데 필요합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FH_backend/src/test/java/com/flashhook/domain/webhook/util/HttpHeaderSanitizerTest.java` around lines 12 - 76, Expand HttpHeaderSanitizerTest with edge-case tests for sanitize(null), headers containing a null value, and an unparsable content type such as application/json; charset=. Assert the expected safe behavior for each case, including the fallback that prevents the exception path in HttpHeaderSanitizer.FH_backend/src/main/java/com/flashhook/domain/webhook/service/WebhookLogService.java (1)
176-182: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick wincatch 범위가
DataAccessException으로 좁습니다.
@Async리스너에서 그 외 런타임 예외가 발생하면AsyncUncaughtExceptionHandler가 없는 한 조용히 사라집니다. 발행 측(SseEmitterServiceLine 112)도Exception으로 넓혀 잡고 있으니 여기도 맞추는 편이 일관됩니다.참고로 PMD의
InvalidLogMessageFormat경고(Line 177-181)는 SLF4J가 마지막Throwable인자를 스택트레이스로 처리하므로 거짓 양성입니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FH_backend/src/main/java/com/flashhook/domain/webhook/service/WebhookLogService.java` around lines 176 - 182, Update the exception handler surrounding the asynchronous webhook log persistence flow in WebhookLogService to catch Exception instead of only DataAccessException, matching the broader handling used by SseEmitterService. Preserve the existing error message, logId argument, and throwable stack-trace logging.Source: Linters/SAST tools
FH_backend/src/main/java/com/flashhook/domain/webhook/event/SseDeliveryFailedEvent.java (1)
7-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value불변
record로 정의하는 편이 이벤트 의미에 부합합니다.이벤트는 발행 후 여러 리스너가 공유하므로 불변이 안전하고, 동일 PR의
WebhookPayloadProcessor.ProcessedPayload도 record를 쓰고 있어 일관성이 좋아집니다. 다만@NoArgsConstructor가 역직렬화 등 다른 용도로 필요하다면 현행 유지도 무방합니다.♻️ 제안
-import lombok.AllArgsConstructor; -import lombok.Getter; -import lombok.NoArgsConstructor; - -@Getter -@NoArgsConstructor -@AllArgsConstructor -public class SseDeliveryFailedEvent { - private String logId; - private String errorMessage; -} +public record SseDeliveryFailedEvent(String logId, String errorMessage) {}record 전환 시
getLogId()/getErrorMessage()호출부(WebhookLogService, 테스트 2종)도 함께 수정해야 합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FH_backend/src/main/java/com/flashhook/domain/webhook/event/SseDeliveryFailedEvent.java` around lines 7 - 13, Convert SseDeliveryFailedEvent from a mutable Lombok class to an immutable record with logId and errorMessage components, unless `@NoArgsConstructor` is required for deserialization. Update all getLogId() and getErrorMessage() callers, including WebhookLogService and the two tests, to use record accessors while preserving existing event behavior.FH_backend/src/test/java/com/flashhook/domain/webhook/service/WebhookLogServiceTest.java (1)
37-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win실패 경로 테스트가 없습니다.
현재는 정상 업데이트 1건만 검증합니다.
DataAccessException발생 시 예외가 전파되지 않고 로깅으로 끝나는지, 그리고 (제안대로 매칭 0건 처리를 추가한다면) 문서 미존재 케이스도 함께 덮어 주세요. 참고로mongoTemplate.updateFirst가 stubbing되지 않아null을 반환하므로,WebhookLogService에서 반환값을 사용하도록 바꾸면 이 테스트도 함께 수정해야 합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FH_backend/src/test/java/com/flashhook/domain/webhook/service/WebhookLogServiceTest.java` around lines 37 - 62, Extend the WebhookLogServiceTest coverage for handleSseDeliveryFailed with a DataAccessException case, stubbing mongoTemplate.updateFirst as needed and verifying the exception is handled without propagation and logging occurs. If WebhookLogService adds zero-match handling based on the update result, also add a test for a null or non-matching UpdateResult and update the existing success test to stub the returned result.FH_backend/src/main/java/com/flashhook/domain/webhook/service/LogCapEnforcer.java (1)
26-43: 🚀 Performance & Scalability | 🔵 Trivial웹훅 수신 요청 스레드에서 cap 집행이 동기 실행됩니다.
updateCountersAndEnforceCap은WebhookService.receive경로에서 호출되며, 초과분이 클 경우find→findAllAndRemove→updateFirst사이클을 반복하면서 수신 응답 지연을 유발합니다. 별도 비동기 실행(@Async)이나 주기적 배치로 분리하는 방안을 검토해 보세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FH_backend/src/main/java/com/flashhook/domain/webhook/service/LogCapEnforcer.java` around lines 26 - 43, WebhookService.receive의 요청 처리 스레드에서 enforceLogCap이 동기 실행되지 않도록 updateCountersAndEnforceCap의 cap 집행을 별도 비동기 작업으로 분리하세요. 카운터 갱신과 updatedEndpoint 반환은 즉시 수행하고, enforceLogCap 호출은 기존 동작을 유지하면서 `@Async` 또는 프로젝트의 비동기 실행 방식으로 위임하세요.FH_backend/src/main/java/com/flashhook/domain/webhook/service/SseEmitterService.java (1)
106-117: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSSE 실패 기록 비동기 예외 처리를 추가해 주세요.
@EnableAsync와taskExecutor는 설정되어 있지만,WebhookLogService.handleSseDeliveryFailed의 persist 실패는log.error로만 끝나서 실패 판별이 없습니다. 실패 이벤트를 발행하지 않거나 별도 유실 카운터로 집계해 주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FH_backend/src/main/java/com/flashhook/domain/webhook/service/SseEmitterService.java` around lines 106 - 117, WebhookLogService.handleSseDeliveryFailed의 비동기 persist 실패가 log.error만으로 종료되지 않도록 처리하세요. SseEmitterService의 publishEx 예외 경로에서 실패 이벤트를 재발행하지 말고, 별도 유실 카운터나 메트릭에 기록해 실패를 판별할 수 있게 하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@FH_backend/src/main/java/com/flashhook/domain/webhook/service/LogCapEnforcer.java`:
- Around line 92-113: Update the deletion flow around findAllAndRemove so it
does not load full WebhookLog bodies into memory; apply a projection limited to
_id and bodySize, reusing the existing oldLogs projection pattern, while
preserving the removedLogs empty check and removedSize/removedCount
calculations.
In
`@FH_backend/src/main/java/com/flashhook/domain/webhook/service/WebhookLogService.java`:
- Around line 167-183: Update handleSseDeliveryFailed to inspect the
UpdateResult from mongoTemplate.updateFirst and emit a warning when no
WebhookLog matches event.getLogId(), while preserving existing exception
logging. Replace the regular `@EventListener` with `@TransactionalEventListener`
using AFTER_COMMIT so handling occurs only after the surrounding transaction
commits.
In
`@FH_backend/src/main/java/com/flashhook/domain/webhook/util/HttpHeaderSanitizer.java`:
- Around line 32-66: Update the content-type handling in HttpHeaderSanitizer to
validate the complete sanitized value with MediaType.parseMediaType, including
values that pass the existing allowlist. Preserve parseable configured or
mock-request values unchanged, and reset any value that fails parsing to
text/plain before headers.add is called so headers.getContentType() cannot throw
InvalidMediaTypeException.
In
`@FH_backend/src/main/java/com/flashhook/domain/webhook/util/WebhookPayloadProcessor.java`:
- Around line 8-9: Update the JSON configuration annotations in
MockConfig.java—@Jacksonized, `@JsonProperty`, and `@JsonCreator`—to use the Jackson
3.x tools.jackson.annotation package, matching the ObjectMapper and
JacksonException imports shown here; leave unrelated Jackson usage unchanged.
---
Nitpick comments:
In
`@FH_backend/src/main/java/com/flashhook/domain/webhook/event/SseDeliveryFailedEvent.java`:
- Around line 7-13: Convert SseDeliveryFailedEvent from a mutable Lombok class
to an immutable record with logId and errorMessage components, unless
`@NoArgsConstructor` is required for deserialization. Update all getLogId() and
getErrorMessage() callers, including WebhookLogService and the two tests, to use
record accessors while preserving existing event behavior.
In
`@FH_backend/src/main/java/com/flashhook/domain/webhook/service/LogCapEnforcer.java`:
- Around line 26-43: WebhookService.receive의 요청 처리 스레드에서 enforceLogCap이 동기 실행되지
않도록 updateCountersAndEnforceCap의 cap 집행을 별도 비동기 작업으로 분리하세요. 카운터 갱신과
updatedEndpoint 반환은 즉시 수행하고, enforceLogCap 호출은 기존 동작을 유지하면서 `@Async` 또는 프로젝트의 비동기
실행 방식으로 위임하세요.
In
`@FH_backend/src/main/java/com/flashhook/domain/webhook/service/SseEmitterService.java`:
- Around line 106-117: WebhookLogService.handleSseDeliveryFailed의 비동기 persist
실패가 log.error만으로 종료되지 않도록 처리하세요. SseEmitterService의 publishEx 예외 경로에서 실패 이벤트를
재발행하지 말고, 별도 유실 카운터나 메트릭에 기록해 실패를 판별할 수 있게 하세요.
In
`@FH_backend/src/main/java/com/flashhook/domain/webhook/service/WebhookLogService.java`:
- Around line 176-182: Update the exception handler surrounding the asynchronous
webhook log persistence flow in WebhookLogService to catch Exception instead of
only DataAccessException, matching the broader handling used by
SseEmitterService. Preserve the existing error message, logId argument, and
throwable stack-trace logging.
In
`@FH_backend/src/main/java/com/flashhook/domain/webhook/util/HttpHeaderSanitizer.java`:
- Around line 27-33: Update the lowercase conversions in HttpHeaderSanitizer to
use Locale.ROOT, including the allowlist key normalization and sanitized
Content-Type value normalization, so matching remains locale-independent.
In
`@FH_backend/src/test/java/com/flashhook/domain/webhook/service/SseEmitterServiceTest.java`:
- Around line 41-43: Remove the empty setUp() method annotated with `@BeforeEach`
from SseEmitterServiceTest and remove the now-unused
org.junit.jupiter.api.BeforeEach import.
In
`@FH_backend/src/test/java/com/flashhook/domain/webhook/service/WebhookLogServiceTest.java`:
- Around line 37-62: Extend the WebhookLogServiceTest coverage for
handleSseDeliveryFailed with a DataAccessException case, stubbing
mongoTemplate.updateFirst as needed and verifying the exception is handled
without propagation and logging occurs. If WebhookLogService adds zero-match
handling based on the update result, also add a test for a null or non-matching
UpdateResult and update the existing success test to stub the returned result.
In
`@FH_backend/src/test/java/com/flashhook/domain/webhook/util/HttpHeaderSanitizerTest.java`:
- Around line 12-76: Expand HttpHeaderSanitizerTest with edge-case tests for
sanitize(null), headers containing a null value, and an unparsable content type
such as application/json; charset=. Assert the expected safe behavior for each
case, including the fallback that prevents the exception path in
HttpHeaderSanitizer.
🪄 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 Plus
Run ID: aee3d282-1138-40b5-919b-efafccbdbaeb
📒 Files selected for processing (12)
.gitignoreFH_backend/src/main/java/com/flashhook/domain/webhook/event/SseDeliveryFailedEvent.javaFH_backend/src/main/java/com/flashhook/domain/webhook/service/LogCapEnforcer.javaFH_backend/src/main/java/com/flashhook/domain/webhook/service/MockResponseScheduler.javaFH_backend/src/main/java/com/flashhook/domain/webhook/service/SseEmitterService.javaFH_backend/src/main/java/com/flashhook/domain/webhook/service/WebhookLogService.javaFH_backend/src/main/java/com/flashhook/domain/webhook/service/WebhookService.javaFH_backend/src/main/java/com/flashhook/domain/webhook/util/HttpHeaderSanitizer.javaFH_backend/src/main/java/com/flashhook/domain/webhook/util/WebhookPayloadProcessor.javaFH_backend/src/test/java/com/flashhook/domain/webhook/service/SseEmitterServiceTest.javaFH_backend/src/test/java/com/flashhook/domain/webhook/service/WebhookLogServiceTest.javaFH_backend/src/test/java/com/flashhook/domain/webhook/util/HttpHeaderSanitizerTest.java
Comment on lines
+8
to
+9
| import tools.jackson.core.JacksonException; | ||
| import tools.jackson.databind.ObjectMapper; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Jackson 2.x/3.x 혼용 여부 확인
rg -n --type=java 'com\.fasterxml\.jackson|tools\.jackson' -g '!**/build/**' | head -50
fd -e gradle -e xml --exec rg -n -i 'jackson' {} \;Repository: pottq577/flashhook
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files relevant =="
git ls-files | rg '(^|/)(build\.gradle(\.kts)?|gradle\.properties|pom\.xml|settings\.gradle(\.kts)?|WebhookPayloadProcessor\.java|application.*\.ya?ml)$' || true
echo
echo "== files count =="
git ls-files | wc -l
echo
echo "== WebhookPayloadProcessor =="
if [ -f FH_backend/src/main/java/com/flashhook/domain/webhook/util/WebhookPayloadProcessor.java ]; then
cat -n FH_backend/src/main/java/com/flashhook/domain/webhook/util/WebhookPayloadProcessor.java
else
fd -a 'WebhookPayloadProcessor.java' -x sh -c 'echo "--- $1"; cat -n "$1"' sh {}
fi
echo
echo "== Jackson import occurrences =="
rg -n --type=java 'import (?:tools\.jackson|com\.fasterxml\.jackson)|JacksonException|ObjectMapper' -g '!**/build/**' || true
echo
echo "== build config jackson occurrences =="
for f in $(git ls-files | rg '(^|/)(build\.gradle|build\.gradle\.kts|gradle\.properties|pom\.xml|settings\.gradle|settings\.gradle\.kts)$' || true); do
echo "--- $f"
rg -n -i 'jackson|spring-boot|version|platform|gradle-plugin|dependencies' "$f" || true
doneRepository: pottq577/flashhook
Length of output: 3122
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== FH_backend/gradle.properties =="
cat -n FH_backend/gradle.properties || true
echo
echo "== FH_backend/build.gradle =="
cat -n FH_backend/build.gradle || true
echo
echo "== FH_backend/settings.gradle =="
cat -n FH_backend/settings.gradle || true
echo
echo "== Jackson 2.x/3.x import occurrences in FH_backend source =="
rg -n --type=java 'import (?:tools\.jackson|com\.fasterxml\.jackson)|JacksonException|JsonProcessingException|ObjectMapper' FH_backend/src || true
echo
echo "== Build config jackson/spring boot occurrences =="
rg -n -i 'jackson|spring-boot|springframework|version|platform|gradle-plugin|dependencies' FH_backend/gradle.properties FH_backend/build.gradle FH_backend/settings.gradle || true
echo
echo "== Dependency graph references to Jackson packages in repo (excluding build dirs) =="
rg -n 'jackson-databind|jackson-core|com\.fasterxml\.jackson|tools\.jackson' FH_backend/src -g '!**/build/**' || trueRepository: pottq577/flashhook
Length of output: 7120
대부분 확인이 필요하지 않으나, annotation 쿼리를 Jackson 3.x로 맞추세요.
MockConfig.java의 JSON 설정 어노테이션 (@Jacksonized, @JsonProperty, @JsonCreator)은 8~9행의 Jackson 3.x 의존성과 달리 Jackson 2.x 좌표로 남아 있어, 이 부분을 tools.jackson.annotation.*로 수정해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@FH_backend/src/main/java/com/flashhook/domain/webhook/util/WebhookPayloadProcessor.java`
around lines 8 - 9, Update the JSON configuration annotations in
MockConfig.java—@Jacksonized, `@JsonProperty`, and `@JsonCreator`—to use the Jackson
3.x tools.jackson.annotation package, matching the ObjectMapper and
JacksonException imports shown here; leave unrelated Jackson usage unchanged.
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
FH_backend/src/main/java/com/flashhook/domain/webhook/service/LogCapEnforcer.java (1)
96-108: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
findAllAndRemove에 projection 누락 — 과거 지적사항이 재발.과거 리뷰에서
removeQuery에_id/bodySizeprojection을 적용하라는 지적이 있었고 "Addressed"로 표기되었으나, 현재 코드에는 projection이 없어 삭제 대상 문서 전체(body 포함)가 메모리에 적재됩니다.oldLogs에서 이미bodySize를 알고 있으므로 동일한 projection을 적용해도removedSize계산(L113-116)에는 영향이 없습니다..superpowers/plans/2026-07-26-pr158-coderabbit-fixes.mdTask 4 Step 2에도 이 항목이 아직 미완료로 남아 있어 실제 미해결 상태로 확인됩니다.♻️ projection 추가 제안
Query removeQuery = new Query( new Criteria().andOperator( Criteria.where("endpointId").is( endpoint.getEndpointId() ), Criteria.where("_id").in(idsToRemove) ) ); + removeQuery.fields().include("_id", "bodySize");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FH_backend/src/main/java/com/flashhook/domain/webhook/service/LogCapEnforcer.java` around lines 96 - 108, Update the removeQuery flow in LogCapEnforcer to apply a projection selecting only _id and bodySize before findAllAndRemove; reuse the existing oldLogs/bodySize data and preserve the removedSize calculation behavior while preventing full webhook bodies from being loaded.FH_backend/src/main/java/com/flashhook/domain/webhook/service/WebhookLogService.java (1)
167-187: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
@TransactionalEventListener(AFTER_COMMIT)가 이 이벤트에는 사실상 무의미할 수 있습니다.과거 리뷰에서 지적된
UpdateResult미확인 문제는 잘 해결됐습니다 (getMatchedCount()==0경고 추가). ``다만
SseDeliveryFailedEvent는 항상SseEmitterService.handleWebhookReceived(@Async)에서 발행됩니다. Spring 문서에 따르면 트랜잭션이 바인딩되지 않은 스레드에서 이벤트가 발행되면@TransactionalEventListener는 활성 트랜잭션이 없는 것으로 간주해fallbackExecution=true에 의해 즉시 실행됩니다(원본WebhookLog저장 트랜잭션의 커밋 여부와 무관). 즉, 이 어노테이션은 이 이벤트 경로에서 실질적으로 일반@EventListener와 동일하게 동작하며, 원본 문서가 아직 MongoDB에 커밋되기 전에updateFirst가 먼저 실행되면getMatchedCount()==0으로 조용히 무시되어sseDeliveryStatus/sseError가 영구히 기록되지 않는 경합 창이 남아 있습니다.
WebhookService의 저장/이벤트 발행 순서를 확인해 주시고, 근본적으로는 (a) 경고 로그만이 아니라 재시도/지연 처리, 또는 (b)WebhookReceivedEvent자체를AFTER_COMMIT로 발행해handleWebhookReceived가 커밋 이후에만 실행되도록 구조를 조정하는 방안을 검토해 주세요.Spring
TransactionalEventListener의 async-thread 트랜잭션 전파 동작에 대한 최신 문서/이슈를 참고해 정확성을 확인해 주시기 바랍니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FH_backend/src/main/java/com/flashhook/domain/webhook/service/WebhookLogService.java` around lines 167 - 187, Review WebhookService and SseEmitterService.handleWebhookReceived so WebhookReceivedEvent is processed only after the WebhookLog save transaction commits, rather than relying on fallbackExecution from the async thread. Update the event publication/listener flow accordingly, or add retry/delayed handling around handleSseDeliveryFailed for an unmatched logId so the SSE failure status is not permanently lost.
🧹 Nitpick comments (3)
.superpowers/plans/architecture-refactor.md (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win작업 공간 경로가 특정 로컬 사용자 환경에 하드코딩됨.
2026-07-26-pr158-coderabbit-fixes.md와 동일하게 절대 경로가 하드코딩되어 있어 이식성이 떨어지고 로컬 사용자명이 노출됩니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.superpowers/plans/architecture-refactor.md at line 4, Remove the hardcoded user-specific absolute workspace path from the architecture refactor plan, matching the portable workspace-path treatment used in 2026-07-26-pr158-coderabbit-fixes.md..superpowers/plans/2026-07-26-pr158-coderabbit-fixes.md (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win작업 공간 경로가 특정 로컬 사용자 환경에 하드코딩됨.
/home/hyun2y00/01_Portfolio/...같은 절대 경로는 다른 개발자나 CI 환경에서는 유효하지 않으며, 로컬 사용자명이 문서에 그대로 노출됩니다. 상대 경로 또는 플레이스홀더로 대체하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.superpowers/plans/2026-07-26-pr158-coderabbit-fixes.md at line 13, Replace the hardcoded user-specific absolute workspace path in the plan’s “Target workspace” entry with a repository-relative path or a clearly documented generic placeholder, without exposing local filesystem details.FH_backend/src/test/java/com/flashhook/domain/webhook/service/WebhookLogServiceTest.java (1)
3-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winimport 블록이 통째로 중복 선언됨.
ExtendWith,ArgumentCaptor,InjectMocks,Mock,MockitoExtension,DataAccessException,MongoTemplate,Query,Update,assertThat,any,eq,verify,when등이 각각 두 번씩 import되어 있습니다(L3-6 vs L37-40, L14 vs L25, L16-22 vs L29-35, L23 vs L36). 동일 타입의 중복 import는 컴파일 에러는 아니지만 병합 실수로 보이며 가독성을 해칩니다. 정리를 권장합니다.♻️ 중복 import 제거 제안
-import org.junit.jupiter.api.extension.ExtendWith; -import org.springframework.boot.test.system.CapturedOutput; -import org.springframework.boot.test.system.OutputCaptureExtension; -import org.mockito.ArgumentCaptor; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.dao.DataAccessException; -import org.springframework.data.mongodb.core.MongoTemplate; -import org.springframework.data.mongodb.core.query.Query; -import org.springframework.data.mongodb.core.query.Update; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when;(L25-40 블록 삭제, L26-27의
CapturedOutput/OutputCaptureExtensionimport만 상단에 유지)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FH_backend/src/test/java/com/flashhook/domain/webhook/service/WebhookLogServiceTest.java` around lines 3 - 40, WebhookLogServiceTest의 import 블록에서 중복 선언된 Mockito, Spring, AssertJ 및 JUnit import를 하나씩만 남기도록 정리하고, 테스트에서 사용하는 CapturedOutput과 OutputCaptureExtension import는 유지하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@FH_backend/src/main/java/com/flashhook/domain/webhook/service/LogCapEnforcer.java`:
- Around line 33-46: Update the asynchronous call in LogCapEnforcer around
enforceLogCap to pass an explicitly configured, dedicated Executor instead of
relying on CompletableFuture’s common pool. Prefer injecting a Spring-managed
TaskExecutor or Executor into LogCapEnforcer and use it with runAsync, while
preserving the existing exception logging behavior.
---
Duplicate comments:
In
`@FH_backend/src/main/java/com/flashhook/domain/webhook/service/LogCapEnforcer.java`:
- Around line 96-108: Update the removeQuery flow in LogCapEnforcer to apply a
projection selecting only _id and bodySize before findAllAndRemove; reuse the
existing oldLogs/bodySize data and preserve the removedSize calculation behavior
while preventing full webhook bodies from being loaded.
In
`@FH_backend/src/main/java/com/flashhook/domain/webhook/service/WebhookLogService.java`:
- Around line 167-187: Review WebhookService and
SseEmitterService.handleWebhookReceived so WebhookReceivedEvent is processed
only after the WebhookLog save transaction commits, rather than relying on
fallbackExecution from the async thread. Update the event publication/listener
flow accordingly, or add retry/delayed handling around handleSseDeliveryFailed
for an unmatched logId so the SSE failure status is not permanently lost.
---
Nitpick comments:
In @.superpowers/plans/2026-07-26-pr158-coderabbit-fixes.md:
- Line 13: Replace the hardcoded user-specific absolute workspace path in the
plan’s “Target workspace” entry with a repository-relative path or a clearly
documented generic placeholder, without exposing local filesystem details.
In @.superpowers/plans/architecture-refactor.md:
- Line 4: Remove the hardcoded user-specific absolute workspace path from the
architecture refactor plan, matching the portable workspace-path treatment used
in 2026-07-26-pr158-coderabbit-fixes.md.
In
`@FH_backend/src/test/java/com/flashhook/domain/webhook/service/WebhookLogServiceTest.java`:
- Around line 3-40: WebhookLogServiceTest의 import 블록에서 중복 선언된 Mockito, Spring,
AssertJ 및 JUnit import를 하나씩만 남기도록 정리하고, 테스트에서 사용하는 CapturedOutput과
OutputCaptureExtension import는 유지하세요.
🪄 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 Plus
Run ID: e7cf9a1d-753a-4ef4-8438-95a1c390ccaf
📒 Files selected for processing (14)
.gitignore.superpowers/plans/2026-07-26-pr158-coderabbit-fixes.md.superpowers/plans/architecture-refactor.mdFH_backend/src/main/java/com/flashhook/domain/webhook/event/SseDeliveryFailedEvent.javaFH_backend/src/main/java/com/flashhook/domain/webhook/service/LogCapEnforcer.javaFH_backend/src/main/java/com/flashhook/domain/webhook/service/MockResponseScheduler.javaFH_backend/src/main/java/com/flashhook/domain/webhook/service/SseEmitterService.javaFH_backend/src/main/java/com/flashhook/domain/webhook/service/WebhookLogService.javaFH_backend/src/main/java/com/flashhook/domain/webhook/service/WebhookService.javaFH_backend/src/main/java/com/flashhook/domain/webhook/util/HttpHeaderSanitizer.javaFH_backend/src/main/java/com/flashhook/domain/webhook/util/WebhookPayloadProcessor.javaFH_backend/src/test/java/com/flashhook/domain/webhook/service/SseEmitterServiceTest.javaFH_backend/src/test/java/com/flashhook/domain/webhook/service/WebhookLogServiceTest.javaFH_backend/src/test/java/com/flashhook/domain/webhook/util/HttpHeaderSanitizerTest.java
Comment on lines
+33
to
+46
| Endpoint updatedEndpoint = mongoTemplate.findAndModify( | ||
| query, | ||
| update, | ||
| FindAndModifyOptions.options().returnNew(true), | ||
| Endpoint.class | ||
| ); | ||
|
|
||
| if (updatedEndpoint != null) { | ||
| java.util.concurrent.CompletableFuture.runAsync(() -> enforceLogCap(updatedEndpoint)) | ||
| .exceptionally(ex -> { | ||
| log.error("Failed to enforce log cap asynchronously for endpointId={}", endpointId, ex); | ||
| return null; | ||
| }); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
전용 스레드풀 없이 공용 ForkJoinPool에서 블로킹 Mongo I/O 실행.
CompletableFuture.runAsync(() -> enforceLogCap(updatedEndpoint))는 Executor를 지정하지 않아 ForkJoinPool.commonPool()에서 실행됩니다. enforceLogCap은 반복적으로 find/findAllAndRemove/updateFirst 같은 블로킹 Mongo 호출을 수행하므로, 트래픽이 몰리면 JVM 전역에서 공유되는 commonPool(병렬 스트림 등에도 사용됨)의 스레드를 고갈시켜 애플리케이션 전반의 성능을 저하시킬 수 있습니다. .superpowers/plans/2026-07-26-pr158-coderabbit-fixes.md Task 4 Step 1에도 이 부분을 비동기화(별도 스레드)해야 한다는 항목이 아직 미완료로 남아 있습니다. 전용 Executor를 명시적으로 전달하는 것을 권장합니다.
♻️ 전용 Executor 사용 제안
+ private final java.util.concurrent.Executor logCapExecutor =
+ java.util.concurrent.Executors.newSingleThreadExecutor();
+
public void updateCountersAndEnforceCap(String endpointId, long bodySize) {
...
if (updatedEndpoint != null) {
- java.util.concurrent.CompletableFuture.runAsync(() -> enforceLogCap(updatedEndpoint))
+ java.util.concurrent.CompletableFuture.runAsync(() -> enforceLogCap(updatedEndpoint), logCapExecutor)
.exceptionally(ex -> {
log.error("Failed to enforce log cap asynchronously for endpointId={}", endpointId, ex);
return null;
});
}
}(실제 서비스에서는 Spring이 관리하는 TaskExecutor 빈을 주입받아 사용하는 편이 자원 관리 측면에서 더 안전합니다.)
📝 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
| Endpoint updatedEndpoint = mongoTemplate.findAndModify( | |
| query, | |
| update, | |
| FindAndModifyOptions.options().returnNew(true), | |
| Endpoint.class | |
| ); | |
| if (updatedEndpoint != null) { | |
| java.util.concurrent.CompletableFuture.runAsync(() -> enforceLogCap(updatedEndpoint)) | |
| .exceptionally(ex -> { | |
| log.error("Failed to enforce log cap asynchronously for endpointId={}", endpointId, ex); | |
| return null; | |
| }); | |
| } | |
| private final java.util.concurrent.Executor logCapExecutor = | |
| java.util.concurrent.Executors.newSingleThreadExecutor(); | |
| Endpoint updatedEndpoint = mongoTemplate.findAndModify( | |
| query, | |
| update, | |
| FindAndModifyOptions.options().returnNew(true), | |
| Endpoint.class | |
| ); | |
| if (updatedEndpoint != null) { | |
| java.util.concurrent.CompletableFuture.runAsync(() -> enforceLogCap(updatedEndpoint), logCapExecutor) | |
| .exceptionally(ex -> { | |
| log.error("Failed to enforce log cap asynchronously for endpointId={}", endpointId, ex); | |
| return null; | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@FH_backend/src/main/java/com/flashhook/domain/webhook/service/LogCapEnforcer.java`
around lines 33 - 46, Update the asynchronous call in LogCapEnforcer around
enforceLogCap to pass an explicitly configured, dedicated Executor instead of
relying on CompletableFuture’s common pool. Prefer injecting a Spring-managed
TaskExecutor or Executor into LogCapEnforcer and use it with runAsync, while
preserving the existing exception logging behavior.
This branch was successfully deployed
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.
Summary by CodeRabbit
content-type은 일반 텍스트로 폴백합니다.