String parameters with format 'date' get no Format in the QueryAttribute

[brease-colin]

Describe the bug
When generating a client for an OpenApi (swagger version 2.0) with url (query) parameters of type string with a format of 'date' are generated as a regular DateTimeOffset without a 'Date' format.

Support Key: mmt1dt0

OpenAPI Specifications
(not the full specs, but this is the gist of it)

{
  "swagger": "2.0",
  "info": {
    "title": "XX",
    "version": "0.0.0"
  },
  "host": "x.io",
  "basePath": "/",
  "schemes": [
    "https"
  ],
  "paths": {
    "/t/dummy/{employee_id}": {
      "get": {
        "summary": "X",
        "description": "X",
        "operationId": "dummy",
        "parameters": [
          {
            "name": "employee_id",
            "in": "path",
            "description": "the specific employee",
            "required": true,
            "format": "int64",
            "type": "integer"
          },
          {
            "name": "valid_from",
            "in": "query",
            "description": "the start of the period",
            "required": true,
            "format": "date",
            "type": "string"
          },
          {
            "name": "valid_to",
            "in": "query",
            "description": "the end of the period",
            "required": true,
            "format": "date",
            "type": "string"
          }
        ],
        "responses": {
          "200": {
            "description": "No response was specified",
            "schema": {
              "$ref": "#/definitions/DummyList"
            }
          }
        },
        "produces": [
          "application/json",
          "application/xml"
        ]
      }
    },
  }
},

Additional context
Resulting code is:

        /// <summary>
        /// X
        /// </summary>
        [Get("/t/dummy/{employee_id}")]
        Task<DummyList> Dummy(long employee_id, [Query(CollectionFormat.Multi)] System.DateTimeOffset valid_from, [Query(CollectionFormat.Multi)] System.DateTimeOffset valid_to);

When executing, this results in errors.

Expected code:

        /// <summary>
        /// X
        /// </summary>
        [Get("/t/dummy/{employee_id}")]
        Task<DummyList> Dummy(long employee_id, [Query(CollectionFormat.Multi, Format = "yyyy-MM-dd")] System.DateTimeOffset valid_from, [Query(CollectionFormat.Multi, Format = "yyyy-MM-dd")] System.DateTimeOffset valid_to);

The same may be true for other date / time related formats, like 'time'. The same may also be true for entity classes where properties are generated as regular 'DateTimeOffset'.

Ideally, for 'time' and 'date', both the arguments of functions and the properties of entity classes would result in 'TimeOnly' and 'DateOnly', respectively, but I'm not sure if those are fully supported in Refit? I think they are for properties, not sure about function arguments.

Edit: apparently 'time' was no format in OpenApi (Swagger) v2, but 'date' apparently is.

[christianhelle]

@brease-colin thanks for taking the time to report this. I was not aware that you could do this with Refit. It looks simple enough and I will try to implement it as soon as possible

[christianhelle]

@all-contributors please add @brease-colin for bugs

[allcontributors]

@christianhelle

I've put up a pull request to add @brease-colin! 🎉

[brease-colin]

Thanks for the quick response (and the library in general)!

I looked into the 'time' format some more. The strange thing is that even though it's not listed as an official data type in the Swagger 2.0 and OpenApi 3.0 specs as far as I can tell, it's still converted to a TimeSpan property type currently, at least on the entity model side.

Also, on the models side, a string format of 'date' results in a DateTimeOffset as well, but with a DateFormatConverter, so it already seems to manage that well. So it may just be the function arguments side where things can be improved.

[christianhelle]

@brease-colin I just started looking into this in detail. I'm not sure there is much to be improved here. I'm not sure that its a good idea to generated code that suggests a specific format, like yyyy-mm-dd by default but I could introduce a CLI tool argument like --datetiemformat yyyy-mm-dd so users can put whatever they want in there. But I'm not really sure what to gain with that, since the generated parameter type is already a DateTimeOffset

[brease-colin]

The information I can find about the specs on the OpenAPI web site and also referenced for example here mention that an API with type string and format date expects a date string in the format yyyy-MM-dd. This is different from the expected input for the date-time type. According to the spec, those are actually two different things.

The internet date/time standard used by OpenAPI is defined in RFC 3339, section 5.6 (effectively ISO 8601) and examples are provided in section 5.8. So for date values should look like "2018-03-20" and for date-time, "2018-03-20T09:12:28Z". As such, when using date or date-time, the pattern should be omitted.

If you need to support dates/times formatted in a way that differs to RFC 3339, you are not allowed to specify your parameter as format: date or format: date-time. Instead, you should specify type: string with an appropriate pattern and remove format.

Finally, note that a pattern of "YYYY-MM-DD" is invalid according to the specification: pattern must be a regular expression, not a placeholder or format string.

I do not have much experience with different OpenAPI server software, so I don't know how it's handled in general when a string in the wrong format (i.e. a full DateTime string for a date format) is supplied, but an API that we're building a client for does not respond well when they're supplied with the default date format that is generated by a DateTimeOffset. In our case, we actually have to change the code for our client code to work.

I could understand you may be reluctant to change the default behavior, because for some, this may be a breaking change, but it's a change that would bring it in line with the specs.

Personally I would not let the users actually pick a format. If you'd decide to support this and also make it optional, I would expect an argument like useIsoDateTimeFormats (or perhaps a separate setting for date and date-time, but one may assume that if one of them is built according to the spec, the other would be as well).

