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

Optional property marked as required #3851

Open
taconaut opened this issue Jan 27, 2022 · 10 comments
Open

Optional property marked as required #3851

taconaut opened this issue Jan 27, 2022 · 10 comments

Comments

@taconaut
Copy link

taconaut commented Jan 27, 2022

I'll repost the entire question posted on Stackoverflow as it looks like a NSwag issue.
Did I misconfigure something I've missed or is this an NSwag bug?


I'm using NSagStudio to generate a c# client for an OpenAPI 3.0.1 service. Since the latest update I get an issue where a property marked as required is missing from the retrieved data and thus cannot be deserialized.

I'm trying to understand if

  • The OpenAPI spec of the service is incomplete/wrong
  • I've misconfigured something with NSwagStudio
  • There is a bug in NSwagStudio

OpenAPI 3.0.1 service spec: https://raw.githubusercontent.com/admin-ch/CovidCertificate-Apidoc/main/open-api/api-doc.yaml

NSwagStudio configuration:

{
  "runtime": "NetCore31",
  "defaultVariables": null,
  "documentGenerator": {
    "fromDocument": {
      "json": "[see URL]",
      "url": "https://raw.githubusercontent.com/admin-ch/CovidCertificate-Apidoc/main/open-api/api-doc.yaml",
      "output": null,
      "newLineBehavior": "Auto"
    }
  },
  "codeGenerators": {
    "openApiToCSharpClient": {
      "clientBaseClass": "BagCovidCertificateClientInternalBase",
      "configurationClass": null,
      "generateClientClasses": true,
      "generateClientInterfaces": false,
      "clientBaseInterface": null,
      "injectHttpClient": false,
      "disposeHttpClient": true,
      "protectedMethods": [],
      "generateExceptionClasses": true,
      "exceptionClass": "BagCovidCertificateApiException",
      "wrapDtoExceptions": true,
      "useHttpClientCreationMethod": false,
      "httpClientType": "System.Net.Http.HttpClient",
      "useHttpRequestMessageCreationMethod": true,
      "useBaseUrl": true,
      "generateBaseUrlProperty": true,
      "generateSyncMethods": false,
      "generatePrepareRequestAndProcessResponseAsAsyncMethods": false,
      "exposeJsonSerializerSettings": false,
      "clientClassAccessModifier": "internal",
      "typeAccessModifier": "internal",
      "generateContractsOutput": false,
      "contractsNamespace": null,
      "contractsOutputFilePath": null,
      "parameterDateTimeFormat": "s",
      "parameterDateFormat": "yyyy-MM-dd",
      "generateUpdateJsonSerializerSettingsMethod": true,
      "useRequestAndResponseSerializationSettings": false,
      "serializeTypeInformation": false,
      "queryNullValue": "",
      "className": "BagCovidCertificateClientInternal",
      "operationGenerationMode": "SingleClientFromOperationId",
      "additionalNamespaceUsages": [],
      "additionalContractNamespaceUsages": [],
      "generateOptionalParameters": false,
      "generateJsonMethods": false,
      "enforceFlagEnums": false,
      "parameterArrayType": "System.Collections.Generic.IEnumerable",
      "parameterDictionaryType": "System.Collections.Generic.IDictionary",
      "responseArrayType": "System.Collections.Generic.ICollection",
      "responseDictionaryType": "System.Collections.Generic.IDictionary",
      "wrapResponses": false,
      "wrapResponseMethods": [],
      "generateResponseClasses": true,
      "responseClass": "SwaggerResponse",
      "namespace": "Documedis.Clients.External.Clients.Internal",
      "requiredPropertiesMustBeDefined": true,
      "dateType": "System.DateTimeOffset",
      "jsonConverters": null,
      "anyType": "object",
      "dateTimeType": "System.DateTimeOffset",
      "timeType": "System.TimeSpan",
      "timeSpanType": "System.TimeSpan",
      "arrayType": "System.Collections.Generic.ICollection",
      "arrayInstanceType": "System.Collections.ObjectModel.Collection",
      "dictionaryType": "System.Collections.Generic.IDictionary",
      "dictionaryInstanceType": "System.Collections.Generic.Dictionary",
      "arrayBaseType": "System.Collections.ObjectModel.Collection",
      "dictionaryBaseType": "System.Collections.Generic.Dictionary",
      "classStyle": "Poco",
      "jsonLibrary": "NewtonsoftJson",
      "generateDefaultValues": true,
      "generateDataAnnotations": true,
      "excludedTypeNames": [],
      "excludedParameterNames": [],
      "handleReferences": false,
      "generateImmutableArrayProperties": false,
      "generateImmutableDictionaryProperties": false,
      "jsonSerializerSettingsTransformationMethod": null,
      "inlineNamedArrays": false,
      "inlineNamedDictionaries": false,
      "inlineNamedTuples": true,
      "inlineNamedAny": false,
      "generateDtoTypes": true,
      "generateOptionalPropertiesAsNullable": false,
      "generateNullableReferenceTypes": false,
      "templateDirectory": null,
      "typeNameGeneratorType": null,
      "propertyNameGeneratorType": null,
      "enumNameGeneratorType": null,
      "serviceHost": null,
      "serviceSchemes": null,
      "output": "BagCovidCertificateClientInternal.Generated.cs",
      "newLineBehavior": "Auto"
    }
  }
}

