From 1df1c983854b8ddc452b8e86ac872de62c995ef8 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 02:56:21 +0000 Subject: [PATCH 1/3] =?UTF-8?q?fix(files):=20"=D7=A0=D7=95=D7=A6=D7=A8"=20?= =?UTF-8?q?=D7=A9=D7=99=D7=99=D7=9A=20=D7=9C=D7=A7=D7=95=D7=91=D7=A5,=20?= =?UTF-8?q?=D7=9C=D7=90=20=D7=9C=D7=92=D7=A8=D7=A1=D7=94=20=E2=80=94=20?= =?UTF-8?q?=D7=94=D7=95=D7=A8=D7=A9=D7=AA=20created=5Fat=20=D7=91=D7=A2?= =?UTF-8?q?=D7=A8=D7=99=D7=9B=D7=94?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit כל עריכת תוכן יוצרת מסמך גרסה חדש, וה-UI מציג את הגרסה האחרונה — ולכן "נוצר" קפץ לתאריך העריכה בכל שמירה, בעוד שעריכת תיאור (עדכון-במקום ב-quick-update) שימרה אותו. תאריך הגרסה ממשיך לחיות ב-updated_at שלה, וההיסטוריה מציגה אותו. ההורשה בשני רבדים: - save_code_snippet: created_at מצטרף לבלוק שכבר מעתיק מהגרסה הקודמת מועדפים ונעיצה. דרכו מכוסים הבוט (ערוך קוד/הערה), MCP, שחזור מגיבוי וייבוא GitHub. - חמשת מסלולי insert_one הישירים בוובאפ — restore, ‏/edit, ‏shared/save, ‏/upload על שם קיים, וייצוא סיפור — יורשים מ-prev שכבר נשלף שם, בלי שאילתה נוספת. אומת שאף אחת מהשאילתות לא מקרינה החוצה את created_at. מיגרציה לקבצים קיימים — עמוד אדמין, לא shell: ‏/admin/migrations/created-at עם dry-run לקריאה בלבד (כמה ייפגעו, מתוך כמה, טבלת דוגמאות), החלה שנפתחת רק אחרי dry-run, ספירת אימות בקריאה חוזרת בסיום, ו-audit לשני המסלולים. הלוגיקה ב-services/created_at_migration.py כדי שהטסטים יריצו אותה ישירות. ההחלה נוגעת בגרסה האחרונה בלבד — תאריכי ההיסטוריה נשארים. תיקוני UI: ‏quick-update מחזיר updated_at_display וה"עודכן" במסך מתרענן בלי כניסה מחודשת לקובץ; כשעודכן==נוצר (בדיוק הפורמט המוצג, דקות) מוצג רק "נוצר" — בעמוד הקובץ ובכרטיס ברשימה. MCP: ‏save_file על שם קיים נדחה עם error=file_exists והפניה ל-edit_file/ append_file — עדכון דורש update_existing=true מפורש. ה-upsert השקט נשך: שמירת תוכן חדש על שם שמחזיק משהו אחר מסתירה את הישן מחיפוש ומהתצוגה, ששניהם קוראים גרסה אחרונה בלבד. עודכנו README, ‏mcp-server.rst והטסטים. אימות: probe מול mongod אמיתי שנפל על הקוד הישן (created נפרד ב-1.2s) ועובר עם התיקון; 17/17 בדפדפן — "נוצר" שורד עריכת תוכן ותיאור, "עודכן" מתרענן חי, היסטוריה מחזירה את כל הגרסאות, ושחזור יוצר גרסה שנושאת את ה"נוצר" המקורי; עמוד המיגרציה הופעל מקצה לקצה עם קובץ שבור אמיתי (2026 ← 2023, remaining=0); שער ה-MCP אומת בקריאה חוזרת — סירוב בלי כתיבה, ואז גרסה 2 יורשת; שתי מוטציות נתפסו (הסרת ההורשה, והחלה לפי שם במקום לפי מזהה הגרסה האחרונה). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019YULCppaQRPYN1RgeY6NBu --- database/repository.py | 10 ++ docs/mcp-server.rst | 9 +- mcp_server/README.md | 2 +- mcp_server/backend.py | 21 +++ mcp_server/handlers.py | 2 + mcp_server/server.py | 13 +- services/created_at_migration.py | 124 ++++++++++++++ tests/test_created_at_inheritance_mongo.py | 161 ++++++++++++++++++ tests/test_mcp_backend.py | 18 +- tests/test_mcp_handlers.py | 2 +- webapp/app.py | 69 +++++++- .../templates/admin_migration_created_at.html | 83 +++++++++ webapp/templates/files.html | 3 + webapp/templates/view_file.html | 15 +- 14 files changed, 513 insertions(+), 19 deletions(-) create mode 100644 services/created_at_migration.py create mode 100644 tests/test_created_at_inheritance_mongo.py create mode 100644 webapp/templates/admin_migration_created_at.html diff --git a/database/repository.py b/database/repository.py index 78621568a..4c0f06e63 100644 --- a/database/repository.py +++ b/database/repository.py @@ -211,6 +211,16 @@ def save_code_snippet(self, snippet: CodeSnippet) -> bool: existing = self._fetch_latest_version(snippet.user_id, snippet.file_name) if existing: snippet.version = existing['version'] + 1 + # ``created_at`` שייך לקובץ הלוגי, לא לגרסה. בלי ההורשה כאן, + # כל עריכת תוכן הייתה מזיזה את "נוצר" בממשק לתאריך העריכה — + # בעוד שעריכת תיאור (עדכון-במקום) משמרת אותו. תאריך הגרסה + # עצמה ממשיך לחיות ב-``updated_at`` שלה, וההיסטוריה מציגה + # אותו. ``or`` ולא השמה עיוורת: מסמך ותיק בלי השדה לא אמור + # לאפס את ברירת המחדל שהמודל כבר קבע. + try: + snippet.created_at = existing.get('created_at') or snippet.created_at + except Exception: + pass # שמור סטטוס מועדפים מהגרסה הקודמת אם לא סופק מפורשות try: prev_is_fav = bool(existing.get('is_favorite', False)) diff --git a/docs/mcp-server.rst b/docs/mcp-server.rst index 265c28f90..eae4c34c3 100644 --- a/docs/mcp-server.rst +++ b/docs/mcp-server.rst @@ -56,9 +56,12 @@ OAuth 2.1) וגם מול **Claude Code / Claude Desktop** (טוקן אישי). * - ``codekeeper_get_file`` - תוכן מלא של קובץ (לפי שם/מזהה, אופציונלית גרסה) * - ``codekeeper_save_file`` - - **כתיבה:** יצירה/עדכון קובץ (גרסה חדשה; בכפוף למגבלת - ``MAX_CODE_SIZE`` — ברירת מחדל 100K תווים, ניתנת להגדלה בקונפיג). - דורש ``write`` + - **כתיבה:** יצירת קובץ חדש. שם קיים נדחה עם ``error=file_exists`` — + עדכון דורש ``update_existing=true`` מפורש (נשמר כגרסה חדשה; + הישנות נשארות בהיסטוריה, אך החיפוש והתצוגה מציגים רק את + האחרונה). לשינוי חלקי — ``edit_file``/``append_file``. בכפוף + למגבלת ``MAX_CODE_SIZE`` (ברירת מחדל 100K תווים, ניתנת להגדלה + בקונפיג). דורש ``write`` * - ``codekeeper_edit_file`` - **כתיבה:** מצא-והחלף מדויק בקובץ קיים (``old_string`` → ``new_string``) — בלי לשלוח את כל הקובץ. נשמר כגרסה חדשה. דורש diff --git a/mcp_server/README.md b/mcp_server/README.md index bc040deff..6a24720b1 100644 --- a/mcp_server/README.md +++ b/mcp_server/README.md @@ -29,7 +29,7 @@ Claude Desktop** (טוקן אישי). קריאה זמינה תמיד; **כתיב | `codekeeper_list_files` | רשימת קבצים (מטא‑דאטה בלבד), עם עימוד | | `codekeeper_search_code` | חיפוש טקסט בקוד → מטא‑דאטה של קבצים תואמים | | `codekeeper_get_file` | תוכן מלא של קובץ לפי `file_name` או `file_id` (אופציונלי: גרסה) | -| `codekeeper_save_file` | **כתיבה:** יצירה/עדכון קובץ לפי `file_name` (גרסה חדשה, לא דורס; בכפוף ל‑`MAX_CODE_SIZE`, ברירת מחדל 100K תווים וניתן להגדלה). דורש `write` | +| `codekeeper_save_file` | **כתיבה:** יצירת קובץ חדש לפי `file_name`. שם קיים נדחה עם `error=file_exists` — עדכון דורש `update_existing=true` מפורש (נשמר כגרסה חדשה; הישנות בהיסטוריה, אך החיפוש והתצוגה מציגים רק את האחרונה). לשינוי חלקי עדיף `edit_file`/`append_file`. בכפוף ל‑`MAX_CODE_SIZE` (ברירת מחדל 100K תווים, ניתן להגדלה). דורש `write` | | `codekeeper_edit_file` | **כתיבה:** מצא‑והחלף מדויק (`old_string`→`new_string`, אופציונלית `replace_all`) בלי לשלוח את כל הקובץ; גרסה חדשה, משמר שפה/תיאור/תגיות. דורש `write` | | `codekeeper_append_file` | **כתיבה:** הוספת טקסט לסוף קובץ קיים (מוסיף שורת‑הפרדה אם צריך); גרסה חדשה. דורש `write` | | `codekeeper_list_versions` | היסטוריית גרסאות של קובץ (מטא‑דאטה) | diff --git a/mcp_server/backend.py b/mcp_server/backend.py index c3babbaad..8cc47ae2a 100644 --- a/mcp_server/backend.py +++ b/mcp_server/backend.py @@ -355,6 +355,7 @@ def save_file( programming_language: str, description: str = "", tags: list[str] | None = None, + update_existing: bool = False, ) -> dict[str, Any]: """Create a new file or append a new version of an existing one. @@ -369,6 +370,26 @@ def save_file( dbm = self._require_dbm() # Captured before the save so we can report create vs. update honestly. prev = _latest_fresh(dbm, user_id, file_name) + # Updating an existing name must be an explicit choice. The silent + # upsert bit for real: saving new content under a name that already + # holds something else hides the old content from search and from the + # normal view (both read the latest version only), leaving it + # reachable only through the version history. Full-content rewrites + # stay possible via update_existing=True; partial changes belong to + # edit_file/append_file. + if prev is not None and not update_existing: + return { + "ok": False, + "error": "file_exists", + "file_name": file_name, + "latest_version": int(prev.get("version") or 0), + "hint": ( + "A file with this name already exists. To change part of it " + "use codekeeper_edit_file or codekeeper_append_file; to " + "replace its content entirely, pass update_existing=true; " + "or pick a different file_name." + ), + } ok = bool( dbm.save_code_snippet( CodeSnippet( diff --git a/mcp_server/handlers.py b/mcp_server/handlers.py index f09a72b88..9718bf1ff 100644 --- a/mcp_server/handlers.py +++ b/mcp_server/handlers.py @@ -123,6 +123,7 @@ def save_file( code: str, language: str | None = None, description: str = "", + update_existing: bool = False, ) -> dict[str, Any]: """Validate + normalize a save request, then delegate to the backend. @@ -157,6 +158,7 @@ def save_file( code=code, programming_language=lang, description=(description or "").strip(), + update_existing=bool(update_existing), ) diff --git a/mcp_server/server.py b/mcp_server/server.py index 1bf348542..7aa89efbd 100644 --- a/mcp_server/server.py +++ b/mcp_server/server.py @@ -33,7 +33,7 @@ "Access the current user's private code files and collections stored in " "CodeKeeper. Use codekeeper_search_code / codekeeper_list_files to find files " "(metadata only), and codekeeper_get_file to read full contents. Use " - "codekeeper_save_file to create or update a file, and prefer " + "codekeeper_save_file to create a file (updating an existing name requires update_existing=true), and prefer " "codekeeper_edit_file / codekeeper_append_file to change part of an existing " "file without resending all of it (write tools require write permission). " "Sticky notes live on a file, on a board (a surface that belongs to no file), or " @@ -232,8 +232,13 @@ def get_file( @mcp.tool( name="codekeeper_save_file", description=( - "Create a new file or update an existing one by file_name (saved as a new, " - "non-destructive version — old versions are kept). Requires write permission." + "Create a NEW file by file_name. If the name already exists the call " + "is refused with error=file_exists — updating an existing file must " + "be explicit: pass update_existing=true to replace its content as a " + "new non-destructive version (old versions are kept in history, but " + "search and the normal view show only the latest), or use " + "codekeeper_edit_file / codekeeper_append_file for partial changes. " + "Requires write permission." ), annotations=_WRITE_TOOL, ) @@ -243,6 +248,7 @@ def save_file( code: str, language: str | None = None, description: str = "", + update_existing: bool = False, ) -> dict: require_write(ctx) # reject a read-only token before touching anything return handlers.save_file( @@ -252,6 +258,7 @@ def save_file( code=code, language=language, description=description, + update_existing=update_existing, ) @mcp.tool( diff --git a/services/created_at_migration.py b/services/created_at_migration.py new file mode 100644 index 000000000..488526872 --- /dev/null +++ b/services/created_at_migration.py @@ -0,0 +1,124 @@ +"""מיגרציית ``created_at`` — "נוצר" של קובץ שייך לקובץ הלוגי, לא לגרסה. + +הרקע: כל עריכת תוכן יוצרת מסמך גרסה חדש, ועד התיקון ההורשה ב- +``save_code_snippet`` השדה ``created_at`` שלו נקבע לרגע העריכה. ה-UI מציג +תמיד את מסמך הגרסה האחרונה, ולכן "נוצר" זז בכל עריכה. התיקון קדימה חי +בקוד; המיגרציה כאן מיישרת את המצב הקיים — בלעדיה קובץ ותיק יוריש הלאה +את התאריך השגוי שכבר יש לו. + +מה היא עושה: לכל ``(user_id, file_name)`` פעיל, קובעת לגרסה **האחרונה +בלבד** ``created_at = המוקדם מבין כל גרסאות הקובץ``. גרסאות ישנות אינן +נגועות — תאריכי ההיסטוריה שלהן נשארים כמות שהם. + +מופעלת מעמוד האדמין ``/admin/migrations/created-at`` בלבד: dry-run +לקריאה בלבד, ואחריו החלה מפורשת. שני המסלולים כותבים מסמך audit +ל-``migration_audit`` כדי שיישאר תיעוד למה שנבדק ומה הוחל. +""" +from __future__ import annotations + +from datetime import datetime, timezone +from typing import Any, Dict, List + +AUDIT_COLLECTION = "migration_audit" +MIGRATION_NAME = "created_at_from_first_version" + +# צינור משותף לשני המסלולים: אותו חישוב בדיוק ב-dry-run ובהחלה, כדי שמה +# שהוצג הוא מה שיוחל. שינוי בצנרת של אחד בלי השני הוא באג, לא גמישות. +_PIPELINE: List[Dict[str, Any]] = [ + {"$match": {"is_active": True}}, + {"$sort": {"user_id": 1, "file_name": 1, "version": -1}}, + { + "$group": { + "_id": {"user_id": "$user_id", "file_name": "$file_name"}, + "latest_id": {"$first": "$_id"}, + "latest_created": {"$first": "$created_at"}, + "earliest_created": {"$min": "$created_at"}, + "versions": {"$sum": 1}, + } + }, + # רק קבצים שבהם יש מה לתקן: התאריך של הגרסה האחרונה מאוחר מהמוקדם. + {"$match": {"$expr": {"$gt": ["$latest_created", "$earliest_created"]}}}, +] + + +def _affected(db) -> List[Dict[str, Any]]: + return list(db.code_snippets.aggregate(_PIPELINE, allowDiskUse=True)) + + +def _write_audit(db, doc: Dict[str, Any]) -> None: + try: + db[AUDIT_COLLECTION].insert_one(doc) + except Exception: + # audit הוא תיעוד, לא שער: כישלון בו לא מפיל את המיגרציה עצמה. + pass + + +def dry_run(db, *, sample_size: int = 20) -> Dict[str, Any]: + """קריאה בלבד. מחזיר כמה קבצים ייפגעו, מתוך כמה, ודוגמאות.""" + affected = _affected(db) + total = 0 + try: + agg = list( + db.code_snippets.aggregate( + [ + {"$match": {"is_active": True}}, + {"$group": {"_id": {"user_id": "$user_id", "file_name": "$file_name"}}}, + {"$count": "n"}, + ] + ) + ) + total = int(agg[0]["n"]) if agg else 0 + except Exception: + total = 0 + samples = [ + { + "file_name": row["_id"]["file_name"], + "user_id": row["_id"]["user_id"], + "versions": row["versions"], + "current_created": row["latest_created"], + "new_created": row["earliest_created"], + } + for row in affected[:sample_size] + ] + result = { + "migration": MIGRATION_NAME, + "mode": "dry_run", + "affected_count": len(affected), + "total_files": total, + "samples": samples, + "ran_at": datetime.now(timezone.utc), + } + _write_audit(db, dict(result)) + return result + + +def apply(db) -> Dict[str, Any]: + """מחיל, ואז **מאמת בקריאה חוזרת** — ערך ההחזרה של הכתיבה אינו אימות.""" + from pymongo import UpdateOne + + affected = _affected(db) + ops = [ + UpdateOne( + {"_id": row["latest_id"]}, + {"$set": {"created_at": row["earliest_created"]}}, + ) + for row in affected + ] + modified = 0 + if ops: + res = db.code_snippets.bulk_write(ops, ordered=False) + modified = int(getattr(res, "modified_count", 0) or 0) + + # אימות: אחרי ההחלה, כמה קבצים עדיין עומדים בתנאי הפער. אמור להיות 0. + remaining = len(_affected(db)) + + result = { + "migration": MIGRATION_NAME, + "mode": "apply", + "planned": len(affected), + "modified": modified, + "remaining_after": remaining, + "ran_at": datetime.now(timezone.utc), + } + _write_audit(db, dict(result)) + return result diff --git a/tests/test_created_at_inheritance_mongo.py b/tests/test_created_at_inheritance_mongo.py new file mode 100644 index 000000000..b5a84d4cc --- /dev/null +++ b/tests/test_created_at_inheritance_mongo.py @@ -0,0 +1,161 @@ +""""נוצר" של קובץ שורד עריכה — מול **מונגו אמיתי**, באותה תבנית של +``test_note_boards_mongo``: רץ כש-``MONGODB_URL`` נגיש (ב-CI תמיד), מדלג +מקומית. + +הרקע: כל עריכת תוכן יוצרת מסמך גרסה חדש, וה-UI מציג את הגרסה האחרונה. +בלי הורשת ``created_at`` ב-``save_code_snippet``, "נוצר" קפץ לתאריך +העריכה בכל שמירה — בעוד שעריכת תיאור (עדכון-במקום) שימרה אותו. ההורשה +מצטרפת לבלוק שכבר מעתיק מועדפים ונעיצה מהגרסה הקודמת. + +כל בדיקה כאן הופלה על הקוד שלפני התיקון (ריצת בקרה מתועדת ב-PR) — +בדיקה שלא הורצה על הקוד הישן אינה ראיה. +""" + +from __future__ import annotations + +import os +import uuid +from datetime import datetime, timedelta, timezone + +import pytest + +pymongo = pytest.importorskip("pymongo") + +from pymongo.errors import ServerSelectionTimeoutError # noqa: E402 + +_TEST_DB_PREFIX = "codebot_created_it_" +_MONGO_URL = os.environ.get("MONGODB_URL", "").strip() + + +def _server_is_reachable(url: str) -> bool: + try: + client = pymongo.MongoClient(url, serverSelectionTimeoutMS=2000, tz_aware=True, tzinfo=timezone.utc) + client.admin.command("ping") + client.close() + return True + except (ServerSelectionTimeoutError, Exception): + return False + + +pytestmark = pytest.mark.skipif( + not _MONGO_URL or not _server_is_reachable(_MONGO_URL), + reason="דורש MONGODB_URL עם שרת מונגו נגיש (קיים ב-CI, לא בהכרח מקומית)", +) + + +@pytest.fixture +def mongo_db(): + name = f"{_TEST_DB_PREFIX}{uuid.uuid4().hex[:12]}" + client = pymongo.MongoClient(_MONGO_URL, tz_aware=True, tzinfo=timezone.utc) + db = client[name] + try: + yield db + finally: + # סורג בטיחות: מוחקים רק מסד שנוצר כאן + assert name.startswith(_TEST_DB_PREFIX), f"סירוב למחוק מסד שאינו של הבדיקות: {name}" + try: + client.drop_database(name) + finally: + client.close() + + +@pytest.fixture +def repo(mongo_db): + """``Repository`` אמיתי מעל המסד הזמני. + + ``Repository`` נוגע במנהל רק דרך ``manager.collection``, ולכן שים + (shim) דק מספיק — ובלי סטאב לכתיבות: ה-insert וה-find הם של מונגו. + """ + from database.repository import Repository + + class _Mgr: + collection = mongo_db.code_snippets + db = mongo_db + + return Repository(_Mgr()) + + +def _snippet(name: str, code: str, **kw): + from database.models import CodeSnippet + + return CodeSnippet(user_id=42, file_name=name, code=code, programming_language="python", **kw) + + +def test_new_version_inherits_created_at(repo, mongo_db): + """גרסה 2 נולדת עם ה"נוצר" של גרסה 1 — ועם "עודכן" משלה.""" + assert repo.save_code_snippet(_snippet("a.py", "v1")) + v1 = mongo_db.code_snippets.find_one({"file_name": "a.py", "version": 1}) + assert v1 is not None + + assert repo.save_code_snippet(_snippet("a.py", "v2")) + v2 = mongo_db.code_snippets.find_one({"file_name": "a.py", "version": 2}) + assert v2 is not None + + assert v2["created_at"] == v1["created_at"], "עריכה אינה לידה מחדש" + assert v2["updated_at"] > v1["updated_at"], "תאריך הגרסה חי ב-updated_at" + + +def test_inheritance_survives_a_chain_of_edits(repo, mongo_db): + """שרשרת: גם גרסה 4 נושאת את ה"נוצר" של גרסה 1, לא של קודמתה בלבד.""" + for body in ("v1", "v2", "v3", "v4"): + assert repo.save_code_snippet(_snippet("chain.py", body)) + docs = {d["version"]: d for d in mongo_db.code_snippets.find({"file_name": "chain.py"})} + assert len(docs) == 4 + origin = docs[1]["created_at"] + assert all(docs[v]["created_at"] == origin for v in (2, 3, 4)) + + +def test_legacy_version_without_created_at_does_not_erase_default(repo, mongo_db): + """מסמך ותיק בלי ``created_at`` — הגרסה החדשה לא יורשת ריק. + + זה ה-``or`` בהורשה: ירושה של None הייתה משאירה את הגרסה החדשה בלי + תאריך בכלל, גרוע מהבאג המקורי. + """ + mongo_db.code_snippets.insert_one( + {"user_id": 42, "file_name": "legacy.py", "code": "old", "programming_language": "python", + "version": 1, "is_active": True, "updated_at": datetime.now(timezone.utc)} + ) + assert repo.save_code_snippet(_snippet("legacy.py", "new")) + v2 = mongo_db.code_snippets.find_one({"file_name": "legacy.py", "version": 2}) + assert isinstance(v2.get("created_at"), datetime) + + +def test_migration_dry_run_reads_only_and_apply_fixes_latest_only(mongo_db): + """שירות המיגרציה: dry-run לא כותב; ההחלה מיישרת רק את האחרונה.""" + from services import created_at_migration as mig + + utc = timezone.utc + t0 = datetime(2024, 1, 1, tzinfo=utc) + t1 = t0 + timedelta(days=100) + t2 = t0 + timedelta(days=500) + for v, t in ((1, t0), (2, t1), (3, t2)): + mongo_db.code_snippets.insert_one( + {"user_id": 1, "file_name": "old.py", "version": v, + "created_at": t, "updated_at": t, "is_active": True} + ) + + d = mig.dry_run(mongo_db) + assert d["affected_count"] == 1 and d["total_files"] == 1 + # dry-run אינו כותב — אימות בקריאה חוזרת, לא בערך ההחזרה + latest = mongo_db.code_snippets.find_one({"file_name": "old.py", "version": 3}) + assert latest["created_at"] == t2 + + a = mig.apply(mongo_db) + assert a == {**a, "planned": 1, "modified": 1, "remaining_after": 0} + latest = mongo_db.code_snippets.find_one({"file_name": "old.py", "version": 3}) + assert latest["created_at"] == t0, "האחרונה קיבלה את המוקדם" + v2 = mongo_db.code_snippets.find_one({"file_name": "old.py", "version": 2}) + assert v2["created_at"] == t1, "גרסה ישנה נגועה — ההיסטוריה שובשה" + + # אידמפוטנטי: החלה שנייה לא מוצאת מה לתקן + again = mig.apply(mongo_db) + assert again["planned"] == 0 and again["modified"] == 0 + + +def test_migration_audit_records_both_modes(mongo_db): + from services import created_at_migration as mig + + mig.dry_run(mongo_db) + mig.apply(mongo_db) + modes = [d["mode"] for d in mongo_db[mig.AUDIT_COLLECTION].find()] + assert modes == ["dry_run", "apply"] diff --git a/tests/test_mcp_backend.py b/tests/test_mcp_backend.py index 5e5f6956f..c1db1a11b 100644 --- a/tests/test_mcp_backend.py +++ b/tests/test_mcp_backend.py @@ -160,11 +160,27 @@ def test_save_file_creates_new_file(): assert dbm.saved.programming_language == "python" and dbm.saved.description == "hi" -def test_save_file_updates_existing_bumps_version(): +def test_save_file_refuses_existing_without_explicit_flag(): + """שם קיים בלי ``update_existing`` — סירוב מוסבר, בלי כתיבה. + + ה-upsert השקט נשך: שמירת תוכן חדש על שם שכבר מחזיק משהו אחר מסתירה + את התוכן הישן מחיפוש ומהתצוגה (שניהם קוראים גרסה אחרונה בלבד). + """ dbm = _FakeDbManager(files=[{"_id": "a", "file_name": "x.py", "version": 1}]) out = ProductionBackend(db_manager=dbm).save_file( 7, file_name="x.py", code="v2", programming_language="python" ) + assert out["ok"] is False and out["error"] == "file_exists" + assert out["latest_version"] == 1 + assert "edit_file" in out["hint"] and "update_existing" in out["hint"] + assert dbm.saved is None # הסירוב קרה לפני כל כתיבה + + +def test_save_file_updates_existing_bumps_version(): + dbm = _FakeDbManager(files=[{"_id": "a", "file_name": "x.py", "version": 1}]) + out = ProductionBackend(db_manager=dbm).save_file( + 7, file_name="x.py", code="v2", programming_language="python", update_existing=True + ) assert out["ok"] is True and out["created"] is False assert out["file"]["version"] == 2 # append-only: new version, not overwrite diff --git a/tests/test_mcp_handlers.py b/tests/test_mcp_handlers.py index 2cf92e8c5..c49b35102 100644 --- a/tests/test_mcp_handlers.py +++ b/tests/test_mcp_handlers.py @@ -35,7 +35,7 @@ def get_collection_items(self, user_id, *, collection_id, page, per_page, folder self.calls.append(("items", user_id, collection_id, page, per_page, folder)) return {} - def save_file(self, user_id, *, file_name, code, programming_language, description): + def save_file(self, user_id, *, file_name, code, programming_language, description, update_existing=False): self.calls.append(("save", user_id, file_name, code, programming_language, description)) return {"ok": True, "created": True, "file": {"file_name": file_name, "version": 1}} diff --git a/webapp/app.py b/webapp/app.py index 1fe57ad25..752682eb1 100644 --- a/webapp/app.py +++ b/webapp/app.py @@ -5869,6 +5869,45 @@ def admin_config_inspector_page(): ) +@app.route('/admin/migrations/created-at', methods=['GET', 'POST']) +@admin_required +def admin_migration_created_at(): + """מיגרציית "נוצר": dry-run והחלה, מהדפדפן בלבד. + + "נוצר" של קובץ שייך לקובץ הלוגי ולא לגרסה. הקוד כבר מוריש את + ``created_at`` קדימה בכל גרסה חדשה; העמוד הזה מיישר את הקבצים + שנוצרו לפני התיקון. ההחלה מותרת רק אחרי ש-dry-run הוצג — הטופס + נושא את מונה ה-dry-run כדי שההחלה תמיד תתייחס למה שנצפה. + """ + from services import created_at_migration as _mig + + db = get_db() + result = None + error = None + if request.method == 'POST': + action = (request.form.get('action') or '').strip() + try: + if action == 'dry_run': + result = _mig.dry_run(db) + elif action == 'apply': + # ההחלה דורשת שה-dry-run רץ באותו טופס; בלי זה — סירוב. + if (request.form.get('dry_run_seen') or '') != '1': + error = 'יש להריץ dry-run לפני החלה.' + else: + result = _mig.apply(db) + else: + error = 'פעולה לא מוכרת.' + except Exception as exc: + logger.exception('created_at migration failed: %s', exc) + error = 'המיגרציה נכשלה — ראו לוגים.' + if result: + # תאריכים לתצוגה, בלי לגעת בערכים שנשמרו ל-audit + for row in result.get('samples', []) or []: + row['current_created_disp'] = format_datetime_display(row.get('current_created')) + row['new_created_disp'] = format_datetime_display(row.get('new_created')) + return render_template('admin_migration_created_at.html', result=result, error=error) + + @app.route('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/admin/cache-inspector') def admin_cache_inspector_page(): """ @@ -13447,7 +13486,8 @@ def api_file_quick_update(file_id): return jsonify({'ok': False, 'error': 'הקובץ לא נמצא'}), 404 data = request.get_json() or {} - updates = {'updated_at': datetime.now(timezone.utc)} + now_updated = datetime.now(timezone.utc) + updates = {'updated_at': now_updated} if 'description' in data: desc = (data.get('description') or '').strip()[:500] @@ -13479,9 +13519,12 @@ def api_file_quick_update(file_id): return jsonify({ 'ok': True, - 'updated_fields': list(updates.keys()) + 'updated_fields': list(updates.keys()), + # לרענון חי של "עודכן" במסך: בלי זה הערך הישן נשאר מוצג עד + # כניסה מחודשת לקובץ, למרות שהשמירה כבר קרתה. + 'updated_at_display': format_datetime_display(now_updated), }) - + except Exception as e: logger.exception(f"Error in quick update: {e}") return jsonify({'ok': False, 'error': 'שגיאה בעדכון'}), 500 @@ -13771,7 +13814,10 @@ def api_restore_file_version(file_id): 'description': description, 'tags': tags, 'version': next_version, - 'created_at': now, + # "נוצר" של הקובץ הלוגי, לא של הגרסה המשוחזרת: יורש מהשרשרת החיה + # (הגרסה האחרונה), ולא מ-``version_doc`` הישן — כדי שהשחזור לא + # ידרוס תיקון עתידי של השרשרת בערך ארכיאולוגי. + 'created_at': (latest_doc or {}).get('created_at') or file_doc.get('created_at') or now, 'updated_at': now, 'is_active': True, 'is_favorite': bool((latest_doc or {}).get('is_favorite', file_doc.get('is_favorite', False))), @@ -14613,7 +14659,10 @@ def _normalize_lang(name: str) -> str: 'description': description, 'tags': tags, 'version': version, - 'created_at': now, + # "נוצר" יורש מהגרסה הקודמת — עריכה אינה לידה מחדש. + # ``file`` הוא הנפילה לשינוי שם, שאז ``prev`` נשלף + # לפי השם החדש ויכול להיות ריק. + 'created_at': (prev or file or {}).get('created_at') or now, 'updated_at': now, 'is_active': True, } @@ -15823,7 +15872,9 @@ def api_save_shared_file(): 'description': description, 'tags': tags, 'version': version, - 'created_at': now_utc, + # שמירת מדריך על שם קיים היא גרסה חדשה של אותו קובץ — "נוצר" + # יורש. לקובץ חדש ``prev`` ריק והתאריך הוא של עכשיו. + 'created_at': (prev or {}).get('created_at') or now_utc, 'updated_at': now_utc, 'is_active': True, } @@ -16531,7 +16582,8 @@ def _normalize_lang(name: str) -> str: 'description': description, 'tags': final_tags, 'version': version, - 'created_at': now, + # העלאה על שם קיים היא גרסה חדשה — "נוצר" יורש. + 'created_at': (prev or {}).get('created_at') or now, 'updated_at': now, 'is_active': True, } @@ -19008,7 +19060,8 @@ def _persist_story_markdown_file( 'description': description[:400], 'tags': dedup_tags, 'version': version, - 'created_at': now, + # ייצוא חוזר של אותו סיפור דורס את אותו שם קובץ — גרסה, לא לידה. + 'created_at': (prev or {}).get('created_at') or now, 'updated_at': now, 'is_active': True, } diff --git a/webapp/templates/admin_migration_created_at.html b/webapp/templates/admin_migration_created_at.html new file mode 100644 index 000000000..09321700b --- /dev/null +++ b/webapp/templates/admin_migration_created_at.html @@ -0,0 +1,83 @@ +{% extends "base.html" %} + +{% block title %}מיגרציית "נוצר"{% endblock %} + +{% block extra_css %} + +{% endblock %} + +{% block content %} +
+

