Conversation
- 기존 PreferWebp 리졸버·S3 HEAD·ClubImageUrlPersistenceService 제거 - 변환 완료 웹훅 수신 후 배치로 Club logo/cover/feedImages 갱신 - ClubRepository 이미지 URL 기준 조회 메서드 추가, ErrorCode WEBHOOK_INVALID_REQUEST 추가 Co-authored-by: Cursor <cursoragent@cursor.com>
…feature/#1133-image-convert-MOA-593
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
웹훅 컨트롤러 backend/src/main/java/moadong/media/webhook/ImageConversionCompletedWebhookController.java, backend/src/main/java/moadong/media/webhook/ConversionBatchAdminController.java |
이미지 변환 완료 웹훅 수신용 공개 엔드포인트(/api/webhook/conversion-batch) 및 관리자용 배치/마이그레이션 엔드포인트(/api/admin/conversion-batch, /api/admin/conversion-batch/webp-migrate) 추가. 이벤트 검증 및 서비스 위임 로직 포함. |
웹훅/마이그레이션 서비스 backend/src/main/java/moadong/media/webhook/ImageConversionCompletedWebhookService.java, backend/src/main/java/moadong/media/webhook/WebpMigrationService.java |
이미지 키 정규화, 전체 URL 생성, 클럽 조회·업데이트(logo/cover/feedImages 교체) 로직과 S3 존재 확인을 통한 WebP 마이그레이션 흐름을 구현. WebpMigrationService는 전체 클럽 수집·검증·업데이트 집계 반환. |
DTO / 레코드 backend/src/main/java/moadong/media/webhook/dto/ImageConversionCompletedRequest.java, backend/src/main/java/moadong/media/webhook/dto/ImageEntry.java, backend/src/main/java/moadong/media/webhook/dto/WebpMigrationResult.java |
웹훅 페이로드 및 마이그레이션 결과를 위한 검증 어노테이션이 포함된 레코드 추가. |
클럽 저장소 및 검색/프로필 backend/src/main/java/moadong/club/repository/ClubRepository.java, backend/src/main/java/moadong/club/service/ClubSearchService.java, backend/src/main/java/moadong/club/service/ClubProfileService.java, backend/src/main/java/moadong/club/payload/dto/ClubDetailedResult.java |
클럽 이미지 필드를 대상으로 하는 새로운 리포지토리 쿼리 추가. ClubSearchService에 결과 재구성(map) 단계 추가. ClubProfileService에서 불필요한 의존성 제거 및 소소한 포맷 정리. ClubDetailedResult.of에 공백 라인 포맷 변경. |
에러 코드 및 설정 backend/src/main/java/moadong/global/exception/ErrorCode.java, backend/src/main/java/moadong/global/config/AsyncConfig.java |
웹훅 검증용 에러 코드 WEBHOOK_INVALID_REQUEST 추가 및 AsyncConfig 포맷 정리. |
관리 UI (dev) backend/src/main/resources/static/dev/index.html |
관리자용 이미지 변환 배치 및 WebP 마이그레이션 UI(네비게이션 항목, 입력 폼, JS 핸들러) 추가(동일 블록 중복 삽입 포함). |
단위 테스트 backend/src/test/java/moadong/media/webhook/..., backend/src/test/java/moadong/unit/club/ClubProfileServiceTest.java |
컨트롤러·서비스·마이그레이션 로직을 검증하는 단위 테스트 추가 및 ClubProfileService 테스트의 목 설정 일부 변경. |
Sequence Diagram
sequenceDiagram
participant Client as 이미지 변환<br/>서버
participant WebhookCtrl as Webhook<br/>Controller
participant WebhookSvc as ImageConversion<br/>WebhookService
participant Repo as ClubRepository
Client->>WebhookCtrl: POST /api/webhook/conversion-batch<br/>(ImageConversionCompletedRequest)
WebhookCtrl->>WebhookCtrl: event 검증 (batch.completed 등)
WebhookCtrl->>WebhookSvc: processImageConversionCompleted(request)
loop 각 이미지 매핑
WebhookSvc->>WebhookSvc: normalizeKey(source/destination)<br/>fullUrl 생성
WebhookSvc->>Repo: findByClubRecruitmentInformation_...(fullOldUrl)
Repo-->>WebhookSvc: matching Club 목록
WebhookSvc->>Repo: save(club) (logo/cover/feedImages 갱신)
end
WebhookSvc-->>WebhookCtrl: 처리 완료
WebhookCtrl-->>Client: 200 OK ("ok")
sequenceDiagram
participant Admin as 관리자
participant AdminCtrl as ConversionBatch<br/>AdminController
participant WebpSvc as WebpMigrationService
participant Repo as ClubRepository
participant S3 as S3/R2
participant WebhookSvc as ImageConversion<br/>WebhookService
Admin->>AdminCtrl: POST /api/admin/conversion-batch/webp-migrate
AdminCtrl->>WebpSvc: migrateAllClubsToWebp()
WebpSvc->>Repo: findAll()
Repo-->>WebpSvc: 모든 Club 반환
loop 수집된 각 이미지 URL
WebpSvc->>WebpSvc: .webp 여부 확인 / 키 추출 / toWebpKey
WebpSvc->>S3: headObject(webpKey)
alt WebP 존재
S3-->>WebpSvc: 200 OK
WebpSvc->>WebhookSvc: updateClubsForImageReplacement(oldUrl, newUrl)
WebhookSvc->>Repo: 클럽 조회 및 업데이트
WebpSvc->>WebpSvc: updatedCount++
else WebP 미존재
S3-->>WebpSvc: NoSuchKeyException
WebpSvc->>WebpSvc: skippedCount++
end
end
WebpSvc-->>AdminCtrl: WebpMigrationResult(updatedCount, skippedCount)
AdminCtrl-->>Admin: 200 OK { message, data }
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
- [BE] Release #1166: 이미지 마이그레이션·웹훅 관련 모듈(마이그레이션 로직, 컨트롤러, 서비스 등)과 중복 또는 유사한 변경이 있음.
- [Release] BE #1169: ClubDetailedResult.of 및 이미지 URL 처리 관련 변경을 포함하여 본 PR의 일부 파일 경로와 겹침.
- [Release] BE v1.0.6 배포 #675: ClubProfileService 및 ClubDetailedResult 사용 흐름을 수정한 PR로, ClubSearchRepository 의존성 변경과 연관성이 높음.
Suggested labels
✨ Feature, 📬 API, 💾 BE, ✅ Test
Suggested reviewers
- seongwon030
- lepitaaar
- PororoAndFriends
🚥 Pre-merge checks | ✅ 2 | ❌ 4
❌ Failed checks (3 warnings, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Out of Scope Changes check | ClubDetailedResult의 포맷팅, ClubProfileService의 의존성 제거, ClubSearchService의 객체 재구성 등 일부 변경사항이 이미지 컨버팅 기능과 직접적인 관련성이 낮습니다. | ClubDetailedResult 포맷팅, ClubProfileService 리팩토링, ClubSearchService 맵 추가는 별도 PR로 분리하거나 해당 변경의 필요성을 문서화하세요. | |
| Docstring Coverage | Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Merge Conflict Detection | |||
| Title check | ❓ Inconclusive | PR 제목이 너무 모호하고 구체적인 정보를 전달하지 못합니다. '웹p 변환 2트'는 실제 변경 사항의 핵심을 명확하게 설명하지 않습니다. | PR 제목을 더 구체적으로 변경하세요. 예를 들어 '[feature] Webhook 컨트롤러 추가 및 WebP 마이그레이션 서비스 구현' 같이 주요 변경 사항을 명확히 반영하는 제목을 사용하세요. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check | ✅ Passed | PR은 r2 스토리지 이미지 컨버팅 서버 구축을 위한 웹훅 처리, 이미지 변환 완료 처리, WebP 마이그레이션 기능 등을 구현했으며 이는 MOA-593의 목표와 일치합니다. |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feature/#1133-image-convert-MOA-593
⚔️ Resolve merge conflicts (beta)
- Auto-commit resolved conflicts to branch
feature/#1133-image-convert-MOA-593 - Create stacked PR with resolved conflicts
- Post resolved changes as copyable diffs in a comment
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 help to get the list of available commands and usage tips.
Test Results85 tests 85 ✅ 15s ⏱️ Results for commit 4e3af32. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@backend/src/main/java/moadong/media/webhook/dto/ImageConversionCompletedRequest.java`:
- Around line 7-12: The record ImageConversionCompletedRequest exposes non-camel
accessor names (processed_count, failed_count); rename the record components to
processedCount and failedCount and annotate them with
`@JsonProperty`("processed_count") and `@JsonProperty`("failed_count") respectively
so JSON payloads still bind to the snake_case fields while Java API uses
camelCase (affects the record declaration and accessors:
ImageConversionCompletedRequest.processedCount() and .failedCount()).
In
`@backend/src/main/java/moadong/media/webhook/ImageConversionCompletedWebhookController.java`:
- Around line 17-35: Add authentication for the webhook by verifying a shared
secret/HMAC signature before processing: in
ImageConversionCompletedWebhookController.handleImageConversionCompleted read
the signature header (e.g. "X-Signature") or a shared token header/query,
retrieve the secret from configuration, compute/verify the HMAC over the request
payload (or compare token) and if verification fails throw
RestApiException(ErrorCode.WEBHOOK_UNAUTHORIZED); alternatively extract the
verification into a new WebhookAuthService.verifySignature(byte[] payload,
String signature) method and call it at the start of
handleImageConversionCompleted before invoking
imageConversionCompletedWebhookService.processImageConversionCompleted. Ensure
the secret is injected from properties/env and log/return a proper unauthorized
error on failure.
In
`@backend/src/main/java/moadong/media/webhook/ImageConversionCompletedWebhookService.java`:
- Around line 98-116: normalizeKey currently only checks for literal ".." and
misses URL-encoded variants; fix it by URL-decoding the incoming key first
(e.g., via URLDecoder.decode(key, StandardCharsets.UTF_8)), then perform the
existing null/empty/trim logic on the decoded value and reject if it contains
".." (case-sensitive) or any percent-encoded traversal pattern by checking for
"%2e%2e" case-insensitively; keep trimming of leading/trailing slashes and
return null for any invalid/empty result. Ensure decode exceptions are handled
(treat as invalid -> return null) and reference the normalizeKey method name
when making changes.
In `@backend/src/main/resources/static/dev/index.html`:
- Around line 717-727: The fetch handler used by btnWebpMigrate incorrectly
calls await res.json() before checking res.status (same bug as earlier); change
the logic in the btnWebpMigrate fetch block to check res.status (or res.ok)
immediately after the fetch and before parsing the body, handling 403 by showing
the toast and returning, and only call await res.json() for responses expected
to carry JSON (or use res.clone() if you must inspect body after status).
- Around line 688-695: The code calls await res.json() before checking
res.status, which throws if the 403 response isn't JSON; change the flow in the
handler that uses res, data, and banner to first inspect res.status (e.g., if
(res.status === 403) { set banner warn and return; }) and only call await
res.json() when the status indicates a JSON payload is expected; apply the same
pattern used in btnLoadClubs to avoid parsing non-JSON error pages.
🧹 Nitpick comments (12)
backend/src/main/java/moadong/club/service/ClubSearchService.java (1)
56-65: 불필요한 identity.map()단계를 제거하세요.이
.map()호출은ClubSearchResult의 모든 필드를 동일한 값으로 재구성하고 있어 실질적으로 no-op입니다.ClubSearchResult는 불변 record이므로 방어적 복사도 불필요하며, 리스트 요소마다 새 객체를 할당하는 오버헤드만 추가됩니다.♻️ 제거 제안
.thenComparing(ClubSearchResult::name) ) - .map(r -> new ClubSearchResult( - r.id(), - r.name(), - r.logo(), - r.tags(), - r.state(), - r.category(), - r.division(), - r.introduction(), - r.recruitmentStatus())) .collect(Collectors.toList());backend/src/main/java/moadong/club/repository/ClubRepository.java (1)
22-23: 파생 쿼리 메서드 이름이 지나치게 길어 가독성과 유지보수성이 떨어집니다.
@Query어노테이션을 사용하여 동일한 로직을 훨씬 명확하게 표현할 수 있습니다.♻️ `@Query` 사용 제안
- List<Club> findByClubRecruitmentInformation_LogoOrClubRecruitmentInformation_CoverOrClubRecruitmentInformation_FeedImagesContaining( - String logo, String cover, String feedImageUrl); + `@Query`("{ '$or': [ " + + "{ 'clubRecruitmentInformation.logo': ?0 }, " + + "{ 'clubRecruitmentInformation.cover': ?1 }, " + + "{ 'clubRecruitmentInformation.feedImages': ?2 } ] }") + List<Club> findByImageUrl(String logo, String cover, String feedImageUrl);호출부에서 동일한 URL을 세 파라미터 모두에 전달하는 패턴이라면, 단일 파라미터로 간소화하는 것도 고려해 보세요.
backend/src/test/java/moadong/media/webhook/ImageConversionCompletedWebhookServiceTest.java (3)
34-38:ReflectionTestUtils.invokeMethod로@PostConstruct메서드를 수동 호출하는 패턴은 취약합니다.
init()메서드가private이므로 리플렉션에 의존하고 있습니다. 메서드 이름이 변경되면 테스트가 런타임에 실패합니다.viewEndpoint를 생성자 파라미터로 주입하거나init()의 가시성을package-private으로 변경하면 리플렉션 없이 테스트할 수 있습니다.
40-74: 로고만 테스트하고 있으며 cover/feedImages 갱신 경로에 대한 테스트가 없습니다.
updateClubImageUrls는 logo, cover, feedImages 세 필드를 모두 치환하지만, 이 테스트에서는 logo만 검증합니다. cover와 feedImages 경로도 테스트를 추가하면 회귀 방지에 도움이 됩니다.
86-88:org.mockito.Mockito.never()를 직접 정규화 참조하고 있습니다.이미 다른 Mockito 정적 메서드는 import하고 있으므로,
never()도 static import로 통일하면 가독성이 향상됩니다.제안
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.never;그 후
org.mockito.Mockito.never()→never()로 교체하세요.Also applies to: 101-103
backend/src/test/java/moadong/media/webhook/ConversionBatchAdminControllerTest.java (1)
43-54:processBatchCompletedForAllClubs테스트에서 서비스 호출 여부를 검증하지 않습니다.HTTP 200 상태 코드만 확인하고 있으며,
imageConversionCompletedWebhookService.processImageConversionCompleted(request)가 실제로 호출되었는지verify하지 않습니다. 웹훅 컨트롤러 테스트(ImageConversionCompletedWebhookControllerTest)에서는 이 검증이 포함되어 있으므로 일관성을 맞추는 것이 좋습니다.제안
ResponseEntity<?> response = conversionBatchAdminController.processBatchCompletedForAllClubs(request); assertEquals(200, response.getStatusCode().value()); + verify(imageConversionCompletedWebhookService).processImageConversionCompleted(request);
verifystatic import도 추가해야 합니다:import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.verify;backend/src/main/java/moadong/media/webhook/ConversionBatchAdminController.java (1)
25-41: 이벤트 검증 로직이ImageConversionCompletedWebhookController와 중복됩니다.
BATCH_COMPLETED_EVENT상수와if (!BATCH_COMPLETED_EVENT.equals(request.event()))검증이 두 컨트롤러에서 동일하게 반복됩니다. 이벤트 검증을 서비스 레이어나ImageConversionCompletedRequestDTO의 커스텀 validation으로 통합하면 유지보수성이 향상됩니다.backend/src/main/java/moadong/media/webhook/ImageConversionCompletedWebhookService.java (1)
40-48: 클럽별 개별save()로 인한 부분 갱신 가능성 및 성능 문제.
updateClubsForImageReplacement에서 매칭되는 클럽을 루프로 돌면서 각각save()를 호출합니다. 이미지 하나에 대해 여러 클럽이 매칭될 경우:
- 중간에 예외가 발생하면 일부 클럽만 갱신된 상태로 남습니다.
processImageConversionCompleted에서 여러 이미지를 처리하면 DB 라운드트립이 (이미지 수 × 매칭 클럽 수)만큼 발생합니다.배치 특성상 즉각적인 문제는 아니지만, 클럽 수가 많아지면
saveAll()로 일괄 저장하는 것을 고려하세요.제안: saveAll() 사용
public void updateClubsForImageReplacement(String fullUrlOld, String fullUrlNew) { List<Club> clubs = clubRepository .findByClubRecruitmentInformation_LogoOrClubRecruitmentInformation_CoverOrClubRecruitmentInformation_FeedImagesContaining( fullUrlOld, fullUrlOld, fullUrlOld); for (Club club : clubs) { updateClubImageUrls(club, fullUrlOld, fullUrlNew); - clubRepository.save(club); } + clubRepository.saveAll(clubs); }backend/src/main/java/moadong/media/webhook/WebpMigrationService.java (4)
93-137:clubRepository.findAll()은 모든 클럽을 메모리에 로드합니다.클럽 수가 증가하면 메모리 부족이나 긴 GC 멈춤이 발생할 수 있습니다. 현재 규모에서는 동작하겠지만, 페이지네이션이나 스트림 기반 처리로 개선하는 것이 좋습니다.
39-45:init()메서드가ImageConversionCompletedWebhookService와 완전히 중복됩니다.두 서비스 모두
viewEndpoint검증 및 정규화(replaceAll("/+$", ""))를 동일하게 수행합니다. 공통 설정 컴포넌트나 유틸리티로 추출하면 중복을 줄이고 변경 시 한 곳만 수정하면 됩니다.
180-196: R2 HEAD 요청마다log.info를 출력하면 마이그레이션 시 로그 볼륨이 매우 커집니다.수백~수천 개 URL을 처리할 때 매 요청마다 info 로그를 남기면 로그 스토리지와 가독성에 부담이 됩니다. Line 186의 요청 로그와 Line 190, 193의 실패 로그를
log.debug로 낮추는 것을 권장합니다.제안
- log.info("R2 HEAD request: bucket={}, key={}", bucket, destKey); + log.debug("R2 HEAD request: bucket={}, key={}", bucket, destKey); s3Client.headObject(request); return true; } catch (NoSuchKeyException e) { - log.info("R2 HEAD 404 (no such key): bucket={}, key={}", bucket, destKey); + log.debug("R2 HEAD 404 (no such key): bucket={}, key={}", bucket, destKey); return false; } catch (S3Exception e) { - log.info("R2 HEAD failed: bucket={}, key={}, error={}", bucket, destKey, e.getMessage()); + log.warn("R2 HEAD failed: bucket={}, key={}, error={}", bucket, destKey, e.getMessage()); return false; }
S3Exception(NoSuchKeyException 제외)은 인프라 문제일 수 있으므로warn이 적절합니다.
51-91:migrateAllClubsToWebp에서 R2 HEAD 요청이 순차적으로 수행됩니다.모든 URL에 대해 하나씩 HEAD 요청을 보내므로, URL이 많으면 전체 마이그레이션 시간이 크게 늘어날 수 있습니다. 현재 운영 규모에서 문제가 없다면 괜찮지만, 향후 병렬 처리(
CompletableFuture, 가상 스레드 등)를 고려할 수 있습니다.
backend/src/main/java/moadong/media/webhook/dto/ImageConversionCompletedRequest.java
Show resolved
Hide resolved
backend/src/main/java/moadong/media/webhook/ImageConversionCompletedWebhookController.java
Show resolved
Hide resolved
| private String normalizeKey(String key) { | ||
| if (key == null) { | ||
| return null; | ||
| } | ||
| String k = key.trim(); | ||
| if (k.isEmpty()) { | ||
| return null; | ||
| } | ||
| if (k.contains("..")) { | ||
| return null; | ||
| } | ||
| while (k.startsWith("/")) { | ||
| k = k.substring(1); | ||
| } | ||
| while (k.endsWith("/")) { | ||
| k = k.substring(0, k.length() - 1); | ||
| } | ||
| return k.isEmpty() ? null : k; | ||
| } |
There was a problem hiding this comment.
normalizeKey의 path traversal 방어가 URL-encoded 패턴(%2e%2e)을 처리하지 않습니다.
".." 문자열 포함 여부만 확인하므로, %2e%2e나 %2E%2E와 같은 인코딩된 패턴은 통과할 수 있습니다. 외부 웹훅에서 입력되는 값이므로, URL 디코딩 후 검사하거나 인코딩된 변형도 함께 차단하는 것이 안전합니다.
제안: URL 디코딩 후 검증
private String normalizeKey(String key) {
if (key == null) {
return null;
}
- String k = key.trim();
+ String k;
+ try {
+ k = java.net.URLDecoder.decode(key.trim(), java.nio.charset.StandardCharsets.UTF_8);
+ } catch (Exception e) {
+ return null;
+ }
if (k.isEmpty()) {
return null;
}
if (k.contains("..")) {
return null;
}🤖 Prompt for AI Agents
In
`@backend/src/main/java/moadong/media/webhook/ImageConversionCompletedWebhookService.java`
around lines 98 - 116, normalizeKey currently only checks for literal ".." and
misses URL-encoded variants; fix it by URL-decoding the incoming key first
(e.g., via URLDecoder.decode(key, StandardCharsets.UTF_8)), then perform the
existing null/empty/trim logic on the decoded value and reject if it contains
".." (case-sensitive) or any percent-encoded traversal pattern by checking for
"%2e%2e" case-insensitively; keep trimming of leading/trailing slashes and
return null for any invalid/empty result. Ensure decode exceptions are handled
(treat as invalid -> return null) and reference the normalizeKey method name
when making changes.
lepitaaar
left a comment
There was a problem hiding this comment.
웹훅을 이용한 이벤트 전달형식으로 변경하셨군요 수고하셨습니다
#️⃣연관된 이슈
#1133
📝작업 내용
webp의 근본적인 로직을 개선함.
웹훅을 받아 DB에 저장된 jpg, png 등등을 가진 url을 webp로 바꿔주는 엔드포인트 추가.
웹훅을 쏘는 주체는 이미지 컨버터
참고) https://github.com/Zepelown/image-converting-server
매우 간단하고 깔끔하고 딱히 문제도 없을 거라고 봄
개발자 페이지에도 마이그레이션 기능있음
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
새 기능
개선
테스트