Skip to content

Remove dead code and simplify adapter tests - #1070

Merged
bootjp merged 2 commits into
mainfrom
refactor/remove-dead-code
Jul 11, 2026
Merged

Remove dead code and simplify adapter tests#1070
bootjp merged 2 commits into
mainfrom
refactor/remove-dead-code

Conversation

@bootjp

@bootjp bootjp commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • remove unreachable DynamoDB legacy JSON decoding, unused encryption/stream helpers, and the obsolete Redis standalone SET dedup no-op
  • consolidate duplicate DynamoDB request-size tests into table-driven coverage
  • replace the deprecated S3 reverse-proxy Director hook with Rewrite while preserving the signed Host, path, query, and forwarding headers
  • update the Redis rollout design note to match the removed compatibility switch

Impact and risk

The removed symbols had no reachable callers in the complete program, including tests, on both macOS and Linux analysis. Active configuration options remain unchanged. The S3 proxy refactor preserves SigV4-sensitive request fields and adds explicit regression coverage.

Validation

  • golangci-lint run ./... --timeout=5m
  • staticcheck ./...
  • dead-code analysis with tests on macOS and linux/amd64
  • go test -race ./adapter -run ^TestS3Server_ProxiesFollowerRequests$ -count=1
  • go test ./adapter -run ^(TestS3Server_ProxiesFollowerRequests|TestDynamoDB_RequestBodyTooLarge)$ -count=1
  • all packages except the known long-running full adapter suite: go list ./... | rg -v ^github.com/bootjp/elastickv/adapter$ | xargs go test -count=1

The full go test ./... -count=1 run reached the adapter suite but was stopped after it made no progress for approximately four minutes; the targeted adapter tests above passed.

Summary by CodeRabbit

  • バグ修正

    • S3のリーダー転送時に、ホスト名・パス・クエリ・転送ヘッダーが正しく引き継がれるよう改善しました。
    • プロキシ転送エラー時のエラーレスポンスを統一しました。
    • RedisのスタンドアロンSETが、トランザクション重複排除設定に従う動作へ整理されました。
  • ドキュメント

    • Redisのデフォルト動作と互換設定の扱いを更新しました。
  • テスト

    • 大きすぎるリクエストボディの検証を統合し、S3転送情報の確認範囲を拡張しました。

@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c85c4010-dc87-487d-8e25-ad79da572ee8

📥 Commits

Reviewing files that changed from the base of the PR and between 57b37a5 and 14052b5.

📒 Files selected for processing (9)
  • adapter/dynamodb_schema.go
  • adapter/dynamodb_test.go
  • adapter/encryption_admin.go
  • adapter/redis.go
  • adapter/s3.go
  • adapter/s3_test.go
  • docs/design/2026_06_10_implemented_redis_onephase_dedup_default_on.md
  • internal/test_helper.go
  • store/stream_helpers.go

📝 Walkthrough

Walkthrough

S3フォロワーのリーダープロキシ転送と検証を更新し、Redisの互換用SET dedup設定、暗号化管理のkeystore注入、DynamoDB正規化、ストリーム補助APIを削除しました。DynamoDBのサイズ超過テストも統合しています。

Changes

S3フォロワープロキシ

Layer / File(s) Summary
リーダー転送のRewrite化
adapter/s3.go, adapter/s3_test.go
ReverseProxyのRewriteで生クエリ、入力ホスト、X-Forwardedヘッダーを保持し、テストで転送先の内容とタイムアウトを検証します。

Redis互換ゲート整理

Layer / File(s) Summary
standalone SET互換ゲートの削除
adapter/redis.go, docs/design/...redis_onephase_dedup_default_on.md
standaloneSetDedup、関連オプション、環境変数初期化を削除し、既定のonePhaseTxnDedupに関する設計記述を更新します。

互換APIと補助関数の削除

Layer / File(s) Summary
旧APIと正規化処理の削除
adapter/encryption_admin.go, adapter/dynamodb_schema.go, store/stream_helpers.go
keystore注入API、DynamoDB GSIのカスタムJSON正規化、ストリームIDのデコード・抽出関数を削除します。

DynamoDBテスト統合

Layer / File(s) Summary
要求ボディ超過テストの統合
adapter/dynamodb_test.go
DeleteItemとPutItemの要求ボディ超過検証を共通のサブテストに統合します。

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Follower as S3フォロワー
  participant Proxy as ReverseProxy
  participant Leader as S3リーダー
  Follower->>Proxy: リーダーへの転送を開始
  Proxy->>Leader: Rewrite済みURL・RawQuery・Host・転送ヘッダー
  Leader-->>Follower: 転送レスポンス
Loading

Possibly related PRs

  • bootjp/elastickv#620: 同じストリーム補助関数とストリームエントリキー構造を扱っています。
  • bootjp/elastickv#1013: standaloneSetDeduponePhaseTxnDedup のRedis実装が重複します。
  • bootjp/elastickv#1019: EncryptionAdminServerの構造体とオプション定義を変更しています。
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

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

Copy link
Copy Markdown
Contributor

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 cleans up various unused fields, methods, and helpers across the codebase, including DynamoDB index unmarshaling, encryption admin keystore configurations, Redis standalone SET deduplication, and unused stream helpers. Additionally, it refactors S3 leader proxying to use the modern httputil.ReverseProxy with a Rewrite callback instead of the deprecated Director approach, and updates the corresponding tests. Feedback on the S3 proxy implementation notes that the manual assignment of the X-Forwarded-For header is redundant, as req.SetXForwarded() handles this automatically.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread adapter/s3.go
@bootjp

bootjp commented Jul 11, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@bootjp

bootjp commented Jul 11, 2026

Copy link
Copy Markdown
Owner Author

Review follow-up:

  • all CI checks are green
  • evaluated the Gemini X-Forwarded-For finding against the Go 1.26 implementation
  • retained the copy because SetXForwarded reads the prior chain from Out.Header; removing it would change legacy Director forwarding semantics
  • replied with the implementation detail and resolved the thread

HEAD: ee9fea8

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ee9fea82a6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread adapter/s3.go
Comment thread adapter/s3_test.go Outdated
@bootjp

bootjp commented Jul 11, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@bootjp

bootjp commented Jul 11, 2026

Copy link
Copy Markdown
Owner Author

Review follow-up for HEAD 14052b5:

  • preserve the exact inbound RawQuery across ReverseProxy Rewrite for SigV4 compatibility
  • bound the proxy regression test channel wait to five seconds
  • resolved both review threads

Validation:

  • go test ./adapter -run ^TestS3Server_ProxiesFollowerRequests$ -count=1
  • go test -race ./adapter -run ^TestS3Server_ProxiesFollowerRequests$ -count=1
  • staticcheck ./adapter
  • golangci-lint run ./adapter --timeout=5m

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: 14052b5e12

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@bootjp
bootjp marked this pull request as ready for review July 11, 2026 13:37
@bootjp
bootjp merged commit 0226b4e into main Jul 11, 2026
13 checks passed
@bootjp
bootjp deleted the refactor/remove-dead-code branch July 11, 2026 13:37
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