Skip to content

Scope history record lookup by context key#176

Merged
msallin merged 1 commit into
masterfrom
fix/history-record-context-scope
Jun 16, 2026
Merged

Scope history record lookup by context key#176
msallin merged 1 commit into
masterfrom
fix/history-record-context-scope

Conversation

@msallin

@msallin msallin commented Jun 16, 2026

Copy link
Copy Markdown
Owner

Summary

SqliteDropCreateDatabaseWhenModelChanges writes the context name onto every history record and documents that a database may be shared by more than one context. The lookup, however, used SingleOrDefault() with no Context filter, so it ignored that scoping entirely.

On a history table that contains rows for more than one context the unscoped lookup matches multiple records:

  • IsSameModel catches the resulting InvalidOperationException and reports a model change (unnecessary drop/recreate).
  • SaveHistory does not guard it and throws.

Changes

  • GetHistoryRecord now looks the record up by context.GetType().FullName, matching the key SaveHistory writes.
  • Extracted the selection into a small internal HistoryRecordSelector helper so the logic is unit-testable without a database.

Tests

Added HistoryRecordSelectorTest: correct record returned for a given context, null when none match or the table is empty, and the one-record-per-context invariant surfaced as an exception if violated. Full suite green (51 tests).

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b277f780-a056-41a4-b88f-a963b4f557a3

📥 Commits

Reviewing files that changed from the base of the PR and between e019947 and c214040.

📒 Files selected for processing (3)
  • SQLite.CodeFirst.Test/UnitTests/Utility/HistoryRecordSelectorTest.cs
  • SQLite.CodeFirst/Internal/Utility/HistoryRecordSelector.cs
  • SQLite.CodeFirst/Public/DbInitializers/SqliteDropCreateDatabaseWhenModelChanges.cs
🚧 Files skipped from review as they are similar to previous changes (3)
  • SQLite.CodeFirst/Public/DbInitializers/SqliteDropCreateDatabaseWhenModelChanges.cs
  • SQLite.CodeFirst/Internal/Utility/HistoryRecordSelector.cs
  • SQLite.CodeFirst.Test/UnitTests/Utility/HistoryRecordSelectorTest.cs

📝 Walkthrough

Walkthrough

Adds an internal HistoryRecordSelector utility with a SelectForContext method that uses SingleOrDefault to find the IHistory record whose Context matches a given key. GetHistoryRecord in SqliteDropCreateDatabaseWhenModelChanges is updated to use this selector with context.GetType().FullName. Four unit tests cover the matching and edge-case behaviors.

Changes

Context-scoped history record selection

Layer / File(s) Summary
HistoryRecordSelector implementation and integration
SQLite.CodeFirst/Internal/Utility/HistoryRecordSelector.cs, SQLite.CodeFirst/Public/DbInitializers/SqliteDropCreateDatabaseWhenModelChanges.cs
Introduces an internal static HistoryRecordSelector.SelectForContext that matches IHistory records by Context via SingleOrDefault. GetHistoryRecord is updated to enumerate query results and delegate to this selector using context.GetType().FullName as the key.
Unit tests
SQLite.CodeFirst.Test/UnitTests/Utility/HistoryRecordSelectorTest.cs
Adds HistoryRecordSelectorTest with a Record factory helper and four tests: match found, no match, empty list, and duplicate-context throw.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐇 A hop through the history, context in paw,
SingleOrDefault — the cleanest of law.
No more ambiguity, no double match throw,
Each context finds just its own row below.
The rabbit tests all four: match, null, empty, and boom —
Tidy and tested, there's plenty of room! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Scope history record lookup by context key' clearly and concisely summarizes the main change: filtering history record lookups by context key rather than matching unscoped records.
Description check ✅ Passed The description is well-related to the changeset, explaining the context scoping issue, the problems it causes, the implemented solution, and the added tests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/history-record-context-scope

Comment @coderabbitai help to get the list of available commands and usage tips.

SqliteDropCreateDatabaseWhenModelChanges stores the context name on each
history record and documents that a database may be shared by several
contexts, but GetHistoryRecord selected with SingleOrDefault and no Context
filter. On a shared history table that matches multiple records: IsSameModel
swallows the resulting exception and reports a model change, while SaveHistory
throws. Look the record up by context.GetType().FullName, matching what
SaveHistory writes.
@msallin msallin force-pushed the fix/history-record-context-scope branch from e019947 to c214040 Compare June 16, 2026 04:21
@sonarqubecloud

Copy link
Copy Markdown

@msallin msallin merged commit 068a19c into master Jun 16, 2026
4 checks passed
@msallin msallin deleted the fix/history-record-context-scope branch June 16, 2026 04:23
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