diff --git a/.gitattributes b/.gitattributes index c8b78cd..478a2db 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,2 +1,10 @@ -/tests export-ignore +* text=auto + +/.* export-ignore /example export-ignore +/tests export-ignore +/*.xml export-ignore +/*.yml export-ignore +/*.lock export-ignore +/*.dist export-ignore +/*.php export-ignore diff --git a/.github/workflows/cs-fix.yml b/.github/workflows/cs-fix.yml new file mode 100644 index 0000000..0395b27 --- /dev/null +++ b/.github/workflows/cs-fix.yml @@ -0,0 +1,12 @@ +on: + push: + branches: + - '*' + +name: Fix Code Style + +jobs: + cs-fix: + permissions: + contents: write + uses: spiral/gh-actions/.github/workflows/cs-fix.yml@master diff --git a/.gitignore b/.gitignore index f82a4a5..ebc7d88 100644 --- a/.gitignore +++ b/.gitignore @@ -1,11 +1,6 @@ -.idea/ -.env +/runtime +/vendor +/.idea +/.env composer.lock -vendor/ -*.db -clover.xml example/vendor/ -go.sum -builds/ -.phpunit.result.cache -.phpunit.cache/ diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php new file mode 100644 index 0000000..7bad12e --- /dev/null +++ b/.php-cs-fixer.dist.php @@ -0,0 +1,11 @@ +include(__DIR__ . '/src') + ->include(__FILE__) + ->allowRisky(true) + ->build(); diff --git a/composer.json b/composer.json index 90d85b9..49f9ef9 100644 --- a/composer.json +++ b/composer.json @@ -51,6 +51,8 @@ "jetbrains/phpstorm-attributes": "^1.0", "mockery/mockery": "^1.4", "phpunit/phpunit": "^10.0", + "spiral/code-style": "^2.2", + "spiral/dumper": "^3.3", "vimeo/psalm": ">=5.8" }, "autoload": { @@ -66,8 +68,11 @@ } }, "scripts": { - "tests": "phpunit", - "psalm": "psalm --no-cache" + "cs:diff": "php-cs-fixer fix --dry-run -v --diff --show-progress dots", + "cs:fix": "php-cs-fixer fix -v", + "psalm": "psalm", + "psalm:baseline": "psalm --set-baseline=psalm-baseline.xml", + "tests": "phpunit" }, "config": { "sort-packages": true diff --git a/phpunit.xml b/phpunit.xml index c41f6b5..0f99fa7 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,19 +1,37 @@ - - - - src - - + tests tests/generated + + + + + + + + + + src + + + tests + + diff --git a/psalm-baseline.xml b/psalm-baseline.xml new file mode 100644 index 0000000..119d221 --- /dev/null +++ b/psalm-baseline.xml @@ -0,0 +1,2 @@ + + diff --git a/psalm.xml b/psalm.xml index c5abbaa..aac55ff 100644 --- a/psalm.xml +++ b/psalm.xml @@ -3,6 +3,7 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="https://getpsalm.org/schema/config" xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd" + errorBaseline="psalm-baseline.xml" errorLevel="1" hoistConstants="true" resolveFromConfigFile="true" diff --git a/src/Context.php b/src/Context.php index 22d07ca..5d73bcc 100644 --- a/src/Context.php +++ b/src/Context.php @@ -16,8 +16,7 @@ final class Context implements ContextInterface, \IteratorAggregate, \Countable, */ public function __construct( private array $values, - ) { - } + ) {} public function withValue(string $key, mixed $value): ContextInterface { diff --git a/src/Internal/Json.php b/src/Internal/Json.php index 9dd9557..0006e8e 100644 --- a/src/Internal/Json.php +++ b/src/Internal/Json.php @@ -33,6 +33,6 @@ public static function encode(mixed $payload): string */ public static function decode(string $payload): array { - return (array)\json_decode($payload, true, self::DEFAULT_JSON_DEPTH, self::DEFAULT_JSON_FLAGS); + return (array) \json_decode($payload, true, self::DEFAULT_JSON_DEPTH, self::DEFAULT_JSON_FLAGS); } } diff --git a/src/Invoker.php b/src/Invoker.php index d234dd2..ebe2360 100644 --- a/src/Invoker.php +++ b/src/Invoker.php @@ -12,7 +12,6 @@ final class Invoker implements InvokerInterface private const ERROR_METHOD_RETURN = 'Method %s must return an object that instance of %s, ' . 'but the result provides type of %s'; - private const ERROR_METHOD_IN_TYPE = 'Method %s input type must be an instance of %s, ' . 'but the input is type of %s'; diff --git a/src/Method.php b/src/Method.php index 4bfbf4f..b1e821a 100644 --- a/src/Method.php +++ b/src/Method.php @@ -16,24 +16,18 @@ final class Method private const ERROR_PARAMS_COUNT = 'The GRPC method %s can only contain 2 parameters (input and output), but ' . 'signature contains an %d parameters'; - private const ERROR_PARAM_UNION_TYPE = 'Parameter $%s of the GRPC method %s cannot be declared using union type'; - private const ERROR_PARAM_CONTEXT_TYPE = 'The first parameter $%s of the GRPC method %s can only take an instance of %s'; - private const ERROR_PARAM_INPUT_TYPE = 'The second (input) parameter $%s of the GRPC method %s can only take ' . 'an instance of %s, but type %s is indicated'; - private const ERROR_RETURN_UNION_TYPE = 'Return type of the GRPC method %s cannot be declared using union type'; - private const ERROR_RETURN_TYPE = 'Return type of the GRPC method %s must return ' . 'an instance of %s, but type %s is indicated'; - private const ERROR_INVALID_GRPC_METHOD = 'Method %s is not valid GRPC method.'; /** @@ -45,7 +39,44 @@ private function __construct( public readonly string $name, public readonly string $inputType, public readonly string $outputType, - ) { + ) {} + + /** + * Returns true if method signature matches. + */ + public static function match(\ReflectionMethod $method): bool + { + try { + self::assertMethodSignature($method); + } catch (\Throwable) { + return false; + } + + return true; + } + + /** + * Creates a new {@see Method} object from a {@see \ReflectionMethod} object. + */ + public static function parse(\ReflectionMethod $method): Method + { + try { + self::assertMethodSignature($method); + } catch (\Throwable $e) { + $message = \sprintf(self::ERROR_INVALID_GRPC_METHOD, $method->getName()); + throw GRPCException::create($message, StatusCode::INTERNAL, $e); + } + + [, $input] = $method->getParameters(); + + /** @var \ReflectionNamedType $inputType */ + $inputType = $input->getType(); + + /** @var \ReflectionNamedType $returnType */ + $returnType = $method->getReturnType(); + + /** @psalm-suppress ArgumentTypeCoercion */ + return new self($method->getName(), $inputType->getName(), $returnType->getName()); } /** @@ -75,20 +106,6 @@ public function getOutputType(): string return $this->outputType; } - /** - * Returns true if method signature matches. - */ - public static function match(\ReflectionMethod $method): bool - { - try { - self::assertMethodSignature($method); - } catch (\Throwable) { - return false; - } - - return true; - } - /** * @throws \ReflectionException */ @@ -231,28 +248,4 @@ private static function assertMethodSignature(\ReflectionMethod $method): void // 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. - */ - public static function parse(\ReflectionMethod $method): Method - { - try { - self::assertMethodSignature($method); - } catch (\Throwable $e) { - $message = \sprintf(self::ERROR_INVALID_GRPC_METHOD, $method->getName()); - throw GRPCException::create($message, StatusCode::INTERNAL, $e); - } - - [, $input] = $method->getParameters(); - - /** @var \ReflectionNamedType $inputType */ - $inputType = $input->getType(); - - /** @var \ReflectionNamedType $returnType */ - $returnType = $method->getReturnType(); - - /** @psalm-suppress ArgumentTypeCoercion */ - return new self($method->getName(), $inputType->getName(), $returnType->getName()); - } } diff --git a/src/ResponseHeaders.php b/src/ResponseHeaders.php index 04532ce..7de321e 100644 --- a/src/ResponseHeaders.php +++ b/src/ResponseHeaders.php @@ -39,7 +39,6 @@ public function set(string $key, string $value): void /** * @param THeaderKey $key - * @param string|null $default * @return THeaderValue|null */ public function get(string $key, ?string $default = null): ?string diff --git a/src/Server.php b/src/Server.php index 6572ec4..7868569 100644 --- a/src/Server.php +++ b/src/Server.php @@ -39,8 +39,7 @@ final class Server public function __construct( private readonly InvokerInterface $invoker = new Invoker(), private readonly array $options = [], - ) { - } + ) {} /** * Register new GRPC service. @@ -63,39 +62,6 @@ public function registerService(string $interface, ServiceInterface $service): v $this->services[$service->getName()] = $service; } - /** - * @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]; - } - - /** - * @psalm-suppress InaccessibleMethod - */ - private function workerSend(WorkerInterface $worker, string $body, string $headers): void - { - $worker->respond(new Payload($body, $headers)); - } - - private function workerError(WorkerInterface $worker, string $message): void - { - $worker->error($message); - } - /** * Serve GRPC over given RoadRunner worker. */ @@ -121,7 +87,7 @@ public function serve(?WorkerInterface $worker = null, ?callable $finalize = nul } catch (GRPCExceptionInterface $e) { $this->workerGrpcError($worker, $e); } catch (\Throwable $e) { - $this->workerError($worker, $this->isDebugMode() ? (string)$e : $e->getMessage()); + $this->workerError($worker, $this->isDebugMode() ? (string) $e : $e->getMessage()); } finally { if ($finalize !== null) { isset($e) ? $finalize($e) : $finalize(); @@ -146,6 +112,39 @@ protected function invoke(string $service, string $method, ContextInterface $con return $this->services[$service]->invoke($method, $context, $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]; + } + + /** + * @psalm-suppress InaccessibleMethod + */ + private function workerSend(WorkerInterface $worker, string $body, string $headers): void + { + $worker->respond(new Payload($body, $headers)); + } + + private function workerError(WorkerInterface $worker, string $message): void + { + $worker->error($message); + } + private function workerGrpcError(WorkerInterface $worker, GRPCExceptionInterface $e): void { $status = new Status([ diff --git a/src/ServiceInterface.php b/src/ServiceInterface.php index bd3a623..4d3fd8a 100644 --- a/src/ServiceInterface.php +++ b/src/ServiceInterface.php @@ -7,6 +7,4 @@ /** * Indicates that given class expected to be GRPC service. */ -interface ServiceInterface -{ -} +interface ServiceInterface {} diff --git a/tests/ContextTest.php b/tests/ContextTest.php index b435655..88b360e 100644 --- a/tests/ContextTest.php +++ b/tests/ContextTest.php @@ -13,7 +13,7 @@ class ContextTest extends TestCase public function testGetValue(): void { $ctx = new Context([ - 'key' => ['value'] + 'key' => ['value'], ]); $this->assertSame(['value'], $ctx->getValue('key')); @@ -22,7 +22,7 @@ public function testGetValue(): void public function testGetNullValue(): void { $ctx = new Context([ - 'key' => ['value'] + 'key' => ['value'], ]); $this->assertSame(null, $ctx->getValue('other')); @@ -31,19 +31,18 @@ public function testGetNullValue(): void public function testGetValues(): void { $ctx = new Context([ - 'key' => ['value'] + 'key' => ['value'], ]); $this->assertSame([ - 'key' => ['value'] + 'key' => ['value'], ], $ctx->getValues()); } - public function testWithValue(): void { $ctx = new Context([ - 'key' => ['value'] + 'key' => ['value'], ]); $this->assertSame(['value'], $ctx->getValue('key')); @@ -60,7 +59,7 @@ public function testWithValue(): void public function testGetOutgoingHeader(): void { $outgoingHeaders = [ - 'Set-Cookie' => 'foobar' + 'Set-Cookie' => 'foobar', ]; $ctx = new Context([ResponseHeaders::class => new ResponseHeaders($outgoingHeaders)]); @@ -71,7 +70,7 @@ public function testGetOutgoingHeader(): void public function testGetOutgoingHeaders(): void { $outgoingHeaders = new ResponseHeaders([ - 'Set-Cookie' => 'foobar' + 'Set-Cookie' => 'foobar', ]); $ctx = new Context([ResponseHeaders::class => $outgoingHeaders]); $this->assertSame($outgoingHeaders, $ctx->getValue(ResponseHeaders::class)); diff --git a/tests/InvalidInterface.php b/tests/InvalidInterface.php index 55c8867..1aa8d6b 100644 --- a/tests/InvalidInterface.php +++ b/tests/InvalidInterface.php @@ -4,6 +4,4 @@ namespace Spiral\RoadRunner\GRPC\Tests; -interface InvalidInterface -{ -} +interface InvalidInterface {} diff --git a/tests/MethodTest.php b/tests/MethodTest.php index 789ee72..295859c 100644 --- a/tests/MethodTest.php +++ b/tests/MethodTest.php @@ -71,23 +71,13 @@ public function testMethodOutputType(): void $this->assertSame(Message::class, $m->outputType); } - public function tM(ContextInterface $context, TestService $input): Message - { - } + public function tM(ContextInterface $context, TestService $input): Message {} - public function tM2(ContextInterface $context, Message $input): TestService - { - } + public function tM2(ContextInterface $context, Message $input): TestService {} - public function tM3(TestService $context, Message $input): TestService - { - } + public function tM3(TestService $context, Message $input): TestService {} - public function tM4(TestService $context, Message $input): Invalid - { - } + public function tM4(TestService $context, Message $input): Invalid {} - public function tM5(TestService $context, Message $input): void - { - } + public function tM5(TestService $context, Message $input): void {} } diff --git a/tests/ServerTest.php b/tests/ServerTest.php index 76bcd50..7d93822 100644 --- a/tests/ServerTest.php +++ b/tests/ServerTest.php @@ -24,14 +24,7 @@ class ServerTest extends TestCase use m\Adapter\Phpunit\MockeryPHPUnitIntegration; private Server $server; - - protected function setUp(): void - { - parent::setUp(); - - $this->server = new Server(); - $this->server->registerService(TestInterface::class, new TestService()); - } + private int $obLevel; public function testInvoke(): void { @@ -41,7 +34,7 @@ public function testInvoke(): void 'service' => 'service.Test', 'method' => 'Echo', 'context' => [], - ] + ], ); $relay->shouldReceive('send')->once()->withArgs(function (Frame $frame) { @@ -49,7 +42,7 @@ public function testInvoke(): void }); $this->server->serve( - new Worker($relay) + new Worker($relay), ); } @@ -61,17 +54,17 @@ public function testNotFound(): void 'service' => 'service.Test2', 'method' => 'Echo', 'context' => [], - ] + ], ); - $relay->shouldReceive('send')->once()->withArgs(function (Frame $frame) { - $error = base64_decode(json_decode($frame->payload, true)['error']); + $relay->shouldReceive('send')->once()->withArgs(static function (Frame $frame) { + $error = \base64_decode(\json_decode($frame->payload, true)['error']); - return str_contains($error, 'Service `service.Test2` not found.'); + return \str_contains($error, 'Service `service.Test2` not found.'); }); $this->server->serve( - new Worker($relay) + new Worker($relay), ); } @@ -83,17 +76,17 @@ public function testNotFound2(): void 'service' => 'service.Test', 'method' => 'Echo2', 'context' => [], - ] + ], ); - $relay->shouldReceive('send')->once()->withArgs(function (Frame $frame) { - $error = base64_decode(json_decode($frame->payload, true)['error']); + $relay->shouldReceive('send')->once()->withArgs(static function (Frame $frame) { + $error = \base64_decode(\json_decode($frame->payload, true)['error']); - return str_contains($error, 'Method `Echo2` not found in service `service.Test`.'); + return \str_contains($error, 'Method `Echo2` not found in service `service.Test`.'); }); $this->server->serve( - new Worker($relay) + new Worker($relay), ); } @@ -105,15 +98,15 @@ public function testServerDebugModeNotEnabled(): void 'service' => 'service.Test', 'method' => 'Throw', 'context' => [], - ] + ], ); - $relay->shouldReceive('send')->once()->withArgs(function (Frame $frame) { + $relay->shouldReceive('send')->once()->withArgs(static function (Frame $frame) { return $frame->payload === 'Just another exception'; }); $this->server->serve( - new Worker($relay) + new Worker($relay), ); } @@ -132,8 +125,8 @@ public function testExceptionDetails(): void $worker->shouldReceive('waitPayload')->once()->andReturnNull(); - $worker->shouldReceive('respond')->once()->withArgs(function (Payload $payload) { - $headers = json_decode($payload->header, true); + $worker->shouldReceive('respond')->once()->withArgs(static function (Payload $payload) { + $headers = \json_decode($payload->header, true); $status = new Status(); $status->mergeFromString(\base64_decode($headers['error'])); @@ -150,18 +143,19 @@ public function testExceptionDetails(): void $server->serve($worker); } - private function packMessage(string $message): string + protected function setUp(): void { - $m = new Message(); - $m->setMsg($message); + parent::setUp(); + $this->obLevel = \ob_get_level(); - return $m->serializeToString(); + $this->server = new Server(); + $this->server->registerService(TestInterface::class, new TestService()); } protected function tearDown(): void { parent::tearDown(); - ob_end_clean(); + $this->obLevel < \ob_get_level() and \ob_end_clean(); m::close(); } @@ -169,18 +163,26 @@ protected function tearDown(): void protected function createRelay(string $body, array $header): RelayInterface { $body = $this->packMessage($body); - $header = json_encode($header); + $header = \json_encode($header); $relay = m::mock(RelayInterface::class); $relay->shouldReceive('waitFrame')->once()->andReturn( - new Frame($header . $body, [mb_strlen($header)]) + new Frame($header . $body, [\mb_strlen($header)]), ); - $header = json_encode(['stop' => true]); + $header = \json_encode(['stop' => true]); $relay->shouldReceive('waitFrame')->once()->andReturn( - new Frame($header, [mb_strlen($header)], Frame::CONTROL) + new Frame($header, [\mb_strlen($header)], Frame::CONTROL), ); return $relay; } + + private function packMessage(string $message): string + { + $m = new Message(); + $m->setMsg($message); + + return $m->serializeToString(); + } } diff --git a/tests/ServiceWrapperTest.php b/tests/ServiceWrapperTest.php index d5f8072..d0e5eac 100644 --- a/tests/ServiceWrapperTest.php +++ b/tests/ServiceWrapperTest.php @@ -20,7 +20,7 @@ public function testName(): void $w = new ServiceWrapper( new Invoker(), TestInterface::class, - new TestService() + new TestService(), ); $this->assertSame('service.Test', $w->getName()); @@ -31,7 +31,7 @@ public function testService(): void $w = new ServiceWrapper( new Invoker(), TestInterface::class, - $t = new TestService() + $t = new TestService(), ); $this->assertSame($t, $w->getService()); @@ -42,7 +42,7 @@ public function testMethods(): void $w = new ServiceWrapper( new Invoker(), TestInterface::class, - new TestService() + new TestService(), ); $this->assertCount(5, $w->getMethods()); @@ -55,7 +55,7 @@ public function testInvokeNotFound(): void $w = new ServiceWrapper( new Invoker(), TestInterface::class, - new TestService() + new TestService(), ); $w->invoke('NotFound', new Context([]), ''); @@ -66,7 +66,7 @@ public function testInvoke(): void $w = new ServiceWrapper( new Invoker(), TestInterface::class, - new TestService() + new TestService(), ); $out = $w->invoke('Echo', new Context([]), $this->packMessage('hello world')); @@ -84,7 +84,7 @@ public function testNotImplemented(): void $w = new ServiceWrapper( new Invoker(), TestInterface::class, - $this + $this, ); } @@ -95,7 +95,7 @@ public function testInvalidInterface(): void $w = new ServiceWrapper( new Invoker(), InvalidInterface::class, - $this + $this, ); } @@ -106,7 +106,7 @@ public function testInvalidInterface2(): void $w = new ServiceWrapper( new Invoker(), 'NotFound', - $this + $this, ); } diff --git a/tests/Stub/TestService.php b/tests/Stub/TestService.php index 355be31..3c10674 100644 --- a/tests/Stub/TestService.php +++ b/tests/Stub/TestService.php @@ -1,5 +1,7 @@ getMsg()); + \error_log($in->getMsg()); return $in; } @@ -55,15 +57,15 @@ public function Info(ContextInterface $ctx, Message $in): Message switch ($in->getMsg()) { case "RR_GRPC": case "ENV_KEY": - $out->setMsg(getenv($in->getMsg())); + $out->setMsg(\getenv($in->getMsg())); break; case "PID": - $out->setMsg(getmypid()); + $out->setMsg(\getmypid()); break; case "MD": - $out->setMsg(json_encode($ctx->getValue('key'))); + $out->setMsg(\json_encode($ctx->getValue('key'))); break; }