mikebridge opened a new pull request, #44015: URL: https://github.com/apache/superset/pull/44015
### SUMMARY `BaseRestoreVersionCommand.run()` wraps `_perform()` in `@transaction()`, but `_do_restore()` never locked the live row or otherwise coordinated with a concurrent writer. An overlapping edit (or a second restore) could commit between the command's read and its write, and the lost update was silent. `_do_restore()` now takes a `FOR UPDATE` lock on the live entity immediately after `validate()` and before resolving/reverting, then `db.session.refresh(entity)` under the lock so the revert — and the version-history delta it records — is computed against the current committed state rather than the pre-lock snapshot. The lock is held for the rest of the command's transaction (it runs on the same scoped session inside the `@transaction()` wrapper). The `FOR UPDATE` helper (`lock_entity_for_update`) is **moved** from `versioning/api_helpers.py` (REST-endpoint handlers) down to `versioning/queries.py` (the persistence layer, alongside `resolve_version`), so both the conditional-write PUT path and this command depend on it from a shared lower module rather than a command reaching up into the API layer; the command's import is now top-level. This is the pessimistic (serialize) half. The optimistic half — having the restore endpoint honor an `If-Match` precondition so a client can *detect* (412) a concurrent edit rather than merely serialize, as the PUT already does via `raise_for_stale_write` — is a separate, client-facing follow-up, noted in the code. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Not applicable — backend concurrency fix, no UI. ### TESTING INSTRUCTIONS - Unit: `tests/unit_tests/commands/test_restore_version_concurrency.py` — parametrized over the three concrete restore commands asserting `_do_restore` acquires the lock with the entity's id, plus an ordering test asserting the lock is taken before `restore_version`. Each fails when the lock is removed. `pytest tests/unit_tests/commands/test_restore_version_concurrency.py` (4 passed). - A real-DB test proving two transactions actually serialize on the row is a noted follow-up; the `FOR UPDATE` helper itself is exercised by the conditional-write PUT path it is shared with. - Changed-file pre-commit (mypy/ruff/ruff-format/pylint) clean. ### ADDITIONAL INFORMATION - [x] Has associated issue: sc-115423 (Preset-internal) - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration - [ ] Introduces new feature or API - [ ] Removes existing feature or API 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
