From 3f0c4b573eb69e3dc743b7a1a8ba0f1390c5204d Mon Sep 17 00:00:00 2001 From: Valery Piashchynski Date: Fri, 2 Oct 2020 14:04:01 +0300 Subject: [PATCH 01/29] Create codeql-analysis.yml --- .github/workflows/codeql-analysis.yml | 71 +++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 .github/workflows/codeql-analysis.yml 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 From 7a3d77d132db20f1e432c429d86a9b5e1b9504b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D0=BB=D0=B5=D1=81=D1=8C=20=D0=95=D1=80=D0=B5=D0=BC?= =?UTF-8?q?=D0=B5=D0=B5=D0=B2?= Date: Tue, 13 Oct 2020 13:07:26 +0200 Subject: [PATCH 02/29] Server error details #50 [server configuration options, only 'debug' for now] --- example/server/worker.php | 1 + src/Server.php | 33 +++++++++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/example/server/worker.php b/example/server/worker.php index 9a13fcd..b089ed5 100644 --- a/example/server/worker.php +++ b/example/server/worker.php @@ -9,6 +9,7 @@ ini_set('display_errors', 'stderr'); require "vendor/autoload.php"; +//To run server in debug mode - new \Spiral\GRPC\Server(null, ['debug' => true]); $server = new \Spiral\GRPC\Server(); $server->registerService(\Service\EchoInterface::class, new EchoService()); diff --git a/src/Server.php b/src/Server.php index d56ba50..2f9fe7b 100644 --- a/src/Server.php +++ b/src/Server.php @@ -29,12 +29,19 @@ final class Server /** @var ServiceWrapper[] */ private $services = []; + /** + * @var array + */ + private $options; + /** * @param InvokerInterface|null $invoker + * @param array $options */ - public function __construct(InvokerInterface $invoker = null) + public function __construct(InvokerInterface $invoker = null, array $options = []) { $this->invoker = $invoker ?? new Invoker(); + $this->options = $options; } /** @@ -86,7 +93,13 @@ public function serve(Worker $worker, callable $finalize = null): void } catch (GRPCException $e) { $worker->error($this->packError($e)); } catch (\Throwable $e) { - $worker->error((string)$e); + if ($this->isDebugMode()) { + $errorText = $e->__toString(); + } else { + $errorText = $e->getMessage(); + } + + $worker->error($errorText); } finally { if ($finalize !== null) { call_user_func($finalize, $e ?? null); @@ -148,4 +161,20 @@ private function packError(GRPCException $e): string 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; + } } From 49c351c3e1e108dd6b940cbaf9d285dc4be4c579 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D0=BB=D0=B5=D1=81=D1=8C=20=D0=95=D1=80=D0=B5=D0=BC?= =?UTF-8?q?=D0=B5=D0=B5=D0=B2?= Date: Tue, 13 Oct 2020 13:38:11 +0200 Subject: [PATCH 03/29] Server error details #50 [add test] --- tests/GRPC/ServerTest.php | 22 ++++++++++++++++++++++ tests/src/Test/TestService.php | 3 +++ 2 files changed, 25 insertions(+) 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/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; From eaf06f911528a3626da589bf05bdfd50f284ac72 Mon Sep 17 00:00:00 2001 From: Valery Piashchynski Date: Tue, 13 Oct 2020 15:40:42 +0300 Subject: [PATCH 04/29] release 1.4.1 --- CHANGELOG.md | 6 ++++++ build.sh | 2 +- go.mod | 4 ++-- 3 files changed, 9 insertions(+), 3 deletions(-) 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/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 From 3918e66902eca87d7a419b6377d832ea0f3f5531 Mon Sep 17 00:00:00 2001 From: Kirill Nesmeyanov Date: Mon, 21 Dec 2020 14:34:41 +0300 Subject: [PATCH 05/29] Update composer.json --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 0da593d..8963b2e 100644 --- a/composer.json +++ b/composer.json @@ -10,7 +10,7 @@ } ], "require": { - "php": "^7.2", + "php": ">=7.2", "ext-json": "*", "google/protobuf": "^3.7", "spiral/roadrunner": "^1.8" From d6c0f997aa9296316f810797f0a463094bce474b Mon Sep 17 00:00:00 2001 From: lampatrampa <30873408+lampatrampa@users.noreply.github.com> Date: Tue, 9 Feb 2021 16:40:57 +0300 Subject: [PATCH 06/29] fix reflection deprecated php 8.0 methods --- src/Method.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Method.php b/src/Method.php index a5f9a4b..3696e91 100644 --- a/src/Method.php +++ b/src/Method.php @@ -64,9 +64,12 @@ public static function match(\ReflectionMethod $method): bool if ($method->getNumberOfParameters() != 2) { return false; } + + $ctxType = $method->getParameters()[0]->getType(); + $inType = $method->getParameters()[1]->getType(); - $ctx = $method->getParameters()[0]->getClass(); - $in = $method->getParameters()[1]->getClass(); + $ctx = $ctxType && ! $ctxType->isBuiltin() ? new \ReflectionClass($ctxType->getName()) : null; + $in = $inType && ! $inType->isBuiltin() ? new \ReflectionClass($inType->getName()) : null; if (empty($ctx) || !$ctx->implementsInterface(ContextInterface::class)) { return false; @@ -110,7 +113,7 @@ public static function parse(\ReflectionMethod $method): Method $m = new self(); $m->name = $method->getName(); - $m->input = $method->getParameters()[1]->getClass()->getName(); + $m->input = $method->getParameters()[1]->getType()->getName(); $m->output = $method->getReturnType()->getName(); return $m; From fe8bf7fa08ac1c2498c43746a0bd31a41dabab65 Mon Sep 17 00:00:00 2001 From: lampatrampa <30873408+lampatrampa@users.noreply.github.com> Date: Wed, 10 Feb 2021 09:25:48 +0300 Subject: [PATCH 07/29] remove whitespace --- src/Method.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Method.php b/src/Method.php index 3696e91..9546a7b 100644 --- a/src/Method.php +++ b/src/Method.php @@ -64,10 +64,8 @@ public static function match(\ReflectionMethod $method): bool if ($method->getNumberOfParameters() != 2) { return false; } - $ctxType = $method->getParameters()[0]->getType(); $inType = $method->getParameters()[1]->getType(); - $ctx = $ctxType && ! $ctxType->isBuiltin() ? new \ReflectionClass($ctxType->getName()) : null; $in = $inType && ! $inType->isBuiltin() ? new \ReflectionClass($inType->getName()) : null; From db2c50360018327dc0b39809ee30f1c8a070d331 Mon Sep 17 00:00:00 2001 From: lampatrampa <30873408+lampatrampa@users.noreply.github.com> Date: Wed, 10 Mar 2021 15:52:55 +0300 Subject: [PATCH 08/29] Checking types are instance of ReflectionNamedType --- src/Method.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Method.php b/src/Method.php index 9546a7b..df22f24 100644 --- a/src/Method.php +++ b/src/Method.php @@ -66,8 +66,8 @@ public static function match(\ReflectionMethod $method): bool } $ctxType = $method->getParameters()[0]->getType(); $inType = $method->getParameters()[1]->getType(); - $ctx = $ctxType && ! $ctxType->isBuiltin() ? new \ReflectionClass($ctxType->getName()) : null; - $in = $inType && ! $inType->isBuiltin() ? new \ReflectionClass($inType->getName()) : null; + $ctx = $ctxType && $ctxType instanceof \ReflectionNamedType && !$ctxType->isBuiltin() ? new \ReflectionClass($ctxType->getName()) : null; + $in = $inType && $inType instanceof \ReflectionNamedType && ! $inType->isBuiltin() ? new \ReflectionClass($inType->getName()) : null; if (empty($ctx) || !$ctx->implementsInterface(ContextInterface::class)) { return false; From 7ce5f931256a49eeb465132576f0689479b492b1 Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Thu, 25 Mar 2021 21:33:10 +0300 Subject: [PATCH 09/29] Code improvements --- .phpunit.result.cache | 1 + composer.json | 84 +++-- example/server/composer.json | 16 +- phpunit.xml | 16 +- psalm.xml | 26 ++ src/Context.php | 92 +++++- src/ContextInterface.php | 14 +- src/Exception/GRPCException.php | 88 ++++-- src/Exception/GRPCExceptionInterface.php | 36 +++ src/Exception/InvokeException.php | 9 +- .../MutableGRPCExceptionInterface.php | 31 ++ src/Exception/NotFoundException.php | 9 +- src/Exception/ServiceException.php | 9 +- src/Exception/UnauthenticatedException.php | 9 +- src/Exception/UnimplementedException.php | 9 +- src/Invoker.php | 101 ++++-- src/InvokerInterface.php | 20 +- src/Method.php | 287 +++++++++++++++--- src/ResponseHeaders.php | 70 ++++- src/Server.php | 10 +- src/ServiceInterface.php | 6 +- src/ServiceWrapper.php | 41 ++- src/StatusCode.php | 253 +++++++++------ tests/GRPC/InvokerTest.php | 19 +- tests/GRPC/MethodTest.php | 5 +- tests/GRPC/ServiceWrapperTest.php | 20 +- 26 files changed, 945 insertions(+), 336 deletions(-) create mode 100644 .phpunit.result.cache create mode 100644 psalm.xml create mode 100644 src/Exception/GRPCExceptionInterface.php create mode 100644 src/Exception/MutableGRPCExceptionInterface.php diff --git a/.phpunit.result.cache b/.phpunit.result.cache new file mode 100644 index 0000000..c23c558 --- /dev/null +++ b/.phpunit.result.cache @@ -0,0 +1 @@ +C:37:"PHPUnit\Runner\DefaultTestResultCache":2131:{a:2:{s:7:"defects";a:0:{}s:5:"times";a:35:{s:43:"Spiral\GRPC\Tests\ContextTest::testGetValue";d:0.004;s:47:"Spiral\GRPC\Tests\ContextTest::testGetNullValue";d:0;s:44:"Spiral\GRPC\Tests\ContextTest::testGetValues";d:0;s:44:"Spiral\GRPC\Tests\ContextTest::testWithValue";d:0;s:52:"Spiral\GRPC\Tests\ContextTest::testGetOutgoingHeader";d:0;s:53:"Spiral\GRPC\Tests\ContextTest::testGetOutgoingHeaders";d:0;s:44:"Spiral\GRPC\Tests\ExceptionTest::testDefault";d:0.001;s:45:"Spiral\GRPC\Tests\ExceptionTest::testNotFound";d:0;s:43:"Spiral\GRPC\Tests\ExceptionTest::testInvoke";d:0;s:52:"Spiral\GRPC\Tests\ExceptionTest::testUnauthenticated";d:0;s:50:"Spiral\GRPC\Tests\ExceptionTest::testUnimplemented";d:0;s:41:"Spiral\GRPC\Tests\InvokerTest::testInvoke";d:0.014;s:46:"Spiral\GRPC\Tests\InvokerTest::testInvokeError";d:0.001;s:46:"Spiral\GRPC\Tests\MethodTest::testInvalidParse";d:0;s:39:"Spiral\GRPC\Tests\MethodTest::testMatch";d:0;s:41:"Spiral\GRPC\Tests\MethodTest::testNoMatch";d:0;s:42:"Spiral\GRPC\Tests\MethodTest::testNoMatch2";d:0;s:42:"Spiral\GRPC\Tests\MethodTest::testNoMatch3";d:0;s:42:"Spiral\GRPC\Tests\MethodTest::testNoMatch4";d:0;s:42:"Spiral\GRPC\Tests\MethodTest::testNoMatch5";d:0;s:44:"Spiral\GRPC\Tests\MethodTest::testMethodName";d:0;s:49:"Spiral\GRPC\Tests\MethodTest::testMethodInputType";d:0;s:50:"Spiral\GRPC\Tests\MethodTest::testMethodOutputType";d:0;s:40:"Spiral\GRPC\Tests\ServerTest::testInvoke";d:0.003;s:42:"Spiral\GRPC\Tests\ServerTest::testNotFound";d:0;s:43:"Spiral\GRPC\Tests\ServerTest::testNotFound2";d:0;s:59:"Spiral\GRPC\Tests\ServerTest::testServerDebugModeNotEnabled";d:0;s:46:"Spiral\GRPC\Tests\ServiceWrapperTest::testName";d:0;s:49:"Spiral\GRPC\Tests\ServiceWrapperTest::testService";d:0;s:49:"Spiral\GRPC\Tests\ServiceWrapperTest::testMethods";d:0;s:56:"Spiral\GRPC\Tests\ServiceWrapperTest::testInvokeNotFound";d:0;s:48:"Spiral\GRPC\Tests\ServiceWrapperTest::testInvoke";d:0;s:56:"Spiral\GRPC\Tests\ServiceWrapperTest::testNotImplemented";d:0;s:58:"Spiral\GRPC\Tests\ServiceWrapperTest::testInvalidInterface";d:0;s:59:"Spiral\GRPC\Tests\ServiceWrapperTest::testInvalidInterface2";d:0;}}} \ No newline at end of file diff --git a/composer.json b/composer.json index 8963b2e..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..bdcd633 100644 --- a/example/server/composer.json +++ b/example/server/composer.json @@ -1,10 +1,12 @@ { - "require": { - "spiral/php-grpc": "^1.0" - }, - "autoload": { - "psr-4": { - "": "src/" + "name": "app/example-grpc-server", + "description": "Example GRPC Server", + "require": { + "spiral/php-grpc": "^1.0" + }, + "autoload": { + "psr-4": { + "": "src" + } } - } } diff --git a/phpunit.xml b/phpunit.xml index 563ac8f..fbd70dd 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,5 +1,5 @@ - - + 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..0218777 --- /dev/null +++ b/psalm.xml @@ -0,0 +1,26 @@ + + + + + + + + + + + + + diff --git a/src/Context.php b/src/Context.php index ec224bf..07d876f 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'); + + throw new \LogicException(__METHOD__ . ' not implemented yet'); + } + + /** + * {@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..df9cba8 --- /dev/null +++ b/src/Exception/GRPCExceptionInterface.php @@ -0,0 +1,36 @@ + + */ + 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()]; try { - return $out->serializeToString(); + /** @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)); + + 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 + * @psalm-suppress InvalidStringClass */ private function makeInput(Method $method, ?string $body): Message { try { $class = $method->getInputType(); + // Note: This validation will only work if the + // assertions option ("zend.assertions") is enabled. + assert($this->assertInputType($method, $class)); + /** @var Message $in */ $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,27 @@ 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()) { + return new \ReflectionClass($type->getName()); + } + + return null; + } + /** * Returns true if method signature matches. * @@ -61,59 +138,187 @@ public function getOutputType(): string */ public static function match(\ReflectionMethod $method): bool { - if ($method->getNumberOfParameters() != 2) { + try { + self::assertMethodSignature($method); + } catch (\Throwable $e) { return false; } - $ctxType = $method->getParameters()[0]->getType(); - $inType = $method->getParameters()[1]->getType(); - $ctx = $ctxType && $ctxType instanceof \ReflectionNamedType && !$ctxType->isBuiltin() ? new \ReflectionClass($ctxType->getName()) : null; - $in = $inType && $inType instanceof \ReflectionNamedType && ! $inType->isBuiltin() ? new \ReflectionClass($inType->getName()) : null; - if (empty($ctx) || !$ctx->implementsInterface(ContextInterface::class)) { - return false; + return true; + } + + /** + * @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]->getType()->getName(); - $m->output = $method->getReturnType()->getName(); + [,$input] = $method->getParameters(); + + /** @var \ReflectionNamedType $inputType */ + $inputType = $input->getType(); + + /** @var \ReflectionNamedType $returnType */ + $returnType = $method->getReturnType(); - return $m; + return new self($method->getName(), $inputType->getName(), $returnType->getName()); } } diff --git a/src/ResponseHeaders.php b/src/ResponseHeaders.php index 71d6455..a15cb94 100644 --- a/src/ResponseHeaders.php +++ b/src/ResponseHeaders.php @@ -1,27 +1,34 @@ + */ +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 +42,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 +51,55 @@ 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); + $flags = \defined('\\JSON_THROW_ON_ERROR') + // Avoid PHP DCE constant inlining + ? \constant('\\JSON_THROW_ON_ERROR') + : 0; + + return $this->toJsonString($this->headers, $flags); + } + + /** + * @param array $payload + * @param int $flags + * @return string + * @throws \JsonException + */ + private function toJsonString(array $payload, int $flags = 0): string + { + $result = @\json_encode($payload, $flags); + + if (($code = \json_last_error()) !== \JSON_ERROR_NONE) { + throw new \JsonException(\json_last_error_msg(), $code); + } + + return $result; } } diff --git a/src/Server.php b/src/Server.php index 2f9fe7b..f2265e8 100644 --- a/src/Server.php +++ b/src/Server.php @@ -1,10 +1,10 @@ services[$service])) { - throw new NotFoundException("Service `{$service}` not found.", StatusCode::NOT_FOUND); + throw NotFoundException::create("Service `{$service}` not found.", StatusCode::NOT_FOUND); } return $this->services[$service]->invoke($method, $context, $body); 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 @@ 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); @@ -94,7 +94,7 @@ public function invoke(string $method, ContextInterface $context, ?string $input /** * Configure service name and methods. * - * @param string $interface + * @param string $interface * @param ServiceInterface $service * * @throws ServiceException @@ -103,23 +103,20 @@ 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." - ); + + if (! $r->hasConstant('NAME')) { + $message = "Invalid service interface `{$interface}`, constant `NAME` not found."; + throw ServiceException::create($message); } + $this->name = $r->getConstant('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; 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/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', From 3ac8f789e3deddcad34ed5e1d804a107ccd6dc1c Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Thu, 25 Mar 2021 21:34:14 +0300 Subject: [PATCH 10/29] Remove phpunit cache --- .gitignore | 3 ++- .phpunit.result.cache | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) delete mode 100644 .phpunit.result.cache 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/.phpunit.result.cache b/.phpunit.result.cache deleted file mode 100644 index c23c558..0000000 --- a/.phpunit.result.cache +++ /dev/null @@ -1 +0,0 @@ -C:37:"PHPUnit\Runner\DefaultTestResultCache":2131:{a:2:{s:7:"defects";a:0:{}s:5:"times";a:35:{s:43:"Spiral\GRPC\Tests\ContextTest::testGetValue";d:0.004;s:47:"Spiral\GRPC\Tests\ContextTest::testGetNullValue";d:0;s:44:"Spiral\GRPC\Tests\ContextTest::testGetValues";d:0;s:44:"Spiral\GRPC\Tests\ContextTest::testWithValue";d:0;s:52:"Spiral\GRPC\Tests\ContextTest::testGetOutgoingHeader";d:0;s:53:"Spiral\GRPC\Tests\ContextTest::testGetOutgoingHeaders";d:0;s:44:"Spiral\GRPC\Tests\ExceptionTest::testDefault";d:0.001;s:45:"Spiral\GRPC\Tests\ExceptionTest::testNotFound";d:0;s:43:"Spiral\GRPC\Tests\ExceptionTest::testInvoke";d:0;s:52:"Spiral\GRPC\Tests\ExceptionTest::testUnauthenticated";d:0;s:50:"Spiral\GRPC\Tests\ExceptionTest::testUnimplemented";d:0;s:41:"Spiral\GRPC\Tests\InvokerTest::testInvoke";d:0.014;s:46:"Spiral\GRPC\Tests\InvokerTest::testInvokeError";d:0.001;s:46:"Spiral\GRPC\Tests\MethodTest::testInvalidParse";d:0;s:39:"Spiral\GRPC\Tests\MethodTest::testMatch";d:0;s:41:"Spiral\GRPC\Tests\MethodTest::testNoMatch";d:0;s:42:"Spiral\GRPC\Tests\MethodTest::testNoMatch2";d:0;s:42:"Spiral\GRPC\Tests\MethodTest::testNoMatch3";d:0;s:42:"Spiral\GRPC\Tests\MethodTest::testNoMatch4";d:0;s:42:"Spiral\GRPC\Tests\MethodTest::testNoMatch5";d:0;s:44:"Spiral\GRPC\Tests\MethodTest::testMethodName";d:0;s:49:"Spiral\GRPC\Tests\MethodTest::testMethodInputType";d:0;s:50:"Spiral\GRPC\Tests\MethodTest::testMethodOutputType";d:0;s:40:"Spiral\GRPC\Tests\ServerTest::testInvoke";d:0.003;s:42:"Spiral\GRPC\Tests\ServerTest::testNotFound";d:0;s:43:"Spiral\GRPC\Tests\ServerTest::testNotFound2";d:0;s:59:"Spiral\GRPC\Tests\ServerTest::testServerDebugModeNotEnabled";d:0;s:46:"Spiral\GRPC\Tests\ServiceWrapperTest::testName";d:0;s:49:"Spiral\GRPC\Tests\ServiceWrapperTest::testService";d:0;s:49:"Spiral\GRPC\Tests\ServiceWrapperTest::testMethods";d:0;s:56:"Spiral\GRPC\Tests\ServiceWrapperTest::testInvokeNotFound";d:0;s:48:"Spiral\GRPC\Tests\ServiceWrapperTest::testInvoke";d:0;s:56:"Spiral\GRPC\Tests\ServiceWrapperTest::testNotImplemented";d:0;s:58:"Spiral\GRPC\Tests\ServiceWrapperTest::testInvalidInterface";d:0;s:59:"Spiral\GRPC\Tests\ServiceWrapperTest::testInvalidInterface2";d:0;}}} \ No newline at end of file From 96ce30fc27b8fc5fa8978b64f5b6603293fc8274 Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Thu, 25 Mar 2021 21:41:45 +0300 Subject: [PATCH 11/29] Implement "Context::offsetSet" method --- src/Context.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Context.php b/src/Context.php index 07d876f..3c9dab7 100644 --- a/src/Context.php +++ b/src/Context.php @@ -91,7 +91,7 @@ public function offsetSet($offset, $value): void { assert(\is_string($offset), 'Offset argument must be a type of string'); - throw new \LogicException(__METHOD__ . ' not implemented yet'); + $this->values[$offset] = $value; } /** From 02b2e56e48c8496eb72fcff1de2610bb6f3c399c Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Tue, 30 Mar 2021 21:40:52 +0300 Subject: [PATCH 12/29] Improve Server code --- src/Internal/Json.php | 77 +++++++++++++++++++++ src/ResponseHeaders.php | 26 +------- src/Server.php | 144 +++++++++++++++++++++++----------------- 3 files changed, 162 insertions(+), 85 deletions(-) create mode 100644 src/Internal/Json.php diff --git a/src/Internal/Json.php b/src/Internal/Json.php new file mode 100644 index 0000000..8727b5a --- /dev/null +++ b/src/Internal/Json.php @@ -0,0 +1,77 @@ + */ @@ -78,28 +80,6 @@ public function packHeaders(): string return '{}'; } - $flags = \defined('\\JSON_THROW_ON_ERROR') - // Avoid PHP DCE constant inlining - ? \constant('\\JSON_THROW_ON_ERROR') - : 0; - - return $this->toJsonString($this->headers, $flags); - } - - /** - * @param array $payload - * @param int $flags - * @return string - * @throws \JsonException - */ - private function toJsonString(array $payload, int $flags = 0): string - { - $result = @\json_encode($payload, $flags); - - if (($code = \json_last_error()) !== \JSON_ERROR_NONE) { - throw new \JsonException(\json_last_error_msg(), $code); - } - - return $result; + return Json::encode($this->headers); } } diff --git a/src/Server.php b/src/Server.php index f2265e8..f01689b 100644 --- a/src/Server.php +++ b/src/Server.php @@ -13,33 +13,53 @@ use Google\Protobuf\Any; use Google\Protobuf\Internal\Message; +use JetBrains\PhpStorm\ArrayShape; use Spiral\GRPC\Exception\GRPCException; use Spiral\GRPC\Exception\NotFoundException; use Spiral\GRPC\Exception\ServiceException; +use Spiral\GRPC\Internal\Json; use Spiral\RoadRunner\Worker; /** * Manages group of services and communication with RoadRunner server. + * + * @psalm-type ServerOptions = array { + * debug?: bool + * } + * + * @psalm-type ContextResponse = array { + * service: string, + * method: string, + * context: array> + * } */ final class Server { - /** @var InvokerInterface */ + /** + * @var InvokerInterface + */ private $invoker; - /** @var ServiceWrapper[] */ + /** + * @var ServiceWrapper[] + */ private $services = []; /** - * @var array + * @var ServerOptions */ + #[ArrayShape(['debug' => 'bool'])] private $options; /** * @param InvokerInterface|null $invoker - * @param array $options + * @param ServerOptions $options */ - public function __construct(InvokerInterface $invoker = null, array $options = []) - { + public function __construct( + InvokerInterface $invoker = null, + #[ArrayShape(['debug' => 'bool'])] + array $options = [] + ) { $this->invoker = $invoker ?? new Invoker(); $this->options = $options; } @@ -47,63 +67,72 @@ public function __construct(InvokerInterface $invoker = null, array $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 string $interface Generated service interface. * @param ServiceInterface $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]; + } + /** * 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) { + try { $body = $worker->receive($ctx); - if (empty($body) && empty($ctx)) { + + if (!$body && !$ctx) { return; } - 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)); - } catch (\Throwable $e) { - if ($this->isDebugMode()) { - $errorText = $e->__toString(); - } else { - $errorText = $e->getMessage(); - } - - $worker->error($errorText); - } finally { - if ($finalize !== null) { - call_user_func($finalize, $e ?? null); - } + /** @var ContextResponse $context */ + $context = Json::decode((string)$ctx); + + [$answerBody, $answerHeaders] = $this->tick((string)$body, $context); + + $worker->send($answerBody, $answerHeaders); + } catch (GRPCException $e) { + $worker->error($this->packError($e)); + } catch (\Throwable $e) { + $worker->error($this->isDebugMode() ? (string)$e : $e->getMessage()); + } finally { + if ($finalize !== null) { + isset($e) ? $finalize($e) : $finalize(); } } } @@ -111,22 +140,16 @@ 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|null $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])) { + 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); } @@ -138,8 +161,8 @@ 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 * @return string @@ -149,9 +172,6 @@ private function packError(GRPCException $e): string $data = [$e->getCode(), $e->getMessage()]; foreach ($e->getDetails() as $detail) { - /** - * @var Message $detail - */ $anyMessage = new Any(); $anyMessage->pack($detail); @@ -159,7 +179,7 @@ private function packError(GRPCException $e): string $data[] = $anyMessage->serializeToString(); } - return implode('|:|', $data); + return \implode('|:|', $data); } /** @@ -172,7 +192,7 @@ private function isDebugMode(): bool $debug = false; if (isset($this->options['debug'])) { - $debug = filter_var($this->options['debug'], FILTER_VALIDATE_BOOLEAN); + $debug = \filter_var($this->options['debug'], \FILTER_VALIDATE_BOOLEAN); } return $debug; From 7bffa8ea4b08ed8b5e04c63a12486a17a426a0c4 Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Tue, 30 Mar 2021 21:54:07 +0300 Subject: [PATCH 13/29] Add base support of RoadRunner 2.x --- src/Server.php | 53 ++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/src/Server.php b/src/Server.php index f01689b..cd7d7ad 100644 --- a/src/Server.php +++ b/src/Server.php @@ -18,6 +18,7 @@ use Spiral\GRPC\Exception\NotFoundException; use Spiral\GRPC\Exception\ServiceException; use Spiral\GRPC\Internal\Json; +use Spiral\RoadRunner\Payload; use Spiral\RoadRunner\Worker; /** @@ -105,6 +106,44 @@ private function tick(string $body, array $data): array return [$response, $responseHeadersString]; } + /** + * @param Worker $worker + * @param string $body + * @param string $headers + */ + 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 + */ + private function workerReceive(Worker $worker): ?array + { + $body = $worker->receive($ctx); + + return [(string)$body, (string)$ctx]; + } + /** * Serve GRPC over given RoadRunner worker. * @@ -114,22 +153,24 @@ private function tick(string $body, array $data): array public function serve(Worker $worker, callable $finalize = null): void { try { - $body = $worker->receive($ctx); + $request = $this->workerReceive($worker); - if (!$body && !$ctx) { + if (! $request) { return; } + [$body, $headers] = $request; + /** @var ContextResponse $context */ - $context = Json::decode((string)$ctx); + $context = Json::decode((string)$headers); [$answerBody, $answerHeaders] = $this->tick((string)$body, $context); - $worker->send($answerBody, $answerHeaders); + $this->workerSend($worker, $answerBody, $answerHeaders); } catch (GRPCException $e) { - $worker->error($this->packError($e)); + $this->workerError($worker, $this->packError($e)); } catch (\Throwable $e) { - $worker->error($this->isDebugMode() ? (string)$e : $e->getMessage()); + $this->workerError($worker, $this->isDebugMode() ? (string)$e : $e->getMessage()); } finally { if ($finalize !== null) { isset($e) ? $finalize($e) : $finalize(); From 3617397fbd1ada4e97236a91c7f99d3b36affa14 Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Tue, 30 Mar 2021 21:54:45 +0300 Subject: [PATCH 14/29] Improve example worker code --- example/server/worker.php | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/example/server/worker.php b/example/server/worker.php index b089ed5..683a234 100644 --- a/example/server/worker.php +++ b/example/server/worker.php @@ -3,15 +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'; -//To run server in debug mode - new \Spiral\GRPC\Server(null, ['debug' => true]); -$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); From 67578c87011d74925dcf16c5a59de45c35b0e19b Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Tue, 30 Mar 2021 21:55:02 +0300 Subject: [PATCH 15/29] Improve example composer --- example/server/composer.json | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/example/server/composer.json b/example/server/composer.json index bdcd633..b630329 100644 --- a/example/server/composer.json +++ b/example/server/composer.json @@ -1,12 +1,23 @@ { "name": "app/example-grpc-server", "description": "Example GRPC Server", + "repositories": [ + { + "type": "path", + "url": "../.." + } + ], "require": { - "spiral/php-grpc": "^1.0" + "spiral/php-grpc": "*" + }, + "require-dev": { + "grpc/grpc": "^1.36" }, "autoload": { "psr-4": { "": "src" } - } + }, + "minimum-stability": "dev", + "prefer-stable": true } From 23f74d40b200b5ce1f8b16e25bc6d1971aecc61f Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Tue, 30 Mar 2021 22:00:21 +0300 Subject: [PATCH 16/29] Rollback test paths --- phpunit.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpunit.xml b/phpunit.xml index fbd70dd..384daa4 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,5 +1,5 @@ - Date: Tue, 30 Mar 2021 22:06:13 +0300 Subject: [PATCH 17/29] Add php 8.0 CI tests and increase phpunit version --- .travis.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index d948824..d89e169 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,7 +22,7 @@ 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 + - composer require phpunit/phpunit:^8.5 --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 +62,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 From 4d68ca7022f30f8cd60a352d62d6127e5cf6cbbb Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Tue, 30 Mar 2021 22:51:34 +0300 Subject: [PATCH 18/29] Remove phpunit CI installation --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index d89e169..da5e25b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,7 +22,6 @@ 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:^8.5 --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 From 98c420cee7b8b277e1ffd2fff7a178ff2c1829f8 Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Tue, 30 Mar 2021 22:52:59 +0300 Subject: [PATCH 19/29] Fix test worker autoload file --- tests/worker.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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); From af70c7465ebdfd910c8a2a53cb3b28aed6d1ddb0 Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Wed, 31 Mar 2021 13:55:27 +0300 Subject: [PATCH 20/29] Fix server logic --- src/Server.php | 50 ++++++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/src/Server.php b/src/Server.php index cd7d7ad..a36cedc 100644 --- a/src/Server.php +++ b/src/Server.php @@ -141,6 +141,10 @@ private function workerReceive(Worker $worker): ?array { $body = $worker->receive($ctx); + if (empty($body) && empty($ctx)) { + return null; + } + return [(string)$body, (string)$ctx]; } @@ -152,28 +156,30 @@ private function workerReceive(Worker $worker): ?array */ public function serve(Worker $worker, callable $finalize = null): void { - try { - $request = $this->workerReceive($worker); - - if (! $request) { - return; - } - - [$body, $headers] = $request; - - /** @var ContextResponse $context */ - $context = Json::decode((string)$headers); - - [$answerBody, $answerHeaders] = $this->tick((string)$body, $context); - - $this->workerSend($worker, $answerBody, $answerHeaders); - } catch (GRPCException $e) { - $this->workerError($worker, $this->packError($e)); - } catch (\Throwable $e) { - $this->workerError($worker, $this->isDebugMode() ? (string)$e : $e->getMessage()); - } finally { - if ($finalize !== null) { - isset($e) ? $finalize($e) : $finalize(); + while (true) { + try { + $request = $this->workerReceive($worker); + + if (! $request) { + return; + } + + [$body, $headers] = $request; + + /** @var ContextResponse $context */ + $context = Json::decode((string)$headers); + + [$answerBody, $answerHeaders] = $this->tick((string)$body, $context); + + $this->workerSend($worker, $answerBody, $answerHeaders); + } catch (GRPCException $e) { + $this->workerError($worker, $this->packError($e)); + } catch (\Throwable $e) { + $this->workerError($worker, $this->isDebugMode() ? (string)$e : $e->getMessage()); + } finally { + if ($finalize !== null) { + isset($e) ? $finalize($e) : $finalize(); + } } } } From aca633625fa30d191a830a6fe398baf9d753c491 Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Wed, 31 Mar 2021 14:16:07 +0300 Subject: [PATCH 21/29] Extract some server logic from try/catch --- src/Server.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Server.php b/src/Server.php index a36cedc..953566c 100644 --- a/src/Server.php +++ b/src/Server.php @@ -157,15 +157,15 @@ private function workerReceive(Worker $worker): ?array public function serve(Worker $worker, callable $finalize = null): void { while (true) { - try { - $request = $this->workerReceive($worker); + $request = $this->workerReceive($worker); - if (! $request) { - return; - } + if (! $request) { + return; + } - [$body, $headers] = $request; + [$body, $headers] = $request; + try { /** @var ContextResponse $context */ $context = Json::decode((string)$headers); From 1aa9f96180164d9747cff87e257785d610d876ea Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Wed, 31 Mar 2021 14:36:51 +0300 Subject: [PATCH 22/29] Fix invoker exceptions wrapping --- src/Invoker.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Invoker.php b/src/Invoker.php index 2a80423..161310b 100644 --- a/src/Invoker.php +++ b/src/Invoker.php @@ -39,14 +39,14 @@ public function invoke(ServiceInterface $service, Method $method, ContextInterfa /** @var callable $callable */ $callable = [$service, $method->getName()]; - try { - /** @var Message $message */ - $message = $callable($ctx, $this->makeInput($method, $input)); + /** @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)); + // Note: This validation will only work if the + // assertions option ("zend.assertions") is enabled. + assert($this->assertResultType($method, $message)); + try { return $message->serializeToString(); } catch (\Throwable $e) { throw InvokeException::create($e->getMessage(), StatusCode::INTERNAL, $e); From faca40e20722e8c891081d4359dd26c176e44d33 Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Wed, 31 Mar 2021 14:39:29 +0300 Subject: [PATCH 23/29] Improve exception handling --- src/Invoker.php | 15 +++++++++------ src/Server.php | 7 ++++--- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/Invoker.php b/src/Invoker.php index 161310b..7f00cb3 100644 --- a/src/Invoker.php +++ b/src/Invoker.php @@ -12,6 +12,7 @@ namespace Spiral\GRPC; use Google\Protobuf\Internal\Message; +use Spiral\GRPC\Exception\GRPCExceptionInterface; use Spiral\GRPC\Exception\InvokeException; final class Invoker implements InvokerInterface @@ -39,15 +40,17 @@ public function invoke(ServiceInterface $service, Method $method, ContextInterfa /** @var callable $callable */ $callable = [$service, $method->getName()]; - /** @var Message $message */ - $message = $callable($ctx, $this->makeInput($method, $input)); + try { + /** @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)); + // Note: This validation will only work if the + // assertions option ("zend.assertions") is enabled. + assert($this->assertResultType($method, $message)); - try { return $message->serializeToString(); + } catch (GRPCExceptionInterface $e) { + throw $e; } catch (\Throwable $e) { throw InvokeException::create($e->getMessage(), StatusCode::INTERNAL, $e); } diff --git a/src/Server.php b/src/Server.php index 953566c..138d36a 100644 --- a/src/Server.php +++ b/src/Server.php @@ -15,6 +15,7 @@ use Google\Protobuf\Internal\Message; use JetBrains\PhpStorm\ArrayShape; use Spiral\GRPC\Exception\GRPCException; +use Spiral\GRPC\Exception\GRPCExceptionInterface; use Spiral\GRPC\Exception\NotFoundException; use Spiral\GRPC\Exception\ServiceException; use Spiral\GRPC\Internal\Json; @@ -172,7 +173,7 @@ public function serve(Worker $worker, callable $finalize = null): void [$answerBody, $answerHeaders] = $this->tick((string)$body, $context); $this->workerSend($worker, $answerBody, $answerHeaders); - } catch (GRPCException $e) { + } catch (GRPCExceptionInterface $e) { $this->workerError($worker, $this->packError($e)); } catch (\Throwable $e) { $this->workerError($worker, $this->isDebugMode() ? (string)$e : $e->getMessage()); @@ -211,10 +212,10 @@ protected function invoke(string $service, string $method, ContextInterface $con * 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()]; From 781d3b16914361495e5626fd3d0c54335f49cb40 Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Wed, 31 Mar 2021 14:57:14 +0300 Subject: [PATCH 24/29] Remove JB attributes (phpcs bug) --- src/Server.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Server.php b/src/Server.php index 138d36a..79070ee 100644 --- a/src/Server.php +++ b/src/Server.php @@ -12,8 +12,6 @@ namespace Spiral\GRPC; use Google\Protobuf\Any; -use Google\Protobuf\Internal\Message; -use JetBrains\PhpStorm\ArrayShape; use Spiral\GRPC\Exception\GRPCException; use Spiral\GRPC\Exception\GRPCExceptionInterface; use Spiral\GRPC\Exception\NotFoundException; @@ -50,18 +48,14 @@ final class Server /** * @var ServerOptions */ - #[ArrayShape(['debug' => 'bool'])] private $options; /** * @param InvokerInterface|null $invoker * @param ServerOptions $options */ - public function __construct( - InvokerInterface $invoker = null, - #[ArrayShape(['debug' => 'bool'])] - array $options = [] - ) { + public function __construct(InvokerInterface $invoker = null, array $options = []) + { $this->invoker = $invoker ?? new Invoker(); $this->options = $options; } From fdd9e82dddfed9c3addbddf6f2f875f8074942ec Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Wed, 31 Mar 2021 14:58:40 +0300 Subject: [PATCH 25/29] Fix unit tests --- src/Invoker.php | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/Invoker.php b/src/Invoker.php index 7f00cb3..c4ca1ee 100644 --- a/src/Invoker.php +++ b/src/Invoker.php @@ -40,17 +40,15 @@ public function invoke(ServiceInterface $service, Method $method, ContextInterfa /** @var callable $callable */ $callable = [$service, $method->getName()]; - try { - /** @var Message $message */ - $message = $callable($ctx, $this->makeInput($method, $input)); + /** @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)); + // Note: This validation will only work if the + // assertions option ("zend.assertions") is enabled. + assert($this->assertResultType($method, $message)); + try { return $message->serializeToString(); - } catch (GRPCExceptionInterface $e) { - throw $e; } catch (\Throwable $e) { throw InvokeException::create($e->getMessage(), StatusCode::INTERNAL, $e); } From 7d56393f60c0a90c1fb5ffde26dce389c64731a6 Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Wed, 31 Mar 2021 15:10:15 +0300 Subject: [PATCH 26/29] AFix psalm errors and improve types --- psalm.xml | 1 + src/Exception/GRPCExceptionInterface.php | 1 + src/Invoker.php | 3 +- src/Method.php | 2 ++ src/Server.php | 11 ++++++-- src/ServiceWrapper.php | 35 ++++++++++++------------ 6 files changed, 32 insertions(+), 21 deletions(-) diff --git a/psalm.xml b/psalm.xml index 0218777..9cb3de6 100644 --- a/psalm.xml +++ b/psalm.xml @@ -16,6 +16,7 @@ + diff --git a/src/Exception/GRPCExceptionInterface.php b/src/Exception/GRPCExceptionInterface.php index df9cba8..75c1894 100644 --- a/src/Exception/GRPCExceptionInterface.php +++ b/src/Exception/GRPCExceptionInterface.php @@ -23,6 +23,7 @@ interface GRPCExceptionInterface extends \Throwable /** * Returns GRPC exception status code. * + * @psalm-suppress MissingImmutableAnnotation * @psalm-return StatusCodeType * @return int */ diff --git a/src/Invoker.php b/src/Invoker.php index c4ca1ee..706ded5 100644 --- a/src/Invoker.php +++ b/src/Invoker.php @@ -81,7 +81,6 @@ private function assertResultType(Method $method, $result): bool * @param string|null $body * @return Message * @throws InvokeException - * @psalm-suppress InvalidStringClass */ private function makeInput(Method $method, ?string $body): Message { @@ -92,7 +91,7 @@ private function makeInput(Method $method, ?string $body): Message // assertions option ("zend.assertions") is enabled. assert($this->assertInputType($method, $class)); - /** @var Message $in */ + /** @psalm-suppress UnsafeInstantiation */ $in = new $class(); if ($body !== null) { diff --git a/src/Method.php b/src/Method.php index 9406647..5c8e19d 100644 --- a/src/Method.php +++ b/src/Method.php @@ -124,6 +124,7 @@ public function getOutputType(): string private static function getReflectionClassByType(?\ReflectionType $type): ?\ReflectionClass { if ($type instanceof \ReflectionNamedType && ! $type->isBuiltin()) { + /** @psalm-suppress ArgumentTypeCoercion */ return new \ReflectionClass($type->getName()); } @@ -319,6 +320,7 @@ public static function parse(\ReflectionMethod $method): Method /** @var \ReflectionNamedType $returnType */ $returnType = $method->getReturnType(); + /** @psalm-suppress ArgumentTypeCoercion */ return new self($method->getName(), $inputType->getName(), $returnType->getName()); } } diff --git a/src/Server.php b/src/Server.php index 79070ee..90886d3 100644 --- a/src/Server.php +++ b/src/Server.php @@ -68,8 +68,10 @@ public function __construct(InvokerInterface $invoker = null, array $options = [ * $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 @@ -105,6 +107,7 @@ private function tick(string $body, array $data): array * @param Worker $worker * @param string $body * @param string $headers + * @psalm-suppress InaccessibleMethod */ private function workerSend(Worker $worker, string $body, string $headers): void { @@ -131,9 +134,13 @@ private function workerError(Worker $worker, string $message): void /** * @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)) { diff --git a/src/ServiceWrapper.php b/src/ServiceWrapper.php index 23b1ba1..77e60e6 100644 --- a/src/ServiceWrapper.php +++ b/src/ServiceWrapper.php @@ -34,17 +34,14 @@ final class ServiceWrapper /** * @param InvokerInterface $invoker - * @param string $interface Service interface name. + * @param class-string $interface Service interface name. * @param ServiceInterface $service - * * @throws ServiceException */ - public function __construct( - InvokerInterface $invoker, - string $interface, - ServiceInterface $service - ) { + public function __construct(InvokerInterface $invoker, string $interface, ServiceInterface $service) + { $this->invoker = $invoker; + $this->configure($interface, $service); } @@ -76,40 +73,44 @@ public function getMethods(): array * @param string $method * @param ContextInterface $context * @param string|null $input - * @param array $metadata * @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 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); + $reflection = new \ReflectionClass($interface); - if (! $r->hasConstant('NAME')) { + 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) { $message = "Invalid service interface `{$interface}`."; throw ServiceException::create($message, StatusCode::INTERNAL, $e); @@ -127,7 +128,7 @@ protected function configure(string $interface, ServiceInterface $service): void /** * @param ServiceInterface $service - * @return array + * @return array */ protected function fetchMethods(ServiceInterface $service): array { From 181d7c6f4769e933a25afc9c9e95ccc69abcaa96 Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Wed, 31 Mar 2021 15:29:05 +0300 Subject: [PATCH 27/29] Using composer update command in CI instead of install --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index da5e25b..8b36a13 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,7 +15,7 @@ install: - popd - export GO111MODULE=on - go mod download - - composer install --no-interaction --prefer-source --ignore-platform-reqs + - composer update --no-interaction --prefer-source --ignore-platform-reqs - go get script: From 9fb357ab3bb94a97fcbcd13975b8a9bdc83cf074 Mon Sep 17 00:00:00 2001 From: SerafimArts Date: Wed, 31 Mar 2021 16:45:16 +0300 Subject: [PATCH 28/29] Remove "--ignore-platform-reqs" flag --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 8b36a13..b63f434 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,7 +15,7 @@ install: - popd - export GO111MODULE=on - go mod download - - composer update --no-interaction --prefer-source --ignore-platform-reqs + - composer install --no-interaction --prefer-source - go get script: From c0b8968e520f4e7c0563ca82f4faca93f48efaaa Mon Sep 17 00:00:00 2001 From: Rodrigo Chacon Date: Tue, 20 Apr 2021 20:26:20 -0300 Subject: [PATCH 29/29] Add gRPC method to call metrics Signed-off-by: Rodrigo Chacon --- cmd/rr-grpc/grpc/metrics.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) 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() } }