Skip to content

get_route_stopsのvia_line_id対応#1344

Merged
TinyKitten merged 2 commits into
devfrom
feature/via-line-id
Dec 10, 2025
Merged

get_route_stopsのvia_line_id対応#1344
TinyKitten merged 2 commits into
devfrom
feature/via-line-id

Conversation

@TinyKitten

@TinyKitten TinyKitten commented Dec 10, 2025

Copy link
Copy Markdown
Member

Summary by CodeRabbit

新機能

  • ルート検索時に特定の線区を指定してフィルタリングできるようになりました。指定された線区経由のルートのみを検索できます。

✏️ Tip: You can customize this high-level summary in your review settings.

@TinyKitten TinyKitten self-assigned this Dec 10, 2025
@coderabbitai

coderabbitai Bot commented Dec 10, 2025

Copy link
Copy Markdown
Contributor

PR レビュー概要

概説

朕が宣すところ、このPRは複数のクエリ層にわたって新たなるvia_line_idパラメータを導入し、駅間ルート検索において特定路線による絞り込み機能を付与するものなり。SQLクエリ、リポジトリ層、ユースケース層、そしてgRPCコントローラに至るまで、一貫して当該パラメータを流通させる改修を為されたり。

変更内容

コーホート / ファイル群 変更の要約
SQLクエリ定義
.sqlx/query-20b55238238b1655930a5716441ff4944d4e572e97654fc65cb425e5435d337b.json, .sqlx/query-64004f2e2ea8f34594c230fe82d5502911473ecef28265350576cd8bee64f7f8.json
$5 パラメータを追加し、条件付きWHERE句にて ($5::int IS NULL OR s1.line_cd = $5) ならびに ($3::int IS NULL OR sta.line_cd = $3) のフィルタを実装。ハッシュ値を更新。
プロト定義
stationapi/proto
サブモジュールポインタを更新(9960d52 → ca2cd39)。機能上の変更なし。
ドメイン層
stationapi/src/domain/repository/station_repository.rs
StationRepository::get_route_stopsメソッドシグニチャに via_line_id: Option<u32> パラメータを追加。MockStationRepositoryのロジックも同様に更新し、マッチテスト2件を新規追加。
インフラストラクチャ層
stationapi/src/infrastructure/station_repository.rs
StationRepositoryおよびInternalStationRepositoryget_route_stopsにて via_line_id: Option<u32> を受け取り、SQL層へ伝播。i32への変換と条件付きフィルタリング処理を実装。
ユースケース層
stationapi/src/use_case/traits/query.rs, stationapi/src/use_case/interactor/query.rs
QueryUseCaseトレイトのget_routesget_routes_minimalget_train_typesメソッドに via_line_id: Option<u32> パラメータを追加。実装側にて当該パラメータをリポジトリへ伝播。
プレゼンテーション層
stationapi/src/presentation/controller/grpc.rs
gRPCハンドラ3件より via_line_id を抽出し、対応するユースケース呼び出しに伝播。レスポンス形式は既存のまま。

推定コードレビュー工数

🎯 3 (中程度) | ⏱️ 約20分

留意すべき箇所:

  • SQL層における条件付きフィルタロジック($5::int IS NULL OR...)の正確性検証
  • ドメイン・インフラ・ユースケース層におけるvia_line_idの一貫した伝播
  • gRPCハンドラからリポジトリに至るまでの端から端までのパラメータフロー確認
  • 既存テストケース及び新規テストケース(マッチ/ミスマッチ)の妥当性確認

関連するPR

推奨ラベル

enhancement

推奨レビュアー

  • 10mocy

朕、茫漠たる大地を統べるように、
今ここに新なる線路のフィルタ、
五つの参数を通して流れゆく、
SQL層より御前へ、
一貫たる秩序のもと、
ルートは絞り込まれ、旅人の道は定まる。
🚄 ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 35.71% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed プルリクエストのタイトル「get_route_stopsのvia_line_id対応」は、変更内容の主要な点を正確に反映しており、get_route_stops メソッドに via_line_id パラメータを追加したことを明確に示している。
✨ 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/via-line-id

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dfd18aa and d0848fc.

