From 08acdd3490356c82caafd079d8da278b1a996a4d Mon Sep 17 00:00:00 2001 From: Danny van der Sluijs Date: Fri, 8 Jan 2021 21:45:27 +0100 Subject: [PATCH 1/2] Move current implementation into seperate class --- lib/Dispatcher.php | 87 ++++++----------------- lib/Reflection/Dto/Method.php | 48 +++++++++++++ lib/Reflection/Dto/Parameter.php | 35 ++++++++++ lib/Reflection/Dto/Type.php | 21 ++++++ lib/Reflection/NativeReflection.php | 104 ++++++++++++++++++++++++++++ 5 files changed, 228 insertions(+), 67 deletions(-) create mode 100644 lib/Reflection/Dto/Method.php create mode 100644 lib/Reflection/Dto/Parameter.php create mode 100644 lib/Reflection/Dto/Type.php create mode 100644 lib/Reflection/NativeReflection.php diff --git a/lib/Dispatcher.php b/lib/Dispatcher.php index 5f045df..408b54a 100644 --- a/lib/Dispatcher.php +++ b/lib/Dispatcher.php @@ -3,13 +3,9 @@ namespace AdvancedJsonRpc; +use AdvancedJsonRpc\Reflection\NativeReflection; use JsonMapper; use JsonMapper_Exception; -use phpDocumentor\Reflection\DocBlockFactory; -use phpDocumentor\Reflection\Types; -use ReflectionException; -use ReflectionMethod; -use ReflectionNamedType; class Dispatcher { @@ -24,21 +20,14 @@ class Dispatcher private $delimiter; /** - * method => ReflectionMethod[] - * - * @var ReflectionMethod - */ - private $methods; - - /** - * @var \phpDocumentor\Reflection\DocBlockFactory + * @var Reflection\NativeReflection */ - private $docBlockFactory; + private $reflection; /** - * @var \phpDocumentor\Reflection\Types\ContextFactory + * @var JsonMapper */ - private $contextFactory; + private $mapper; /** * @param object $target The target object that should receive the method calls @@ -48,9 +37,8 @@ public function __construct($target, $delimiter = '->') { $this->target = $target; $this->delimiter = $delimiter; - $this->docBlockFactory = DocBlockFactory::createInstance(); - $this->contextFactory = new Types\ContextFactory(); $this->mapper = new JsonMapper(); + $this->reflection = new NativeReflection(); } /** @@ -81,33 +69,19 @@ public function dispatch($msg) } $obj = $obj->$part; } - if (!isset($this->methods[$msg->method])) { - try { - $method = new ReflectionMethod($obj, $fn); - $this->methods[$msg->method] = $method; - } catch (ReflectionException $e) { - throw new Error($e->getMessage(), ErrorCode::METHOD_NOT_FOUND, null, $e); - } - } - $method = $this->methods[$msg->method]; - $parameters = $method->getParameters(); - if ($method->getDocComment()) { - $docBlock = $this->docBlockFactory->create( - $method->getDocComment(), - $this->contextFactory->createFromReflector($method->getDeclaringClass()) - ); - $paramTags = $docBlock->getTagsByName('param'); - } + + $method = $this->reflection->getMethodDetails($msg->method, $obj, $fn); + $args = []; if (isset($msg->params)) { // Find out the position if (is_array($msg->params)) { $args = $msg->params; } else if (is_object($msg->params)) { - foreach ($parameters as $pos => $parameter) { + foreach ($method->getParameters() as $pos => $parameter) { $value = null; foreach(get_object_vars($msg->params) as $key => $val) { - if ($parameter->name === $key) { + if ($parameter->getName() === $key) { $value = $val; break; } @@ -117,46 +91,25 @@ public function dispatch($msg) } else { throw new Error('Params must be structured or omitted', ErrorCode::INVALID_REQUEST); } + foreach ($args as $position => $value) { try { // If the type is structured (array or object), map it with JsonMapper if (is_object($value)) { // Does the parameter have a type hint? - $param = $parameters[$position]; + $param = $method->getParameters()[$position]; if ($param->hasType()) { - $paramType = $param->getType(); - if ($paramType instanceof ReflectionNamedType) { - // We have object data to map and want the class name. - // This should not include the `?` if the type was nullable. - $class = $paramType->getName(); - } else { - // Fallback for php 7.0, which is still supported (and doesn't have nullable). - $class = (string)$paramType; - } + $class = $param->getType()->getName(); $value = $this->mapper->map($value, new $class()); } - } else if (is_array($value) && isset($docBlock)) { - // Get the array type from the DocBlock - $type = $paramTags[$position]->getType(); - // For union types, use the first one that is a class array (often it is SomeClass[]|null) - if ($type instanceof Types\Compound) { - for ($i = 0; $t = $type->get($i); $i++) { - if ( - $t instanceof Types\Array_ - && $t->getValueType() instanceof Types\Object_ - && (string)$t->getValueType() !== 'object' - ) { - $class = (string)$t->getValueType()->getFqsen(); - $value = $this->mapper->mapArray($value, [], $class); - break; - } - } - } else if ($type instanceof Types\Array_) { - $class = (string)$type->getValueType()->getFqsen(); - $value = $this->mapper->mapArray($value, [], $class); - } else { + } else if (is_array($value) && $method->getDocComment() !== '') { + if (!array_key_exists($position, $method->getParamTags())) { throw new Error('Type is not matching @param tag', ErrorCode::INVALID_PARAMS); } + + // Get the array type from the DocBlock + $class = $method->getParamTags()[$position]->getType()->getName(); + $value = $this->mapper->mapArray($value, [], $class); } } catch (JsonMapper_Exception $e) { throw new Error($e->getMessage(), ErrorCode::INVALID_PARAMS, null, $e); diff --git a/lib/Reflection/Dto/Method.php b/lib/Reflection/Dto/Method.php new file mode 100644 index 0000000..e161c0f --- /dev/null +++ b/lib/Reflection/Dto/Method.php @@ -0,0 +1,48 @@ +declaringClass = $declaringClass; + $this->docComment = $docComment; + $this->parameters = $parameters; + $this->paramTags = $paramTags; + } + + public function getDeclaringClass(): string + { + return $this->declaringClass; + } + + public function getDocComment(): string + { + return $this->docComment; + } + + public function getParameters(): array + { + return $this->parameters; + } + + public function getParamTags(): array + { + return $this->paramTags; + } +} diff --git a/lib/Reflection/Dto/Parameter.php b/lib/Reflection/Dto/Parameter.php new file mode 100644 index 0000000..9e5f374 --- /dev/null +++ b/lib/Reflection/Dto/Parameter.php @@ -0,0 +1,35 @@ +name = $name; + $this->type = $type; + } + + public function getName(): string + { + return $this->name; + } + + public function getType(): Type + { + return $this->type; + } + + public function hasType(): bool + { + return isset($this->type); + } +} diff --git a/lib/Reflection/Dto/Type.php b/lib/Reflection/Dto/Type.php new file mode 100644 index 0000000..11d0134 --- /dev/null +++ b/lib/Reflection/Dto/Type.php @@ -0,0 +1,21 @@ +name = $name; + } + + public function getName(): string + { + return $this->name; + } +} diff --git a/lib/Reflection/NativeReflection.php b/lib/Reflection/NativeReflection.php new file mode 100644 index 0000000..d6d7ddf --- /dev/null +++ b/lib/Reflection/NativeReflection.php @@ -0,0 +1,104 @@ +docBlockFactory = DocBlockFactory::createInstance(); + $this->contextFactory = new Types\ContextFactory(); + } + + public function getMethodDetails($rpcMethod, $target, $nativeMethod): Method + { + if (array_key_exists($rpcMethod, $this->methods)) { + return $this->methods[$rpcMethod]; + } + + try { + $nativeMethod = new ReflectionMethod($target, $nativeMethod); + } catch (ReflectionException $e) { + throw new Error($e->getMessage(), ErrorCode::METHOD_NOT_FOUND, null, $e); + } + + $paramTags = []; + if ($nativeMethod->getDocComment()) { + $docBlock = $this->docBlockFactory->create( + $nativeMethod->getDocComment(), + $this->contextFactory->createFromReflector($nativeMethod->getDeclaringClass()) + ); + $paramTags = $docBlock->getTagsByName('param'); + } + + $method = new Method( + $nativeMethod->getDeclaringClass()->getName(), + $nativeMethod->getDocComment() ?: null, + array_map(function($p) { return $this->mapNativeReflectionParameterToParameter($p); }, $nativeMethod->getParameters()), + array_map(function($p) { return $this->mapDocBlockTagToParameter($p); }, $paramTags) + ); + + $this->methods[$rpcMethod] = $method; + + return $method; + } + + private function mapNativeReflectionParameterToParameter(\ReflectionParameter $native): Parameter + { + $types = $this->mapNativeReflectionTypeToType($native->getType()); + return new Parameter($native->getName(), $types); + } + + private function mapDocBlockTagToParameter(Tag $tag): Parameter + { + $type = $tag->getType(); + // For union types, use the first one that is a class array (often it is SomeClass[]|null) + if ($type instanceof Types\Compound) { + for ($i = 0; $t = $type->get($i); $i++) { + if ( + $t instanceof Types\Array_ + && $t->getValueType() instanceof Types\Object_ + && (string)$t->getValueType() !== 'object' + ) { + return new Parameter($tag->getName(), new Type((string)$t->getValueType()->getFqsen())); + } + } + } else if ($type instanceof Types\Array_) { + return new Parameter($tag->getName(), new Type((string)$type->getValueType()->getFqsen())); + } + } + + private function mapNativeReflectionTypeToType(\ReflectionType $native = null): Type + { + if ($native instanceof ReflectionNamedType) { + // We have object data to map and want the class name. + // This should not include the `?` if the type was nullable. + return new Type($native->getName()); + } else { + // Fallback for php 7.0, which is still supported (and doesn't have nullable). + return new Type((string) $native); + } + } +} From e2693994157e8bcc6919cf26ad5e8c22a442f618 Mon Sep 17 00:00:00 2001 From: Danny van der Sluijs Date: Fri, 8 Jan 2021 23:04:49 +0100 Subject: [PATCH 2/2] Add (rough) version of PhpDocumentor reflection to get method arguments. --- composer.json | 3 +- lib/Dispatcher.php | 16 ++-- lib/Reflection/Dto/Method.php | 31 ++------ lib/Reflection/Dto/Parameter.php | 2 +- lib/Reflection/NativeReflection.php | 45 +++++++---- lib/Reflection/PhpDocumentorReflection.php | 92 ++++++++++++++++++++++ lib/Reflection/ReflectionInterface.php | 12 +++ 7 files changed, 151 insertions(+), 50 deletions(-) create mode 100644 lib/Reflection/PhpDocumentorReflection.php create mode 100644 lib/Reflection/ReflectionInterface.php diff --git a/composer.json b/composer.json index 1c154df..b82274d 100644 --- a/composer.json +++ b/composer.json @@ -22,7 +22,8 @@ "require": { "php": "^7.1 || ^8.0", "netresearch/jsonmapper": "^1.0 || ^2.0", - "phpdocumentor/reflection-docblock": "^4.3.4 || ^5.0.0" + "phpdocumentor/reflection": "^4.0" + }, "require-dev": { "phpunit/phpunit": "^7.0 || ^8.0" diff --git a/lib/Dispatcher.php b/lib/Dispatcher.php index 408b54a..cc73367 100644 --- a/lib/Dispatcher.php +++ b/lib/Dispatcher.php @@ -3,7 +3,7 @@ namespace AdvancedJsonRpc; -use AdvancedJsonRpc\Reflection\NativeReflection; +use AdvancedJsonRpc\Reflection; use JsonMapper; use JsonMapper_Exception; @@ -20,7 +20,7 @@ class Dispatcher private $delimiter; /** - * @var Reflection\NativeReflection + * @var Reflection\ReflectionInterface */ private $reflection; @@ -38,7 +38,7 @@ public function __construct($target, $delimiter = '->') $this->target = $target; $this->delimiter = $delimiter; $this->mapper = new JsonMapper(); - $this->reflection = new NativeReflection(); + $this->reflection = new Reflection\NativeReflection(); } /** @@ -97,18 +97,18 @@ public function dispatch($msg) // If the type is structured (array or object), map it with JsonMapper if (is_object($value)) { // Does the parameter have a type hint? - $param = $method->getParameters()[$position]; + $param = $method->getParameter($position); if ($param->hasType()) { $class = $param->getType()->getName(); $value = $this->mapper->map($value, new $class()); } - } else if (is_array($value) && $method->getDocComment() !== '') { - if (!array_key_exists($position, $method->getParamTags())) { - throw new Error('Type is not matching @param tag', ErrorCode::INVALID_PARAMS); + } else if (is_array($value)) { + if (!$method->hasParameter($position)) { + throw new Error('Type information is missing', ErrorCode::INVALID_PARAMS); } // Get the array type from the DocBlock - $class = $method->getParamTags()[$position]->getType()->getName(); + $class = $method->getParameter($position)->getType()->getName(); $value = $this->mapper->mapArray($value, [], $class); } } catch (JsonMapper_Exception $e) { diff --git a/lib/Reflection/Dto/Method.php b/lib/Reflection/Dto/Method.php index e161c0f..0fc3c1d 100644 --- a/lib/Reflection/Dto/Method.php +++ b/lib/Reflection/Dto/Method.php @@ -6,43 +6,26 @@ class Method { - /** @var string */ - private $declaringClass; - /** @var string|null */ - private $docComment; /** @var Parameter[] */ private $parameters; - /** @var Parameter[] */ - private $paramTags; - /** - * @param string|null $docComment - */ - public function __construct(string $declaringClass, $docComment, array $parameters, array $paramTags) + public function __construct(array $parameters) { - $this->declaringClass = $declaringClass; - $this->docComment = $docComment; $this->parameters = $parameters; - $this->paramTags = $paramTags; - } - - public function getDeclaringClass(): string - { - return $this->declaringClass; } - public function getDocComment(): string + public function getParameters(): array { - return $this->docComment; + return $this->parameters; } - public function getParameters(): array + public function hasParameter(int $name): bool { - return $this->parameters; + return array_key_exists($name, $this->parameters); } - public function getParamTags(): array + public function getParameter(int $name): Parameter { - return $this->paramTags; + return $this->parameters[$name]; } } diff --git a/lib/Reflection/Dto/Parameter.php b/lib/Reflection/Dto/Parameter.php index 9e5f374..6754a3b 100644 --- a/lib/Reflection/Dto/Parameter.php +++ b/lib/Reflection/Dto/Parameter.php @@ -30,6 +30,6 @@ public function getType(): Type public function hasType(): bool { - return isset($this->type); + return $this->type !== null; } } diff --git a/lib/Reflection/NativeReflection.php b/lib/Reflection/NativeReflection.php index d6d7ddf..dc7281e 100644 --- a/lib/Reflection/NativeReflection.php +++ b/lib/Reflection/NativeReflection.php @@ -17,7 +17,7 @@ use ReflectionMethod; use ReflectionNamedType; -class NativeReflection +class NativeReflection implements ReflectionInterface { /** @var DocBlockFactory */ private $docBlockFactory; @@ -44,21 +44,32 @@ public function getMethodDetails($rpcMethod, $target, $nativeMethod): Method throw new Error($e->getMessage(), ErrorCode::METHOD_NOT_FOUND, null, $e); } - $paramTags = []; + $parameters = array_map(function($p) { return $this->mapNativeReflectionParameterToParameter($p); }, $nativeMethod->getParameters()); + if ($nativeMethod->getDocComment()) { $docBlock = $this->docBlockFactory->create( $nativeMethod->getDocComment(), $this->contextFactory->createFromReflector($nativeMethod->getDeclaringClass()) ); - $paramTags = $docBlock->getTagsByName('param'); + + /* Improve types from the doc block */ + if ($docBlock !== null) { + $docBlockParameters = []; + + foreach ($docBlock->getTagsByName('param') as $param) { + $docBlockParameters[$param->getVariableName()] = $this->mapDocBlockTagToParameter($param); + } + + foreach ($parameters as $position => $param) { + if (array_key_exists($param->getName(), $docBlockParameters) && $docBlockParameters[$param->getName()]->hasType()) + { + $parameters[$position] = $docBlockParameters[$param->getName()]; + } + } + } } - $method = new Method( - $nativeMethod->getDeclaringClass()->getName(), - $nativeMethod->getDocComment() ?: null, - array_map(function($p) { return $this->mapNativeReflectionParameterToParameter($p); }, $nativeMethod->getParameters()), - array_map(function($p) { return $this->mapDocBlockTagToParameter($p); }, $paramTags) - ); + $method = new Method($parameters); $this->methods[$rpcMethod] = $method; @@ -67,8 +78,8 @@ public function getMethodDetails($rpcMethod, $target, $nativeMethod): Method private function mapNativeReflectionParameterToParameter(\ReflectionParameter $native): Parameter { - $types = $this->mapNativeReflectionTypeToType($native->getType()); - return new Parameter($native->getName(), $types); + $type = $this->mapNativeReflectionTypeToType($native->getType()); + return new Parameter($native->getName(), $type); } private function mapDocBlockTagToParameter(Tag $tag): Parameter @@ -82,21 +93,23 @@ private function mapDocBlockTagToParameter(Tag $tag): Parameter && $t->getValueType() instanceof Types\Object_ && (string)$t->getValueType() !== 'object' ) { - return new Parameter($tag->getName(), new Type((string)$t->getValueType()->getFqsen())); + return new Parameter($tag->getVariableName(), new Type((string)$t->getValueType()->getFqsen())); } } } else if ($type instanceof Types\Array_) { - return new Parameter($tag->getName(), new Type((string)$type->getValueType()->getFqsen())); + return new Parameter($tag->getVariableName(), new Type((string)$type->getValueType()->getFqsen())); } } - private function mapNativeReflectionTypeToType(\ReflectionType $native = null): Type + /** @return Type|null */ + private function mapNativeReflectionTypeToType(\ReflectionType $native = null) { - if ($native instanceof ReflectionNamedType) { + if ($native instanceof ReflectionNamedType && $native->getName() !== '') { // We have object data to map and want the class name. // This should not include the `?` if the type was nullable. return new Type($native->getName()); - } else { + } + if ((string) $native !== '') { // Fallback for php 7.0, which is still supported (and doesn't have nullable). return new Type((string) $native); } diff --git a/lib/Reflection/PhpDocumentorReflection.php b/lib/Reflection/PhpDocumentorReflection.php new file mode 100644 index 0000000..67d6c30 --- /dev/null +++ b/lib/Reflection/PhpDocumentorReflection.php @@ -0,0 +1,92 @@ +projectFactory = ProjectFactory::createInstance(); + } + + public function getMethodDetails($rpcMethod, $target, $nativeMethodName): Method + { + $nativeMethod = new ReflectionMethod($target, $nativeMethodName); + $projectFiles = [new LocalFile($nativeMethod->getFileName())]; + /** @var Project $project */ + $project = $this->projectFactory->create('php-advanced-json-rpc', $projectFiles); + + /** @var \phpDocumentor\Reflection\Php\Class_ $class */ + $class = $project->getFiles()[$nativeMethod->getFileName()]->getClasses()['\\' . $nativeMethod->class]; /* @todo add error handling of multiple classes in single file */ + $methodName = '\\' . $nativeMethod->class . '::' . $nativeMethod->getName() . '()'; + $method = $class->getMethods()[$methodName]; /* @todo add error handling for missing method */ + + $parameters = array_map(function ($a) { return $this->mapPhpDocumentorReflectionParameterToParameter($a); }, $method->getArguments()); + + /* Improve types from the doc block */ + $docBlock = $method->getDocBlock(); + if ($docBlock !== null) { + $docBlockParameters = []; + + foreach ($method->getDocBlock()->getTagsByName('param') as $param) { + $docBlockParameters[$param->getVariableName()] = $this->mapDocBlockTagToParameter($param); + } + + foreach ($parameters as $position => $param) { + if (array_key_exists($param->getName(), $docBlockParameters) && $docBlockParameters[$param->getName()]->hasType()) + { + $parameters[$position] = $docBlockParameters[$param->getName()]; + } + } + } + + return new Method($parameters); + } + + private function mapPhpDocumentorReflectionParameterToParameter(Argument $argument): Parameter + { + $phpDocumentorType = $argument->getType(); + if ($phpDocumentorType === null) { + return new Parameter($argument->getName()); + } + + return new Parameter($argument->getName(), new Type((string) $phpDocumentorType)); + } + + private function mapDocBlockTagToParameter(Tag $tag): Parameter + { + $type = $tag->getType(); + // For union types, use the first one that is a class array (often it is SomeClass[]|null) + if ($type instanceof Types\Compound) { + for ($i = 0; $t = $type->get($i); $i++) { + if ( + $t instanceof Types\Array_ + && $t->getValueType() instanceof Types\Object_ + && (string)$t->getValueType() !== 'object' + ) { + return new Parameter($tag->getVariableName(), new Type((string)$t->getValueType()->getFqsen())); + } + } + } else if ($type instanceof Types\Array_) { + return new Parameter($tag->getVariableName(), new Type((string)$type->getValueType()->getFqsen())); + } + } +} diff --git a/lib/Reflection/ReflectionInterface.php b/lib/Reflection/ReflectionInterface.php new file mode 100644 index 0000000..541d19f --- /dev/null +++ b/lib/Reflection/ReflectionInterface.php @@ -0,0 +1,12 @@ +