Skip to content

Commit

Permalink
Add Message Object Access in Interceptors for gRPC Component (#92)
Browse files Browse the repository at this point in the history
* Add Message Object Access in Interceptors for gRPC Component

fixes #90

* Fix CS

* Remove extra parameter

* Add psalm-suppress

* Remove Redundant `assertInputType` Method from `Invoker.php`

This change is due to the `Spiral\RoadRunner\GRPC\Method` class already performing checks on `in` and `out` parameters during the service class parsing phase. Therefore, it's unnecessary to repeat this check in the `Invoker.php` file.

* fix

---------

Co-authored-by: Maxim Smakouz <[email protected]>
  • Loading branch information
butschster and msmakouz committed Dec 12, 2023
1 parent 43458fe commit f428e6b
Show file tree
Hide file tree
Showing 14 changed files with 208 additions and 42 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,5 @@ build
clover.xml
.env
builds

.phpunit.result.cache
1 change: 1 addition & 0 deletions .styleci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,4 @@ enabled:
finder:
exclude:
- "tests/app/GRPC/Generator"
- "tests/generated"
4 changes: 3 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"grpc/grpc": "^1.42",
"roadrunner-php/centrifugo": "^2.0",
"spiral/roadrunner-http": "^3.0",
"spiral/roadrunner-grpc": "^3.0",
"spiral/roadrunner-grpc": "^3.2",
"spiral/roadrunner-jobs": "^4.0",
"spiral/roadrunner-kv": "^4.0",
"spiral/roadrunner-tcp": "^3.0",
Expand All @@ -48,6 +48,8 @@
},
"autoload-dev": {
"psr-4": {
"GPBMetadata\\": "tests/generated/GPBMetadata",
"Service\\": "tests/generated/Service",
"Spiral\\App\\": "tests/app",
"Spiral\\Tests\\": "tests/src"
}
Expand Down
29 changes: 28 additions & 1 deletion src/GRPC/Interceptor/Invoker.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,22 @@

namespace Spiral\RoadRunnerBridge\GRPC\Interceptor;

use Google\Protobuf\Internal\Message;
use Spiral\Core\CoreInterface;
use Spiral\RoadRunner\GRPC\ContextInterface;
use Spiral\RoadRunner\GRPC\Exception\InvokeException;
use Spiral\RoadRunner\GRPC\InvokerInterface;
use Spiral\RoadRunner\GRPC\Method;
use Spiral\RoadRunner\GRPC\ServiceInterface;
use Spiral\RoadRunner\GRPC\StatusCode;

/**
* @internal
*/
final class Invoker implements InvokerInterface
{
public function __construct(
private readonly CoreInterface $core
private readonly CoreInterface $core,
) {
}

Expand All @@ -27,6 +30,30 @@ public function invoke(ServiceInterface $service, Method $method, ContextInterfa
'method' => $method,
'ctx' => $ctx,
'input' => $input,
'message' => $this->makeInput($method, $input),
]);
}

/**
* Converts the input from the GRPC service method to the Message object.
*
* @throws InvokeException
*/
private function makeInput(Method $method, ?string $body): Message
{
try {
$class = $method->inputType;

/** @psalm-suppress UnsafeInstantiation */
$in = new $class();

if ($body !== null) {
$in->mergeFromString($body);
}

return $in;
} catch (\Throwable $e) {
throw InvokeException::create($e->getMessage(), StatusCode::INTERNAL, $e);
}
}
}
15 changes: 12 additions & 3 deletions src/GRPC/Interceptor/InvokerCore.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Spiral\RoadRunnerBridge\GRPC\Interceptor;

use Google\Protobuf\Internal\Message;
use Spiral\Core\CoreInterface;
use Spiral\RoadRunner\GRPC\ContextInterface;
use Spiral\RoadRunner\GRPC\InvokerInterface;
Expand All @@ -13,7 +14,7 @@
final class InvokerCore implements CoreInterface
{
public function __construct(
private readonly InvokerInterface $invoker
private readonly InvokerInterface $invoker,
) {
}

Expand All @@ -22,13 +23,21 @@ public function callAction(string $controller, string $action, array $parameters
\assert($parameters['service'] instanceof ServiceInterface);
\assert($parameters['method'] instanceof Method);
\assert($parameters['ctx'] instanceof ContextInterface);
\assert(\is_string($parameters['input']) || null === $parameters['input']);
\assert(
\is_string($parameters['input'])
|| null === $parameters['input'],
);

$input = (isset($parameters['message']) && $parameters['message'] instanceof Message)
? $parameters['message']
: $parameters['input'];

/** @psalm-suppress PossiblyInvalidArgument */
return $this->invoker->invoke(
$parameters['service'],
$parameters['method'],
$parameters['ctx'],
$parameters['input']
$input,
);
}
}
9 changes: 0 additions & 9 deletions tests/app/GRPC/Message.php

This file was deleted.

15 changes: 0 additions & 15 deletions tests/app/GRPC/PingService.php