My issue occurs with this element:

    IssuableRapidTestDto:
      type: object
      properties:
        code:
          type: string
          description: Code of rapid test as string.
          example: "1232"
        display:
          type: string
          description: Manufacturer and display name of rapid test as string.
          example: "Abbott Rapid Diagnostics, Panbio Covid-19 Ag Rapid Test"
        validUntil:
          type: string
          description: Deadline after which the rapid-test can no longer be used to
            establish a certificate.
          format: date-time
          example: 2022-01-06T00:00:00+01:00

Which results in following class being generated:

    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "13.15.5.0 (NJsonSchema v10.6.6.0 (Newtonsoft.Json v12.0.0.0))")]
    internal partial class IssuableRapidTestDto
    {
        [...]

        /// <summary>
        /// Deadline after which the rapid-test can no longer be used to establish a certificate.
        /// </summary>
        [Newtonsoft.Json.JsonProperty("validUntil", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public System.DateTimeOffset ValidUntil { get; set; }

        [...]
    }

My issue occurs because not all IssuableRapidTestDto have a ValidUntil date set, when retrieving the data.

If I remove Required = Newtonsoft.Json.Required.DisallowNull the deserialization works and ValidUntil holds DateTimeOffet.MinValue. As this is generated code I don't want to adapt it manually.

I'm having a hard time figuring out if ValidUntil is actually required or not.

@taconaut
Copy link
Author

taconaut commented Jan 27, 2022

By setting "generateOptionalPropertiesAsNullable": true (instead of false before) the property becomes nullable (that's good). But I've noticed ALL properties which are not Newtonsoft.Json.Required.Always are marked with Newtonsoft.Json.Required.DisallowNull whereas I'd expect them to be Newtonsoft.Json.Required.Default (as, from my understanding, they are optional).
I'm still not sure if I misunderstood something or if this is an NSwag bug.

@insylogo
Copy link

This is a serious issue, has anyone looked at it?

@taconaut
Copy link
Author

taconaut commented May 6, 2022

I've encountered this issue again when integrating a new API. The fix I've applied is pretty simple: Replace Required = Newtonsoft.Json.Required.Always with Required = Newtonsoft.Json.Required.Default but it is a pain to think of having to do this after every re-regenration of the client.

@Aeroverra
Copy link

Same issue but with an api significant in size. Changing all of these by hand is not practical. At this point I may have to separate the work arounds into its own project. This whole thing desperately needs more dedicated maintainers.

@seansanchez
Copy link

Might be able to utilize the ClientBaseClass property of CSharpClientGeneratorSettings to handle this issue instead of manually editing the generated classes.

public class GeneratedClientBase
{
    public void UpdateJsonSerializerSettings(JsonSerializerSettings settings)
    {
        settings.ContractResolver = new SafeContractResolver();
    }
}

public class SafeContractResolver : DefaultContractResolver
{
    protected override JsonProperty CreateProperty(MemberInfo member, MemberSerialization memberSerialization)
    {
        var jsonProperty = base.CreateProperty(member, memberSerialization);
        jsonProperty.Required = Required.Default;

        return jsonProperty;
    }
}

@Wangor
Copy link

Wangor commented Dec 8, 2022

Is there any update? Unfortunately it makes NSwag not useable anymore... or is there a propper workaround? -> (To manipulate the specs before is not an option)

@taconaut
Copy link
Author

@Wangor No update I'm aware of unfortunately

@paulallington
Copy link

I'm finding this same issue. I guess there hasn't been a workaround?

@bennycoomans
Copy link

The workaround of @seansanchez did not work for me, but I managed to get it working using this code:

public partial class GeneratedClient // Replace by your client name
{
    partial void UpdateJsonSerializerSettings(JsonSerializerSettings settings)
    {
        settings.ContractResolver = new SafeContractResolver();
    }
}

public class SafeContractResolver : DefaultContractResolver
{
    protected override JsonProperty CreateProperty(MemberInfo member, MemberSerialization memberSerialization)
    {
        var jsonProperty = base.CreateProperty(member, memberSerialization);

        jsonProperty.Required = Required.Default;

        return jsonProperty;
    }
}

However, I strongly feel that RicoSuter/NJsonSchema#1563 would be the definitive fix for this issue, removing the need for this partial class.

@mjmadhu
Copy link

mjmadhu commented Apr 23, 2024

+1, hitting this issue for production clients and handling this issue in generated clients everytime is painful. Any update or ETA for fix?

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

8 participants