microsoftgraph / msgraph-sdk-dotnet-core

The core Microsoft Graph client library for .Net. (Microsoft.Graph.Core)
Other
149 stars 45 forks source link

Add support for own request Id when making batch requests #871

Closed MichaMican closed 1 month ago

MichaMican commented 1 month ago

Is your feature request related to a problem? Please describe the problem.

When doing batch calls i sometimes use a custom requestId. For example if i want to request some group information for multiple different groups, i used the groupId as a requestId in the past to simply get the correct response from the body without the need to have to map a generated requestId to a groupId

Describe the solution you'd like.

The Method AddBatchRequestStepAsync inside BatchRequestContent.cs is generating the requestId. This method should have an optional requestId paramter which should get passed up so to the AddBatchRequestStepAsync of BatchRequestContentCollection.cs function.

so basically i would propose to have AddBatchRequestStepAsync in BatchRequestContentCollection.cs look like this:

        public Task<string> AddBatchRequestStepAsync(RequestInformation requestInformation, string? requestId = null) //added string? requestId = null
        {
            SetupCurrentRequest();
#pragma warning disable CS0618
            return currentRequest.AddBatchRequestStepAsync(requestInformation, requestId);
#pragma warning restore CS0618
        }

and AddBatchRequestStepAsync in BatchRequestContent.cs look like this:

public async Task<string> AddBatchRequestStepAsync(RequestInformation requestInformation, string? requestId = null) //added string? requestId = null
{
    if (BatchRequestSteps.Count >= CoreConstants.BatchRequest.MaxNumberOfRequests)
        throw new ArgumentException(string.Format(ErrorConstants.Messages.MaximumValueExceeded, "Number of batch request steps", CoreConstants.BatchRequest.MaxNumberOfRequests));
    // change start
    if(requestId == null){
        requestId = Guid.NewGuid().ToString()
    }
    // change end -> requestId generation replaced so that it only is generated if no custom requestId is provided
    var requestMessage = await RequestAdapter.ConvertToNativeRequestAsync<HttpRequestMessage>(requestInformation);
    BatchRequestStep batchRequestStep = new BatchRequestStep(requestId, requestMessage);
    (BatchRequestSteps as IDictionary<string, BatchRequestStep>)!.Add(batchRequestStep.RequestId, batchRequestStep);
    return requestId;
}

This solution would also be backwords compatible, meaning it has no breaking changes because the new requestId parameter of AddBatchRequestStepAsync is optional

Additional context?

If not already in development somewehere & it's agreed that this is in scope for the library i can open a PR with the proposed changes from above

shemogumbe commented 1 month ago

Thanks @MichaMican for using the SDK and for requesting this, considering your use case, I see it as a positive to add requestId field as an optional parameter to batch requests. Feel free to contribute a PR to this

MichaMican commented 1 month ago

I have opened a PR (#887) with my proposed changes :)