Add support in models for dictionary types

[darrelmiller]

No description provided.

[baywet]

@darrelmiller can you add a YAML sample for that on the repo please?

[darrelmiller]

This is blocked on OpenAPI 3.1 support as the patternedProperties keyword is not supported in OpenAPI 3.0

[baywet]

https://github.com/microsoft/api-guidelines/blob/vNext/graph/patterns/dictionary.md
https://github.com/microsoft/api-guidelines/blob/vNext/graph/patterns/dictionary-client-guidance.md

[baywet]

Implementation guidance.

There are two cases dictionary, and patterns.
For dictionary, we just want to represent the model as a dictionary, with no other property.
For patterns, there can be multiple pattern per model, each pattern will map to a property on the model. (e.g. odata instance fields would map to a dedicated dictionary property on the model)

deserialization for dictionaries

At generation time

1. add a .* entry in the field deserializers which would contain the code to insert into the dictionary.
2. make the model inherit from map/dictionary.

At runtime

1. look for a .* entry.
2. call the deserializer for every json value we find.

serialization for dictionaries

generation time

1. update the serialization method to loop on every entry and call the right serialization method.

deserialization for patterns

generation time

This case is a bit more advanced but similar to the dictionary

1. generate a property for each pattern
2. generate a deserializer for each pattern

at runtime

1. if a json value has an exact match, call the deserializer
2. if a json value matches one of the patterns (do the detection of them, and regex generation once outside of the loop), call the pattern deserializer
3. if the model supports additional data, stick if there

serialization of patterns

at runtime

Iterate over the entries found in the properties

[LockTar]

Is this the reason why I'm getting an object when I use a Microsoft.AspNetCore.Mvc.ValidationProblemDetails (link) class as a 400 Bad Request response in my API?

When I generate the client I get a class for the Errors dictionary of ValidationProblemDetails.

Errors is of the type public System.Collections.Generic.IDictionary<string,string[]> Errors { get; } but is generated as:

public IDictionary<string, object> AdditionalData { get; set; }

public ValidationProblemDetails_errors() {
    AdditionalData = new Dictionary<string, object>();
}

So as type of string, object in stead of string, string[] and also wrapped around an object...

@baywet or @darrelmiller do you have any idea or workaround for this? Thanks!

[baywet]

@LockTar yes it seems this is impacting your client generation. Are you trying to access the data in this dictionary or could you ignore it in your application?

[LockTar]

@LockTar yes it seems this is impacting your client generation. Are you trying to access the data in this dictionary or could you ignore it in your application?

Yes I need the data in the dictionary. My API has FluentValidation and when it has validation errors I convert them to a ValidationProblemDetails. That is a class in .NET and is an industry standard for communication of errors/ProblemDetails (another class). The ValidationProblemDetails object is inheriting ProblemDetails and is extending it with an Errors property of the type Dictionary<string, string[]>. So you will get a list of validation errors with the key as the field and a string[] with all the errors of that field.

My client (user interface) binds the errors dictionary to my form... I have a workaround by deserializing the object to a string[] first but if it can be optimized it would be nice. Also I don't understand the extra class in between.

Also, how could we change the name of the exception? The exception is now called ValidationProblemDetails. But can we change it to something like in example ValidationException? Maybe with an attribute or something? The docs are a bit unclear about models and serialization.

[baywet]

I'm not sure I follow the comment about the extra class in between?

As for the class name, it's driven by the component name in the API description or the properties/endpoint names if inline. So you can tweak those to change the end class name.

[LockTar]

I needed some time to figure everything out...

As for the class name, it's driven by the component name in the API description or the properties/endpoint names if inline. So you can tweak those to change the end class name.

Ok so I played a bit with it. Because I generate the OpenAPI spec with Swashbuckle. I haven't figured out a way to change the name of the component with an attribute. So I created a new class called ValidationProblemDetailsException (originally I used the ValidationProblemDetails class from .net) that has exactly the same properties as the class in .net. But now I can add some things like example values and nice titles. So my class in example has the [SwaggerSchema(Title = "Validation Problem Details")] attribute. See below for more information.