[christianhelle]

The information I can find about the specs on the OpenAPI web site and also referenced for example here mention that an API with type string and format date expects a date string in the format yyyy-MM-dd. This is different from the expected input for the date-time type. According to the spec, those are actually two different things.

The internet date/time standard used by OpenAPI is defined in RFC 3339, section 5.6 (effectively ISO 8601) and examples are provided in section 5.8. So for date values should look like "2018-03-20" and for date-time, "2018-03-20T09:12:28Z". As such, when using date or date-time, the pattern should be omitted.
If you need to support dates/times formatted in a way that differs to RFC 3339, you are not allowed to specify your parameter as format: date or format: date-time. Instead, you should specify type: string with an appropriate pattern and remove format.
Finally, note that a pattern of "YYYY-MM-DD" is invalid according to the specification: pattern must be a regular expression, not a placeholder or format string.

I do not have much experience with different OpenAPI server software, so I don't know how it's handled in general when a string in the wrong format (i.e. a full DateTime string for a date format) is supplied, but an API that we're building a client for does not respond well when they're supplied with the default date format that is generated by a DateTimeOffset. In our case, we actually have to change the code for our client code to work.

I could understand you may be reluctant to change the default behavior, because for some, this may be a breaking change, but it's a change that would bring it in line with the specs.

Personally I would not let the users actually pick a format. If you'd decide to support this and also make it optional, I would expect an argument like useIsoDateTimeFormats (or perhaps a separate setting for date and date-time, but one may assume that if one of them is built according to the spec, the other would be as well).

@brease-colin Thanks for looking into the details of this

It's true that it might be a breaking change for some, but since multiple versions of the tool are available from nuget.org, I'm not that afraid of introducing breaking changes.

I like the idea of bringing this in line with the OpenAPI specification so I will implement this starting with introducing the --useIsoDateTimeFormats CLI tool argument

[christianhelle]

Does this work for you @brease-colin

USAGE:
    refitter [URL or input file] [OPTIONS]

EXAMPLES:
    refitter ./openapi.json
    refitter https://petstore3.swagger.io/api/v3/openapi.yaml
    refitter ./openapi.json --namespace "Your.Namespace.Of.Choice.GeneratedCode" --output ./GeneratedCode.cs
    refitter ./openapi.json --namespace "Your.Namespace.Of.Choice.GeneratedCode" --internal
    refitter ./openapi.json --output ./IGeneratedCode.cs --interface-only
    refitter ./openapi.json --use-api-response
    refitter ./openapi.json --cancellation-tokens
    refitter ./openapi.json --no-operation-headers
    refitter ./openapi.json --use-iso-date-format

ARGUMENTS:
    [URL or input file]    URL or file path to OpenAPI Specification file

OPTIONS:
                                      DEFAULT                                                                           
    -h, --help                                         Prints help information                                          
    -n, --namespace                   GeneratedCode    Default namespace to use for generated types                     
    -o, --output                      Output.cs        Path to Output file                                              
        --no-auto-generated-header                     Don't add <auto-generated> header to output file                 
        --interface-only                               Don't generate contract types                                    
        --use-api-response                             Return Task<IApiResponse<T>> instead of Task<T>                  
        --internal                                     Set the accessibility of the generated types to 'internal'       
        --cancellation-tokens                          Use cancellation tokens                                          
        --no-operation-headers                         Don't generate operation headers                                 
        --no-logging                                   Don't log errors or collect telemetry                            
        --use-iso-date-format                          Explicitly format date query string parameters in ISO 8601       
                                                       standard date format using delimiters (2023-06-15)               

The following example OpenAPI spec:

{
  "swagger": "2.0",
  "info": {
    "title": "XX",
    "version": "0.0.0"
  },
  "host": "x.io",
  "basePath": "/",
  "schemes": [
    "https"
  ],
  "paths": {
    "/t/dummy/{employee_id}": {
      "get": {
        "summary": "X",
        "description": "X",
        "operationId": "dummy",
        "parameters": [
          {
            "name": "employee_id",
            "in": "path",
            "description": "the specific employee",
            "required": true,
            "format": "int64",
            "type": "integer"
          },
          {
            "name": "valid_from",
            "in": "query",
            "description": "the start of the period",
            "required": true,
            "format": "date",
            "type": "string"
          },
          {
            "name": "valid_to",
            "in": "query",
            "description": "the end of the period",
            "required": true,
            "format": "date",
            "type": "string"
          }
        ],
        "responses": {
          "200": {
            "description": "No response was specified",
            "schema": {
              "$ref": "#/definitions/DummyList"
            }
          }
        },
        "produces": [
          "application/json",
          "application/xml"
        ]
      }
    },
  }
}

generates the following code when using the --use-iso-date-format CLI tool argument

EDITED

public interface IXX
{
    /// <summary>
    /// X
    /// </summary>
    [Get("/t/dummy/{employee_id}")]
    Task Dummy(long employee_id, [Query(Format = "yyyy-MM-dd")] System.DateTimeOffset valid_from, [Query(Format = "yyyy-MM-dd")] System.DateTimeOffset valid_to);
}

[brease-colin]

I assume the test_time parameter will not appear out of nowhere (as it's not in the spec you mention), but other than that, looks great!

[christianhelle]

Copy/paste error there 😂

[christianhelle]

This is now release to nuget.org as v0.6.0