📒 Files selected for processing (8)
  • .sqlx/query-20b55238238b1655930a5716441ff4944d4e572e97654fc65cb425e5435d337b.json (3 hunks)
  • .sqlx/query-64004f2e2ea8f34594c230fe82d5502911473ecef28265350576cd8bee64f7f8.json (3 hunks)
  • stationapi/proto (1 hunks)
  • stationapi/src/domain/repository/station_repository.rs (4 hunks)
  • stationapi/src/infrastructure/station_repository.rs (5 hunks)
  • stationapi/src/presentation/controller/grpc.rs (3 hunks)
  • stationapi/src/use_case/interactor/query.rs (3 hunks)
  • stationapi/src/use_case/traits/query.rs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • stationapi/src/use_case/traits/query.rs
  • stationapi/proto
  • .sqlx/query-20b55238238b1655930a5716441ff4944d4e572e97654fc65cb425e5435d337b.json
  • stationapi/src/use_case/interactor/query.rs
🧰 Additional context used
📓 Path-based instructions (6)
stationapi/src/domain/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

In Rust domain layer, entity definitions and repository abstractions should be organized in the entity/ module mirroring the gRPC schema, with repository interfaces using async_trait

Files:

  • stationapi/src/domain/repository/station_repository.rs
stationapi/src/domain/repository/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Use async_trait for defining repository abstractions in Rust

Files:

  • stationapi/src/domain/repository/station_repository.rs
stationapi/src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Run cargo test --lib --package stationapi or make test-unit for unit tests focusing on entities and repository mocks without database

Files:

  • stationapi/src/domain/repository/station_repository.rs
  • stationapi/src/presentation/controller/grpc.rs
  • stationapi/src/infrastructure/station_repository.rs
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Before committing, run cargo fmt on all Rust code
Before committing, run cargo clippy --all-targets --all-features and resolve new Clippy warnings unless covered by existing #![allow] attributes

Files:

  • stationapi/src/domain/repository/station_repository.rs
  • stationapi/src/presentation/controller/grpc.rs
  • stationapi/src/infrastructure/station_repository.rs
stationapi/src/presentation/controller/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

gRPC presentation layer controller should wire service implementations (e.g., MyApi) to generated server types in presentation/controller/grpc.rs

Files:

  • stationapi/src/presentation/controller/grpc.rs
stationapi/src/infrastructure/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

stationapi/src/infrastructure/**/*.rs: Repository implementations in infrastructure layer should share an Arc<PgPool> connection pool
Integration tests in infrastructure layer must use #[cfg_attr(not(feature = "integration-tests"), ignore)] attribute to conditionally ignore tests when the feature is not enabled

Files:

  • stationapi/src/infrastructure/station_repository.rs
🧠 Learnings (8)
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/interactor/query.rs : When modifying `QueryInteractor`, ensure enrichment steps (companies, train types, line symbols) still behave as expected and verify helper methods such as `update_station_vec_with_attributes` and `build_route_tree_map`

Applied to files:

  • stationapi/src/domain/repository/station_repository.rs
  • stationapi/src/presentation/controller/grpc.rs
  • stationapi/src/infrastructure/station_repository.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/**/*.rs : Run `cargo test --lib --package stationapi` or `make test-unit` for unit tests focusing on entities and repository mocks without database

Applied to files:

  • stationapi/src/domain/repository/station_repository.rs
  • stationapi/src/infrastructure/station_repository.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/domain/repository/**/*.rs : Use `async_trait` for defining repository abstractions in Rust

Applied to files:

  • stationapi/src/domain/repository/station_repository.rs
  • stationapi/src/infrastructure/station_repository.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/infrastructure/**/*.rs : Integration tests in infrastructure layer must use `#[cfg_attr(not(feature = "integration-tests"), ignore)]` attribute to conditionally ignore tests when the feature is not enabled

