[CSharp] feat!: add useDateOnly flag

[Anakael]

resolve: #13877

Here is setup I've tested on:

server:

app.MapGet("/echodate", (DateOnly date) =>
{
    return date.ToString();
})
.WithName("GetWeatherForecast")
.WithOpenApi();

spec:

{
  "openapi": "3.0.1",
  "info": {
    "title": "TestOpenApiDateOnly",
    "version": "1.0"
  },
  "paths": {
    "/echodate": {
      "get": {
        "tags": [
          "TestOpenApiDateOnly"
        ],
        "operationId": "GetWeatherForecast",
        "parameters": [
          {
            "name": "date",
            "in": "query",
            "required": true,
            "style": "form",
            "schema": {
              "type": "string",
              "format": "date"
            }
          }
        ],
        "responses": {
          "200": {
            "description": "OK",
            "content": {
              "text/plain": {
                "schema": {
                  "type": "string"
                }
              }
            }
          }
        }
      }
    }
  },
  "components": { }
}

generate with: java -jar ../openapi-generator/modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -i Server/spec.json -g csharp -o GeneratedClient --additional-properties=library=httpclient,useDateOnly=true

generated client:

namespace Org.OpenAPITools.Api
{

    /// <summary>
    /// Represents a collection of functions to interact with the API endpoints
    /// </summary>
    public interface ITestOpenApiDateOnlyApiSync : IApiAccessor
    {
        #region Synchronous Operations
        /// <summary>
        /// 
        /// </summary>
        /// <exception cref="Org.OpenAPITools.Client.ApiException">Thrown when fails to make API call</exception>
        /// <param name="date"></param>
        /// <returns>string</returns>
        string GetWeatherForecast(DateOnly date);

        /// <summary>
        /// 
        /// </summary>
        /// <remarks>
        /// 
        /// </remarks>
        /// <exception cref="Org.OpenAPITools.Client.ApiException">Thrown when fails to make API call</exception>
        /// <param name="date"></param>
        /// <returns>ApiResponse of string</returns>
        ApiResponse<string> GetWeatherForecastWithHttpInfo(DateOnly date);
        #endregion Synchronous Operations
    }
    // ..rest
}

Note: useDateOnly flag adds usage of DateOnly type, which is available only from .NET6, but I haven't add any check for using it with older frameworks. Is it worth to add or no?

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh ./bin/configs/*.yaml
  ./bin/utils/export_docs_generators.sh
  

  (For Windows users, please run the script in Git BASH)
  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
• File the PR against the correct branch: master (upcoming 7.1.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

[Anakael]

@mandrean @shibayan @Blackclaws @lucamazzanti @iBicha

[devhl-labs]

Is there a way to tell in the spec so we could avoid another option?

[Anakael]

Is there a way to tell in the spec so we could avoid another option?

I don't think so. We could use DateOnly as default type for date string format, but it would be breaking change for generated code's consumers then and it's probably not worth it.

[devhl-labs]

According to the spec there are formats for date and date-time. Adding support would be a breaking change, but it is a correct breaking change. We should find a way to do it while providing a fallback to the legacy incorrect way.

https://swagger.io/docs/specification/data-models/data-types/

[Anakael]

-> legacy incorrect way
It neather legacy or incorrect way.
In C# we have 4 core types for dates and time: DateTime, DateTimeOffset (for which we already have option in generator), DateOnly, TimeOnly and all of them are actual correct way. But the last two were introduced only in .NET6 and available only in core framework and not available for .netstandard (which is actual).
The property I've added use difference between date and date-time and map them to DateOnly and DateTime C# types. Without that flag both of spec types were maped to just DateTime.

[devhl-labs]

My point is only that date and date-time formats should get translated to different types in languages which support both without the need for opting in. The option should instead enable users to avoid breaking changes.

[Anakael]

I see. So you suggest to make my flag as default behavior for .NET6+ and add new one: useDateTimeForDate to preserve old behaviour?

[devhl-labs]

Yes that makes sense to me. Gives a fallback to avoid breaking changes while ensuring we can more closely follow the spec going forward.

[Anakael]

Ok, I'll do it.

[Anakael]

There are a lot of regenerated samples here ._.

[devhl-labs]

Please remove the spacing changes in this file.

[Anakael]

Done.

[devhl-labs]

Please remove the spacing changes in this file.

[Anakael]

Done.

[devhl-labs]

startsWith("net6.0") fails to capture the or later part of NET_60_OR_LATER. Also, I think the target framework could contain multiple frameworks.

This is something that really should be in the abstract and not needed here, but sadly it is not :(

Thanks for your work on this pr. It is looking good.

[Anakael]

-> startsWith("net6.0") fails to capture the or later part of NET_60_OR_LATER
Do you mean versions like net7.0, net.8.0? aspnetcore generator supports only 6.0 as for now.
-> Also, I think the target framework could contain multiple frameworks.
targetFramework from csharp generator could, indeed. But aspnetcore generator use aspnetCoreVersion parameter, which couldn't.
-> This is something that really should be in the abstract and not needed here, but sadly it is not :(
This is because of different parameters and logic around it. Also, csharp generator supports .netstandard and .NET framework, but aspnetcore doesn't. So merge it in abstract class will break LS solid principe.

[devhl-labs]

I would still suggest handling past .net6. The end of life on it is this year. Surprising to me that no one has updated it already. Other than that, LGTM.

[Anakael]

Done.
-> Surprising to me that no one has updated it already
I think it's because aspnetcore generator is not popular since it's not what you can generate on every spec update.

[devhl-labs]

Please revert the spacing change.

[Anakael]

Done.

[devhl-labs]

Please revert the spacing change.

[Anakael]

Done.

[Anakael]

@devhl-labs @wing328
Can we merge it now? :)

[wing328]

in https://github.com/OpenAPITools/openapi-generator/tree/master/.github/workflows, there are workflows for dotnet.

can you please add the new sample folders to these workflow so that CI will tests to make sure these samples compile without issues moving forward?

[Anakael]

Done.
I've also noticed that netcore-latest samples not captured in on section's regexes. Is it intended or mistake?

[wing328]

likely a mistake. can you please file another PR to fix it?

[Anakael]

Sure.

[wing328]

thanks again for the PR. just merged it