frioux / DBIx-Class-Helpers

https://metacpan.org/pod/DBIx::Class::Helpers
20 stars 38 forks source link

Passing parameters in ProxyResultSetMethod #39

Open moltar opened 10 years ago

moltar commented 10 years ago

Sometimes it is useful to pass a parameter to build a query. E.g. with_user_roles($role). However, it is only possible to do with a ResultSet, but if calling on a row, the parameters are omitted.

See:

https://github.com/frioux/DBIx-Class-Helpers/blob/master/lib/DBIx/Class/Helper/Row/ProxyResultSetMethod.pm#L35

I propose this should be ->$rs_method(@_) (but shift the first value first, of course).

What do you think?

frioux commented 10 years ago

That's a good idea, but don't forget that the return value gets cached, and additionally, the idea is to be compatible with using with_foo on the rs. I think adding a way to pass args is a good idea, but we need to be careful. Lemme ponder.

If you want to play along at home, here are my reservations: if the user calls it twice with two sep sets of args, they'll get what they called the first time, due to the caching. Same sort of issue with using with_foo(1) On the rs but then calling foo(2) On the row. The former values will be returned.

-frew (sent from my HTC 5c Galaxy megaphone, please pardon my brevity) On Sep 10, 2014 10:39 PM, "moltar" notifications@github.com wrote:

Sometimes it is useful to pass a parameter to build a query. E.g. with_user_roles($role). However, it is only possible to do with a ResultSet, but if calling on a row, the parameters are omitted.

See:

https://github.com/frioux/DBIx-Class-Helpers/blob/master/lib/DBIx/Class/Helper/Row/ProxyResultSetMethod.pm#L35

I propose this should be ->$rsmethod(@) (but shift the first value first, of course).

What do you think?

— Reply to this email directly or view it on GitHub https://github.com/frioux/DBIx-Class-Helpers/issues/39.

moltar commented 10 years ago

But with_foo on RS works already, that's the thing.

sub with_is_manager_assigned {
    my ($self, $user) = @_;

    return $self->search(
        undef,
        {   '+columns' => {
                is_manager_assigned =>
                    $self->correlate('map_managers')
                    ->count_rs({ user_id => $user->id })->as_query
            },
        },
    );
}

This allows me to:

$rs->with_is_manager_assigned($user);

But not:

$row->is_manager_assigned($user);
moltar commented 10 years ago

Where does the caching happen at? Is it the DBIC internal caching via {_column_data}?

frioux commented 10 years ago

yeah, it's DBIC internal caching. I see your problem and am not trying to say it doesn't matter, I'm just saying that you can

$rs->with_is_manager_assigned('frew');

and then after that on the row you get back do

$row->is_manager_assigned('moltar');

and get what it would have been for frew, and I'm not sure how to get past that except some kind of crazy serialization of the args smashed into the hash key for {_column_data}...

moltar commented 10 years ago

Ah, I see. I will poke around internals to see what is possible. But I am sure you are more intimately familiar with them and already know what is possible and what is not :)

frioux commented 10 years ago

I sorta think that what you want might be better part of either another helper or an extension/option on top of this one. How would you feel about:

proxy_resultset_method 'foo', arity => 3; # up to three args

?

moltar commented 10 years ago

Are you thinking of caching the results with serialized key? I'll be honest, the argument is definitely making it look ugly. But if that's the only way :)

frioux commented 10 years ago

I agree that it's gross and probably perilous. I think maybe a better optiono is to have a NonCachingProxyResultSetMethod that just always hits the db. Obviously it's not as awesome as this is, but it would solve your problem. How do you feel about that?