🗓️ מיגרציית "נוצר"

+ +
+

"נוצר" של קובץ שייך לקובץ, לא לגרסה. הקוד כבר משמר אותו בעריכות חדשות; העמוד הזה מיישר קבצים שנערכו לפני התיקון: הגרסה האחרונה של כל קובץ תקבל את התאריך המוקדם ביותר מבין גרסאותיו. גרסאות ישנות אינן נגועות.

+

‏dry-run לקריאה בלבד — לא משנה דבר. ההחלה נפתחת רק אחרי שצפית בתוצאה שלו. שתי הפעולות נרשמות ל-audit.

+
+ + {% if error %} +
⚠️ {{ error }}
+ {% endif %} + +
+
+ + + {% if result and result.mode == 'dry_run' %} + + {% endif %} +
+
+ + {% if result and result.mode == 'dry_run' %} +
+

ייפגעו {{ result.affected_count }} קבצים מתוך {{ result.total_files }}.

+ {% if result.samples %} +
+ + + + {% for s in result.samples %} + + + + + + + {% endfor %} + +
קובץגרסאות"נוצר" הנוכחי"נוצר" אחרי
{{ s.file_name }}{{ s.versions }}{{ s.current_created_disp }}{{ s.new_created_disp }}
+
+ {% else %} +

אין מה לתקן — כל הקבצים כבר עקביים. 🎉

+ {% endif %} +
+ {% endif %} + + {% if result and result.mode == 'apply' %} +
+

תוכננו {{ result.planned }} · עודכנו בפועל {{ result.modified }} · נותרו לא-תואמים אחרי אימות: {{ result.remaining_after }}

+ {% if result.remaining_after == 0 %} +

✅ האימות בקריאה חוזרת עבר — אין פערים שנותרו.

+ {% else %} +

⚠️ נותרו פערים. הרץ dry-run שוב לראות מה נשאר, ואל תריץ החלה נוספת לפני שמבינים למה.

+ {% endif %} +
+ {% endif %} +
+{% endblock %} diff --git a/webapp/templates/files.html b/webapp/templates/files.html index 9d97b538a..27a33589c 100644 --- a/webapp/templates/files.html +++ b/webapp/templates/files.html @@ -368,7 +368,10 @@

נוצר: {{ file.created_at }} + {# עודכן==נוצר פירושו שהקובץ מעולם לא נערך — "עודכן" זהה הוא רעש #} + {% if file.updated_at != file.created_at %} עודכן: {{ file.updated_at }} + {% endif %} diff --git a/webapp/templates/view_file.html b/webapp/templates/view_file.html index 53c7896a5..90fbfde55 100644 --- a/webapp/templates/view_file.html +++ b/webapp/templates/view_file.html @@ -943,9 +943,12 @@

{{ file.file_name }}

נוצר
{{ file.created_at }}
-
+ {# כשהקובץ מעולם לא עודכן, שני התאריכים זהים ו"עודכן" הוא רעש — + מציגים רק "נוצר". הפריט נשאר ב-DOM מוסתר, כי עריכת תיאור חיה + (quick-update) צריכה להחזיר אותו לתצוגה בלי רענון. #} +
עודכן
-
{{ file.updated_at }}
+
{{ file.updated_at }}
@@ -1587,6 +1590,14 @@

🗑️ העבר לסל

if (p) { p.textContent = desc; p.hidden = !desc; } const label = document.getElementById('descMenuLabel'); if (label) { label.textContent = desc ? '📝 ערוך תיאור' : '➕ הוסף תיאור'; } + // "עודכן" מתרענן במקום — עד עכשיו הערך הישן נשאר מוצג עד כניסה + // מחודשת לקובץ. הפריט גם נחשף אם היה מוסתר (עודכן==נוצר). + if (data.updated_at_display) { + const uv = document.getElementById('metaUpdatedValue'); + if (uv) { uv.textContent = data.updated_at_display; } + const ui = document.getElementById('metaUpdatedItem'); + if (ui) { ui.hidden = false; } + } showToast('התיאור נשמר', 'success'); closeEditDescriptionModal(); } catch (e) { From 06392b19c2db6aafba8efb0ccfcf23f1196dbfa5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 03:56:10 +0000 Subject: [PATCH 2/3] =?UTF-8?q?fix(created-at):=20=D7=A8=D7=92=D7=A8=D7=A1?= =?UTF-8?q?=D7=99=D7=94=20=D7=91-MCP,=20=D7=90=D7=95=D7=91=D7=93=D7=9F=20?= =?UTF-8?q?=D7=AA=D7=90=D7=A8=D7=99=D7=9A=20=D7=91-/edit,=20=D7=95=D7=9E?= =?UTF-8?q?=D7=99=D7=92=D7=A8=D7=A6=D7=99=D7=94=20=D7=A9=D7=93=D7=99=D7=9C?= =?UTF-8?q?=D7=92=D7=94=20=D7=A2=D7=9C=20=D7=94=D7=A7=D7=91=D7=A6=D7=99?= =?UTF-8?q?=D7=9D=20=D7=94=D7=A9=D7=91=D7=95=D7=A8=D7=99=D7=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit תיקון ממצאי הריוויו על PR #3305 — ובראשם רגרסיה שנכנסה בו עצמו. **‏P0: ‏edit_file ו-append_file נשברו.** שניהם עוברים ב-``_resave_edited`` שקרא ל-``backend.save_file`` בלי ``update_existing=True``, ושניהם פועלים בהגדרה על קובץ קיים — כלומר השער החדש החזיר להם ``file_exists`` והם סירבו לכתוב. השורש שאפשר לזה חשוב מהתיקון: הפייקים בטסטים עודכנו רק כדי שלא יזרקו ``TypeError`` על הפרמטר החדש, בלי לשאול אם הם עדיין מספרים את האמת על החוזה. עכשיו הם **אוכפים** אותו — הפייק מחזיר ``file_exists`` כשהדגל חסר, ולכן החזרת הרגרסיה מפילה גם אותו וגם את ``test_edit_file_accumulates``. **אובדן תאריך ב-/edit.** ‏``(prev or file or {}).get('created_at')`` בוחר מילון ואז שולף; ברגע ש-``prev`` הוא מילון לא-ריק בלי השדה, ``file`` לעולם לא נבדק והתאריך נופל ל-now. עבר לנפילה **לפי שדה**. **"עודכן" מול "נוצר" ירד משכבת התבנית לשכבת הנתונים.** שתי התבניות השוו מחרוזות מפורמטות, ו-``format_datetime_display`` מעגל לדקות — קובץ שנוצר ב-10:30:10 ונערך ב-10:30:50 נראה כאילו מעולם לא נערך. ``has_real_update`` משווה את ה-``datetime`` הגולמי, והדגל ``has_update`` מועבר לתבניות. **המיגרציה.** ‏``$gt`` מול ``null`` מחזיר false (נמדד מול mongod 7.0.14), ולכן הצינור דילג בדיוק על הקבצים השבורים ביותר — אלה שלגרסה האחרונה שלהם אין ``created_at`` כלל. בנוסף ``$first`` בחר מסמך יחיד בזמן שהאפליקציה עצמה ממיינת בלי שובר-שוויון, כלומר הבחירה אינה יציבה; עכשיו מתוקנים **כל** המסמכים בגרסה הגבוהה ביותר ואין ניחוש. ‏``total_files`` שנכשלת נזרקת במקום להיות מוצגת כ-0, כשל audit חוזר בתוצאה, ההחלה מבטלת קאש ורצה באצוות עם דיווח יתרה. **עמוד האדמין.** השער ``dry_run_seen=1`` היה שדה מוסתר — קלט מהלקוח. עבר לאסימון בסשן עם חתימת התוצאה: אם קבוצת המושפעים השתנתה מאז ההצגה, ההחלה נדחית. בנוסף ``.btn-danger`` לא הייתה מוגדרת גלובלית והכפתור ההרסני נראה רגיל, ו-``--danger-border`` (שקוף 45%) שימש כצבע טקסט. **טסטים.** ‏harness המונגו המשוכפל עבר ל-``tests/mongo_it.py``, נוסף כיסוי לחמשת מסלולי הכתיבה בוובאפ ולשער האדמין, והקביעות המעורפלות (``assert a == {**a, ...}``) הוחלפו במפורשות. כל בדיקה חדשה הורצה כמוטציה על הקוד שלפני התיקון ונפלה שם. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019YULCppaQRPYN1RgeY6NBu --- mcp_server/handlers.py | 5 + services/created_at_migration.py | 265 +++++++++++++----- tests/mongo_it.py | 85 ++++++ tests/test_admin_migration_gate_mongo.py | 124 ++++++++ tests/test_created_at_inheritance_mongo.py | 183 +++++++++--- tests/test_created_at_webapp_routes_mongo.py | 246 ++++++++++++++++ tests/test_mcp_edit_append.py | 13 +- tests/test_mcp_handlers.py | 20 +- tests/test_note_boards_mongo.py | 52 +--- webapp/app.py | 98 ++++++- .../templates/admin_migration_created_at.html | 51 +++- webapp/templates/files.html | 6 +- webapp/templates/view_file.html | 10 +- 13 files changed, 964 insertions(+), 194 deletions(-) create mode 100644 tests/mongo_it.py create mode 100644 tests/test_admin_migration_gate_mongo.py create mode 100644 tests/test_created_at_webapp_routes_mongo.py diff --git a/mcp_server/handlers.py b/mcp_server/handlers.py index 9718bf1ff..5e5369412 100644 --- a/mcp_server/handlers.py +++ b/mcp_server/handlers.py @@ -211,6 +211,11 @@ def _resave_edited( programming_language=str(doc.get("programming_language") or doc.get("language") or "text"), description=str(doc.get("description") or ""), tags=list(doc.get("tags") or []), + # ``edit_file``/``append_file`` פועלים **בהגדרה** על קובץ קיים — הם + # מגיעים לכאן רק אחרי ``get_file`` מוצלח. לכן ``prev`` תמיד מלא, + # ובלי הדגל הזה שער ה-``file_exists`` היה חוסם את שני הכלים לגמרי. + # השער נועד למנוע דריסה בשוגג ב-``save_file``, לא לחסום עריכה. + update_existing=True, ) diff --git a/services/created_at_migration.py b/services/created_at_migration.py index 488526872..71d153b5c 100644 --- a/services/created_at_migration.py +++ b/services/created_at_migration.py @@ -6,119 +6,240 @@ בקוד; המיגרציה כאן מיישרת את המצב הקיים — בלעדיה קובץ ותיק יוריש הלאה את התאריך השגוי שכבר יש לו. -מה היא עושה: לכל ``(user_id, file_name)`` פעיל, קובעת לגרסה **האחרונה -בלבד** ``created_at = המוקדם מבין כל גרסאות הקובץ``. גרסאות ישנות אינן -נגועות — תאריכי ההיסטוריה שלהן נשארים כמות שהם. +מה היא עושה: לכל ``(user_id, file_name)`` פעיל, קובעת לכל מסמך שנמצא +ב**גרסה הגבוהה ביותר** ``created_at = המוקדם מבין כל גרסאות הקובץ``. +גרסאות ישנות אינן נגועות — תאריכי ההיסטוריה שלהן נשארים כמות שהם. + +**למה "כל מסמך בגרסה הגבוהה" ולא "המסמך האחרון":** מרוץ כתיבה ידוע +מייצר שני מסמכים עם אותו ``version``, והאפליקציה בוחרת ביניהם עם +``{"$sort": {"file_name": 1, "version": -1}}`` בלי שובר-שוויון +(``database/repository.py`` — שבעה מופעים) — כלומר הבחירה אינה יציבה. +לכן ``$first`` כאן היה מתקן מסמך שאולי אינו זה שמוצג. תיקון של כל +התאומים מוציא את הניחוש מהמשוואה. מופעלת מעמוד האדמין ``/admin/migrations/created-at`` בלבד: dry-run -לקריאה בלבד, ואחריו החלה מפורשת. שני המסלולים כותבים מסמך audit -ל-``migration_audit`` כדי שיישאר תיעוד למה שנבדק ומה הוחל. +לקריאה בלבד, ואחריו החלה מפורשת **באצוות** — כל הרצה מטפלת בכמות חסומה +כדי שלא תחסום בקשת HTTP, ומדווחת כמה נותרו. ההחלה אידמפוטנטית, ולכן +הרצה חוזרת בטוחה. שני המסלולים כותבים מסמך audit ל-``migration_audit``. """ from __future__ import annotations +import logging from datetime import datetime, timezone -from typing import Any, Dict, List +from typing import Any, Dict, List, Optional + +logger = logging.getLogger(__name__) AUDIT_COLLECTION = "migration_audit" MIGRATION_NAME = "created_at_from_first_version" -# צינור משותף לשני המסלולים: אותו חישוב בדיוק ב-dry-run ובהחלה, כדי שמה -# שהוצג הוא מה שיוחל. שינוי בצנרת של אחד בלי השני הוא באג, לא גמישות. -_PIPELINE: List[Dict[str, Any]] = [ - {"$match": {"is_active": True}}, - {"$sort": {"user_id": 1, "file_name": 1, "version": -1}}, - { - "$group": { - "_id": {"user_id": "$user_id", "file_name": "$file_name"}, - "latest_id": {"$first": "$_id"}, - "latest_created": {"$first": "$created_at"}, - "earliest_created": {"$min": "$created_at"}, - "versions": {"$sum": 1}, - } - }, - # רק קבצים שבהם יש מה לתקן: התאריך של הגרסה האחרונה מאוחר מהמוקדם. - {"$match": {"$expr": {"$gt": ["$latest_created", "$earliest_created"]}}}, -] - - -def _affected(db) -> List[Dict[str, Any]]: - return list(db.code_snippets.aggregate(_PIPELINE, allowDiskUse=True)) - - -def _write_audit(db, doc: Dict[str, Any]) -> None: - try: - db[AUDIT_COLLECTION].insert_one(doc) - except Exception: - # audit הוא תיעוד, לא שער: כישלון בו לא מפיל את המיגרציה עצמה. - pass +#: כמה קבצים מטופלים בהחלה אחת. ההחלה רצה בתוך בקשת HTTP של האדמין, +#: ולכן חייבת להיחסם בזמן. הערך נבחר כדי שגם ``bulk_write`` וגם ביטול +#: הקאש שאחריו יסתיימו הרבה לפני timeout של פרוקסי. +DEFAULT_BATCH_SIZE = 500 -def dry_run(db, *, sample_size: int = 20) -> Dict[str, Any]: - """קריאה בלבד. מחזיר כמה קבצים ייפגעו, מתוך כמה, ודוגמאות.""" - affected = _affected(db) - total = 0 +class MigrationError(RuntimeError): + """כשל שאסור להציג כ-0 או כהצלחה — הדו"ח הזה הוא בסיס להחלטה.""" + + +def _pipeline(*, limit: Optional[int] = None) -> List[Dict[str, Any]]: + """הצינור המשותף ל-dry-run ולהחלה — אותו חישוב בדיוק בשניהם. + + ``$min`` מתעלם מ-``null`` ומשדה חסר (נמדד מול mongod 7.0.14), ולכן + ``earliest_created`` הוא תמיד תאריך אמיתי אם קיים ולו אחד; קובץ שאין + בו אף ``created_at`` יקבל ``null`` ויוסנן החוצה — אין לו מה לרשת. + """ + stages: List[Dict[str, Any]] = [ + {"$match": {"is_active": True}}, + { + "$group": { + "_id": {"user_id": "$user_id", "file_name": "$file_name"}, + "max_version": {"$max": "$version"}, + "earliest_created": {"$min": "$created_at"}, + "versions": {"$sum": 1}, + "docs": { + "$push": {"_id": "$_id", "version": "$version", "created_at": "$created_at"} + }, + } + }, + # רק קבצים שיש להם ממה לרשת. + {"$match": {"earliest_created": {"$ne": None}}}, + { + "$addFields": { + # המועמדים לתיקון: מסמכי הגרסה הגבוהה ביותר שהתאריך שלהם + # מאוחר מהמוקדם — או חסר לגמרי. השוואת ``$gt`` מול ``null`` + # מחזירה ``false`` (נמדד), ולכן החסרים חייבים תנאי נפרד; + # בלעדיו קובץ שגרסתו האחרונה בלי ``created_at`` היה נשאר + # שבור והורשת התיקון קדימה הייתה נותנת לו ``now`` שוב. + "targets": { + "$filter": { + "input": "$docs", + "as": "d", + "cond": { + "$and": [ + {"$eq": ["$$d.version", "$max_version"]}, + { + "$or": [ + {"$eq": [{"$ifNull": ["$$d.created_at", None]}, None]}, + {"$gt": ["$$d.created_at", "$earliest_created"]}, + ] + }, + ] + }, + } + }, + } + }, + {"$match": {"targets.0": {"$exists": True}}}, + {"$project": {"docs": 0}}, + # סדר יציב בין הרצות, כדי שהחלה באצוות תתקדם ולא תדשדש. + {"$sort": {"_id.user_id": 1, "_id.file_name": 1}}, + ] + if limit is not None: + stages.append({"$limit": int(limit)}) + return stages + + +def _affected(db, *, limit: Optional[int] = None) -> List[Dict[str, Any]]: + return list(db.code_snippets.aggregate(_pipeline(limit=limit), allowDiskUse=True)) + + +def count_affected(db) -> int: + """כמה קבצים לוגיים עדיין דורשים תיקון. חלק מה-API הציבורי: עמוד + האדמין משתמש בו כחתימת התוצאה, כדי לוודא שההחלה חלה על מה שהוצג.""" + rows = list( + db.code_snippets.aggregate(_pipeline() + [{"$count": "n"}], allowDiskUse=True) + ) + return int(rows[0]["n"]) if rows else 0 + + +def _count_total_files(db) -> int: + """סך הקבצים הלוגיים הפעילים. כשל כאן **נזרק**, לא מוחלף ב-0. + + המספר הזה הוא המכנה בדו"ח שעליו האדמין מחליט אם להחיל. ``0`` שקט + הופך "1200 מתוך 40000" ל-"1200 מתוך 0" — מטעה בדיוק ברגע ההחלטה. + """ try: - agg = list( + rows = list( db.code_snippets.aggregate( [ {"$match": {"is_active": True}}, {"$group": {"_id": {"user_id": "$user_id", "file_name": "$file_name"}}}, {"$count": "n"}, - ] + ], + allowDiskUse=True, ) ) - total = int(agg[0]["n"]) if agg else 0 - except Exception: - total = 0 - samples = [ - { - "file_name": row["_id"]["file_name"], - "user_id": row["_id"]["user_id"], - "versions": row["versions"], - "current_created": row["latest_created"], - "new_created": row["earliest_created"], - } - for row in affected[:sample_size] - ] - result = { + except Exception as exc: # pragma: no cover - תלוי בכשל DB אמיתי + raise MigrationError(f"ספירת סך הקבצים נכשלה: {exc}") from exc + return int(rows[0]["n"]) if rows else 0 + + +def _write_audit(db, doc: Dict[str, Any]) -> Optional[str]: + """כותב רשומת audit ומחזיר את הודעת השגיאה אם נכשל — ``None`` בהצלחה. + + לא בולעים: "מיגרציה שאפשר לעמוד מאחוריה" כוללת את התיעוד שלה. דיווח + הצלחה בזמן שהרשומה לא נכתבה הוא בדיוק הדפוס של הצלחה מדומה, ולכן + הכישלון חוזר בתוצאה ומוצג לאדמין. + """ + try: + db[AUDIT_COLLECTION].insert_one(dict(doc)) + return None + except Exception as exc: + logger.warning("כתיבת audit למיגרציה %s נכשלה: %s", MIGRATION_NAME, exc) + return str(exc) + + +def _invalidate_users(user_ids) -> Dict[str, Any]: + """מבטל קאש למשתמשים שנגענו בהם, באותו מנגנון של ``save_code_snippet``. + + בלי זה רשימות הקבצים ממשיכות להיות מוגשות מהקאש עם התאריך הישן עד + שה-TTL פג. מדווחים כמה משתמשים טופלו וכמה נכשלו — לא מכריזים הצלחה. + """ + ok, failed = 0, 0 + try: + from cache_manager import cache # type: ignore + except Exception as exc: + return {"attempted": len(user_ids), "ok": 0, "failed": len(user_ids), "error": str(exc)} + for uid in user_ids: + try: + cache.invalidate_user_cache(int(uid)) + ok += 1 + except Exception: + failed += 1 + return {"attempted": len(user_ids), "ok": ok, "failed": failed} + + +def _sample_of(row: Dict[str, Any]) -> Dict[str, Any]: + targets = row.get("targets") or [] + currents = [t.get("created_at") for t in targets] + return { + "file_name": row["_id"]["file_name"], + "user_id": row["_id"]["user_id"], + "versions": row.get("versions"), + "duplicate_latest": len(targets) > 1, + "current_created": currents[0] if currents else None, + "new_created": row.get("earliest_created"), + } + + +def dry_run(db, *, sample_size: int = 20) -> Dict[str, Any]: + """קריאה בלבד. מחזיר כמה קבצים ייפגעו, מתוך כמה, ודוגמאות.""" + affected_count = count_affected(db) + total = _count_total_files(db) + samples = [_sample_of(row) for row in _affected(db, limit=max(0, int(sample_size)))] + result: Dict[str, Any] = { "migration": MIGRATION_NAME, "mode": "dry_run", - "affected_count": len(affected), + "affected_count": affected_count, "total_files": total, + "batch_size": DEFAULT_BATCH_SIZE, "samples": samples, "ran_at": datetime.now(timezone.utc), } - _write_audit(db, dict(result)) + result["audit_error"] = _write_audit(db, result) return result -def apply(db) -> Dict[str, Any]: - """מחיל, ואז **מאמת בקריאה חוזרת** — ערך ההחזרה של הכתיבה אינו אימות.""" +def apply(db, *, batch_size: int = DEFAULT_BATCH_SIZE) -> Dict[str, Any]: + """מחיל אצווה אחת, ואז **מאמת בקריאה חוזרת** — ערך ההחזרה של הכתיבה אינו אימות.""" from pymongo import UpdateOne - affected = _affected(db) - ops = [ - UpdateOne( - {"_id": row["latest_id"]}, - {"$set": {"created_at": row["earliest_created"]}}, - ) - for row in affected - ] + batch = _affected(db, limit=int(batch_size)) + ops: List[Any] = [] + user_ids = set() + for row in batch: + for target in row.get("targets") or []: + ops.append( + UpdateOne( + {"_id": target["_id"]}, + {"$set": {"created_at": row["earliest_created"]}}, + ) + ) + user_ids.add(row["_id"]["user_id"]) + modified = 0 if ops: res = db.code_snippets.bulk_write(ops, ordered=False) modified = int(getattr(res, "modified_count", 0) or 0) - # אימות: אחרי ההחלה, כמה קבצים עדיין עומדים בתנאי הפער. אמור להיות 0. - remaining = len(_affected(db)) + cache_report = _invalidate_users(sorted(user_ids)) if user_ids else {"attempted": 0, "ok": 0, "failed": 0} + + # אימות בקריאה חוזרת: כמה קבצים עדיין עומדים בתנאי. הצינור אינו רואה + # את מה שכבר תוקן, ולכן זהו גם מונה ההתקדמות של האצוות הבאות. + remaining = count_affected(db) - result = { + result: Dict[str, Any] = { "migration": MIGRATION_NAME, "mode": "apply", - "planned": len(affected), + "files_in_batch": len(batch), + "documents_planned": len(ops), "modified": modified, "remaining_after": remaining, + "done": remaining == 0, + "cache_invalidation": cache_report, "ran_at": datetime.now(timezone.utc), } - _write_audit(db, dict(result)) + result["audit_error"] = _write_audit(db, result) return result diff --git a/tests/mongo_it.py b/tests/mongo_it.py new file mode 100644 index 000000000..ab1f982c6 --- /dev/null +++ b/tests/mongo_it.py @@ -0,0 +1,85 @@ +"""‏harness משותף לבדיקות שרצות מול **מונגו אמיתי**. + +מתי הן רצות: כש-``MONGODB_URL`` מוגדר והשרת נענה. ב-CI זה תמיד — הג'וב +``Unit Tests`` מרים ``mongo:6.0`` כשירות (ראו ``.github/workflows/ci.yml``). +מקומית הן מדלגות, כדי שהרצה רגילה תישאר מהירה. + +**למה זה כאן ולא משוכפל בכל קובץ:** ה-harness הזה חזר מילה במילה בשני +קבצים, וכפילות של קוד בדיקה היא כפילות של **החלטות בטיחות** — סורג +המחיקה, ה-``tz_aware``, ותנאי הדילוג. עותק שמתעדכן לבד הוא בדיוק המקום +שבו הבדיקה מפסיקה לרוץ בשקט. + +``tz_aware=True`` אינו פרט טכני: בלעדיו pymongo מחזיר ``datetime`` נאיבי, +והשוואות בין תאריכים שנשלפו למודעים-לאזור זורקות ``TypeError`` — שנבלע +ב-``except Exception`` ומשתיק את הבדיקה במקום להפיל אותה. + +**בטיחות מחיקה:** כל הרצה עובדת על מסד עם שם ייחודי משלה, וה-teardown +מוודא שהשם תואם לתחילית הצפויה לפני ``drop_database``. מסד שלא נוצר כאן +לא נמחק כאן. +""" + +from __future__ import annotations + +import os +import uuid +from datetime import timezone +from typing import Iterator + +import pytest + +pymongo = pytest.importorskip("pymongo") + +from pymongo.errors import ServerSelectionTimeoutError # noqa: E402 + +MONGO_URL = os.environ.get("MONGODB_URL", "").strip() + + +def server_is_reachable(url: str) -> bool: + """האם יש שרת בקצה השני. בלי זה הבדיקות היו נתלות עד timeout ארוך.""" + if not url: + return False + try: + client = pymongo.MongoClient( + url, serverSelectionTimeoutMS=2000, tz_aware=True, tzinfo=timezone.utc + ) + client.admin.command("ping") + client.close() + return True + except (ServerSelectionTimeoutError, Exception): + return False + + +#: נמדד פעם אחת בטעינת המודול, ולא פעם לכל קובץ בדיקות. +MONGO_AVAILABLE = server_is_reachable(MONGO_URL) + +#: ``pytestmark = requires_mongo`` בראש קובץ בדיקות שדורש מונגו אמיתי. +requires_mongo = pytest.mark.skipif( + not MONGO_AVAILABLE, + reason="דורש MONGODB_URL עם שרת מונגו נגיש (קיים ב-CI, לא בהכרח מקומית)", +) + + +def make_mongo_db_fixture(prefix: str): + """מייצר fixture ``mongo_db`` עם תחילית ייעודית לקובץ הקורא. + + התחילית אינה קוסמטית — היא הסורג שמונע מחיקה של מסד שלא נוצר כאן, + ולכן היא פרמטר מפורש ולא ערך משותף. + """ + if not prefix: + raise ValueError("חובה תחילית ייעודית — היא סורג הבטיחות של המחיקה") + + @pytest.fixture + def mongo_db() -> Iterator["pymongo.database.Database"]: + name = f"{prefix}{uuid.uuid4().hex[:12]}" + client = pymongo.MongoClient(MONGO_URL, tz_aware=True, tzinfo=timezone.utc) + db = client[name] + try: + yield db + finally: + assert name.startswith(prefix), f"סירוב למחוק מסד שאינו של הבדיקות: {name}" + try: + client.drop_database(name) + finally: + client.close() + + return mongo_db diff --git a/tests/test_admin_migration_gate_mongo.py b/tests/test_admin_migration_gate_mongo.py new file mode 100644 index 000000000..ebb3be5b2 --- /dev/null +++ b/tests/test_admin_migration_gate_mongo.py @@ -0,0 +1,124 @@ +"""שער ה-dry-run בעמוד המיגרציה — מול **מונגו אמיתי** ודרך הראוט עצמו. + +השער הקודם היה שדה מוסתר בטופס (``dry_run_seen=1``). שדה מוסתר הוא קלט +מהלקוח: כל POST של אדמין נשא אותו בלי שדבר הוצג, ולכן "ההחלה נפתחת רק +אחרי dry-run" לא היה נכון. השער עבר לסשן: ה-dry-run מנפיק אסימון אקראי +ושומר לצידו את מספר הקבצים המושפעים, וההחלה מאמתת את שניהם. + +כל בדיקה כאן מאמתת גם שה-DB **לא השתנה** כשהשער דחה — סירוב שמדווח +בטקסט אבל כותב בכל זאת אינו שער. +""" + +from __future__ import annotations + +import re +from datetime import datetime, timedelta, timezone + +import pytest + +from mongo_it import make_mongo_db_fixture, requires_mongo + +pytestmark = requires_mongo + +mongo_db = make_mongo_db_fixture("codebot_mig_gate_it_") + +ADMIN_ID = 4242 +ORIGIN = datetime(2024, 1, 1, tzinfo=timezone.utc) +LATER = ORIGIN + timedelta(days=5) + +URL = "/admin/migrations/created-at" + + +@pytest.fixture +def client(mongo_db, monkeypatch): + import webapp.app as W + + monkeypatch.setattr(W, "get_db", lambda: mongo_db) + monkeypatch.setattr(W, "is_admin", lambda uid: int(uid) == ADMIN_ID) + W.app.config["TESTING"] = True + c = W.app.test_client() + with c.session_transaction() as sess: + sess["user_id"] = ADMIN_ID + sess["user_data"] = {"id": ADMIN_ID, "first_name": "a", "username": "a"} + return c + + +def _seed_broken(mongo_db, name: str, latest_created=LATER): + """קובץ שגרסתו האחרונה נושאת "נוצר" מאוחר — מועמד לתיקון.""" + mongo_db.code_snippets.insert_many([ + {"user_id": 1, "file_name": name, "version": 1, "is_active": True, + "created_at": ORIGIN, "updated_at": ORIGIN}, + {"user_id": 1, "file_name": name, "version": 2, "is_active": True, + "created_at": latest_created, "updated_at": latest_created}, + ]) + + +def _latest_created(mongo_db, name: str): + return mongo_db.code_snippets.find_one({"file_name": name, "version": 2})["created_at"] + + +def _token(html: str): + m = re.search(r'name="gate_token" value="([^"]+)"', html) + return m.group(1) if m else None + + +def _text(resp) -> str: + return resp.get_data(as_text=True) + + +def test_apply_without_dry_run_is_refused_and_writes_nothing(client, mongo_db): + _seed_broken(mongo_db, "x.py") + body = _text(client.post(URL, data={"action": "apply"})) + assert "יש להריץ dry-run" in body + assert _latest_created(mongo_db, "x.py") == LATER, "נכתב למרות הסירוב" + + +def test_forged_hidden_field_does_not_open_the_gate(client, mongo_db): + """‏``dry_run_seen=1`` — השדה של הגרסה הקודמת — כבר לא פותח כלום.""" + _seed_broken(mongo_db, "x.py") + body = _text(client.post(URL, data={"action": "apply", "dry_run_seen": "1"})) + assert "יש להריץ dry-run" in body + assert _latest_created(mongo_db, "x.py") == LATER + + +def test_forged_token_does_not_open_the_gate(client, mongo_db): + _seed_broken(mongo_db, "x.py") + client.post(URL, data={"action": "dry_run"}) + body = _text(client.post(URL, data={"action": "apply", "gate_token": "z" * 32})) + assert "יש להריץ dry-run" in body + assert _latest_created(mongo_db, "x.py") == LATER + + +def test_gate_refuses_when_the_affected_set_changed_since_the_dry_run(client, mongo_db): + """מה שאושר חייב להיות מה שיוחל — אחרת סירוב ובקשה להריץ מחדש.""" + _seed_broken(mongo_db, "x.py") + token = _token(_text(client.post(URL, data={"action": "dry_run"}))) + assert token + + _seed_broken(mongo_db, "y.py") # קובץ שבור נוסף אחרי ההצגה + body = _text(client.post(URL, data={"action": "apply", "gate_token": token})) + assert "השתנתה מאז ה-dry-run" in body + assert _latest_created(mongo_db, "x.py") == LATER + assert _latest_created(mongo_db, "y.py") == LATER + + +def test_dry_run_then_apply_migrates_and_consumes_the_token(client, mongo_db): + _seed_broken(mongo_db, "x.py") + dry = _text(client.post(URL, data={"action": "dry_run"})) + token = _token(dry) + assert token and 'value="apply"' in dry + assert _latest_created(mongo_db, "x.py") == LATER, "dry-run כתב — הוא אמור לקרוא בלבד" + + body = _text(client.post(URL, data={"action": "apply", "gate_token": token})) + assert "האימות בקריאה חוזרת עבר" in body + assert _latest_created(mongo_db, "x.py") == ORIGIN + + # האסימון נצרך: הרצה חוזרת עם אותו אסימון נדחית + again = _text(client.post(URL, data={"action": "apply", "gate_token": token})) + assert "יש להריץ dry-run" in again + + +def test_get_does_not_offer_apply_before_a_dry_run(client, mongo_db): + _seed_broken(mongo_db, "x.py") + body = _text(client.get(URL)) + assert 'value="apply"' not in body diff --git a/tests/test_created_at_inheritance_mongo.py b/tests/test_created_at_inheritance_mongo.py index b5a84d4cc..91702468b 100644 --- a/tests/test_created_at_inheritance_mongo.py +++ b/tests/test_created_at_inheritance_mongo.py @@ -13,50 +13,16 @@ from __future__ import annotations -import os -import uuid from datetime import datetime, timedelta, timezone import pytest -pymongo = pytest.importorskip("pymongo") +from mongo_it import make_mongo_db_fixture, requires_mongo -from pymongo.errors import ServerSelectionTimeoutError # noqa: E402 +pytestmark = requires_mongo -_TEST_DB_PREFIX = "codebot_created_it_" -_MONGO_URL = os.environ.get("MONGODB_URL", "").strip() - - -def _server_is_reachable(url: str) -> bool: - try: - client = pymongo.MongoClient(url, serverSelectionTimeoutMS=2000, tz_aware=True, tzinfo=timezone.utc) - client.admin.command("ping") - client.close() - return True - except (ServerSelectionTimeoutError, Exception): - return False - - -pytestmark = pytest.mark.skipif( - not _MONGO_URL or not _server_is_reachable(_MONGO_URL), - reason="דורש MONGODB_URL עם שרת מונגו נגיש (קיים ב-CI, לא בהכרח מקומית)", -) - - -@pytest.fixture -def mongo_db(): - name = f"{_TEST_DB_PREFIX}{uuid.uuid4().hex[:12]}" - client = pymongo.MongoClient(_MONGO_URL, tz_aware=True, tzinfo=timezone.utc) - db = client[name] - try: - yield db - finally: - # סורג בטיחות: מוחקים רק מסד שנוצר כאן - assert name.startswith(_TEST_DB_PREFIX), f"סירוב למחוק מסד שאינו של הבדיקות: {name}" - try: - client.drop_database(name) - finally: - client.close() +#: תחילית ייעודית לקובץ הזה — סורג המחיקה ב-``mongo_it`` נשען עליה. +mongo_db = make_mongo_db_fixture("codebot_created_it_") @pytest.fixture @@ -92,7 +58,10 @@ def test_new_version_inherits_created_at(repo, mongo_db): assert v2 is not None assert v2["created_at"] == v1["created_at"], "עריכה אינה לידה מחדש" - assert v2["updated_at"] > v1["updated_at"], "תאריך הגרסה חי ב-updated_at" + # ``>=`` ולא ``>``: שתי שמירות ברצף יכולות ליפול על אותה חותמת אם + # רזולוציית השעון מגסה. הטענה שנבדקת כאן היא ש-``updated_at`` **אינו + # יורש** מהגרסה הקודמת אלא נקבע מחדש — ולכן הוא לעולם לא נסוג אחורה. + assert v2["updated_at"] >= v1["updated_at"], "תאריך הגרסה חי ב-updated_at" def test_inheritance_survives_a_chain_of_edits(repo, mongo_db): @@ -141,7 +110,12 @@ def test_migration_dry_run_reads_only_and_apply_fixes_latest_only(mongo_db): assert latest["created_at"] == t2 a = mig.apply(mongo_db) - assert a == {**a, "planned": 1, "modified": 1, "remaining_after": 0} + assert a["files_in_batch"] == 1 + assert a["documents_planned"] == 1 + assert a["modified"] == 1 + assert a["remaining_after"] == 0 + assert a["done"] is True + assert a["audit_error"] is None latest = mongo_db.code_snippets.find_one({"file_name": "old.py", "version": 3}) assert latest["created_at"] == t0, "האחרונה קיבלה את המוקדם" v2 = mongo_db.code_snippets.find_one({"file_name": "old.py", "version": 2}) @@ -149,7 +123,9 @@ def test_migration_dry_run_reads_only_and_apply_fixes_latest_only(mongo_db): # אידמפוטנטי: החלה שנייה לא מוצאת מה לתקן again = mig.apply(mongo_db) - assert again["planned"] == 0 and again["modified"] == 0 + assert again["documents_planned"] == 0 + assert again["modified"] == 0 + assert again["remaining_after"] == 0 def test_migration_audit_records_both_modes(mongo_db): @@ -159,3 +135,128 @@ def test_migration_audit_records_both_modes(mongo_db): mig.apply(mongo_db) modes = [d["mode"] for d in mongo_db[mig.AUDIT_COLLECTION].find()] assert modes == ["dry_run", "apply"] + + +def test_migration_fixes_latest_version_without_created_at(mongo_db): + """הגרסה האחרונה בלי ``created_at`` — הקובץ שהצינור הישן דילג עליו. + + ``$gt`` מול ``null`` מחזיר ``false`` (נמדד מול mongod 7.0.14), ולכן + תנאי הפער לבדו הסתיר בדיוק את הקבצים השבורים ביותר: אלה שאין להם + תאריך כלל. ההורשה קדימה הייתה נותנת להם ``now`` בעריכה הבאה. + """ + from services import created_at_migration as mig + + t0 = datetime(2024, 1, 1, tzinfo=timezone.utc) + mongo_db.code_snippets.insert_one( + {"user_id": 7, "file_name": "nodate.py", "version": 1, "is_active": True, "created_at": t0} + ) + mongo_db.code_snippets.insert_one( + {"user_id": 7, "file_name": "nodate.py", "version": 2, "is_active": True} + ) + + assert mig.count_affected(mongo_db) == 1, "הקובץ לא זוהה כמושפע" + a = mig.apply(mongo_db) + assert a["modified"] == 1 and a["remaining_after"] == 0 + + latest = mongo_db.code_snippets.find_one({"file_name": "nodate.py", "version": 2}) + assert latest["created_at"] == t0 + + +def test_migration_fixes_every_document_at_the_highest_version(mongo_db): + """שני מסמכים באותה גרסה — שניהם מתוקנים, בלי לנחש מי "האחרון". + + מרוץ כתיבה ידוע מייצר תאומים, והאפליקציה בוחרת ביניהם עם ``$sort`` + בלי שובר-שוויון — כלומר הבחירה אינה יציבה. תיקון של אחד מהם היה + משאיר את ה-UI מציג לפעמים את התאריך הישן. + """ + from services import created_at_migration as mig + + t0 = datetime(2024, 1, 1, tzinfo=timezone.utc) + mongo_db.code_snippets.insert_many([ + {"user_id": 8, "file_name": "twin.py", "version": 1, "is_active": True, "created_at": t0}, + {"user_id": 8, "file_name": "twin.py", "version": 2, "is_active": True, + "created_at": t0 + timedelta(days=9)}, + {"user_id": 8, "file_name": "twin.py", "version": 2, "is_active": True, + "created_at": t0 + timedelta(days=3)}, + ]) + + a = mig.apply(mongo_db) + assert a["files_in_batch"] == 1 + assert a["documents_planned"] == 2, "רק אחד מהתאומים תוקן" + assert a["modified"] == 2 + + twins = [d["created_at"] for d in mongo_db.code_snippets.find({"file_name": "twin.py", "version": 2})] + assert twins == [t0, t0] + + +def test_migration_leaves_files_that_have_no_date_to_inherit(mongo_db): + """קובץ שלאף גרסה שלו אין ``created_at`` — אין ממה לרשת, לא נוגעים.""" + from services import created_at_migration as mig + + mongo_db.code_snippets.insert_many([ + {"user_id": 9, "file_name": "blank.py", "version": 1, "is_active": True}, + {"user_id": 9, "file_name": "blank.py", "version": 2, "is_active": True}, + ]) + + assert mig.count_affected(mongo_db) == 0 + a = mig.apply(mongo_db) + assert a["documents_planned"] == 0 + + for doc in mongo_db.code_snippets.find({"file_name": "blank.py"}): + assert doc.get("created_at") is None + + +def test_migration_applies_in_batches(mongo_db): + """אצווה חסומה בגודל, ודיווח יתרה — כדי שלא תחסום בקשת HTTP.""" + from services import created_at_migration as mig + + t0 = datetime(2024, 1, 1, tzinfo=timezone.utc) + for i in range(3): + mongo_db.code_snippets.insert_many([ + {"user_id": 10, "file_name": f"b{i}.py", "version": 1, "is_active": True, "created_at": t0}, + {"user_id": 10, "file_name": f"b{i}.py", "version": 2, "is_active": True, + "created_at": t0 + timedelta(days=5)}, + ]) + + first = mig.apply(mongo_db, batch_size=2) + assert first["files_in_batch"] == 2 + assert first["remaining_after"] == 1 + assert first["done"] is False + + second = mig.apply(mongo_db, batch_size=2) + assert second["files_in_batch"] == 1 + assert second["remaining_after"] == 0 + assert second["done"] is True + + for i in range(3): + latest = mongo_db.code_snippets.find_one({"file_name": f"b{i}.py", "version": 2}) + assert latest["created_at"] == t0 + + +def test_migration_reports_audit_failure_instead_of_swallowing_it(mongo_db): + """כשל בכתיבת ה-audit חוזר בתוצאה — לא נבלע מאחורי דיווח הצלחה. + + זהו הדפוס שכבר עלה בריפו הזה: פעולה שהכשל שלה מוחזר כערך ולא נזרק, + ומעליה דיווח "הצליח". ה-audit הוא חלק מהחוזה של מיגרציה, ולכן + הכישלון שלו חייב להגיע לעיני האדמין. + """ + from services import created_at_migration as mig + + class _FailingAudit: + def __init__(self, real): + self._real = real + + def __getitem__(self, name): + if name == mig.AUDIT_COLLECTION: + class _Broken: + def insert_one(self, *a, **k): + raise RuntimeError("audit collection is read-only") + return _Broken() + return self._real[name] + + def __getattr__(self, name): + return getattr(self._real, name) + + db = _FailingAudit(mongo_db) + assert mig.dry_run(db)["audit_error"] == "audit collection is read-only" + assert mig.apply(db)["audit_error"] == "audit collection is read-only" diff --git a/tests/test_created_at_webapp_routes_mongo.py b/tests/test_created_at_webapp_routes_mongo.py new file mode 100644 index 000000000..5535b2e37 --- /dev/null +++ b/tests/test_created_at_webapp_routes_mongo.py @@ -0,0 +1,246 @@ +"""חמשת מסלולי הכתיבה בוובאפ משמרים את "נוצר" — מול **מונגו אמיתי**. + +הבדיקות ב-``test_created_at_inheritance_mongo`` מכסות את ``save_code_snippet``, +כלומר את מסלול ה-repository. אבל הוובאפ עוקף אותו: חמישה מקומות כותבים +``insert_one`` ישירות לאוסף, כל אחד עם ההיגיון שלו לירושה. מסלול שאין +עליו בדיקה יידרדר בשקט ברגע שמישהו יערוך את המילון שם. + +הבדיקות מריצות את הראוטים **האמיתיים** דרך ``test_client`` מול מסד זמני, +ומאמתות בקריאה חוזרת מה-DB — לא לפי ערך ההחזרה של הראוט. +""" + +from __future__ import annotations + +import io +import json +from datetime import datetime, timedelta, timezone + +import pytest + +from mongo_it import make_mongo_db_fixture, requires_mongo + +pytestmark = requires_mongo + +#: תחילית ייעודית לקובץ הזה — סורג המחיקה ב-``mongo_it`` נשען עליה. +mongo_db = make_mongo_db_fixture("codebot_created_web_it_") + +USER_ID = 4242 +ORIGIN = datetime(2024, 1, 1, tzinfo=timezone.utc) + + +@pytest.fixture +def client(mongo_db, monkeypatch): + """‏test client מחובר, מעל אותו מסד שהבדיקה מכינה. + + ``get_db`` מוחלף ולא ``MongoClient``: כל חמשת המסלולים ניגשים למסד + דרכו, ולכן זו נקודת ההזרקה היחידה שמכסה את כולם בלי לגעת בקוד. + """ + import webapp.app as W + + monkeypatch.setattr(W, "get_db", lambda: mongo_db) + W.app.config["TESTING"] = True + c = W.app.test_client() + with c.session_transaction() as sess: + sess["user_id"] = USER_ID + sess["user_data"] = {"id": USER_ID, "first_name": "t", "username": "t"} + return c + + +def _seed(mongo_db, file_name: str, *, version: int = 1, created_at=ORIGIN, **extra): + """גרסה קיימת עם "נוצר" ותיק — נקודת ההשוואה של כל בדיקה.""" + doc = { + "user_id": USER_ID, + "file_name": file_name, + "code": "original", + "programming_language": "python", + "description": "", + "tags": [], + "version": version, + "is_active": True, + "created_at": created_at, + "updated_at": created_at, + } + doc.update(extra) + return mongo_db.code_snippets.insert_one(doc).inserted_id + + +def _latest(mongo_db, file_name: str): + return mongo_db.code_snippets.find_one({"file_name": file_name}, sort=[("version", -1)]) + + +def test_edit_route_keeps_created_at(client, mongo_db): + """‏POST ל-``/edit/`` יוצר גרסה חדשה עם ה"נוצר" של הקודמת.""" + file_id = _seed(mongo_db, "edit_me.py") + + resp = client.post( + f"/edit/{file_id}", + data={"code": "changed", "file_name": "edit_me.py", "language": "python", + "description": "desc", "tags": "a,b"}, + follow_redirects=False, + ) + assert resp.status_code in (200, 302), resp.status_code + + latest = _latest(mongo_db, "edit_me.py") + assert latest["version"] == 2, "לא נוצרה גרסה חדשה — הבדיקה אינה בודקת כלום" + assert latest["created_at"] == ORIGIN + assert latest["updated_at"] > ORIGIN + + +def test_edit_route_falls_back_per_field_when_prev_lacks_created_at(client, mongo_db): + """‏``prev`` קיים אך בלי ``created_at`` — הנפילה היא **לפי שדה**. + + ‏``file`` נטען לפי ה-id שבכתובת, ו-``prev`` לפי שם הקובץ עם + ``version`` הגבוה ביותר — אלה שני מסמכים שונים. כשפותחים לעריכה id + של גרסה ותיקה שיש בה ``created_at``, בעוד שהגרסה האחרונה בשם הזה + כבר איבדה אותו, ``(prev or file).get(...)`` בוחר את ``prev`` (מילון + לא-ריק), מקבל ``None``, ונופל ל-``now``. הנפילה לפי שדה מוצאת את + התאריך ב-``file``. + """ + old_id = mongo_db.code_snippets.insert_one({ + "user_id": USER_ID, "file_name": "twohop.py", "code": "v1", + "programming_language": "python", "description": "", "tags": [], + "version": 1, "is_active": True, "created_at": ORIGIN, "updated_at": ORIGIN, + }).inserted_id + # הגרסה האחרונה — ``prev`` — בלי ``created_at`` כלל + mongo_db.code_snippets.insert_one({ + "user_id": USER_ID, "file_name": "twohop.py", "code": "v2", + "programming_language": "python", "description": "", "tags": [], + "version": 2, "is_active": True, "updated_at": ORIGIN + timedelta(days=1), + }) + + resp = client.post( + f"/edit/{old_id}", + data={"code": "v3", "file_name": "twohop.py", "language": "python", + "description": "", "tags": ""}, + ) + assert resp.status_code in (200, 302) + latest = _latest(mongo_db, "twohop.py") + assert latest["version"] == 3, "לא נוצרה גרסה חדשה — הבדיקה אינה בודקת כלום" + assert latest["created_at"] == ORIGIN + + +def test_edit_route_rename_inherits_from_the_edited_document(client, mongo_db): + """שינוי שם: אין ``prev`` בשם החדש — התאריך בא מהמסמך שנערך. + + זה הענף שבו ``file`` הוא המקור היחיד. בלעדיו כל שינוי שם היה מאפס + את "נוצר". + """ + file_id = _seed(mongo_db, "before_rename.py") + + resp = client.post( + f"/edit/{file_id}", + data={"code": "renamed body", "file_name": "after_rename.py", + "language": "python", "description": "", "tags": ""}, + ) + assert resp.status_code in (200, 302) + latest = _latest(mongo_db, "after_rename.py") + assert latest is not None, "שינוי השם לא יצר מסמך" + assert latest["created_at"] == ORIGIN + + +def test_restore_route_keeps_created_at(client, mongo_db): + """שחזור גרסה ישנה יוצר גרסה חדשה — ו"נוצר" נשאר של הקובץ.""" + _seed(mongo_db, "restore_me.py", version=1, created_at=ORIGIN) + later = ORIGIN + timedelta(days=30) + latest_id = _seed(mongo_db, "restore_me.py", version=2, created_at=ORIGIN, + updated_at=later, code="v2") + + resp = client.post(f"/api/file/{latest_id}/restore", json={"version": 1}) + assert resp.status_code == 200, resp.get_data(as_text=True) + + latest = _latest(mongo_db, "restore_me.py") + assert latest["version"] == 3, "לא נוצרה גרסה משוחזרת" + assert latest["code"] == "original", "לא שוחזר התוכן הישן" + assert latest["created_at"] == ORIGIN + + +def test_upload_route_keeps_created_at(client, mongo_db): + """העלאת קובץ בשם שכבר קיים — גרסה חדשה, "נוצר" ישן.""" + _seed(mongo_db, "uploaded.py") + + resp = client.post( + "/upload", + # שם השדה הוא ``code_file`` — כך הראוט קורא אותו + data={"code_file": (io.BytesIO(b"print('new')"), "uploaded.py"), + "file_name": "uploaded.py", "language": "python"}, + content_type="multipart/form-data", + follow_redirects=False, + ) + assert resp.status_code in (200, 302), resp.status_code + + latest = _latest(mongo_db, "uploaded.py") + assert latest["version"] == 2, "ההעלאה לא יצרה גרסה חדשה" + assert latest["created_at"] == ORIGIN + + +def test_shared_save_route_keeps_created_at(client, mongo_db, monkeypatch): + """שמירת קובץ משותף על שם קיים — "נוצר" יורש מהגרסה הקיימת.""" + import webapp.app as W + + _seed(mongo_db, "shared_doc.md", programming_language="markdown") + monkeypatch.setattr( + W, "get_internal_share", + lambda share_id: {"code": "# new content", "file_name": "shared_doc.md", + "language": "markdown", "description": "d"}, + ) + + resp = client.post("/api/shared/save", + json={"share_id": "abc", "file_name": "shared_doc.md"}) + assert resp.status_code == 200, resp.get_data(as_text=True) + assert json.loads(resp.get_data(as_text=True)).get("ok") is True + + latest = _latest(mongo_db, "shared_doc.md") + assert latest["version"] == 2 + assert latest["created_at"] == ORIGIN + + +def test_story_export_keeps_created_at(client, mongo_db): + """ייצוא סיפור לקובץ ``.md`` קיים — גרסה חדשה עם "נוצר" ישן. + + נקראת הפונקציה עצמה ולא הראוט: היא הבעלים של הכתיבה, והראוט רק + מספק לה סיפור. בדיקה דרך הראוט הייתה מוסיפה תלות בבניית הסיפור בלי + להוסיף כיסוי ליחידה שנבדקת. + """ + import webapp.app as W + + _seed(mongo_db, "story.md", programming_language="markdown") + + with W.app.test_request_context(): + result = W._persist_story_markdown_file( + user_id=USER_ID, file_name="story.md", markdown="# updated story" + ) + assert result, "הכתיבה לא דיווחה על הצלחה" + + latest = _latest(mongo_db, "story.md") + assert latest["version"] == 2 + assert latest["created_at"] == ORIGIN + + +def test_updated_badge_shows_for_an_edit_within_the_same_minute(client, mongo_db): + """קובץ שנערך **באותה דקה** שבה נוצר — "עודכן" חייב להופיע. + + ‏``format_datetime_display`` מעגל לדקות, ולכן השוואת המחרוזות + המפורמטות שהייתה בתבנית הכריזה "מעולם לא נערך" על עריכה אמיתית + והסתירה את השדה. ההכרעה עברה ל-``has_real_update`` שמשווה את + ה-``datetime`` הגולמי, ו-``has_update`` מועבר לתבנית. + """ + created = datetime(2024, 1, 1, 10, 30, 10, tzinfo=timezone.utc) + edited = created.replace(second=50) + file_id = _seed(mongo_db, "sameminute.py", created_at=created, updated_at=edited) + + html = client.get(f"/file/{file_id}").get_data(as_text=True) + assert 'id="metaUpdatedItem"' in html, "פריט 'עודכן' לא מרונדר כלל" + marker = html[html.index('id="metaUpdatedItem"'):] + marker = marker[: marker.index(">")] + assert "hidden" not in marker, '"עודכן" הוסתר למרות עריכה באותה דקה' + + +def test_updated_badge_hidden_when_the_file_was_never_edited(client, mongo_db): + """הכיוון השני: בלי עריכה, "עודכן" הוא רעש ולא מוצג.""" + stamp = datetime(2024, 1, 1, 10, 30, 10, tzinfo=timezone.utc) + file_id = _seed(mongo_db, "never_edited.py", created_at=stamp, updated_at=stamp) + + html = client.get(f"/file/{file_id}").get_data(as_text=True) + marker = html[html.index('id="metaUpdatedItem"'):] + marker = marker[: marker.index(">")] + assert "hidden" in marker, '"עודכן" מוצג על קובץ שמעולם לא נערך' diff --git a/tests/test_mcp_edit_append.py b/tests/test_mcp_edit_append.py index 1c1bb0eab..ec8d9c428 100644 --- a/tests/test_mcp_edit_append.py +++ b/tests/test_mcp_edit_append.py @@ -18,13 +18,24 @@ def __init__(self, doc=None): def get_file(self, user_id, *, file_name, file_id=None, version=None): return self.doc - def save_file(self, user_id, *, file_name, code, programming_language, description, tags=None): + def save_file(self, user_id, *, file_name, code, programming_language, description, + tags=None, update_existing=False): + """מדמה את **החוזה** של הבקנד, לא רק את החתימה שלו. + + הפייק הקודם החזיר ``ok`` ללא תנאי, ולכן המשיך לעבור כששער + ``file_exists`` נוסף לבקנד ו-``_resave_edited`` שכח את הדגל — + בזמן ש-``edit_file`` ו-``append_file`` נשברו בפרודקשן לגמרי. + פייק שלא מדמה את התנאי הוא ירוק שקרי. + """ + if self.doc is not None and not update_existing: + return {"ok": False, "error": "file_exists", "file_name": file_name} self.saved = { "file_name": file_name, "code": code, "programming_language": programming_language, "description": description, "tags": tags, + "update_existing": update_existing, } return {"ok": True, "created": False, "file": {"file_name": file_name, "version": 4}} diff --git a/tests/test_mcp_handlers.py b/tests/test_mcp_handlers.py index c49b35102..5dfc9cc12 100644 --- a/tests/test_mcp_handlers.py +++ b/tests/test_mcp_handlers.py @@ -35,8 +35,13 @@ def get_collection_items(self, user_id, *, collection_id, page, per_page, folder self.calls.append(("items", user_id, collection_id, page, per_page, folder)) return {} - def save_file(self, user_id, *, file_name, code, programming_language, description, update_existing=False): - self.calls.append(("save", user_id, file_name, code, programming_language, description)) + def save_file(self, user_id, *, file_name, code, programming_language, description, + update_existing=False): + # הדגל נרשם ולא נבלע: בלעדיו אי אפשר לאמת אף מסלול — לא שיצירה + # מעבירה False, ולא שעריכה מעבירה True. + self.calls.append( + ("save", user_id, file_name, code, programming_language, description, update_existing) + ) return {"ok": True, "created": True, "file": {"file_name": file_name, "version": 1}} @@ -112,7 +117,16 @@ def test_save_file_passes_explicit_language_and_trims_name(): be = _RecordingBackend() out = handlers.save_file(be, 7, file_name=" a.py ", code="print(1)", language="python") assert out["ok"] is True - assert be.calls[0] == ("save", 7, "a.py", "print(1)", "python", "") + # ברירת המחדל היא יצירה: ``update_existing`` חייב להישאר False, אחרת + # ``save_file`` היה דורס קובץ קיים בלי שהמשתמש ביקש. + assert be.calls[0] == ("save", 7, "a.py", "print(1)", "python", "", False) + + +def test_save_file_forwards_update_existing_when_asked(): + """הצד השני של אותו חוזה: הדגל עובר הלאה כשמבקשים אותו במפורש.""" + be = _RecordingBackend() + handlers.save_file(be, 7, file_name="a.py", code="x", language="python", update_existing=True) + assert be.calls[0][-1] is True def test_save_file_fills_a_language_when_omitted(): diff --git a/tests/test_note_boards_mongo.py b/tests/test_note_boards_mongo.py index 9cd9bb042..816772b5f 100644 --- a/tests/test_note_boards_mongo.py +++ b/tests/test_note_boards_mongo.py @@ -26,60 +26,18 @@ from __future__ import annotations -import os -import uuid from datetime import datetime, timezone import pytest -pymongo = pytest.importorskip("pymongo") +from pymongo.errors import DuplicateKeyError -from pymongo.errors import DuplicateKeyError, ServerSelectionTimeoutError # noqa: E402 +from mongo_it import make_mongo_db_fixture, requires_mongo -#: תחילית מסדי הבדיקה. ה-teardown מוחק **רק** מסד שמתחיל בה. -_TEST_DB_PREFIX = "codebot_notes_it_" +pytestmark = requires_mongo -_MONGO_URL = os.environ.get("MONGODB_URL", "").strip() - - -def _server_is_reachable(url: str) -> bool: - """האם יש שרת בקצה השני. בלי זה הבדיקות היו נתלות עד timeout ארוך.""" - try: - client = pymongo.MongoClient(url, serverSelectionTimeoutMS=2000, tz_aware=True, tzinfo=timezone.utc) - client.admin.command("ping") - client.close() - return True - except (ServerSelectionTimeoutError, Exception): - return False - - -pytestmark = pytest.mark.skipif( - not _MONGO_URL or not _server_is_reachable(_MONGO_URL), - reason="דורש MONGODB_URL עם שרת מונגו נגיש (קיים ב-CI, לא בהכרח מקומית)", -) - - -@pytest.fixture -def mongo_db(): - """מסד חד-פעמי, עם ``tz_aware=True`` בדיוק כמו בפרודקשן. - - ``tz_aware`` אינו פרט טכני: בלעדיו pymongo מחזיר ``datetime`` נאיבי, - וההשוואה שמחליטה על 409 (``prev_dt < note['updated_at']``) זורקת - ``TypeError`` — שנבלע ב-``except Exception``. כלומר בדיקת הקונקרנטיות - הייתה מפסיקה לרוץ בשקט. יש על כך בדיקה בהמשך הקובץ. - """ - name = f"{_TEST_DB_PREFIX}{uuid.uuid4().hex[:12]}" - client = pymongo.MongoClient(_MONGO_URL, tz_aware=True, tzinfo=timezone.utc) - db = client[name] - try: - yield db - finally: - # סורג בטיחות: מוחקים רק מסד שנוצר כאן - assert name.startswith(_TEST_DB_PREFIX), f"סירוב למחוק מסד שאינו של הבדיקות: {name}" - try: - client.drop_database(name) - finally: - client.close() +#: תחילית ייעודית לקובץ הזה — סורג המחיקה ב-``mongo_it`` נשען עליה. +mongo_db = make_mongo_db_fixture("codebot_notes_it_") @pytest.fixture diff --git a/webapp/app.py b/webapp/app.py index 752682eb1..a6f2fe228 100644 --- a/webapp/app.py +++ b/webapp/app.py @@ -5872,15 +5872,25 @@ def admin_config_inspector_page(): @app.route('/admin/migrations/created-at', methods=['GET', 'POST']) @admin_required def admin_migration_created_at(): - """מיגרציית "נוצר": dry-run והחלה, מהדפדפן בלבד. + """מיגרציית "נוצר": dry-run והחלה באצוות, מהדפדפן בלבד. "נוצר" של קובץ שייך לקובץ הלוגי ולא לגרסה. הקוד כבר מוריש את ``created_at`` קדימה בכל גרסה חדשה; העמוד הזה מיישר את הקבצים - שנוצרו לפני התיקון. ההחלה מותרת רק אחרי ש-dry-run הוצג — הטופס - נושא את מונה ה-dry-run כדי שההחלה תמיד תתייחס למה שנצפה. + שנוצרו לפני התיקון. + + **השער של "ראית dry-run" חי בסשן, לא בטופס.** שדה מוסתר הוא קלט + מהלקוח: כל POST של אדמין יכול לשאת ``dry_run_seen=1`` בלי שדבר + הוצג. במקום זה ה-dry-run מנפיק אסימון אקראי ושומר לצידו את מספר + הקבצים המושפעים; ההחלה מאמתת את שניהם, כך שמה שאושר הוא מה שיוחל. + אם קבוצת המושפעים השתנתה בינתיים — סירוב ובקשה להריץ dry-run מחדש. + + האסימון אינו הגנת CSRF (אין CSRF פעיל באפליקציה) אלא שער תהליכי: + הוא מוודא שההחלה נשענת על תמונת מצב שהוצגה בפועל. """ from services import created_at_migration as _mig + SESSION_KEY = 'created_at_migration_gate' + db = get_db() result = None error = None @@ -5889,14 +5899,43 @@ def admin_migration_created_at(): try: if action == 'dry_run': result = _mig.dry_run(db) + session[SESSION_KEY] = { + 'token': secrets.token_urlsafe(24), + 'affected': int(result.get('affected_count') or 0), + } + session.modified = True elif action == 'apply': - # ההחלה דורשת שה-dry-run רץ באותו טופס; בלי זה — סירוב. - if (request.form.get('dry_run_seen') or '') != '1': - error = 'יש להריץ dry-run לפני החלה.' + gate = session.get(SESSION_KEY) or {} + posted = str(request.form.get('gate_token') or '') + expected = str(gate.get('token') or '') + if not expected or not hmac.compare_digest(posted, expected): + error = 'יש להריץ dry-run לפני החלה (או שהאסימון פג).' else: - result = _mig.apply(db) + # חתימת התוצאה: אם קבוצת המושפעים השתנתה מאז ההצגה, + # מה שהאדמין אישר אינו מה שיוחל — לכן סירוב. + current = _mig.count_affected(db) + if current != int(gate.get('affected') or -1): + session.pop(SESSION_KEY, None) + error = ( + f'קבוצת הקבצים המושפעים השתנתה מאז ה-dry-run ' + f'({gate.get("affected")} ← {current}). הרץ dry-run מחדש.' + ) + else: + result = _mig.apply(db) + remaining = int(result.get('remaining_after') or 0) + if remaining > 0: + # ההחלה באצוות: האסימון נשאר תקף להמשך, עם + # חתימה מעודכנת ליתרה שנמדדה בפועל. + gate['affected'] = remaining + session[SESSION_KEY] = gate + else: + session.pop(SESSION_KEY, None) + session.modified = True else: error = 'פעולה לא מוכרת.' + except _mig.MigrationError as exc: + logger.exception('created_at migration reporting failed: %s', exc) + error = f'הדו"ח נכשל ולכן אין על מה להחליט: {exc}' except Exception as exc: logger.exception('created_at migration failed: %s', exc) error = 'המיגרציה נכשלה — ראו לוגים.' @@ -5905,7 +5944,13 @@ def admin_migration_created_at(): for row in result.get('samples', []) or []: row['current_created_disp'] = format_datetime_display(row.get('current_created')) row['new_created_disp'] = format_datetime_display(row.get('new_created')) - return render_template('admin_migration_created_at.html', result=result, error=error) + gate_token = (session.get(SESSION_KEY) or {}).get('token') or '' + return render_template( + 'admin_migration_created_at.html', + result=result, + error=error, + gate_token=gate_token, + ) @app.route('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/admin/cache-inspector') @@ -10273,6 +10318,24 @@ def _to_display_datetime(value) -> Optional[datetime]: # עיצוב תאריך בטוח לתצוגה ללא נפילה לברירת מחדל של עכשיו +def has_real_update(created_at, updated_at) -> bool: + """האם הקובץ נערך אי פעם — **על ה-datetime הגולמי**. + + ההשוואה חייבת לקרות כאן ולא בתבנית. ``format_datetime_display`` + מעגל לדקות, ולכן קובץ שנוצר ב-10:30:10 ונערך ב-10:30:50 מקבל שתי + מחרוזות זהות — ו"עודכן" היה נעלם למרות עריכה אמיתית. נמדד. + """ + try: + a = _to_display_datetime(created_at) + b = _to_display_datetime(updated_at) + if a is None or b is None: + return False + return b > a + except Exception: + # ספק ⇒ להציג. הסתרה שגויה מוחקת מידע מהמשתמש; הצגה מיותרת לא. + return True + + def format_datetime_display(value) -> str: try: dt = _to_display_datetime(value) @@ -12234,6 +12297,7 @@ def _fallback_files_created_at_page( 'lines': lines_count, 'created_at': format_datetime_display(latest.get('created_at')), 'updated_at': format_datetime_display(latest.get('updated_at')), + 'has_update': has_real_update(latest.get('created_at'), latest.get('updated_at')), 'last_opened_at': format_datetime_display(recent_map.get(fname)), }) @@ -12393,7 +12457,8 @@ def _fallback_files_created_at_page( 'size': format_file_size(size_bytes), 'lines': lines_count, 'created_at': format_datetime_display(file.get('created_at')), - 'updated_at': format_datetime_display(file.get('updated_at')) + 'updated_at': format_datetime_display(file.get('updated_at')), + 'has_update': has_real_update(file.get('created_at'), file.get('updated_at')) }) # רשימת שפות לפילטר - רק מקבצים פעילים @@ -12672,6 +12737,7 @@ def view_file(file_id): 'lines': len(code.split('\n')) if code else 0, 'created_at': format_datetime_display(file.get('created_at')), 'updated_at': format_datetime_display(file.get('updated_at')), + 'has_update': has_real_update(file.get('created_at'), file.get('updated_at')), 'version': (file.get('version', 1) if not is_large else None), 'is_large': is_large, 'can_pin': False, @@ -12704,6 +12770,7 @@ def view_file(file_id): 'lines': 0, 'created_at': format_datetime_display(file.get('created_at')), 'updated_at': format_datetime_display(file.get('updated_at')), + 'has_update': has_real_update(file.get('created_at'), file.get('updated_at')), 'version': (file.get('version', 1) if not is_large else None), 'is_large': is_large, 'can_pin': False, @@ -12765,6 +12832,7 @@ def view_file(file_id): 'lines': len(code.split('\n')) if code else 0, 'created_at': format_datetime_display(file.get('created_at')), 'updated_at': format_datetime_display(file.get('updated_at')), + 'has_update': has_real_update(file.get('created_at'), file.get('updated_at')), 'version': (file.get('version', 1) if not is_large else None), 'is_large': is_large, 'can_pin': not is_large, @@ -14660,9 +14728,15 @@ def _normalize_lang(name: str) -> str: 'tags': tags, 'version': version, # "נוצר" יורש מהגרסה הקודמת — עריכה אינה לידה מחדש. - # ``file`` הוא הנפילה לשינוי שם, שאז ``prev`` נשלף - # לפי השם החדש ויכול להיות ריק. - 'created_at': (prev or file or {}).get('created_at') or now, + # + # **נפילה לפי שדה, לא לפי מילון.** ``(prev or file)`` + # בוחר את ``prev`` ברגע שהוא מילון לא-ריק, וגם אם אין + # בו ``created_at`` כלל — ואז ``file`` לעולם לא נבדק + # והתאריך נופל ל-now. מסמכים ותיקים בלי השדה קיימים + # בפועל, ויש עליהם טסט. + 'created_at': ((prev or {}).get('created_at') + or (file or {}).get('created_at') + or now), 'updated_at': now, 'is_active': True, } diff --git a/webapp/templates/admin_migration_created_at.html b/webapp/templates/admin_migration_created_at.html index 09321700b..9ed1c3015 100644 --- a/webapp/templates/admin_migration_created_at.html +++ b/webapp/templates/admin_migration_created_at.html @@ -12,9 +12,15 @@ .mig-table th, .mig-table td { padding: .45rem .6rem; border-bottom: 1px solid var(--card-border); text-align: right; } .mig-stat { font-size: 1.15rem; } .mig-stat b { font-variant-numeric: tabular-nums; } -.mig-error { color: var(--danger-border, #b91c1c); } +/* ‏--danger הוא צבע הטקסט של הערכה; --danger-border נגזר ממנו ב-45% + שקיפות ונועד לגבולות, ולכן כטקסט הוא חלש מדי לקריאה. */ +.mig-error { color: var(--danger); } .mig-note { opacity: .8; font-size: .92rem; } .mig-table-wrap { overflow-x: auto; } +/* ‏.btn-danger אינה מוגדרת גלובלית (הכלל היחיד בריפו הוא מקומי + ל-collections), ולכן הכפתור ההרסני היה נראה ככפתור רגיל. */ +.mig-actions .btn-danger { background: var(--danger); border-color: var(--danger); color: var(--text-on-warning); } +.mig-actions .btn-danger:hover { background: color-mix(in srgb, var(--danger) 82%, var(--text-primary)); border-color: color-mix(in srgb, var(--danger) 82%, var(--text-primary)); } {% endblock %} @@ -23,8 +29,8 @@

🗓️ מיגרציית "נוצר"

-

"נוצר" של קובץ שייך לקובץ, לא לגרסה. הקוד כבר משמר אותו בעריכות חדשות; העמוד הזה מיישר קבצים שנערכו לפני התיקון: הגרסה האחרונה של כל קובץ תקבל את התאריך המוקדם ביותר מבין גרסאותיו. גרסאות ישנות אינן נגועות.

-

‏dry-run לקריאה בלבד — לא משנה דבר. ההחלה נפתחת רק אחרי שצפית בתוצאה שלו. שתי הפעולות נרשמות ל-audit.

+

"נוצר" של קובץ שייך לקובץ, לא לגרסה. הקוד כבר משמר אותו בעריכות חדשות; העמוד הזה מיישר קבצים שנערכו לפני התיקון: כל מסמך בגרסה הגבוהה ביותר של קובץ מקבל את התאריך המוקדם ביותר מבין גרסאותיו. גרסאות ישנות אינן נגועות.

+

‏dry-run לקריאה בלבד — לא משנה דבר. ההחלה נפתחת רק אחרי שצפית בתוצאה שלו, ורצה באצוות: כל לחיצה מטפלת בכמות חסומה ומדווחת כמה נותרו. ההחלה אידמפוטנטית, ולכן הרצה חוזרת בטוחה. שתי הפעולות נרשמות ל-audit.

{% if error %} @@ -33,13 +39,22 @@

🗓️ מיגרציית "נוצר"

- - {% if result and result.mode == 'dry_run' %} + {% if gate_token %} + {# האסימון מונפק ונשמר בסשן בצד השרת; כאן רק מוחזר. שדה מוסתר + לבדו אינו שער — האימות הוא מול הסשן. #} + + {% if result and result.mode == 'apply' %} + {% elif result and result.mode == 'dry_run' and result.affected_count %} + + {% endif %} {% endif %}
@@ -47,6 +62,9 @@

🗓️ מיגרציית "נוצר"

{% if result and result.mode == 'dry_run' %}

ייפגעו {{ result.affected_count }} קבצים מתוך {{ result.total_files }}.

+ {% if result.audit_error %} +

⚠️ רשומת ה-audit לא נכתבה: {{ result.audit_error }}

+ {% endif %} {% if result.samples %}
@@ -54,7 +72,7 @@

🗓️ מיגרציית "נוצר"

{% for s in result.samples %} - + @@ -71,11 +89,20 @@

🗓️ מיגרציית "נוצר"

{% if result and result.mode == 'apply' %}
-

תוכננו {{ result.planned }} · עודכנו בפועל {{ result.modified }} · נותרו לא-תואמים אחרי אימות: {{ result.remaining_after }}

- {% if result.remaining_after == 0 %} -

✅ האימות בקריאה חוזרת עבר — אין פערים שנותרו.

+

באצווה זו: {{ result.files_in_batch }} קבצים · {{ result.documents_planned }} מסמכים תוכננו · {{ result.modified }} עודכנו בפועל · נותרו אחרי אימות: {{ result.remaining_after }}

+ {% if result.done %} +

✅ האימות בקריאה חוזרת עבר — לא נותרו פערים.

{% else %} -

⚠️ נותרו פערים. הרץ dry-run שוב לראות מה נשאר, ואל תריץ החלה נוספת לפני שמבינים למה.

+

עוד יש עבודה. לחצו "המשך" לאצווה הבאה — הפעולה אידמפוטנטית.

+ {% endif %} + {% if result.audit_error %} +

⚠️ רשומת ה-audit לא נכתבה: {{ result.audit_error }}

+ {% endif %} + {% set ci = result.cache_invalidation %} + {% if ci and ci.failed %} +

⚠️ ביטול הקאש נכשל עבור {{ ci.failed }} מתוך {{ ci.attempted }} משתמשים{% if ci.error %} ({{ ci.error }}){% endif %} — ייתכן שרשימות ימשיכו להציג את התאריך הישן עד שה-TTL יפוג.

+ {% elif ci %} +

קאש בוטל עבור {{ ci.ok }} משתמשים.

{% endif %}
{% endif %} diff --git a/webapp/templates/files.html b/webapp/templates/files.html index 27a33589c..58286b213 100644 --- a/webapp/templates/files.html +++ b/webapp/templates/files.html @@ -368,8 +368,10 @@

נוצר: {{ file.created_at }} - {# עודכן==נוצר פירושו שהקובץ מעולם לא נערך — "עודכן" זהה הוא רעש #} - {% if file.updated_at != file.created_at %} + {# ``has_update`` מחושב ב-app.py על ה-datetime הגולמי. השוואת + המחרוזות שהייתה כאן עיגלה לדקות, וקובץ שנערך באותה דקה + שבה נוצר איבד את "עודכן". #} + {% if file.has_update %} עודכן: {{ file.updated_at }} {% endif %} diff --git a/webapp/templates/view_file.html b/webapp/templates/view_file.html index 90fbfde55..7b8008456 100644 --- a/webapp/templates/view_file.html +++ b/webapp/templates/view_file.html @@ -943,10 +943,12 @@

{{ file.file_name }}

נוצר
{{ file.created_at }}
- {# כשהקובץ מעולם לא עודכן, שני התאריכים זהים ו"עודכן" הוא רעש — - מציגים רק "נוצר". הפריט נשאר ב-DOM מוסתר, כי עריכת תיאור חיה - (quick-update) צריכה להחזיר אותו לתצוגה בלי רענון. #} -
+ {# כשהקובץ מעולם לא עודכן "עודכן" הוא רעש — מציגים רק "נוצר". הפריט + נשאר ב-DOM מוסתר, כי עריכת תיאור חיה (quick-update) צריכה להחזיר + אותו לתצוגה בלי רענון. + ``has_update`` מחושב ב-app.py על ה-datetime הגולמי: השוואת המחרוזות + שהייתה כאן עיגלה לדקות, והסתירה עריכה שקרתה באותה דקה. #} +
עודכן
{{ file.updated_at }}
From 5b22c19baa7a59f7df3ccd6abba8e778248cefdf Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 05:30:37 +0000 Subject: [PATCH 3/3] =?UTF-8?q?fix(view-file):=20"=D7=A2=D7=95=D7=93=D7=9B?= =?UTF-8?q?=D7=9F"=20=D7=94=D7=95=D7=A6=D7=92=20=D7=AA=D7=9E=D7=99=D7=93?= =?UTF-8?q?=20=E2=80=94=20hidden=20=D7=9C=D7=90=20=D7=94=D7=A1=D7=AA=D7=99?= =?UTF-8?q?=D7=A8,=20=D7=95=D7=91=D7=99=D7=98=D7=95=D7=9C=20=D7=94=D7=A7?= =?UTF-8?q?=D7=90=D7=A9=20=D7=93=D7=99=D7=95=D7=95=D7=97=20=D7=94=D7=A6?= =?UTF-8?q?=D7=9C=D7=97=D7=94=20=D7=9E=D7=93=D7=95=D7=9E=D7=94?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit השלמת מה שדילגתי עליו: קריאת הקבצים ב-amir-bug-patterns שהטריגרים שלהם נדלקו. הם מצאו שלושה ממצאים בקוד שכבר נדחף. **‏"עודכן" הוצג גם על קובץ שמעולם לא נערך — באג שקדם לכל ה-PR הזה.** ‏``base.html`` טוען את ``global_search.css`` בכל עמוד, ושם ``.meta-item{display:flex}`` מוגדר בלי תיחום לתוצאות החיפוש. הכרזת ``display`` של מחבר מנצחת את ``[hidden]{display:none}`` של ה-user-agent, ולכן ``hidden`` על פריט המטא היה חסר משמעות. נמדד ב-Chromium: ``getComputedStyle`` החזיר ``display:flex`` ותיבה של 283×56 על אלמנט שנשא ``hidden``. בדיקת שרת שקוראת את התכונה ב-HTML לא יכולה לראות את זה — זה בדיוק ``TESTING-PATTERNS`` T1(c). שתי ריצות בקרה בידדו את התרומה של כל חלק: על הקוד המקורי שני באגים ביטלו זה את זה ויצרו מראה תקין, ועם תיקון ה-CSS בלבד "עודכן" נעלם דווקא על קובץ שכן נערך. שני החלקים נדרשים יחד. נשאר בריפו שומר טקסטואלי עם שלוש מוטציות. **ביטול הקאש דיווח "בוטל עבור X משתמשים" על קריאה שאין לה ערוץ כשל.** ‏``invalidate_user_cache`` עוטף את כל גופו ב-``except Exception`` ומחזיר ``int`` — הוא אינו זורק. ה-``try/except`` שכתבתי סביבו לא היה רץ לעולם, המונה ``failed`` היה אפס מבנית, וענף האזהרה בתבנית היה קוד מת. עכשיו נספר ערך ההחזרה, ו-0 אינו מסומן ככשל (קאש קר תקין) בעוד שהיעדר Redis — שבו הניקוי חל רק על התהליך הזה — כן מדווח. **האסימון אינו חד-פעמי, ולכן ההצהרה תוקנה.** מדידה מקבילית: שתי בקשות עם אותו אסימון התקבלו שתיהן, כי הסשן הוא עוגייה חתומה שהשרת אינו יכול לבטל. מה שמגן על הנתונים הוא ש-``apply`` מחשב את קבוצת המושפעים מחדש. הבדיקה קובעת את מה שנכון בכל תזמון ולא כמה בקשות התקבלו, ועברה 8 ריצות רצופות. **הטסטים לוובאפ גוזרים את שדות הטופס מה-HTML** (T1(a)) במקום לכתוב אותם ביד — הניחוש הזה כבר הפיל אותי כאן על ``file`` מול ``code_file``. מוטציה בתבנית מפילה אותם עכשיו. ‏CLAUDE.md: שתי שורות טריגר וכלל תמידי חדש, כדי שהדפוסים האלה ייקראו בזמן המימוש ולא אחריו. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019YULCppaQRPYN1RgeY6NBu --- CLAUDE.md | 3 + services/created_at_migration.py | 52 +++++++-- tests/test_admin_migration_gate_mongo.py | 76 +++++++++++- tests/test_created_at_migration_cache.py | 110 ++++++++++++++++++ tests/test_created_at_webapp_routes_mongo.py | 77 +++++++++++- tests/test_view_file_hidden_meta_guard.py | 80 +++++++++++++ webapp/app.py | 11 +- .../templates/admin_migration_created_at.html | 12 +- webapp/templates/view_file.html | 13 +++ 9 files changed, 412 insertions(+), 22 deletions(-) create mode 100644 tests/test_created_at_migration_cache.py create mode 100644 tests/test_view_file_hidden_meta_guard.py diff --git a/CLAUDE.md b/CLAUDE.md index 29a24572a..40fd9c408 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,6 +51,8 @@ codekeeper_search_repo(repo="amir-bug-patterns", query="<מונח>") | PyGithub / קריאות SDK חיצוני | `BY-STACK/external-sdk.md` | | קבצי `docs/**/*.rst` | `bugbot-rules/line-number-coupling.md` | | טסטים עם סטאבים ידניים | `TESTING-PATTERNS.md` + `bugbot-rules/widened-exception-scope.md` | +| שינוי חתימה או חוזה של פונקציה שיש לה דאבל בטסטים (fake/stub/mock) | `TESTING-PATTERNS.md` T4 + `bugbot-rules/double-mirrors-signature-not-contract.md` | +| תכונה שמסתמכת על `hidden`, או כלל CSS שנטען גלובלית ועלול לדלוף לעמוד אחר | `TESTING-PATTERNS.md` T1(c) — בדיקת שרת רואה את התכונה, לא את ההתנהגות | | הרכבת URL/מחרוזת שמכילה סוד, הודעות חריגה, ניקוי לוגים/Sentry | `CRITICAL-PATTERNS.md` K13 + `bugbot-rules/secret-in-derived-text.md` | | מפתח/טוקן שמועבר כפרמטר URL (`params={"key": ...}`), או שינוי ברשימת דפוסי הניקוי | `CRITICAL-PATTERNS.md` K14 + `bugbot-rules/secret-in-url-query.md` | | מסיר שורת לוג, או עוטף אותה ב-guard שמונע הערכת ארגומנטים (הטריגר הנפוץ: תיקון PII) | `bugbot-rules/side-effect-riding-on-log-line.md` | @@ -61,6 +63,7 @@ codekeeper_search_repo(repo="amir-bug-patterns", query="<מונח>") הדפוס הזה כבר עלה בריפו הזה **שלוש פעמים** (`save_backup_bytes` ב-PR #3232 ב-#3172, ו-`delete_pattern` של הקאש). - **לפני כתיבת טסט חדש** → `claude-md-snippets/testing.md`. בפרט: טסט שנוסח עם תיקון חייב להיכשל בלי התיקון — הרץ אותו על הקוד הישן וּודא שהוא נופל. - **אחרי שטסט נופל על חריגה** → `bugbot-rules/widened-exception-scope.md`. אל תרחיב `except` כדי לעבור; בדוק קודם את הסטאב/fixture — שם השורש בדרך כלל. +- **אחרי ששינית חוזה של פונקציה** → עדכן את הדאבלים שלה כך ש**יאכפו** את החוזה החדש, לא רק יקבלו את החתימה. דאבל שממשיך להחזיר הצלחה ללא תנאי הוא בדיקה שאינה מסוגלת להיכשל. ובחר את חבילות הטסטים להרצה לפי **מי קורא לפונקציה**, לא לפי דפוס שם קובץ. עלה כאן ב-PR #3305: `edit_file` ו-`append_file` נשברו לגמרי וכל הטסטים נשארו ירוקים. ### סגירת הלולאה (חובה, לא רשות) diff --git a/services/created_at_migration.py b/services/created_at_migration.py index 71d153b5c..b364bf9fe 100644 --- a/services/created_at_migration.py +++ b/services/created_at_migration.py @@ -152,23 +152,47 @@ def _write_audit(db, doc: Dict[str, Any]) -> Optional[str]: def _invalidate_users(user_ids) -> Dict[str, Any]: - """מבטל קאש למשתמשים שנגענו בהם, באותו מנגנון של ``save_code_snippet``. + """מבטל קאש למשתמשים שנגענו בהם, ומדווח את **מה שנמדד**. - בלי זה רשימות הקבצים ממשיכות להיות מוגשות מהקאש עם התאריך הישן עד - שה-TTL פג. מדווחים כמה משתמשים טופלו וכמה נכשלו — לא מכריזים הצלחה. + ‏``cache.invalidate_user_cache`` עוטף את כל גופו ב-``except Exception`` + ומחזיר ``int`` — מספר המפתחות שנמחקו בפועל. כלומר הוא **אינו זורק**, + ולכן ``try/except`` סביבו הוא ``except`` שלא ירוץ לעולם, ו-"הקריאה + חזרה" אינו מידע. ערוץ הכשל היחיד שלו הוא ערך ההחזרה — וזה מה שנקרא + כאן (K11, ו-``return-value-failure-unchecked`` §4). + + **‏0 אינו מסומן ככשל.** לפי K11 הקובע הוא החוזה: מפתח קיים רק אם + מישהו שלף את רשימת הקבצים של המשתמש קודם, ולכן קאש קר הוא מצב + לגיטימי. מה שכן ניתן להבחין בו — ולכן מדווח בנפרד — הוא היעדר + backend קאש בכלל, שהוא כשל אמיתי שהיה מוסתר מאחורי "0 מפתחות". + + מחזיר ``users``, ``keys_deleted`` ו-``backend`` (האם יש קאש פעיל), + ו-``error`` כשה-import עצמו נכשל. """ - ok, failed = 0, 0 try: from cache_manager import cache # type: ignore except Exception as exc: - return {"attempted": len(user_ids), "ok": 0, "failed": len(user_ids), "error": str(exc)} + logger.warning("ביטול קאש למיגרציה נכשל: לא ניתן לטעון cache_manager: %s", exc) + return {"users": len(user_ids), "keys_deleted": 0, "backend": False, "error": str(exc)} + + # ``is_enabled`` הוא הדגל ש-``cache_manager`` מציב כשיש Redis חי, והוא + # מה ש-``delete_pattern`` עצמו בודק. הדוקסטרינג שלו קובע במפורש שכאשר + # הוא כבוי הניקוי חל **רק על הפולבק שבתהליך הזה**, ושבמצב הזה 0 אינו + # מבחין בין "לא היה מה למחוק" לבין "לא יכולתי לגשת". לכן מדווחים אותו + # לאדמין ולא מסתפקים במספר. + backend = bool(getattr(cache, "is_enabled", False)) + + keys_deleted = 0 for uid in user_ids: - try: - cache.invalidate_user_cache(int(uid)) - ok += 1 - except Exception: - failed += 1 - return {"attempted": len(user_ids), "ok": ok, "failed": failed} + keys_deleted += int(cache.invalidate_user_cache(int(uid)) or 0) + + if not backend: + logger.warning( + "המיגרציה עדכנה %d משתמשים בזמן ש-Redis אינו זמין — ביטול הקאש " + "חל רק על הפולבק שבתהליך הזה, ו-workers אחרים עשויים להמשיך " + "להגיש את התאריך הישן עד ש-TTL יפוג", + len(user_ids), + ) + return {"users": len(user_ids), "keys_deleted": keys_deleted, "backend": backend} def _sample_of(row: Dict[str, Any]) -> Dict[str, Any]: @@ -224,7 +248,11 @@ def apply(db, *, batch_size: int = DEFAULT_BATCH_SIZE) -> Dict[str, Any]: res = db.code_snippets.bulk_write(ops, ordered=False) modified = int(getattr(res, "modified_count", 0) or 0) - cache_report = _invalidate_users(sorted(user_ids)) if user_ids else {"attempted": 0, "ok": 0, "failed": 0} + cache_report = ( + _invalidate_users(sorted(user_ids)) + if user_ids + else {"users": 0, "keys_deleted": 0, "backend": True} + ) # אימות בקריאה חוזרת: כמה קבצים עדיין עומדים בתנאי. הצינור אינו רואה # את מה שכבר תוקן, ולכן זהו גם מונה ההתקדמות של האצוות הבאות. diff --git a/tests/test_admin_migration_gate_mongo.py b/tests/test_admin_migration_gate_mongo.py index ebb3be5b2..8ebbe709a 100644 --- a/tests/test_admin_migration_gate_mongo.py +++ b/tests/test_admin_migration_gate_mongo.py @@ -102,7 +102,7 @@ def test_gate_refuses_when_the_affected_set_changed_since_the_dry_run(client, mo assert _latest_created(mongo_db, "y.py") == LATER -def test_dry_run_then_apply_migrates_and_consumes_the_token(client, mongo_db): +def test_dry_run_then_apply_migrates_and_clears_the_token_from_the_session(client, mongo_db): _seed_broken(mongo_db, "x.py") dry = _text(client.post(URL, data={"action": "dry_run"})) token = _token(dry) @@ -113,7 +113,9 @@ def test_dry_run_then_apply_migrates_and_consumes_the_token(client, mongo_db): assert "האימות בקריאה חוזרת עבר" in body assert _latest_created(mongo_db, "x.py") == ORIGIN - # האסימון נצרך: הרצה חוזרת עם אותו אסימון נדחית + # האסימון נוקה מהסשן, ולכן **לקוח שממשיך עם העוגייה המעודכנת** נדחה. + # זו אינה חד-פעמיות: עותק ישן של העוגייה עדיין יעבור — ראו הבדיקה + # המקבילית בהמשך, שמודדת את זה במפורש. again = _text(client.post(URL, data={"action": "apply", "gate_token": token})) assert "יש להריץ dry-run" in again @@ -122,3 +124,73 @@ def test_get_does_not_offer_apply_before_a_dry_run(client, mongo_db): _seed_broken(mongo_db, "x.py") body = _text(client.get(URL)) assert 'value="apply"' not in body + + +def test_concurrent_applies_never_corrupt_the_data(client, mongo_db): + """שתי בקשות מקבילות עם אותו אסימון — הנתונים נכונים בכל תזמון. + + ‏``TESTING-PATTERNS`` T1(d): תכונת "חד-פעמי" נבדקת במקביל, לא ברצף. + ההרצה המקבילית גילתה שני דברים שהבדיקה הסדרתית הסתירה: + + 1. **השער אינו נעילה.** הסשן הוא עוגייה חתומה, ולכן השרת אינו יכול + לבטל עותק שכבר בידי הלקוח. נמדד: שתי בקשות מקבילות התקבלו שתיהן. + 2. **התוצאה תלוית-תזמון.** לפעמים בדיקת החתימה מספיקה לתפוס את + השנייה (היא מודדת ``count_affected`` מחדש, ואם הראשונה כבר סיימה + המספר השתנה), ולפעמים לא. מדדתי את שני המצבים על אותו קוד. + + לכן הבדיקה אינה קובעת כמה בקשות התקבלו — קביעה כזו הייתה flaky + מעצם היותה תלוית-תזמון. היא קובעת את מה שנכון תמיד: **הנתונים + נכונים, וההחלה אינה זוחלת בהרצה חוזרת**, כי ``apply`` מחשב את + קבוצת המושפעים מחדש בכל קריאה. + + השער נשאר תהליכי ולא נעילה **במכוון**: נעילה אמיתית דורשת מצב בצד + השרת, וההגנה האמיתית — אידמפוטנטיות — כבר קיימת ונבדקת. + """ + import threading + + for i in range(4): + _seed_broken(mongo_db, f"c{i}.py") + + token = _token(_text(client.post(URL, data={"action": "dry_run"}))) + assert token + + import webapp.app as W + + def _twin(): + """לקוח נפרד שנושא עותק של אותה עוגיית סשן — כמו לשונית שנייה.""" + twin = W.app.test_client() + for cookie in client._cookies.values(): + twin.set_cookie(cookie.key, cookie.value) + return twin + + bodies = {} + start = threading.Barrier(2) + + def run(idx): + c = _twin() + start.wait() + bodies[idx] = _text(c.post(URL, data={"action": "apply", "gate_token": token})) + + threads = [threading.Thread(target=run, args=(i,)) for i in range(2)] + for t in threads: + t.start() + for t in threads: + t.join() + + # אף בקשה לא קרסה, ואף אחת לא נדחתה בטענה שלא רץ dry-run. + assert len(bodies) == 2 + for body in bodies.values(): + assert "יש להריץ dry-run" not in body + + # הקביעה שנכונה בכל תזמון: המיגרציה הושלמה והנתונים נכונים. + for i in range(4): + assert _latest_created(mongo_db, f"c{i}.py") == ORIGIN + + from services import created_at_migration as mig + + assert mig.count_affected(mongo_db) == 0, "נשארו קבצים לא מתוקנים" + + # וההיסטוריה לא נפגעה: גרסה 1 שומרת על התאריך שלה. + for i in range(4): + v1 = mongo_db.code_snippets.find_one({"file_name": f"c{i}.py", "version": 1}) + assert v1["created_at"] == ORIGIN diff --git a/tests/test_created_at_migration_cache.py b/tests/test_created_at_migration_cache.py new file mode 100644 index 000000000..66fa70381 --- /dev/null +++ b/tests/test_created_at_migration_cache.py @@ -0,0 +1,110 @@ +"""ביטול הקאש במיגרציה מדווח **מה שנמדד**, לא היעדר חריגה. + +הרקע (K11 ו-``return-value-failure-unchecked`` §4): ``invalidate_user_cache`` +עוטף את כל גופו ב-``except Exception`` ומחזיר ``int``. הוא אינו זורק, ולכן +‏``try/except`` סביבו הוא ``except`` שלא ירוץ לעולם — וספירת "הקריאה חזרה" +אינה ספירה של מפתחות שנמחקו. הגרסה הקודמת בדיוק עשתה את זה, ואז העמוד +דיווח לאדמין "קאש בוטל עבור X משתמשים". + +הבדיקות כאן אינן נוגעות במונגו: היחידה הנבדקת היא פונקציה טהורה מעל +אובייקט הקאש, ולכן הקריאה הישירה **היא** הממשק (``TESTING-PATTERNS`` T1, +סעיף ה-false-positives). +""" + +from __future__ import annotations + +import pytest + +from services import created_at_migration as mig + + +class _CountingCache: + """קאש מדומה שמחזיר מספר ידוע — ומתעד את מי שנקרא עבורו. + + מדמה את **החוזה** של ``CacheManager``: לא זורק לעולם, ומסמן את + התוצאה בערך ההחזרה בלבד. + """ + + def __init__(self, per_user: int = 3, is_enabled: bool = True): + self.per_user = per_user + self.is_enabled = is_enabled + self.calls: list[int] = [] + + def invalidate_user_cache(self, user_id: int) -> int: + self.calls.append(user_id) + return self.per_user + + +@pytest.fixture +def use_cache(monkeypatch): + """מזריק קאש מדומה למודול ``cache_manager`` שהשירות מייבא ממנו.""" + + def _install(cache_obj): + import cache_manager + + monkeypatch.setattr(cache_manager, "cache", cache_obj, raising=False) + return cache_obj + + return _install + + +def test_report_counts_keys_actually_deleted(use_cache): + """המספר בדו"ח הוא סכום ערכי ההחזרה, לא מספר הקריאות שלא נפלו.""" + cache = use_cache(_CountingCache(per_user=3)) + + report = mig._invalidate_users([11, 22]) + + assert cache.calls == [11, 22] + assert report["users"] == 2 + assert report["keys_deleted"] == 6, "נספרו קריאות במקום מפתחות" + assert report["backend"] is True + + +def test_zero_keys_is_not_reported_as_failure(use_cache): + """קאש קר מחזיר 0 — וזה מצב תקין, לא כשל. + + לפי K11 הקובע הוא החוזה של הפונקציה: מפתח קיים רק אם מישהו שלף + קודם את רשימת הקבצים. סימון 0 ככשל היה מציף אזהרה על כל מיגרציה + שרצה על משתמש שלא נכנס לאתר. + """ + use_cache(_CountingCache(per_user=0)) + + report = mig._invalidate_users([11]) + + assert report["keys_deleted"] == 0 + assert report["backend"] is True + assert "error" not in report + + +def test_missing_redis_is_surfaced_even_though_the_call_succeeds(use_cache): + """בלי Redis הניקוי חל רק על התהליך הזה — וזה חייב להגיע לאדמין. + + זה ההבדל שהמימוש הקודם לא ידע לעשות: גם "קאש קר" וגם "אין backend" + נראו כמו הצלחה שקטה. + """ + use_cache(_CountingCache(per_user=1, is_enabled=False)) + + report = mig._invalidate_users([11]) + + assert report["backend"] is False + assert report["keys_deleted"] == 1 + + +def test_unimportable_cache_module_is_reported(monkeypatch): + """כשל ייבוא חוזר בשדה ``error`` ולא נבלע.""" + import builtins + + real_import = builtins.__import__ + + def _boom(name, *args, **kwargs): + if name == "cache_manager": + raise ImportError("no cache backend in this deployment") + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", _boom) + + report = mig._invalidate_users([11]) + + assert report["error"] == "no cache backend in this deployment" + assert report["backend"] is False + assert report["keys_deleted"] == 0 diff --git a/tests/test_created_at_webapp_routes_mongo.py b/tests/test_created_at_webapp_routes_mongo.py index 5535b2e37..ed4c5d1d8 100644 --- a/tests/test_created_at_webapp_routes_mongo.py +++ b/tests/test_created_at_webapp_routes_mongo.py @@ -7,6 +7,13 @@ הבדיקות מריצות את הראוטים **האמיתיים** דרך ``test_client`` מול מסד זמני, ומאמתות בקריאה חוזרת מה-DB — לא לפי ערך ההחזרה של הראוט. + +**השדות נגזרים מה-HTML שנוצר, לא נכתבים ביד.** ``TESTING-PATTERNS`` T1(a) +ו-``test-mirrors-spec-not-client`` §1: הצרכן של ``/edit`` ושל ``/upload`` +הוא משתמש שלוחץ על כפתור בטופס, ולכן בדיקה שמרכיבה POST משמות שקראתי +במקור מוכיחה שה-handler לא קורס — לא שהטופס עובד. זה כבר תפס אותי כאן: +ניחשתי ``file`` במקום ``code_file``. אחרי הגזירה, שינוי שם שדה בתבנית +מפיל את הבדיקה במקום להשאיר אותה ירוקה על מסלול שאיש אינו מריץ. """ from __future__ import annotations @@ -14,6 +21,7 @@ import io import json from datetime import datetime, timedelta, timezone +from html.parser import HTMLParser import pytest @@ -46,6 +54,61 @@ def client(mongo_db, monkeypatch): return c +class _FormReader(HTMLParser): + """גוזר ``action``, ``method`` ושמות השדות של טופס מתוך HTML מרונדר. + + ``html.parser`` מהספרייה הסטנדרטית — בלי תלות חדשה בשביל בדיקה. + """ + + def __init__(self, form_id: str): + super().__init__(convert_charrefs=True) + self._want = form_id + self._inside = False + self.action = None + self.method = None + self.fields: dict[str, str] = {} + + def handle_starttag(self, tag, attrs): + a = dict(attrs) + if tag == "form": + if a.get("id") == self._want: + self._inside = True + self.action = a.get("action") + self.method = (a.get("method") or "get").lower() + return + if self._inside and tag in {"input", "textarea", "select", "button"}: + name = a.get("name") + if name: + self.fields[name] = a.get("type") or tag + + def handle_endtag(self, tag): + if tag == "form" and self._inside: + self._inside = False + + +def read_form(html: str, form_id: str) -> _FormReader: + """מוצא טופס לפי ``id`` ומוודא שהוא באמת נמצא ושיש בו שדות.""" + reader = _FormReader(form_id) + reader.feed(html) + assert reader.action is not None or reader.method is not None, ( + f"הטופס {form_id!r} לא נמצא ב-HTML — הבדיקה לא בודקת שום מסלול" + ) + assert reader.fields, f"לטופס {form_id!r} אין שדות עם name" + return reader + + +def require_fields(reader: _FormReader, *names: str) -> None: + """נכשל אם שדה שהבדיקה מסתמכת עליו אינו קיים בטופס האמיתי. + + זה הקשר שהופך את הבדיקה למסוגלת להיכשל על שינוי בתבנית. + """ + missing = [n for n in names if n not in reader.fields] + assert not missing, ( + f"שדות שהבדיקה שולחת אינם קיימים בטופס: {missing}. " + f"קיימים: {sorted(reader.fields)}" + ) + + def _seed(mongo_db, file_name: str, *, version: int = 1, created_at=ORIGIN, **extra): """גרסה קיימת עם "נוצר" ותיק — נקודת ההשוואה של כל בדיקה.""" doc = { @@ -72,8 +135,12 @@ def test_edit_route_keeps_created_at(client, mongo_db): """‏POST ל-``/edit/`` יוצר גרסה חדשה עם ה"נוצר" של הקודמת.""" file_id = _seed(mongo_db, "edit_me.py") + # הצרכן הוא הטופס בעמוד — אז קוראים אותו, ולא מנחשים שמות שדות. + form = read_form(client.get(f"/edit/{file_id}").get_data(as_text=True), "editForm") + require_fields(form, "code", "file_name", "language", "description", "tags") + resp = client.post( - f"/edit/{file_id}", + form.action or f"/edit/{file_id}", data={"code": "changed", "file_name": "edit_me.py", "language": "python", "description": "desc", "tags": "a,b"}, follow_redirects=False, @@ -158,9 +225,13 @@ def test_upload_route_keeps_created_at(client, mongo_db): """העלאת קובץ בשם שכבר קיים — גרסה חדשה, "נוצר" ישן.""" _seed(mongo_db, "uploaded.py") + # שם שדה הקובץ נגזר מהטופס. בגרסה קודמת ניחשתי ``file`` והבדיקה נפלה; + # ``require_fields`` הופך את הניחוש הזה לבלתי אפשרי. + form = read_form(client.get("/upload").get_data(as_text=True), "uploadForm") + require_fields(form, "code_file", "file_name", "language") + resp = client.post( - "/upload", - # שם השדה הוא ``code_file`` — כך הראוט קורא אותו + form.action or "/upload", data={"code_file": (io.BytesIO(b"print('new')"), "uploaded.py"), "file_name": "uploaded.py", "language": "python"}, content_type="multipart/form-data", diff --git a/tests/test_view_file_hidden_meta_guard.py b/tests/test_view_file_hidden_meta_guard.py new file mode 100644 index 000000000..79dbb5d1b --- /dev/null +++ b/tests/test_view_file_hidden_meta_guard.py @@ -0,0 +1,80 @@ +"""שומר: ``hidden`` על פריט המטא בעמוד הקובץ באמת מסתיר. + +**מה זה שומר ומה זה לא.** הבדיקה הזו אינה מודדת — היא שומרת על מסקנה +שנמדדה בדפדפן. ‏Playwright אינו בתלויות הפרויקט, ולכן ההארנס עצמו לא +נשאר בריפו; מה שנשאר הוא הקביעה שההחלטה לא בוטלה בעריכה עתידית. + +**מה נמדד ולמה בדיקת שרת לא יכולה לתפוס את זה.** ‏``base.html`` טוען +את ``global_search.css`` בכל עמוד, ושם ``.meta-item{display:flex}`` +מוגדר בלי תיחום לתוצאות החיפוש. הכרזת ``display`` של מחבר מנצחת את +``[hidden]{display:none}`` של ה-user-agent — גם בלי ספציפיות גבוהה. +נמדד ב-Chromium: הפריט נשא ``hidden``, ``getComputedStyle`` החזיר +``display:flex``, ו-``getBoundingClientRect`` החזיר 283×56. כלומר +"עודכן" הוצג גם על קובץ שמעולם לא נערך. בדיקה שקוראת את ה-HTML +שהשרת החזיר רואה ``hidden`` ועוברת — הפער חי רק בדפדפן. + +הפתרון הוא באותה מוסכמה שכבר קיימת בריפו: ``note-boards.css`` מתעד +את הדפוס במפורש, ובקובץ הזה עצמו כבר יש +``.file-actions__dropdown[hidden]`` מאותה סיבה. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +TEMPLATE = Path(__file__).resolve().parents[1] / "webapp" / "templates" / "view_file.html" + + +@pytest.fixture(scope="module") +def template_text() -> str: + return TEMPLATE.read_text(encoding="utf-8") + + +def test_meta_item_has_an_explicit_hidden_rule(template_text): + """בלי הכלל הזה, ``hidden`` על ``.meta-item`` הוא חסר משמעות.""" + assert re.search( + r"\.meta-item\[hidden\]\s*\{[^}]*display\s*:\s*none", template_text + ), ( + "הכלל .meta-item[hidden]{display:none} הוסר. " + "‏global_search.css מגדיר .meta-item{display:flex} גלובלית, " + "ובלי הכלל הזה 'עודכן' מוצג גם על קובץ שמעולם לא נערך." + ) + + +def test_updated_item_visibility_is_driven_by_the_data_layer_flag(template_text): + """התבנית נשענת על ``has_update``, לא על השוואת מחרוזות מפורמטות. + + ‏``format_datetime_display`` מעגל לדקות, ולכן קובץ שנוצר ונערך באותה + דקה נראה כאילו מעולם לא נערך. נמדד בדפדפן: עם ההשוואה הישנה + + הכלל החדש, "עודכן" נעלם על קובץ שכן נערך. + """ + assert "{% if not file.has_update %}hidden{% endif %}" in template_text, ( + "פריט 'עודכן' חזר להישען על משהו שאינו has_update" + ) + assert "file.updated_at == file.created_at" not in template_text, ( + "חזרה להשוואת מחרוזות מפורמטות — היא מעגלת לדקות" + ) + + +def test_the_global_stylesheet_that_causes_this_is_still_loaded(template_text): + """אם ``global_search.css`` יתוחם או יוסר, השומר הזה מאבד את הסיבה שלו. + + הבדיקה מתעדת את התלות במפורש כדי שמי שיתחם אותו יידע שהוא יכול + לשקול להסיר את הכלל — ולא ימחק אותו בלי לדעת למה הוא שם. + """ + base = TEMPLATE.parent / "base.html" + loaded = "css/global_search.css" in base.read_text(encoding="utf-8") + unscoped = re.search( + r"^\s*\.meta-item\s*\{[^}]*display\s*:\s*flex", + (TEMPLATE.parents[1] / "static" / "css" / "global_search.css").read_text(encoding="utf-8"), + re.MULTILINE, + ) + if not (loaded and unscoped): + pytest.skip( + "‏global_search.css כבר אינו נטען גלובלית או ש-.meta-item שלו תוחם — " + "אפשר לשקול מחדש את .meta-item[hidden] בעמוד הקובץ" + ) + assert loaded and unscoped diff --git a/webapp/app.py b/webapp/app.py index a6f2fe228..ef302f233 100644 --- a/webapp/app.py +++ b/webapp/app.py @@ -5884,8 +5884,15 @@ def admin_migration_created_at(): הקבצים המושפעים; ההחלה מאמתת את שניהם, כך שמה שאושר הוא מה שיוחל. אם קבוצת המושפעים השתנתה בינתיים — סירוב ובקשה להריץ dry-run מחדש. - האסימון אינו הגנת CSRF (אין CSRF פעיל באפליקציה) אלא שער תהליכי: - הוא מוודא שההחלה נשענת על תמונת מצב שהוצגה בפועל. + **גבולות השער — נמדדו, לא הונחו.** הוא אינו הגנת CSRF (אין CSRF פעיל + באפליקציה) ואינו נעילה. הסשן הוא עוגייה חתומה, ולכן השרת אינו יכול + לבטל עותק שכבר בידי הלקוח: שתי בקשות מקבילות עם אותו אסימון מתקבלות + שתיהן (נמדד — שתיהן החזירו הצלחה), וכך גם עותק ישן של העוגייה. + מה שמונע נזק אינו השער אלא ש-``apply`` מחשב את קבוצת המושפעים מחדש + בכל קריאה, ולכן ההחלה השנייה מוצאת 0 לתקן. יש בדיקה על שני הדברים. + + השער הוא אפוא שער **תהליכי**: הוא מוודא שההחלה נשענת על תמונת מצב + שהוצגה בפועל, ולא שהיא תרוץ פעם אחת בלבד. """ from services import created_at_migration as _mig diff --git a/webapp/templates/admin_migration_created_at.html b/webapp/templates/admin_migration_created_at.html index 9ed1c3015..46398a2c1 100644 --- a/webapp/templates/admin_migration_created_at.html +++ b/webapp/templates/admin_migration_created_at.html @@ -99,10 +99,16 @@

🗓️ מיגרציית "נוצר"

⚠️ רשומת ה-audit לא נכתבה: {{ result.audit_error }}

{% endif %} {% set ci = result.cache_invalidation %} - {% if ci and ci.failed %} -

⚠️ ביטול הקאש נכשל עבור {{ ci.failed }} מתוך {{ ci.attempted }} משתמשים{% if ci.error %} ({{ ci.error }}){% endif %} — ייתכן שרשימות ימשיכו להציג את התאריך הישן עד שה-TTL יפוג.

+ {# מדווחים את מה שנמדד: כמה מפתחות נמחקו בפועל. "בוטל בהצלחה" היה + טענה על מצב שנשענת על קריאה שאין לה ערוץ כשל. 0 מפתחות אינו + כשל — קאש קר הוא מצב לגיטימי — אבל היעדר Redis כן, כי אז + הניקוי חל רק על התהליך הזה. #} + {% if ci and ci.error %} +

⚠️ ביטול הקאש לא רץ כלל ({{ ci.error }}) — רשימות עשויות להמשיך להציג את התאריך הישן עד שה-TTL יפוג.

+ {% elif ci and not ci.backend %} +

⚠️ ‏Redis אינו זמין. נמחקו {{ ci.keys_deleted }} מפתחות מהפולבק של התהליך הזה בלבד — ‏workers אחרים עשויים להמשיך להגיש את התאריך הישן עד שה-TTL יפוג.

{% elif ci %} -

קאש בוטל עבור {{ ci.ok }} משתמשים.

+

נמחקו {{ ci.keys_deleted }} מפתחות קאש עבור {{ ci.users }} משתמשים. ‏(0 אינו כשל — קאש קר הוא מצב תקין.)

{% endif %}
{% endif %} diff --git a/webapp/templates/view_file.html b/webapp/templates/view_file.html index 7b8008456..d9ecf6c56 100644 --- a/webapp/templates/view_file.html +++ b/webapp/templates/view_file.html @@ -492,6 +492,19 @@ border-radius: 10px; } +/* ``hidden`` מול ``display`` של מחבר: הדפדפן מגדיר ``[hidden]{display:none}`` + במקור ה-user-agent, וכל הכרזת ``display`` של מחבר מנצחת אותו. ל-``.meta-item`` + כאן אין ``display`` משלו, אבל ``global_search.css`` — שנטען גלובלית מ- + ``base.html`` — מגדיר ``.meta-item{display:flex}`` בלי לתחום אותו לתוצאות + החיפוש, והכלל הזה דולף לעמוד הזה. נמדד בדפדפן: הפריט נשא ``hidden`` + ובכל זאת ``getComputedStyle`` החזיר ``display:flex`` ותיבה של 283×56. + כלומר "עודכן" הוצג תמיד, גם על קובץ שמעולם לא נערך — בדיקת שרת שקוראת + את התכונה ב-HTML לא יכולה לראות את זה. + אותו פתרון כבר קיים בקובץ הזה ב-``.file-actions__dropdown[hidden]``. */ +.meta-item[hidden] { + display: none; +} + .meta-label { font-size: 0.9rem; opacity: 0.7;
{{ s.file_name }}{{ s.file_name }}{% if s.duplicate_latest %} ⧉{% endif %} {{ s.versions }} {{ s.current_created_disp }} {{ s.new_created_disp }}