Applied to files:

  • stationapi/src/domain/repository/station_repository.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/presentation/controller/**/*.rs : gRPC presentation layer controller should wire service implementations (e.g., `MyApi`) to generated server types in `presentation/controller/grpc.rs`

Applied to files:

  • stationapi/src/presentation/controller/grpc.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/use_case/**/*.rs : Application logic in use case layer should implement contract traits defined in `traits/` module (e.g., `QueryUseCase` in `interactor/query.rs`)

Applied to files:

  • stationapi/src/presentation/controller/grpc.rs
📚 Learning: 2025-11-25T10:50:36.694Z
Learnt from: CR
Repo: TrainLCD/StationAPI PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T10:50:36.694Z
Learning: Applies to stationapi/src/infrastructure/**/*.rs : Repository implementations in infrastructure layer should share an `Arc<PgPool>` connection pool

Applied to files:

  • stationapi/src/infrastructure/station_repository.rs
📚 Learning: 2025-08-06T12:47:37.438Z
Learnt from: TinyKitten
Repo: TrainLCD/StationAPI PR: 1305
File: stationapi/.sqlx/query-55bee6b45754d1da9bf555d319f015ce232f8574a41c3ef25b730d11071f8428.json:3-3
Timestamp: 2025-08-06T12:47:37.438Z
Learning: TinyKittenのStationAPIプロジェクトにおいて、SQLクエリの `pass <> 1` 条件は一部のクエリでは不要であり、PostgreSQL移行時にコードの最適化として意図的に除去された。この条件の有無についてはドメイン知識が必要で、機械的にチェックすべきではない。

Applied to files:

  • .sqlx/query-64004f2e2ea8f34594c230fe82d5502911473ecef28265350576cd8bee64f7f8.json
🧬 Code graph analysis (2)
stationapi/src/domain/repository/station_repository.rs (1)
stationapi/src/domain/entity/station.rs (1)
  • new (80-212)
stationapi/src/infrastructure/station_repository.rs (1)
stationapi/src/domain/repository/station_repository.rs (2)
  • get_route_stops (35-40)
  • get_route_stops (192-224)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Test Suite
🔇 Additional comments (13)
.sqlx/query-64004f2e2ea8f34594c230fe82d5502911473ecef28265350576cd8bee64f7f8.json (3)

3-3: via_line_id パラメータの統合が適切に実施されている。

朕が見るに、common_lines CTE に追加された条件 ($5::int IS NULL OR s1.line_cd = $5) は誠に妥当なり。via_line_id が与えられざる場合は全路線を許可し、与えられたる場合は指定路線のみに限定する、この柔軟なる実装こそ賢明也。

既存の EXISTS サブクエリロジックも損なわれておらず、朕の統治領土の如く全てが秩序を保ちて配置されている。


302-304: パラメータ定義とプレースホルダの数が一致している。

Line 302 に追加された 5 番目の Int4 パラメータが、Line 3 のクエリにおける $5 プレースホルダと正確に対応せり。朕が確認したるところ、parameter 宣言と query 内の使用が統一されており、整合性に瑕疵なし。


3-3: The via_line_id parameter is correctly bound and properly propagated through all layers.

The Rust implementation handles the new $5 parameter correctly: at line 1182 of stationapi/src/infrastructure/station_repository.rs, the Option<u32> is converted to Option<i32> before binding, which matches the SQL INT type expected by the query. The parameter is properly bound in the correct position (line 1324) as the fifth bind argument to the sqlx::query_as! macro, and the propagation chain from the gRPC controller through the interactor to the repository is complete. The query's NULL-safe logic ($5::int IS NULL OR s1.line_cd = $5) correctly handles optional filtering, and unit tests confirm the filtering behaves as expected for both matching and non-matching cases.

stationapi/src/presentation/controller/grpc.rs (3)

