diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml new file mode 100644 index 0000000..7e8ead3 --- /dev/null +++ b/.github/workflows/codeql-analysis.yml @@ -0,0 +1,71 @@ +# For most projects, this workflow file will not need changing; you simply need +# to commit it to your repository. +# +# You may wish to alter this file to override the set of languages analyzed, +# or to provide custom queries or build logic. +name: "CodeQL" + +on: + push: + branches: [master] + pull_request: + # The branches below must be a subset of the branches above + branches: [master] + schedule: + - cron: '0 12 * * 6' + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + # Override automatic language detection by changing the below list + # Supported options are ['csharp', 'cpp', 'go', 'java', 'javascript', 'python'] + language: ['go'] + # Learn more... + # https://docs.github.com/en/github/finding-security-vulnerabilities-and-errors-in-your-code/configuring-code-scanning#overriding-automatic-language-detection + + steps: + - name: Checkout repository + uses: actions/checkout@v2 + with: + # We must fetch at least the immediate parents so that if this is + # a pull request then we can checkout the head. + fetch-depth: 2 + + # If this run was triggered by a pull request event, then checkout + # the head of the pull request instead of the merge commit. + - run: git checkout HEAD^2 + if: ${{ github.event_name == 'pull_request' }} + + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v1 + with: + languages: ${{ matrix.language }} + # If you wish to specify custom queries, you can do so here or in a config file. + # By default, queries listed here will override any specified in a config file. + # Prefix the list here with "+" to use these queries and those in the config file. + # queries: ./path/to/local/query, your-org/your-repo/queries@main + + # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). + # If this step fails, then you should remove it and run the build manually (see below) + - name: Autobuild + uses: github/codeql-action/autobuild@v1 + + # ℹī¸ Command-line programs to run using the OS shell. + # 📚 https://git.io/JvXDl + + # ✏ī¸ If the Autobuild fails above, remove it and uncomment the following three lines + # and modify them (or add more) to build your code if your project + # uses a compiled language + + #- run: | + # make bootstrap + # make release + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v1 diff --git a/.gitignore b/.gitignore index 42dc51b..bd558f8 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,5 @@ vendor/ clover.xml example/vendor/ go.sum -builds/ \ No newline at end of file +builds/ +.phpunit.result.cache diff --git a/.travis.yml b/.travis.yml index d948824..b63f434 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,14 +15,13 @@ install: - popd - export GO111MODULE=on - go mod download - - composer install --no-interaction --prefer-source --ignore-platform-reqs + - composer install --no-interaction --prefer-source - go get script: - go test -race -v -coverprofile=grpc.txt -covermode=atomic - go test ./parser -race -v -coverprofile=parser.txt -covermode=atomic - go test -race -v -coverprofile=protoc-gen-php-grpc.txt ./cmd/protoc-gen-php-grpc -covermode=atomic - - composer require phpunit/phpunit:~7.0 --ignore-platform-reqs - vendor/bin/spiral-cs check src tests/GRPC - vendor/bin/phpunit --coverage-clover=coverage.xml - go build cmd/protoc-gen-php-grpc/main.go @@ -62,3 +61,12 @@ jobs: - sudo cp `which php7.4` `which php` - php -v - composer self-update + - stage: Test + env: "PHP=8.0" + before_install: + - sudo add-apt-repository -y ppa:ondrej/php + - sudo apt-get update + - sudo apt-get install -y php8.0-cli php8.0-xml php8.0-xdebug + - sudo cp `which php8.0` `which php` + - php -v + - composer self-update diff --git a/CHANGELOG.md b/CHANGELOG.md index 87f720f..0a3e96e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ CHANGELOG ========= +v1.4.1 (13.10.2020) +------------------- +- RoadRunner version update to 1.8.3 +- Golang version in go.mod bump to 1.15 +- Add server configuration options (debug) (@aldump) + v1.4.0 (1.08.2020) ------------------- - Add all major gRPC configuration options. [Docs](https://github.com/spiral/docs/blob/master/grpc/configuration.md#application-server) diff --git a/build.sh b/build.sh index fa12e35..f7bed77 100755 --- a/build.sh +++ b/build.sh @@ -2,7 +2,7 @@ cd $(dirname "${BASH_SOURCE[0]}") OD="$(pwd)" # Pushes application version into the build information. -RR_VERSION=1.4.0 +RR_VERSION=1.4.1 # Hardcode some values to the core package LDFLAGS="$LDFLAGS -X github.com/spiral/roadrunner/cmd/rr/cmd.Version=${RR_VERSION}" diff --git a/cmd/rr-grpc/grpc/metrics.go b/cmd/rr-grpc/grpc/metrics.go index 51de6cb..a5510dd 100644 --- a/cmd/rr-grpc/grpc/metrics.go +++ b/cmd/rr-grpc/grpc/metrics.go @@ -73,14 +73,14 @@ func newCollector() *metricCollector { Name: "rr_grpc_call_total", Help: "Total number of handled grpc requests after server restart.", }, - []string{"code"}, + []string{"code", "method"}, ), callDuration: prometheus.NewHistogramVec( prometheus.HistogramOpts{ Name: "rr_grpc_call_duration_seconds", Help: "GRPC call duration.", }, - []string{"code"}, + []string{"code", "method"}, ), workersMemory: prometheus.NewGauge( prometheus.GaugeOpts{ @@ -103,8 +103,12 @@ func (c *metricCollector) listener(event int, ctx interface{}) { code = st.Code().String() } - c.callDuration.With(prometheus.Labels{"code": code}).Observe(uc.Elapsed().Seconds()) - c.callCounter.With(prometheus.Labels{"code": code}).Inc() + method := "Unknown" + if uc.Info != nil { + method = uc.Info.FullMethod + } + c.callDuration.With(prometheus.Labels{"code": code, "method": method}).Observe(uc.Elapsed().Seconds()) + c.callCounter.With(prometheus.Labels{"code": code, "method": method}).Inc() } } diff --git a/composer.json b/composer.json index 0da593d..cb350cd 100644 --- a/composer.json +++ b/composer.json @@ -1,33 +1,55 @@ { - "name": "spiral/php-grpc", - "type": "server", - "description": "High-Performance GRPC server for PHP applications", - "license": "MIT", - "authors": [ - { - "name": "Anton Titov / Wolfy-J", - "email": "wolfy.jd@gmail.com" - } - ], - "require": { - "php": "^7.2", - "ext-json": "*", - "google/protobuf": "^3.7", - "spiral/roadrunner": "^1.8" - }, - "require-dev": { - "phpunit/phpunit": "~7.0", - "spiral/code-style": "^1.0" - }, - "autoload": { - "psr-4": { - "Spiral\\GRPC\\": "src/" - } - }, - "autoload-dev": { - "psr-4": { - "": "tests/src/", - "Spiral\\GRPC\\Tests\\": "tests/GRPC/" - } - } + "name": "spiral/php-grpc", + "type": "library", + "description": "High-Performance GRPC server for PHP applications", + "license": "MIT", + "authors": [ + { + "name": "Anton Titov / Wolfy-J", + "email": "wolfy.jd@gmail.com" + } + ], + "require": { + "php": ">=7.2", + "ext-json": "*", + "symfony/polyfill-php80": "^1.22", + "symfony/polyfill-php73": "^1.22", + "google/protobuf": "^3.7", + "spiral/roadrunner": "^1.8" + }, + "require-dev": { + "phpunit/phpunit": "^8.5|^9.0", + "spiral/code-style": "^1.0", + "jetbrains/phpstorm-attributes": "^1.0", + "symfony/var-dumper": ">=4.4", + "vimeo/psalm": "^4.6" + }, + "autoload": { + "psr-4": { + "Spiral\\GRPC\\": "src" + } + }, + "autoload-dev": { + "psr-4": { + "": "tests/src", + "Spiral\\GRPC\\Tests\\": "tests/GRPC" + } + }, + "scripts": { + "test": [ + "spiral-cs check src tests/GRPC", + "psalm --no-cache", + "phpunit" + ] + }, + "extra": { + "branch-alias": { + "dev-master": "2.0.x-dev" + } + }, + "config": { + "sort-packages": true + }, + "minimum-stability": "dev", + "prefer-stable": true } diff --git a/example/server/composer.json b/example/server/composer.json index 71825a7..b630329 100644 --- a/example/server/composer.json +++ b/example/server/composer.json @@ -1,10 +1,23 @@ { - "require": { - "spiral/php-grpc": "^1.0" - }, - "autoload": { - "psr-4": { - "": "src/" - } - } + "name": "app/example-grpc-server", + "description": "Example GRPC Server", + "repositories": [ + { + "type": "path", + "url": "../.." + } + ], + "require": { + "spiral/php-grpc": "*" + }, + "require-dev": { + "grpc/grpc": "^1.36" + }, + "autoload": { + "psr-4": { + "": "src" + } + }, + "minimum-stability": "dev", + "prefer-stable": true } diff --git a/example/server/worker.php b/example/server/worker.php index 9a13fcd..683a234 100644 --- a/example/server/worker.php +++ b/example/server/worker.php @@ -3,14 +3,24 @@ * Sample GRPC PHP server. */ -use Spiral\Goridge; -use Spiral\RoadRunner; +use Service\EchoInterface; +use Spiral\Goridge\StreamRelay; +use Spiral\GRPC\Server; +use Spiral\RoadRunner\Worker; -ini_set('display_errors', 'stderr'); -require "vendor/autoload.php"; +require __DIR__ . '/vendor/autoload.php'; -$server = new \Spiral\GRPC\Server(); -$server->registerService(\Service\EchoInterface::class, new EchoService()); +$server = new Server(null, [ + 'debug' => false, // optional (default: false) +]); -$w = new RoadRunner\Worker(new Goridge\StreamRelay(STDIN, STDOUT)); -$server->serve($w); \ No newline at end of file +$server->registerService(EchoInterface::class, new EchoService()); + +$worker = \method_exists(Worker::class, 'create') + // RoadRunner >= 2.x + ? Worker::create() + // RoadRunner 1.x + : new Worker(new StreamRelay(STDIN, STDOUT)) +; + +$server->serve($worker); diff --git a/go.mod b/go.mod index 657ba32..55cd06a 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/spiral/php-grpc -go 1.13 +go 1.15 require ( github.com/buger/goterm v0.0.0-20200322175922-2f3e71b85129 @@ -10,7 +10,7 @@ require ( github.com/prometheus/client_golang v1.6.0 github.com/sirupsen/logrus v1.6.0 github.com/spf13/cobra v1.0.0 - github.com/spiral/roadrunner v1.8.2 + github.com/spiral/roadrunner v1.8.3 github.com/stretchr/testify v1.5.1 golang.org/x/net v0.0.0-20200520182314-0ba52f642ac2 google.golang.org/grpc v1.29.1 diff --git a/phpunit.xml b/phpunit.xml index 563ac8f..384daa4 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -10,17 +10,19 @@ processIsolation="false" stopOnFailure="false" stopOnError="false" -> - + stderr="true"> - - ./tests/ + + tests - src - \ No newline at end of file + + + + + diff --git a/psalm.xml b/psalm.xml new file mode 100644 index 0000000..9cb3de6 --- /dev/null +++ b/psalm.xml @@ -0,0 +1,27 @@ + + + + + + + + + + + + + + diff --git a/src/Context.php b/src/Context.php index ec224bf..3c9dab7 100644 --- a/src/Context.php +++ b/src/Context.php @@ -1,23 +1,29 @@ + * @template-implements \ArrayAccess + */ +final class Context implements ContextInterface, \IteratorAggregate, \Countable, \ArrayAccess { - /** @var array */ + /** + * @var array + */ private $values; /** - * @param array $values + * @param array $values */ public function __construct(array $values) { @@ -25,7 +31,7 @@ public function __construct(array $values) } /** - * @inheritdoc + * {@inheritDoc} */ public function withValue(string $key, $value): ContextInterface { @@ -36,18 +42,82 @@ public function withValue(string $key, $value): ContextInterface } /** - * @inheritdoc + * {@inheritDoc} + * @param mixed|null $default */ - public function getValue(string $key) + public function getValue(string $key, $default = null) { - return $this->values[$key] ?? null; + return $this->values[$key] ?? $default; } /** - * @inheritdoc + * {@inheritDoc} */ public function getValues(): array { return $this->values; } + + /** + * {@inheritDoc} + */ + public function offsetExists($offset): bool + { + assert(\is_string($offset), 'Offset argument must be a type of string'); + + /** + * Note: PHP Opcode optimisation + * @see https://www.php.net/manual/pt_BR/internals2.opcodes.isset-isempty-var.php + * + * Priority use `ZEND_ISSET_ISEMPTY_VAR !0` opcode instead of `DO_FCALL 'array_key_exists'`. + */ + return isset($this->values[$offset]) || \array_key_exists($offset, $this->values); + } + + /** + * {@inheritDoc} + */ + public function offsetGet($offset) + { + assert(\is_string($offset), 'Offset argument must be a type of string'); + + return $this->values[$offset] ?? null; + } + + /** + * {@inheritDoc} + */ + public function offsetSet($offset, $value): void + { + assert(\is_string($offset), 'Offset argument must be a type of string'); + + $this->values[$offset] = $value; + } + + /** + * {@inheritDoc} + */ + public function offsetUnset($offset): void + { + assert(\is_string($offset), 'Offset argument must be a type of string'); + + unset($this->values[$offset]); + } + + + /** + * {@inheritDoc} + */ + public function getIterator(): \Traversable + { + return new \ArrayIterator($this->values); + } + + /** + * {@inheritDoc} + */ + public function count(): int + { + return \count($this->values); + } } diff --git a/src/ContextInterface.php b/src/ContextInterface.php index 9fa7558..2d53297 100644 --- a/src/ContextInterface.php +++ b/src/ContextInterface.php @@ -1,10 +1,10 @@ */ public function getValues(): array; } diff --git a/src/Exception/GRPCException.php b/src/Exception/GRPCException.php index 23ef36e..b146129 100644 --- a/src/Exception/GRPCException.php +++ b/src/Exception/GRPCException.php @@ -1,10 +1,10 @@ */ private $details; /** - * @param string $message - * @param int $code - * @param array $details - * @param Throwable|null $previous + * @param string $message + * @param StatusCodeType|null $code + * @param array $details + * @param \Throwable|null $previous */ - public function __construct(string $message = '', int $code = 0, $details = [], Throwable $previous = null) - { - if ($code == 0) { - $code = static::CODE; - } - - parent::__construct($message, $code, $previous); + final public function __construct( + string $message = '', + #[ExpectedValues(valuesFromClass: StatusCode::class)] + int $code = null, + array $details = [], + \Throwable $previous = null + ) { + parent::__construct($message, (int)($code ?? static::CODE), $previous); $this->details = $details; } /** - * @return array + * @param string $message + * @param StatusCodeType|null $code + * @param array $details + * @param \Throwable|null $previous + * @return static + */ + public static function create( + string $message, + #[ExpectedValues(valuesFromClass: StatusCode::class)] + int $code = null, + \Throwable $previous = null, + array $details = [] + ): self { + return new static($message, $code, $details, $previous); + } + + /** + * {@inheritDoc} */ public function getDetails(): array { @@ -55,22 +83,32 @@ public function getDetails(): array } /** - * @param array $details + * {@inheritDoc} */ public function setDetails(array $details): void { $this->details = $details; } + /** + * {@inheritDoc} + */ + public function addDetails(Message $message): void + { + $this->details[] = $message; + } + /** * Push details message to the exception. * - * @param $details - * + * @param Message $details * @return $this + * @deprecated Please use {@see GRPCException::addDetails()} method instead. */ - public function withDetails($details) - { + #[Deprecated('Please use GRPCException::addDetails() instead', '%class%::addDetails(%parameter0%)')] + public function withDetails( + $details + ): self { $this->details[] = $details; return $this; diff --git a/src/Exception/GRPCExceptionInterface.php b/src/Exception/GRPCExceptionInterface.php new file mode 100644 index 0000000..75c1894 --- /dev/null +++ b/src/Exception/GRPCExceptionInterface.php @@ -0,0 +1,37 @@ + + */ + public function getDetails(): array; +} diff --git a/src/Exception/InvokeException.php b/src/Exception/InvokeException.php index a19ba72..ee18751 100644 --- a/src/Exception/InvokeException.php +++ b/src/Exception/InvokeException.php @@ -1,10 +1,10 @@ $details + */ + public function setDetails(array $details): void; + + /** + * Appends details message to the GRPC Exception. + * + * @param Message $message + */ + public function addDetails(Message $message): void; +} diff --git a/src/Exception/NotFoundException.php b/src/Exception/NotFoundException.php index 384704a..8ce0a7c 100644 --- a/src/Exception/NotFoundException.php +++ b/src/Exception/NotFoundException.php @@ -1,10 +1,10 @@ getName()], - [&$context,$this->makeInput($method, $input)] - ); + private const ERROR_METHOD_RETURN = + 'Method %s must return an object that instance of %s, ' . + 'but the result provides type of %s'; + + /** + * @var string + */ + private const ERROR_METHOD_IN_TYPE = + 'Method %s input type must be an instance of %s, ' . + 'but the input is type of %s' + ; + + /** + * {@inheritDoc} + */ + public function invoke(ServiceInterface $service, Method $method, ContextInterface $ctx, ?string $input): string + { + /** @var callable $callable */ + $callable = [$service, $method->getName()]; + + /** @var Message $message */ + $message = $callable($ctx, $this->makeInput($method, $input)); + + // Note: This validation will only work if the + // assertions option ("zend.assertions") is enabled. + assert($this->assertResultType($method, $message)); try { - return $out->serializeToString(); + return $message->serializeToString(); } catch (\Throwable $e) { - throw new InvokeException($e->getMessage(), StatusCode::INTERNAL, [], $e); + throw InvokeException::create($e->getMessage(), StatusCode::INTERNAL, $e); } } /** + * Checks that the result from the GRPC service method returns the + * Message object. + * * @param Method $method - * @param string $body + * @param mixed $result + * @return bool + * @throws \BadFunctionCallException + */ + private function assertResultType(Method $method, $result): bool + { + if (! $result instanceof Message) { + $type = \is_object($result) ? \get_class($result) : \get_debug_type($result); + + throw new \BadFunctionCallException( + \sprintf(self::ERROR_METHOD_RETURN, $method->getName(), Message::class, $type) + ); + } + + return true; + } + + /** + * @param Method $method + * @param string|null $body * @return Message - * * @throws InvokeException */ private function makeInput(Method $method, ?string $body): Message @@ -49,15 +87,40 @@ private function makeInput(Method $method, ?string $body): Message try { $class = $method->getInputType(); - /** @var Message $in */ + // Note: This validation will only work if the + // assertions option ("zend.assertions") is enabled. + assert($this->assertInputType($method, $class)); + + /** @psalm-suppress UnsafeInstantiation */ $in = new $class(); + if ($body !== null) { $in->mergeFromString($body); } return $in; } catch (\Throwable $e) { - throw new InvokeException($e->getMessage(), StatusCode::INTERNAL, [], $e); + throw InvokeException::create($e->getMessage(), StatusCode::INTERNAL, $e); } } + + /** + * Checks that the input of the GRPC service method contains the + * Message object. + * + * @param Method $method + * @param string $class + * @return bool + * @throws \InvalidArgumentException + */ + private function assertInputType(Method $method, string $class): bool + { + if (! \is_subclass_of($class, Message::class)) { + throw new \InvalidArgumentException( + \sprintf(self::ERROR_METHOD_IN_TYPE, $method->getName(), Message::class, $class) + ); + } + + return true; + } } diff --git a/src/InvokerInterface.php b/src/InvokerInterface.php index 8b10f97..80e2afe 100644 --- a/src/InvokerInterface.php +++ b/src/InvokerInterface.php @@ -1,10 +1,10 @@ + */ private $input; - /** @var string */ + /** + * @var class-string + */ private $output; + /** + * @param string $name + * @param class-string $input + * @param class-string $output + */ + private function __construct(string $name, string $input, string $output) + { + $this->name = $name; + $this->input = $input; + $this->output = $output; + } + /** * @return string */ @@ -38,7 +101,7 @@ public function getName(): string } /** - * @return string + * @return class-string */ public function getInputType(): string { @@ -46,13 +109,28 @@ public function getInputType(): string } /** - * @return string + * @return class-string */ public function getOutputType(): string { return $this->output; } + /** + * @param \ReflectionType|null $type + * @return \ReflectionClass|null + * @throws \ReflectionException + */ + private static function getReflectionClassByType(?\ReflectionType $type): ?\ReflectionClass + { + if ($type instanceof \ReflectionNamedType && ! $type->isBuiltin()) { + /** @psalm-suppress ArgumentTypeCoercion */ + return new \ReflectionClass($type->getName()); + } + + return null; + } + /** * Returns true if method signature matches. * @@ -61,58 +139,188 @@ public function getOutputType(): string */ public static function match(\ReflectionMethod $method): bool { - if ($method->getNumberOfParameters() != 2) { + try { + self::assertMethodSignature($method); + } catch (\Throwable $e) { return false; } - $ctx = $method->getParameters()[0]->getClass(); - $in = $method->getParameters()[1]->getClass(); + return true; + } - if (empty($ctx) || !$ctx->implementsInterface(ContextInterface::class)) { - return false; + /** + * @param \ReflectionMethod $method + * @param \ReflectionParameter $context + * @throws \ReflectionException + */ + private static function assertContextParameter(\ReflectionMethod $method, \ReflectionParameter $context): void + { + $type = $context->getType(); + + // When the type is not specified, it means that it is declared as + // a "mixed" type, which is a valid case + if ($type !== null) { + if (! $type instanceof \ReflectionNamedType) { + $message = \sprintf(self::ERROR_PARAM_UNION_TYPE, $context->getName(), $method->getName()); + throw new \DomainException($message, 0x02); + } + + // If the type is not declared as a generic "mixed" or "object", + // then it can only be a type that implements ContextInterface. + if (! \in_array($type->getName(), ['mixed', 'object'], true)) { + /** @psalm-suppress ArgumentTypeCoercion */ + $isContextImplementedType = ! $type->isBuiltin() + && (new \ReflectionClass($type->getName())) + ->implementsInterface(ContextInterface::class) + ; + + // Checking that the signature can accept the context. + // + // TODO If the type is any other implementation of the Spiral\GRPC\ContextInterface other than + // class Spiral\GRPC\Context, it may cause an error. + // It might make sense to check for such cases? + if (! $isContextImplementedType) { + $message = \vsprintf(self::ERROR_PARAM_CONTEXT_TYPE, [ + $context->getName(), + $method->getName(), + ContextInterface::class + ]); + + throw new \DomainException($message, 0x03); + } + } } + } - if (empty($in) || !$in->isSubclassOf(Message::class)) { - return false; + /** + * @param \ReflectionMethod $method + * @param \ReflectionParameter $input + * @throws \ReflectionException + */ + private static function assertInputParameter(\ReflectionMethod $method, \ReflectionParameter $input): void + { + $type = $input->getType(); + + // Parameter type cannot be omitted ("mixed") + if ($type === null) { + $message = \vsprintf(self::ERROR_PARAM_INPUT_TYPE, [ + $input->getName(), + $method->getName(), + Message::class, + 'mixed' + ]); + + throw new \DomainException($message, 0x04); } - if (empty($method->getReturnType())) { - return false; + // Parameter type cannot be declared as singular non-named type + if (! $type instanceof \ReflectionNamedType) { + $message = \sprintf(self::ERROR_PARAM_UNION_TYPE, $input->getName(), $method->getName()); + throw new \DomainException($message, 0x05); } - try { - $return = $method->getReturnType()->getName(); - if (!class_exists($return)) { - return false; - } - $return = new \ReflectionClass($return); - } catch (\ReflectionException $e) { - return false; + /** @psalm-suppress ArgumentTypeCoercion */ + $isProtobufMessageType = ! $type->isBuiltin() + && (new \ReflectionClass($type->getName())) + ->isSubclassOf(Message::class) + ; + + if (! $isProtobufMessageType) { + $message = \vsprintf(self::ERROR_PARAM_INPUT_TYPE, [ + $input->getName(), + $method->getName(), + Message::class, + $type->getName(), + ]); + throw new \DomainException($message, 0x06); } + } - return $return->isSubclassOf(Message::class); + /** + * @param \ReflectionMethod $method + * @throws \ReflectionException + */ + private static function assertOutputReturnType(\ReflectionMethod $method): void + { + $type = $method->getReturnType(); + + // Return type cannot be omitted ("mixed") + if ($type === null) { + $message = \sprintf(self::ERROR_RETURN_TYPE, $method->getName(), Message::class, 'mixed'); + throw new \DomainException($message, 0x07); + } + + // Return type cannot be declared as singular non-named type + if (! $type instanceof \ReflectionNamedType) { + $message = \sprintf(self::ERROR_RETURN_UNION_TYPE, $method->getName()); + throw new \DomainException($message, 0x08); + } + + /** @psalm-suppress ArgumentTypeCoercion */ + $isProtobufMessageType = ! $type->isBuiltin() + && (new \ReflectionClass($type->getName())) + ->isSubclassOf(Message::class) + ; + + if (! $isProtobufMessageType) { + $message = \sprintf(self::ERROR_RETURN_TYPE, $method->getName(), Message::class, $type->getName()); + throw new \DomainException($message, 0x09); + } } /** - * Returns true if method signature matches. + * @param \ReflectionMethod $method + * @throws \ReflectionException + * @throws \DomainException + */ + private static function assertMethodSignature(\ReflectionMethod $method): void + { + // Check that there are only two parameters + if ($method->getNumberOfParameters() !== 2) { + $message = \sprintf(self::ERROR_PARAMS_COUNT, $method->getName(), $method->getNumberOfParameters()); + throw new \DomainException($message, 0x01); + } + + /** + * @var \ReflectionParameter $context + * @var \ReflectionParameter $input + */ + [$context, $input] = $method->getParameters(); + + // The first parameter can only take a context object + self::assertContextParameter($method, $context); + + // The second argument can only be a subtype of the Google\Protobuf\Internal\Message class + self::assertInputParameter($method, $input); + + // The return type must be declared as a Google\Protobuf\Internal\Message class + self::assertOutputReturnType($method); + } + + /** + * Creates a new {@see Method} object from a {@see \ReflectionMethod} object. * * @param \ReflectionMethod $method * @return Method */ public static function parse(\ReflectionMethod $method): Method { - if (!self::match($method)) { - throw new GRPCException( - "Method `{$method->getName()}` is not valid GRPC method.", - StatusCode::INTERNAL - ); + try { + self::assertMethodSignature($method); + } catch (\Throwable $e) { + $message = \sprintf(self::ERROR_INVALID_GRPC_METHOD, $method->getName()); + throw GRPCException::create($message, StatusCode::INTERNAL, $e); } - $m = new self(); - $m->name = $method->getName(); - $m->input = $method->getParameters()[1]->getClass()->getName(); - $m->output = $method->getReturnType()->getName(); + [,$input] = $method->getParameters(); + + /** @var \ReflectionNamedType $inputType */ + $inputType = $input->getType(); + + /** @var \ReflectionNamedType $returnType */ + $returnType = $method->getReturnType(); - return $m; + /** @psalm-suppress ArgumentTypeCoercion */ + return new self($method->getName(), $inputType->getName(), $returnType->getName()); } } diff --git a/src/ResponseHeaders.php b/src/ResponseHeaders.php index 71d6455..c1ee345 100644 --- a/src/ResponseHeaders.php +++ b/src/ResponseHeaders.php @@ -1,27 +1,36 @@ + */ +final class ResponseHeaders implements \IteratorAggregate, \Countable { - /** @var array */ - private $headers; + /** + * @var array + */ + private $headers = []; /** - * @param array $headers + * @param iterable $headers */ - public function __construct(array $headers = []) + public function __construct(iterable $headers = []) { - $this->headers = $headers; + foreach ($headers as $key => $value) { + $this->set($key, $value); + } } /** @@ -35,8 +44,8 @@ public function set(string $key, string $value): void /** * @param string $key - * @param string $default $default - * @return string $default|null + * @param string|null $default + * @return string|null */ public function get(string $key, string $default = null): ?string { @@ -44,22 +53,33 @@ public function get(string $key, string $default = null): ?string } /** - * @return array + * {@inheritDoc} + */ + public function getIterator(): \Traversable + { + return new \ArrayIterator($this->headers); + } + + /** + * @return int */ - public function getIterator(): array + public function count(): int { - return $this->headers; + return \count($this->headers); } /** * @return string + * @throws \JsonException */ public function packHeaders(): string { + // If an empty array is serialized, it is cast to the string "[]" + // instead of object string "{}" if ($this->headers === []) { return '{}'; } - return json_encode($this->headers); + return Json::encode($this->headers); } } diff --git a/src/Server.php b/src/Server.php index d56ba50..90886d3 100644 --- a/src/Server.php +++ b/src/Server.php @@ -1,10 +1,10 @@ > + * } */ final class Server { - /** @var InvokerInterface */ + /** + * @var InvokerInterface + */ private $invoker; - /** @var ServiceWrapper[] */ + /** + * @var ServiceWrapper[] + */ private $services = []; + /** + * @var ServerOptions + */ + private $options; + /** * @param InvokerInterface|null $invoker + * @param ServerOptions $options */ - public function __construct(InvokerInterface $invoker = null) + public function __construct(InvokerInterface $invoker = null, array $options = []) { $this->invoker = $invoker ?? new Invoker(); + $this->options = $options; } /** * Register new GRPC service. * - * Example: $server->registerService(EchoServiceInterface::class, new EchoService()); + * For example: + * + * $server->registerService(EchoServiceInterface::class, new EchoService()); + * * - * @param string $interface Generated service interface. - * @param ServiceInterface $service Must implement interface. + * @template T of ServiceInterface * + * @param class-string $interface Generated service interface. + * @param T $service Must implement interface. * @throws ServiceException */ public function registerService(string $interface, ServiceInterface $service): void { $service = new ServiceWrapper($this->invoker, $interface, $service); + $this->services[$service->getName()] = $service; } + /** + * @param string $body + * @param ContextResponse $data + * @return array{ 0: string, 1: string } + * @throws \JsonException + * @throws \Throwable + */ + private function tick(string $body, array $data): array + { + $context = (new Context($data['context'])) + ->withValue(ResponseHeaders::class, new ResponseHeaders()) + ; + + $response = $this->invoke($data['service'], $data['method'], $context, $body); + + /** @var ResponseHeaders|null $responseHeaders */ + $responseHeaders = $context->getValue(ResponseHeaders::class); + $responseHeadersString = $responseHeaders ? $responseHeaders->packHeaders() : '{}'; + + return [$response, $responseHeadersString]; + } + + /** + * @param Worker $worker + * @param string $body + * @param string $headers + * @psalm-suppress InaccessibleMethod + */ + private function workerSend(Worker $worker, string $body, string $headers): void + { + // RoadRunner 1.x + if (\method_exists($worker, 'send')) { + $worker->send($body, $headers); + + return; + } + + // RoadRunner 2.x + $worker->respond(new Payload($body, $headers)); + } + + /** + * @param Worker $worker + * @param string $message + */ + private function workerError(Worker $worker, string $message): void + { + $worker->error($message); + } + + /** + * @param Worker $worker + * @return array { 0: string, 1: string } | null + * + * @psalm-suppress UndefinedMethod + * @psalm-suppress PossiblyUndefinedVariable + */ + private function workerReceive(Worker $worker): ?array + { + /** @var string|\Stringable $body */ + $body = $worker->receive($ctx); + + if (empty($body) && empty($ctx)) { + return null; + } + + return [(string)$body, (string)$ctx]; + } + /** * Serve GRPC over given RoadRunner worker. * - * @param Worker $worker + * @param Worker $worker * @param callable|null $finalize */ public function serve(Worker $worker, callable $finalize = null): void { while (true) { - $body = $worker->receive($ctx); - if (empty($body) && empty($ctx)) { + $request = $this->workerReceive($worker); + + if (! $request) { return; } + [$body, $headers] = $request; + try { - $ctx = json_decode($ctx, true); - $grpcCtx = new Context( - $ctx['context'] + [ResponseHeaders::class => new ResponseHeaders([])] - ); - - $resp = $this->invoke( - $ctx['service'], - $ctx['method'], - $grpcCtx, - $body - ); - - /** @var ResponseHeaders|null $responseHeaders */ - $responseHeaders = $grpcCtx->getValue(ResponseHeaders::class); - $worker->send($resp, $responseHeaders ? $responseHeaders->packHeaders() : '{}'); - } catch (GRPCException $e) { - $worker->error($this->packError($e)); + /** @var ContextResponse $context */ + $context = Json::decode((string)$headers); + + [$answerBody, $answerHeaders] = $this->tick((string)$body, $context); + + $this->workerSend($worker, $answerBody, $answerHeaders); + } catch (GRPCExceptionInterface $e) { + $this->workerError($worker, $this->packError($e)); } catch (\Throwable $e) { - $worker->error((string)$e); + $this->workerError($worker, $this->isDebugMode() ? (string)$e : $e->getMessage()); } finally { if ($finalize !== null) { - call_user_func($finalize, $e ?? null); + isset($e) ? $finalize($e) : $finalize(); } } } @@ -98,23 +189,17 @@ public function serve(Worker $worker, callable $finalize = null): void /** * Invoke service method with binary payload and return the response. * - * @param string $service - * @param string $method - * @param Context $context - * @param string $body + * @param string $service + * @param string $method + * @param ContextInterface $context + * @param string $body * @return string - * * @throws GRPCException - * @throws \Throwable */ - protected function invoke( - string $service, - string $method, - Context $context, - ?string $body - ): string { - if (!isset($this->services[$service])) { - throw new NotFoundException("Service `{$service}` not found.", StatusCode::NOT_FOUND); + protected function invoke(string $service, string $method, ContextInterface $context, string $body): string + { + if (! isset($this->services[$service])) { + throw NotFoundException::create("Service `{$service}` not found.", StatusCode::NOT_FOUND); } return $this->services[$service]->invoke($method, $context, $body); @@ -125,20 +210,17 @@ protected function invoke( * * Internal agreement: * - * Details will be sent as serialized google.protobuf.Any messages after code and exception message - * separated with |:| delimeter. + * Details will be sent as serialized google.protobuf.Any messages after + * code and exception message separated with |:| delimiter. * - * @param GRPCException $e + * @param GRPCExceptionInterface $e * @return string */ - private function packError(GRPCException $e): string + private function packError(GRPCExceptionInterface $e): string { $data = [$e->getCode(), $e->getMessage()]; foreach ($e->getDetails() as $detail) { - /** - * @var Message $detail - */ $anyMessage = new Any(); $anyMessage->pack($detail); @@ -146,6 +228,22 @@ private function packError(GRPCException $e): string $data[] = $anyMessage->serializeToString(); } - return implode('|:|', $data); + return \implode('|:|', $data); + } + + /** + * If server runs in debug mode + * + * @return bool + */ + private function isDebugMode(): bool + { + $debug = false; + + if (isset($this->options['debug'])) { + $debug = \filter_var($this->options['debug'], \FILTER_VALIDATE_BOOLEAN); + } + + return $debug; } } diff --git a/src/ServiceInterface.php b/src/ServiceInterface.php index 074aefe..991ae1f 100644 --- a/src/ServiceInterface.php +++ b/src/ServiceInterface.php @@ -1,10 +1,10 @@ invoker = $invoker; + $this->configure($interface, $service); } @@ -73,53 +70,54 @@ public function getMethods(): array } /** - * @param string $method + * @param string $method * @param ContextInterface $context - * @param string $input - * @param array $metadata + * @param string|null $input * @return string - * * @throws NotFoundException * @throws InvokeException */ - public function invoke(string $method, ContextInterface $context, ?string $input, array &$metadata = []): string + public function invoke(string $method, ContextInterface $context, ?string $input): string { - if (!isset($this->methods[$method])) { - throw new NotFoundException("Method `{$method}` not found in service `{$this->name}`."); + if (! isset($this->methods[$method])) { + throw NotFoundException::create("Method `{$method}` not found in service `{$this->name}`."); } - return $this->invoker->invoke($this->service, $this->methods[$method], $context, $input, $metadata); + return $this->invoker->invoke($this->service, $this->methods[$method], $context, $input); } /** * Configure service name and methods. * - * @param string $interface + * @param class-string $interface * @param ServiceInterface $service - * * @throws ServiceException */ protected function configure(string $interface, ServiceInterface $service): void { try { - $r = new \ReflectionClass($interface); - if (!$r->hasConstant('NAME')) { - throw new ServiceException( - "Invalid service interface `{$interface}`, constant `NAME` not found." - ); + $reflection = new \ReflectionClass($interface); + + if (! $reflection->hasConstant('NAME')) { + $message = "Invalid service interface `{$interface}`, constant `NAME` not found."; + throw ServiceException::create($message); } - $this->name = $r->getConstant('NAME'); + + $name = $reflection->getConstant('NAME'); + + if (! \is_string($name)) { + $message = "Constant `NAME` of service interface `{$interface}` must be a type of string"; + throw ServiceException::create($message); + } + + $this->name = $name; } catch (\ReflectionException $e) { - throw new ServiceException( - "Invalid service interface `{$interface}`.", - StatusCode::INTERNAL, - [], - $e - ); + $message = "Invalid service interface `{$interface}`."; + throw ServiceException::create($message, StatusCode::INTERNAL, $e); } - if (!$service instanceof $interface) { - throw new ServiceException("Service handler does not implement `{$interface}`."); + if (! $service instanceof $interface) { + throw ServiceException::create("Service handler does not implement `{$interface}`."); } $this->service = $service; @@ -130,7 +128,7 @@ protected function configure(string $interface, ServiceInterface $service): void /** * @param ServiceInterface $service - * @return array + * @return array */ protected function fetchMethods(ServiceInterface $service): array { diff --git a/src/StatusCode.php b/src/StatusCode.php index 7c0f7fd..5ff9c30 100644 --- a/src/StatusCode.php +++ b/src/StatusCode.php @@ -1,10 +1,10 @@ invoke( - $s, - $m, - new Context([]), - $this->packMessage('hello') - ); + $out = $i->invoke($s, $m, new Context([]), $this->packMessage('hello')); $m = new Message(); $m->mergeFromString($out); @@ -40,22 +35,16 @@ public function testInvoke(): void $this->assertSame('hello', $m->getMsg()); } - /** - * @expectedException \Spiral\GRPC\Exception\InvokeException - */ public function testInvokeError(): void { + $this->expectException(\Spiral\GRPC\Exception\InvokeException::class); + $s = new TestService(); $m = Method::parse(new \ReflectionMethod($s, 'Echo')); $i = new Invoker(); - $i->invoke( - $s, - $m, - new Context([]), - 'invalid-message' - ); + $i->invoke($s, $m, new Context([]), 'invalid-message'); } private function packMessage(string $message): string diff --git a/tests/GRPC/MethodTest.php b/tests/GRPC/MethodTest.php index 6a782d4..7491e84 100644 --- a/tests/GRPC/MethodTest.php +++ b/tests/GRPC/MethodTest.php @@ -19,11 +19,10 @@ class MethodTest extends TestCase { - /** - * @expectedException \Spiral\GRPC\Exception\GRPCException - */ public function testInvalidParse(): void { + $this->expectException(\Spiral\GRPC\Exception\GRPCException::class); + Method::parse(new \ReflectionMethod($this, 'testInvalidParse')); } diff --git a/tests/GRPC/ServerTest.php b/tests/GRPC/ServerTest.php index 8409269..205399e 100644 --- a/tests/GRPC/ServerTest.php +++ b/tests/GRPC/ServerTest.php @@ -85,6 +85,28 @@ public function testNotFound2(): void $this->assertTrue($w->done()); } + public function testServerDebugModeNotEnabled(): void + { + $s = new Server(); + $s->registerService(TestInterface::class, new TestService()); + + $w = new TestWorker($this, [ + [ + 'ctx' => [ + 'service' => 'service.Test', + 'method' => 'Throw', + 'context' => [], + ], + 'send' => $this->packMessage('regularException'), + 'error' => 'Just another exception' + ] + ]); + + $s->serve($w); + + $this->assertTrue($w->done()); + } + private function packMessage(string $message): string { $m = new Message(); diff --git a/tests/GRPC/ServiceWrapperTest.php b/tests/GRPC/ServiceWrapperTest.php index 5032a51..bae551b 100644 --- a/tests/GRPC/ServiceWrapperTest.php +++ b/tests/GRPC/ServiceWrapperTest.php @@ -55,11 +55,10 @@ public function testMethods(): void $this->assertCount(5, $w->getMethods()); } - /** - * @expectedException \Spiral\GRPC\Exception\NotFoundException - */ public function testInvokeNotFound(): void { + $this->expectException(\Spiral\GRPC\Exception\NotFoundException::class); + $w = new ServiceWrapper( new Invoker(), TestInterface::class, @@ -85,11 +84,10 @@ public function testInvoke(): void $this->assertSame('hello world', $m->getMsg()); } - /** - * @expectedException \Spiral\GRPC\Exception\ServiceException - */ public function testNotImplemented(): void { + $this->expectException(\Spiral\GRPC\Exception\ServiceException::class); + $w = new ServiceWrapper( new Invoker(), TestInterface::class, @@ -97,11 +95,10 @@ public function testNotImplemented(): void ); } - /** - * @expectedException \Spiral\GRPC\Exception\ServiceException - */ public function testInvalidInterface(): void { + $this->expectException(\Spiral\GRPC\Exception\ServiceException::class); + $w = new ServiceWrapper( new Invoker(), InvalidInterface::class, @@ -109,11 +106,10 @@ public function testInvalidInterface(): void ); } - /** - * @expectedException \Spiral\GRPC\Exception\ServiceException - */ public function testInvalidInterface2(): void { + $this->expectException(\Spiral\GRPC\Exception\ServiceException::class); + $w = new ServiceWrapper( new Invoker(), 'NotFound', diff --git a/tests/src/Test/TestService.php b/tests/src/Test/TestService.php index 6d4a385..12b9479 100644 --- a/tests/src/Test/TestService.php +++ b/tests/src/Test/TestService.php @@ -33,6 +33,9 @@ public function Throw(ContextInterface $ctx, Message $in): Message $grpcException = new GRPCException("main exception message", 3, [$detailsMessage]); throw $grpcException; + case "regularException": { + throw new \Exception("Just another exception"); + } } return $out; diff --git a/tests/worker.php b/tests/worker.php index e8210bb..7a80416 100644 --- a/tests/worker.php +++ b/tests/worker.php @@ -7,10 +7,10 @@ use Spiral\RoadRunner; ini_set('display_errors', 'stderr'); -require "vendor/autoload.php"; +require __DIR__ . '/../vendor/autoload.php'; $server = new \Spiral\GRPC\Server(); $server->registerService(\Service\TestInterface::class, new \Test\TestService()); $w = new RoadRunner\Worker(new Goridge\StreamRelay(STDIN, STDOUT)); -$server->serve($w); \ No newline at end of file +$server->serve($w);