Parameter model binding to resolve method dependencies

[BenceSzalai]

Introduction

I've been a bit disappointed to loose the ability to automatically bind method parameters based on the requests. With a REST-ish interface Laravel provides the Route Model Binding feature, which makes it convenient to just type-hint models on the controller methods, and let them be automatically resolved and injected to be used. I could see an explicit mention of a workaround in the documentation, but I wanted to achieve something which provides a more elegant syntax to do it so.

Considerations

I've been looking at how the default Route Model Binding works and tried to achieve a similar level of convenience, however since with the RPC route definition all Procedure classes are listed together, defining the parameter bindings in the route files would create convoluted and hard to read syntax. Also while the default Route Model Binding provides the Route::model() and the Route::bind() methods, they require the registration of a new Service and Facade accessors and I wanted to keep the solution to minimal and to implement with adding the least overhead to the library.

Solution

Since the official recommentation to work around the problem was already to use custom FormRequest instances on the Procedure methods, I think it is kind of easy to add the additional binding definitions there too. This way the implementer gets the full control over how to resolve the custom parameters, but can rely on the service container to help with the binding and after all simply just type hint what they need on the Procedure methods.

The entry point of the solution is the HandleProcedure::handle() method, where I've replaced App::call($this->procedure); with BoundMethod::call(app(), $this->procedure); which is pretty much the same, however by this change we can extend the \Illuminate\Container\BoundMethod::addDependencyForCallParameter() with the custom logic needed to inject the parameters.

The only catch is that the FormRequest parameter must come before any of the type hinted parameters that need custom resolution logic to apply. This is because addDependencyForCallParameter() resolves the parameters in the order they are needed for the Procedure method. So the first parameter is a FormRequest (or in fact any class) that implements the new BindsParameters Interface. During the resolution of the subsequent parameters, addDependencyForCallParameter() first checks each previously resolved parameters if they implement the BindsParameters Interface, and if so, uses them for the resolution. If there is no such parameters or no matches to be resolved, it passes the resolution back to parent::addDependencyForCallParameter() ensuring the implementation stays compatible with the original behaviour, if someone does not use custom FormRequests or the BindsParameters Interface.

Now the BindsParameters Interface defines two methods to be used for the resolution.

• resolveParameter() is the "replacement" for Route::bind(). It receives the name of the argument of the Procedure method, and expects the dependency instance to be returned. False can be returned to indicate to proceed with the rest of the resolution methods. null can be also used, if the parameter is optional.
• getBindings() is the "replacement" for the default Implicit Binding. Since the Route files do not define how request parameters corresponds to Model parameters, "implicit" binding is not possible: it must be explicit. On the other hand, while Route::model() really matches the route parameters to Model types, in our case this is not needed, since the Model type can always be determined by the type hint on the Procedure method. What we need instead is a way to explicitly specify which parameters of the RPC request correspond to which parameter of the Processor method: the getBindings() therefore should return an array mapping the Processor method parameters to the RPC request parameters. The resolution follows the same logic as for route parameters. By default the request parameter is expected to contain a primary key (id or whatever is defined as the key of the given Model), or may contain other keys, in the same fashion as for route model binding using custom keys; the parameter and the key separated using a colon, e.g.: post:slug.

Resolution by resolveParameter() takes precedence over resolution based on the getBindings(), and getBindings() takes precedence over the default resolution by the service container.

Note

Since any parameter that implements the BindsParameters interface can resolve subsequent dependencies, it is also possible to use a separate Request or FormRequest and another dependency of a BindsParameters class, effectively separating the Request from the Resolution, but this is more like a sideefect than a feature, tho someone may find it useful to use different FormRequest casses with the same BindsParametersClass or using the same FormRequest with a different BindsParametersClass for different methods.

Examples

You can find the tests in the PR, which indicates how to use the bindings:

• FixtureRequest demonstrates the implementation of BindsParameters::getBindings() and BindsParameters::resolveParameter().
• FixtureProcedure has been extended with few new getUserName... methods, which utilise the parameter resolution (and suit the test cases).

In the Test cases I've user the Illuminate\Foundation\Auth\User as the type to be resolved to avoid adding an additional fixture class to the library just for the tests' sake.

Questions

Error codes