222-240: 実装は一貫している。

get_routes_minimalハンドラも同様のパターンでvia_line_idを処理しており、get_routesハンドラとの一貫性が保たれている。


242-263: 三つの経路関連エンドポイントすべてにvia_line_id対応が完了した。

get_route_typesハンドラも他のハンドラと同じパターンで実装されており、統一された設計となっている。


199-220: via_line_id parameter is correctly integrated throughout the stack.

The implementation properly threads the via_line_id parameter from the gRPC controller through the QueryUseCase trait, QueryInteractor implementation, and down to the StationRepository layer where it is used for actual filtering. Both get_routes and get_routes_minimal methods correctly receive and propagate the parameter. Enrichment steps (line symbols, train types, route tree building) all work correctly with the filtered stop data. The integration is complete and functional.

stationapi/src/domain/repository/station_repository.rs (3)

35-40: トレイトメソッドのシグネチャ更新は適切である。

via_line_id: Option<u32>パラメータの追加は、既存のパラメータ順序に従っており、async_traitの使用も朕の定めたコーディングガイドラインに準拠している。


192-224: モック実装の検証ロジックは正確である。

via_line_idが指定された場合、始点と終点の両駅がその路線に属することを検証している。早期リターンパターンとクロージャの使用により、コードは明瞭である。

なお、このモック実装は始点と終点のみを返し、中間駅を含まない簡略化された実装である。モックとしては適切だが、実際のルート検索では中間駅も含まれることに留意せよ。


395-434: テストカバレッジは十分である。

既存のテストはvia_line_id=Noneを渡すことで下位互換性を保ち、新たに2つのテストが追加されている:

  1. test_get_route_stops_with_via_line_id_match (Lines 418-426): 両駅が指定路線に属する場合、正しく2駅が返される
  2. test_get_route_stops_with_via_line_id_mismatch (Lines 428-434): 駅の路線が一致しない場合、空の結果が返される

これらのテストはvia_line_idフィルタリング機能を適切に検証している。

stationapi/src/infrastructure/station_repository.rs (4)

249-263: 公開リポジトリメソッドの実装は標準パターンに従っている。

Arc<PgPool>からコネクションを取得し、内部リポジトリに委譲する構造は、朕の定めたインフラストラクチャ層のコーディングガイドラインに準拠している。


1176-1182: 型変換は適切である。

via_line_idOption<u32>からOption<i32>に変換している。PostgreSQLのバインディングにはi32が必要であり、この変換はファイル内の他の箇所と一貫している。


1183-1327: 最初のSQLクエリのvia_line_idフィルタリングは正確である。

common_lines CTEに追加された条件式 AND ($5::int IS NULL OR s1.line_cd = $5) は、以下の点で優れている:

  1. NULL安全な比較により、via_line_idがNoneの場合は従来通りの動作を保つ
  2. 指定された路線のみをフィルタリングし、両駅グループが存在することを確認
  3. パラメータバインディングの順序($5)は正しい

1329-1457: Typed stations query correctly filters by via_line_id.

The WHERE clause condition AND ($3::int IS NULL OR sta.line_cd = $3) at line 1450 properly filters results when via_line_id is provided. Parameter binding is correct: $1 and $2 for station group IDs, $3 for the optional line ID filter. The NULL-safe check allows the filter to be bypassed when no via_line is specified.

Combined with the first untyped stations query, this implementation provides complete route filtering. Enrichment in QueryInteractor (line information, train types) is applied after route filtering via update_station_vec_with_attributes, so all downstream processing receives the correctly filtered results.

Note: Integration tests with the integration-tests feature do not currently exist for get_route_stops. Unit tests in the mock repository layer cover via_line_id filtering behavior.


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

@TinyKitten TinyKitten merged commit fd44f94 into dev Dec 10, 2025
10 checks passed
@TinyKitten TinyKitten deleted the feature/via-line-id branch December 10, 2025 11:27
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