[Dbix-class] ResultSet->new() ignores custom result_class [patch + tests]

[Matt S Trout]

On Wed, Feb 06, 2008 at 08:23:00PM +0100, Emanuele Zeppieri wrote:
> Could you please verify this bug report ( and review the attached 
> patch(es) )?
> 
> The message could seem lengthy (well, it really is), but it will 
> hopefully help the reader in quickly visualize the problem and its 
> implications, and in promptly identifying the relevant code portions.
> 
> 1. The problem
> ~~~~~~~~~~~~~~
> 
> To illustrate this bug, let's first see its effects rather than its 
> cause: currently its main effect is that it prevents HashRefInflator 
> from working (with the exception of the /simplest/ cases).
> 
> That is, though something like this works as advertised:
> 
> my $rs = $schema->resultset('Artist');
> $rs->result_class('DBIx::Class::ResultClass::HashRefInflator');
> my $hash_ref = $rs->find(1);
> 
> as soon as we pass *any* \%attr to find, the above code stops working:
> 
> my $hash_ref = $rs->find( 1, { key => 'primary' } ); # For example.
> print ref $hash_ref; # Prints 'DBICTest::Artist'!
>                      # (assuming we're using that.)
> print hash_ref->{name}; # Uninitialized value!

That's a bug in find(). Actually, probably two bugs in find.

find uses search if it's got attributes. It shouldn't need to do that for
just 'key'.

it also -should- pass $self->result_class to the search attrs, since
$self->result_class doesn't persist across searches (only result_class
supplied as an attr).

> my $rs = $schema->resultset('Artist');
> $rs->result_class('DBIx::Class::ResultClass::HashRefInflator');
> my $hash_ref = $rs->search({ artistid => 1 })->first;
> # Again $hash_ref is not an hash ref!

That's because you passed it to the accessor.

If you want it to survive across the chain it needs to be passed an an
attribute (turns out having the option to have either behaviour is really
handy occasionally :).

A doc patch making this clear would be much appreciated.
 
> or when we use search_related() or $rs->page() (please check the 
> attached test script for even more failing cases).

search_related doesn't preserve the result_class and it isn't logical for
it to do so. A search_related traverses the relationship on the source.

Though it might be possible for a result class to provide an __related_class
API or something so that we can traverse the result class at the same time
(and that way HashRefInflator could just return $self for that). I'd like
to discuss that separately though, since it's fairly major work (so start
another thread if you still want to pursue it :)

page() doesn't work because it uses search(), which behaves as described
above.

I've cut the rest of your e-mail because it's basically following on (with
impeccable logic, admittedly) from incorrect assumptions.

If you'd like to have a go at reworking your tests/patch along with doc
tweaks as described, I'd love to have the find() behaviour fixed.

-- 
      Matt S Trout       Need help with your Catalyst or DBIx::Class project?
   Technical Director                    http://www.shadowcat.co.uk/catalyst/
 Shadowcat Systems Ltd.  Want a managed development or deployment platform?
http://chainsawblues.vox.com/            http://www.shadowcat.co.uk/servers/