From bc0976c48ac207f79c48995dbe0794797072b444 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 12 Jul 2026 08:22:16 +0000 Subject: [PATCH 1/2] =?UTF-8?q?=E4=BA=AC=E7=8E=8B=E3=83=90=E3=82=B9GTFS?= =?UTF-8?q?=E3=81=AE=E3=83=80=E3=82=A6=E3=83=B3=E3=83=AD=E3=83=BC=E3=83=89?= =?UTF-8?q?404=E3=82=92=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ODPTのバージョン付きファイルエンドポイント (files/odpt/KeioBus/AllLines.zip) は `date` セレクタが必須で、 省略すると有効なトークンを付けても HTTP 404 を返す。京王バスの フィードURLに `?date=current` を付与し、常に「本日有効なダイヤ」の GTFSを取得するようにする（同一エンドポイント形式の宇野バスで date=current が200・正常なGTFS zipを返すこと、date省略で404に なることを確認済み）。 あわせて以下を修正: - `gtfs_download_url` のクエリ結合バグを修正。URLに既にクエリ 文字列がある場合は `&` で `acl:consumerKey` を連結する （純粋関数 `append_consumer_key` に切り出しユニットテストを追加）。 これが無いと `...?date=current?acl:consumerKey=...` と不正な URLになる - 1フィードのダウンロード失敗を非致命化。従来は京王の404が全 フィード(Toei/Seibu含む)のインポートを巻き添えで中断させて いたが、該当フィードのみスキップして他は継続するようにする （ディレクトリ欠落フィードは後続の取り込みループが元々スキップ） - `test_gtfs_feeds` のURL期待値を更新 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01SfBspXi8MZnEjwrAxvbBzz --- stationapi/src/import.rs | 55 +++++++++++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 7 deletions(-) diff --git a/stationapi/src/import.rs b/stationapi/src/import.rs index 92a9d5db..d4f78f4f 100644 --- a/stationapi/src/import.rs +++ b/stationapi/src/import.rs @@ -287,10 +287,12 @@ const GTFS_FEEDS: &[GtfsFeed] = &[ requires_consumer_key: true, }, GtfsFeed { + // ODPT's versioned files endpoint requires a `date` selector; `current` + // always resolves to the timetable in effect today (omitting it 404s). id: "keio", name: "Keio Bus", path: "data/KeioBus-GTFS", - url: "https://api.odpt.org/api/v4/files/odpt/KeioBus/AllLines.zip", + url: "https://api.odpt.org/api/v4/files/odpt/KeioBus/AllLines.zip?date=current", requires_consumer_key: true, }, ]; @@ -306,6 +308,14 @@ fn scoped_gtfs_id_opt(feed: &GtfsFeed, id: Option<&str>) -> Option { .map(|s| scoped_gtfs_id(feed, s)) } +/// Append the ODPT `acl:consumerKey` query parameter to a feed URL, using the +/// correct separator (`?` for the first parameter, `&` when the URL already +/// carries a query string such as `?date=current`). +fn append_consumer_key(url: &str, token: &str) -> String { + let separator = if url.contains('?') { '&' } else { '?' }; + format!("{}{}acl:consumerKey={}", url, separator, token) +} + fn gtfs_download_url(feed: &GtfsFeed) -> Result> { if !feed.requires_consumer_key { return Ok(feed.url.to_string()); @@ -318,7 +328,7 @@ fn gtfs_download_url(feed: &GtfsFeed) -> Result Result<(), Box> { .join(", ") ); - // Download GTFS data if not present (use spawn_blocking to avoid blocking async runtime) + // Download GTFS data if not present (use spawn_blocking to avoid blocking async runtime). + // A single feed's download failure (e.g. an operator's ODPT outage or a moved URL) + // must not abort the whole pipeline: log it and continue so the other feeds still + // import. The per-feed import loop below skips any feed whose directory is missing. let download_start = std::time::Instant::now(); for feed in enabled_feeds.iter().copied() { - tokio::task::spawn_blocking(move || download_gtfs(feed)) + let result = tokio::task::spawn_blocking(move || download_gtfs(feed)) .await - .map_err(|e| format!("Failed to spawn blocking task: {}", e))? - .map_err(|e| -> Box { e })?; + .map_err(|e| format!("Failed to spawn blocking task: {}", e))?; + if let Err(e) = result { + warn!( + "Failed to download {} GTFS: {}. Skipping this feed; other feeds continue.", + feed.name, e + ); + } } info!( "[gtfs] download/extract finished in {:?}", @@ -3144,13 +3162,36 @@ mod tests { let keio = GTFS_FEEDS.iter().find(|feed| feed.id == "keio").unwrap(); assert_eq!(keio.name, "Keio Bus"); assert_eq!(keio.path, "data/KeioBus-GTFS"); + // ODPT's versioned files endpoint 404s without a `date` selector; `current` + // keeps the URL pinned to the timetable in effect today. assert_eq!( keio.url, - "https://api.odpt.org/api/v4/files/odpt/KeioBus/AllLines.zip" + "https://api.odpt.org/api/v4/files/odpt/KeioBus/AllLines.zip?date=current" ); assert!(keio.requires_consumer_key); } + #[test] + fn test_append_consumer_key() { + // URL without an existing query string uses `?`. + assert_eq!( + append_consumer_key( + "https://api.odpt.org/api/v4/files/SeibuBus/data/SeibuBus-GTFS.zip", + "TOKEN" + ), + "https://api.odpt.org/api/v4/files/SeibuBus/data/SeibuBus-GTFS.zip?acl:consumerKey=TOKEN" + ); + // URL that already carries a query string (e.g. `?date=current`) uses `&` + // so the consumer key does not clobber the existing parameter. + assert_eq!( + append_consumer_key( + "https://api.odpt.org/api/v4/files/odpt/KeioBus/AllLines.zip?date=current", + "TOKEN" + ), + "https://api.odpt.org/api/v4/files/odpt/KeioBus/AllLines.zip?date=current&acl:consumerKey=TOKEN" + ); + } + #[test] fn test_escape_sql_string_single_quotes() { // Single quotes should be doubled From 6137327c2b20fb36b05c84072550f45d98df6271 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 12 Jul 2026 08:39:13 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=E3=83=AC=E3=83=93=E3=83=A5=E3=83=BC?= =?UTF-8?q?=E6=8C=87=E6=91=98=E5=AF=BE=E5=BF=9C:=20GTFS=E3=83=80=E3=82=A6?= =?UTF-8?q?=E3=83=B3=E3=83=AD=E3=83=BC=E3=83=89=E3=81=AE=E5=A0=85=E7=89=A2?= =?UTF-8?q?=E6=80=A7=E3=81=A8=E3=83=88=E3=83=BC=E3=82=AF=E3=83=B3=E7=A7=98?= =?UTF-8?q?=E5=8C=BF=E3=82=92=E6=94=B9=E5=96=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbitのレビュー指摘のうち、妥当かつ小さいものを対応: - ダウンロードタスクの JoinError(パニック/キャンセル)も非致命化。 従来は spawn_blocking の `?` 伝播で全フィードのインポートが中断 していたが、該当フィードのみログしてスキップし他は継続する - ダウンロード/展開が途中失敗した際に部分生成ディレクトリを削除。 後続の存在確認で不完全なデータを有効フィードとして取り込むのを防ぐ (download_gtfs をラッパ + download_and_extract_gtfs に分割) - reqwest エラーから `without_url()` でリクエストURLを除去し、 `acl:consumerKey` トークンがログや伝播エラーに漏れないようにする Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01SfBspXi8MZnEjwrAxvbBzz --- stationapi/src/import.rs | 58 +++++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 12 deletions(-) diff --git a/stationapi/src/import.rs b/stationapi/src/import.rs index d4f78f4f..10283f9b 100644 --- a/stationapi/src/import.rs +++ b/stationapi/src/import.rs @@ -331,7 +331,11 @@ fn gtfs_download_url(feed: &GtfsFeed) -> Result Result<(), Box> { let gtfs_path = Path::new(feed.path); @@ -344,11 +348,33 @@ fn download_gtfs(feed: GtfsFeed) -> Result<(), Box Ok(()), + Err(e) => { + if gtfs_path.exists() { + if let Err(cleanup_err) = fs::remove_dir_all(gtfs_path) { + warn!( + "Failed to remove partial {} GTFS directory after error: {}", + feed.name, cleanup_err + ); + } + } + Err(e) + } + } +} + +fn download_and_extract_gtfs( + feed: &GtfsFeed, + gtfs_path: &Path, +) -> Result<(), Box> { info!("Downloading {} GTFS data from ODPT API...", feed.name); - // Download the ZIP file + // Download the ZIP file. `without_url()` strips the request URL (which carries + // the `acl:consumerKey` token) from any transport error so the token cannot + // leak into logs or propagated error messages. let request_start = std::time::Instant::now(); - let response = reqwest::blocking::get(gtfs_download_url(&feed)?)?; + let response = reqwest::blocking::get(gtfs_download_url(feed)?).map_err(|e| e.without_url())?; info!( "[gtfs-download:{}] response received in {:?} (status={})", feed.id, @@ -366,7 +392,7 @@ fn download_gtfs(feed: GtfsFeed) -> Result<(), Box Result<(), Box> { // import. The per-feed import loop below skips any feed whose directory is missing. let download_start = std::time::Instant::now(); for feed in enabled_feeds.iter().copied() { - let result = tokio::task::spawn_blocking(move || download_gtfs(feed)) - .await - .map_err(|e| format!("Failed to spawn blocking task: {}", e))?; - if let Err(e) = result { - warn!( - "Failed to download {} GTFS: {}. Skipping this feed; other feeds continue.", - feed.name, e - ); + match tokio::task::spawn_blocking(move || download_gtfs(feed)).await { + Ok(Ok(())) => {} + Ok(Err(e)) => { + warn!( + "Failed to download {} GTFS: {}. Skipping this feed; other feeds continue.", + feed.name, e + ); + } + // A panic or cancellation of the blocking task must not abort the whole + // pipeline either: log it and let the remaining feeds proceed. + Err(join_err) => { + warn!( + "{} GTFS download task did not complete ({}). Skipping this feed; other feeds continue.", + feed.name, join_err + ); + } } } info!(