Aligns request options with configuration revamp

[andrueastman]

This PR is part of microsoft/kiota#1494 and depends on microsoft/kiota#1520

Changes include

• Added a vanity "addRequestHeaders" method and added a respective test
• Updates the generated test code to use latest changes from request config revamp kiota#1520
• updates package version to preview.3

To make the PR easier to review due to the number of changes in the generation samples, the changes specifically tied to the Abstraction library can isolated by following the link below.

373ff2d

[nikithauc]

/** Configuration for the request such as headers, query parameters, and middleware options.  */
export class AttachmentsRequestBuilderPostRequestConfiguration {
    /** Request headers  */
    public headers?: Record<string, string> | undefined;
    /** Request options  */
    public options?: RequestOption[] | undefined;
}

export class ContentRequestBuilderPutRequestConfiguration {
    /** Request headers  */
    public headers?: Record<string, string> | undefined;
    /** Request options  */
    public options?: RequestOption[] | undefined;
}

The configuration properties are the same for almost of the post/put/delete request configuration classes. These RequestConfigurations contain only headers and options.

It varies for the GET request config classes based on the OData Query options for the Graph API.

My concern is this bulks up the SDK with a lot of duplicate code. Plus, an additional step is introduced to create and initiate headers or request options every time which can be skipped.

The issue targeted by this PR considers that query params may not always be present and generating separate configurations for these optional parameters works fine.

But headers and request options are commonly used and something the Kiota adds for every request. Instead of duplicating the code we could maintain a separate common wrapper for the headers and request options configurations and another one for the query params specific to request paths.

[andrueastman]

Thanks @nikithauc.

I do see that there is the possibility of optimization here if the request configuration object only has the two properties of the (headers and options). Let me investigate this and push an update on the same.

[nikithauc]

@andrueastman Thank you so much for taking in the comments!

Few thoughts but I don't have a strong preference:

• How does HTTPRequestConfiguration sound as compared to CommonRequestConfiguration or any other name which is more self explanatory ?
• I also feel
  • the X-GetRequestConfiguration classes could extend the CommonRequestConfiguration to inherit the headers and request options
    Or
  • the other way is to not include the headers and request options properties in the X-GetRequestConfigurations?
    So requests would look like :
    client.users.get(options: CommonReqConfigs, getOptions: X-GetReqConfigs)
client.users.post(options: CommonReqConfigs)

[andrueastman]

/** Configuration for the request such as headers, query parameters, and middleware options.  */
export class AttachmentsRequestBuilderPostRequestConfiguration {
    /** Request headers  */
    public headers?: Record<string, string> | undefined;
    /** Request options  */
    public options?: RequestOption[] | undefined;
}

export class ContentRequestBuilderPutRequestConfiguration {
    /** Request headers  */
    public headers?: Record<string, string> | undefined;
    /** Request options  */
    public options?: RequestOption[] | undefined;
}

The configuration properties are the same for almost of the post/put/delete request configuration classes. These RequestConfigurations contain only headers and options.

It varies for the GET request config classes based on the OData Query options for the Graph API.

My concern is this bulks up the SDK with a lot of duplicate code. Plus, an additional step is introduced to create and initiate headers or request options every time which can be skipped.

The issue targeted by this PR considers that query params may not always be present and generating separate configurations for these optional parameters works fine.

But headers and request options are commonly used and something the Kiota adds for every request. Instead of duplicating the code we could maintain a separate common wrapper for the headers and request options configurations and another one for the query params specific to request paths.

Hey @nikithauc,

Apologies but I have had to revert the PR to the changes made earlier. While working on the other languages, I realized I had sort of missed a major problem Vincent was also trying to solve.

Essentially, the reason behind the multiple classes is to avoid unnecessary breaking changes on the API surface (e.g. if QueryParameters are added to an endpoints description).

If we have a common/shared request configuration class and it results in the generation of an API method like this,

public get(requestConfiguration?: CommonRequestConfiguration | undefined, responseHandler?: ResponseHandler | undefined) : Promise<InferenceClassification | undefined> {

In the event the metadata of the API is updated (in this case to add request parameters), the type of the request configuration parameter would have to change to a specific model/type that caters for it(from CommonRequestConfiguration to InferenceClassificationGetRequestConfiguration )

public get(requestConfiguration?: InferenceClassificationGetRequestConfiguration | undefined, responseHandler?: ResponseHandler | undefined) : Promise<InferenceClassification | undefined> {

This would therefore cause a breaking change in the API surface of the SDK needing to rev it up.
However, being specific from the start would just result in the addition of a new property to the InferenceClassificationGetRequestConfiguration class and there would be no need to rev up the major version of the SDK unnecessarily.

Hopefully, this is helpful and looking forward to your feedback/thoughts on the PR.

[andrueastman]

Any chance you can take a second look? @nikithauc

[andrueastman]

Thanks @nikithauc.
Will merge this later today after microsoft/kiota#1520 as I don't have the proper permissions to do the merge.

[nikithauc]

@andrueastman There is a demo is being worked on for https://devblogs.microsoft.com/microsoft365dev/get-inspired-with-microsoft-365-apps-at-microsoft-tech-days/ and the latest releases contained bug fixes reported by the team making the demo.

I am waiting on confirmation that the latest releases work for them. It looks like they are using an older generated code. The changes in the PR is breaking so I will hold onto merging in case I need to make another bug fix release.