Skip to content

Refuse to save a subclass item that JSON would save as its base type [patch] - #113

Open
matt-edmondson wants to merge 1 commit into
mainfrom
fix/97-subclass-items-lost-on-save
Open

matt-edmondson wants to merge 1 commit into
mainfrom
fix/97-subclass-items-lost-on-save

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #97

What was wrong

JsonFilePersistenceProvider<T>.SaveStateAsync serialized items as the declared T. A NavigationItem subclass in a Navigation<NavigationItem> was saved without its type or the properties it adds, and LoadStateAsync still returned true. Nothing reported the loss.

Change

This takes the issue's "at minimum" option, which leaves the polymorphism setup to the consumer.

  • Before writing anything, SaveStateAsync checks each item. If an item's runtime type isn't T and the JSON options configure no polymorphism for T (GetTypeInfo(typeof(T)).PolymorphismOptions is null), it throws NotSupportedException. The message names the subclass and says how to fix it. The temp-file swap never starts, so the last good save stays in place.
  • When polymorphism is configured, System.Text.Json handles the item as before, including its own error for an unregistered derived type.
  • An interface or abstract T is skipped. That state can't be loaded back in any case, and LoadStateAsync already returns null for it (LoadStateAsync_InterfaceItemType_ReturnsNullInsteadOfThrowing), so its behaviour is unchanged.
  • A README section under "With Persistence" shows how to register derived types through jsonOptions.

Behaviour change: an app that saved subclass items without polymorphism configured now gets an exception from SaveStateAsync. Until now, that data was dropped without any report.

Tests

  • SaveStateAsync_SubclassItemWithoutPolymorphism_ThrowsNamingTheTypeAndKeepsTheLastSave fails on main (no exception, data dropped) and passes with the change.
  • SaveThenLoad_SubclassItemWithPolymorphismConfigured_RoundTripsItsTypeAndProperties checks the documented setup. A plain item and a subclass round-trip with their runtime types and the subclass's FilePath.

Full suite: 67 tests, 65 passed, 2 skipped (the unreadable-file tests are inconclusive when run as root).

Open PR #108 also edits SaveStateAsync, in the swap at its end, while this change adds a check at its start. Whichever merges second may need a small rebase.


Generated by Claude Code

…[patch]

JsonFilePersistenceProvider<T> serializes items as the declared T, so a
NavigationItem subclass in a Navigation<NavigationItem> lost its type and
the properties it adds, and LoadStateAsync still reported success.

SaveStateAsync now throws NotSupportedException naming the subclass when
T has no polymorphism configured, before touching the saved file, so the
last good save stays in place. The README shows how to configure
polymorphism through the JSON options, and a test covers that round trip.

Fixes #97

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW5pmswBjLMVcM1HgMb
@sonarqubecloud

sonarqubecloud Bot commented Nov 8, 2026

Copy link
Copy Markdown

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.

JsonFilePersistenceProvider silently drops NavigationItem subclass properties and restores plain NavigationItems

2 participants