Skip to content

style: Lombok 생성자·@Slf4j 도입 + 전 소스 주석 Javadoc 전환 - #18

Merged
m-a-king merged 4 commits into
mainfrom
style/lombok-javadoc-slf4j
Aug 3, 2026
Merged

style: Lombok 생성자·@Slf4j 도입 + 전 소스 주석 Javadoc 전환#18
m-a-king merged 4 commits into
mainfrom
style/lombok-javadoc-slf4j

Conversation

@m-a-king

@m-a-king m-a-king commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

무엇을

세 갈래의 스타일 정리를 한 브랜치에 담았다. 전부 동작 등가이며, 산출물은 규약(CLAUDE.md)과 결정 기록(docs/style-decisions.md)에 남겼다.

1. Lombok — 보일러플레이트 표적 채택 (12개 클래스)

순수 필드-대입 생성자만 @RequiredArgsConstructor 로 대체했다. @Qualifier 는 필드로 옮기고 lombok.configcopyableAnnotations 로 생성자 파라미터에 복사되게 했다.

조립 로직이 있는 생성자 4개는 손으로 유지했다 — HttpPageFetcher·HttpHeadlessRenderer(주입받은 resolver 로 InternalHostGuard 를 직접 조립, IP-pin 계약), GeminiHttpClient(RestClient 빌드), RequestScopedDnsResolver(편의 생성자 2개).

값 객체는 여전히 record 이고 @Builder·@Setter·@Data·@Getter 는 미채택이다.

2. 로거 — @Slf4j (12개 클래스)

명시적 private static final Logger log = LoggerFactory.getLogger(X.class) 를 전부 애노테이션으로 바꿨다. 로그 출력은 바뀌지 않는다 — 바이트코드를 확인했다:

static {}:  ldc class .../HttpPageFetcher
            invokestatic LoggerFactory.getLogger:(Class)Logger
            putstatic    Field log:Lorg/slf4j/Logger;

수동 선언과 동일(필드 수정자·로거 이름 모두). 전 대상이 필드명 log, 인자가 자기 클래스여서 예외 케이스가 없었다.

3. 주석 → Javadoc (97개 파일)

선언부 주석을 Javadoc 으로 옮기고, "숙련 Java 독자가 이 주석 없이 놓칠 정보가 있는가" 하나를 기준으로 정리했다.

  • 라인주석 827 → 166 (-80%), Javadoc 5 → 196블록
  • 남은 라인주석은 전부 본문의 "왜"다 — 실제 사이트 지식(유니클로 | UNIQLO KR 꼬리표와 lastIndexOf 인 이유, 29cm contentUrl, 카카오 charset 누락), Java 함정(intValue wrap vs intValueExact, Jackson 3 FAIL_ON_NULL_FOR_PRIMITIVES), 보안·계약 근거(IP pin 은 같은 resolver 로만 성립, 매 hop SSRF 재검증)
  • 지운 것: 시그니처 재진술 @param, 클래스 Javadoc 과의 중복, 흐름 나레이션, SSOT 위반(호출자 repo 의 수치·코드 기본값·다른 클래스가 정본인 분류)

./gradlew javadoc 을 고쳤다. 이 브랜치 이전엔 error 1 로 실패하던 상태였다(GeminiProperties Retry Javadoc 본문 줄머리의 @DefaultValue → unknown block tag). 이제 error 0.

origin/main 병합

