From 2399db91a8ad5530464216685368d696e99904de Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 20 Dec 2016 13:48:10 +1300 Subject: [PATCH] API Standardise OperationResolver::resolve() API PaginatedQueryCreator is abstract PSR2 cleanup PHPDoc cleanup --- README.md | 9 +-- src/Controller.php | 5 +- src/DataObjectInterfaceTypeCreator.php | 13 ++-- src/FieldCreator.php | 6 +- src/GraphiQLController.php | 77 ++++++++++++----------- src/InterfaceTypeCreator.php | 13 ++-- src/Manager.php | 4 +- src/MutationCreator.php | 2 +- src/OperationResolver.php | 24 +++++++ src/Pagination/Connection.php | 52 ++++++++------- src/Pagination/PageInfoType.php | 2 +- src/Pagination/PaginatedQueryCreator.php | 19 +++--- src/Pagination/SortDirectionType.php | 3 +- src/Pagination/SortInputType.php | 7 ++- src/TypeCreator.php | 32 ++++------ src/Util/CaseInsensitiveFieldAccessor.php | 36 +++++------ tests/FieldCreatorTest.php | 3 +- 17 files changed, 160 insertions(+), 147 deletions(-) create mode 100644 src/OperationResolver.php diff --git a/README.md b/README.md index 7e6cb64c9..7817a7eeb 100644 --- a/README.md +++ b/README.md @@ -98,10 +98,11 @@ namespace MyProject\GraphQL; use GraphQL\Type\Definition\Type; use SilverStripe\GraphQL\QueryCreator; +use SilverStripe\GraphQL\OperationResolver; use MyProject\MyDataObject; use SilverStripe\Security\Member; -class ReadMembersQueryCreator extends QueryCreator +class ReadMembersQueryCreator extends QueryCreator implements OperationResolver { public function attributes() @@ -126,7 +127,7 @@ class ReadMembersQueryCreator extends QueryCreator } - public function resolve($args) + public function resolve($object, array $args, $context, $info) { $list = Member::get(); @@ -372,10 +373,10 @@ namespace MyProject\GraphQL; use GraphQL\Type\Definition\Type; use SilverStripe\GraphQL\MutationCreator; +use SilverStripe\GraphQL\OperationResolver; use SilverStripe\Security\Member; - -class CreateMemberMutationCreator extends MutationCreator +class CreateMemberMutationCreator extends MutationCreator implements OperationResolver { public function attributes() diff --git a/src/Controller.php b/src/Controller.php index dd09990e1..8418caaa9 100644 --- a/src/Controller.php +++ b/src/Controller.php @@ -5,7 +5,6 @@ use SilverStripe\Control\Controller as BaseController; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; -use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Config\Config; use SilverStripe\Control\Director; use Exception; @@ -37,7 +36,7 @@ public function index(HTTPRequest $request) $variables = isset($data['variables']) ? $data['variables'] : null; // Some clients (e.g. GraphiQL) double encode as string - if(is_string($variables)) { + if (is_string($variables)) { $variables = json_decode($variables, true); } @@ -48,7 +47,7 @@ public function index(HTTPRequest $request) } catch (Exception $exception) { $error = ['message' => $exception->getMessage()]; - if(Director::isDev()) { + if (Director::isDev()) { $error['code'] = $exception->getCode(); $error['file'] = $exception->getFile(); $error['line'] = $exception->getLine(); diff --git a/src/DataObjectInterfaceTypeCreator.php b/src/DataObjectInterfaceTypeCreator.php index e119d2d6c..0a746668b 100644 --- a/src/DataObjectInterfaceTypeCreator.php +++ b/src/DataObjectInterfaceTypeCreator.php @@ -2,14 +2,13 @@ namespace SilverStripe\GraphQL; -use SilverStripe\GraphQL\Util\CaseInsensitiveFieldAccessor; use GraphQL\Type\Definition\Type; /** * Base interface for any {@link DataObject} passed back as a node. - * */ -class DataObjectInterfaceTypeCreator extends InterfaceTypeCreator { +class DataObjectInterfaceTypeCreator extends InterfaceTypeCreator +{ public function attributes() { @@ -19,7 +18,8 @@ public function attributes() ]; } - public function fields() { + public function fields() + { return [ 'id' => [ 'type' => Type::nonNull(Type::int()), @@ -37,15 +37,14 @@ public function resolveType($object) { $type = null; - if($fqnType = $this->manager->getType(get_class($object))) { + if ($fqnType = $this->manager->getType(get_class($object))) { $type = $fqnType; } - if($baseType = $this->manager->getType(get_class($object))) { + if ($baseType = $this->manager->getType(get_class($object))) { $type = $baseType; } return $type; } - } diff --git a/src/FieldCreator.php b/src/FieldCreator.php index 5feed2358..f5e80e713 100644 --- a/src/FieldCreator.php +++ b/src/FieldCreator.php @@ -4,8 +4,6 @@ use SilverStripe\Core\Object; use GraphQL\Type\Definition\Type; -use SilverStripe\GraphQL\Manager; -use SilverStripe\ORM\Limitable; class FieldCreator extends Object { @@ -97,10 +95,12 @@ public function __get($key) return isset($attributes[$key]) ? $attributes[$key] : null; } + /** * Dynamically check if an attribute is set. * * @param string $key + * @return bool */ public function __isset($key) { @@ -114,7 +114,7 @@ public function __isset($key) */ protected function getResolver() { - if (!method_exists($this, 'resolve')) { + if (! method_exists($this, 'resolve')) { return null; } diff --git a/src/GraphiQLController.php b/src/GraphiQLController.php index d8f060ad9..37fbd02e5 100644 --- a/src/GraphiQLController.php +++ b/src/GraphiQLController.php @@ -5,45 +5,46 @@ use SilverStripe\Control\Controller as BaseController; use SilverStripe\Control\Director; use SilverStripe\View\Requirements; - -class GraphiQLController extends BaseController + +class GraphiQLController extends BaseController { - /** - * @var string - */ - protected $template = 'GraphiQL'; - - /** - * Initialise the controller, sanity check, load javascript - */ - public function init() - { - parent::init(); - - if(!Director::isDev()) { - return $this->httpError(403, 'The GraphiQL tool is only available in dev mode'); - } - - $routes = Director::config()->get('rules'); - $route = null; - - foreach($routes as $pattern => $controllerInfo) { - if($controllerInfo == Controller::class || is_subclass_of($controllerInfo, Controller::class)) { - $route = $pattern; - break; - } - } - - if(!$route) { - throw new \RuntimeException("There are no routes set up for a GraphQL server. You will need to add one to the SilverStripe\Control\Director.rules config setting."); - } - - Requirements::customScript( -<<httpError(403, 'The GraphiQL tool is only available in dev mode'); + return; + } + + $routes = Director::config()->get('rules'); + $route = null; + + foreach ($routes as $pattern => $controllerInfo) { + if ($controllerInfo == Controller::class || is_subclass_of($controllerInfo, Controller::class)) { + $route = $pattern; + break; + } + } + + if (!$route) { + throw new \RuntimeException("There are no routes set up for a GraphQL server. You will need to add one to the SilverStripe\Control\Director.rules config setting."); + } + + Requirements::customScript( + <<getTypeResolver(); - if(isset($resolver)) - { + if (isset($resolver)) { $attributes['resolveType'] = $resolver; } @@ -43,5 +41,4 @@ public function toType() { return new BaseInterfaceType($this->toArray()); } - } diff --git a/src/Manager.php b/src/Manager.php index 01baa7909..c97a53fbf 100644 --- a/src/Manager.php +++ b/src/Manager.php @@ -160,7 +160,7 @@ public function queryAndReturnResult($query, $params = [], $schema = null) */ public function addType(Type $type, $name = '') { - if(!$name) { + if (!$name) { $name = (string)$type; } @@ -174,7 +174,7 @@ public function addType(Type $type, $name = '') */ public function getType($name) { - if(isset($this->types[$name])) { + if (isset($this->types[$name])) { return $this->types[$name]; } else { throw new \InvalidArgumentException("Type '$name' is not a registered GraphQL type"); diff --git a/src/MutationCreator.php b/src/MutationCreator.php index 62f7c18b4..396d3b0cd 100644 --- a/src/MutationCreator.php +++ b/src/MutationCreator.php @@ -7,7 +7,7 @@ * * @todo Validation support */ -class MutationCreator extends FieldCreator +abstract class MutationCreator extends FieldCreator { } diff --git a/src/OperationResolver.php b/src/OperationResolver.php new file mode 100644 index 000000000..004c93e14 --- /dev/null +++ b/src/OperationResolver.php @@ -0,0 +1,24 @@ + */ -class Connection extends Object +class Connection extends Object implements OperationResolver { /** * @var string @@ -162,13 +159,12 @@ public function getDescription() } /** - * @param array See {@link $sortableFields} - * + * @param array $fields See {@link $sortableFields} * @return $this */ public function setSortableFields($fields) { - foreach($fields as $field => $lookup) { + foreach ($fields as $field => $lookup) { $this->sortableFields[is_numeric($field) ? $lookup : $field] = $lookup; } @@ -225,7 +221,7 @@ public function args() { $existing = $this->args; - if(!is_array($existing)) { + if (!is_array($existing)) { $existing = []; } @@ -238,7 +234,7 @@ public function args() ] ]); - if($fields = $this->getSortableFields()) { + if ($fields = $this->getSortableFields()) { $args['sortBy'] = [ 'type' => Type::listOf( Injector::inst()->create(SortInputType::class, $this->connectionName) @@ -275,7 +271,7 @@ public function fields() */ public function getEdgeType() { - if(!$this->connectedType) { + if (!$this->connectedType) { throw new InvalidArgumentException('Missing connectedType callable'); } @@ -286,7 +282,7 @@ public function getEdgeType() 'node' => [ 'type' => call_user_func($this->connectedType), 'description' => 'The node at the end of the collections edge', - 'resolve' => function($obj) { + 'resolve' => function ($obj) { return $obj; } ] @@ -313,17 +309,17 @@ public function toType() * @param array $args * @param array $context * @param ResolveInfo $info - * * @return array + * @throws \Exception */ - public function resolve($value, $args, $context, ResolveInfo $info) + public function resolve($value, array $args, $context, ResolveInfo $info) { $result = call_user_func_array( $this->connectionResolver, func_get_args() ); - if(!$result instanceof SS_List) { + if (!$result instanceof SS_List) { throw new \Exception('Connection::resolve() must resolve to a SS_List instance.'); } @@ -336,15 +332,17 @@ public function resolve($value, $args, $context, ResolveInfo $info) * {@link ArrayList}. * * @param SS_List $list - * + * @param array $args + * @param null $context + * @param ResolveInfo $info * @return array */ - public function resolveList($list, $args, $context = null, $info = null) + public function resolveList($list, array $args, $context = null, ResolveInfo $info = null) { $limit = (isset($args['limit']) && $args['limit']) ? $args['limit'] : $this->defaultLimit; $offset = (isset($args['offset'])) ? $args['offset'] : 0; - if($limit > $this->maximumLimit) { + if ($limit > $this->maximumLimit) { $limit = $this->maximumLimit; } @@ -352,31 +350,31 @@ public function resolveList($list, $args, $context = null, $info = null) $previousPage = false; $count = $list->count(); - if($list instanceof Limitable) { + if ($list instanceof Limitable) { $list = $list->limit($limit, $offset); - if($limit && (($limit + $offset) < $count)) { + if ($limit && (($limit + $offset) < $count)) { $nextPage = true; } - if($offset > 0) { + if ($offset > 0) { $previousPage = true; } } - if($list instanceof Sortable) { + if ($list instanceof Sortable) { $sortableFields = $this->getSortableFields(); - if(isset($args['sortBy']) && !empty($args['sortBy'])) { + if (isset($args['sortBy']) && !empty($args['sortBy'])) { // convert the input from the input format of field, direction // to an accepted SS_List sort format. // https://github.com/graphql/graphql-relay-js/issues/20#issuecomment-220494222 $sort = []; - foreach($args['sortBy'] as $sortInput) { + foreach ($args['sortBy'] as $sortInput) { $direction = (isset($sortInput['direction'])) ? $sortInput['direction'] : 'ASC'; - if(isset($sortInput['field'])) { - if(!array_key_exists($sortInput['field'], $sortableFields)) { + if (isset($sortInput['field'])) { + if (!array_key_exists($sortInput['field'], $sortableFields)) { throw new InvalidArgumentException(sprintf( '"%s" is not a valid sort column', $sortInput['field'] @@ -388,7 +386,7 @@ public function resolveList($list, $args, $context = null, $info = null) } } - if($sort) { + if ($sort) { $list = $list->sort($sort); } } diff --git a/src/Pagination/PageInfoType.php b/src/Pagination/PageInfoType.php index f00d4a5b0..c39bb6e4a 100644 --- a/src/Pagination/PageInfoType.php +++ b/src/Pagination/PageInfoType.php @@ -18,7 +18,7 @@ class PageInfoType extends Object public function toType() { - if(!$this->type) { + if (!$this->type) { $this->type = new ObjectType([ 'name' => 'PageInfo', 'description' => 'Information about pagination in a connection.', diff --git a/src/Pagination/PaginatedQueryCreator.php b/src/Pagination/PaginatedQueryCreator.php index 286da38ce..f3d286086 100644 --- a/src/Pagination/PaginatedQueryCreator.php +++ b/src/Pagination/PaginatedQueryCreator.php @@ -3,6 +3,7 @@ namespace SilverStripe\GraphQL\Pagination; use SilverStripe\GraphQL\Manager; +use SilverStripe\GraphQL\OperationResolver; use SilverStripe\GraphQL\QueryCreator; use GraphQL\Type\Definition\ResolveInfo; @@ -11,10 +12,10 @@ * {@link Connection} object type to encapsulate the edges, nodes and page * information. */ -class PaginatedQueryCreator extends QueryCreator +abstract class PaginatedQueryCreator extends QueryCreator implements OperationResolver { /** - * @var SilverStripe\GraphQL\Pagination\Connection + * @var Connection */ protected $connection; @@ -28,10 +29,12 @@ public function __construct(Manager $manager) $this->connection = $this->connection(); } - public function connection() - { - throw new \Exception('Missing connection() definition on "'. get_class($this) .'"'); - } + /** + * Get connection for this query + * + * @return Connection + */ + abstract public function connection(); /** * @return array @@ -46,7 +49,7 @@ public function args() */ public function type() { - return function() { + return function () { return $this->connection->toType(); }; } @@ -54,7 +57,7 @@ public function type() /** * {@inheritDoc} */ - public function resolve($value, $args, $context, ResolveInfo $info) + public function resolve($value, array $args, $context, ResolveInfo $info) { return $this->connection->resolve( $value, diff --git a/src/Pagination/SortDirectionType.php b/src/Pagination/SortDirectionType.php index a55a394f7..b42d608f0 100644 --- a/src/Pagination/SortDirectionType.php +++ b/src/Pagination/SortDirectionType.php @@ -3,7 +3,6 @@ namespace SilverStripe\GraphQL\Pagination; use SilverStripe\Core\Object; -use GraphQL\Type\Definition\Type; use GraphQL\Type\Definition\EnumType; use GraphQL\Type\Definition\ObjectType; @@ -19,7 +18,7 @@ class SortDirectionType extends Object */ public function toType() { - if(!$this->type) { + if (!$this->type) { $this->type = new EnumType([ 'name' => 'SortDirection', 'description' => 'Set order order to either ASC or DESC', diff --git a/src/Pagination/SortInputType.php b/src/Pagination/SortInputType.php index 2f99bee1d..d882ac2d7 100644 --- a/src/Pagination/SortInputType.php +++ b/src/Pagination/SortInputType.php @@ -32,6 +32,7 @@ class SortInputType extends Object */ public function __construct($name) { + parent::__construct(); $this->inputName = $name; } @@ -48,13 +49,13 @@ public function setSortableFields($sortableFields) } /** - * @return ObjectType + * @return Type */ public function toType() { $values = []; - foreach($this->sortableFields as $fieldAlias => $fieldName) { + foreach ($this->sortableFields as $fieldAlias => $fieldName) { $values[$fieldAlias] = [ 'value' => $fieldAlias ]; @@ -66,7 +67,7 @@ public function toType() 'values' => $values ]); - if(!$this->type) { + if (!$this->type) { $this->type = new InputObjectType([ 'name' => ucfirst($this->inputName) .'SortInputType', 'description' => 'Define the sorting', diff --git a/src/TypeCreator.php b/src/TypeCreator.php index 6f013add9..66e5b8d62 100644 --- a/src/TypeCreator.php +++ b/src/TypeCreator.php @@ -2,11 +2,10 @@ namespace SilverStripe\GraphQL; +use GraphQL\Type\Definition\Type; use SilverStripe\Core\Object; use GraphQL\Type\Definition\ObjectType; use GraphQL\Type\Definition\InputObjectType; -use GraphQL\Type\Definition\Type; -use SilverStripe\GraphQL\Manager; /** * Represents a GraphQL type in a way that allows customization through @@ -72,11 +71,9 @@ public function getFields() $fields = $this->fields(); $allFields = []; - foreach($fields as $name => $field) - { + foreach ($fields as $name => $field) { $resolver = $this->getFieldResolver($name, $field); - if($resolver) - { + if ($resolver) { $field['resolve'] = $resolver; } $allFields[$name] = $field; @@ -94,11 +91,11 @@ public function isInputObject() } /** - * @return ObjectType + * @return Type */ public function toType() { - if($this->isInputObject()) { + if ($this->isInputObject()) { return new InputObjectType($this->toArray()); } @@ -126,7 +123,7 @@ public function getAttributes() ] ); - if(sizeof($interfaces)) { + if (sizeof($interfaces)) { $attributes['interfaces'] = $interfaces; } @@ -141,27 +138,20 @@ public function getAttributes() protected function getFieldResolver($name, $field) { $resolveMethod = 'resolve'.ucfirst($name).'Field'; - if(isset($field['resolve'])) - { + if (isset($field['resolve'])) { // Preconfigured method return $field['resolve']; - } - else if(method_exists($this, $resolveMethod)) - { + } elseif (method_exists($this, $resolveMethod)) { // Method for a particular field $resolver = array($this, $resolveMethod); - return function() use ($resolver) - { + return function () use ($resolver) { $args = func_get_args(); return call_user_func_array($resolver, $args); }; - } - else if(method_exists($this, 'resolveField')) - { + } elseif (method_exists($this, 'resolveField')) { // Method for all fields $resolver = array($this, 'resolveField'); - return function() use ($resolver) - { + return function () use ($resolver) { $args = func_get_args(); // See 'resolveType' on https://github.com/webonyx/graphql-php return call_user_func_array($resolver, $args); diff --git a/src/Util/CaseInsensitiveFieldAccessor.php b/src/Util/CaseInsensitiveFieldAccessor.php index b3b790fb2..c0b1001e7 100644 --- a/src/Util/CaseInsensitiveFieldAccessor.php +++ b/src/Util/CaseInsensitiveFieldAccessor.php @@ -21,7 +21,8 @@ * @see http://www.php.net/manual/en/functions.user-defined.php * @see http://php.net/manual/en/function.array-change-key-case.php */ -class CaseInsensitiveFieldAccessor { +class CaseInsensitiveFieldAccessor +{ const HAS_METHOD = 'HAS_METHOD'; const HAS_FIELD = 'HAS_FIELD'; @@ -47,7 +48,7 @@ public function getValue(ViewableData $object, $fieldName, $opts = []) $objectFieldName = $this->getObjectFieldName($object, $fieldName, $opts); - if(!$objectFieldName) { + if (!$objectFieldName) { throw new InvalidArgumentException(sprintf( 'Field name or method "%s" does not exist on %s', $fieldName, @@ -56,12 +57,12 @@ public function getValue(ViewableData $object, $fieldName, $opts = []) } // Correct case for methods (e.g. canView) - if($object->hasMethod($objectFieldName)) { + if ($object->hasMethod($objectFieldName)) { return $object->{$objectFieldName}(); } // Correct case (and getters) - if($object->hasField($objectFieldName)) { + if ($object->hasField($objectFieldName)) { return $object->{$objectFieldName}; } @@ -88,7 +89,7 @@ public function setValue(ViewableData $object, $fieldName, $value, $opts = []) $objectFieldName = $this->getObjectFieldName($object, $fieldName, $opts); - if(!$objectFieldName) { + if (!$objectFieldName) { throw new InvalidArgumentException(sprintf( 'Field name "%s" does not exist on %s', $fieldName, @@ -97,17 +98,17 @@ public function setValue(ViewableData $object, $fieldName, $value, $opts = []) } // Correct case for methods (e.g. canView) - if($object->hasMethod($objectFieldName)) { + if ($object->hasMethod($objectFieldName)) { $object->{$objectFieldName}($value); } // Correct case (and getters) - if($object->hasField($objectFieldName)) { + if ($object->hasField($objectFieldName)) { $object->{$objectFieldName} = $value; } // Infer casing - if($object instanceof DataObject) { + if ($object instanceof DataObject) { $object->setField($objectFieldName, $value); } @@ -115,7 +116,7 @@ public function setValue(ViewableData $object, $fieldName, $value, $opts = []) } /** - * @param $object The object to resolve a name on + * @param ViewableData $object The object to resolve a name on * @param string $fieldName Name in different casing * @param array $opts Map of which lookups to use (class constants to booleans). * Example: [ViewableDataCaseInsensitiveFieldMapper::HAS_METHOD => true] @@ -123,22 +124,22 @@ public function setValue(ViewableData $object, $fieldName, $value, $opts = []) */ protected function getObjectFieldName(ViewableData $object, $fieldName, $opts = []) { - $optFn = function($type) use(&$opts) { + $optFn = function ($type) use (&$opts) { return (in_array($type, $opts) && $opts[$type] === true); }; // Correct case (and getters) - if($optFn(self::HAS_FIELD) && $object->hasField($fieldName)) { + if ($optFn(self::HAS_FIELD) && $object->hasField($fieldName)) { return $fieldName; } // Infer casing from DataObject fields - if($optFn(self::DATAOBJECT) && $object instanceof DataObject) { + if ($optFn(self::DATAOBJECT) && $object instanceof DataObject) { $parents = ClassInfo::ancestry($object, true); - foreach($parents as $parent) { + foreach ($parents as $parent) { $fields = DataObject::getSchema()->databaseFields($parent); - foreach($fields as $objectFieldName => $fieldClass) { - if(strcasecmp($objectFieldName, $fieldName) === 0) { + foreach ($fields as $objectFieldName => $fieldClass) { + if (strcasecmp($objectFieldName, $fieldName) === 0) { return $objectFieldName; } } @@ -148,16 +149,15 @@ protected function getObjectFieldName(ViewableData $object, $fieldName, $opts = // Setters // TODO Support for Object::$extra_methods (case sensitive array key check) $setterName = "set" . ucfirst($fieldName); - if($optFn(self::HAS_SETTER) && $object->hasMethod($setterName)) { + if ($optFn(self::HAS_SETTER) && $object->hasMethod($setterName)) { return $setterName; } // Correct case for methods (e.g. canView) - method_exists() is case insensitive - if($optFn(self::HAS_METHOD) && $object->hasMethod($fieldName)) { + if ($optFn(self::HAS_METHOD) && $object->hasMethod($fieldName)) { return $fieldName; } return null; } - } diff --git a/tests/FieldCreatorTest.php b/tests/FieldCreatorTest.php index c5c9047f5..62e2e2165 100644 --- a/tests/FieldCreatorTest.php +++ b/tests/FieldCreatorTest.php @@ -8,7 +8,8 @@ class FieldCreatorTest extends SapphireTest { public function testGetAttributesIncludesResolver() { - $mock = $this->getMockBuilder('SilverStripe\GraphQL\FieldCreator') + /** @var FieldCreator $mock */ + $mock = $this->getMockBuilder(FieldCreator::class) ->setMethods(['resolve']) ->getMock(); $mock->method('resolve')->willReturn(function () {