From 4c50dd95ae252be32666932ee7ee79f5ce793583 Mon Sep 17 00:00:00 2001 From: Aaron Carlino Date: Thu, 29 Nov 2018 15:56:06 +1300 Subject: [PATCH 1/3] BUGFIX: Ensure list is limited appropriately before evaluating --- code/GraphQL/FolderTypeCreator.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/code/GraphQL/FolderTypeCreator.php b/code/GraphQL/FolderTypeCreator.php index 6a18cbad4..a62f38201 100644 --- a/code/GraphQL/FolderTypeCreator.php +++ b/code/GraphQL/FolderTypeCreator.php @@ -152,6 +152,8 @@ public function resolveChildrenConnection( /** @var DataList $list */ $list = Versioned::get_by_stage(File::class, 'Stage'); + $result = $childrenConnection->resolveList($list, $args, $context, $info); + $list = $result['edges']; $filterInputType = new FileFilterInputTypeCreator($this->manager); $filter['parentId'] = $object->ID; From 4a9dbd0a37111b03c1a333682ed3dee066e4fc3f Mon Sep 17 00:00:00 2001 From: Aaron Carlino Date: Sat, 1 Dec 2018 11:43:08 +1300 Subject: [PATCH 2/3] Fix pagination --- code/GraphQL/FolderTypeCreator.php | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/code/GraphQL/FolderTypeCreator.php b/code/GraphQL/FolderTypeCreator.php index a62f38201..936819d7e 100644 --- a/code/GraphQL/FolderTypeCreator.php +++ b/code/GraphQL/FolderTypeCreator.php @@ -152,13 +152,15 @@ public function resolveChildrenConnection( /** @var DataList $list */ $list = Versioned::get_by_stage(File::class, 'Stage'); - $result = $childrenConnection->resolveList($list, $args, $context, $info); - $list = $result['edges']; $filterInputType = new FileFilterInputTypeCreator($this->manager); $filter['parentId'] = $object->ID; $list = $filterInputType->filterList($list, $filter); + // Ensure that we're looking at a subset of relevant data. + $result = $childrenConnection->resolveList($list, $args); + $list = $result['edges']; + // Filter by permission $ids = $list->column('ID'); $permissionChecker = File::singleton()->getPermissionChecker(); @@ -166,16 +168,12 @@ public function resolveChildrenConnection( $ids, $context['currentUser'] ))); - // Filter by visible IDs (or force empty set if none are visible) - $list = $list->filter('ID', $canViewIDs ?: 0); - // Apply pagination - $return = $childrenConnection->resolveList( - $list, - $args - ); + $canViewList = File::get()->byIDs($canViewIDs); + + $response['edges'] = $canViewList; - return $return; + return $response; } /** From e5273bb557dfb8d0dde8870fde3709180724f85e Mon Sep 17 00:00:00 2001 From: Aaron Carlino Date: Sat, 1 Dec 2018 12:07:12 +1300 Subject: [PATCH 3/3] Fix sorting --- code/GraphQL/FolderTypeCreator.php | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/code/GraphQL/FolderTypeCreator.php b/code/GraphQL/FolderTypeCreator.php index 936819d7e..5791d9e6c 100644 --- a/code/GraphQL/FolderTypeCreator.php +++ b/code/GraphQL/FolderTypeCreator.php @@ -139,6 +139,8 @@ public function resolveChildrenConnection( Connection $childrenConnection ) { // canView() checks on parent folder are implied by the query returning $object + // Note: The inability to query permissions against the entire set means pagination + // is inaccurate when any item in the list returns false on canView() $filter = (!empty($args['filter'])) ? $args['filter'] : []; @@ -168,12 +170,15 @@ public function resolveChildrenConnection( $ids, $context['currentUser'] ))); + // Filter by visible IDs (or force empty set if none are visible) + // Remove the limit as it no longer applies. We've already filtered down to the exact + // IDs we need. + $canViewList = $list->filter('ID', $canViewIDs ?: 0) + ->limit(null); - $canViewList = File::get()->byIDs($canViewIDs); + $result['edges'] = $canViewList; - $response['edges'] = $canViewList; - - return $response; + return $result; } /**