작업 중 origin/main 이 11커밋 앞서갔고(#3~#17), 그중 #17 이 같은 파일들을 건드려 27 hunk 가 충돌했다. 원격의 의미 + 이 브랜치의 스타일 로 해결했고, 원격 산출물 무손실을 실측 확인했다:

확인 항목 결과
ExtractionMethod enum · ObservationConfig 존재
ProductSnapshot.withOrigin · 편의 생성자 존재
ExtractionResponsefinalUrl·method 존재
maxRedirects 기본값 5 (원격 상향값 유지)
testing-principles import 유지

원격의 PIKI-Servercore 명칭 정리도 반영했고, 원격이 새로 들여온 ExtractionResponse 의 명시적 Logger 도 @Slf4j 규약에 맞췄다.

검증

  • ./gradlew test · ./gradlew javadoc 통과 (javadoc error 0)
  • 주석 전환 단계에서는 주석을 제거한 소스가 전후 동일한지 기계 비교하는 게이트를 걸어 105개 파일 전부 "코드 변경 없음" 을 확인했다(과거 같은 작업에서 필드 선언이 함께 삭제돼 빌드가 깨진 적이 있어 도입)

후속 (이 PR 범위 밖)

주석을 쓰며 코드를 정독하다 드러난 이상 8건을 docs/style-decisions.md §4 에 기록만 해 뒀다. 심각한 셋은 헤드리스 경로의 redirect SSRF 비대칭, ProductImage.of(null) 이 422 대신 500, 이미지 경로가 ProductSnapshot.fromExtracted 정규화를 우회하는 것이다.

m-a-king added 4 commits July 11, 2026 01:11
- renderer(구 PIKI-HeadlessBrowser)가 파싱을 걷어내고 HTML 렌더러로 재편되면서 /render 응답에서 title/price 가 빠지고 verdict 가 OK|BLOCK|EMPTY|ERROR 로 바뀜. 기존 번역 규칙(BLOCK→차단, 그 외는 html 유무)은 verdict 세부 값에 결합돼 있지 않아 그대로 유효 — 안 쓰게 된 source 필드만 제거하고 주석·테스트·api-contract 서술을 신계약으로 갱신
- compress=true 요청 시 응답이 zstd raw 바이트(X-Encoding: zstd)로 옴 → 해제 분기를 요청 플래그가 아닌 응답 헤더 기준으로 구현. 구버전 renderer 는 compress 필드를 무시(pydantic 기본)하고 plain JSON 을 주므로 renderer/extractor 어느 쪽이 먼저 배포돼도 안전 — 기본 on, product.extract.headless.compress 가 kill-switch
- 사전 해제는 ZstdDictionaries 신설(파일명=사전ID, renderer compress.py 의 DICT_ID 와 대칭). X-Zstd-Dict 빈값=plain 해제, 미보유 사전ID 는 롤아웃 규약(사전은 extractor 먼저 배포) 위반 신호라 일시 실패(HEADLESS_UPSTREAM)로 번역. zstd-dict-dir 오설정은 부팅 fail-fast(baseUrl 검증과 같은 결)
- zstd-jni(JNI) 로드가 Java 25 에서 native access 경고를 내(미래 JDK 는 차단, JEP 472) 테스트 jvmArgs 와 Dockerfile ENTRYPOINT 에 --enable-native-access=ALL-UNNAMED 명시
- 구 PIKI-HeadlessBrowser 가 TeamPiKi/renderer 로 재편되면서 CLAUDE.md 와 주석의 참조 명칭을 현행화 (주석만 바뀐 파일이라 신계약 대응 커밋과 분리)
충돌 27 hunk 를 '원격의 의미 + 이 브랜치의 스타일' 로 해결했다. 원격이 새로 들여온 사실은
전부 살리고(ExtractionMethod·withOrigin·finalUrl/method 근거·maxRedirects 상향 근거·verdict 설명),
주석 형식은 Javadoc 규약을 적용했다. 원격의 PIKI-Server → core 명칭 정리도 반영.

원격 산출물 무손실 확인: ExtractionMethod·ObservationConfig 파일 존재, withOrigin·finalUrl·method
필드, maxRedirects 기본값 5, testing-principles import 유지. 전체 테스트·javadoc 통과.

원격이 새로 들여온 ExtractionResponse 의 명시적 Logger 선언도 @slf4j 규약에 맞췄다.
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Important

Review skipped

Too many files!

This PR contains 103 files, which is 3 over the limit of 100.

To get a review, narrow the scope:
• coderabbit review --committed # exclude uncommitted changes
• coderabbit review --dir # limit to a subdirectory
• coderabbit review --base # compare against a closer base

Upgrade to a paid plan to raise the limit.

This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3a33e566-05ac-4b79-b6c4-26664f35de59

📥 Commits

Reviewing files that changed from the base of the PR and between 2ff1883 and 2e1b452.

📒 Files selected for processing (103)
  • CLAUDE.md
  • Dockerfile
  • build.gradle.kts
  • docs/api-contract.md
  • docs/style-decisions.md
  • lombok.config
  • src/main/java/com/depromeet/piki/extractor/api/ExtractionController.java
  • src/main/java/com/depromeet/piki/extractor/api/ExtractionExceptionHandler.java
  • src/main/java/com/depromeet/piki/extractor/api/ExtractionFailureResponse.java
  • src/main/java/com/depromeet/piki/extractor/api/ExtractionResponse.java
  • src/main/java/com/depromeet/piki/extractor/api/ImageExtractionRequest.java
  • src/main/java/com/depromeet/piki/extractor/api/LinkExtractionRequest.java
  • src/main/java/com/depromeet/piki/extractor/common/exception/ExtractionErrorCode.java
  • src/main/java/com/depromeet/piki/extractor/common/exception/ExtractionException.java
  • src/main/java/com/depromeet/piki/extractor/common/storage/ImageStorage.java
  • src/main/java/com/depromeet/piki/extractor/common/storage/ImageStorageException.java
  • src/main/java/com/depromeet/piki/extractor/common/storage/S3Config.java
  • src/main/java/com/depromeet/piki/extractor/common/storage/S3ImageStorage.java
  • src/main/java/com/depromeet/piki/extractor/common/storage/S3Properties.java
  • src/main/java/com/depromeet/piki/extractor/common/storage/StoredImage.java
  • src/main/java/com/depromeet/piki/extractor/domain/CurrencyCode.java
  • src/main/java/com/depromeet/piki/extractor/domain/ProductLink.java
  • src/main/java/com/depromeet/piki/extractor/domain/ProductLinkException.java
  • src/main/java/com/depromeet/piki/extractor/domain/ProductSnapshot.java
  • src/main/java/com/depromeet/piki/extractor/domain/ProductSnapshotException.java
  • src/main/java/com/depromeet/piki/extractor/extraction/DefaultProductLinkExtractor.java
  • src/main/java/com/depromeet/piki/extractor/extraction/FallbackProductLinkExtractor.java
  • src/main/java/com/depromeet/piki/extractor/extraction/GeminiHtmlExtractor.java
  • src/main/java/com/depromeet/piki/extractor/extraction/HeadlessExtractionProperties.java
  • src/main/java/com/depromeet/piki/extractor/extraction/HeadlessProductLinkExtractor.java
  • src/main/java/com/depromeet/piki/extractor/extraction/HtmlSnapshotPipeline.java
  • src/main/java/com/depromeet/piki/extractor/extraction/LinkExtractionStrategy.java
  • src/main/java/com/depromeet/piki/extractor/extraction/PageContent.java
  • src/main/java/com/depromeet/piki/extractor/extraction/PageFetcher.java
  • src/main/java/com/depromeet/piki/extractor/extraction/ProductLinkExtractor.java
  • src/main/java/com/depromeet/piki/extractor/extraction/gemini/GeminiApiException.java
  • src/main/java/com/depromeet/piki/extractor/extraction/gemini/GeminiClient.java
  • src/main/java/com/depromeet/piki/extractor/extraction/gemini/GeminiExtractionRequest.java
  • src/main/java/com/depromeet/piki/extractor/extraction/gemini/GeminiExtractionResult.java
  • src/main/java/com/depromeet/piki/extractor/extraction/gemini/GeminiGenerateContentResponse.java
  • src/main/java/com/depromeet/piki/extractor/extraction/gemini/GeminiHttpClient.java
  • src/main/java/com/depromeet/piki/extractor/extraction/gemini/GeminiProperties.java
  • src/main/java/com/depromeet/piki/extractor/extraction/gemini/GeminiRetry.java
  • src/main/java/com/depromeet/piki/extractor/extraction/headless/HeadlessRenderException.java
  • src/main/java/com/depromeet/piki/extractor/extraction/headless/HeadlessRenderHttpClientConfig.java
  • src/main/java/com/depromeet/piki/extractor/extraction/headless/HeadlessRenderRequest.java
  • src/main/java/com/depromeet/piki/extractor/extraction/headless/HeadlessRenderResponse.java
  • src/main/java/com/depromeet/piki/extractor/extraction/headless/HeadlessRenderer.java
  • src/main/java/com/depromeet/piki/extractor/extraction/headless/HttpHeadlessRenderer.java
  • src/main/java/com/depromeet/piki/extractor/extraction/headless/ZstdDictionaries.java
  • src/main/java/com/depromeet/piki/extractor/extraction/http/FetchProperties.java
  • src/main/java/com/depromeet/piki/extractor/extraction/http/HttpPageFetcher.java
  • src/main/java/com/depromeet/piki/extractor/extraction/http/InternalHostGuard.java
  • src/main/java/com/depromeet/piki/extractor/extraction/http/PageFetchException.java
  • src/main/java/com/depromeet/piki/extractor/extraction/http/PageFetchHttpClientConfig.java
  • src/main/java/com/depromeet/piki/extractor/extraction/http/RequestScopedDnsResolver.java
  • src/main/java/com/depromeet/piki/extractor/extraction/structured/StructuredDataExtractor.java
  • src/main/java/com/depromeet/piki/extractor/extraction/structured/StructuredExtraction.java
  • src/main/java/com/depromeet/piki/extractor/image/ImageCropper.java
  • src/main/java/com/depromeet/piki/extractor/image/ImageExtraction.java
  • src/main/java/com/depromeet/piki/extractor/image/ImageExtractionService.java
  • src/main/java/com/depromeet/piki/extractor/image/ProductImageExtractor.java
  • src/main/java/com/depromeet/piki/extractor/image/domain/BoundingBox.java
  • src/main/java/com/depromeet/piki/extractor/image/domain/ProductImage.java
  • src/main/java/com/depromeet/piki/extractor/image/domain/ProductImageException.java
  • src/main/java/com/depromeet/piki/extractor/image/gemini/GeminiImageRequest.java
  • src/main/java/com/depromeet/piki/extractor/image/gemini/GeminiImageResult.java
  • src/main/java/com/depromeet/piki/extractor/image/gemini/GeminiProductImageExtractor.java
  • src/test/java/com/depromeet/piki/extractor/api/ExtractionImageIntegrationTest.java
  • src/test/java/com/depromeet/piki/extractor/api/ExtractionLinkIntegrationTest.java
  • src/test/java/com/depromeet/piki/extractor/common/storage/ImageStorageExceptionTest.java
  • src/test/java/com/depromeet/piki/extractor/common/storage/S3ImageStorageTest.java
  • src/test/java/com/depromeet/piki/extractor/domain/ProductLinkTest.java
  • src/test/java/com/depromeet/piki/extractor/domain/ProductSnapshotTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/FallbackProductLinkExtractorTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/GeminiHtmlExtractorTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/HeadlessExtractionPropertiesTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/HeadlessProductLinkExtractorTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/HtmlSnapshotPipelineMetricTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/ProductLinkExtractE2ETest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/gemini/GeminiApiExceptionTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/gemini/GeminiHtmlExtractorE2ETest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/gemini/GeminiPropertiesTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/gemini/GeminiRetryTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/headless/HttpHeadlessRendererTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/headless/ZstdDictionariesTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/http/HttpPageFetcherCharsetTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/http/HttpPageFetcherRedirectE2ETest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/http/HttpPageFetcherRedirectTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/http/HttpPageFetcherServerErrorTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/http/InternalHostGuardTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/http/PageFetchExceptionTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/http/RequestScopedDnsResolverTest.java
  • src/test/java/com/depromeet/piki/extractor/extraction/structured/StructuredDataExtractorTest.java
  • src/test/java/com/depromeet/piki/extractor/image/ImageCropperTest.java
  • src/test/java/com/depromeet/piki/extractor/image/domain/ProductImageTest.java
  • src/test/java/com/depromeet/piki/extractor/support/IntegrationStubs.java
  • src/test/java/com/depromeet/piki/extractor/support/IntegrationTestSupport.java
  • src/test/java/com/depromeet/piki/extractor/support/StubGeminiClient.java
  • src/test/java/com/depromeet/piki/extractor/support/StubHeadlessRenderer.java
  • src/test/java/com/depromeet/piki/extractor/support/StubImageStorage.java
  • src/test/java/com/depromeet/piki/extractor/support/StubPageFetcher.java
  • src/test/java/com/depromeet/piki/extractor/support/TestConventionTest.java

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@m-a-king
m-a-king merged commit add91a6 into main Aug 3, 2026
2 checks passed
@m-a-king
m-a-king deleted the style/lombok-javadoc-slf4j branch August 3, 2026 12:46
m-a-king added a commit that referenced this pull request Aug 3, 2026
)

