Preserve unknown singular enum values in unknown fields in Swift - #3708
Preserve unknown singular enum values in unknown fields in Swift#3708loganblevins wants to merge 1 commit into
Conversation
When decoding with the .returnNil strategy, an unrecognized value in a singular (or oneof) enum field was read and discarded, so reencoding the message silently dropped the field. Repeated and map enum fields already preserve unrecognized values in unknown fields, as does generated Kotlin/Java code via EnumConstantNotFoundException, and proto2 semantics call for unknown enum values to be treated like unknown fields. Route singular unknown enum values into the current message frame's unknown fields, mirroring the repeated-field path, so they survive a decode/reencode round trip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🤖 One behavior change worth calling out explicitly for reviewers, since it isn't covered by the "Known pre-existing edge" section: After this PR, a stale client that explicitly edits a singular enum field decoded with an unrecognized value will have that edit overridden on the wire. Example ( Before this PR the same flow produced This is exactly what Wire Kotlin/Java already do today. Generated Kotlin catches Recommendation: merge with this added to the behavior-change note and pinned by a test ( Separately, a small nit in the "Known pre-existing edge" paragraph: the problematic order is |
Under
ProtoDecoder's.returnNilstrategy, an unrecognized value in a singular (or oneof) enum field is read and discarded: it never reachesunknownFields, so reencoding the message silently drops the field. This diverges from:ProtoReader.decode(into:));EnumConstantNotFoundExceptionand callsaddUnknownField(...)for singular fields (pinned byUnknownFieldsTest, which asserts the raw varint survives inunknownFields);.returnNildoc comment itself, which already described unknown values as "added to a collection for the same tag in unknown fields" — previously true only for collections.The practical impact is in fetch → decode → mutate → reencode → full-object update flows: when a server starts sending a new enum value, a stale client that edits any other field of the object silently clears the enum field on the way back up.
Change
Route the unrecognized raw value into the current message frame's unknown fields in the singular enum decode path, mirroring the existing repeated-field path (including its
currentTagguard). Runtime-only; no generator or generated-code changes.Tests
testDecodeUnknownEnumNilStrategynow asserts the preserved bytes;testDecodeUnknownEnumInOneOfNilStrategynow expects the value inunknownFields).RoundTripTestscovering a singular/oneof unknown enum (proto2 shape) and a proto3 field (interaction with the zero-value backfill), both asserting the reencoded bytes are identical to the input.swift test: full suite passing locally; all CI jobs green.Behavior change note
Messages decoded under
.returnNilnow carryunknownFieldsentries where they previously had none, which is observable throughEquatable/Hashableand reencoded bytes. Suggested CHANGELOG entry (under### Swift):Known pre-existing edge (disclosed, not addressed here)
If the same singular enum tag occurs twice on the wire as
[unknown, known], generated Swift assigns each decode result unconditionally, so the later unknown occurrence overwrites the previously decoded known value withnil(generated Kotlin'scatchskips the assignment; protobuf runtimes keep the recognized value). That generator shape predates this PR; preservation makes it observable across multiple decode/reencode hops. Happy to follow up with a generator change (skip assignment when the decode returnsnil) if full parity is wanted there.The Codable path's
JSONDecoder.EnumDecodingStrategy.returnNilis a distinct type and is unchanged.