Skip to content

Refactor persistence manifest handling and add unit tests for loading behavior - #5867

Merged
rbev merged 1 commit into
masterfrom
scmu-sql-parse-issue
Sep 9, 2026
Merged

rbev merged 1 commit into
masterfrom
scmu-sql-parse-issue

Conversation

@warwickschroeder

@warwickschroeder warwickschroeder commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

An instance configured for SQL Server will not start. It dies with Could not load persistence customization type SQLServer. over a NullReferenceException.

No released version is affected.

What is wrong

PersistenceManifestLibrary reads every Persisters\<Name>\persistence.manifest in one loop wrapped by a single try, so the first file that throws discards every file after it, logged as a warning and nothing more.

The file that throws is RavenDB35. Its manifest has no AssemblyName or TypeName, correctly, because there is no assembly left to load, but the model marked both required and System.Text.Json rejects it. Folders are read in name order, so RavenDB35 throws third and SQLServer is never read. PostgreSQL sorts earlier and survives, which is what makes it look SQL-Server-specific.

It is invisible from a source checkout, where manifests come from DevelopmentPersistenceLocations and RavenDB 3.5 never appears. Only a real install shows it.

The fix

  • Per-file try/catch in a new LoadManifests, so a bad manifest costs you that manifest and nothing else.
  • AssemblyName and TypeName are nullable again. Name, DisplayName and Description keep required.

Tests

Two tests, one per defect, staging a real install layout in a temp directory. Each half of the fix was reverted separately to confirm the matching test goes red.

@rbev
rbev enabled auto-merge September 9, 2026 04:52
@rbev
rbev merged commit 6879ba9 into master Sep 9, 2026
133 of 138 checks passed
@rbev
rbev deleted the scmu-sql-parse-issue branch September 9, 2026 05:21
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.

4 participants