I've read the error code section of the JSON:RPC documentation, but couldn't truly figure out what the ranges really mean. I came to the conclusion that the library implementing the standard may use the codes between -32000 to -32099 so I've assigned few of those in the new BoundMethod class to various resolution issues, but please advice if this usage is correct or should it use different codes?

Documentation

The contribution guide says documentation is expected with patches, but would that mean to submit a separate PR against the https://github.com/sajya/sajya.github.io repo with the documentation?

---

Please let me know what do you think. I hope you find the addition useful! Let me know if something should be changed!

[tabuna]

Hey @BenceSzalai! Thanks for your suggestion. I, just like you, pondered on this topic.

At first glance, this makes sense:

public function getBindings(): array
{
    return [
        'user' => 'user:email',
    ];
}

Against:

public function user(): User
{
    return User::where('email', $this->user)->firstOrFail();
}

But this has several disadvantages, for example:

• The entry will be less explicit
• There will be no more hints for IDE
• We definitely need to use FormRequest (or an alternative) and declare each value (Inheritance can solve even with the current version)
• Support for eloquent models only

I am very glad that you are also interested in this issue. Let's try to discuss a little.I tend to lean more towards the general notation of bindings for a point. For example:

PRC::bind('user', function ($value) {
    return User::where('email', $value)->firstOrFail();
});

This avoids multiple descriptions for each action, regardless of the procedure. In this case, the binding should not matter at what depth the key is (often seen it in analytics.):

// Any multidimensional array
$params = [
    'terminal' => '127.0.0.0',
    'command'  => [
        'patch' => '/',
        'user'  => 1,
        'execute' => 'ps -a'
    ],
];

// After binding
$params = [
    'terminal' => '127.0.0.0',
    'command'  => [
        'patch' => '/',
        'user'  => EloquentModel,
        'execute' => 'ps -a'
    ],
];

In addition to eliminating duplication, this also expands the possibilities since we can form any pre-built classes (That is, the ability to use not only with eloquent models). But that doesn't cover many of the cons, like the IDE hints.

This shouldn't greatly increase the responsibilities of the package, what do you think of such a solution?

About the questions:

• We should not use the range -32768 to -32000 unless we explicitly implement the codes already described.
• Yes, to make changes to the documentation, you need to PR to the jigsaw branch.

[BenceSzalai]

getBindings() vs. custom resolution method

I agree that this is a much simplified version, but please note that I've added this way of binding to cover the simple cases. The other method: resolveParameter() can be used instead to implement custom advanced resolution logic, when needed. So it's optional:

• if you want to bind Eloquent Models based on the primary key or some other keys without much coding: use getBindings()
• if you need more control over how the binding happens use the resolveParameter() method, which allows to return any instance to be bound

With resolveParameter():

• "The entry will be less explicit": true, but the point of this method is to make things simple for the cases when simple is enough
• "There will be no more hints for IDE": Well, I'm not sure if I understand what do you mean? The resolved instance will be passed as the typed parameter to the Procedure method, so IDE type hinting would work just fine inside the Process class. Sure, inside FormRequest::resolveParameter() the hinting is useless as we are dealing with string arrays, but this is a very simple configuration array mapping request parameters to method parameters. Similar as to the validation config array inside rules() method of a FormRequest: there is no hinting, but it's clear what is going on.
• "We definitely need to use FormRequest (or an alternative) and declare each value (Inheritance can solve even with the current version)": right, my premise was to use a FormRequest, because it is simple and didn't want to register additional Service and Facades. Still that could be added as well to provide an independent entry point for the resolution. However I can imagine that not all calls would need parameters with the same name to be resolved to the same objects. From that perspective putting the resolution logic into the FormRequest is very convenient, because it also creates the link between the resolution logic and the Procedure method that handles the request. If the resolution logic can be configured on a Facade, there must be a way to define the exact call it is relevant for. Possible indeed, but could be a bit more messy.
• "Support for eloquent models only": covered above.

PRC::bind

I agree that there could be a Facade to configure the bindings. Even in that case I'd keep both approaches: the simple one, that can resolve Eloquent models (and anything implementing UrlRoutable) based on only the parameter names, and the custom one, that can resolve anything, but need to be coded explicitly.

The disadvantage of the Facade based approach is:

