Skip to content

Conversation

@TinyKitten
Copy link
Member

@TinyKitten TinyKitten commented Jan 26, 2026

Summary by CodeRabbit

リリースノート

  • New Features

    • REST APIを追加:ロケーション更新(POST /api/location)、ログ記録(POST /api/log)、ヘルスチェック(GET /healthz)の3つのエンドポイントに対応
    • ベアラートークンによるAPIアクセス認証を実装
  • Documentation

    • OpenAPI 3.0.3仕様書を新規追加

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

@TinyKitten TinyKitten self-assigned this Jan 26, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 26, 2026

📝 Walkthrough

Walkthrough

REST APIエンドポイント(/api/location/api/log)をBearer認証付きで追加し、WebSocketハンドラを簡潔化。ドメインモデルを更新してLocationUpdateRequestとLogRequestを導入。OpenAPI仕様を新規作成し、tower/hyper依存関係を追加。

Changes

Cohort / File(s) 変更内容
依存関係の追加
Cargo.toml
tower 0.4とhyper 0.14の2つの新しい依存関係を追加
API仕様ファイル
openapi.yaml
OpenAPI 3.0.3仕様ファイルを新規作成。POST /api/location、POST /api/log、GET /healthzの3つのエンドポイントを定義。Bearer認証とLocationUpdateRequest、LogRequest等のスキーマを含む
ドメインモデル更新
src/domain.rs
IncomingMessage列挙型からLocationUpdateとLog変種を削除。LocationUpdateRequestとLogRequestの新しい公開構造体を追加(camelCase field naming)。ErrorType列挙型からPayloadParseError、AccuracyLow、InvalidCoords、_Unknownを削除
REST API実装とサーバー拡張
src/server.rs
post_locationとpost_logハンドラを追加。FromRequestParts実装によるBearer認証メカニズム導入。ApiResponse構造体を追加。WebSocketハンドラからstorageとsegmenterの依存関係を削除。REST APIとauth設定用のテストを追加

Sequence Diagram(s)

sequenceDiagram
    actor Client
    participant Server
    participant Authenticated as Auth Extractor
    participant AppState as AppState<br/>(config, storage, hub)
    participant Handler as Request Handler<br/>(post_location/log)
    participant Response

    Client->>Server: POST /api/location or /api/log<br/>(with Authorization header)
    Server->>Authenticated: Extract Bearer token
    alt Token present and enabled
        Authenticated->>Authenticated: Validate token
        Authenticated->>Handler: Pass Authenticated marker
    else Token missing/disabled
        Authenticated-->>Client: 401 Unauthorized<br/>Json(ApiResponse)
        Note over Client: Reject request
    end
    
    Handler->>AppState: Access state (storage, hub, etc.)
    Handler->>Handler: Deserialize JSON body<br/>& validate payload
    alt Validation successful
        Handler->>AppState: Process request<br/>(store location/log, broadcast)
        Handler->>Response: 200 OK<br/>Json(ApiResponse)
    else Validation fails
        Handler->>Response: 400 Bad Request<br/>Json(ApiResponse)
    end
    
    Response-->>Client: Return response with<br/>optional error/warning fields
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • 取得データ永続化 #11: 同じhandle_socket/handle_text関数を修正し、サーバーを通じてStorageをスレッド化する方法に関わる変更。新しいPRはREST APIの追加に伴う削除と再構造化を行い、既存PRはDB永続化を追加するため、同一関数の改変で関連性が高い。

Poem

🐰✨ REST APIの道を切り開き、

Bearer認証で守られたエンドポイント、

オープンAPI仕様が道を示す—

