SwaggerProvider icon indicating copy to clipboard operation
SwaggerProvider copied to clipboard

String field cannot be optional.

Open yuto-katsuragi opened this issue 6 years ago • 3 comments

Description

Primitive Types can be optional. but string field can not be optional.

Repro steps

  1. Step A provide following this in swagger yaml
      number_value:
        type: number
        nullable: true
      string_value:
        type: string
        nullable: true
  1. Step B

read property by SwaggerProvider

type Provider = SwaggerProvider<schema, PreferAsync = true>
let client = Provider.Client()
let result = client.Operation() |> Asnyc.RunSynchronously

let numberValue = result.NumberValue // ==> the type is option float
let stringValue = result.StringValue // ==> the type is string

Expected behavior

nullable string field should be Option type.

Actual behavior

nullable string field is not Option type.

Known workarounds

  • null checking

Related information

  • Operating system Windows 10
  • NuGet Version 0.10.0-beta05
  • .NET Framework 4.7.2

yuto-katsuragi avatar Apr 11 '19 21:04 yuto-katsuragi

This is by design. string in .net is nullable type. I do not see benefits of generating Option<string>

So, technically speaking, the issue is that we model

string_value:
        type: string
        nullable: false

as string that can be null in runtime. But there is no easy way to provide not nullable string.

sergey-tihon avatar Apr 11 '19 21:04 sergey-tihon

There are several facts.

  1. string in .NET is nullable type
  2. string and null are different types in JSON

It can be understood that null can not be removed. (by 1.) but I see benefits as reflect intention as option . (by 2.)

Similar situations.

SQL Server has intended to be null (NULL Column) and Popular Type Provider takes care of it.

https://github.com/fsprojects/FSharp.Data.SqlClient

this library type following. varchar NULL in DDL -> string option in .NET

I think so. Even if can not remove null, can reflect intention as option

Honestly, there is a problem of preference. But I think the string option is worth it.

yuto-katsuragi avatar Apr 11 '19 22:04 yuto-katsuragi

I came here to post about the same thing, it'd be really helpful to have this as an option like we do with IgnoreOperationId, at least in my case.

I have a RESTful API serving data from SQL where some columns are nullable text while others are not. A lot of these endpoints serve basic CRUD operations, and so I made a generic editor that uses reflection on the client generated by SwaggerProvider to save myself time from coding each one by hand. Unfortunately I don't really see a good way to determine which strings are actually nullable and which are not, so I can't easily communicate that through the generic editors (and pretty much every one has a mixture of non-nullable and nullable strings)

While .NET does have nullable strings, if the spec is communicating that they're non-nullable, I think it makes sense to try and conform to the spec as close as possible with an option type.

weebs avatar Jun 13 '19 20:06 weebs