• A syntax need to be defined, that allows to set up the binding for all requests globally, all requests of a specific Procedure and a single method of a specific Procedure. It is not that difficult, as it can be a single parameter, which can be left empty for global, contain only a Procedure classname or a ProcedureClass@method string (or callback array). The same can be achieved easily by inheritance or traits in the various FormRequest classes.
• We need to register a service and a Facade accessor. Not a big deal, I just didn't want to add that much changes without discussing.

I see benefits in both the FormRequest approach and the Facade approach. For the Facade I like that it can be independent from the type of Request being used. With the FormRequest based solution I like however that the mapping between the Procedure method and the parameter resolution is very implicit, there's no need to type Procedure@method for the binding, it just works were it is needed. In fact I'd suggest to implement the service+facade solution beside my FormRequest solution, and not instead.

I also like both the simple mapping approach (getBindings() - kind of like a replacement for Laravel's implicit binding) and the custom resolution logic approach (resolveParameter() - which is there to handle more complex cases).

Putting it all together so far I can see 4 things:

• simple resolution using FormRequest
• custom resolution using FormRequest
• simple resolution using Facade
• custom resolution using Facade

My PR so far contains the first two. I can include the second two as well I think...

Method Paramater injection vs. Call parameter replacement

But there is one more thing I can see a bit of a difference: in your before and after $params examples you suggest that the resolved instances would be included in the $params array itself. However my solution does not touch the $params array. This PR is about a solution that injects the right classes into the Procedure methods.

So for example you could write:

class UserRequest extends FormRequest implements BindsParameters
{
    public function resolveParameter(string $parameterName) {
        if ( 'user' === $parameterName ) { // here `user` is the name of the 2nd parameter of the `isAdmin()` method
            return User::getUserInstanceBasedOnASecretHash( $this->input('user_hash') );
        }
        return false;
    }
}

class UserDetailsProcedure extends Procedure
{
    public static string $name = 'UserDetailsProcedure';

    public function isAdmin(UserRequest $request, User $user)
    {
        return [
            'hash' => $request->get('user_hash'), // The hash parameter is still just a string, not replaced with an instance
            'is_admin' => $user->is_admin() // $user model is resolved AND injected as a parameter
        ];
    }
}

I can think of cases where directly replacing the parameters in the $request may be useful, most probably when there is a lot of object to be resolved and the method parameter list would become too long. However I've not so far myself got into such situation.

Laravel's Route parameter binding also replaces the parameters with the class instances inside the $route object, however as far as I understood it's more like an implementation detail rather than the intended way to access those instances. It is not even in the documentation. The documentation suggests to use type hinted parameters on the controller methods. This is why i implemented the same in this PR.

We could in fact replace the parameters with instances inside the $request too, but in that case the binding logic must tell explicitly the type of object to inject. When the objects are injected as the Procedure method parameters, the type can be inferred from the method signature. However if the binding happens inside the request parameters array, we need to tell the resolver what type to resolve, otherwise we cannot do the simple resolution, only one based on an explicit method. Also we should keep a copy of the original request, just in case someone would need to look at the unchanged values. Laravel's Route parameter binding also keeps a copy of the original request parameters, even after binding.

But I don't really see the benefit of this approach yet. In my word it is cleaner to think about the request as the unmodified data as it was requested, and receive additional resolved classes as additional parameters.

Deep linking

You are right about the ability to handle parameters inside other parameters. I'll extend the resolution logic to allow selecting nested parameters.

Error codes

The documentation is very unclear. It says the range -32000 to -32099 is "Reserved for implementation-defined server-errors." As far as I can see this IS exactly an "implementation-defined server-error". It is clearly a server error as it happens on the server and prevents the server from serving the request, and it is implementation specific, which means it is specific to the way the JSON:RPC API is being implemented by this library. I've tried to look for discussions and it seems others have the same understanding. https://groups.google.com/g/json-rpc/c/3o2xWBr4FmI/m/upGH09mALGkJ I'm also thinking about using error codes from the -32000 to -32099 range, because any error codes outside the -32768 to -32000 range can be used by the Application itself therefore may collide with the business logic error codes. The only way to avoid that is to use the -32000 to -32099 range, as that is specified as forbidden for the Application layer, and will not be used by the Specification either as it is marked as reserved for the implementation.

[tabuna]

Hi @BenceSzalai, I've seen the latest changes. I will merge after I study and check (Very soon). Can you please create a PR for adding documentation?

[BenceSzalai]

Nested parameters

Now it is possible to bind to a request parameter, for example if the request has a customer which has an id, the binding can be configured with [ 'customer', 'id' ]. Custom fields are supported too when the binding is for Eloquent models, e.g.: [ 'customer', 'address:email' ].

Global binding

I've added a facade which can be used to define bindings globally, without using a special FormRequest. It mirrors the logic of Laravel's Route::bind() and Route::model() calls, but with an extended call signature to address various needs. Examples talk more, so:

RPC::model()

Basics

The simplest case is to bind an Eloquent model based on a request attribute:

RPC::model('user', User::class);

With this on the Procedure method that handles the RPC call an User $user parameter will be injected with the resolved User model:

/**
 * @param User $user The user resolved by global bindings.
 */
public function getUserName(User $user): string
{
    return $user->getAttribute('name');
}

Scoping

One may need to apply different models for different Procedure methods, so the binding can be scoped with the third argument:

RPC::model('user', RegularUser::class, 'myNamespace\MyProcedure@handleUser');
RPC::model('user', AdminUser::class, 'myNamespace\MyProcedure@handleAdminUser');
RPC::model('user', SpecialUser::class, 'myNamespace\special');
RPC::model('user', User::class, ''); // = RPC::model('user', User::class);

These lines mean:

• Resolve a RegularUser model for the myNamespace\MyProcedure@handleUser method
• Resolve an AdminUser model for the myNamespace\MyProcedure@handleAdminUser method
• Resolve a SpecialUser model for all Procedures and methods under the myNamespace\special namespace
• Resolve a User model for every other Procedure and method

The resolution happens in the order the binders are registered, so start with the more specific ones and progress with more generic towards the unscoped global binding.

It is also possible to bind multiple scopes in one call:

RPC::model('user', RegularUser::class, [ 
    'myNamespace\MyProcedure@handleUser', 
    'myNamespace\MyOtherProcedure@handleUser'
]);

Method parameter mapping

In some cases the method parameter may have a different name than the request parameter. In that case the 4th argument can be used to declare which method parameter should the binding apply to:

RPC::model('customer', User::class, '', 'user');

In this case the User model resolved based on the customer request parameter will be injected into the Procedure method argument $user, instead of the default $customer. This can be particularly useful with nested parameters, as their names may even collide. Consider these two bindings:

RPC::model(['seller','id'], User::class, '', 'seller');
RPC::model(['buyer','id'], User::class, '', 'buyer');

Without the 4th parameter both the seller and the buyer User would be attempted to be injected for a parameter called $id, however that is not possible to have two method parameters with the same name, therefore the first line will resolve a User model based on the parameter id nested under the parameter seller and inject the resulting User instance under $seller, while the second will inject another User resolved based on the id of the buyer and inject that for the parameter called $buyer.

Error handling

Similar to Laravel's Route::model() the last optional parameter is an error handler callback. It is called if the automatic resolution of the model fails. Anything returned from this method will be injected instead.

RPC::model('user', User::class, '', null, static function () {
    return auth()->user();
});

This code will attempt automatic Eloquent model resolution, but if it fails to find the User it will inject the currently logged in user instead.

RPC::bind()

It is the same for Route::bind() what RPC::model() is for Route::model(). The 1st, 3rd and 4th parameters are the same. The 2nd parameter is instead of $class is $binder, which is a callback that performs the binding. It receives the value of the request parameter configured by the 1st argument and is expected to return the instance to be injected into the Procedure method. E.g.:

RPC::bind(
    'user_hash',
    /**
     * @param  string  $parameter
     * @return User
     */
    static function (string $parameter) {
        return User::getUserByHash($parameter);
    },
    '', // global scope
    'user' // Inject for the method parameter called $user
);

Replacing parameters inside the request

I've had trouble to make it work in an elegant way. Indeed it would be easy to replace the parameters inside the request with instances, but I think somehow the original, unmodified request should be available as well. This in turn means Laravel's Request class should be inherited with a special RpcRequest which is able to store the request parameters in their original and bound form. And since FormRequests are commonly used too, another one would be needed that inherits from that, s.g. like RpcFormRequest and I think it does not worth the hassle. After all if someone wants to bind parameters inside the request, they can always just make their own FromRequest and just do the binding there, e.g.:

class MyRequest extends FormRequest
{
    /**
     * Bind instances into the Request
     */
    public function passedValidation()
    {
        $user = new User();
        $user = $user->resolveRouteBinding($this->user);
        $this->merge(
            [
                'user_original' => $this->user,
                'user' => $user,
            ]
        );
    }
}

[BenceSzalai]

I am still in the process of the main work. I will be back shortly. It seems to me that you can write the code better and concisely in many places.

Ahh, sorry, than for my push again to the branch. Ignore them please! I'll stop then for now looking at it.

And thanks for looking into it! I can learn from your changes I think!

[BenceSzalai]

Hi! Is there anything I can do to move this forward? Is there something missing or wrong in the solution, or is it just the lack of free capacity it is staled for?

[tabuna]

@BenceSzalai Hi. Yes, I remember this. I have no time at all now. I will try to free myself as quickly as possible.

@Mamau Can you please do this?

[BenceSzalai]

Sure, don't get me wrong, I was just wondering, no rushing or so! :)

[tabuna]

Hi @BenceSzalai. I came back a month after our last productive conversation and looked at what we had to offer. It seems we overcomplicated this thing. It may bring some practices that I don't really like. Therefore, I have prepared a small alternative that should cover almost all our expectations from binding:

Make it possible to receive parameters directly in the method:

For example, let's take the following simple procedure, where we pass two values for which we want to get the subtract:

{
    "jsonrpc": "2.0",
    "method": "math@subtract",
    "params": {
        "a": 4,
        "b": 2
    },
    "id": 1
}

In this case, we need to write something like the following in the handler:

public function subtract(Request $request) {
    return $request->get('a') - $request->get('b');
}

It is rather strange when we operate with a small number of parameters and without specifying their type. So let us make it possible to pass these arguments on-demand automatically. That could be written as follows:

public function subtract(int $a, int $b): int
{
    return $a - $b;
}

It will also work for deep things, via camelCase:

{
    "jsonrpc": "2.0",
    "method": "....",
    "params": {
        "user": {
            "name": "Alex"
        }
    },
    "id": 1
}

for argument:

public function handler(string $userName): string
{
    return $userName;
}

Specifying the same names when working with code and documentation

We can use different variable names in the code and expected JSON parameters in the proposed implementation. However, this can lead to confusion over time. For example, one command is called $user_id, and the other $client_id. It seems like an absolutely odd thing that hurts rather than adds value.

Binding declaration

I think we should rely on how laravel does it. Take our substract example:

RPC::bind('a', function () {
    return 100;
});

This will automatically replace the substituted value in our method:

public function subtract(int $a, int $b): int
{
    return $a - $b; // $a = 100
}

But at the same time, the request will contain the original value that was passed.

As for the binding to the models, here I am on the side of the sequence, as I mentioned earlier. However, it is extraordinary that the API would accept different values for the same value in other places. For example, the $user parameter in one place was defined by id, in another by email.

Therefore, you should rely on what is indicated in the model. By default, it looks like this. (Every Eloquent model has this)

/**
 * Retrieve the model for a bound value.
 *
 * @param  mixed  $value
 * @param  string|null  $field
 * @return \Illuminate\Database\Eloquent\Model|null
 */
public function resolveRouteBinding($value, $field = null)
{
    return $this->where($field ?? $this->getRouteKeyName(), $value)->first();
}

If this needs to be changed, and accept more email or something else. You can change this condition by adding ->orWhere. This will be consistent, since in all procedures, the user will be the same, and not depend on how the FormRequest is written:

And this is how we declare the model binding in a straightforward way:

RPC::model('user', User::class);

It will work for anyone where the $user argument is expected, and this value is passed.

As with automatic substitution, bindings RPC::bind and RPC::model can be set for nested elements. To do this, you need to use the dot notation in the declaration. For example:

RPC::model('user.order', ...);
RPC::bind('user.order', ...;

In turn, this will expect camelCase in the arguments:

public function handler($userOrder)

I regret that I did not have a long time to get to this, and, of course, in the end, I distorted everything. Still, I sincerely believe that the proposed alternative is more practical.

I respect and appreciate your work.