upgate / laravel-jsonrpc Goto Github PK

Hey,

See: https://github.com/upgate/laravel-jsonrpc/blob/master/src/Server/Server.php#L146

The run method of the Server catches Exception as final attempt to create a valid response.
Since php 7.0 there's a Throwable class which catches even syntax errors and the like.

Laravel itself does catch all exceptions via } catch (Throwable $e) {
See https://github.com/laravel/framework/blob/5.8/src/Illuminate/Routing/Pipeline.php#L33

Would it be possible for this package to do the same?
Example code snippet:

        } catch (JsonRpcException $e) {
            return $request->getId() ? RequestResponse::constructExceptionErrorResponse($request->getId(), $e) : null;
        } catch (\Throwable $e) {
            return $this->handleException($e, $request);
        }

Let me know what you think :)

I found that you don't support form request object in laravel, because you haven't found request parameters initialization and passing them to form request or a default request object if it's passed to a controller action.
/upgate/laravel-jsonrpc/src/Server/RouteDispatcher.php
`if ($requestParams) {
$params = $requestParams->getParams() ?: null;
if ($params) {
$areParamsNamed = $requestParams->areParamsNamed();
}

    }

    $args = [];
    foreach ($method->getParameters() as $parameter) {
        $class = $parameter->getClass();
        if ($class) {
            try {
                /** I suppose some initialization must be implemented here*/
                $args[] = $this->container->make($class->name);
            } catch (ReflectionException $e) {
                throw new InternalErrorException($e->getMessage(), 0, $e);
            }
        } else {
            if (null !== $params) {
                if ($parameter->isVariadic()) {
                    foreach ($params as $key => $value) {
                        $args[] = $this->cast($value, $parameter);
                    }
                    break;
                }

                if ($areParamsNamed) {
                    $name = $parameter->getName();
                    if (array_key_exists($name, $params)) {
                        $args[] = $this->cast($params[$name], $parameter);
                        unset($params[$name]);
                        continue;
                    }
                } else {
                    if (count($params)) {
                        $args[] = $this->cast(array_shift($params), $parameter);
                        continue;
                    }
                }
            }
            try {
                $args[] = $parameter->getDefaultValue();
            } catch (ReflectionException $e) {
                throw new InvalidParamsException("\"{$parameter->getName()}\" is required", 0, $e);
            }
        }
    }`

Hello, i have the issue with installing you library in Laravel Framework 5.5. After I defined routing in RouteServiceProvider according to your readme file and try to run the server, the following error appeared:
Declaration of Upgate\LaravelJsonRpc\Server\Server::setPayload(string $payload): Upgate\LaravelJsonRpc\Server\void must be compatible with Upgate\LaravelJsonRpc\Contract\ServerInterface::setPayload(
string $payload): Upgate\LaravelJsonRpc\Contract\void

I look at Server class and it seems the implementation of the contract is correct, but it can't figure out where the error is. Thanks for help in advance.

Hello!
I've noticed a little problem, which appears when the Router::resolve method is being called.

This method finds the appropriate controller or method Binding and resolves specific Route.

The problem is that Binding can be created before Router gets knowledge about middleware aliases. So Binding will contain different middleware aliases than Router.

Doesn't work properly:

$server->router()->bindController(...); // creates Binding here
$server->registerMiddlewareAliases(...); // sets middleware aliases to router, Bindings won't know about them

Works fine:

$server->registerMiddlewareAliases(...); // sets middleware aliases
$server->router()->bindController(...); // new Binding will get actual aliases

Currently, I didn't research a good solution. As an option, I suppose it would be good to try:

• remove MiddlewaresCollection from Bindings constructor
• pass them during calling Router::resolve

// \Upgate\LaravelJsonRpc\Server\Router::resolve
public function resolve(string $method): RouteContract
{
    return $this
        ->findBinding($method)
        ->setMiddlewaresCollection($this->middlewaresCollection)
        ->resolveRoute($method);
}

Also, I didn't research the problem deeply, so I can't say for sure, is that a problem with an inconsistent state exists only for middleware aliases.

I appreciate any feedback you may have.

Hey,

I've created a PR which adds Laravel 10 support. #38

Is there something I can help you with to further it's progress? :)

Hi,
i use this library 0.2 on PHP 7.0 and found requirements bug.
In linbrary is used return type void:

function () : void

This return type is only in PHP 7.1 and later, but in composer.json is require rule:

"php": ">=7.0",

Can you fix it for people who have project on php 7.0 that they need version 0.1 of this library.
It it confusing because if some on see composer.json on github, he thinks than it works on 7.0 but not.
I realized this after i have integrated library in project and run first tests.

Hey,

See: https://github.com/upgate/laravel-jsonrpc/blob/master/src/ServiceProvider/JsonRpcServerServiceProvider.php#L20

Right now when resolving the Server via Dependency Injection in Controllers, a new instance is created for each injection.
This makes it impossible to configure the Server in separate files since a new instance is given for each DI.

I managed to fix it by adding this package to dont-discover in composer.json:

"dont-discover": [
        "upgate/laravel-jsonrpc"
      ]

And registering a separate Service Provider which binds the classes as singletons:

