Repository navigation
stjm: IJsonMigrationTracked flag is not set on struct targets #227
Description
Problem
When a struct target implements IJsonMigrationTracked, the converter never sets MigratedDuringDeserialization. A migrated struct reports false. As a result, the read-migrate-write-back pattern in the tracking recipe never writes migrated structs back. The recipe does not restrict the target kind, and a class target with the same members works.
Repro
[JsonMigratable(TypeDiscriminator = "source-v1")]
public readonly record struct SourceV1(int Value);
[JsonMigratable(TypeDiscriminator = "target-v2")]
public struct StructTarget : IMigrateFrom<SourceV1, StructTarget>, IJsonMigrationTracked
{
public int Value { get; set; }
[JsonIgnore]
public bool MigratedDuringDeserialization { get; set; }
public static bool TryMigrateFrom(SourceV1 source, out StructTarget result)
{
result = new StructTarget { Value = source.Value };
return true;
}
}var options = new JsonSerializerOptions(JsonSerializerDefaults.Web).AddJsonMigrationSupport();
var result = JsonSerializer.Deserialize<StructTarget>("""{"$type":"source-v1","value":42}""", options);
// result.Value == 42
// result.MigratedDuringDeserialization == false, expected trueOutput for the struct target and for a sealed class target with the same members:
struct target: Value=42 MigratedDuringDeserialization=False
class target: Value=42 MigratedDuringDeserialization=True
Reproduced on .NET 10 with the library at a6c07fd (main) and on #223 (afde4f1).
Cause
JsonMigratableConverter<T>.SetMigrationTracking:
private static void SetMigrationTracking(T? value, bool migratedDuringDeserialization)
{
// The interface keeps tracking opt-in so regular domain models stay free of migration concerns.
if (value is IJsonMigrationTracked tracked)
{
tracked.MigratedDuringDeserialization = migratedDuringDeserialization;
}
}When T is a value type:
valueis already a copy of the caller's local.- The pattern match boxes a second copy.
- The setter changes that box, which is then discarded.
Readreturns its local unchanged.
All four call sites in Read are affected. With [JsonIgnore] on the property, as the recipe recommends:
| Read path | Intended | Struct target gets |
|---|---|---|
| Migrated from a source | true |
whatever the migrator set, normally false |
| Legacy payload without discriminator | true |
false |
| Current format | false |
false, but only because it is the default |
Failed migration with FallBackToTargetType |
false |
false, but only because it is the default |
Suggested fix
Return the value, so the boxed copy can be written back:
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static T? WithMigrationTracking(T? value, bool migratedDuringDeserialization)
{
// The interface keeps tracking opt-in so regular domain models stay free of migration concerns.
if (value is IJsonMigrationTracked tracked)
{
tracked.MigratedDuringDeserialization = migratedDuringDeserialization;
// The pattern match boxed a value type, so the flag was set on the box. Unbox it back.
if (typeof(T).IsValueType)
{
return (T)tracked;
}
}
return value;
}return WithMigrationTracking(DeserializeTarget(ref reader, typeToConvert), migratedDuringDeserialization: true);typeof(T).IsValueType is a JIT-time constant, so reference-type targets compile to the same code as today. The pattern match still boxes tracked struct targets.
Tests
Add struct-target versions of the existing tracking tests in RegistrationAndTrackingTests:
Tracking_is_true_when_migratedmust fail before the fix.Tracking_is_true_when_discriminator_is_missingmust fail before the fix.Tracking_is_false_when_payload_matches_target_typeis a regression guard only. It passes today becausefalseis the default.
Notes
- Workaround until fixed: set
MigratedDuringDeserialization = trueinside the migrator'sTryMigrateFrom. That covers migrated payloads only. Legacy payloads without a discriminator have no workaround. - Ship the fix as its own
fix(stjm)commit with its tests, without[skip notes], because the change is user-visible. - Found while reviewing #223. That PR adds a
CanTrackMigrationguard to this method but keeps the same pattern match, so the bug is unchanged there.
Activity