From 2508b5aec121e812347a2f9ef72ac0b143dd901c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B5=85=E5=BF=B5?= Date: Sat, 1 Aug 2026 15:23:43 +0800 Subject: [PATCH] fix: live-validation fixes (list-coercion, .env session loading, smoke summary) Cleanup after successful live validation (10 real customers scraped+stored): - app/config.py: add yk_userid / yk_company / yk_cookie Settings fields so existing-session creds can be supplied via .env (bypasses programmatic login for validation with a live browser session). - app/storage/repos.py: coerce list values to ';'-joined strings in _build_column_values. Fixes a real bug: the live friends-list API returns Customer.deptRoute as a LIST, but dept_route is a scalar String(255) column. raw_json still preserves the original list (no data loss). - scripts/smoke_wechat.py: * Revert build_client_from_env to read os.environ ONLY (no get_settings fallbacks), so the monkeypatch.setenv unit tests stay hermetic and the real .env stops leaking in. 3 previously-red tests green again. * Add _push_settings_into_environ() (Settings -> os.environ, real env wins, only fills gaps) and call it as the first line of main(); so the smoke run still honors .env without the pure builder depending on Settings. .env -> Settings -> os.environ (once) -> build_client_from_env. * Keep the await _print_summary fix from validation. - tests/test_repos.py: regression test for list dept_route coercion (stored joined 'A;B', raw_json keeps the list). Co-Authored-By: Claude --- yunke-server/app/config.py | 6 ++++ yunke-server/app/storage/repos.py | 6 ++++ yunke-server/scripts/smoke_wechat.py | 54 ++++++++++++++++++++++++++-- yunke-server/tests/test_repos.py | 36 +++++++++++++++++++ 4 files changed, 99 insertions(+), 3 deletions(-) diff --git a/yunke-server/app/config.py b/yunke-server/app/config.py index b188858..a013cba 100644 --- a/yunke-server/app/config.py +++ b/yunke-server/app/config.py @@ -22,6 +22,12 @@ class Settings(BaseSettings): # Fernet key for encrypting stored credentials. Required for real runs; # empty default lets tests construct Settings without a key. encryption_key: str = "" + + # Existing-session creds (optional; bypasses programmatic login — e.g. for + # validation with a live browser session). Read from .env / environment. + yk_userid: str = "" + yk_company: str = "" + yk_cookie: str = "" # Shared bearer token for the viewing API. Empty (default) -> auth DISABLED # (we trust the network boundary; this is a local/intranet tool). When set, # every API request must present this value via ``Authorization: Bearer diff --git a/yunke-server/app/storage/repos.py b/yunke-server/app/storage/repos.py index 7a59007..beff727 100644 --- a/yunke-server/app/storage/repos.py +++ b/yunke-server/app/storage/repos.py @@ -193,6 +193,12 @@ def _build_column_values( v = _coerce_bool(v) elif col_name in _INT_FIELDS: v = _coerce_int(v) + # Some upstream fields are lists (e.g. Customer.deptRoute is a + # list of dept ids) but map to scalar String columns — join so the + # bind succeeds and the data is still recoverable (raw_json keeps + # the original list). + if isinstance(v, list): + v = ";".join(str(x) for x in v) values[col_name] = v # Preserve the verbatim source record. If the scraper already set raw_json # (it does), that wins; otherwise the input dict itself is the source of diff --git a/yunke-server/scripts/smoke_wechat.py b/yunke-server/scripts/smoke_wechat.py index 13aac01..1af70d4 100644 --- a/yunke-server/scripts/smoke_wechat.py +++ b/yunke-server/scripts/smoke_wechat.py @@ -94,6 +94,10 @@ MAX_MESSAGES = 50 MESSAGE_WINDOW_MS = 7 * 86_400_000 DEFAULT_SMOKE_DB_URL = "sqlite+aiosqlite:///./smoke.db" +# Base URL used when neither ``YUNKE_BASE_URL`` env nor Settings supply one. +# Mirrors the Settings default so build_client_from_env never hands httpx a +# ``None`` base_url (which would raise TypeError at client construction). +DEFAULT_YUNKE_BASE_URL = "https://crm2.yunkecn.com" def _parse_cookie_env(raw: str) -> dict[str, str]: @@ -133,6 +137,12 @@ def _parse_cookie_env(raw: str) -> dict[str, str]: def build_client_from_env() -> YunkeClient | None: """Construct a :class:`YunkeClient` from env vars (pure / network-free). + Reads ``os.environ`` ONLY — never :func:`get_settings`. The smoke script's + ``main()`` calls :func:`_push_settings_into_environ` first so a ``.env`` + file is honored (Settings -> os.environ, once at startup); tests call this + function directly without the push, so they stay a pure function of + ``os.environ`` and can be exercised with ``monkeypatch.setenv``. + Selection rules (in priority order): 1. **Existing-session mode** — if EITHER of ``YK_USERID`` / ``YK_COMPANY`` @@ -148,7 +158,7 @@ def build_client_from_env() -> YunkeClient | None: Pure / network-free — safe to unit-test with ``monkeypatch.setenv``. """ - base_url = os.environ.get("YUNKE_BASE_URL") or get_settings().yunke_base_url + base_url = os.environ.get("YUNKE_BASE_URL", "").strip() or DEFAULT_YUNKE_BASE_URL userid = os.environ.get("YK_USERID", "").strip() company = os.environ.get("YK_COMPANY", "").strip() @@ -180,6 +190,39 @@ def build_client_from_env() -> YunkeClient | None: return None +def _push_settings_into_environ() -> None: + """Honor ``.env`` by pushing non-empty Settings values into ``os.environ``. + + The smoke script is run by a controller that supplies creds via ``.env``, + but :func:`build_client_from_env` reads ``os.environ`` only (so it stays + unit-testable with ``monkeypatch.setenv``). This helper bridges the two: + it reads the process-wide :func:`get_settings` singleton (which loads + ``.env``) and copies each non-empty session/login value into + ``os.environ`` **only if that var is not already set there** — real env + wins, exactly as the original ``os.environ.get(...) or settings.*`` chain + did, but evaluated ONCE at smoke startup instead of inside the pure + builder. + + Synchronous and side-effectful by design; called as the first line of + :func:`main`. Not called by the unit tests, so they remain hermetic. + """ + settings = get_settings() + _ENV_MAP = { + "YUNKE_USERNAME": settings.yunke_username, + "YUNKE_PASSWORD": settings.yunke_password, + "YK_USERID": settings.yk_userid, + "YK_COMPANY": settings.yk_company, + "YK_COOKIE": settings.yk_cookie, + } + for key, value in _ENV_MAP.items(): + if value and not os.environ.get(key): + os.environ[key] = value + # YUNKE_BASE_URL: only push when the setting diverges from the default and + # the env var is not already set (avoids overwriting an explicit real env). + if settings.yunke_base_url and not os.environ.get("YUNKE_BASE_URL"): + os.environ["YUNKE_BASE_URL"] = settings.yunke_base_url + + def _print_usage(stream=sys.stderr) -> None: stream.write( "smoke_wechat: missing auth env vars.\n" @@ -191,6 +234,11 @@ def _print_usage(stream=sys.stderr) -> None: async def main() -> int: """Run the smoke check. Returns a process exit code (0 = success).""" + # Honor .env (via Settings) by lifting non-empty values into os.environ + # BEFORE the pure, os.environ-only builder runs. Real env vars win; this + # only fills gaps. Done once at startup so build_client_from_env stays a + # pure function of os.environ (and stays unit-testable with setenv/delenv). + _push_settings_into_environ() client = build_client_from_env() if client is None: _print_usage() @@ -261,7 +309,7 @@ async def main() -> int: if first_friend is None: print("[smoke] no friends returned; nothing more to do.") - _print_summary(session_maker, customers_upserted, messages_upserted) + await _print_summary(session_maker, customers_upserted, messages_upserted) return 0 print(f"[smoke] customers upserted: {customers_upserted}") @@ -327,7 +375,7 @@ async def main() -> int: ) # 4) Verify rows landed by querying the smoke DB back. - _print_summary(session_maker, customers_upserted, messages_upserted) + await _print_summary(session_maker, customers_upserted, messages_upserted) return 0 finally: await client.aclose() diff --git a/yunke-server/tests/test_repos.py b/yunke-server/tests/test_repos.py index a4ff322..b5a8312 100644 --- a/yunke-server/tests/test_repos.py +++ b/yunke-server/tests/test_repos.py @@ -216,6 +216,42 @@ async def test_upsert_customer_keeps_raw_json(): await engine.dispose() +async def test_upsert_customer_coerces_list_dept_route_to_joined_string(): + """A list ``dept_route`` is stored joined ("A;B"); raw_json keeps the list. + + Regression for the live-validation fix: the real yunke friends-list API + returns ``deptRoute`` as a LIST of dept ids, but ``Customer.dept_route`` is + a scalar String(255) column. Without the list->";"-join coercion in + :func:`_build_column_values` the bind raises (cannot store a list in a + String column). This test pins the coercion: the column holds the joined + scalar, while ``raw_json`` still carries the original list verbatim so no + information is lost. + """ + engine = await _make_engine() + try: + SessionLocal = async_sessionmaker(engine, expire_on_commit=False) + async with SessionLocal() as session: + rec = { + "friend_id": "wxid_list", + "channel": "wechat", + "dept_route": ["A", "B"], # list upstream shape + } + await upsert_customer(session, rec) + await session.commit() + + fetched = ( + await session.execute( + sa.select(Customer).where(Customer.friend_id == "wxid_list") + ) + ).scalar_one() + # Stored as the joined scalar string. + assert fetched.dept_route == "A;B" + # The verbatim list is still recoverable from raw_json. + assert fetched.raw_json["dept_route"] == ["A", "B"] + finally: + await engine.dispose() + + async def test_upsert_customers_batch_returns_count(): """Batch upsert returns the number of records processed.""" engine = await _make_engine()