Fix CloudEvents Extension Attributes - #69
Merged
Merged
Conversation
CloudEvents closes its attribute type system to seven types and maps them onto three JSON forms, so Double, Single, Decimal, and out-of-range Int64 are currently written in a form no CloudEvents type maps to. The plan records the decision to map through the type system rather than deviate, using String as the spec's escape hatch. Also covers the null rule in both directions, the character contract that String imposes, and the value-dependent Int64 encoding that is kept for its round trip and documented as a deviation. Refs #53 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrQRRJp3wRtHPgoKfngajd
… boundary Validating only in DefaultCloudEventsAttributeConversionService left the rule advisory: a custom conversion service or a directly constructed CloudEventsEnvelopeForWriting reaches the writer without it, while the complex-kind rule is absolute at the same site. Move enforcement into WriteExtensionAttributes ahead of the property name, share a private core with the public value writer so no attribute is scanned twice, and keep the validator public for services that prefer to fail before serialization starts. Refs #53 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrQRRJp3wRtHPgoKfngajd
The value-dependent encoding was justified against an alternative that was described as more costly than it is: TryGetInt64 could gain canonical-string parsing like TryGetUInt64, TryGetSingle, TryGetChar, and TryGetDecimal already have, which makes a stable String encoding cheap for consumers. Present both options honestly and decide for the threshold plus the parsing arm. The arm is what makes the threshold defensible: without it an out-of-range value writes as a string that TryGetInt64 refuses, so the accessor is now total across both encodings and only Kind stays observable. Refs #53 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrQRRJp3wRtHPgoKfngajd
0055-0 assigned the deferred Bytes kind to #53 on the premise that this issue would map MetadataKind onto the abstract CloudEvents types, with Binary as the first consumer. This plan models the JSON encoding instead, and Binary is not one: the JSON Event Format renders it as a base64 string, which is what a MetadataKind.String already produces. Re-defer Bytes explicitly to its own issue rather than leaving the earlier assignment orphaned, and note that its blockers - equality semantics and defensive copying - are metadata-system decisions that a conformance fix is the wrong venue to settle. Refs #53 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrQRRJp3wRtHPgoKfngajd
The plan said Utf8JsonWriter's handling of ill-formed UTF-16 is not this library's contract and then asked for a test pinning it. Such a test asserts an incidental and breaks on a System.Text.Json update that changes nothing here. Assert the rejection through the public serialization API instead, which is the behavior the library owns. Also scope the kind matrix to non-null primitives: an omitted null property has no JsonValueKind or text, and its coverage lives in the omission and unset criteria. Refs #53 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrQRRJp3wRtHPgoKfngajd
…e value A value-only overload cannot meet the failure contract: the caller must write the property name first, so validation cannot name the attribute, a failure leaves an incomplete property behind, and null omission is already impossible by the time the value is reached. Take the name and value together. That method owns validation, omission, WritePropertyName, and emission, which removes both the shared private core and the carve-out that wrote null when the value writer was called directly. Name policy splits on legitimacy: never-valid names are rejected there, while skipping standard names stays with the envelope writer that knows they were already emitted. Refs #53 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrQRRJp3wRtHPgoKfngajd
The criterion promised that no conformant attribute is scanned twice, while the design offers preflight validation in a custom conversion service. Since writer validation stays mandatory, preflighting necessarily adds a second pass, so the unscoped promise was unkeepable. Scope it to the built-in write path and state the trade at the seam: the second scan buys an ArgumentException at ToCloudEvent time, before any byte is serialized. Refs #53 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrQRRJp3wRtHPgoKfngajd
Standard-attribute writing was called spec-conformant while the same plan records that their text is never scanned for the characters CloudEvents excludes from a String. Claim only what holds: their JSON rendering is unaffected by this plan. The release-note section scoped every entry to extension attributes, but TryGetInt64 reading canonical text applies to any MetadataValue of kind String regardless of origin. Give that entry its own global scope. Refs #53 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrQRRJp3wRtHPgoKfngajd
…riter The public writer owned the whole attribute but only rejected data, data_base64, and lproutcome, so an invalid name reached the envelope whenever conversion was bypassed, and a standard name got the extension mapping applied to it: "type" with an in-range Int64 emits a JSON number where the specification requires a string. Reject every reserved and standard name and enforce the lowercase alphanumeric grammar. WriteExtensionAttributes still skips the standard names ahead of the call, so the guard exists for external callers and is tested directly. Also narrow the validity guarantee to the extension attributes the library emits, since standard-attribute character validation is deferred. Refs #53 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrQRRJp3wRtHPgoKfngajd
Cut the plan from ~5.6k to ~2.3k words. Every acceptance criterion and technical decision survives; the reduction comes from merging related criteria and trimming rationale prose, not from removing scope. Repairs three defects the condensing introduced: an inverted sentence about the decimal test replacement, an acceptance criterion that read as if TryGetInt64 had stopped accepting MetadataKind.Int64, and a reference to a nonexistent MetadataValue.String. Restores the implementation anchors and the two rejected-alternative arguments that the shortening lost: MetadataValue.cs:385, DefaultHttpHeaderParsingService, MetadataKindExtensions, TreatWarningsAsErrors, the System.Text.Json reason for not pinning Utf8JsonWriter behavior, the map-versus-document-the-deviation decision, and the always-String failure-mode argument. Records one new constraint found while checking those anchors: IsValidExtensionAttributeName is private on DefaultCloudEventsAttributeConversionService, so the name grammar has to move somewhere both it and the writer can call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UrQRRJp3wRtHPgoKfngajd
Map extension metadata through the CloudEvents JSON Event Format type system, omit null attributes, and validate names and string text before committing a property. Support canonical Int64 text on read-back, document the value-dependent encoding and stable converter remedy, and cover the public write/read surfaces plus allocation contracts. Closes #53 Signed-off-by: Kenny Pflug <kenny.pflug@live.de>
Remove an unreachable encoding fallback and close the warning suppression. Cover the public name and text validation gaps, and document reserved outcome handling in the release notes.
feO2x
changed the title
Minimum allowed line rate is |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #53