Skip to content

fix!: reject repeated provider registration and fix deletion log - #54

Merged
giammin merged 3 commits into
mainfrom
fix/review-followups
Oct 23, 2026
Merged

giammin merged 3 commits into
mainfrom
fix/review-followups

Conversation

@giammin

@giammin giammin commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Follow-up to #53 with the remaining code review findings. One commit per change; each commit builds and passes the full test suite.

  • dapper: PersistKeysWithDapperInSqlServer and PersistKeysWithDapperInPostgreSQL throw an InvalidOperationException when an IDbDataProtectionStorage is already registered. Both providers share one unnamed DapperDataProtectionConfig, so a second call silently replaced the first storage, overrode its configuration and ran both validators on it.
  • core: deleted keys are logged only after Delete() succeeds, as DeletedKeyFromStorage (event id 6, replacing DeletingKeyFromStorage; both are new in v2). A failed, rolled-back deletion no longer logs every key as deleted.
  • refactor: the SQL Server INSERT brackets its column names like every other statement, and both repositories build the SELECT once in the constructor.

README (single registration, deletion logging, v2 migration notes) and AGENTS.md are updated.

Not addressed: restoring the DeletableStorageWrapper constructor guard (removed on purpose in #50) and moving the duplicated provider registration into a shared file (not worth it with two providers).

BREAKING CHANGE: calling PersistKeysWithDapperInSqlServer or PersistKeysWithDapperInPostgreSQL more than once now throws an InvalidOperationException. Keep a single call.

PersistKeysWithDapperInSqlServer and PersistKeysWithDapperInPostgreSQL share
one unnamed DapperDataProtectionConfig. A second call, to either provider,
silently replaced the storage of the first one, overrode its configuration
and ran both validators on it. Both methods now throw an
InvalidOperationException when an IDbDataProtectionStorage is already
registered.

BREAKING CHANGE: calling PersistKeysWithDapperInSqlServer or
PersistKeysWithDapperInPostgreSQL more than once now throws an
InvalidOperationException. Keep a single call.
Each key was logged as being deleted before Delete() ran, so a failed,
rolled-back deletion still showed every key as deleted. The keys are now
logged after Delete() returns, as DeletedKeyFromStorage (event id 6), which
replaces DeletingKeyFromStorage; both events are new in v2.
The SQL Server INSERT now brackets its column names like every other
statement, and both repositories build the SELECT statement once in the
constructor instead of on every read.
@giammin
giammin merged commit 3028e98 into main Oct 23, 2026
1 check passed
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.

1 participant