From 064a4ef1856bd1455875919e29308235d22089ec Mon Sep 17 00:00:00 2001 From: Patrick McLain Date: Tue, 4 Jun 2019 08:10:14 -0400 Subject: [PATCH 1/3] Return error when invalid currentPage is provided Adds missing validation for the minimum page value Fixes #727 --- .../Model/Resolver/Category/Products.php | 4 +++ .../Model/Resolver/Products.php | 4 +++ .../GraphQl/Catalog/ProductSearchTest.php | 31 +++++++++++++++++++ 3 files changed, 39 insertions(+) diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Category/Products.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Category/Products.php index 7a41f8fc94e74..1d84fd258f96c 100644 --- a/app/code/Magento/CatalogGraphQl/Model/Resolver/Category/Products.php +++ b/app/code/Magento/CatalogGraphQl/Model/Resolver/Category/Products.php @@ -66,6 +66,10 @@ public function resolve( ] ]; $searchCriteria = $this->searchCriteriaBuilder->build($field->getName(), $args); + if ($args['currentPage'] < 1) { + throw new GraphQlInputException(__('currentPage value must be greater than 0.')); + } + $searchCriteria->setCurrentPage($args['currentPage']); $searchCriteria->setPageSize($args['pageSize']); $searchResult = $this->filterQuery->getResult($searchCriteria, $info); diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products.php index 24c5e664831e4..80607351b9c4c 100644 --- a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products.php +++ b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products.php @@ -71,6 +71,10 @@ public function resolve( array $args = null ) { $searchCriteria = $this->searchCriteriaBuilder->build($field->getName(), $args); + if ($args['currentPage'] < 1) { + throw new GraphQlInputException(__('currentPage value must be greater than 0.')); + } + $searchCriteria->setCurrentPage($args['currentPage']); $searchCriteria->setPageSize($args['pageSize']); if (!isset($args['search']) && !isset($args['filter'])) { diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php index 99de6088b19a7..d113363875c96 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php @@ -1131,6 +1131,37 @@ public function testFilterProductsThatAreOutOfStockWithConfigSettings() $this->assertEquals(1, $response['products']['total_count']); } + /** + * Verify that invalid page numbers return an error + * + * @magentoApiDataFixture Magento/Catalog/_files/products_with_layered_navigation_attribute.php + * @expectedException \Exception + * @expectedExceptionMessage currentPage value must be greater than 0 + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + */ + public function testInvalidPageNumbers() + { + $query = <<graphQlQuery($query); + } + /** * Asserts the different fields of items returned after search query is executed * From 8c1829d7b4fe806ddb4674b6ca1538c4633ad5b8 Mon Sep 17 00:00:00 2001 From: Lena Orobei Date: Fri, 7 Jun 2019 15:44:09 -0500 Subject: [PATCH 2/3] GraphQl-727: currentPage: 0, currentPage: 1 and currentPage: -1 produces the same output for products query when filtering is used - added pageSize validation; - fixed code style issues. --- .../Model/Resolver/Category/Products.php | 3 ++ .../Model/Resolver/Products.php | 3 ++ .../GraphQl/Catalog/ProductSearchTest.php | 51 ++++++++++++++++--- 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Category/Products.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Category/Products.php index 1d84fd258f96c..556bfe4524868 100644 --- a/app/code/Magento/CatalogGraphQl/Model/Resolver/Category/Products.php +++ b/app/code/Magento/CatalogGraphQl/Model/Resolver/Category/Products.php @@ -69,6 +69,9 @@ public function resolve( if ($args['currentPage'] < 1) { throw new GraphQlInputException(__('currentPage value must be greater than 0.')); } + if ($args['pageSize'] < 1) { + throw new GraphQlInputException(__('pageSize value must be greater than 0.')); + } $searchCriteria->setCurrentPage($args['currentPage']); $searchCriteria->setPageSize($args['pageSize']); diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products.php index 80607351b9c4c..f588266aca08e 100644 --- a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products.php +++ b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products.php @@ -74,6 +74,9 @@ public function resolve( if ($args['currentPage'] < 1) { throw new GraphQlInputException(__('currentPage value must be greater than 0.')); } + if ($args['pageSize'] < 1) { + throw new GraphQlInputException(__('pageSize value must be greater than 0.')); + } $searchCriteria->setCurrentPage($args['currentPage']); $searchCriteria->setPageSize($args['pageSize']); diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php index d113363875c96..4b7d8d64f9459 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php @@ -381,8 +381,10 @@ public function testSearchWithFilterWithPageSizeEqualTotalCount() } QUERY; $this->expectException(\Exception::class); - $this->expectExceptionMessage('GraphQL response contains errors: currentPage value 2 specified is greater ' . - 'than the 1 page(s) available'); + $this->expectExceptionMessage( + 'GraphQL response contains errors: currentPage value 2 specified is greater ' . + 'than the 1 page(s) available' + ); $this->graphQlQuery($query); } @@ -1043,8 +1045,10 @@ public function testQueryPageOutOfBoundException() QUERY; $this->expectException(\Exception::class); - $this->expectExceptionMessage('GraphQL response contains errors: currentPage value 2 specified is greater ' . - 'than the 1 page(s) available.'); + $this->expectExceptionMessage( + 'GraphQL response contains errors: currentPage value 2 specified is greater ' . + 'than the 1 page(s) available.' + ); $this->graphQlQuery($query); } @@ -1075,8 +1079,10 @@ public function testQueryWithNoSearchOrFilterArgumentException() QUERY; $this->expectException(\Exception::class); - $this->expectExceptionMessage('GraphQL response contains errors: \'search\' or \'filter\' input argument is ' . - 'required.'); + $this->expectExceptionMessage( + 'GraphQL response contains errors: \'search\' or \'filter\' input argument is ' . + 'required.' + ); $this->graphQlQuery($query); } @@ -1162,6 +1168,37 @@ public function testInvalidPageNumbers() $this->graphQlQuery($query); } + /** + * Verify that invalid page size returns an error + * + * @magentoApiDataFixture Magento/Catalog/_files/products_with_layered_navigation_attribute.php + * @expectedException \Exception + * @expectedExceptionMessage pageSize value must be greater than 0 + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + */ + public function testInvalidPageSize() + { + $query = <<graphQlQuery($query); + } + /** * Asserts the different fields of items returned after search query is executed * @@ -1171,7 +1208,7 @@ public function testInvalidPageNumbers() private function assertProductItems(array $filteredProducts, array $actualResponse) { $productItemsInResponse = array_map(null, $actualResponse['products']['items'], $filteredProducts); - + // phpcs:ignore Generic.CodeAnalysis.ForLoopWithTestFunctionCall for ($itemIndex = 0; $itemIndex < count($filteredProducts); $itemIndex++) { $this->assertNotEmpty($productItemsInResponse[$itemIndex]); $this->assertResponseFields( From 5d4520baea3712ef44e1f54c4291e8ce3b193f2e Mon Sep 17 00:00:00 2001 From: Lena Orobei Date: Fri, 7 Jun 2019 15:50:41 -0500 Subject: [PATCH 3/3] GraphQl-727: currentPage: 0, currentPage: 1 and currentPage: -1 produces the same output for products query when filtering is used - removed redundant SuppressWarnings --- .../testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php index 4b7d8d64f9459..d3a3655c08d7a 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php @@ -1143,7 +1143,6 @@ public function testFilterProductsThatAreOutOfStockWithConfigSettings() * @magentoApiDataFixture Magento/Catalog/_files/products_with_layered_navigation_attribute.php * @expectedException \Exception * @expectedExceptionMessage currentPage value must be greater than 0 - * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function testInvalidPageNumbers() { @@ -1174,7 +1173,6 @@ public function testInvalidPageNumbers() * @magentoApiDataFixture Magento/Catalog/_files/products_with_layered_navigation_attribute.php * @expectedException \Exception * @expectedExceptionMessage pageSize value must be greater than 0 - * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function testInvalidPageSize() {