[http-client-csharp] Fix duration integer encoding for non-Int32 wire types#10832
Conversation
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
jorgerangel-msft
left a comment
There was a problem hiding this comment.
@copilot lets add unit tests to validate the json deserialization + serialization of a model property specified with this encoding. Also rerun the generate.ps1 script and ensure all tests are passing.
…on encoding Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Added in 4a84e15:
Ran |
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
commit: |
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
|
regen preview: Azure/azure-sdk-for-net#59519 |
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes integer-encoded TypeSpec duration serialization in the C# emitter so wire types other than int32 (e.g. integer, int64, safeint, uint*) are emitted as integers rather than fractional doubles, and large values use int64 to avoid overflow. Adds explicit Math.Round wrapping so fractional milliseconds/seconds round predictably.
Changes:
- Extend
TypeFactory.GetSerializationFormatduration switches to cover all integer wire kinds, routing larger kinds to newDuration_Seconds_Int64/Duration_Milliseconds_Int64formats. - Add the new enum members to both the input
SerializationFormatenum and the generatedSerializationFormatDefinition, and handle them in MRW JSON ser/deser andTypeFormattersDefinition'sConvertToString; wrap existing Int32 paths inMath.Round. - Add
ConvertSnippets.InvokeToInt64, newMathSnippets.InvokeRound, and broad TypeFactory / MRW / generated-model / deserialization tests; regenerate Sample-TypeSpecSerializationFormat.csandTypeFormatters.cs.
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Microsoft.TypeSpec.Generator/src/TypeFactory.cs | Map full integer/long integer wire-kind sets to Int32 vs new Int64 duration formats. |
| Microsoft.TypeSpec.Generator/src/Snippets/MathSnippets.cs | New helper emitting Math.Round(arg). |
| Microsoft.TypeSpec.Generator/src/Snippets/ConvertSnippets.cs | Adds InvokeToInt64 helper. |
| Microsoft.TypeSpec.Generator.Input/src/InputTypes/Serialization/SerializationFormat.cs | Adds Duration_Seconds_Int64 and Duration_Milliseconds_Int64 enum values. |
| Microsoft.TypeSpec.Generator.ClientModel/src/Providers/SerializationFormatDefinition.cs | Emits the new Int64 enum members in the generated client enum. |
| Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs | JSON ser/deser branches for new Int64 formats and Math.Round wrapping for Int32 formats. |
| Microsoft.TypeSpec.Generator.ClientModel/src/Providers/TypeFormattersDefinition.cs | Adds Int64 TimeSpan switch cases and wraps Int32 cases in Math.Round. |
| Microsoft.TypeSpec.Generator/test/TypeFactoryTests.cs | Parametrized tests for integer/float wire-kind routing. |
| Microsoft.TypeSpec.Generator.ClientModel/test/.../MrwSerializationTypeDefinitionTests.cs | Adds Int64 ser/deser expression test cases and updated Math.Round expectations. |
| Microsoft.TypeSpec.Generator.ClientModel/test/.../JsonModelCoreTests.cs | New tests asserting integer vs float duration model output by wire kind. |
| Microsoft.TypeSpec.Generator.ClientModel/test/.../DeserializationTests.cs | New deserialization test for milliseconds integer wire kinds. |
| .../TestData/JsonModelCoreTests/DurationMillisecondsWireType.cs | Expected per-wire-kind generated model write output. |
| .../TestData/DeserializationTests/TestDeserializationOfDurationMillisecondsIntegerWireType(*).cs | Expected per-wire-kind generated deserialization output. |
| TestProjects/Local/Sample-TypeSpec/.../SerializationFormat.cs | Regenerated enum with the new Int64 members. |
| TestProjects/Local/Sample-TypeSpec/.../TypeFormatters.cs | Regenerated formatter cases for the new Int64 formats and Math.Round wrapping. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
No changes needing a change description found. |
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
A TypeSpec
durationencoded as integer milliseconds/seconds with a wire type other thanint32(e.g.integer,int64,safeint,uint*) was serialized as adoubleviaTimeSpan.TotalMilliseconds/TotalSeconds, producing fractional JSON output that violates the integer contract. Additionally, for wire types larger thanint32(e.g.int64,uint32,uint64,safeint, unboundedinteger), usingInt32-based ser/deser would overflow/truncate large values.Previously generated:
Now generates (for
int32-sized wire types):And for wire types larger than
int32(int64,uint32,uint64,safeint, unboundedinteger):The
Math.Roundwrapper makes the rounding behavior explicit so fractional values (e.g.1500.7ms) are rounded to the nearest integer (1501) rather than relying onConvert.ToInt32/ToInt64's implicit rounding.Changes
TypeFactory.GetSerializationFormat: TheDurationKnownEncoding.Seconds/Millisecondsswitches only matchedInputPrimitiveTypeKind.Int32for the integer arm; every other integer kind fell through the_default intoDuration_*_Double. Extended both arms to cover the full integer-kind set, splitting based on .NET range:Int8,Int16,Int32,UInt8,UInt16→Duration_Seconds/Duration_Milliseconds(usesInt32).Int64,UInt32,UInt64,SafeInt, unboundedInteger→ newDuration_Seconds_Int64/Duration_Milliseconds_Int64(usesInt64).Float/Float32still map to_Float, andFloat64/ others to_Double.SerializationFormatvalues: AddedDuration_Seconds_Int64andDuration_Milliseconds_Int64to the input enum and to the generatedSerializationFormatenum (SerializationFormatDefinition).MrwSerializationTypeDefinition: Handles the new Int64 formats by emittingJsonElement.GetInt64()for deserialization andConvert.ToInt64(Math.Round(...))for JSON serialization. The Int32 formats now also wrap inMath.Roundfor explicit rounding.TypeFormattersDefinition: Handles the new Int64 formats in the URI/query-stringConvertToStringpath withConvert.ToInt64(Math.Round(...)).ToString(...). The Int32 formats now also wrap inMath.Round.ConvertSnippets: AddedInvokeToInt64helper.MathSnippets(new): AddedInvokeRoundhelper that emitsMath.Round(arg).TypeFactoryTests.DurationIntegerWireTypeSerializationFormatandDurationFloatWireTypeSerializationFormatcover every integer and float wire-type kind for both encodings, asserting Int32 vs Int64 routing.MrwSerializationTypeDefinitionTestsTestTimeSpanDeserializeExpressionandTestTimeSpanSerializeStatementextended with the new Int64 format cases and theMath.Roundwrapping.JsonModelCoreTests.DurationMillisecondsIntegerWireTypeWritesAsInt/DurationMillisecondsFloatWireTypeWritesAsDoubleandDeserializationTests.TestDeserializationOfDurationMillisecondsIntegerWireTypebuild a model with adurationproperty and compare the full generated output against per-caseTestDatafiles (Int32 emitsGetInt32/Convert.ToInt32(Math.Round(...)); Int64 and unboundedintegeremitGetInt64/Convert.ToInt64(Math.Round(...))).Validation
eng/scripts/Generate.ps1ran to completion; the only regenerated changes are the expected updates toTestProjects/Local/Sample-TypeSpec/src/Generated/Internal/SerializationFormat.csandTypeFormatters.csreflecting the new Int64 enum members, switch arms, andMath.Roundwrapping.