fix(data-integrity): remove session.rollback() calls from ProjectRepository #10990

Merged
HAL9000 merged 4 commits from fix/8179-remove-session-rollback-calls into master 2026-06-14 15:59:54 +00:00

4 Commits

Author SHA1 Message Date
HAL9000 dd80d05558 fix(tests): align BDD scenarios with rollback-removal behaviour (PR #8179)
CI / lint (pull_request) Successful in 37s
CI / typecheck (pull_request) Successful in 1m5s
CI / push-validation (pull_request) Successful in 39s
CI / quality (pull_request) Successful in 1m8s
CI / build (pull_request) Successful in 53s
CI / helm (pull_request) Successful in 57s
CI / security (pull_request) Successful in 1m25s
CI / unit_tests (pull_request) Successful in 8m49s
CI / integration_tests (pull_request) Successful in 9m50s
CI / docker (pull_request) Successful in 1m47s
CI / coverage (pull_request) Successful in 12m55s
CI / status-check (pull_request) Successful in 3s
Three CI gates were failing on this PR; this commit addresses the root
causes for each:

* lint (ruff format): drop the blank line between the docstring close
  and first statement in step_pr_create_with_error, and add the missing
  second blank line between step_pr_check_remove_link_persisted and the
  "Data integrity BDD step extensions" section comment block.

* unit_tests: two scenarios were inverted by `@tdd_expected_fail` on
  post-fix assertions, masking unrelated test-logic problems.
  - Remove `@tdd_expected_fail` from both `@tdd_issue_8179` scenarios in
    project_repository.feature - they describe post-fix behaviour and
    must report PASS as PASS, not as inverted-FAIL.
  - Drop the "Given project exists" precondition from the Update-non-
    existent scenario; the Background already initialises the in-memory
    DB and creating the same project being "updated as non-existent" is
    self-contradictory (caused the prior scenario to silently report
    inverted-PASS while actually never raising).
  - Update the OperationalError scenario in database_repository_coverage
    to assert the post-fix invariant: the repository no longer calls
    session.rollback() itself; that responsibility is delegated to the
    outer UnitOfWork.  Step text + assertion both flipped.

ISSUES CLOSED: #8179
2026-06-14 11:36:15 -04:00
controller-ci-rerun f5261af868 chore: re-trigger CI [controller] 2026-06-14 11:36:15 -04:00
HAL9000 f0b374eb5d fix(data-integrity): address PR #10990 review feedback (PR #8179)
- Fix CI lint failure: remove unused IntegrityError import in BDD steps
- Append session.rollback() before session.close() in all NamespacedProjectRepository methods that own the session outside UoW
- Merge duplicate CHANGELOG ### Fixed sections into single header
- Apply @tdd_issue @tdd_issue_8179 @tdd_expected_fail tags to new BDD scenarios
- Consolidate near-duplicate step definitions; fix misleading docstring (NsP operates outside UoW)

ISSUES CLOSED: #8179
2026-06-14 11:36:15 -04:00
HAL9000 022b354359 fix(data-integrity): remove session.rollback() calls from ProjectRepository
Removed unconditional session.rollback() calls within exception handlers in:

- ProjectRepository.create()
- NamespacedProjectRepository.create() (IntegrityError handler)
- NamespacedProjectRepository.create() (OperationalError handler)
- NamespacedProjectRepository.update()
- NamespacedProjectRepository.delete()

The Unit of Work pattern already handles transaction rollback at the outer layer
via its except Exception: session.rollback() handler, making these inner rollbacks
redundant. SQLAlchemy automatically invalidates the transaction state when exceptions
occur after flush(), preventing partial data from being committed.

Removing the redundant rollbacks improves clarity, eliminates potential issues related
to exception chaining across retry boundary layers, and aligns repository implementations
with explicit transaction boundaries.

ISSUES CLOSED: #8179
2026-06-14 11:36:15 -04:00