I'm not sure I follow the comment about the extra class in between?

To focus on the Errors dictionary:
My API in example returns the following response (.net ValidationProblemDetails):

{
    "title": "One or more validation errors occurred.",
    "status": 400,
    "detail": "Please refer to the errors property for additional details.",
    "instance": "/api/address/here",
    "errors": {
        "Prop1": [
            "Error for prop1"
        ],
        "Code": [
            "'Code' must not be empty.",
            "'Code' must be 6 characters in length. You entered 0 characters."
        ]
    }
}

My response in the swagger file is generated from that same class (or see above from ValidationProblemDetailsException that is exactly the same).

So this is the complete C#:

[SwaggerSchema(Title = "Validation Problem Details")]
public class ValidationProblemDetailsException : ProblemDetailsException
{
    /// <summary>
    /// Gets the validation errors associated with this instance.
    /// </summary>
    /// <example>{"prop1": ["error1","error2"], "prop2": ["error1","error2"]}</example>
    public IDictionary<string, string[]> Errors { get; } = new Dictionary<string, string[]>(StringComparer.Ordinal);
}

public class ProblemDetailsException
{
    /// <summary>
    /// A URI reference [RFC3986] that identifies the problem type. This specification encourages that, when
    /// dereferenced, it provide human-readable documentation for the problem type
    /// (e.g., using HTML [W3C.REC-html5-20141028]).  When this member is not present, its value is assumed to be
    /// "about:blank".
    /// </summary>
    [JsonPropertyName("type")]
    public string? Type { get; set; }

    /// <summary>
    /// A short, human-readable summary of the problem type.It SHOULD NOT change from occurrence to occurrence
    /// of the problem, except for purposes of localization(e.g., using proactive content negotiation;
    /// see[RFC7231], Section 3.4).
    /// </summary>
    /// <example>One or more validation errors occurred.</example>
    [JsonPropertyName("title")]
    public string? Title { get; set; }

    /// <summary>
    /// The HTTP status code([RFC7231], Section 6) generated by the origin server for this occurrence of the problem.
    /// </summary>
    /// <example>400</example>
    [JsonPropertyName("status")]
    public int? Status { get; set; }

    /// <summary>
    /// A human-readable explanation specific to this occurrence of the problem.
    /// </summary>
    /// <example>Please refer to the errors property for additional details.</example>
    [JsonPropertyName("detail")]
    public string? Detail { get; set; }

    /// <summary>
    /// A URI reference that identifies the specific occurrence of the problem.It may or may not yield further information if dereferenced.
    /// </summary>
    /// <example>/api-resource/v1/action</example>
    [JsonPropertyName("instance")]
    public string? Instance { get; set; }

    /// <summary>
    /// Gets the <see cref="IDictionary{TKey, TValue}"/> for extension members.
    /// <para>
    /// Problem type definitions MAY extend the problem details object with additional members. Extension members appear in the same namespace as
    /// other members of a problem type.
    /// </para>
    /// </summary>
    /// <remarks>
    /// The round-tripping behavior for <see cref="Extensions"/> is determined by the implementation of the Input \ Output formatters.
    /// In particular, complex types or collection types may not round-trip to the original type when using the built-in JSON or XML formatters.
    /// </remarks>
    [JsonExtensionData]
    public IDictionary<string, object?> Extensions { get; } = new Dictionary<string, object?>(StringComparer.Ordinal);
}

The response in the swagger file is:

"ValidationProblemDetailsException": {
  "title": "Validation Problem Details",
  "type": "object",
  "properties": {
    "type": {
      "type": "string",
      "description": "A URI reference [RFC3986] that identifies the problem type. This specification encourages that, when\r\ndereferenced, it provide human-readable documentation for the problem type\r\n(e.g., using HTML [W3C.REC-html5-20141028]).  When this member is not present, its value is assumed to be\r\n\"about:blank\".",
      "nullable": true
    },
    "title": {
      "type": "string",
      "description": "A short, human-readable summary of the problem type.It SHOULD NOT change from occurrence to occurrence\r\nof the problem, except for purposes of localization(e.g., using proactive content negotiation;\r\nsee[RFC7231], Section 3.4).",
      "nullable": true,
      "example": "One or more validation errors occurred."
    },
    "status": {
      "type": "integer",
      "description": "The HTTP status code([RFC7231], Section 6) generated by the origin server for this occurrence of the problem.",
      "format": "int32",
      "nullable": true,
      "example": 400
    },
    "detail": {
      "type": "string",
      "description": "A human-readable explanation specific to this occurrence of the problem.",
      "nullable": true,
      "example": "Please refer to the errors property for additional details."
    },
    "instance": {
      "type": "string",
      "description": "A URI reference that identifies the specific occurrence of the problem.It may or may not yield further information if dereferenced.",
      "nullable": true,
      "example": "/api-resource/v1/action"
    },
    "errors": {
      "type": "object",
      "additionalProperties": {
        "type": "array",
        "items": {
          "type": "string"
        }
      },
      "description": "Gets the validation errors associated with this instance.",
      "nullable": true,
      "readOnly": true,
      "example": {
        "prop1": [
          "error1",
          "error2"
        ],
        "prop2": [
          "error1",
          "error2"
        ]
      }
    }
  },
  "additionalProperties": {}
}

The client is generated is as follow:

public class ValidationProblemDetailsException : ApiException, IAdditionalDataHolder, IParsable {
    /// <summary>Stores additional data not described in the OpenAPI description found when deserializing. Can be used for serialization as well.</summary>
    public IDictionary<string, object> AdditionalData { get; set; }
    /// <summary>A human-readable explanation specific to this occurrence of the problem.</summary>
#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP3_1_OR_GREATER
#nullable enable
    public string? Detail { get; set; }
#nullable restore
#else
    public string Detail { get; set; }
#endif
    /// <summary>Gets the validation errors associated with this instance.</summary>
#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP3_1_OR_GREATER
#nullable enable
    public ValidationProblemDetailsException_errors? Errors { get; private set; }
#nullable restore
#else
    public ValidationProblemDetailsException_errors Errors { get; private set; }
#endif
    /// <summary>A URI reference that identifies the specific occurrence of the problem.It may or may not yield further information if dereferenced.</summary>
#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP3_1_OR_GREATER
#nullable enable
    public string? Instance { get; set; }
#nullable restore
#else
    public string Instance { get; set; }
#endif
    /// <summary>The HTTP status code([RFC7231], Section 6) generated by the origin server for this occurrence of the problem.</summary>
    public int? Status { get; set; }
    /// <summary>A short, human-readable summary of the problem type.It SHOULD NOT change from occurrence to occurrenceof the problem, except for purposes of localization(e.g., using proactive content negotiation;see[RFC7231], Section 3.4).</summary>
#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP3_1_OR_GREATER
#nullable enable
    public string? Title { get; set; }
#nullable restore
#else
    public string Title { get; set; }
#endif
    /// <summary>A URI reference [RFC3986] that identifies the problem type. This specification encourages that, whendereferenced, it provide human-readable documentation for the problem type(e.g., using HTML [W3C.REC-html5-20141028]).  When this member is not present, its value is assumed to be&quot;about:blank&quot;.</summary>
#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP3_1_OR_GREATER
#nullable enable
    public string? Type { get; set; }
#nullable restore
#else
    public string Type { get; set; }
#endif
    /// <summary>
    /// Instantiates a new ValidationProblemDetailsException and sets the default values.
    /// </summary>
    public ValidationProblemDetailsException() {
        AdditionalData = new Dictionary<string, object>();
    }
    /// <summary>
    /// Creates a new instance of the appropriate class based on discriminator value
    /// </summary>
    /// <param name="parseNode">The parse node to use to read the discriminator value and create the object</param>
    public static ValidationProblemDetailsException CreateFromDiscriminatorValue(IParseNode parseNode) {
        _ = parseNode ?? throw new ArgumentNullException(nameof(parseNode));
        return new ValidationProblemDetailsException();
    }
    /// <summary>
    /// The deserialization information for the current model
    /// </summary>
    public IDictionary<string, Action<IParseNode>> GetFieldDeserializers() {
        return new Dictionary<string, Action<IParseNode>> {
            {"detail", n => { Detail = n.GetStringValue(); } },
            {"errors", n => { Errors = n.GetObjectValue<ValidationProblemDetailsException_errors>(ValidationProblemDetailsException_errors.CreateFromDiscriminatorValue); } },
            {"instance", n => { Instance = n.GetStringValue(); } },
            {"status", n => { Status = n.GetIntValue(); } },
            {"title", n => { Title = n.GetStringValue(); } },
            {"type", n => { Type = n.GetStringValue(); } },
        };
    }
    /// <summary>
    /// Serializes information the current object
    /// </summary>
    /// <param name="writer">Serialization writer to use to serialize this model</param>
    public void Serialize(ISerializationWriter writer) {
        _ = writer ?? throw new ArgumentNullException(nameof(writer));
        writer.WriteStringValue("detail", Detail);
        writer.WriteStringValue("instance", Instance);
        writer.WriteIntValue("status", Status);
        writer.WriteStringValue("title", Title);
        writer.WriteStringValue("type", Type);
        writer.WriteAdditionalData(AdditionalData);
    }
}

So it generates an extra ValidationProblemDetailsException_errors class for the Errors property. This is the part that I don't understand. That class has a property public IDictionary<string, object> AdditionalData { get; set; } which is actually the original Dictionary<string, string[]>.

So to catch the exception and really get the errors for my API I have to do this:

catch (ValidationProblemDetailsException ex)
{
    Dictionary<string, string[]> errors = ex.Errors.AdditionalData.ToDictionary(d => d.Key, d => JsonSerializer.Deserialize<string[]>(d.Value.ToString()));

    // See errors for actual errors! Display errors in userinterface here

    throw;
}

[baywet]

Thanks for the extensive details here. Yes that extra error class is because Kiota doesn't know what to do with the dictionary (it doesn't get parsed properly by OpenAPI.net at this point) and it results in generating an extra class. It's also because the definition is inline with the property, if it was a component schema, at least the name would be "cleaner".

[LockTar]

Your welcome 👍🏻. You now have a good unit test haha.

So this will be fixed in the future? Any timeline when because this ticket is already open for a long time... Would be very nice to have. Thanks!

[baywet]

Yep, thanks :)

We're really dependent on OpenAPI.net adding support for 3.1 here. My team also works on that library and the work for 3.1 has already started a while ago. But it's a significant undertaking. You can follow progress of that with this milestone and although the release date is set to the end of the month, it's probably going to take at least another few more months.

Meanwhile we're focusing on making most of the existing languages here stable BEFORE we take that change in, that's because this change will be breaking (addition to interfaces so we can serialize/deserialize dictionaries properly).

[darrelmiller]

@LockTar We are making reasonable progress on OpenAPI 3.1 but as @baywet says it is going to a take another couple of months. As we have to make breaking changes anyway to support OpenAPI 3.1 we are taking advantage of the opportunity to make a number of other fixes that will resolve performance issues and hopefully fix external references.

@baywet We have never discussed schemas with additionalProperties that define a schema for the additional properties. I wonder if we could leverage that schema during deserialization of AdditionalData so that AdditionalData doesn't just contain strings of JSON? That would be a really interesting solution to handling OData instance annotations in a more strongly typed way.

[baywet]

@darrelmiller the challenge being we materialize that property with a non generic interface today, and changing that would also be a breaking change https://github.com/microsoft/kiota-abstractions-dotnet/blob/main/src/serialization/IAdditionalDataHolder.cs

[LockTar]

We're really dependent on OpenAPI.net adding support for 3.1 here. My team also works on that library and the work for 3.1 has already started a while ago. But it's a significant undertaking. You can follow progress of that with this milestone

I think that 795 is the best epic to track from the milestone. Thanks!

[fpoppinga]

Hey @baywet,

I also just stumbled across this issue. I'm not 100% getting behind why the OpenAPI 3.1. update and the support for dictionaries is related. I tried to identify, why e.g. this example does not generate correct code.

openapi: 3.0.0
info:
  title: Sample API
  version: 0.1.9
paths:
  /users:
    get:
      responses:
        "200":
          content:
            application/json:
              schema:
                type: object
                additionalProperties:
                  schema:
                    type: object
                    properties:
                      data:
                        type: string

And from my quick reading of the Microsoft.OpenAPI code, there exists a branch that tries to parse the schema of the additionalProperties field (cf. OpenApiSchemaDeserializer.cs#L161), but that seems not to work. Is this a bug?

I would expect the deserializer to return the schema information of the additionalProperties, but that is not the case:

using Microsoft.OpenApi.Models;
using Microsoft.OpenApi.Readers;

var document = new OpenApiStreamReader().Read(File.OpenRead("example.yml"), out var diagnostics);

var additionalProperties = document.Paths["/users"].Operations[OperationType.Get].Responses[
    "200"
].Content["application/json"]
    .Schema
    .AdditionalProperties;

Console.WriteLine(additionalProperties.Properties.Count); // prints 0

From my rough understanding of kiota, it should in principle be able to generate the correct dictionary types in this case, if the OpenApi deserializer would work correctly, shouldn't it?

[baywet]

Hi @fpoppinga
Thanks for looking into this and sharing your findings here.

The fact that you're not getting the schema properties in OpenAPI.net seems like a bug to me as well, would you mind opening an issue over there so we can have a focused discussion on this please?

As to Darrel's earlier comment, when additional properties are schematized, we haven't defined what should happen in kiota from a generation perspective. What would you expect to happen?

Lastly, dictionary types (the original issue of this topic) were introduced with 3.1, so we'll need the implementation to be complete before we can benefit from those improvements in kiota.

[fpoppinga]

Thanks for the quick reply, I appreciate it!

The fact that you're not getting the schema properties in OpenAPI.net seems like a bug to me as well, would you mind opening an issue over there so we can have a focused discussion on this please?

I did that, see #OpenAPI.NET/1427

As to Darrel's earlier comment, when additional properties are schematized, we haven't defined what should happen in kiota from a generation perspective. What would you expect to happen?

I would expect the generated AdditionalData field to be of type IDictionary<string, Dummy> and Dummy being generated from schema, instead of IDictionary<string, object> as it is now:

    public class UsersResponse : IAdditionalDataHolder, IParsable {
        /// <summary>Stores additional data not described in the OpenAPI description found when deserializing. Can be used for serialization as well.</summary>
        public IDictionary<string, object> AdditionalData { get; set; }
// ... 

Lastly, dictionary types (the original issue of this topic) were introduced with 3.1, so we'll need the implementation to be complete before we can benefit from those improvements in kiota.

Maybe I'm confusing something here, but when I talk about dictionaries, I was meaning dictionaries as defined here, and that's the OAS 3.0.

[baywet]

Thanks!
We're referring to pattern properties which was added in 3.1 here. But the work for the additional properties could be slated at the same time because we're talking about similar things.

[julealgon]

Now that the work to support OpenAPI 3.1 is complete, is there a new timeline for this particular issue? We just hit a situation where one of our endpoints relies on returning a model with a Dictionary inside it and using Kiota with that model is not working for us.

[tiagojsrios]

As mentioned by @julealgon, it would be great to have a new timeline for this.

We're looking to the possibility of moving all our generated http clients to use Kiota. However, this is currently a blocker for us.

[baywet]

@julealgon can you provide more context around OAS 3.1 support being completed please?

[julealgon]

@julealgon can you provide more context around OAS 3.1 support being completed please?

I was alluding to this being in completed state:

• Add OpenAPI 3.1 Support  OpenAPI.NET#795

[baywet]

Right, As per my last comment on this thread, although most of the implementation work is done, there's remaining work to be done before a preview version is published and we can grab that in kiota.
The better thing to follow for progress is this milestone AFAIK