microsoft / kiota-abstractions-dotnet

Abstractions library for the Kiota generated SDKs in dotnet
https://aka.ms/kiota/docs
MIT License
24 stars 21 forks source link

C# - Required parameters are omitted if empty #172

Closed Ksdmg closed 5 months ago

Ksdmg commented 6 months ago

When trying to use this endpoint without all parameters being set, I would expect Kiota to send empty parameters as they have the property "allowEmptyValue" but also are "required".

    "/{currentCompany}/Erp.BO.LaborSvc/GetRows": {
      "get": {
        "tags": [ "Custom methods" ],
        "summary": "Invoke method GetRows",
        "description": "Returns a dataset containing all rows that satisfy the where clauses.",
        "operationId": "Get_GetRows",
        "parameters": [
          { "$ref": "#/parameters/currentCompany" },
          { "$ref": "#/parameters/X-API-Key" },
          {
            "name": "whereClauseLaborHed",
            "in": "query",
            "description": "whereClauseLaborHed",
            "required": true,
            "type": "string",
            "allowEmptyValue": true
          },
          {
            "name": "whereClauseLaborDtl",
            "in": "query",
            "description": "whereClauseLaborDtl",
            "required": true,
            "type": "string",
            "allowEmptyValue": true
          },
          {
            "name": "whereClauseLaborDtlAttch",
            "in": "query",
            "description": "whereClauseLaborDtlAttch",
            "required": true,
            "type": "string",
            "allowEmptyValue": true
          },
          {
            "name": "whereClauseLaborDtlComment",
            "in": "query",
            "description": "whereClauseLaborDtlComment",
            "required": true,
            "type": "string",
            "allowEmptyValue": true
          },
          {
            "name": "whereClauseLaborEquip",
            "in": "query",
            "description": "whereClauseLaborEquip",
            "required": true,
            "type": "string",
            "allowEmptyValue": true
          },
          {
            "name": "whereClauseLaborPart",
            "in": "query",
            "description": "whereClauseLaborPart",
            "required": true,
            "type": "string",
            "allowEmptyValue": true
          },
          {
            "name": "whereClauseLbrScrapSerialNumbers",
            "in": "query",
            "description": "whereClauseLbrScrapSerialNumbers",
            "required": true,
            "type": "string",
            "allowEmptyValue": true
          },
          {
            "name": "whereClauseLaborDtlGroup",
            "in": "query",
            "description": "whereClauseLaborDtlGroup",
            "required": true,
            "type": "string",
            "allowEmptyValue": true
          },
          {
            "name": "whereClauseSelectedSerialNumbers",
            "in": "query",
            "description": "whereClauseSelectedSerialNumbers",
            "required": true,
            "type": "string",
            "allowEmptyValue": true
          },
          {
            "name": "whereClauseSNFormat",
            "in": "query",
            "description": "whereClauseSNFormat",
            "required": true,
            "type": "string",
            "allowEmptyValue": true
          },
          {
            "name": "whereClauseTimeWeeklyView",
            "in": "query",
            "description": "whereClauseTimeWeeklyView",
            "required": true,
            "type": "string",
            "allowEmptyValue": true
          },
          {
            "name": "whereClauseTimeWorkHours",
            "in": "query",
            "description": "whereClauseTimeWorkHours",
            "required": true,
            "type": "string",
            "allowEmptyValue": true
          },
          {
            "name": "pageSize",
            "in": "query",
            "description": "pageSize",
            "required": true,
            "type": "integer",
            "format": "int32"
          },
          {
            "name": "absolutePage",
            "in": "query",
            "description": "absolutePage",
            "required": true,
            "type": "integer",
            "format": "int32"
          }
        ],
        "responses": {
          "200": {
            "description": "OK",
            "schema": { "$ref": "#/definitions/GetRows_output" }
          },
          "500": { "description": "Internal server error. Server is unable to process the request." }
        }
      }
    },

What Kiota actually does, is it omits these parameters resulting in an error.

As an example, this is the url that swagger generates when I select to sent empty values for all parameters where allowed:

