REST API page size & current page search criteria options return item(s) past end of data set

[careys7]

Preconditions

1. Magento 2.0.0 - 2.1.3 (Community or Enterprise) with Sample Data installed
2. Environment using magento2-docker-compose

Steps to reproduce

This issue appears to affect at least /products/ and /categories/ requests. I have added below an example for products.

1. Make API request for products specifying page size as 1, current page as 99999, eg:

GET /rest/default/V1/products/?searchCriteria[pageSize]=1&searchCriteria[currentPage]=9999 
HTTP/1.1
Host: m2.localhost:8000
Authorization: OAuth oauth_consumer_key="sitoq7tu4b1aj7kikj4irkog8tggh1ch",oauth_token="kr3bly2kbbnrr9flyws9r4mxdaagxngo",oauth_signature_method="HMAC-SHA1",oauth_timestamp="1484095699",oauth_nonce="TivkWr",oauth_version="1.0",oauth_signature="Q02C2wk4AgkDPkYAbACUoLMoXa4%3D"
Cache-Control: no-cache

Note: Page size of '1' above is a nominal value to make examples given below more concise.

Expected result

1. No items returned in response if (current page * page size) > total products, eg:

{
  "items": [],
  "search_criteria": {
    "filter_groups": [],
    "page_size": 1,
    "current_page": 99999
  },
  "total_count": 2048
}

Actual result

1. The product with highest id is returned, eg:

{
  "items": [
    {
      "id": 2048,
      "sku": "24-WG085_Group",
      "name": "Set of Sprite Yoga Straps",
      "attribute_set_id": 12,
      "status": 1,
      "visibility": 4,
      "type_id": "grouped",
      "created_at": "2016-11-13 21:24:31",
      "updated_at": "2016-11-13 21:24:31",
      "extension_attributes": [],
      "product_links": [
        {
          "sku": "24-WG085_Group",
          "link_type": "associated",
          "linked_product_sku": "24-WG085",
          "linked_product_type": "simple",
          "position": 0,
          "extension_attributes": {
            "qty": 0
          }
        },
        {
          "sku": "24-WG085_Group",
          "link_type": "associated",
          "linked_product_sku": "24-WG086",
          "linked_product_type": "simple",
          "position": 1,
          "extension_attributes": {
            "qty": 0
          }
        },
        {
          "sku": "24-WG085_Group",
          "link_type": "associated",
          "linked_product_sku": "24-WG087",
          "linked_product_type": "simple",
          "position": 2,
          "extension_attributes": {
            "qty": 0
          }
        }
      ],
      "tier_prices": [],
      "custom_attributes": [
        {
          "attribute_code": "description",
          "value": "<p>Great set of Sprite Yoga Straps for every stretch and hold you need. There are three straps in this set: 6', 8' and 10'.</p>\n<ul>\n<li> 100% soft and durable cotton.\n<li> Plastic cinch buckle is easy to use.\n<li> Choice of three natural colors made from phthalate and heavy metal free dyes.\n</ul>"
        },
        {
          "attribute_code": "image",
          "value": "/l/u/luma-yoga-strap-set.jpg"
        },
        {
          "attribute_code": "small_image",
          "value": "/l/u/luma-yoga-strap-set.jpg"
        },
        {
          "attribute_code": "thumbnail",
          "value": "/l/u/luma-yoga-strap-set.jpg"
        },
        {
          "attribute_code": "options_container",
          "value": "container2"
        },
        {
          "attribute_code": "required_options",
          "value": "0"
        },
        {
          "attribute_code": "has_options",
          "value": "0"
        },
        {
          "attribute_code": "url_key",
          "value": "set-of-sprite-yoga-straps"
        },
        {
          "attribute_code": "is_returnable",
          "value": "2"
        },
        {
          "attribute_code": "activity",
          "value": "17"
        },
        {
          "attribute_code": "material",
          "value": "41,53"
        },
        {
          "attribute_code": "gender",
          "value": "89,90,93"
        },
        {
          "attribute_code": "category_gear",
          "value": "96"
        },
        {
          "attribute_code": "size",
          "value": "100"
        }
      ]
    }
  ],
  "search_criteria": {
    "filter_groups": [],
    "page_size": 1,
    "current_page": 99999
  },
  "total_count": 2048
}

[veloraven]

@careysizer thank you for your report.
EE issues should be reported via the Support portal of your account or Partner portal if you are a partner reporting on behalf of a merchant.
Github is intended for Community edition reports given no account management for CE users. This will allow for proper tracking of issues at the account level.
Please let us know if you already have reported this issue via some portal in order to avoid duplicates in our internal tracking system.

[careys7]

@veloraven - thanks, I've updated the issue to specify that CE is also affected (appears to be all versions of both editions that I've checked).

I haven't reported this issue through the partner portal or other Magento channels.

[mbrinton01]

@misha-kotov please review MAGETWO-58841 before adding comment, this seems to be the same issue.

[fvschie]

The workaround we currently use is to check whether the last item on the previous page is identical to the last item on the current page.

[misha-kotov]

Verified and the internal issue is MAGETWO-58841

[nekobul]

This appears a very substantial bug and there is still no resolution in the latest 2.1.7 version. Do you have estimate when you will fix this issue?

[magento-engcom-team]

@careys7, thank you for your report.
We've created internal ticket(s) MAGETWO-58841 to track progress on the issue.

[stevethedev]

The root of this problem seems to live in the file vendor/magento/framework/Data/Collection.php:

    /**
     * Get current collection page
     *
     * @param  int  $displacement
     * @return int
     */
    public function getCurPage($displacement = 0)
    {
        if ($this->_curPage + $displacement < 1) {
            return 1;
        } elseif ($this->_curPage + $displacement > $this->getLastPageNumber()) {
            return $this->getLastPageNumber();
        } else {
            return $this->_curPage + $displacement;
        }
    }

The upper-limit overflow can be addressed here, but the lower-limit overflow (page <= 0) is handled by vendor/magento/zendframework1/library/Zend/Db/Select.php:

    /**
     * Sets the limit and count by page number.
     *
     * @param int $page Limit results to this page number.
     * @param int $rowCount Use this many rows per page.
     * @return Zend_Db_Select This Zend_Db_Select object.
     */
    public function limitPage($page, $rowCount)
    {
        $page     = ($page > 0)     ? $page     : 1;  //<---- This line right here
        $rowCount = ($rowCount > 0) ? $rowCount : 1;
        $this->_parts[self::LIMIT_COUNT]  = (int) $rowCount;
        $this->_parts[self::LIMIT_OFFSET] = (int) $rowCount * ($page - 1);
        return $this;
    }

Lower-limit overflow is easy to detect (page <= 0), but upper-limit overflow is not.

I have submitted a Pull Request at: #12475

[orlangur]

The workaround we currently use is to check whether the last item on the previous page is identical to the last item on the current page.

@fvschie @nekobul

From the issue description I see that API returns total amount of records: "total_count": 2048.

Could you please add more details why you need to access invalid page at all? It seems like there is some bug with last page determination in your code but why not fix invalid pagination instead of hacking core?

[fvschie]

It's certainly possible to calculate this. It's just a lot easier to keep asking for data until there's no more data.

The bug is that the request returns incorrect data. If I request page 11 of 10, showing the results for page 10 is objectively wrong.

Either an empty list. or a Bad Request or some such (given that we can calculate the max page, it would be a client error) would be correct.

[orlangur]

It's just a lot easier to keep asking for data until there's no more data.

Well, such approach is buggy and should never be used as it simply does not work. "check whether the last item on the previous page is identical" is even more strange idea.

The bug is that the request returns incorrect data. If I request page 11 of 10, showing the results for page 10 is objectively wrong.

It just always fixes invalid page to first or last. It always worked like this since Magento 1 and there is nothing wrong with it.

or a Bad Request or some such (given that we can calculate the max page, it would be a client error) would be correct.

This idea sounds good to me. It would be not OK to return some 500 HTTP error when you open invalid page in browser but looks pretty smooth for API.

[stevethedev]

Well, such approach is buggy and should never be used as it simply does not work.

Why? There is absolutely nothing wrong with a loop that says "get data until there is no more data". This a basic pattern for manipulating arrays, queues, stacks, maps, sets, dictionaries, tuples, and loops. I can't think of a single instance of this statement being true that doesn't also involve some programming error that doesn't account for asynchronicity.

[orlangur]

get data until there is no more data

@stevethedev because this criteria is not applicable here and thus causes bugs. You cannot take random algorithm, apply it to core, meet a bug and then try to "fix" core instead of fixing buggy algorithm.

[stevethedev]

random algorithm ... buggy algorithm

This is one of the most prolific algorithms in all of computing. Magento's API returns a result set that makes one of the most widely used algorithms ever created into an infinite loop.

[nekobul]

Vlad,

I have worked with more than 50 different API services and I can assure you Steve is correct. This is how you work with paging APIs. This is a definite bug in the Magento API and you have to correct it and not ask clients to jump thru hoops to make it work.

[orlangur]

@nekobul, why you don't like the

or a Bad Request or some such (given that we can calculate the max page, it would be a client error) would be correct.

approach?

[fvschie]

"It always worked like this since Magento 1 and there is nothing wrong with it."

That statement is illogical. It worked like that, but it is inherently wrong to return something different from what you are requesting.

I agree that, if it is inherited behavior, it's an improvement request and not a bug.

Also, it would be a 4xx error, not 5xx error if you go that route. Frontend should handle errors on calls to backs, should not just show up, so I don't quite get that.

As for desired behavior, I'm talking about the API, so a 400 error would probably be best.

[nekobul]

Vlad,

No data returned is preferable because it will be easier to handle from the client side.

[orlangur]

That statement is illogical. It worked like that, but it is inherently wrong to return something different from what you are requesting.

Sorry, I didn't mean this will work like this forever, more thinking of "how many customizations will be broken if we change behavior consistently".

@fvschie @nekobul if we fix it for Catalog pages as well, what would be the good behavior? Redirect to closest valid page or just show page with 0 products?

No data returned is preferable because it will be easier to handle from the client side.

@nekobul well,

"page_size": 1,
    "current_page": 99999
  },
  "total_count": 2048

request seems pretty wrong to me. Client side has some error handling anyway, isn't it? And if by accident it requests invalid page, it will be more visible than if some products are returned, like it is currently.

Looking at https://stackoverflow.com/a/3290198 I'm not sure whether 400 or 422 would be the best choice.

[piotrekkaminski]

Thank you for your submission.

We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues.

Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

We are closing this GitHub ticket and have moved your request to the new forum.

[careys7]

@piotrekkaminski is this a feature?

[amenk]

Had the same problem, changed my client...
https://magento.stackexchange.com/questions/280052/catalog-rest-api-paging-not-working/280076#280076

[atwixfirster]

@okorshenko, could you please share a status of MAGETWO-58841 ticket (in progress, closed, etc)?

Thank you!

CC: @dmytro-ch

[sdzhepa]

Hello @atwixfirster @amenk @careys7 @dmytro-ch

I just checked the internal Jira ticket MAGETWO-58841.

I see it has a huge amount of records/actions since it was created in 2016
It was Fixed, Closed, Reopen, Reverted multiple times(were a lot of attempts to fix and deliver it).

Last comments from engineering were in March-April of 2019 and it is about
"Current implementation of collections intentionally returns last page when requested page exceeds collection size. Either API changes or new collection API required. Needs to be reprioritized and converted into the Story type instead of Bug because it will need much more time and efforts"

At this moment MAGETWO-58841 still in OPEN status and has Priority = P2.
Unfortunately, I do not have any additional information like ETA or when it will be done and delivered
but I have requested from all participants of this internal ticket to share more details/updates as a comment for this issue.

[dmytro-ch]

Hello @sdzhepa, thank you for the details!

[ihor-sviziev]

FYI we have #26988 that should solve this issue.

[lbajsarowicz]

You can buy me a beer at the next conference @careys7 :-D