[Repo Assist] eng: add TimeOnly support for format:time schemas (+11 tests, 295→306)#394
Conversation
OpenAPI `format: time` fields were previously silently treated as strings. On .NET 6+ (when useDateOnly=true), they now map to System.TimeOnly — the natural .NET counterpart, parallel to the existing DateOnly handling for format:date. Changes: - DefinitionCompiler: add 'time' format case → TimeOnly when useDateOnly=true, falling back to string on older runtimes (same guard as DateOnly) - RuntimeHelpers: add tryFormatViaMethods helper shared by DateOnly and TimeOnly; add tryFormatTimeOnly (format HH:mm:ss.FFFFFFF); extend toQueryParams array detection to also match TimeOnly and Option<TimeOnly> arrays - Add 9 RuntimeHelpers unit tests covering toParam/toQueryParams for TimeOnly - Add 2 Schema.TypeMappingTests covering the new format:time mapping Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds System.TimeOnly support across schema type-mapping and runtime parameter formatting so OpenAPI format: time values are no longer silently treated as string when useDateOnly=true (parallel to existing DateOnly behavior for format: date).
Changes:
- Add
format: timemapping inDefinitionCompilertoSystem.TimeOnlywhenuseDateOnly=true(withstringfallback). - Extend
RuntimeHelpers.toParam/toQueryParamsto formatTimeOnlyandTimeOnly-typed arrays/options. - Add runtime tests for
TimeOnlyformatting and query-param expansion; add a schema type-mapping fallback test.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/SwaggerProvider.DesignTime/DefinitionCompiler.fs |
Adds schema mapping for string + format: time to TimeOnly behind useDateOnly. |
src/SwaggerProvider.Runtime/RuntimeHelpers.fs |
Adds TimeOnly formatting support and expands array detection to include TimeOnly/Option<TimeOnly>. |
tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs |
Adds TimeOnly coverage for toParam and toQueryParams (scalar, option, array, option-array). |
tests/SwaggerProvider.Tests/Schema.TypeMappingTests.fs |
Adds a format: time test for the useDateOnly=false fallback path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot address inline comments and ensure that tests properly cover new feature |
Agent-Logs-Url: https://github.com/fsprojects/SwaggerProvider/sessions/6dbf9572-532a-4c7c-9f10-38f0a68f83dd Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>
Agent-Logs-Url: https://github.com/fsprojects/SwaggerProvider/sessions/6dbf9572-532a-4c7c-9f10-38f0a68f83dd Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>
Addressed in 18f7695 (plus supporting test commit 69a810b). I added explicit schema coverage for |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
🤖 This PR was created by Repo Assist, an automated AI assistant.
Summary
OpenAPI
format: timefields were previously silently treated asstring. On .NET 6+ (whenuseDateOnly=true), they now map toSystem.TimeOnly— the natural .NET counterpart, exactly parallel to the existingDateOnlyhandling forformat: date.Root cause
DefinitionCompiler.compileBySchemahandledformat: dateandformat: date-timebut had no explicit case forformat: time, so it fell through to the catch-allstringmapping.RuntimeHelpers.toParamandtoQueryParamshad noTimeOnlyhandling either.Changes
DefinitionCompiler.fs: add"time"format case →System.TimeOnlywhenuseDateOnly=true, withstringfallback on older runtimes (same runtime-version guard asDateOnly)RuntimeHelpers.fs:tryFormatViaMethodshelper shared byDateOnlyandTimeOnly(removes duplication)tryFormatTimeOnly(formats with"HH:mm:ss.FFFFFFF", trailing zeros stripped)toQueryParamsarray detection to also matchTimeOnlyandOption<TimeOnly>arraysRuntimeHelpersTestscoveringtoParam/toQueryParamsforTimeOnly; 2Schema.TypeMappingTestsfor the newformat: timemappingTest Status
✅
dotnet build— succeeded (0 errors)✅
dotnet tests/SwaggerProvider.Tests/...— 306 passed, 0 failed (1 skipped pre-existing)✅
dotnet fake build -t CheckFormat— no formatting issuesTrade-offs
useDateOnlyflag already implies .NET 6+, so it correctly gatesTimeOnly(same asDateOnly)"HH:mm:ss.FFFFFFF"uses Fantomas'sFFFFFFFspecifier (trailing zeros omitted), matching the ISO 8601 partial-time format expected by most OpenAPI serversnetstandard2.0target continues to receivestringas a safe fallback