lokalise / php-lokalise-api

Lokalise API v2 PHP library.
https://app.lokalise.com/api2docs/curl/
MIT License
16 stars 11 forks source link

Cursor based pagination in v4.2 is broken (affects "keys" and "translations" endpoints) #33

Open ureimers opened 5 months ago

ureimers commented 5 months ago

Hi there,

we realized that we got no results when fetching keys and translations from the keys and translations endpoints since we updated to v4.2.

The problem appears to be twofold:

  1. The Lokalise API returns a cursor in the X-Pagination-Next-Cursor field, even if there's no next result.
  2. The Lokalise PHP SDK overwrites the previous request's result with an empty array if no results are returned in the second request.

The PHP SDK error is in \Lokalise\Endpoints\Endpoint::requestAllUsingCursor() line 199 (https://github.com/lokalise/php-lokalise-api/blob/4.2.0/Api/Endpoints/Endpoint.php#L199):

The loop reads like this:

        while (true) {
            $queryParams['cursor'] = $cursor;

            $result = $this->request($requestType, $uri, $queryParams, $body);

            // Here's the problem.
            if (is_array($result->body[$bodyResponseKey]) && !empty($result->body[$bodyResponseKey])) {
                $bodyData = array_merge($bodyData, $result->body[$bodyResponseKey]);
                $result->body[$bodyResponseKey] = $bodyData;
            }

            if (!$result->hasNextCursor()) {
                break;
            }

            $cursor = $result->getNextCursor();
        }

The problem is:

Currently the Lokalise API returns a cursor even if there aren't any more results. So $result is empty in the second iteration of the while loop. But the !empty($result->body[$bodyResponseKey])´ in theif` statement skips over the merge of the results and returns the empty result instead.

So either remove the second condition of the if statement, because array_merge($arrayWithItems, []) doesn't do any harm, or don't overwrite the $result variable.

I'll try to do a PR for you to check.

Until this if fixed, the workaround is to revert back to 4.1.2.

ureimers commented 5 months ago

@lokalise-engineering Please note that even though my PR fixes the problem for the Lokalise PHP SDK, the root cause for this is that the Lokalise API returns a cursor in the X-Pagination-Next-Cursor header even though there aren't any further results.

Otherwise, this will make all your SDKs, that support cursors, to always perform at least two requests to the keys and translations endpoints. So if you experienced strange spikes in the amount of API requests lately, this might be the cause.

In addition to the unnecessary request, this might affect other Lokalise projects as well (i.e., make them buggy).

For instance the Lokalise CLI v2 (https://github.com/lokalise/lokalise-cli-2-go) now simply returns two results:

docker run -v /tmp/locale:/opt/dest lokalise/lokalise-cli-2 lokalise2 --token=<api-token> --project-id=<project-id> translation list returns:

{
  "translations": [x entries]
}
{
  "translations" []
}

I already had an extensive chat with Artūrs about this using your on-site Intercom, so maybe he already got in touch with you.