* fix: 추출 경로의 안전 공백 3건 - 헤드리스 redirect SSRF·null 바이트 오분류·이미지 경로 검증 우회

주석 정리(#18) 중 코드를 정독하다 드러난 것들이다. 셋 다 회귀 테스트를 함께 넣었고,
수정 전에는 네 테스트가 모두 실패함을 확인했다.

1) 헤드리스 경로의 redirect SSRF — HttpHeadlessRenderer 는 원본 URL 만 InternalHostGuard 로
   검증하고, 렌더 서비스가 따라간 최종 URL(final_url)은 검증 없이 그대로 썼다. '외부 URL →
   내부 주소' redirect 를 렌더 서비스가 대신 따라가 주면 내부망 응답이 상품 HTML 로 흘러들고,
   #17 이후로는 그 주소가 응답 계약의 finalUrl 로 호출자의 정체성(canonical) 입력까지 나간다.
   정적 fetch 가 매 hop 을 검증하는 것과 같은 기준을 세웠다 — BLOCKED_HOST 면 렌더 전체를
   거부하고, DNS 미해결처럼 '검증 불가'인 경우만 원본 link 로 폴백해 렌더 결과를 살린다.

2) ProductImage.of(null, ...) 가 500 — null 검사 전에 bytes.length 를 읽어 NPE 가 됐다.
   계약상 확정 실패(422)여야 할 입력이 일시 실패로 오분류돼 호출자가 무의미한 재시도를 한다.

3) 이미지 경로가 정규화를 우회 — GeminiImageResult 가 ProductSnapshot.fromExtracted 를 건너뛰고
   직접 생성해, 같은 LLM 이 만든 값인데도 음수 가격·공백 이름이 link 경로에서는 막히고 이미지
   경로에서만 호출자에게 새어 나갔다. 같은 팩토리를 태워 검증을 일치시켰다.

* docs: 헤드리스 SSRF 검증의 사후 한계 명시 + 백로그 현행화 (CodeRabbit 리뷰 반영)

CodeRabbit 이 지적한 대로 resolveFinalUrl 의 검증은 렌더 응답 이후에 도는 사후 검증이라,
내부 주소로의 요청 자체와 '외부 → 내부 → 외부' 체인은 막지 못한다. 코드가 그 한계를 스스로
말하도록 Javadoc 에 명시하고(다층 방어의 마지막 층이라는 위치까지), hop 단위 차단이 renderer
소관으로 남았음을 백로그에 남긴다. renderer 에 SSRF·egress 가드가 없음은 실측 확인했다.

* docs: 백로그 2번 해소 표시 누락 보정
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant