fix(versioning): refuse a chart restore whose datasource no longer exists

[mikebridge]

SUMMARY

Restoring a chart version whose datasource (a semantic view or dataset) had since been deleted reported success and left the chart pointing at nothing.

• The restore now refuses with a 422 when the snapshot's datasource no longer exists, and leaves the chart unchanged: no partial write and no new version row.
• The datasource row is locked in the restore's transaction before the chart is written, so a concurrent delete cannot slip in between the check and the write. The datasource is locked before the chart, the same order a rename uses.
• A soft-deleted dataset still counts as existing, since it can be restored. When a chart is restored onto one, its permission fields are now copied from that dataset; previously they kept the previous dataset's values (a stale chart permission after restore).

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A (backend).

TESTING INSTRUCTIONS

• pytest tests/unit_tests/versioning/test_restore.py tests/unit_tests/versioning/test_restore_missing_datasource.py tests/unit_tests/versioning/test_restore_datasource_race.py
• The tests run real capture-enabled restores: the missing-datasource refusal and its 422 mapping, the soft-deleted restore with all three permission fields asserted, a two-connection test where a delete races the restore on a file-backed SQLite database, and a lock-order test. Each fails without its fix.
• Not run: the concurrency behaviour on PostgreSQL and MySQL. SQLite serialises writers, so the row-lock path (FOR UPDATE) is exercised there only for statement shape.
• Known limit: a chart snapshot stores the datasource type and integer ID, not a UUID, so a purged datasource whose ID has been reused by a different row still passes the existence check.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
• Introduces new feature or API
• Removes existing feature or API

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 94.82759% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.43%. Comparing base (d6c871a) to head (78b38da).
⚠️ Report is 26 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/versioning/restore.py	93.33%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #44925      +/-   ##
==========================================
- Coverage   82.47%   82.43%   -0.05%     
==========================================
  Files        3002     3000       -2     
  Lines      186186   186415     +229     
  Branches    43122    43102      -20     
==========================================
+ Hits       153565   153675     +110     
- Misses      29866    29973     +107     
- Partials     2755     2767      +12     

Flag	Coverage Δ
hive	36.19% <20.68%> (-0.07%)	⬇️
mysql	55.09% <77.58%> (-0.13%)	⬇️
postgres	55.10% <77.58%> (-0.13%)	⬇️
presto	38.05% <25.86%> (-0.09%)	⬇️
python	86.41% <94.82%> (+0.08%)	⬆️
sqlite	54.83% <82.75%> (-0.13%)	⬇️
unit	79.52% <94.82%> (+0.13%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	4e9de65
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6ac563cd6f9e1e0008ce4d1a
😎 Deploy Preview	https://deploy-preview-44925--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[codeant-ai-for-open-source]

Suggestion: A legacy query datasource has no catalog_perm, and SavedQuery has no perm; restoring either raises AttributeError instead of completing.

Assessment: 🟠 Major · 🔁 Occurrence: Rarely · 🏷️ Type error

Prompt for AI Agent 🤖

This is a comment left during a code review.

**Path:** superset/versioning/restore.py
**Line:** 225:227
**Comment:**
	*Type Error: A legacy `query` datasource has no `catalog_perm`, and `SavedQuery` has no `perm`; restoring either raises `AttributeError` instead of completing.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

[codeant-ai-for-open-source]

✅ CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved as of 78b38da.

Permission fields are read with getattr(..., None), so missing catalog_perm or perm attributes no longer raise AttributeError.

_{If that's not right, unresolve this thread and CodeAnt will leave it open.}

[codeant-ai-for-open-source]

CodeAnt PR Risk: Low Risk

• The PR appears safe to merge; the datasource locking and missing-source refusal have focused regression tests.
• Optional datasource permission fields use safe defaults, and the added tests cover query-backed chart restores.

Assessed commit: 78b38da04218

[aminghadersohi]

Leaving this as a comment until the query/saved_query restore stops returning a 500 (inline below).

[aminghadersohi]

Both earlier findings are fixed at 78b38da. Approval is held only by the open codeant thread (its fix is in) and the required test-postgres-required check, which never ran in https://github.com/apache/superset/actions/runs/37652181604; a re-run should clear it.

[aminghadersohi]

Approving at 78b38da: CI green (incl. test-postgres-required), and both earlier findings are fixed; reverting either fix fails the new unknown-type and query/saved-query restore tests.