Skip to content

After a save/load round trip, ChangeMetadata.CustomData values come back as JsonElement, so casting them to their original types throws InvalidCastException #120

Description

@matt-edmondson

What's wrong

#77 made a reloaded command keep its ChangeMetadata, including CustomData. The dictionary is typed IReadOnlyDictionary<string, object> (UndoRedo/Models/ChangeMetadata.cs:18), though, so System.Text.Json deserializes every value as a System.Text.Json.JsonElement. Values the app stored as int, string, DateTime and so on are no longer those types after LoadStateAsync. The keys survive, but every value changes type.

The regression test added for #77 (SerializationTests.cs:171) compares CustomData["author"].ToString(). That hides the problem, because JsonElement.ToString() happens to return the raw string.

Repro

var s = new UndoRedoService(new StackManager(), new SaveBoundaryManager(), new CommandMerger());
s.SetSerializer(new JsonUndoRedoSerializer());
s.Execute(new DelegateCommand("a", () => { }, () => { },
    customData: new Dictionary<string, object> { ["line"] = 42, ["name"] = "x" }));

await s.LoadStateAsync(await s.SaveStateAsync());

foreach (var kv in s.Commands[0].Metadata.CustomData!)
    Console.WriteLine($"{kv.Key}: {kv.Value.GetType().FullName} = {kv.Value}");
int line = (int)s.Commands[0].Metadata.CustomData["line"];

Observed:

line: System.Text.Json.JsonElement = 42
name: System.Text.Json.JsonElement = x
Unhandled: InvalidCastException: Unable to cast object of type 'System.Text.Json.JsonElement' to type 'System.Int32'.

Why it matters

CustomData is where apps keep things like a caret line or a selection id, which they read back to drive navigation or history UI. Code that casts or pattern-matches those values (is int line) works before a save and load, then throws or silently stops matching afterwards. Because the type depends on whether the history was reloaded, this is easy to miss in testing.

Suggested fix / acceptance criteria

  • On deserialize, convert each JsonElement in CustomData back into a plain CLR value, for example with a custom converter on ChangeMetadata.CustomData:
    • string → string
    • number → long, or double when it isn't a whole number
    • true/false → bool
    • array → List<object>
    • object → Dictionary<string, object>
  • Alternatively, store each value's type name next to it and deserialize to that type.
  • Document which types survive a round trip, for example that an int comes back as a long.
  • Change the SaveState/LoadState discards command metadata: timestamps reset to load time, ChangeSize to 1, CustomData lost #77 regression test to check each value's type and value, not its ToString(), and add a numeric entry.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions