cancel
Showing results for 
Search instead for 
Did you mean: 

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

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

Feature request from careys7, posted on GitHub Jan 10, 2017

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
}
20 Comments
apiuser
New Member

Comment from fvschie, posted on GitHub Nov 29, 2017

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.

apiuser
New Member

Comment from orlangur, posted on GitHub Nov 29, 2017

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.

apiuser
New Member

Comment from stevethedev, posted on GitHub Nov 29, 2017

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.

apiuser
New Member

Comment from orlangur, posted on GitHub Nov 29, 2017

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.

apiuser
New Member

Comment from stevethedev, posted on GitHub Nov 29, 2017

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.

apiuser
New Member

Comment from nekobul, posted on GitHub Nov 29, 2017

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.

apiuser
New Member

Comment from orlangur, posted on GitHub Nov 29, 2017

@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?

apiuser
New Member

Comment from fvschie, posted on GitHub Nov 29, 2017

"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.

apiuser
New Member

Comment from nekobul, posted on GitHub Nov 29, 2017

Vlad,

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

apiuser
New Member

Comment from orlangur, posted on GitHub Nov 29, 2017

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.