From 5178f3f1d3a16c6a5028022f32566c4b6cfdb41c Mon Sep 17 00:00:00 2001 From: Guy Marriott Date: Tue, 29 Jan 2019 17:24:32 +1300 Subject: [PATCH 1/4] FIX Update GraphQL controller flushing to allow failure when building DB for the first time --- src/Controller.php | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/Controller.php b/src/Controller.php index a8ba30fca..eb3b6c1f6 100644 --- a/src/Controller.php +++ b/src/Controller.php @@ -13,6 +13,7 @@ use SilverStripe\Core\Injector\Injector; use SilverStripe\GraphQL\Auth\Handler; use SilverStripe\GraphQL\Scaffolding\StaticSchema; +use SilverStripe\ORM\Connect\DatabaseException; use SilverStripe\Security\Member; use SilverStripe\Security\Permission; use SilverStripe\Security\SecurityToken; @@ -438,10 +439,24 @@ public static function flush() foreach ($routes as $pattern => $controllerInfo) { $routeClass = (is_string($controllerInfo)) ? $controllerInfo : $controllerInfo['Controller']; if (stristr($routeClass, Controller::class) !== false) { - $inst = Injector::inst()->convertServiceProperty($routeClass); - if ($inst instanceof Controller) { - /* @var Controller $inst */ - $inst->processTypeCaching(); + try { + $inst = Injector::inst()->convertServiceProperty($routeClass); + if ($inst instanceof Controller) { + /* @var Controller $inst */ + $inst->processTypeCaching(); + } + } catch (DatabaseException $e) { + // Allow failures on table doesn't exist or no database selected as we're flushing in first DB build + $messageByLine = explode(PHP_EOL, $e->getMessage()); + + // Get the last line + $last = array_pop($messageByLine); + + if (!strpos($last, 'No database selected') !== false + && !preg_match('/\s*(table|relation) .* does(n\'t| not) exist/i', $last) + ) { + throw $e; + } } } } From ca0a898b6b0ef993c14f4d53d2a467d97e27c103 Mon Sep 17 00:00:00 2001 From: Guy Marriott Date: Thu, 31 Jan 2019 12:18:12 +1300 Subject: [PATCH 2/4] FIX Check readOne permissions against resolved DataObject In most cases the canView permission was intended to be checked against the actual object from the database as the object state may have some play on whether the object is accessible by the user. --- src/Scaffolding/Scaffolders/CRUD/ReadOne.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Scaffolding/Scaffolders/CRUD/ReadOne.php b/src/Scaffolding/Scaffolders/CRUD/ReadOne.php index e58133951..9808b1ca5 100644 --- a/src/Scaffolding/Scaffolders/CRUD/ReadOne.php +++ b/src/Scaffolding/Scaffolders/CRUD/ReadOne.php @@ -60,16 +60,20 @@ protected function createDefaultArgs(Manager $manager) */ public function resolve($object, array $args, $context, ResolveInfo $info) { - if (!$this->getDataObjectInstance()->canView($context['currentUser'])) { + // get as a list so extensions can influence it pre-query + $list = DataList::create($this->getDataObjectClass()) + ->filter('ID', $args['ID']); + $this->extend('updateList', $list, $args, $context, $info); + + $item = $list->first(); + + // Check permissions on the individual item as some permission checks may investigate saved state + if (!$item->canView($context['currentUser'])) { throw new Exception(sprintf( 'Cannot view %s', $this->getDataObjectClass() )); } - // get as a list so extensions can influence it pre-query - $list = DataList::create($this->getDataObjectClass()) - ->filter('ID', $args['ID']); - $this->extend('updateList', $list, $args, $context, $info); return $list->first(); } From 9d5d4d2918f059429550cc78d21fa44483781241 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Thu, 31 Jan 2019 11:22:21 +0200 Subject: [PATCH 3/4] FIX Correct string matching assertion for "No database selected" exception message --- src/Controller.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Controller.php b/src/Controller.php index eb3b6c1f6..72b90b81f 100644 --- a/src/Controller.php +++ b/src/Controller.php @@ -452,7 +452,7 @@ public static function flush() // Get the last line $last = array_pop($messageByLine); - if (!strpos($last, 'No database selected') !== false + if (strpos($last, 'No database selected') === false && !preg_match('/\s*(table|relation) .* does(n\'t| not) exist/i', $last) ) { throw $e; From e22aa5da881f18ba3439e1e58c607ce446c04892 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Thu, 31 Jan 2019 12:31:17 +0200 Subject: [PATCH 4/4] FIX Fall back to checking a singleton for canView() if no specific item exists This restores the similar fallback behaviour before https://github.com/silverstripe/silverstripe-graphql/pull/207 was merged, but still gives priority to the specific item you are viewing if it exists --- src/Scaffolding/Scaffolders/CRUD/ReadOne.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Scaffolding/Scaffolders/CRUD/ReadOne.php b/src/Scaffolding/Scaffolders/CRUD/ReadOne.php index 9808b1ca5..d23ecab0a 100644 --- a/src/Scaffolding/Scaffolders/CRUD/ReadOne.php +++ b/src/Scaffolding/Scaffolders/CRUD/ReadOne.php @@ -65,7 +65,8 @@ public function resolve($object, array $args, $context, ResolveInfo $info) ->filter('ID', $args['ID']); $this->extend('updateList', $list, $args, $context, $info); - $item = $list->first(); + // Fall back to getting an empty singleton to use for permission checking + $item = $list->first() ?: $this->getDataObjectInstance(); // Check permissions on the individual item as some permission checks may investigate saved state if (!$item->canView($context['currentUser'])) {