https://{Servername}/api/v2/odata/{Company}/Erp.BO.LaborSvc/GetRows?whereClauseLaborHed=&whereClauseLaborDtl=&whereClauseLaborDtlAttch=&whereClauseLaborDtlComment=&whereClauseLaborEquip=&whereClauseLaborPart=&whereClauseLbrScrapSerialNumbers=&whereClauseLaborDtlGroup=&whereClauseSelectedSerialNumbers=&whereClauseSNFormat=&whereClauseTimeWeeklyView=&whereClauseTimeWorkHours=&pageSize=0&absolutePage=0

Versus Kiota where all parameters that have not been set are omitted:

https://{Servername}/api/v2/odata/{Company}/Erp.BO.LaborSvc/GetRows?pageSize=0&absolutePage=0

I have tried to set the parameters to string.Empty and to null but both result in the parameters being omitted. The only workaround I found was setting the parameters to a whitespace which will be set by Kiota.

darrelmiller commented 6 months ago

Thank you for reporting this. I'm pretty sure that's a bug in our URITemplate generation here https://github.com/microsoft/kiota/blob/b2fa1faad08e61a6dad249a443e9b955ec3dc93a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs#L199 . If you state that the parameters are required the key should always appear in the URL.
The URL Template for a required parameter should be created like this:

https://example.org?key={key}{&otherparam1, otherParam2}

It think we are currently generating it like this,

https://example.org{?key,otherparam1, otherParam2}

I ran into something similar with work I was doing.

baywet commented 6 months ago

Hi @ksdmg, Thanks for using kiota and for reaching out. Can you open the GetRowsRequestBuilder class please and provide us with the URI Template please? (it should start with {+baseurl}/ ), I'd like to confirm whether this is a generation issue or a runtime behavior in the abstractions code. Thanks!

Ksdmg commented 6 months ago

@baywet , here is the uri template:

"{+baseurl}/{currentCompany}/Erp.BO.EmpBasicSvc/GetRows{?whereClauseEmpBasic*,whereClauseEmpBasicAttch*,whereClauseEmpLabExpRate*,whereClauseEmpCal*,whereClausePartner*,whereClauseResourceCal*,whereClauseEmpRole*,whereClauseEntityGLC*,whereClauseEmpRoleRt*,pageSize*,absolutePage*}"
baywet commented 6 months ago

@ksdmg Thanks. I think there was a mismatch here. None of the query parameters match what's in the description you've provided (besides the paging ones) and the Erp.BO.EmpBasicSvc doesn't match the Erp.BO.LaborSvc path segment from the description. Can you double check you've provided the URI template from the matching request builder please?

Ksdmg commented 6 months ago

@baywet sorry I got confused because the methods had the same name. Here is the Erp.BO.LaborSvc.GetRows:

{+baseurl}/{currentCompany}/Erp.BO.LaborSvc/GetRows{?whereClauseLaborHed*,whereClauseLaborDtl*,whereClauseLaborDtlAttch*,whereClauseLaborDtlComment*,whereClauseLaborEquip*,whereClauseLaborPart*,whereClauseLbrScrapSerialNumbers*,whereClauseLaborDtlGroup*,whereClauseSelectedSerialNumbers*,whereClauseSNFormat*,whereClauseTimeWeeklyView*,whereClauseTimeWorkHours*,pageSize*,absolutePage*}
baywet commented 6 months ago

Thanks for confirming, this is most likely caused because of this line at runtime

https://github.com/microsoft/kiota-abstractions-dotnet/blob/a8aec2d7dc49dff89430faa59f1065b1ccd36900/src/RequestInformation.cs#L171

Transferring the issue.

baywet commented 6 months ago

@ksdmg do you think you'd be able to implement a unit test for your use case like this one and validate that it fails with the current code, but it passes when the line I outlined is removed?

Ksdmg commented 5 months ago

@baywet, no I do not have enough knowledge about Kiota to create a meaningful Unit test for this.

baywet commented 5 months ago

Thanks for letting us know. @andrueastman can you take this over when you have some time please?