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

Incorrectly sends repeated properties #146

Closed
causal-agent opened this issue Feb 6, 2016 · 5 comments
Closed

Incorrectly sends repeated properties #146

causal-agent opened this issue Feb 6, 2016 · 5 comments

Comments

@causal-agent
Copy link
Contributor

https://github.com/Byron/google-apis-rs/blob/master/src/mako/api/lib/mbuild.mako#L522-L529

In this code, repeated properties are added to the query string as a single /-separated string.

When this is done for google_gmail1::UserMessageListCall, this error is returned:

thread '<main>' panicked at 'called `Result::unwrap()` on an `Err` value: BadRequest(ErrorResponse { error: ServerError { errors: [ServerMessage { domain: "global", reason: "invalidArgument", message: "Invalid label: /INBOX", location_type: None, location: None }], code: 400, message: "Invalid label: /INBOX" } })', ../src/libcore/result.rs:741

When performing the same API call from the API documentation, it constructs the request like this:

GET https://www.googleapis.com/gmail/v1/users/me/messages?labelIds=INBOX&labelIds=UNREAD&key={YOUR_API_KEY}
@Byron
Copy link
Owner

Byron commented Feb 7, 2016

Thanks for letting me know.

I tried to reproduce the issue using the CLI:

./gmail1 users messages-list  <my-email@google> -p label-ids=INBOX label-ids=UNREAD

However, no matter which of the three authorization scopes I use, it will always respond with

'Not authorized to request the scopes: [<one of the three scopes listed on the api documentation site>]

I know from using drive that some scopes are only allowed using a non-device authentication, which currently is not supported by yup-oauth. Maybe this is the issue here as well.
However, you seem to be able to through authorization, and I wonder if you can help me do the same to reproduce the issue.

After that, fixing it should be trivial.
Thank you

@causal-agent
Copy link
Contributor Author

I'm doing authorization outside of yup-oauth2 (with inth-oauth2 and implementing GetToken to convert tokens). The set up is a bit involved. Perhaps we could find a similar endpoint that yup-oauth2 can authorize that produces the same error?

@Byron
Copy link
Owner

Byron commented Feb 7, 2016

It seems that int-oauth2 is the crate I would have wanted to have back in the days when I started this project, and considering it can do non-device oauth, it's what I want for this project as well.

To fix your issue, it should be enough to change the separator, wouldn't it ? You could try locally, and if you come to the conclusion that it is that way for all google APIs, I'd be happy to change it or merge your PR.

What do you think ?

In the meanwhile, I will see if I can ... 'upgrade' ... yup-oauth to use inth-oauth2 for a non-device flow, which would hugely increase the usefulness of all CLIs provided here.

@causal-agent
Copy link
Contributor Author

You could try locally, and if you come to the conclusion that it is that way for all google APIs, I'd be happy to change it or merge your PR.

This is what I was wondering about. Is there documentation from Google on the way this is currently handled? I would expect all the APIs to accept repeated parameters in the same way, but perhaps that is not the case.

@Byron
Copy link
Owner

Byron commented Feb 7, 2016

I'd expect the same, and to be honest, I didn't really try my implementation in that regard. My workflow was to try certain functionality I was interested in, and fix issues on the way.

As google provides its discovery API, and is likely to auto-implement its clients/servers to some extend, I would believe that this kind of parameter handling is consistent across all APIs. If not, it's easy to make the code-generator handle exceptional cases per API, once these are discovered. Thinking about it, using a / doesn't really make sense anyway.

That said, I guess we can go for a different separator. Would you try it locally, and send a PR my way once it worked ? I'd really like your name among the list of contributors to the project.

Thanks in advance

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants