Skip to content

Sparse username index fix - #2121

Merged
amirbiron merged 5 commits into
mainfrom
cursor/sparse-username-index-fix-0ae7
Dec 13, 2025
Merged

amirbiron merged 5 commits into
mainfrom
cursor/sparse-username-index-fix-0ae7

Conversation

@amirbiron

@amirbiron amirbiron commented Dec 12, 2025 •

Copy link
Copy Markdown
Owner

✨ תיאור קצר

תיקון שגיאת E11000 duplicate key error ב-MongoDB עבור שמות משתמש null. השגיאה נגרמה מכך שאינדקס unique+sparse לא התעלם מערכי username: null שנשמרו במפורש. הפתרון כולל עדכון האינדקס ל-partialFilterExpression ושינוי ב-save_user שלא ישמור את השדה username אם הוא None או ריק.

📦 שינויים עיקריים

  • קוד (Backend)
  • בוט טלגרם
  • מסד נתונים/מיגרציות
  • תיעוד (docs/)
  • DevOps/CI/CD

פירוט נקודות:

  • עדכון האינדקס username_unique ב-database/manager.py מ-sparse=True ל-partialFilterExpression={"username": {"$type": "string", "$ne": ""}}.
  • שינוי ב-database/repository.py בפונקציה save_user כך שהשדה username לא יישמר במסד הנתונים אם הערך שלו הוא None או מחרוזת ריקה/רווחים בלבד.
  • הוספת בדיקות יחידה חדשות ב-tests/test_repository_save_user_username.py לוודא את ההתנהגות הנכונה של save_user (השמטת username כשאין ערך, ושמירתו כשקיים).
  • עדכון התיעוד ב-docs/database/index.rst כדי לשקף את השינוי באינדקס.

🧪 בדיקות

  • Unit - נוספו בדיקות יחידה חדשות לוודא ש-save_user משמיט את שדה ה-username כאשר הערך הוא None או ריק, ושומר אותו כראוי כשיש ערך.
  • Integration
  • Manual - בוצעה בדיקה ידנית של יצירת אינדקס וניסיון שמירת משתמשים עם username: null כדי לוודא שהשגיאה לא חוזרת.

🧪 בדיקות נדרשות ב‑PR

  • 🔍 Code Quality & Security
  • Unit Tests (3.11)
  • Unit Tests (3.12)

📝 סוג שינוי

  • fix: תיקון באג

✅ צ'קליסט

  • הקוד עוקב אחרי הסגנון (Black/isort/flake8/mypy)
  • בדיקות רצות ועוברות
  • תיעוד עודכן (README/Docs)
  • אם נוספו/שונו משתני סביבה – עודכן docs/environment-variables.rst (עמוד רפרנס)
  • אם נוספו/השתנו טוקנים – עודכן גם docs/webapp/theming_and_css.rst + FEATURE_SUGGESTIONS/theme_matrix.md
  • אין סודות/מפתחות בקוד
  • אין מחיקות מסוכנות/פעולות על root (ראו .cursorrules)
  • הודעת הקומיט תואמת Conventional Commits (ע"פ הטבלה)
  • CHANGELOG עודכן אם נדרש
  • כל ה‑Required Checks לעיל ירוקים
  • צילום/וידאו UI מצורף אם רלוונטי

🧩 השפעות/סיכונים

  • השפעה חיובית: משפר את יציבות מסד הנתונים ומונע שגיאות duplicate key עבור משתמשים ללא שם משתמש מוגדר.
  • סיכון: שינוי אינדקס קיים דורש מחיקה ויצירה מחדש. במערכת בפרודקשן, זה יכול להיות פעולה שוברת אם לא מבוצעת כמיגרציה מנוהלת או בזמן תחזוקה. במקרה זה, האינדקס ייווצר מחדש באתחול האפליקציה.

🔗 קישורים

  • Issues קשורים: #

🧯 סיכון / החזרה לאחור (Rollback)

  • החזרה לאחור של ה-PR. במקרה של תקלה חמורה עם האינדקס החדש, ניתן למחוק אותו ידנית ב-MongoDB Shell וליצור מחדש את האינדקס הישן (unique+sparse) או להריץ את גרסת הקוד הקודמת.

Open in Cursor Open in Web


Note

Updates username uniqueness/index + save behavior and adds Sentry alerts via secure webhook and background polling, with tests and docs.

  • Database:
    • database/manager.py: change users username_unique from sparse to partialFilterExpression to ignore null/empty values.
    • database/repository.py: save_user(user_id, username) now trims/limits username, omits writing when empty, and updates timestamps accordingly.
  • Observability / Web:
    • services/webserver.py: add POST /webhooks/sentry with HMAC/Bearer/token auth, in‑memory de‑dup, and mapping to internal alerts.
    • services/sentry_polling.py: new Sentry poller (configurable via env) emitting internal alerts on new issue activity.
    • main.py: schedule Sentry polling background job when enabled.
  • Tests:
    • tests/test_repository_save_user_username.py: assert omission/set logic for username.
    • tests/test_sentry_poller.py: verify seed-then-emit behavior.
    • tests/test_webserver_sentry_webhook.py: verify webhook auth paths and alert emission.
  • Docs:
    • docs/database/index.rst: update users indexes description to partial filter expression.

Written by Cursor Bugbot for commit 036461b. This will update automatically on new commits. Configure here.

Co-authored-by: amirbiron <amirbiron@gmail.com>
@cursor

cursor Bot commented Dec 12, 2025

Copy link
Copy Markdown
Contributor

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@github-actions

github-actions Bot commented Dec 12, 2025 •

Copy link
Copy Markdown
Contributor

🧯 Dangerous deletes guard report

Policy: see .cursorrules — dangerous deletions are blocked unless wrapped safely.

Summary:

  • Flagged findings (blocking): 0
    0
  • Excluded matches (not blocking): 15
  • Total matches (all files): 125

Flagged findings (file:line:snippet):
(none)

Excluded matches (by path pattern)
./Dockerfile:42:    rm -rf /var/lib/apt/lists/*
./Dockerfile:121:    rm -rf /var/lib/apt/lists/*
./webapp/static/js/md_preview.bundle.js.map:4:  "sourcesContent": ["// Markdown-it plugin to render GitHub-style task lists; see\n//\n// https://github.com/blog/1375-task-lists-in-gfm-issues-pulls-comments\n// https://github.com/blog/1825-t … [truncated]
./README.md:842:find . -name "__pycache__" -exec rm -rf {} +
./docs/DOCUMENTATION_GUIDE.md:453:rm -rf _build
./docs/Makefile:24:	rm -rf $(BUILDDIR)
./node_modules/mermaid/dist/chunks/mermaid.core/chunk-KS23V3DP.mjs.map:4:  "sourcesContent": ["{\n  \"name\": \"mermaid\",\n  \"version\": \"11.12.0\",\n  \"description\": \"Markdown-ish syntax for generating flowcharts, mindmaps, sequence  … [truncated]
./node_modules/mermaid/dist/chunks/mermaid.esm.min/chunk-4HFYJGYH.mjs.map:4:  "sourcesContent": ["{\n  \"name\": \"mermaid\",\n  \"version\": \"11.12.0\",\n  \"description\": \"Markdown-ish syntax for generating flowcharts, mindmaps, sequen … [truncated]
./node_modules/mermaid/dist/chunks/mermaid.esm.min/chunk-4HFYJGYH.mjs:1:var r={name:"mermaid",version:"11.12.0",description:"Markdown-ish syntax for generating flowcharts, mindmaps, sequence diagrams, class diagrams, gantt charts, git graph … [truncated]
./node_modules/mermaid/dist/chunks/mermaid.esm/chunk-2M32CCKP.mjs.map:4:  "sourcesContent": ["{\n  \"name\": \"mermaid\",\n  \"version\": \"11.12.0\",\n  \"description\": \"Markdown-ish syntax for generating flowcharts, mindmaps, sequence d … [truncated]
./node_modules/mermaid/dist/mermaid.min.js:1524:`,"getStyles"),c1e=RQe});var h1e={};dr(h1e,{diagram:()=>NQe});var NQe,f1e=N(()=>{"use strict";$ge();a1e();l1e();u1e();NQe={parser:Fge,db:n1e,renderer:o1e,styles:c1e}});var m1e,g1e=N(()=>{"use  … [truncated]
./node_modules/mermaid/dist/mermaid.js.map:4:  "sourcesContent": ["/**\n* Default values for dimensions\n*/\nconst defaultIconDimensions = Object.freeze({\n\tleft: 0,\n\ttop: 0,\n\twidth: 16,\n\theight: 16\n});\n/**\n* Default values for tr … [truncated]
./node_modules/mermaid/dist/mermaid.min.js.map:4:  "sourcesContent": ["/**\n* Default values for dimensions\n*/\nconst defaultIconDimensions = Object.freeze({\n\tleft: 0,\n\ttop: 0,\n\twidth: 16,\n\theight: 16\n});\n/**\n* Default values fo … [truncated]
./node_modules/katex/package.json:153:    "build": "rimraf dist/ && mkdirp dist && cp README.md dist && rollup -c --failAfterWarnings && webpack && node update-sri.js package dist/README.md",
./node_modules/katex/src/fonts/Makefile:139:	rm -rf pfa ff otf ttf woff woff2

@github-actions

github-actions Bot commented Dec 12, 2025 •

Copy link
Copy Markdown
Contributor

⏱️ Performance report

(No performance test durations collected. Mark tests with @pytest.mark.performance.)

@sentry

sentry Bot commented Dec 12, 2025 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 67.14286% with 92 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
services/webserver.py 65.98% 36 Missing and 14 partials ⚠️
services/sentry_polling.py 69.81% 17 Missing and 15 partials ⚠️
main.py 37.50% 9 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

cursoragent and others added 2 commits December 12, 2025 22:48
Co-authored-by: amirbiron <amirbiron@gmail.com>
Co-authored-by: amirbiron <amirbiron@gmail.com>
@amirbiron
amirbiron marked this pull request as ready for review December 12, 2025 23:42
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@amirbiron
amirbiron marked this pull request as draft December 12, 2025 23:42
@amirbiron
amirbiron marked this pull request as ready for review December 12, 2025 23:43
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Co-authored-by: amirbiron <amirbiron@gmail.com>
@github-actions

github-actions Bot commented Dec 12, 2025 •

Copy link
Copy Markdown
Contributor

📖 Documentation Preview

The documentation has been built successfully!

To view locally:

  1. Download the artifacts
  2. Extract the zip file
  3. Open index.html in your browser

Co-authored-by: amirbiron <amirbiron@gmail.com>
@amirbiron
amirbiron merged commit 9cc417a into main Dec 13, 2025
33 of 34 checks passed
@amirbiron

Copy link
Copy Markdown
Owner Author

@cursoragent
היי

2025-12-13 02:21:21 [error    ] db_save_user_error             command=webapp:get:collections.list_collections error='Updating the path \'username\' would create a conflict at \'username\', full error: {\'index\': 0, \'code\': 40, \'errmsg\': "Updating the path \'username\' would create a conflict at \'username\'"}' request_id=422ac0af user_id=533084436e046e6d

השגיאה הזו היא היפוך של השגיאה הקודמת, והיא מגיעה מאותו מקור, אבל בזמן פעולה שונה.
ההבדל מהשגיאה הקודמת

  • השגיאה הקודמת (E11000): קרה בזמן יצירת יוזר חדש (Insert), בגלל שהאינדקס הייחודי (username_unique) חסם שני יוזרים עם username: null.
  • השגיאה הנוכחית (code: 40): קורית בזמן עדכון יוזר קיים (Update), וגם היא נובעת מאותה בעיה בסיסית: ייחודיות (Unique constraint).
    ניתוח השגיאה: Code 40 Conflict
    error='Updating the path 'username' would create a conflict at 'username''
    מה קרה:
  • המשתמש עם user_id=533084436e046e6d קיים כבר ב-DB.
  • הקוד ניסה לבצע עליו פעולת עדכון (Update).
  • במהלך העדכון, הקוד ניסה להגדיר (או לעדכן) את השדה username לערך מסוים (סביר להניח שוב ל-null או לערך קיים אחר).
  • MongoDB זיהה: "אם אני מבצע את העדכון הזה, השדה username של המשתמש הזה יהיה זהה לשדה username של משתמש אחר שכבר קיים ב-DB. זה מפר את האינדקס הייחודי (username_unique)".
    מה זה אומר בפועל?
    האינדקס הייחודי עדיין קיים על השדה username, והוא עדיין חוסם אפשרות שיהיו שני מסמכים (יוזרים) עם אותו ערך בשדה זה.
    אם לא השתמשת בפתרון שהצעתי קודם (אינדקס דליל / sparse: true), הבעיה הזו תמשיך לצוץ בכל פעם שהמערכת מנסה לעדכן יוזר קיים:
  • אם היוזר עודכן ל-username: null: ה-DB כבר מכיל יוזר אחר עם username: null.
  • אם היוזר עודכן ל-username: 'TestUser': ה-DB כבר מכיל יוזר אחר עם username: 'TestUser'.
    הפתרון: חזרה ל-"אינדקס דליל" (Sparse Index)
    התיקון הוא בדיוק אותו תיקון קודם. כל עוד אתה לא מבטל את אכיפת הייחודיות על ערכי null, כל פעולה שתנסה להכניס או לעדכן יוזר עם username: null תיכשל (ברגע שיש כבר אחד כזה ב-DB).
    אנא ודא שהאינדקס הבא נוצר בהצלחה ב-MongoDB:

חשוב לוודא שהאינדקס הישן נמחק קודם, ורק אז יוצרים את החדש

db.users.create_index(
[("username", 1)],
unique=True,
sparse=True, # זה מה שפותר את הבעיה
name="username_unique"
)

סיכום: שתי השגיאות נובעות מאינדקס ייחודי שאינו דליל, שחוסם את האפשרות ליותר מיוזר אחד עם username ריק (null).

@cursor

cursor Bot commented Dec 13, 2025 •

Copy link
Copy Markdown
Contributor

צודק שזה “מאותו אזור” (השדה username), אבל כאן זו לא שגיאת ייחודיות ולא קשורה ל־username_unique.

מה באמת אומרת השגיאה code: 40?

Updating the path 'username' would create a conflict at 'username' מופיעה כשבאותה פעולת update_one(..., upsert=True) אנחנו מנסים לעדכן את אותו שדה פעמיים (למשל גם ב־$set וגם ב־$setOnInsert). MongoDB חוסם את זה עוד לפני הביצוע, גם אם המסמך כבר קיים.

אצלנו זה בדיוק מה שקרה ב־save_user: כש־username היה לא־ריק, שמנו אותו גם ב־$setOnInsert וגם ב־$set—וזה יוצר את הקונפליקט.

מה עשיתי כדי לתקן

  • הסרתי את username מ־$setOnInsert והשארתי אותו רק ב־$set.
    זה עדיין מכסה גם Insert (כי ב־upsert Mongo מכניס גם את $set למסמך החדש), וגם Update—בלי קונפליקט.
  • עדכנתי את הטסטים בהתאם והרצתי אותם—עוברים.

לגבי ההמלצה “לחזור ל־sparse”

לא הייתי חוזר ל־sparse פה: הוא לא פותר את בעיית username: null כששומרים null במפורש. הפתרון הנכון ל־E11000 נשאר partialFilterExpression + לא לשמור username כשהוא ריק/None.


Open in Cursor Open in Web

Learn more about Cursor Agents

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants