diff --git a/Directory.Build.props b/Directory.Build.props index 23897ffc..5a00f4c3 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -10,6 +10,6 @@ Kenny Pflug Kenny Pflug Copyright (c) 2026 Kenny Pflug - 0.6.0 + 0.7.0 diff --git a/README.md b/README.md index e5b5dbb3..d3ca7dd3 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ *The Result Pattern for .NET that travels. Every `Result` serializes reliably over HTTP (with RFC-9457 Problem Details support), CloudEvents, and back — with a validation framework that is at least 5x faster and uses less than 9% of the memory of FluentValidation.* [![License](https://img.shields.io/badge/License-MIT-green.svg?style=for-the-badge)](https://github.com/feO2x/Light.PortableResults/blob/main/LICENSE) -[![NuGet](https://img.shields.io/badge/NuGet-0.6.0-blue.svg?style=for-the-badge)](https://www.nuget.org/packages?q=Light.PortableResults) +[![NuGet](https://img.shields.io/badge/NuGet-0.7.0-blue.svg?style=for-the-badge)](https://www.nuget.org/packages?q=Light.PortableResults) [![Documentation](https://img.shields.io/badge/Docs-Changelog-yellowgreen.svg?style=for-the-badge)](https://github.com/feO2x/Light.PortableResults/releases) Most Result Pattern libraries stop at the application boundary. Light.PortableResults does not: a `Result` can be written as an HTTP response (including RFC-9457 Problem Details support), published as a CloudEvents JSON message, read back from both protocols on the other side, and arrive as a fully-typed `Result` — without losing errors, metadata, or structure. If you also need validation, the built-in framework lets you write FluentValidation-style rules with a fraction of the allocations. Plus: Roslyn Source Generators write OpenAPI error schemas and examples for you. diff --git a/ai-plans/0052-decimal-metadata-kind.md b/ai-plans/0052-decimal-metadata-kind.md new file mode 100644 index 00000000..375c9afd --- /dev/null +++ b/ai-plans/0052-decimal-metadata-kind.md @@ -0,0 +1,113 @@ +# Decimal Metadata Kind + +## Rationale + +`MetadataValue.FromDecimal` converts its input to invariant text and stores `MetadataKind.String`. Decimals therefore serialize into JSON bodies as quoted strings, while `PortableOpenApiSchemaTypeMapper` maps `decimal` to `JsonSchemaType.Number` and `PortableResultsOpenApiDocumentTransformer` emits decimal examples as unquoted numbers. The published contract and the runtime payload disagree. The mismatch is reachable through the built-in validation error definitions: `CreateMetadataValue` routes every `TypeCode.Decimal` parameter through `FromDecimal`, and it backs the `ComparativeValue`, `LowerBoundary`, and `UpperBoundary` metadata keys. Any comparison or range check on a decimal-typed value — `IsGreaterThan(19.99m)`, `IsInRange(9.99m, 99.99m)` — therefore produces a `problem+json` body that violates the OpenAPI document generated for the same endpoint. + +Storing decimals as text also loses type information (a decimal is indistinguishable from a numeric-looking string), forces `TryGetDecimal` to run `decimal.TryParse` on every call, and allocates more than necessary. This plan introduces a dedicated `MetadataKind.Decimal` backed by a boxed `decimal`. + +## Acceptance Criteria + +- [x] `MetadataValue.FromDecimal` produces a value whose `Kind` is `MetadataKind.Decimal`. +- [x] A decimal metadata value is written into JSON bodies as an unquoted JSON number that preserves all significant digits and the original scale. +- [x] A `problem+json` body produced by a comparison or range validation rule on a decimal-typed value conforms to the OpenAPI document generated for the same endpoint, asserted by an integration test that inspects the raw response body. +- [x] `MetadataKind.Decimal` is classified as primitive, so decimals remain valid inside arrays annotated for header serialization and valid as CloudEvents extension attributes. +- [x] Every declared `MetadataKind` member is asserted to be classified correctly as primitive or complex by a test that enumerates the enum, so a member declared outside the reserved primitive range fails the build. +- [x] `TryGetDecimal` returns the stored value for `MetadataKind.Decimal` without parsing text, and continues to convert from `Int64`, `Double`, and numeric strings. +- [x] `TryGetString` returns `false` for a decimal metadata value. +- [x] `MetadataValue.ToString()` renders decimals as unquoted invariant-culture numeric text. +- [x] A decimal metadata value resolves correctly when used as a CloudEvents core string attribute instead of silently becoming `null`. +- [x] HTTP header formatting emits decimals without quote characters. +- [x] The JSON reader's treatment of numeric tokens is explicitly specified and covered by tests, including the documented cases where a decimal does not read back as `MetadataKind.Decimal`. +- [x] A test pins `Unsafe.SizeOf()` to the value it has before this plan, so any future change to the payload layout has to be deliberate. +- [x] Test code coverage stays above 95%. + +## Technical Details + +### Storage + +`MetadataKind.Decimal` stores a **boxed** `decimal` in the existing `Reference` slot of `MetadataPayload` at offset 8. An inline 128-bit field would push `Reference` to offset 16 and grow every `MetadataValue` by 8 bytes, including array elements stored inline in `MetadataArrayData` — an unacceptable cost in a library whose primary claim is reduced allocation. Boxing costs 24 bytes on x64, less than the 32–56 bytes the current string representation typically occupies, and removes the parse on every read. + +`double` remains the representation for general floating-point numbers. It covers the full JSON numeric range, where `decimal` is limited to ±7.9e28 and could not represent legitimate inbound values such as `1e100`. + +### Enum ordering is a hard constraint + +`MetadataKindExtensions.IsPrimitive` is implemented as `kind < MetadataKind.Array`, so membership of the primitive set is decided purely by ordering. Adding `Decimal` after `Object` compiles cleanly and silently classifies decimals as complex values: they would be rejected as CloudEvents extension attributes, rejected inside arrays annotated for header serialization, and would flip `HasOnlyPrimitiveChildren` to `false` on any containing array or object. No compiler diagnostic catches this. + +`Decimal` is therefore appended to the primitive block as `5`, and the complex kinds move to a reserved range: + +```csharp +public enum MetadataKind : byte +{ + Null = 0, Boolean = 1, Int64 = 2, Double = 3, String = 4, Decimal = 5, + // 6-199 are reserved for future primitive kinds + Array = 200, + Object = 201 +} +``` + +The gap exists so that later primitives can be added without renumbering the complex kinds a second time. This is not speculative: CloudEvents defines `Binary`, `URI`, `URI-reference`, and `Timestamp` as attribute types, all of which are currently flattened into `String`. The values are renumbered in this change because `Array` and `Object` move regardless, making the reservation free now and a separate breaking change later. It also settles the numbering before any gRPC mapping can make it wire-visible. + +The reserved range must be documented on the enum itself, and `IsPrimitive` must keep the boundary comparison rather than switching to an enumerated list — the comparison is a single instruction on a path used by every array and object construction. + +A gap alone does not enforce the invariant. `IsPrimitive` currently has no direct test coverage at all, so the ordering constraint is unguarded. A test must iterate `Enum.GetValues()` and assert each member against an explicit expected classification, so that a future member declared on the wrong side of the boundary fails immediately rather than degrading silently. + +### Read side: an explicit non-guarantee + +A JSON number carries no discriminator between a decimal and a double, so the reader must choose. Two candidate behaviours exist, and the choice must be recorded rather than implied: + +- **Default:** `SharedJsonSerialization.Reading.MetadataJsonReader.ReadNumber` keeps its current `Int64`-then-`Double` behaviour and never produces `MetadataKind.Decimal`. `Http.Reading.Json.MetadataJsonReader` delegates to it, so this is a single site. +- **Opt-in:** a reader option, following the precedent of `HeaderValueParsingMode`, that prefers `Decimal` for numeric tokens outside `Int64` range that fit in `decimal`. + +The default is chosen. Preferring `Decimal` unconditionally would make the resulting kind depend on the *magnitude* of the value and would break round-tripping in the opposite direction, with `FromDouble(0.1)` reading back as `Decimal`. + +This plan therefore improves **outbound** fidelity. It deliberately does not claim that a decimal round-trips as a decimal; that limitation is inherent to untyped numeric wire formats and is the same constraint recorded for HTTP headers in `0051`. Acceptance criteria must not assert round-trip symmetry for decimals. + +### Affected components: there is no compile-time safety net + +Every site that dispatches on `MetadataKind` has a `default` arm or a silent fall-through, so **adding the enum member breaks nothing at compile time**. A partial implementation ships silent data corruption rather than a build error. Each of these must be updated deliberately: + +| Site | Behaviour if left unhandled | Detected | +| --- | --- | --- | +| `SharedJsonSerialization/Writing/MetadataExtensions.WriteMetadataValue` | `default:` writes `WriteNullValue()` — a decimal serializes as JSON `null` | Silent | +| `MetadataValue.Equals` | `_ => false` — a decimal never equals another decimal | Silent | +| `MetadataValue.GetHashCode` | no `default` and no case — every decimal hashes to the kind alone | Silent | +| `CloudEventsResultExtensions.GetStringAttribute` | `if`-chain falls through to `null` — a decimal-valued `subject`, `type`, or `source` becomes `null` | Silent | +| `CloudEvents/MetadataValueAnnotationHelper.WithAnnotation` | `default:` throws `ArgumentOutOfRangeException` | Runtime | +| `MetadataValue.ToString()` | `_ =>` throws `InvalidOperationException` | Runtime | + +Two of these are worse than the defect being fixed. `WriteNullValue()` replaces a quoted string with `null`, losing the value entirely. `Equals` returning `false` alongside a kind-only hash breaks `MetadataObject` and dictionary lookups for decimals without any error surfacing. + +Everything else reaches decimals through `IsPrimitive` or the `TryGet*` accessors and needs no change. + +### Equality and hashing + +`MetadataKind.Decimal` joins the existing `String or Array or Object` group in `GetHashCode`, since a boxed `decimal` hashes correctly through `Reference.GetHashCode()`. `Equals` needs a branch that unboxes and compares as `decimal` rather than comparing references. + +This changes observable behaviour: `decimal.Equals` and `decimal.GetHashCode` are scale-insensitive and mutually consistent, so `19.50m` and `19.5m` become equal metadata values. Under the current string storage they compare as `"19.50"` and `"19.5"` and are *not* equal. Scale is still preserved for serialization — only equality changes. + +### Breaking changes + +The library is pre-1.0 and breaking changes are permitted, but these are silent at compile time for downstream callers and must be listed in the package release notes: + +- `TryGetString` no longer returns `true` for decimals. +- `Kind` for a decimal is no longer `MetadataKind.String`. +- The numeric values of `MetadataKind.Array` and `MetadataKind.Object` change to `200` and `201`. No code in the solution casts `MetadataKind` to a numeric type and the enum is not exposed by the OpenAPI, Validation, or source-generation packages, so this is invisible today — but any consumer that persisted or transmitted the numeric value is affected. +- Decimal metadata appears as a JSON number rather than a JSON string in serialized bodies. +- Decimals differing only in trailing zeros now compare equal. + +### Sequencing with #51 + +This plan should land before `0051-http-header-value-formatting`. That plan's kind table currently folds decimals into the `String` row; landing it first would require amending both the table and its test matrix immediately afterwards. With this plan first, `0051` gains a `Decimal` row from the outset — invariant numeric text, unquoted, identical in shape to the `Double` row. + +### Tests + +`MetadataValueTests.FromDecimal_ShouldStoreAsString` inverts and must be renamed alongside its assertions. `MetadataObjectTests` and `MetadataValueAnnotationTests` also construct decimal values and need review. + +Because no dispatch site fails to compile, the kind matrix must be exercised explicitly for every site in the table above rather than relying on the build. Equality and hashing in particular need a test that puts decimals into a `MetadataObject` and reads them back, since the failure there is silent in both directions. + +Beyond the unit-level matrix, the criterion that carries the actual defect is the OpenAPI conformance test: trigger a comparison or range validation failure on a decimal-typed value through an integration test app and assert that the raw `problem+json` body carries an unquoted number for the `ComparativeValue` or boundary metadata. + +### Out of scope + +Making decimals round-trip as decimals, and the opt-in reader mode described above. Both are follow-up work once the outbound representation is correct. diff --git a/samples/NativeAotMovieRating/packages.lock.json b/samples/NativeAotMovieRating/packages.lock.json index 14fb023f..e5490397 100644 --- a/samples/NativeAotMovieRating/packages.lock.json +++ b/samples/NativeAotMovieRating/packages.lock.json @@ -142,33 +142,33 @@ "light.portableresults.aspnetcore.minimalapis": { "type": "Project", "dependencies": { - "Light.PortableResults.AspNetCore.Shared": "[0.6.0, )" + "Light.PortableResults.AspNetCore.Shared": "[0.7.0, )" } }, "light.portableresults.aspnetcore.openapi": { "type": "Project", "dependencies": { - "Light.PortableResults.AspNetCore.Shared": "[0.6.0, )", + "Light.PortableResults.AspNetCore.Shared": "[0.7.0, )", "Microsoft.AspNetCore.OpenApi": "[10.0.10, )" } }, "light.portableresults.aspnetcore.shared": { "type": "Project", "dependencies": { - "Light.PortableResults": "[0.6.0, )" + "Light.PortableResults": "[0.7.0, )" } }, "light.portableresults.validation": { "type": "Project", "dependencies": { - "Light.PortableResults": "[0.6.0, )" + "Light.PortableResults": "[0.7.0, )" } }, "light.portableresults.validation.openapi": { "type": "Project", "dependencies": { - "Light.PortableResults.AspNetCore.OpenApi": "[0.6.0, )", - "Light.PortableResults.Validation": "[0.6.0, )" + "Light.PortableResults.AspNetCore.OpenApi": "[0.7.0, )", + "Light.PortableResults.Validation": "[0.7.0, )" } }, "Microsoft.Bcl.HashCode": { @@ -190,20 +190,20 @@ "contentHash": "V6crLJ8a29raWeNwxYGfH9RTKA3H0nR0D9LAGzN3KtEsbiiaWkUjDor6OT5Oz7pxCK+NaY2hu2FLoYEOa8oCkA==" } }, - "net10.0/linux-x64": { + "net10.0/osx-arm64": { "Microsoft.DotNet.ILCompiler": { "type": "Direct", "requested": "[10.0.10, )", "resolved": "10.0.10", "contentHash": "tnG8ntt/Bk6odvHREnGLMo3PEiihy5iSlIFVp0JbIo00GKtNRt2k73eKZbPqR5yaJNIa3z8R86YLwbxfqpb17g==", "dependencies": { - "runtime.linux-x64.Microsoft.DotNet.ILCompiler": "10.0.10" + "runtime.osx-arm64.Microsoft.DotNet.ILCompiler": "10.0.10" } }, - "runtime.linux-x64.Microsoft.DotNet.ILCompiler": { + "runtime.osx-arm64.Microsoft.DotNet.ILCompiler": { "type": "Transitive", "resolved": "10.0.10", - "contentHash": "WRjSRBfv6A6UjgjO8EQuLe9xqdICpkQx1hACUziCw4B2uGL+2jVhkFLq/G7rxRr3MGvqLo9B+nNdfIJ/5CYN7A==" + "contentHash": "cY7edFqVviQMiSvPodJeLZhF6k56grh0QsvcR+2foAUgO0xXsMsVBQiHcZkeiIbhr+SPIklq90OZiJ7JVrc+Dg==" } } } diff --git a/src/Light.PortableResults.AspNetCore.MinimalApis/Light.PortableResults.AspNetCore.MinimalApis.csproj b/src/Light.PortableResults.AspNetCore.MinimalApis/Light.PortableResults.AspNetCore.MinimalApis.csproj index f04b50fd..4ce24234 100644 --- a/src/Light.PortableResults.AspNetCore.MinimalApis/Light.PortableResults.AspNetCore.MinimalApis.csproj +++ b/src/Light.PortableResults.AspNetCore.MinimalApis/Light.PortableResults.AspNetCore.MinimalApis.csproj @@ -4,7 +4,7 @@ true Integration package for turning result instances into Minimal API's IResult instances. Compatible with Native AOT. Compatible with RFC 9457 (and RFC 7807) Problem Details responses. - Light.PortableResults.AspNetCore.MinimalApis 0.6.0 + Light.PortableResults.AspNetCore.MinimalApis 0.7.0 --------------------------------- - LightResult and LightResult<T> and corresponding extension methods to turn result instances into HTTP success responses or RFC 9457 (and RFC 7807) compatible Problem Details responses. diff --git a/src/Light.PortableResults.AspNetCore.Mvc/Light.PortableResults.AspNetCore.Mvc.csproj b/src/Light.PortableResults.AspNetCore.Mvc/Light.PortableResults.AspNetCore.Mvc.csproj index 25f8203a..b200f447 100644 --- a/src/Light.PortableResults.AspNetCore.Mvc/Light.PortableResults.AspNetCore.Mvc.csproj +++ b/src/Light.PortableResults.AspNetCore.Mvc/Light.PortableResults.AspNetCore.Mvc.csproj @@ -3,7 +3,7 @@ Integration package for turning result instances into MVC's IActionResult instances. Compatible with RFC 9457 (and RFC 7807) Problem Details responses. - Light.PortableResults.AspNetCore.MVC 0.6.0 + Light.PortableResults.AspNetCore.MVC 0.7.0 --------------------------------- - LightActionResult and LightActionResult<T> and corresponding extension methods to turn result instances into HTTP success responses or RFC 9457 (and RFC 7807) compatible Problem Details responses. diff --git a/src/Light.PortableResults.AspNetCore.OpenApi/Light.PortableResults.AspNetCore.OpenApi.csproj b/src/Light.PortableResults.AspNetCore.OpenApi/Light.PortableResults.AspNetCore.OpenApi.csproj index 37ceea2c..4edf7980 100644 --- a/src/Light.PortableResults.AspNetCore.OpenApi/Light.PortableResults.AspNetCore.OpenApi.csproj +++ b/src/Light.PortableResults.AspNetCore.OpenApi/Light.PortableResults.AspNetCore.OpenApi.csproj @@ -4,7 +4,7 @@ true Opt-in OpenAPI integration package for Light.PortableResults ASP.NET Core applications. Provides a library-authored schema catalog, endpoint metadata attributes, Minimal API helpers, and a document transformer. - Light.PortableResults.AspNetCore.OpenApi 0.6.0 + Light.PortableResults.AspNetCore.OpenApi 0.7.0 ------------------------------------- - Opt-in OpenAPI integration via IServiceCollection.AddPortableResultsOpenApi. diff --git a/src/Light.PortableResults.AspNetCore.Shared/Light.PortableResults.AspNetCore.Shared.csproj b/src/Light.PortableResults.AspNetCore.Shared/Light.PortableResults.AspNetCore.Shared.csproj index 9ef99dab..deed6dd1 100644 --- a/src/Light.PortableResults.AspNetCore.Shared/Light.PortableResults.AspNetCore.Shared.csproj +++ b/src/Light.PortableResults.AspNetCore.Shared/Light.PortableResults.AspNetCore.Shared.csproj @@ -4,7 +4,7 @@ true The Light.PortableResults.AspNetCore.Shared package contains shared functionality for writing ASP.NET Core HTTP responses. Compatible with Native AOT. Check out the integration packages Light.PortableResults.AspNetCore.MinimalApis or Light.PortableResults.AspNetCore.Mvc. - Light.PortableResults.AspNetCore.Shared 0.6.0 + Light.PortableResults.AspNetCore.Shared 0.7.0 --------------------------------- - Result enrichment with ASP.NET Core's HttpContext. diff --git a/src/Light.PortableResults.Validation.OpenApi/Light.PortableResults.Validation.OpenApi.csproj b/src/Light.PortableResults.Validation.OpenApi/Light.PortableResults.Validation.OpenApi.csproj index 8af97d9b..6fbbe305 100644 --- a/src/Light.PortableResults.Validation.OpenApi/Light.PortableResults.Validation.OpenApi.csproj +++ b/src/Light.PortableResults.Validation.OpenApi/Light.PortableResults.Validation.OpenApi.csproj @@ -4,7 +4,7 @@ true OpenAPI bridge package for Light.PortableResults.Validation. Provides built-in validation error metadata contracts and typed helpers for endpoint-specific validation error narrowing. - Light.PortableResults.Validation.OpenApi 0.6.0 + Light.PortableResults.Validation.OpenApi 0.7.0 ---------------------------------------- - Built-in OpenAPI metadata contracts for Light.PortableResults.Validation error codes. diff --git a/src/Light.PortableResults.Validation/Light.PortableResults.Validation.csproj b/src/Light.PortableResults.Validation/Light.PortableResults.Validation.csproj index 4b2d7090..3cf8d285 100644 --- a/src/Light.PortableResults.Validation/Light.PortableResults.Validation.csproj +++ b/src/Light.PortableResults.Validation/Light.PortableResults.Validation.csproj @@ -4,7 +4,7 @@ false Framework-agnostic validation foundations for Light.PortableResults, including validation contexts, low-allocation checks, validator base classes, and validated value pipelines. - Light.PortableResults.Validation 0.6.0 + Light.PortableResults.Validation 0.7.0 --------------------------------- - Validation foundations with validation contexts, checks, validated value pipelines, and sync/async validator base classes. diff --git a/src/Light.PortableResults/CloudEvents/MetadataValueAnnotationHelper.cs b/src/Light.PortableResults/CloudEvents/MetadataValueAnnotationHelper.cs index 54bf214d..2e69fc43 100644 --- a/src/Light.PortableResults/CloudEvents/MetadataValueAnnotationHelper.cs +++ b/src/Light.PortableResults/CloudEvents/MetadataValueAnnotationHelper.cs @@ -39,6 +39,9 @@ public static MetadataValue WithAnnotation(MetadataValue value, MetadataValueAnn case MetadataKind.String: value.TryGetString(out var stringValue); return MetadataValue.FromString(stringValue, annotation); + case MetadataKind.Decimal: + value.TryGetDecimal(out var decimalValue); + return MetadataValue.FromDecimal(decimalValue, annotation); case MetadataKind.Array: value.TryGetArray(out var arrayValue); return MetadataValue.FromArray(WithAnnotation(arrayValue, annotation), annotation); diff --git a/src/Light.PortableResults/CloudEvents/Writing/CloudEventsResultExtensions.cs b/src/Light.PortableResults/CloudEvents/Writing/CloudEventsResultExtensions.cs index 5d4725ef..b4597ae0 100644 --- a/src/Light.PortableResults/CloudEvents/Writing/CloudEventsResultExtensions.cs +++ b/src/Light.PortableResults/CloudEvents/Writing/CloudEventsResultExtensions.cs @@ -605,6 +605,14 @@ private static ResolvedAttributes ResolveAttributes( return doubleValue.ToString(CultureInfo.InvariantCulture); } + // TryGetDecimal also converts from Int64, Double, and numeric strings. The checks above are all strict + // kind checks and cannot be reached by a decimal, thus the position of this branch does not matter today + // - but the kind is checked explicitly so that it cannot swallow those values if it is ever moved up. + if (metadataValue.Kind == MetadataKind.Decimal && metadataValue.TryGetDecimal(out var decimalValue)) + { + return decimalValue.ToString(CultureInfo.InvariantCulture); + } + return null; } diff --git a/src/Light.PortableResults/Http/Reading/Json/MetadataJsonReader.cs b/src/Light.PortableResults/Http/Reading/Json/MetadataJsonReader.cs index 9af3c449..3d6ab777 100644 --- a/src/Light.PortableResults/Http/Reading/Json/MetadataJsonReader.cs +++ b/src/Light.PortableResults/Http/Reading/Json/MetadataJsonReader.cs @@ -4,12 +4,16 @@ namespace Light.PortableResults.Http.Reading.Json; /// -/// Provides low-level JSON parsing helpers for metadata values. +/// Provides low-level JSON parsing helpers for metadata values. All members delegate to +/// - see the remarks there for how +/// JSON numbers are mapped onto . /// public static class MetadataJsonReader { /// - /// Reads a from the current JSON token. + /// Reads a from the current JSON token. JSON numbers are read as + /// or , never as + /// . /// /// The JSON reader. /// The annotation applied to parsed values. diff --git a/src/Light.PortableResults/Light.PortableResults.csproj b/src/Light.PortableResults/Light.PortableResults.csproj index 7ff79372..b983be85 100644 --- a/src/Light.PortableResults/Light.PortableResults.csproj +++ b/src/Light.PortableResults/Light.PortableResults.csproj @@ -4,11 +4,24 @@ false The Light.PortableResults package implements the core functionality: Results, Errors, Metadata, Functional Extensions, and serialization support for various formats like HTTP and CloudEvents. Compatible with Native AOT. Check out the integration packages Light.PortableResults.AspNetCore.MinimalApis or Light.PortableResults.AspNetCore.Mvc. - Light.PortableResults 0.6.0 + Light.PortableResults 0.7.0 --------------------------------- - Contains core functionality: Results, Errors, Metadata, Functional Extensions, and JSON serialization support for HTTP and CloudEvents. - Compatible with .NET Native AOT. + + Breaking changes + --------------------------------- + + - Decimal metadata values now have the dedicated kind MetadataKind.Decimal instead of MetadataKind.String, + and they are serialized as JSON numbers instead of quoted strings. This aligns the response bodies with + the generated OpenAPI documents, which have always described decimals as numbers. + - MetadataValue.TryGetString no longer returns true for decimal metadata values. Use TryGetDecimal instead. + - Decimal metadata values that differ only in trailing zeros (19.50 and 19.5) now compare equal, because + equality is delegated to decimal. The scale is still preserved during serialization. + - The numeric values of MetadataKind.Array and MetadataKind.Object changed to 200 and 201 so that the + values 6 to 199 are reserved for future primitive kinds. This only affects callers that persisted or + transmitted the numeric value of MetadataKind. diff --git a/src/Light.PortableResults/Metadata/MetadataKind.cs b/src/Light.PortableResults/Metadata/MetadataKind.cs index 7860ded1..14e445fb 100644 --- a/src/Light.PortableResults/Metadata/MetadataKind.cs +++ b/src/Light.PortableResults/Metadata/MetadataKind.cs @@ -1,7 +1,17 @@ namespace Light.PortableResults.Metadata; /// +/// /// Discriminates the kind of value stored in a . +/// +/// +/// The numeric values of the members are a hard constraint: +/// decides membership of the primitive set with a single +/// comparison against . All primitive kinds must therefore be declared with values below +/// . The values 6 to 199 are reserved for future primitive kinds (CloudEvents, for example, +/// defines Binary, URI, URI-reference, and Timestamp attribute types that are currently flattened into +/// ), so that adding one of them later does not renumber the complex kinds again. +/// /// public enum MetadataKind : byte { @@ -30,16 +40,24 @@ public enum MetadataKind : byte /// String = 4, + /// + /// The metadata value represents a decimal number with 128 bits. This is considered a primitive value. + /// + Decimal = 5, + + // 6 - 199 are reserved for future primitive kinds. Do not declare a primitive kind at 200 or above, + // and do not declare a complex kind below 200 - see the remarks on this enum for details. + /// /// The metadata value represents an array, consisting of other metadata values. This is considered a complex value. /// - Array = 5, + Array = 200, /// /// The metadata value represents an object (a key-value store), consisting of other metadata values. /// This is considered a complex value. /// - Object = 6 + Object = 201 } /// diff --git a/src/Light.PortableResults/Metadata/MetadataPayload.cs b/src/Light.PortableResults/Metadata/MetadataPayload.cs index 908cbd53..b99c6676 100644 --- a/src/Light.PortableResults/Metadata/MetadataPayload.cs +++ b/src/Light.PortableResults/Metadata/MetadataPayload.cs @@ -13,7 +13,9 @@ namespace Light.PortableResults.Metadata; /// /// - is at offset 8 to avoid overlapping with the primitives. This separation /// is critical: the .NET GC tracks object references, and if Ref overlapped with I64/F64, the GC -/// could misinterpret a raw integer as a pointer, causing crashes or heap corruption. +/// could misinterpret a raw integer as a pointer, causing crashes or heap corruption. Besides strings, +/// arrays, and objects, it also holds boxed decimals - see +/// for why decimals are not stored inline. /// /// /// Total struct size: 16 bytes (8 for primitives + 8 for reference on 64-bit systems). diff --git a/src/Light.PortableResults/Metadata/MetadataValue.cs b/src/Light.PortableResults/Metadata/MetadataValue.cs index 50086596..e6fc4a91 100644 --- a/src/Light.PortableResults/Metadata/MetadataValue.cs +++ b/src/Light.PortableResults/Metadata/MetadataValue.cs @@ -5,7 +5,7 @@ namespace Light.PortableResults.Metadata; /// /// Represents a JSON-compatible metadata value. This is a discriminated union -/// that can hold null, boolean, int64, double, string, array, or object values. +/// that can hold null, boolean, int64, double, string, decimal, array, or object values. /// public readonly struct MetadataValue : IEquatable { @@ -111,7 +111,14 @@ public static MetadataValue FromString( value is null ? Null : new MetadataValue(MetadataKind.String, new MetadataPayload(value), annotation); /// + /// /// Creates a from a decimal value. + /// + /// + /// The decimal is boxed and stored in the reference slot of the payload. Storing it inline would push the + /// reference slot to offset 16 and grow every - including the ones stored inline + /// in arrays and objects - by 8 bytes. + /// /// /// The decimal value. /// The serialization annotation. @@ -119,11 +126,8 @@ public static MetadataValue FromString( public static MetadataValue FromDecimal( decimal value, MetadataValueAnnotation annotation = DefaultAnnotation - ) - { - var @string = value.ToString(CultureInfo.InvariantCulture); - return new MetadataValue(MetadataKind.String, new MetadataPayload(@string), annotation); - } + ) => + new (MetadataKind.Decimal, new MetadataPayload((object) value), annotation); /// /// Creates a from a . @@ -347,7 +351,8 @@ public bool TryGetString(out string? value) } /// - /// Attempts to get a decimal value. + /// Attempts to get a decimal value. Besides , this method also converts + /// from , , and numeric strings. /// /// When this method returns, contains the decimal value if present. /// if the value can be represented as a decimal; otherwise, . @@ -355,6 +360,9 @@ public bool TryGetDecimal(out decimal value) { switch (Kind) { + case MetadataKind.Decimal when _payload.Reference is decimal @decimal: + value = @decimal; + return true; case MetadataKind.String when _payload.Reference is string @string: return decimal.TryParse(@string, NumberStyles.Number, CultureInfo.InvariantCulture, out value); case MetadataKind.Double: @@ -450,6 +458,13 @@ public bool Equals(MetadataValue other) (string?) other._payload.Reference, StringComparison.Ordinal ), + // decimal.Equals is scale-insensitive, thus 19.50m and 19.5m are equal metadata values, + // although the scale is preserved when they are serialized. The reference is matched instead of + // being cast so that a payload which does not carry a boxed decimal is unequal like every other + // kind that cannot be interpreted, rather than throwing. + MetadataKind.Decimal => _payload.Reference is decimal @decimal && + other._payload.Reference is decimal otherDecimal && + @decimal == otherDecimal, MetadataKind.Array => ((MetadataArrayData?) _payload.Reference)?.Equals( (MetadataArrayData?) other._payload.Reference ) ?? @@ -487,7 +502,9 @@ public override int GetHashCode() case MetadataKind.Double: hashCodeBuilder.Add(_payload.Float64); break; - case MetadataKind.String or MetadataKind.Array or MetadataKind.Object: + // decimal.GetHashCode is scale-insensitive and consistent with decimal.Equals, thus boxed decimals + // can be hashed through the reference like the other reference-backed kinds. + case MetadataKind.String or MetadataKind.Decimal or MetadataKind.Array or MetadataKind.Object: hashCodeBuilder.Add(_payload.Reference?.GetHashCode() ?? 0); break; } @@ -515,7 +532,9 @@ public override int GetHashCode() /// Returns the string representation of this instance. /// /// The string representation. - /// Thrown when the value kind is unknown. + /// + /// Thrown when the value kind is unknown or when its payload cannot be interpreted for that kind. + /// public override string ToString() => Kind switch { @@ -524,9 +543,13 @@ public override string ToString() => MetadataKind.Int64 => _payload.Int64.ToString(CultureInfo.InvariantCulture), MetadataKind.Double => _payload.Float64.ToString(CultureInfo.InvariantCulture), MetadataKind.String => $"\"{_payload.Reference}\"", + // A payload that does not carry a boxed decimal falls through to the arm below instead of throwing + // a NullReferenceException or an InvalidCastException, mirroring the other reference-backed kinds. + MetadataKind.Decimal when _payload.Reference is decimal @decimal => + @decimal.ToString(CultureInfo.InvariantCulture), MetadataKind.Array => ((MetadataArrayData?) _payload.Reference)?.ToString() ?? MetadataArray.EmptyArrayStringRepresentation, MetadataKind.Object => "{...}", - _ => throw new InvalidOperationException($"Kind '{Kind}' is unknown") + _ => throw new InvalidOperationException($"Kind '{Kind}' is unknown or its payload is invalid") }; } diff --git a/src/Light.PortableResults/SharedJsonSerialization/Reading/MetadataJsonReader.cs b/src/Light.PortableResults/SharedJsonSerialization/Reading/MetadataJsonReader.cs index 977fed1e..e95e59ba 100644 --- a/src/Light.PortableResults/SharedJsonSerialization/Reading/MetadataJsonReader.cs +++ b/src/Light.PortableResults/SharedJsonSerialization/Reading/MetadataJsonReader.cs @@ -4,12 +4,26 @@ namespace Light.PortableResults.SharedJsonSerialization.Reading; /// +/// /// Provides low-level JSON parsing helpers for metadata values. +/// +/// +/// A JSON number carries no discriminator that would tell a decimal from a double, thus the reader +/// never produces : numbers that fit into a 64-bit integer become +/// , all others become . Decimals +/// consequently do not round-trip as decimals - a value written with +/// reads back as or +/// . This is deliberate: preferring decimals would make the resulting +/// kind depend on the magnitude of the value and would break round-tripping in the opposite direction, +/// where would read back as a decimal. +/// /// public static class MetadataJsonReader { /// - /// Reads a from the current JSON token. + /// Reads a from the current JSON token. JSON numbers are read as + /// or , never as + /// - see the remarks on . /// /// The JSON reader. /// The annotation applied to parsed values. diff --git a/src/Light.PortableResults/SharedJsonSerialization/Writing/MetadataExtensions.cs b/src/Light.PortableResults/SharedJsonSerialization/Writing/MetadataExtensions.cs index 65f62cf2..c0002369 100644 --- a/src/Light.PortableResults/SharedJsonSerialization/Writing/MetadataExtensions.cs +++ b/src/Light.PortableResults/SharedJsonSerialization/Writing/MetadataExtensions.cs @@ -74,6 +74,10 @@ MetadataValueAnnotation requiredAnnotation value.TryGetString(out var stringMetadataValue); writer.WriteStringValue(stringMetadataValue); break; + case MetadataKind.Decimal: + value.TryGetDecimal(out var decimalMetadataValue); + writer.WriteNumberValue(decimalMetadataValue); + break; case MetadataKind.Array: value.TryGetArray(out var arrayMetadataValue); writer.WriteMetadataArray(arrayMetadataValue, requiredAnnotation); diff --git a/tests/Light.PortableResults.Tests/CloudEvents/MetadataValueAnnotationHelperTests.cs b/tests/Light.PortableResults.Tests/CloudEvents/MetadataValueAnnotationHelperTests.cs index 98e43d6e..3591e7eb 100644 --- a/tests/Light.PortableResults.Tests/CloudEvents/MetadataValueAnnotationHelperTests.cs +++ b/tests/Light.PortableResults.Tests/CloudEvents/MetadataValueAnnotationHelperTests.cs @@ -1,8 +1,8 @@ using System; -using System.Reflection; using FluentAssertions; using Light.PortableResults.CloudEvents; using Light.PortableResults.Metadata; +using Light.PortableResults.Tests.Metadata; using Xunit; namespace Light.PortableResults.Tests.CloudEvents; @@ -73,6 +73,21 @@ public void WithAnnotation_ShouldRewriteStringValue() rewritten.Annotation.Should().Be(MetadataValueAnnotation.SerializeInCloudEventsExtensionAttributes); } + [Fact] + public void WithAnnotation_ShouldRewriteDecimalValue() + { + var rewritten = MetadataValueAnnotationHelper.WithAnnotation( + MetadataValue.FromDecimal(19.50m, MetadataValueAnnotation.SerializeInHttpResponseBody), + MetadataValueAnnotation.SerializeInCloudEventsExtensionAttributes + ); + + rewritten.Kind.Should().Be(MetadataKind.Decimal); + rewritten.TryGetDecimal(out var value).Should().BeTrue(); + value.Should().Be(19.50m); + rewritten.ToString().Should().Be("19.50"); + rewritten.Annotation.Should().Be(MetadataValueAnnotation.SerializeInCloudEventsExtensionAttributes); + } + [Fact] public void WithAnnotation_ShouldRewriteNestedArrayAndObjectValues() { @@ -138,7 +153,7 @@ public void WithAnnotation_ForEmptyMetadataObject_ShouldReturnEmpty() [Fact] public void WithAnnotation_WithUnsupportedMetadataKind_ShouldThrowArgumentOutOfRangeException() { - var invalidValue = CreateMetadataValueWithInvalidKind(); + var invalidValue = MetadataValueTestFactory.CreateWithUndeclaredKind(); var act = () => MetadataValueAnnotationHelper.WithAnnotation( invalidValue, @@ -149,25 +164,4 @@ public void WithAnnotation_WithUnsupportedMetadataKind_ShouldThrowArgumentOutOfR .Which.ParamName.Should().Be("value"); } - private static MetadataValue CreateMetadataValueWithInvalidKind() - { - var metadataValueType = typeof(MetadataValue); - var metadataPayloadType = metadataValueType.Assembly.GetType( - "Light.PortableResults.Metadata.MetadataPayload", - throwOnError: true - )!; - - var constructor = metadataValueType.GetConstructor( - BindingFlags.Instance | BindingFlags.NonPublic, - binder: null, - [typeof(MetadataKind), metadataPayloadType, typeof(MetadataValueAnnotation)], - modifiers: null - )!; - - var payload = Activator.CreateInstance(metadataPayloadType)!; - - return (MetadataValue) constructor.Invoke( - [(MetadataKind) byte.MaxValue, payload, MetadataValue.DefaultAnnotation] - ); - } } diff --git a/tests/Light.PortableResults.Tests/CloudEvents/Writing/CloudEventsResultExtensionsTests.cs b/tests/Light.PortableResults.Tests/CloudEvents/Writing/CloudEventsResultExtensionsTests.cs index f8771b2b..7209288b 100644 --- a/tests/Light.PortableResults.Tests/CloudEvents/Writing/CloudEventsResultExtensionsTests.cs +++ b/tests/Light.PortableResults.Tests/CloudEvents/Writing/CloudEventsResultExtensionsTests.cs @@ -384,6 +384,63 @@ public void ToCloudEvent_ShouldResolveAttributes_FromNonStringMetadataValues() root.GetProperty("id").GetString().Should().Be("12.5"); } + [Fact] + public void ToCloudEvent_ShouldResolveCoreStringAttributes_FromDecimalMetadataValues() + { + var metadata = MetadataObject.Create( + ( + "type", + MetadataValue.FromString( + "app.success", + MetadataValueAnnotation.SerializeInCloudEventsExtensionAttributes + ) + ), + ( + "source", + MetadataValue.FromString( + "urn:test:source", + MetadataValueAnnotation.SerializeInCloudEventsExtensionAttributes + ) + ), + ( + "subject", + MetadataValue.FromDecimal(19.50m, MetadataValueAnnotation.SerializeInCloudEventsExtensionAttributes) + ) + ); + var result = Result.Ok(metadata); + + var json = result.ToCloudEvent(options: CreateWriteOptions(source: null)); + + using var document = JsonDocument.Parse(json); + + document.RootElement.GetProperty("subject").GetString().Should().Be("19.50"); + } + + [Fact] + public void ToCloudEvent_ShouldWriteDecimalExtensionAttribute_AsUnquotedNumber() + { + var metadata = MetadataObject.Create( + ( + "price", + MetadataValue.FromDecimal(19.99m, MetadataValueAnnotation.SerializeInCloudEventsExtensionAttributes) + ) + ); + var result = Result.Ok(metadata); + + var json = result.ToCloudEvent( + successType: "app.success", + failureType: "app.failure", + id: "evt-decimal", + options: CreateWriteOptions() + ); + + using var document = JsonDocument.Parse(json); + var price = document.RootElement.GetProperty("price"); + + price.ValueKind.Should().Be(JsonValueKind.Number); + price.GetDecimal().Should().Be(19.99m); + } + [Fact] public void ToCloudEvent_ShouldResolveTimeFromMetadata_WhenProvidedAsExtensionAttribute() { diff --git a/tests/Light.PortableResults.Tests/Http/Reading/Json/MetadataJsonReaderTests.cs b/tests/Light.PortableResults.Tests/Http/Reading/Json/MetadataJsonReaderTests.cs index 4a62c298..a6b7626c 100644 --- a/tests/Light.PortableResults.Tests/Http/Reading/Json/MetadataJsonReaderTests.cs +++ b/tests/Light.PortableResults.Tests/Http/Reading/Json/MetadataJsonReaderTests.cs @@ -149,6 +149,47 @@ public void ReadMetadataValue_ShouldParseFalseBoolean() boolValue.Should().BeFalse(); } + // A JSON number carries no discriminator between a decimal and a double, thus the reader deliberately never + // produces MetadataKind.Decimal. These tests pin that non-guarantee - decimals do not round-trip as decimals. + [Theory] + [InlineData("42", MetadataKind.Int64)] + [InlineData("-9223372036854775808", MetadataKind.Int64)] + [InlineData("3.5", MetadataKind.Double)] + [InlineData("19.99", MetadataKind.Double)] + [InlineData("1e100", MetadataKind.Double)] + public void ReadMetadataValue_ShouldNeverProduceDecimalKind(string json, MetadataKind expectedKind) + { + var reader = CreateReader(json); + + var value = MetadataJsonReader.ReadMetadataValue(ref reader); + + value.Kind.Should().Be(expectedKind); + } + + [Fact] + public void ReadMetadataValue_ShouldReadWrittenDecimalAsDouble() + { + var reader = CreateReader(MetadataValue.FromDecimal(19.99m).ToString()); + + var value = MetadataJsonReader.ReadMetadataValue(ref reader); + + value.Kind.Should().Be(MetadataKind.Double); + value.TryGetDecimal(out var decimalValue).Should().BeTrue(); + decimalValue.Should().Be(19.99m); + } + + [Fact] + public void ReadMetadataValue_ShouldReadWrittenIntegralDecimalAsInt64() + { + var reader = CreateReader(MetadataValue.FromDecimal(20m).ToString()); + + var value = MetadataJsonReader.ReadMetadataValue(ref reader); + + value.Kind.Should().Be(MetadataKind.Int64); + value.TryGetDecimal(out var decimalValue).Should().BeTrue(); + decimalValue.Should().Be(20m); + } + [Fact] public void ReadMetadataValue_ShouldThrow_OnUnsupportedToken() { diff --git a/tests/Light.PortableResults.Tests/Http/Writing/Headers/DefaultHttpHeaderConversionServiceTests.cs b/tests/Light.PortableResults.Tests/Http/Writing/Headers/DefaultHttpHeaderConversionServiceTests.cs index 13043003..f8a7dd01 100644 --- a/tests/Light.PortableResults.Tests/Http/Writing/Headers/DefaultHttpHeaderConversionServiceTests.cs +++ b/tests/Light.PortableResults.Tests/Http/Writing/Headers/DefaultHttpHeaderConversionServiceTests.cs @@ -50,6 +50,20 @@ public void PrepareHttpHeader_ShouldFallbackToMetadataKeyAndStringValue_WhenNoCo header.Value.ToString().Should().Be("42"); } + [Fact] + public void PrepareHttpHeader_ShouldFormatDecimalWithoutQuotes() + { + var service = new DefaultHttpHeaderConversionService( + new Dictionary().ToFrozenDictionary() + ); + + var header = service.PrepareHttpHeader("price", MetadataValue.FromDecimal(19.50m)); + + header.Key.Should().Be("price"); + header.Value.ToString().Should().Be("19.50"); + header.Value.ToString().Should().NotContain("\""); + } + private sealed class TraceIdConverter : HttpHeaderConverter { public TraceIdConverter() : base(["traceId"]) { } diff --git a/tests/Light.PortableResults.Tests/Metadata/MetadataKindTests.cs b/tests/Light.PortableResults.Tests/Metadata/MetadataKindTests.cs new file mode 100644 index 00000000..df0955bd --- /dev/null +++ b/tests/Light.PortableResults.Tests/Metadata/MetadataKindTests.cs @@ -0,0 +1,65 @@ +using System; +using System.Collections.Generic; +using FluentAssertions; +using Light.PortableResults.Metadata; +using Xunit; + +namespace Light.PortableResults.Tests.Metadata; + +public sealed class MetadataKindTests +{ + // MetadataKindExtensions.IsPrimitive decides membership of the primitive set purely by ordering + // (kind < MetadataKind.Array). A new member declared on the wrong side of that boundary compiles cleanly + // and silently changes behavior, thus every declared member is pinned here - both its numeric value, which + // keeps the range 6 - 199 reserved for future primitive kinds and is visible to callers that persisted or + // transmitted it, and its classification. + private static readonly Dictionary ExpectedMembers = new () + { + [MetadataKind.Null] = (0, true), + [MetadataKind.Boolean] = (1, true), + [MetadataKind.Int64] = (2, true), + [MetadataKind.Double] = (3, true), + [MetadataKind.String] = (4, true), + [MetadataKind.Decimal] = (5, true), + [MetadataKind.Array] = (200, false), + [MetadataKind.Object] = (201, false) + }; + + [Fact] + public void EveryDeclaredKindShouldBePinned() + { + Enum.GetValues().Should().BeEquivalentTo(ExpectedMembers.Keys); + } + + [Fact] + public void EveryDeclaredKindShouldKeepItsNumericValue() + { + foreach (var kind in Enum.GetValues()) + { + ExpectedMembers.Should().ContainKey(kind); + ((byte) kind) + .Should() + .Be( + ExpectedMembers[kind].Value, + "the numeric value of '{0}' must not change silently", + kind + ); + } + } + + [Fact] + public void EveryDeclaredKindShouldBeClassifiedCorrectly() + { + foreach (var kind in Enum.GetValues()) + { + ExpectedMembers.Should().ContainKey(kind); + kind.IsPrimitive() + .Should() + .Be( + ExpectedMembers[kind].IsPrimitive, + "the classification of '{0}' must not change silently", + kind + ); + } + } +} diff --git a/tests/Light.PortableResults.Tests/Metadata/MetadataObjectTests.cs b/tests/Light.PortableResults.Tests/Metadata/MetadataObjectTests.cs index 9774d957..4369aada 100644 --- a/tests/Light.PortableResults.Tests/Metadata/MetadataObjectTests.cs +++ b/tests/Light.PortableResults.Tests/Metadata/MetadataObjectTests.cs @@ -104,14 +104,55 @@ public void TryGetValue_MissingKey_ShouldReturnFalse() } [Fact] - public void TryGetDecimal_ShouldParseDecimalString() + public void TryGetDecimal_ShouldReturnStoredDecimal() { var obj = MetadataObject.Create(("price", MetadataValue.FromDecimal(19.99m))); + obj.TryGetValue("price", out var value).Should().BeTrue(); + value.Kind.Should().Be(MetadataKind.Decimal); obj.TryGetDecimal("price", out var price).Should().BeTrue(); price.Should().Be(19.99m); } + [Fact] + public void TryGetString_ShouldReturnFalse_ForDecimal() + { + var obj = MetadataObject.Create(("price", MetadataValue.FromDecimal(19.99m))); + + obj.TryGetString("price", out var price).Should().BeFalse(); + price.Should().BeNull(); + } + + // Equals and GetHashCode fail silently for a kind they do not handle, thus decimals are exercised through + // object equality and a dictionary lookup here. + [Fact] + public void ObjectsWithEqualDecimals_ShouldBeEqualAndHashAlike() + { + var first = MetadataObject.Create(("price", MetadataValue.FromDecimal(19.99m))); + var second = MetadataObject.Create(("price", MetadataValue.FromDecimal(19.99m))); + + first.Equals(second).Should().BeTrue(); + first.GetHashCode().Should().Be(second.GetHashCode()); + } + + [Fact] + public void ObjectsWithDifferentDecimals_ShouldNotBeEqual() + { + var first = MetadataObject.Create(("price", MetadataValue.FromDecimal(19.99m))); + var second = MetadataObject.Create(("price", MetadataValue.FromDecimal(24.99m))); + + first.Equals(second).Should().BeFalse(); + } + + [Fact] + public void DecimalMetadataValue_ShouldBeUsableAsDictionaryKey() + { + var dictionary = new Dictionary { [MetadataValue.FromDecimal(19.99m)] = "price" }; + + dictionary.TryGetValue(MetadataValue.FromDecimal(19.99m), out var target).Should().BeTrue(); + target.Should().Be("price"); + } + [Fact] public void TryGetArray_ShouldReturnNestedArray() { diff --git a/tests/Light.PortableResults.Tests/Metadata/MetadataValueTestFactory.cs b/tests/Light.PortableResults.Tests/Metadata/MetadataValueTestFactory.cs new file mode 100644 index 00000000..05f65869 --- /dev/null +++ b/tests/Light.PortableResults.Tests/Metadata/MetadataValueTestFactory.cs @@ -0,0 +1,37 @@ +using System; +using System.Reflection; +using Light.PortableResults.Metadata; + +namespace Light.PortableResults.Tests.Metadata; + +/// +/// Creates metadata values whose kind and payload do not match what the factory methods on +/// produce - a kind that is not declared on the enum, or a declared kind whose payload +/// is empty. No dispatch site on fails to compile when a new kind is added, thus the +/// fallback arms are the only safety net left and they have to be pinned by tests. +/// +internal static class MetadataValueTestFactory +{ + public static MetadataValue CreateWithUndeclaredKind(MetadataKind kind = (MetadataKind) byte.MaxValue) => + CreateWithEmptyPayload(kind); + + public static MetadataValue CreateWithEmptyPayload(MetadataKind kind) + { + var metadataValueType = typeof(MetadataValue); + var metadataPayloadType = metadataValueType.Assembly.GetType( + "Light.PortableResults.Metadata.MetadataPayload", + throwOnError: true + )!; + + var constructor = metadataValueType.GetConstructor( + BindingFlags.Instance | BindingFlags.NonPublic, + binder: null, + [typeof(MetadataKind), metadataPayloadType, typeof(MetadataValueAnnotation)], + modifiers: null + )!; + + var payload = Activator.CreateInstance(metadataPayloadType)!; + + return (MetadataValue) constructor.Invoke([kind, payload, MetadataValue.DefaultAnnotation]); + } +} diff --git a/tests/Light.PortableResults.Tests/Metadata/MetadataValueTests.cs b/tests/Light.PortableResults.Tests/Metadata/MetadataValueTests.cs index aec64c7b..9b959443 100644 --- a/tests/Light.PortableResults.Tests/Metadata/MetadataValueTests.cs +++ b/tests/Light.PortableResults.Tests/Metadata/MetadataValueTests.cs @@ -1,4 +1,6 @@ using System; +using System.Globalization; +using System.Runtime.CompilerServices; using FluentAssertions; using Light.PortableResults.Metadata; using Xunit; @@ -103,25 +105,61 @@ public void FromString_NullShouldReturnNullValue() } [Fact] - public void FromDecimal_ShouldStoreAsString() + public void FromDecimal_ShouldStoreAsDecimal() { var input = 123.456m; var value = MetadataValue.FromDecimal(input); - value.Kind.Should().Be(MetadataKind.String); - value.TryGetString(out var str).Should().BeTrue(); - str.Should().Be("123.456"); + value.Kind.Should().Be(MetadataKind.Decimal); + value.TryGetDecimal(out var result).Should().BeTrue(); + result.Should().Be(input); } [Fact] - public void TryGetDecimal_ShouldParseDecimalString() + public void FromDecimal_ShouldPreserveScale() + { + var value = MetadataValue.FromDecimal(19.50m); + + value.TryGetDecimal(out var result).Should().BeTrue(); + result.ToString(CultureInfo.InvariantCulture).Should().Be("19.50"); + } + + [Fact] + public void TryGetString_ShouldReturnFalse_ForDecimal() { var value = MetadataValue.FromDecimal(99.99m); + value.TryGetString(out var result).Should().BeFalse(); + result.Should().BeNull(); + } + + [Fact] + public void TryGetDecimal_ShouldReturnStoredValue() + { + var value = MetadataValue.FromDecimal(99.99m); + + value.TryGetDecimal(out var result).Should().BeTrue(); + result.Should().Be(99.99m); + } + + [Fact] + public void TryGetDecimal_ShouldParseNumericString() + { + var value = MetadataValue.FromString("99.99"); + value.TryGetDecimal(out var result).Should().BeTrue(); result.Should().Be(99.99m); } + [Fact] + public void TryGetDecimal_ShouldReturnFalse_ForNonNumericString() + { + var value = MetadataValue.FromString("not a number"); + + value.TryGetDecimal(out var result).Should().BeFalse(); + result.Should().Be(0m); + } + [Fact] public void TryGetDecimal_ShouldConvertFromInt64() { @@ -195,9 +233,9 @@ public void ImplicitConversion_FromDecimal_ShouldWork() { MetadataValue value = 123.456m; - value.Kind.Should().Be(MetadataKind.String); - value.TryGetString(out var result).Should().BeTrue(); - result.Should().Be("123.456"); + value.Kind.Should().Be(MetadataKind.Decimal); + value.TryGetDecimal(out var result).Should().BeTrue(); + result.Should().Be(123.456m); } [Fact] @@ -557,4 +595,111 @@ public void Equals_Objects_ShouldCompareByValue() value1.Equals(value2).Should().BeTrue(); value1.Equals(value3).Should().BeTrue(); } + + [Fact] + public void Equals_Decimals_ShouldCompareByValue() + { + var value1 = MetadataValue.FromDecimal(19.99m); + var value2 = MetadataValue.FromDecimal(19.99m); + var value3 = MetadataValue.FromDecimal(24.99m); + + value1.Equals(value2).Should().BeTrue(); + value1.Equals(value3).Should().BeFalse(); + } + + [Fact] + public void Equals_Decimals_ShouldIgnoreScale() + { + var value1 = MetadataValue.FromDecimal(19.50m); + var value2 = MetadataValue.FromDecimal(19.5m); + + value1.Equals(value2).Should().BeTrue(); + value1.GetHashCode().Should().Be(value2.GetHashCode()); + } + + [Fact] + public void Equals_DecimalAndDouble_ShouldNotBeEqual() + { + var decimalValue = MetadataValue.FromDecimal(19.5m); + var doubleValue = MetadataValue.FromDouble(19.5); + + decimalValue.Equals(doubleValue).Should().BeFalse(); + } + + // FromDecimal is the only way to create a decimal value, thus a decimal kind without a boxed decimal in the + // payload is unreachable through the public API. Equals and ToString still handle it, because the dispatch + // sites are the last safety net for a kind that is only half-implemented. + [Fact] + public void Equals_Decimal_ShouldReturnFalse_ForEmptyPayload() + { + var wellFormed = MetadataValue.FromDecimal(19.99m); + var malformed = MetadataValueTestFactory.CreateWithEmptyPayload(MetadataKind.Decimal); + + wellFormed.Equals(malformed).Should().BeFalse(); + malformed.Equals(wellFormed).Should().BeFalse(); + malformed.Equals(malformed).Should().BeFalse(); + } + + [Fact] + public void ToString_Decimal_ShouldThrow_ForEmptyPayload() + { + var malformed = MetadataValueTestFactory.CreateWithEmptyPayload(MetadataKind.Decimal); + + var act = () => malformed.ToString(); + + act.Should().Throw(); + } + + [Fact] + public void GetHashCode_Decimal_ShouldReturnConsistentValue() + { + var value1 = MetadataValue.FromDecimal(19.99m); + var value2 = MetadataValue.FromDecimal(19.99m); + + value1.GetHashCode().Should().Be(value2.GetHashCode()); + } + + [Fact] + public void ToString_Decimal_ShouldReturnUnquotedNumber() + { + var value = MetadataValue.FromDecimal(19.50m); + + value.ToString().Should().Be("19.50"); + } + + [Fact] + public void ToString_Decimal_ShouldUseInvariantCulture() + { + var previousCulture = CultureInfo.CurrentCulture; + CultureInfo.CurrentCulture = CultureInfo.GetCultureInfo("de-DE"); + try + { + MetadataValue.FromDecimal(1234.56m).ToString().Should().Be("1234.56"); + } + finally + { + CultureInfo.CurrentCulture = previousCulture; + } + } + + [Fact] + public void Decimal_ShouldBeClassifiedAsPrimitive() + { + MetadataKind.Decimal.IsPrimitive().Should().BeTrue(); + } + + // Decimals are stored boxed in the reference slot of the payload precisely so that MetadataValue does not + // grow: array and object elements are stored inline, thus every additional byte multiplies. This test pins + // the size to the value it had before MetadataKind.Decimal was introduced. + // + // The pinned value describes a 64-bit process: the payload holds 8 bytes of primitives plus a pointer-sized + // reference slot. On a 32-bit runtime the reference slot shrinks and the expected value is a different one, + // so the bitness is asserted first - a failure there means the pin needs a second case, not a new number. + [Fact] + public void MetadataValue_ShouldNotExceedItsPinnedSize() + { + Environment.Is64BitProcess.Should().BeTrue("the pinned size describes a 64-bit process"); + + Unsafe.SizeOf().Should().Be(24); + } } diff --git a/tests/Light.PortableResults.Tests/Metadata/UndeclaredMetadataKindFallbackTests.cs b/tests/Light.PortableResults.Tests/Metadata/UndeclaredMetadataKindFallbackTests.cs new file mode 100644 index 00000000..308c8653 --- /dev/null +++ b/tests/Light.PortableResults.Tests/Metadata/UndeclaredMetadataKindFallbackTests.cs @@ -0,0 +1,59 @@ +using System; +using System.IO; +using System.Text; +using System.Text.Json; +using FluentAssertions; +using Light.PortableResults.Metadata; +using Light.PortableResults.SharedJsonSerialization.Writing; +using Xunit; + +namespace Light.PortableResults.Tests.Metadata; + +// Every site that dispatches on MetadataKind has a fallback arm, thus adding a kind and forgetting a site breaks +// nothing at compile time. These tests pin what the fallback arms do so that the blast radius of a half-finished +// kind is at least documented. +public sealed class UndeclaredMetadataKindFallbackTests +{ + [Fact] + public void Equals_ShouldReturnFalse_ForUndeclaredKind() + { + var first = MetadataValueTestFactory.CreateWithUndeclaredKind(); + var second = MetadataValueTestFactory.CreateWithUndeclaredKind(); + + first.Equals(second).Should().BeFalse(); + } + + [Fact] + public void GetHashCode_ShouldOnlyHashTheKind_ForUndeclaredKind() + { + var first = MetadataValueTestFactory.CreateWithUndeclaredKind(); + var second = MetadataValueTestFactory.CreateWithUndeclaredKind((MetadataKind) (byte.MaxValue - 1)); + + first.GetHashCode().Should().NotBe(second.GetHashCode()); + } + + [Fact] + public void ToString_ShouldThrow_ForUndeclaredKind() + { + var value = MetadataValueTestFactory.CreateWithUndeclaredKind(); + + var act = () => value.ToString(); + + act.Should().Throw().WithMessage("*is unknown*"); + } + + [Fact] + public void WriteMetadataValue_ShouldWriteNull_ForUndeclaredKind() + { + var value = MetadataValueTestFactory.CreateWithUndeclaredKind(); + + using var stream = new MemoryStream(); + using (var writer = new Utf8JsonWriter(stream)) + { + writer.WriteMetadataValue(value, MetadataValueAnnotation.SerializeInHttpResponseBody); + writer.Flush(); + } + + Encoding.UTF8.GetString(stream.ToArray()).Should().Be("null"); + } +} diff --git a/tests/Light.PortableResults.Tests/MetadataValueAnnotationTests.cs b/tests/Light.PortableResults.Tests/MetadataValueAnnotationTests.cs index 4d37f73b..ed570352 100644 --- a/tests/Light.PortableResults.Tests/MetadataValueAnnotationTests.cs +++ b/tests/Light.PortableResults.Tests/MetadataValueAnnotationTests.cs @@ -1,5 +1,6 @@ using System; using FluentAssertions; +using Light.PortableResults.CloudEvents.Writing; using Light.PortableResults.Metadata; using Xunit; @@ -40,13 +41,53 @@ public void FromString_WithAnnotation_SetsAnnotation() } [Fact] - public void FromDecimal_WithAnnotation_SetsAnnotation() + public void FromDecimal_WithoutAnnotation_UsesDefaultAnnotation() { var value = MetadataValue.FromDecimal(123.45m); value.Annotation.Should().Be(MetadataValueAnnotation.SerializeInBodies); } + [Fact] + public void FromDecimal_WithAnnotation_SetsAnnotation() + { + var value = MetadataValue.FromDecimal(123.45m, MetadataValueAnnotation.SerializeInHttpHeader); + + value.Annotation.Should().Be(MetadataValueAnnotation.SerializeInHttpHeader); + } + + // Decimals are primitive, thus they must be accepted in the two places where the primitive classification + // is enforced: header-annotated arrays and CloudEvents extension attributes. + [Fact] + public void FromArray_WithDecimals_AllowsHeaderAnnotation() + { + var array = MetadataArray.Create( + MetadataValue.FromDecimal(9.99m), + MetadataValue.FromDecimal(19.99m) + ); + + array.HasOnlyPrimitiveChildren.Should().BeTrue(); + var value = MetadataValue.FromArray(array, MetadataValueAnnotation.SerializeInHttpHeader); + + value.Annotation.Should().Be(MetadataValueAnnotation.SerializeInHttpHeader); + } + + [Fact] + public void FromDecimal_AllowsCloudEventsExtensionAttributeAnnotation() + { + var value = MetadataValue.FromDecimal( + 19.99m, + MetadataValueAnnotation.SerializeInCloudEventsExtensionAttributes + ); + + var attribute = DefaultCloudEventsAttributeConversionService.Instance.PrepareCloudEventsAttribute( + "price", + value + ); + + attribute.Value.Kind.Should().Be(MetadataKind.Decimal); + } + [Fact] public void FromArray_WithPrimitives_AllowsHeaderAnnotation() { diff --git a/tests/Light.PortableResults.Tests/SharedJsonSerialization/Writing/SharedWritingExtensionsTests.cs b/tests/Light.PortableResults.Tests/SharedJsonSerialization/Writing/SharedWritingExtensionsTests.cs index e9d813d3..2184864d 100644 --- a/tests/Light.PortableResults.Tests/SharedJsonSerialization/Writing/SharedWritingExtensionsTests.cs +++ b/tests/Light.PortableResults.Tests/SharedJsonSerialization/Writing/SharedWritingExtensionsTests.cs @@ -1,4 +1,5 @@ using System; +using System.Globalization; using System.IO; using System.Text; using System.Text.Json; @@ -118,6 +119,49 @@ public void WriteMetadataPropertyAndValue_ShouldWriteMetadataWithAnnotation() json.Should().Be("{\"metadata\":{\"traceId\":\"abc\"}}"); } + [Theory] + [InlineData("19.99", "19.99")] + [InlineData("19.50", "19.50")] + [InlineData("-0.0001", "-0.0001")] + [InlineData("79228162514264337593543950335", "79228162514264337593543950335")] + public void WriteMetadataValue_ShouldWriteDecimalAsUnquotedNumber(string input, string expectedJson) + { + var value = MetadataValue.FromDecimal(decimal.Parse(input, CultureInfo.InvariantCulture)); + + var json = Serialize( + writer => writer.WriteMetadataValue(value, MetadataValueAnnotation.SerializeInHttpResponseBody) + ); + + json.Should().Be(expectedJson); + } + + [Fact] + public void WriteMetadataObject_ShouldWriteDecimalPropertyAsUnquotedNumber() + { + var metadata = MetadataObject.Create(("comparativeValue", MetadataValue.FromDecimal(19.99m))); + + var json = Serialize( + writer => writer.WriteMetadataObject(metadata, MetadataValueAnnotation.SerializeInHttpResponseBody) + ); + + json.Should().Be("{\"comparativeValue\":19.99}"); + } + + [Fact] + public void WriteMetadataArray_ShouldWriteDecimalElementsAsUnquotedNumbers() + { + var array = MetadataArray.Create( + MetadataValue.FromDecimal(9.99m), + MetadataValue.FromDecimal(99.90m) + ); + + var json = Serialize( + writer => writer.WriteMetadataArray(array, MetadataValueAnnotation.SerializeInHttpResponseBody) + ); + + json.Should().Be("[9.99,99.90]"); + } + [Fact] public void WriteRichErrors_ShouldThrow_WhenWriterIsNull() { diff --git a/tests/Light.PortableResults.Validation.OpenApi.Tests/DecimalMetadataOpenApiConformanceTests.cs b/tests/Light.PortableResults.Validation.OpenApi.Tests/DecimalMetadataOpenApiConformanceTests.cs new file mode 100644 index 00000000..27665b99 --- /dev/null +++ b/tests/Light.PortableResults.Validation.OpenApi.Tests/DecimalMetadataOpenApiConformanceTests.cs @@ -0,0 +1,158 @@ +using System; +using System.Linq; +using System.Net.Http; +using System.Text.Json; +using System.Threading.Tasks; +using FluentAssertions; +using Light.PortableResults.AspNetCore.MinimalApis; +using Light.PortableResults.AspNetCore.OpenApi; +using Light.PortableResults.Http.Writing; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.OpenApi; +using Xunit; + +namespace Light.PortableResults.Validation.OpenApi.Tests; + +// This is the defect that motivated MetadataKind.Decimal: comparison and range rules on decimal-typed values +// used to serialize their metadata as quoted strings, while the OpenAPI document generated for the very same +// endpoint documents them as numbers. +public sealed class DecimalMetadataOpenApiConformanceTests +{ + [Fact] + public async Task DecimalValidationProblemBody_ShouldConformToGeneratedOpenApiDocument() + { + await using var app = CreateApp(); + var document = await ValidationOpenApiDocumentTestUtilities.GetOpenApiDocumentAsync(app); + using var httpClient = app.GetTestClient(); + + using var response = await httpClient.PostAsync( + "/decimal-validation", + content: null, + TestContext.Current.CancellationToken + ); + var body = await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + + body.Should().Contain("\"comparativeValue\":19.99"); + body.Should().Contain("\"lowerBoundary\":9.99"); + body.Should().Contain("\"upperBoundary\":99.99"); + + using var jsonDocument = JsonDocument.Parse(body); + var errors = jsonDocument.RootElement.GetProperty("errors").EnumerateArray().ToArray(); + + var greaterThanMetadata = GetMetadata(errors, ValidationErrorCodes.GreaterThan); + var inRangeMetadata = GetMetadata(errors, ValidationErrorCodes.InRange); + + greaterThanMetadata.GetProperty("comparativeValue").ValueKind.Should().Be(JsonValueKind.Number); + greaterThanMetadata.GetProperty("comparativeValue").GetDecimal().Should().Be(19.99m); + inRangeMetadata.GetProperty("lowerBoundary").ValueKind.Should().Be(JsonValueKind.Number); + inRangeMetadata.GetProperty("upperBoundary").ValueKind.Should().Be(JsonValueKind.Number); + + GetMetadataPropertySchema(document, ValidationErrorCodes.GreaterThan, "comparativeValue") + .Should() + .Match( + schema => ValidationOpenApiDocumentTestUtilities.SchemaIncludesType(schema, JsonSchemaType.Number) + ); + GetMetadataPropertySchema(document, ValidationErrorCodes.InRange, "lowerBoundary") + .Should() + .Match( + schema => ValidationOpenApiDocumentTestUtilities.SchemaIncludesType(schema, JsonSchemaType.Number) + ); + GetMetadataPropertySchema(document, ValidationErrorCodes.InRange, "upperBoundary") + .Should() + .Match( + schema => ValidationOpenApiDocumentTestUtilities.SchemaIncludesType(schema, JsonSchemaType.Number) + ); + } + + private static WebApplication CreateApp() + { + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + builder.Services.AddPortableResultsForMinimalApis(); + builder.Services.AddValidationForPortableResults(); + builder.Services.AddSingleton(); + builder.Services.AddPortableResultsOpenApi(contracts => contracts.RegisterBuiltInValidationErrors()); + builder.Services.Configure( + options => options.ValidationProblemSerializationFormat = ValidationProblemSerializationFormat.Rich + ); + builder.Services.AddOpenApi(); + + var app = builder.Build(); + app.MapPost("/decimal-validation", ValidateFixedPrice) + .WithName("DecimalValidation") + .ProducesPortableValidationProblemFor( + configure: static openApi => openApi.UseFormat(ValidationProblemSerializationFormat.Rich) + ); + return app; + } + + private static IResult ValidateFixedPrice(DecimalPriceValidator validator) + { + var dto = new DecimalPriceDto { Price = 5.00m, Discount = 199.00m }; + var validationContext = validator.ValidationContextFactory.CreateValidationContext(); + return validator.CheckForErrors(dto, validationContext, out var errorResult) ? + Result.Fail(errorResult.Errors).ToMinimalApiResult() : + Result.Ok(dto).ToMinimalApiResult(); + } + + private static JsonElement GetMetadata(JsonElement[] errors, string errorCode) + { + var error = errors.Single(element => element.GetProperty("code").GetString() == errorCode); + return error.GetProperty("metadata"); + } + + private static OpenApiSchema GetMetadataPropertySchema( + OpenApiDocument document, + string errorCode, + string metadataKey + ) + { + var response = (OpenApiResponse) document.Paths["/decimal-validation"] + .Operations![HttpMethod.Post] + .Responses![StatusCodes.Status400BadRequest.ToString()]; + var envelopeReference = (OpenApiSchemaReference) response.Content!["application/problem+json"].Schema!; + var envelope = ValidationOpenApiDocumentTestUtilities.GetSchemaComponent( + document, + ValidationOpenApiDocumentTestUtilities.GetSchemaReferenceId(envelopeReference) + ); + var errorItems = (OpenApiSchema) ((OpenApiSchema) envelope.Properties!["errors"]).Items!; + var errorSchemaId = errorItems.OneOf! + .Select( + static schema => + ValidationOpenApiDocumentTestUtilities.GetSchemaReferenceId((OpenApiSchemaReference) schema) + ) + .Single(schemaId => schemaId.EndsWith("__" + errorCode, StringComparison.Ordinal)); + var metadataSchema = ValidationOpenApiDocumentTestUtilities.GetSchemaComponent( + document, + errorSchemaId + "__Metadata" + ); + return (OpenApiSchema) metadataSchema.Properties![metadataKey]; + } +} + +public sealed class DecimalPriceDto +{ + public decimal Price { get; init; } + public decimal Discount { get; init; } +} + +[GeneratePortableValidationOpenApi] +public sealed partial class DecimalPriceValidator : Validator +{ + public DecimalPriceValidator(IValidationContextFactory validationContextFactory) + : base(validationContextFactory) { } + + protected override ValidatedValue PerformValidation( + ValidationContext context, + ValidationCheckpoint checkpoint, + DecimalPriceDto dto + ) + { + context.Check(dto.Price).IsGreaterThan(19.99m); + context.Check(dto.Discount).IsInRange(9.99m, 99.99m); + return checkpoint.ToValidatedValue(dto); + } +}