Skip to content

[REFAC#388] fetcher/rule/upgrader → publisher.PublishUpgrade 이동 (#385 Sub 3) - #398

Merged
juhy0987 merged 2 commits into
mainfrom
refactor/#388/publisher-upgrade
May 12, 2026
Merged

juhy0987 merged 2 commits into
mainfrom
refactor/#388/publisher-upgrade

Conversation

@juhy0987

Copy link
Copy Markdown
Member

연관 이슈


구현 내용

#385 Sub 3 — auto-upgrade republish 의 Kafka publish 책임을 publisher facade 로 흡수. fetcher 의사결정 로직은 잔존 (책임 분리 원칙).

신규 (publisher)

파일 내용
`publisher/upgrade.go` (신규) `UpgradePublisher` interface (이슈 #396 원칙 — publisher 측 정의) + `PublishUpgrade(ctx, host, msgs) error` 메소드

책임 분리 (메타 #385)

역할 위치
PublishBatch + 일관 로그 publisher.PublishUpgrade
의사결정 (in-flight lock / GetByHost / Upsert / Invalidate / PeekByHost / RemoveByHost) fetcher Upgrader
force_fetcher metadata + token 부착 fetcher Upgrader
retry_reason / original_raw_id header 부착 fetcher Upgrader
stale tracking + CrawlJob 빌드 fetcher Upgrader

fetcher 변경

  • `internal/processor/fetcher/rule/upgrader.go`:
    • `producer queue.Producer` → `publisher publisher.UpgradePublisher`
    • `u.producer.PublishBatch(ctx, msgs)` → `u.publisher.PublishUpgrade(ctx, host, msgs)`
    • `NewUpgrader` 시그니처 변경 + 에러 메시지 정정 ("Producer" → "UpgradePublisher")

cmd/issuetracker/main.go wiring

  • `crawlerProducer` → `jobPublisher` (단일 facade)

테스트

`test/internal/publisher/upgrade_test.go` 신규 (3건):

  • empty msgs → noop (PublishBatch 호출 X)
  • non-empty → PublishBatch 1회 호출 검증
  • 실패 시 host / count 포함 에러 wrap

CI / 머지 게이트 점검

변경 영향 범위

  • 영향: `internal/publisher/upgrade.go`, `internal/processor/fetcher/rule/upgrader.go`, `cmd/issuetracker/main.go`
  • 위험도: Low — 책임 분리 + 인터페이스 도입. 동작 무변경.

검증

  • `go build ./...` 통과
  • `go vet ./...` 통과
  • 전체 37 패키지 `go test -race -count=1` (clean cache) 통과

Required Status Checks

  • 통과 확인 대상: Commit Lint / PR Title Lint / Linked Issue Check / Format Check / Build / Test / Lint

롤백 계획

  • PR revert — fetcher 측에 producer 의존 복원

TODO

  • (없음) Sub 3 scope 완료

논의 사항

🤖 Generated with Claude Code

#385 (Publisher 통합 모듈화 메타) Sub 3 — auto-upgrade republish 의 Kafka publish
책임을 publisher facade 로 흡수. fetcher 의사결정 로직은 잔존 (책임 분리 원칙).

신규 (publisher):
- publisher/upgrade.go (신규):
  · UpgradePublisher 인터페이스 정의 (이슈 #396 원칙 — publisher 측 정의)
  · PublishUpgrade(ctx, host, msgs) 메소드 — PublishBatch + 일관 로그
  · upgrade republish 의 Kafka I/O 단일 책임

책임 분리 (메타 #385):
  - publisher 책임: PublishBatch 호출 + "upgrade republish published" 로그
  - fetcher Upgrader 잔존: 의사결정 (in-flight lock / GetByHost / Upsert /
    Invalidate / PeekByHost / RemoveByHost), force_fetcher metadata + token,
    retry_reason / original_raw_id header, stale tracking, CrawlJob 빌드

fetcher 변경:
- internal/processor/fetcher/rule/upgrader.go:
  · producer queue.Producer → publisher publisher.UpgradePublisher
  · u.producer.PublishBatch(ctx, msgs) → u.publisher.PublishUpgrade(ctx, host, msgs)
  · NewUpgrader 시그니처 변경 + 에러 메시지 정정

cmd/issuetracker/main.go wiring:
- crawlerProducer → jobPublisher (단일 facade)

테스트:
- test/internal/publisher/upgrade_test.go 신규 (3건):
  · empty msgs → noop (PublishBatch 호출 X)
  · non-empty → PublishBatch 1회 호출 검증
  · 실패 시 host / count 포함 에러 wrap

검증:
- go build + go vet 통과
- 전체 37 패키지 race test (clean cache) 통과
Copilot AI review requested due to automatic review settings May 12, 2026 14:50
@juhy0987 juhy0987 added the refactor Code refactoring label May 12, 2026
@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@juhy0987 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 36 minutes and 11 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: aa8be8ff-b1c1-4d2b-9df2-9547f12e76d4

📥 Commits

Reviewing files that changed from the base of the PR and between eea7290 and c95b7ac.

📒 Files selected for processing (4)
  • cmd/issuetracker/main.go
  • internal/processor/fetcher/rule/upgrader.go
  • internal/publisher/upgrade.go
  • test/internal/publisher/upgrade_test.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/#388/publisher-upgrade

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 and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the Upgrader to use a new UpgradePublisher interface instead of a raw queue.Producer, centralizing Kafka publishing logic. Key changes include updating the Upgrader struct, implementing the PublishUpgrade method in the publisher package, and adding unit tests. Review feedback suggests renaming the publisher field to avoid shadowing the package name, adding a missing nil check for the logger in NewUpgrader to ensure consistency with its documentation, and performing defensive nil checks on the logger in PublishUpgrade to prevent potential panics.

Comment thread internal/processor/fetcher/rule/upgrader.go Outdated
Comment thread internal/processor/fetcher/rule/upgrader.go Outdated
Comment thread internal/processor/fetcher/rule/upgrader.go Outdated
Comment thread internal/processor/fetcher/rule/upgrader.go
Comment thread internal/publisher/upgrade.go Outdated
@juhy0987 juhy0987 self-assigned this May 12, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

이 PR은 fetcher의 auto-upgrade republish 경로에서 Kafka 발행(PublishBatch) 책임을 publisher facade로 이동시켜, #385 메타의 “Kafka I/O 단일 책임 hub = publisher” 원칙에 맞게 책임을 분리합니다. fetcher 쪽 Upgrader는 의사결정/메시지 빌드를 유지하고, publisher는 배치 발행 + 일관 로그만 담당하도록 정리되었습니다.

Changes:

  • internal/publisher/upgrade.go 신규: UpgradePublisher 인터페이스 및 (*Publisher).PublishUpgrade 추가
  • fetcher Upgrader가 queue.Producer 대신 publisher.UpgradePublisher에 의존하도록 변경
  • main wiring 및 publisher PublishUpgrade 단위 테스트 추가

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
test/internal/publisher/upgrade_test.go PublishUpgrade의 noop/배치 발행/에러 래핑 동작 단위 테스트 추가
internal/publisher/upgrade.go Upgrade republish 전용 UpgradePublisher 계약 및 PublishUpgrade 구현 추가
internal/processor/fetcher/rule/upgrader.go Upgrader 의존성 producerpublisher.UpgradePublisher로 교체 및 호출부 변경
cmd/issuetracker/main.go Upgrader 생성 시 jobPublisher(facade)를 주입하도록 wiring 갱신

Comment thread internal/processor/fetcher/rule/upgrader.go
gemini medium 5건 + Copilot 1건 모두 수용:

1) field shadowing (gemini 3건 — upgrader.go:65/104/253)
   - Upgrader.publisher 필드명이 publisher 패키지 이름과 shadow → 명확성/충돌 위험
   - publisher → upgradePub 으로 rename (struct 정의 / NewUpgrader 초기화 / 메소드 호출)

2) NewUpgrader log nil 체크 부재 (gemini upgrader.go:98 + Copilot upgrader.go:74)
   - docstring "모든 인자 nil 허용 안 함" 과 실제 동작 (log==nil 허용) 모순
   - if log == nil { return error } 추가 — docstring 과 일치
   - logWarn/logDebug 내부의 u.log==nil 분기는 잔존 가능 (test 등에서 nil 이 전달돼도
     panic 회피 — defense in depth)

3) PublishUpgrade p.log nil 방어 (gemini publisher/upgrade.go:49)
   - publisher.New 가 log 검증 안 하므로 caller 보호 — p.log == nil 시 로그 skip
   - 본 PR 의 신규 메소드 한정 — 다른 publisher 메소드의 nil 일관성은 별도 작업

검증:
- go build + go vet 통과
- publisher / fetcher/rule race test 통과
@juhy0987
juhy0987 merged commit 3fe6259 into main May 12, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Code refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[REFACTOR] fetcher/rule/upgrader → publisher/upgrade.go 이동 (#385 Sub 3)

2 participants