Skip to content

Issue 4423 replace properties tx clean#6325

Closed
kazuhidelee wants to merge 5 commits into
mindersec:mainfrom
kazuhidelee:issue-4423-replace-properties-tx-clean
Closed

Issue 4423 replace properties tx clean#6325
kazuhidelee wants to merge 5 commits into
mindersec:mainfrom
kazuhidelee:issue-4423-replace-properties-tx-clean

Conversation

@kazuhidelee
Copy link
Copy Markdown
Contributor

Summary

ReplaceAllProperties runs a delete followed by a series of upserts. Before the change, if no transaction was provided with opts, getStoreOrTransaction returned the plain store and Postgres ran each statement in autocommit mode. so a failed upsert in middle would leave the entity with no properties at all.
The fix wraps the delete/insert sequence in a transaction when the caller hasn't already provided one, and reuses the existing querier when they have.

To keep the code organized and avoid duplication, I used helper functions for this implementation:

  • replaceAllPropertiesWithQuerier() for the delete-then-reinsert sequence
  • saveAllPropertiesWithQuerier() for the shared property upsert loop

Fixes #4423

Testing

New tests cover:

  • transaction is opened when called with plain store
  • existing transaction querier is reused
  • transaction wrapper errors surface correctly
  • original properties are preserved when a mid-reinsertion failure triggers rollback.

@kazuhidelee kazuhidelee requested a review from a team as a code owner April 10, 2026 04:45
@coveralls
Copy link
Copy Markdown

coveralls commented Apr 10, 2026

Coverage Status

coverage: 59.417% (-0.001%) from 59.418% — kazuhidelee:issue-4423-replace-properties-tx-clean into mindersec:main

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.

Ensure a transaction is open in ReplaceAllProperties routine

2 participants