From 283d5208f1093741491dddc6e80e426807a5e2ee Mon Sep 17 00:00:00 2001 From: vasilportey Date: Mon, 23 May 2016 19:42:35 +0300 Subject: [PATCH 1/3] refactor --- .../Validator/TypeValidationRuleTest.php | 2 +- src/Config/AbstractConfig.php | 11 ++--- .../ConfigValidator/ConfigValidator.php | 48 +++++++++---------- 3 files changed, 29 insertions(+), 32 deletions(-) diff --git a/Tests/Library/Validator/TypeValidationRuleTest.php b/Tests/Library/Validator/TypeValidationRuleTest.php index cd6c54f0..b3ceebab 100644 --- a/Tests/Library/Validator/TypeValidationRuleTest.php +++ b/Tests/Library/Validator/TypeValidationRuleTest.php @@ -29,7 +29,7 @@ class TypeValidationRuleTest extends \PHPUnit_Framework_TestCase protected function setUp() { - $this->rule = new TypeValidationRule(new ConfigValidator()); + $this->rule = new TypeValidationRule(ConfigValidator::getInstance()); } diff --git a/src/Config/AbstractConfig.php b/src/Config/AbstractConfig.php index 06e533cf..fbc8d448 100644 --- a/src/Config/AbstractConfig.php +++ b/src/Config/AbstractConfig.php @@ -10,7 +10,6 @@ use Youshido\GraphQL\Validator\ConfigValidator\ConfigValidator; -use Youshido\GraphQL\Validator\ConfigValidator\ConfigValidatorInterface; use Youshido\GraphQL\Validator\Exception\ConfigurationException; use Youshido\GraphQL\Validator\Exception\ValidationException; @@ -32,9 +31,6 @@ abstract class AbstractConfig protected $extraFieldsAllowed = null; - /** @var ConfigValidatorInterface */ - protected $validator; - /** * TypeConfig constructor. * @param array $configData @@ -53,10 +49,11 @@ public function __construct(array $configData, $contextObject = null, $finalClas $this->contextObject = $contextObject; $this->data = $configData; $this->finalClass = $finalClass; - $this->validator = new ConfigValidator($contextObject); - if (!$this->validator->validate($this->data, $this->getContextRules(), $this->extraFieldsAllowed)) { - throw new ConfigurationException('Config is not valid for ' . ($contextObject ? get_class($contextObject) : null) . "\n" . implode("\n", $this->validator->getErrorsArray(false))); + $validator = ConfigValidator::getInstance(); + + if (!$validator->validate($this->data, $this->getContextRules(), $this->extraFieldsAllowed)) { + throw new ConfigurationException('Config is not valid for ' . ($contextObject ? get_class($contextObject) : null) . "\n" . implode("\n", $validator->getErrorsArray(false))); } $this->build(); diff --git a/src/Validator/ConfigValidator/ConfigValidator.php b/src/Validator/ConfigValidator/ConfigValidator.php index c1e3b90b..e1c1f91e 100644 --- a/src/Validator/ConfigValidator/ConfigValidator.php +++ b/src/Validator/ConfigValidator/ConfigValidator.php @@ -21,19 +21,33 @@ class ConfigValidator implements ConfigValidatorInterface protected $rules = []; - protected $contextObject; - protected $extraFieldsAllowed = false; /** @var ValidationRuleInterface[] */ protected $validationRules = []; - public function __construct($contextObject = null) + /** @var ConfigValidator */ + protected static $instance; + + private function __construct() { - $this->contextObject = $contextObject; $this->initializeRules(); } + /** + * @return ConfigValidator + */ + public static function getInstance() + { + if (empty(self::$instance)) { + self::$instance = new self(); + } + + self::$instance->clearErrors(); + + return self::$instance; + } + public function validate($data, $rules = [], $extraFieldsAllowed = null) { if ($extraFieldsAllowed !== null) $this->setExtraFieldsAllowed($extraFieldsAllowed); @@ -47,7 +61,7 @@ public function validate($data, $rules = [], $extraFieldsAllowed = null) unset($fieldRules['required']); if (!array_key_exists($fieldName, $data)) { - $this->addError(new ValidationException('Field \'' . $fieldName . '\' of ' . $this->getContextName() . ' is required')); + $this->addError(new ValidationException(sprintf('Field "%s" is required', $fieldName))); continue; } @@ -59,15 +73,13 @@ public function validate($data, $rules = [], $extraFieldsAllowed = null) /** Validation of all other rules*/ foreach ($fieldRules as $ruleName => $ruleInfo) { if (!array_key_exists($ruleName, $this->validationRules)) { - $this->addError(new ValidationException('Field \'' . $fieldName . '\' has invalid rule \'' . $ruleInfo . '\'')); + $this->addError(new ValidationException(sprintf('Field "%s" has invalid rule "%s"', $fieldName, $ruleInfo))); continue; } if (!$this->validationRules[$ruleName]->validate($data[$fieldName], $ruleInfo)) { - $this->addError( - new ValidationException('Field \'' . $fieldName . '\' of ' . $this->getContextName() - . ' expected to be ' . $ruleName . ': \'' . (string)$ruleInfo . '\', but got: ' . gettype($data[$fieldName]))); + $this->addError(new ValidationException(sprintf('Field "%s" expected to be "%s" but got "%s"', $fieldName, $ruleName, gettype($data[$fieldName])))); } } } @@ -75,9 +87,7 @@ public function validate($data, $rules = [], $extraFieldsAllowed = null) if (!$this->isExtraFieldsAllowed()) { foreach (array_keys($data) as $fieldName) { if (!in_array($fieldName, $processedFields)) { - $this->addError( - new ValidationException('Field \'' . $fieldName . '\' is not expected in ' . $this->getContextName())); - + $this->addError(new ValidationException(sprintf('Field "%s" is not expected', $fieldName))); } } } @@ -90,19 +100,9 @@ protected function initializeRules() $this->validationRules['type'] = new TypeValidationRule($this); } - /** - * @return string - */ - protected function getContextName() + public function addRule($name, ValidationRuleInterface $rule) { - if (is_object($this->contextObject)) { - $class = get_class($this->contextObject); - $class = substr($class, strrpos($class, '\\') + 1); - - return $class; - } else { - return $this->contextObject ? $this->contextObject : '(context)'; - } + $this->validationRules[$name] = $rule; } public function isValid() From e56214e653beb2128ae7c2ff122c96bf56a4e957 Mon Sep 17 00:00:00 2001 From: vasilportey Date: Wed, 1 Jun 2016 20:19:01 +0300 Subject: [PATCH 2/3] new resolve field logic --- src/Execution/Processor.php | 13 ++++++++++--- src/Field/AbstractField.php | 8 -------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/Execution/Processor.php b/src/Execution/Processor.php index 6497dea2..790cb5e6 100644 --- a/src/Execution/Processor.php +++ b/src/Execution/Processor.php @@ -222,7 +222,13 @@ protected function resolveFieldValue(AbstractField $field, $contextValue, Query { $resolveInfo = new ResolveInfo($field, $query->getFields(), $field->getType(), $this->executionContext); - return $field->resolve($contextValue, $this->parseArgumentsValues($field, $query), $resolveInfo); + if ($resolveFunc = $field->getConfig()->getResolveFunction()) { + return $resolveFunc($contextValue, $this->parseArgumentsValues($field, $query), $resolveInfo); + } elseif ($propertyValue = TypeService::getPropertyValue($contextValue, $field->getName())) { + return $propertyValue; + } else { + return $field->resolve($contextValue, $this->parseArgumentsValues($field, $query), $resolveInfo); + } } /** @@ -251,9 +257,10 @@ protected function getPreResolvedValue($contextValue, FieldAst $fieldAst, Abstra $resolved = true; } - if ($field->getConfig()->getResolveFunction()) { + if ($resolveFunction = $field->getConfig()->getResolveFunction()) { $resolveInfo = new ResolveInfo($field, [$fieldAst], $field->getType(), $this->executionContext); - $resolverValue = $field->resolve($resolved ? $resolverValue : $contextValue, $fieldAst->getKeyValueArguments(), $resolveInfo); + + $resolverValue = $resolveFunction($resolved ? $resolverValue : $contextValue, $fieldAst->getKeyValueArguments(), $resolveInfo); } if (!$resolverValue && !$resolved) { diff --git a/src/Field/AbstractField.php b/src/Field/AbstractField.php index ac3fe170..12447fca 100644 --- a/src/Field/AbstractField.php +++ b/src/Field/AbstractField.php @@ -59,14 +59,6 @@ public function setType($type) */ public function resolve($value, array $args, ResolveInfo $info) { - if ($resolveFunc = $this->getConfig()->getResolveFunction()) { - $info->setReturnType($this->getType()); - - return $resolveFunc($value, $args, $info); - } elseif ($propertyValue = TypeService::getPropertyValue($value, $this->getName())) { - return $propertyValue; - } - return null; } From d3ecfe70bb61b55c84f4275fe921ac3aaa4d6843 Mon Sep 17 00:00:00 2001 From: vasilportey Date: Wed, 1 Jun 2016 20:22:31 +0300 Subject: [PATCH 3/3] to previous commit (phpstorm bug) --- Tests/Library/Field/FieldTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Tests/Library/Field/FieldTest.php b/Tests/Library/Field/FieldTest.php index 076594cd..fe5ba217 100644 --- a/Tests/Library/Field/FieldTest.php +++ b/Tests/Library/Field/FieldTest.php @@ -41,8 +41,7 @@ public function testInlineFieldCreation() } ]); - $this->assertEquals('true', $fieldWithResolve->resolve(true, [], $resolveInfo), 'Resolve bool to string'); - $this->assertEquals('CTO', $fieldWithResolve->resolve('CTO', [], $resolveInfo)); + $this->assertEquals(null, $fieldWithResolve->resolve(true, [], $resolveInfo), 'Resolve bool to string'); $fieldWithResolve->setType(new IntType()); $this->assertEquals(new IntType(), $fieldWithResolve->getType());