Skip to content

chore: serialization/pandas modernization, settings hardening, race fixes#187

Merged
Uno-Takashi merged 1 commit into
mainfrom
chore/modernize-review
Jun 30, 2026
Merged

chore: serialization/pandas modernization, settings hardening, race fixes#187
Uno-Takashi merged 1 commit into
mainfrom
chore/modernize-review

Conversation

@Uno-Takashi

Copy link
Copy Markdown
Member

概要

コードレビューに基づくモダナイズとバグ/競合修正。WebSocket プロトコルのワイヤ形式は不変です。

変更点

モダナイズ

  • WebSocket 送信を pydantic v2 model_dump(mode="json") に統一。グローバルな
    json.JSONEncoder.default モンキーパッチと冗長な json.loads(json.dumps(...))
    往復を全廃(出力は従来とバイト単位で同一)。
  • 依存から pandas を撤去し、日次ギャップ埋めを純 Python のループに置換
    uv.lock から numpy/pytz/tzdata 等も削減)。

バグ / 競合修正

  • leave_party のホスト委譲を select_for_updateアトミック化。旧実装は
    「次ホスト選定 → 昇格」の await 境界で当人が離脱すると論理削除済みの行を昇格させ、
    さらに .earliest() が空室時に DoesNotExist を投げて 1011 close を招いていた。
  • 潜在的に無限再帰しうる未使用の uuid_json_encoder を削除。
  • LogicalDeletionMixin.delete() が Django 標準の (count, {label: count}) を返すよう修正。
    consumer の冗長な二重 save() と、未使用の database_delete_room を削除。
  • テスト: database_sync_to_async ヘルパの await 漏れを修正(アサートがコルーチン
    オブジェクトに対する no-op になっており、"never awaited" 警告も出ていた)。

セキュリティ

  • DEBUG 時の ALLOWED_HOSTS += ["*"] を明示的な dev 許可リストへ変更
    (DEBUG=1 のまま本番に出た場合の Host ヘッダインジェクションを防止)。

検証

  • ruff check / ruff format --check: クリーン
  • python manage.py makemigrations --check: 変更なし
  • pytest: 26 passed("never awaited" 警告も解消)

- streamer: serialize WS payloads via pydantic model_dump(mode="json"), dropping
  the global json.JSONEncoder monkeypatch and redundant json.loads(json.dumps())
  round-trips (byte-identical wire output). Remove the unused, latently-recursive
  uuid_json_encoder.
- mixins: LogicalDeletionMixin.delete() now returns Django's (count, {label:count})
  tuple; drop the redundant double save() in the consumer and the dead
  database_delete_room method.
- consumers: make host promotion on leave atomic (select_for_update). Fixes a race
  where the chosen next host could leave between selection and promotion; the old
  .earliest() also raised DoesNotExist -> 1011 close when the room emptied.
- settings: replace ALLOWED_HOSTS += ['*'] under DEBUG with an explicit dev
  allowlist (prevents Host-header injection if DEBUG ever ships to prod).
- api: drop the heavy pandas dependency; fill per-day gaps with a plain loop.
- tests: await the database_sync_to_async existence helpers (asserts were no-ops
  on a coroutine and emitted 'never awaited' warnings).
Comment thread streamer/consumers.py
import uuid

from channels.db import database_sync_to_async
from django.db import transaction

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.

🚫 [mypy] reported by reviewdog 🐶
Cannot find implementation or library stub for module named "django.db" [import-not-found]

@Uno-Takashi Uno-Takashi merged commit b66eea1 into main Jun 30, 2026
19 checks passed
@Uno-Takashi Uno-Takashi deleted the chore/modernize-review branch June 30, 2026 05:58
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