[BUG] [Angular] Form-Query-Params for recursive objects don't include property path

[angelaki]

In my C# WebAPI I have a key[(key/value)] query param ([FromQuery] IDictionary<string, IDictionary<string, string>> properties) producing the following OpenAPI schema:

{ "name": "properties", "in": "query", "schema": { "type": "object", "additionalProperties": { "type": "object", "additionalProperties": { "type": "string" } } } },

Generator 7.11 recursively added the properties so that e.g. an object {a:{x:1}} was passed as a.x=1 to the query params:

Object.keys(value).forEach( k => httpParams = this.addToHttpParamsRecursive(
   httpParams, value[k], key != null ? `${key}.${k}` : k));

The current version doesn't include the key:

if (explode) {
   Object.keys(value).forEach(k => {
      httpParams = this.addToHttpParams(httpParams, k, value[k], paramStyle, explode);
   });
   return httpParams;
}

always resulting in x=1. The full path to the property is lost. I guess the old logic needed to be used here, too but this will sure change the current behavior (what just is wrong, isn't it?). I'm going to create a PR immediatelly.

[angelaki]

Just created the PR. But if it's right, I'm quite confused: why wasn't this issue noticed earlier? Maybe I'm mistaken and misusing it?

[angelaki]

Just created the PR. But if it's right, I'm quite confused: why wasn't this issue noticed earlier? Maybe I'm mistaken and misusing it?

Maybe check my comment on the old, current and (my?) prefered solution: #23896 (comment)

[Mattias-Sehlstedt]

As far as I know form and explode does not support deep objects (or rather, behavior is undefined). Any reason why you are not using style deepObject? See https://swagger.io/docs/specification/v3_0/serialization/.

[angelaki]

As far as I know form and explode does not support deep objects (or rather, behavior is undefined). Any reason why you are not using style deepObject? See https://swagger.io/docs/specification/v3_0/serialization/.

I'm actually just relying on the .Net WebAPI behavior expecting dot separated property names for object on query params.

[Mattias-Sehlstedt]

What I mean is that your current specification

{
  "name": "properties",
  "in": "query",
  "schema": {
    "type": "object",
    "additionalProperties": {
      "type": "object",
      "additionalProperties": {
        "type": "string"
      }
    }
  }
}

States that your query should be serialized with style: form explode: true, since that is the default.

The behavior for this setting with a deep object is undefined, thus I would argue that it is illogical that you implement a default interpretation for what this undefined behavior is. I would argue that the proper solution is to properly document your query parameter in the specification so it has deepObject, which has a defined format as per the documentation that I shared.

[angelaki]

What I mean is that your current specification

{
"name": "properties",
"in": "query",
"schema": {
"type": "object",
"additionalProperties": {
"type": "object",
"additionalProperties": {
"type": "string"
}
}
}
}
States that your query should be serialized with style: form explode: true, since that is the default.

The behavior for this setting with a deep object is undefined, thus I would argue that it is illogical that you implement a default interpretation for what this undefined behavior is. I would argue that the proper solution is to properly document your query parameter in the specification so it has deepObject, which has a defined format as per the documentation that I shared.

Thank you pretty much for this hint! I'm not very familiar with OpenAPI so I wasn't aware that this definition is undefined. Both Microsofts OpenAPI implementation and Swashbuckle produce this output for a parameter of type IDictionary<string, IDictionary<string, string>> so they should probably adopt style to deepObject in this case.

Never the less, even if this scenario is undefined, the current implementation could maybe swap to deepObject as a fallback for type form with an object? Since the current output is just wrong. Or maybe just throw an exception to make people aware of it?

And: the deepObject implementation doesn't work for nested objects. Sure this behavior is undefined, too. But nesting them in brackets (param[a][b]=c) seams to be some kind of "actual standard"? At least the .Net implementation understands it and I guess this solution is pretty natural.

Edit: digging a bit deeper I now unterstand that this case actually isn't supported by any standard. .Net WebAPI just seams to accept it and I was using a misbehavior of an old version of this generator. But can't my fix still be applied? Both of the behaviors are wrong / undefined but at least my fix doesn't lose information (what the current implementation does) and is accepted by a pretty widespreaded implementation.

[Mattias-Sehlstedt]

Style form with an object is allowed, it is just nested objects that are not permitted. Having the client generator assume type formatting based on assumptions made from the object structure I feel is a bad practice. Why would the specification (the server) not instead simply state exactly what it wants so the client knows exactly what to do? Better to throw in that case (or make some spec-validator/codegenModel state that it is incorrect/unexpected).

Does Swashbuckle not offer some sort of post-processing that could enable setting deepObject in the specification? I have personally only worked with springdoc which is for Java-Spring applications, and there you have almost full control everywhere with regards to customization.

Adding proper handling of deep nesting to deepObject is the proper solution I would argue. And my understanding is that {a: {x: 1}} should become ?p[a][x]=1.

It is also possible to state that the query should be raw Json if that is a parsable format. But this would of course once again require specification processing/correction.

[angelaki]

Yeah, sure. I could (and will) adopt the docs (what totally makes sense!) to deepObject but I guess there still is an issue with the current implementation? For testing purposes I hard-coded changed the parameter:

localVarQueryParameters = this.addToHttpParams(
    localVarQueryParameters,
    'properties',
    <any>properties,
    QueryParamStyle.DeepObject,
    true,
);

and for an object {"Default":{"Company":"6859"}} current query is generated:

?properties%5BDefault%5D=%5Bobject%20Object%5D

I'd expect it to be

?properties%5BDefault%5D%5BCompany%5D=6859

And regarding my initial request: why not just support deepObject as form (or at least throw an exception to be technically correct)? The current behavior is a bit misleading.

[angelaki]

Yeah, sure. I could (and will) adopt the docs (what totally makes sense!) to deepObject but I guess there still is an issue with the current implementation? For testing purposes I hard-coded changed the parameter:

localVarQueryParameters = this.addToHttpParams(
    localVarQueryParameters,
    'properties',
    <any>properties,
    QueryParamStyle.DeepObject,
    true,
);

and for an object {"Default":{"Company":"6859"}} current query is generated:

?properties%5BDefault%5D=%5Bobject%20Object%5D

I'd expect it to be

?properties%5BDefault%5D%5BCompany%5D=6859

And regarding my initial request: why not just support deepObject as form (or at least throw an exception to be technically correct)? The current behavior is a bit misleading.

I've adopted my code now to generate the correct OpenAPI documentation (thats for hint!):

          {
            "name": "properties",
            "in": "query",
            "style": "deepObject",
            "explode": true,
            "schema": {
              "type": "object",
              "additionalProperties": {
                "type": "object",
                "additionalProperties": {
                  "type": "string"
                }
              }
            }
          },

but as mentioned already, the generated query is broken. Is a PR welcome? Or am I using it wrong causing the misformed query?