Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

string with format date is generated as DateTimeOffset but should be DateOnly #240

Open
1 of 2 tasks
kMutagene opened this issue Feb 22, 2024 · 7 comments
Open
1 of 2 tasks

Comments

@kMutagene
Copy link

kMutagene commented Feb 22, 2024

Description

Hi there, thanks for this awesome library.

I have a ASP.NET core API that uses the DateOnly type for a field, which looks like this in the generated OpenAPI doc:

"ReleaseDate": {
    "type": "string",
    "format": "date"
}

From the docs of the OpenAPI spec, "format": "date" indicates:

full-date notation as defined by RFC 3339, section 5.6, for example, 2017-07-21

But the OpenAPI provider sets the field type as DateTimeOffset in the generated spec. This only works when parsing responses, but fails when posting data with this field to the API, because it cannot parse the provided string to DateOnly:

RequestBody: {"ReleaseDate":"2024-02-22T14:30:01.8849759+01:00"}
Microsoft.AspNetCore.Http.BadHttpRequestException: Failed to read parameter "d" from the request body as JSON.
---> System.Text.Json.JsonException: The JSON value could not be converted to System.DateOnly. Path: $.ReleaseDate | LineNumber: 0 | BytePositionInLine: 154.

Is this happening because you also want to target netstandard2.0? IIRC DateOnly is available starting with net6.0.

Affected Type Providers

  • SwaggerClientProvider
  • OpenApiClientProvider (only used this one as my schema is OpenAPI v3.0)

Related information

  • Operating system: Win 11
  • .NET Runtime, CoreCLR or Mono Version: .NET 8
@Thorium
Copy link
Member

Thorium commented Feb 22, 2024

You have option to customize the serialization
https://fsprojects.github.io/SwaggerProvider/#/Customization#serialization

@kMutagene
Copy link
Author

kMutagene commented Feb 23, 2024

@Thorium thanks - that feels more like a workaround though, since the client generated by the provider creates API calls that are wrong according to the schema.

I have taken a look at the C# client that gets generated by NSwag for the same schema, and while they also use DateTimeOffset (which makes sense from a backwards compatibility perspective i guess), the client contains a customized serialization setting from the get-go:

    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "14.0.3.0 (NJsonSchema v11.0.0.0 (Newtonsoft.Json v13.0.0.0))")]
    internal class DateFormatConverter : Newtonsoft.Json.Converters.IsoDateTimeConverter
    {
        public DateFormatConverter()
        {
            DateTimeFormat = "yyyy-MM-dd";
        }
    }

which then decorates the fields that are marked via format: date. If you could point in the right direction in the source code, i would be happy to try to file a small PR that adds something similar automatically for these cases.

@sergey-tihon
Copy link
Member

Here is the place where we define a type for date-time string for OpenApiClientProvider
https://github.com/fsprojects/SwaggerProvider/blob/master/src/SwaggerProvider.DesignTime/v3/DefinitionCompiler.fs#L412

Here is the code that add a custom attribute to the generated property
https://github.com/fsprojects/SwaggerProvider/blob/master/src/SwaggerProvider.DesignTime/v3/DefinitionCompiler.fs#L205-L208

@Thorium
Copy link
Member

Thorium commented Feb 23, 2024

The offset (with time-zone) is a bit weird guy with "date" as there is no time-zone conversions and SwaggerProvider is a server-side tool where you'd expect everything to be UTC. If you want, you could do line 411 conditional compilation something like

| "date" ->
#if NETSTANDARD2.1
    typeof<DateOnly>
#else
    typeof<DateTime>
#endif

...but that could make the maintenance more complex than the real gain. Because the end-user may compile the same SwaggerProvider using source-code to multiple targets too, and then they would have to also do conditional compilations.

@sergey-tihon
Copy link
Member

Type inference code (DefinitionCompiler.fs) runs in design-time, so netstandard2.1 assembly may be loaded by compiler/ide instead of loading netstandard2.0 and then TP will always emit DateOnly no matter what the target runtime for of the project that uses SwaggerProvider.

I am not sure if it is possible to conditionally check the target compilation runtime for the TP design-time component.

@Thorium
Copy link
Member

Thorium commented Feb 23, 2024

oh right it goes straight to the problem where VS Code runs on netstandard2.1 and VS full runs on netstandard 2.0

@MichaelMay81
Copy link
Contributor

What if there were a switch for the Provider where the user can decide if Date is a DateTimeOffset or a DateOnly?
Default would be DateTimeOffset to not break legacy code and in the documentation it must be stated, that this only works for .Net 6 and above...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants