Skip to content

[R39] Add collection-field copy and rename support to schema upgrades - #40

Open
JetF0x wants to merge 1 commit into
Carsillas:rewritefrom
JetF0x:fix/review-r39
Open

JetF0x wants to merge 1 commit into
Carsillas:rewritefrom
JetF0x:fix/review-r39

Conversation

@JetF0x

@JetF0x JetF0x commented Sep 12, 2026

Copy link
Copy Markdown

Addresses review finding R39.

Renaming a collection in a version upgrade currently fails because typed Get<NetworkList<T>> / Get<NetworkDictionary<TKey, TValue>> and Write do not support collections. CopyExcept can retain the original name, but cannot express the rename. This fixes R39 by adding NetworkObjectUpgradeWriter.Copy(sourceName, destinationName), which appends an existing source field's encoded value under a new name without decoding it. For example, a generated version-one Values = [3, 5] payload now migrates to version-two Items = [3, 5] through the public upgrade API.

writer.CopyExcept("Values", "Labels");
writer.Copy("Values", "Items");
writer.Copy("Labels", "Names");

Both names are validated and the source reader and field must exist. Source lookup uses the same ordinal, case-sensitive, last-duplicate-wins behavior as Get<T>. Copies use the existing raw writer, including its completion and 65,535-field guards. Copying appends a field; it does not remove source fields or replace previous output fields, so rename migrations exclude old names and any destination fields being replaced from CopyExcept. No public arbitrary raw-buffer writer is introduced.

Validation

  • Before the runtime change, the new public consumer failed compilation solely with CS1061 because Copy was absent. The original runtime audit also reproduced the typed collection rename/write failures and passing unchanged-copy control.
  • All 19 focused cases pass. Generated version-one serializers produce valid old payloads, and generated version-two deserialization runs the real upgrade methods for empty/nonempty lists and dictionaries, nullable values, object properties, nullable object collection elements, and nested collections. Tests verify the old names are excluded, destination names are present, and every copied value retains identical bytes. Unchanged CopyExcept also preserves the entire upgrade payload byte-for-byte.
  • Additional cases cover invalid names, absent readers/fields, ordinal and duplicate-source semantics, repeated/same-name copies, existing Write behavior, opaque empty fields, writer completion, and the existing field-count limit. Typed list/dictionary reads and writes remain explicitly tested as unsupported.
  • Full Release suite: 247 passed, 0 failed, 0 skipped (174 runtime, 20 generator, 53 analyzer), including the original 228 tests. Tested with SDK 10.0.400, .NET 8 runtime, and the existing Roslyn 5.6/C# 14 configuration. The existing CS8600 warning at MemoryRelayTransport.cs:21 remains; no dependency, SDK, language, or CI settings changed.

Compatibility and integration

The API is additive and changes no wire encoding. Existing Get, TryGet, Write, and CopyExcept behavior is preserved. Copy only operates on a present source field and preserves its bytes; it does not decode or transform collection contents, convert element types, or upgrade legacy encodings. Typed collection transformations remain unsupported, and the destination must accept the existing value encoding.

R10, R11, and R38 independently change value encodings. Raw copying cannot convert data written with their earlier encodings into a corrected encoding; any such stored-data conversion needs a separate compatible migration. This branch was tested independently, not as a combined integration of those fixes.

The README documents the copy API and remaining limits, and requires all upgrade reads/writes to finish synchronously before returning because generated deserialization then completes the writer. This section overlaps R31 (#36); when combining, retain R31's synchronous-upgrade contract and its CN0018 guidance for rejected async upgrade methods.

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