Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding a batch request step should not authenticate the request #2774

Open
olivermue opened this issue Dec 12, 2024 · 0 comments
Open

Adding a batch request step should not authenticate the request #2774

olivermue opened this issue Dec 12, 2024 · 0 comments
Labels
status:waiting-for-triage An issue that is yet to be reviewed or assigned type:feature New experience request

Comments

@olivermue
Copy link

olivermue commented Dec 12, 2024

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

Recently I created through the SDK a batch request to receive the members and owners of a bunch of group ids. During testing, I noticed that adding all these requests to the batch takes a relatively long time.

After some digging into the code I found the root source to be the call to RequestAdapter.ConvertToNativeRequestAsync<HttpRequestMessage>() which will call AuthenticateRequestAsync() of the given authentication provider. This is not needed, cause the token on the individual request will be ignored and not being serialized into the batch step information.

Describe the solution you'd like.

To solve this issue I created an intermediate service client with my own do nothing authentication provider, used that to create the native request and added it via AddBatchRequestStep(). After doing so, the loop with 200 requests dropped from 300ms down to 1ms:

private class DoNothingAuthenticationProvider : IAuthenticationProvider
{
    public static readonly IAuthenticationProvider Default = new DoNothingAuthenticationProvider();

    public Task AuthenticateRequestAsync(
        RequestInformation request,
        Dictionary<string, object>? additionalAuthenticationContext = null,
        CancellationToken cancellationToken = default)
    {
        return Task.CompletedTask;
    }
}

var noAuthServiceClient = new GraphServiceClient(DoNothingAuthenticationProvider.Default);

var membersRequest = serviceClient.Groups[groupId].Members.ToGetRequestInformation();
var nativeRequest = await noAuthServiceClient.RequestAdapter.ConvertToNativeRequestAsync<HttpRequestMessage>(membersRequest);
batch.AddBatchRequestStep(new BatchRequestStep($"{groupId}|members", nativeRequest));

Additional context?

To make it easier to reproduce the problem, here is some sample code that shows how AddBatchRequestStepAsync() internally calls the authentication validator:

private class SlowAuthenticationProvider : IAuthenticationProvider
{
    public async Task AuthenticateRequestAsync(
        RequestInformation request,
        Dictionary<string, object>? additionalAuthenticationContext = null,
        CancellationToken cancellationToken = default)
    {
        Console.WriteLine("Trying to authenticate request...");
        await Task.Delay(100, cancellationToken);
    }
}

public static async Task Main()
{
    var authProvider = new SlowAuthenticationProvider();
    var serviceClient = new GraphServiceClient(authProvider);
    var batch = new BatchRequestContentCollection(serviceClient);

    var membersRequest = serviceClient.Groups["abc"].Members.ToGetRequestInformation();
    // Calls the authentication provider (can't be avoided IMHO)
    var nativeRequest = await serviceClient.RequestAdapter.ConvertToNativeRequestAsync<HttpRequestMessage>(membersRequest);
    batch.AddBatchRequestStep(new BatchRequestStep("abc|members", nativeRequest));

    // Calls the authentication provider (could be avoided IMHO)
    await batch.AddBatchRequestStepAsync(membersRequest);
}
@olivermue olivermue added status:waiting-for-triage An issue that is yet to be reviewed or assigned type:feature New experience request labels Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:waiting-for-triage An issue that is yet to be reviewed or assigned type:feature New experience request
Projects
None yet
Development

No branches or pull requests

1 participant