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

Laravel & Xero #2

Closed
crichardson9 opened this issue Apr 27, 2021 · 4 comments
Closed

Laravel & Xero #2

crichardson9 opened this issue Apr 27, 2021 · 4 comments
Assignees
Labels
question Further information is requested

Comments

@crichardson9
Copy link

This package looks really useful, but I'm struggling to use it with webfox/laravel-xero-oauth2 (a package that implements the official Xero PHP library: xeroapi/xero-php-oauth2). Can anyone point me in the right direction?

@judgej
Copy link
Member

judgej commented Apr 27, 2021

With Xero OAuth 1, the rate limit was always a bit of a mystery, and you never knew we the rate limit was going to be exceeded until it was already exceeded. This package was created to keep track of the count of requests in a *rolling period (60 seconds) to give is advance notice.

For Xero OAuth 2.0, every request comes with headers to give at least some clues as to how many requests are available. It does not give you quite as much information, but for us it is enough to tell whether we can go ahead and run a job, or put the job to sleep for some time before checking again. Each job starts by making a single request, then checking the headers of the response to get an idea.

We also use distributed locks yo ensure only one job is making requests to a single Xero organisation at any one time, so once a job starts, and knows it has - say - 50 requests to use without overrunnig the rate limit, then it knows it has exclusive access.

That's where I am right now, and it seems to be working well. That said, this package can still give useful information for other strategies.

@judgej
Copy link
Member

judgej commented Apr 27, 2021

Now, to use. The official Xero API library is built to use Guzzle as the HTTP client. It generates PSR-7 requests and sends it via Guzzle (using the non PSR-18 method, send() rather than sendRequest(), but the result is largely the same).

What this package does, is wrap itself around the PSR-18 Guzzle client, and count what requests flow through it and when. If you instantiate a Guzzle client, wrap it in this package, then pass this into the API class (e.g. the Accounting PI below, check out the constructor) then it should function.

https://github.com/XeroAPI/xero-php-oauth2/blob/master/lib/Api/AccountingApi.php

Now, the problem I think is the official library using send() rather than sendRequest(). They are almost the same, but sendRequest() does not throw exceptions on non-2xx responses, as PSR-18 specifies. That is a feature or a bug of the library, depending on your needs.

This package intercepts messages going through sendRequest() so it may not count requests that the API library sends. A PR to expand this package to monitor send() too would be accepted. In our case, we build the API libraries ourselves, so have full control of the client methods used.

Laravel 7+ also has PSR-6 cache pools built in, which this package can use, but it's not well documented.

So options are:

  • Generate your own API library to use PSR-18 client methods. I think Xero have included the build scripts and Mustache templates on their repo).
  • A PR for this package to use the non-PSR-18 client method method.
  • Switch to monitoring the response headers that Xero return instead. Use $api->fooBarWithHttpInfo(...) methods to get access to the raw PSR-7 response with the headers in, rather that just the parsed payload of the .$api->fooBar(...) methods.

@crichardson9
Copy link
Author

Thanks @judgej that's all really helpful. I've dug a little deeper into XeroAPI\XeroPHP\Api\AccountingApi and think our best option right now is to monitor the response headers that Xero return (e.g. $api->fooBarWithHttpInfo(...)).

If I get chance, I'll look at putting a PR together for this package, so send() is monitored too, but think I'll also need to extend the AccountingApi class to get hold of $this->client.

@judgej
Copy link
Member

judgej commented Apr 27, 2021

AccountingApi will take a Guzzle instance when you instantiate it. If you pass one, it uses it; if you don't, it instantiates it's own. It doesn't take a PSR-18 client interface that is not Guzzle though.

I think the approach you are taking is the best way forward though, so I'll close this issue for now as a question. Have fun.

@judgej judgej closed this as completed Apr 27, 2021
@judgej judgej self-assigned this Apr 27, 2021
@judgej judgej added the question Further information is requested label Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants