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

Misalignment of OpenAPI.NET and Menes handling of date parameters #113

Open
edfreeman opened this issue Aug 27, 2020 · 2 comments
Open

Misalignment of OpenAPI.NET and Menes handling of date parameters #113

edfreeman opened this issue Aug 27, 2020 · 2 comments

Comments

@edfreeman
Copy link
Contributor

OpenAPI.NET recognises a parameter of type string and format date as an OpenApiDate, whose underlying value is of type System.DateTime.
Similarly, it recognises a parameter of type string and format date-time as an OpenApiDateTime, whose underlying value is of type System.DateTimeOffset.

In Menes, parameters of type string and format date are captured by our DateConverter and cast as a System.DateTimeOffset (not a System.DateTime, as per the OpenAPI.NET behaviour).

For parameters of type string and format date-time, we don't actually have a specific converter, so the StringConverter is used as a fallback. Hence, the value is cast as a System.String. It seems strange to allow that to be cast as a string, as opposed to a DateTime (of any kind, really). Maybe we should create a task to create a DateTimeConverter, in line with the DateConverter which currently exists.

Anyway, this has been brought to our attention from the work to support the capturing of default parameter values.

What it means is that, currently, we're having to convert the types that the OpenAPI.NET library infers (from the default values) back to the types that we're returning when parameters are explicitly set in the request (and hence are ran through our converters).

We should determine whether this is the desired behaviour. Should we change our handling of dates/datetimes to align with OpenAPI.NET?

@mwadams
Copy link
Contributor

mwadams commented Aug 27, 2020

We support date but for some reason don't support time or date-time.

We should add the converters for those types.

It is notable that we haven't needed this before.

@idg10
Copy link
Contributor

idg10 commented Aug 28, 2020

Here's an attempt to describe all the types.

First, note that there are several sources of information. The OpenAPI Spec's Data Types section lists only date and date-time.

There is also the Dates, Times, and Duration section of the JSON Schema draft spec. Interestingly, the OpenAPI spec currently (3.0.3) does not defer to this. It provides a short list of standard formats, and says that the field is free text so applications can go on to use it. So choosing to interpret the format types listed in the JSON Schema draft is allowed, but not something the Open API spec talks about. However, the draft 3.1 spec (rc0) does defer to JSON Schema here.

Finally, there's RFC3339. The sections of the OpenAPI spec and JSON Schema draft spec just linked to both defer to the Internet Date/Time Format section of that RFC. So whether you're using OpenAPI or JSON Schema (2019-09 draft), in either case the underlying definition of time-based string formats comes from that RFC. And that RFC in turn defers to the 1988 edition of ISO8601.

The following table shows a row for each RCF3339 time/date type, and where applicable, the corresponding OpenAPI, JSON Schema, NodaTime, and .NET BCL types.

RFC3339 OpenAPI JSON Schema NodaTime .NET Class Library
full-date date date LocalDate (with Era.Common) See note 1
full-time time OffsetTime
date-time date-time date-time OffsetDateTime DateTimeOffset
duration duration Duration Timespan

Note 1: although there is no direct representation of a plain date built into .NET's class library, OpenApi.NET has had to pick something when it tells us that the spec defines a default value of that type. Its OpenApiDate class uses .NET's DateTime. Given the available options build into .NET this is probably the least bad choice: it's better than DateTimeOffset because a DateTimeOffset always includes an offset, and RFC3339's full-date does not specify an offset.

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

3 participants