This file was deleted.

27 changes: 27 additions & 0 deletions tests/generated/GPBMetadata/Service.php

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

56 changes: 56 additions & 0 deletions tests/generated/Service/Message.php

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions tests/generated/Service/PingInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php
# Generated by the protocol buffer compiler (spiral/php-grpc). DO NOT EDIT!
# source: ping.proto

namespace Service;

use Spiral\RoadRunner\GRPC;

interface PingInterface extends GRPC\ServiceInterface
{
// GRPC specific service name.
public const NAME = "service.Ping";

/**
* @param GRPC\ContextInterface $ctx
* @param Message $in
* @return Message
*
* @throws GRPC\Exception\InvokeException
*/
public function Ping(GRPC\ContextInterface $ctx, Message $in): Message;
}
17 changes: 17 additions & 0 deletions tests/generated/Service/PingService.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

declare(strict_types=1);

namespace Service;

use Spiral\RoadRunner\GRPC\ContextInterface;

class PingService implements PingInterface
{
public function Ping(ContextInterface $ctx, Message $in): Message
{
$out = new Message();

return $out->setMsg('PONG');
}
}
5 changes: 3 additions & 2 deletions tests/src/GRPC/DispatcherTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public function testCanServe(): void
$this->assertDispatcherCanBeServed(Dispatcher::class);
}

public function testServe()
public function testServe(): void
{
$worker = $this->mockContainer(WorkerInterface::class, Worker::class);
$this->getContainer()->bind(RoadRunnerMode::class, RoadRunnerMode::Grpc);
Expand All @@ -48,7 +48,8 @@ public function testServe()
);

$worker->shouldReceive('respond')->once()->withArgs(function (Payload $payload) {
return $payload->body === (new Message())->setMsg('PONG')->serializeToString();
$this->assertSame($payload->body, (new Message())->setMsg('PONG')->serializeToString());
return true;
});

$worker->shouldReceive('waitPayload')->once()->with()->andReturnNull();
Expand Down
2 changes: 1 addition & 1 deletion tests/src/GRPC/Interceptor/InvokerCoreTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

namespace Spiral\Tests\GRPC\Interceptor;

use Spiral\App\GRPC\PingService;
use Service\PingService;
use Spiral\RoadRunner\GRPC\ContextInterface;
use Spiral\RoadRunner\GRPC\InvokerInterface;
use Spiral\RoadRunnerBridge\GRPC\Interceptor\InvokerCore;
Expand Down
46 changes: 36 additions & 10 deletions tests/src/GRPC/Interceptor/InvokerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@

namespace Spiral\Tests\GRPC\Interceptor;

use Spiral\App\GRPC\PingService;
use Mockery as m;
use Service\PingService;
use Spiral\Core\CoreInterface;
use Service\Message;
use Spiral\RoadRunner\GRPC\ContextInterface;
use Spiral\RoadRunner\GRPC\Exception\InvokeException;
use Spiral\RoadRunner\GRPC\Method;
use Spiral\RoadRunner\GRPC\ServiceInterface;
use Spiral\RoadRunnerBridge\GRPC\Interceptor\Invoker;
use Spiral\Tests\TestCase;
use Mockery as m;

final class InvokerTest extends TestCase
{
Expand All @@ -22,16 +24,40 @@ public function testInvoke(): void
$service = m::mock(ServiceInterface::class);
$method = Method::parse(new \ReflectionMethod(PingService::class, 'Ping'));

$input = (new Message(['msg' => 'hello']))->serializeToString();
$output = (new Message(['msg' => 'world']))->serializeToString();

$ctx = m::mock(ContextInterface::class);
$core
->shouldReceive('callAction')
->once()
->with($service::class, 'Ping', [
'service' => $service,
'method' => $method,
'ctx' => $ctx = m::mock(ContextInterface::class),
'input' => $input = 'test',
])->andReturn('hello');

$this->assertSame('hello', $invoker->invoke($service, $method, $ctx, $input));
->withArgs(function (string $class, string $method, array $params) use ($service, $input) {
$this->assertSame($class, $service::class);
$this->assertSame('Ping', $method);
$this->assertInstanceOf(ContextInterface::class, $params['ctx']);
$this->assertSame($input, $params['input']);
$this->assertInstanceOf(Message::class, $params['message']);
$this->assertSame('hello', $params['message']->getMsg());

return true;
})->andReturn($output);

$this->assertSame($output, $invoker->invoke($service, $method, $ctx, $input));
}

public function testInvokeWithBrokenText(): void
{
$this->expectException(InvokeException::class);

$invoker = new Invoker(m::mock(CoreInterface::class));

$service = m::mock(ServiceInterface::class);
$method = Method::parse(new \ReflectionMethod(PingService::class, 'Ping'));

$input = 'input';
$output = 'output';

$ctx = m::mock(ContextInterface::class);
$this->assertSame($output, $invoker->invoke($service, $method, $ctx, $input));
}
}

0 comments on commit f428e6b

Please sign in to comment.