googleapis / google-api-dotnet-client

Google APIs Client Library for .NET
https://developers.google.com/api-client-library/dotnet
Apache License 2.0
1.35k stars 526 forks source link

Per-request AddCredential not working with BatchRequest #2065

Open mikequ-taggysoft opened 2 years ago

mikequ-taggysoft commented 2 years ago

With the per-request credential example provided by @amanda-tarafa in #2064, I was able to get it working for services like Calendar and Oauth2. But I'm having trouble with GmailService.

I use GmailService via BatchRequest, like this

var cred = GoogleCredential.FromAccessToken(myToken);

var batchRequest = new BatchRequest(_gmailService);

foreach (var gmailMessage in gmailMessages)
{
    batchRequest.Queue<SendRequest>
        (_gmailService.Users.Messages.Send(gmailMessage, "me").AddCredential(cred), myCallback));
}

await batchRequest.ExecuteAsync();

I'm getting error:

Error 401, Request is missing required authentication credential. Expected OAuth 2 access token, login cookie or other valid authentication credential. See https://developers.google.com/identity/sign-in/web/devconsole-project.

mikequ-taggysoft commented 2 years ago

Just did a test, using GmailService directly works, rather it's about the BatchRequest.

Now I think of it, BatchRequest must send all requests together via one request, so obviously it cannot use multiple credentials provided by each underlying request. The BatchRequest itself must need a credential somehow.

Previously credential was provided by me on the client level so it worked. Now the client no longer has a credential.

But I do not see an AddCredential method available on BatchRequest, as the class does not inherit ClientServiceRequest

amanda-tarafa commented 2 years ago

Per call credential is not currently supported for batch (by the library) as far as I can see. I'll take a closer look tomorrow and confirm.

I'll also confirm what would be the fasted way to support it, whether to support it at the BatchRequest level or at the individual requests level. Would either work for you?

amanda-tarafa commented 2 years ago

We crossed comments. It can be implemented either way, as individual batched requests may have their own credential. But possibly implementing at the BatchRequest level and maybe failing if the individual requests have a per call credentials is going to be fastest. Would having the credential at the BatchRequest level work for you?

All of this is pending confirmation when I've taken a closer look. In the meantime if you can use non-batch requests as a workaround that would allow your app to work.

mikequ-taggysoft commented 2 years ago

Per call credential is not currently supported for batch (by the library) as far as I can see. I'll take a closer look tomorrow and confirm.

I'll also confirm what would be the fasted way to support it, whether to support it at the BatchRequest level or at the individual requests level. Would either work for you?

You're so fast. I barely finished my follow up comment :)

I'm not familiar on the internals of the Google API batch mechanism. I'd imagine the BatchRequest itself is a single request that has to carry a token of its own? Then once it gets to Google, does Google actually execute each inner request separately on their side? Or is this done from the client side (in which case the batch seems less useful?).

Yes for now, the BatchRequest level AddCredential support would be sufficient for our app, because we only batch requests from a single user at a time. Is multi-user batch even supported? I'm not sure how that'd work.

amanda-tarafa commented 2 years ago

All requests in a batch are grouped together by the library and send as part of the content of a single request, the batch request. Headers on individual requests override headers in the batch request and this applies to the Authorization header as well. This applies to most REST Google APIS, and to Gmail in particular:

If you specify a given HTTP header in both the outer request and an individual call, then the individual call header's value overrides the outer batch request header's value. The headers for an individual call apply only to that call.

For example, if you provide an Authorization header for a specific call, then that header applies only to that call. If you provide an Authorization header for the outer request, then that header applies to all of the individual calls unless they override it with Authorization headers of their own.

I still need to take a closer look at the library and which is easier/faster/cleaner to support. I'll report back here when I know more, but just to set expectations, that probably won't be until next week.

mikequ-taggysoft commented 2 years ago

@amanda-tarafa Thanks for the clarification. That makes sense. Either approach you mentioned would work for our use case. Thanks again!

amanda-tarafa commented 2 years ago

Just an update, this is still something I'll be working on soon, but I haven't yet the change to get to it.

mikequ-taggysoft commented 2 years ago

@amanda-tarafa By any chance this request can receive some love? :)

jskeet commented 2 years ago

@mikequ-taggysoft: Amanda is currently on vacation, and when she's back there are higher priorities. We haven't forgotten about this and will get to it, but I'm afraid we do need to prioritize work that will affect more customers.

amanda-tarafa commented 2 years ago

@mikequ-taggysoft Yes, this is still on my radar, but as @jskeet said some higher priority work popped up. We'll work on this for sure.