ウサギも喜ぶ新機能!🚀

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 56.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PRのタイトルは、WebSocketからREST APIへの移行というPRの主要な変更内容を明確に表現しており、変更セットと関連性がある。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/server.rs`:
- Around line 204-221: The current token extraction uses h.strip_prefix("Bearer
") which is case-sensitive and rejects valid "bearer" variants; update the
closure that produces token (the code using auth_header and token) to perform a
case-insensitive check for the "Bearer " scheme instead of strip_prefix("Bearer
"), e.g. test the prefix with an ASCII case-insensitive comparison
(eq_ignore_ascii_case on the first 7 chars) and then slice the remainder as the
token; keep the same error handling branch so behavior is unchanged when the
header is missing or malformed.
🧹 Nitpick comments (2)
openapi.yaml (1)

291-307: log.message の空白のみ禁止をスキーマにも反映したい

サーバ側は空白のみを拒否するため、OpenAPI に pattern を追加するとクライアント側でも検出できます。

🛠️ 修正案
         message:
           type: string
           minLength: 1
+          pattern: '.*\S.*'
           description: Log message (must not be empty or whitespace-only)
Cargo.toml (1)

27-28: テスト専用依存は dev-dependencies へ移動すべき

towerhyper#[cfg(test)] モジュール内の テストコードでのみ使用されています(ServiceExtto_bytes など)。ランタイム依存を最小限に保つため、これらを [dev-dependencies] に移すことをお勧めします。

修正案
csv = "1.4"
-tower = "0.4"
-hyper = "0.14"

+[dev-dependencies]
+tower = "0.4"
+hyper = "0.14"

Comment on lines +204 to +221
let auth_header = parts
.headers
.get(AUTHORIZATION)
.and_then(|v| v.to_str().ok());

let token = auth_header
.and_then(|h| h.strip_prefix("Bearer "))
.ok_or_else(|| {
(
StatusCode::UNAUTHORIZED,
Json(ApiResponse {
ok: false,
id: None,
warning: None,
error: Some("missing or invalid Authorization header".to_string()),
}),
)
})?;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Bearerスキームの大文字小文字を許容したい

スキームは大文字小文字を区別しないため、bearer を弾くと一部クライアントで401になります。case-insensitive 判定を推奨します。

🛠️ 修正案
-        let token = auth_header
-            .and_then(|h| h.strip_prefix("Bearer "))
+        let token = auth_header
+            .and_then(|h| {
+                let mut parts = h.splitn(2, ' ');
+                let scheme = parts.next()?;
+                let token = parts.next()?;
+                if scheme.eq_ignore_ascii_case("bearer") {
+                    Some(token)
+                } else {
+                    None
+                }
+            })
             .ok_or_else(|| {
                 (
                     StatusCode::UNAUTHORIZED,
                     Json(ApiResponse {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let auth_header = parts
.headers
.get(AUTHORIZATION)
.and_then(|v| v.to_str().ok());
let token = auth_header
.and_then(|h| h.strip_prefix("Bearer "))
.ok_or_else(|| {
(
StatusCode::UNAUTHORIZED,
Json(ApiResponse {
ok: false,
id: None,
warning: None,
error: Some("missing or invalid Authorization header".to_string()),
}),
)
})?;
let auth_header = parts
.headers
.get(AUTHORIZATION)
.and_then(|v| v.to_str().ok());
let token = auth_header
.and_then(|h| {
let mut parts = h.splitn(2, ' ');
let scheme = parts.next()?;
let token = parts.next()?;
if scheme.eq_ignore_ascii_case("bearer") {
Some(token)
} else {
None
}
})
.ok_or_else(|| {
(
StatusCode::UNAUTHORIZED,
Json(ApiResponse {
ok: false,
id: None,
warning: None,
error: Some("missing or invalid Authorization header".to_string()),
}),
)
})?;
🤖 Prompt for AI Agents
In `@src/server.rs` around lines 204 - 221, The current token extraction uses
h.strip_prefix("Bearer ") which is case-sensitive and rejects valid "bearer"
variants; update the closure that produces token (the code using auth_header and
token) to perform a case-insensitive check for the "Bearer " scheme instead of
strip_prefix("Bearer "), e.g. test the prefix with an ASCII case-insensitive
comparison (eq_ignore_ascii_case on the first 7 chars) and then slice the
remainder as the token; keep the same error handling branch so behavior is
unchanged when the header is missing or malformed.

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.

2 participants