class JsonRpcServerServiceProvider extends ServiceProvider
{
    public function register(): void
    {
        $this->app->singleton(RouteRegistryInterface::class, Router::class);
        $this->app->singleton(RequestFactoryInterface::class, RequestFactory::class);
        $this->app->bind(RouteDispatcherInterface::class, RouteDispatcher::class);
        $this->app->singleton(MiddlewareDispatcherInterface::class, MiddlewarePipelineDispatcher::class);
        $this->app->singleton(ServerInterface::class, Server::class);
    }
}

And separated code to setup routing:

final class JobApplicationJsonRpcRegistrator implements Registrator
{
    /** @var JsonRpcServer */
    private $server;

    public function __construct(JsonRpcServer $server)
    {
        $this->server = $server;
    }

    public function register(): void
    {
        $this->server
            ->router()
            ->bindController('job-application', JobApplicationController::class);
    }
}

final class VacancyJsonRpcRegistrator implements Registrator
{
    /** @var JsonRpcServer */
    private $server;

    public function __construct(JsonRpcServer $server)
    {
        $this->server = $server;
    }

    public function register(): void
    {
        $this->server
            ->router()
            ->bindController('vacancy', VacancyController::class);
    }
}

Would it be possible to bind the ServerInterface as singleton to the concrete Server as is shown in the JsonRpcServerServiceProvider snippet above?

Let me know what you think :)

Hi There,

While working on a project I have encountered a problem while attempting to assign 3 middelwares to my router. I am using version 0.3.2 as provided by composer and receive the following error while attempting to replicate the example in some way for my use cases.

The error:

(1/1) ErrorCall to undefined method Upgate\LaravelJsonRpc\Server\Router::setMiddlewares()
--
in rpc.php line 18

My routes/rpc.php:

$router->post('/rpc', function (Request $request) use ($jsonRpcServer) {
    $jsonRpcServer->router()
        ->setMiddlewares(['auth', '2fa', 'ssp'])
        ->bindController('server', 'Controller');
    return $jsonRpcServer->run($request);
});

Do you have any idea how I can work around this? providing the middlewares inside of the controller constructor does not seem to work, and I'm also unsure if this is an actual bug in the code.

Regards,
Ely Haughie

When I use jwt library, I add authentication middleware from jwt to group of routes. When authentication is failed, middleware throws exception UnauthorizedHttpException, but server returns internal server error. I implement my own middleware and exception which inherits from JsonRpcException, but not everything can be rewrite in a such manner.
Maybe it will be better to add some flexibility during exceptoin handling. What do you think?

4fdace4 (v0.7.0)

Здравствйте. Столкнулся с проблемой. Если использовать правила валидации через дочерние классы
Upgate\LaravelJsonRpc\Server\FormRequest для внутренних структур типа field.*.field то эти поля после валидации примут значение null.

Пример:

class TestRequest extends \Upgate\LaravelJsonRpc\Server\FormRequest
{
    public function rules(): array
    {
        return [
            'simpleParam1' => 'string',
            'simpleParam2' => 'integer',
            'complexParamArray' => 'sometimes|array',
            'complexParamArray.*.a' => 'integer',
            'complexParamArray.*.b' => 'string',
        ];
    }
}

Запрос:

{
  "id": 1597253370,
  "method": "test.test",
  "jsonrpc": "2.0",
  "params": {
    "simpleParam1": "string",
    "simpleParam2": 42,
    "complexParamArray": [
      {
        "a": 1,
        "b": 2
      },
      {
        "a": 3,
        "b": 4
      }
    ]
  }
}

Результат dd($request->all())

array:4 [
  "simpleParam1" => "string"
  "simpleParam2" => 42
  "complexParamArray" => array:2 [
    0 => {#1218
      +"a": null
      +"b": null
    }
    1 => {#1009
      +"a": null
      +"b": null
    }
  ]
]

Происходит это из-за этой логики https://github.com/laravel/framework/blob/c7d954003627b46d54d990c354d521384c80edf7/src/Illuminate/Validation/ValidationData.php#L43 в коде Laravel. Она копирует параметры запроса, и не рассчитывает, что внутри могут быть объекты, а поэтому она обнуляет их свойства, а объекты передаются по ссылке.

А здесь https://github.com/upgate/laravel-jsonrpc/blob/master/src/Server/RequestFactory.php#L21 запрос парсится в объект.

Если делать аналогичный JSON запрос POST-ом в Laravel используя аналогичные правила валидации — такой проблемы не возникает.

Я предполагаю два решения:

• можно попробовать затащить в Laravel PR и попытаться исправить это там
• сделать PR сюда и парсить тело запроса в ассоциативный массив, но я не уверен, насколько пользователи вашей библиотеки используют такие вложенные структуры и как это скажется на них

Я склоняюсь ближе ко второму варианту, потому что Laravel сама так делает: https://github.com/laravel/framework/blob/c7d954003627b46d54d990c354d521384c80edf7/src/Illuminate/Http/Request.php#L354

Hey,

See: https://github.com/upgate/laravel-jsonrpc/blob/master/src/Server/FormRequestFactory.php#L60

The signature of the make method is public function make(array $data, array $rules, array $messages = [], array $customAttributes = [])

Would is be possible to pass the attributes along to the validation similar to how that works for the rules?

Simple code snippet:

        return $this->validationFactory->make(
            $formRequest->all(),
            $this->container->call([$formRequest, 'rules']),
            $formRequest->messages(),
            $formRequest->attributes()
        );

Let me know what you think :)