[Bast-commits] r3571 - in trunk/DBIx-Class: . lib/DBIx/Class
lib/DBIx/Class/Storage t
captainL at dev.catalyst.perl.org
captainL at dev.catalyst.perl.org
Wed Jul 4 23:03:59 GMT 2007
Author: captainL
Date: 2007-07-04 23:03:59 +0100 (Wed, 04 Jul 2007)
New Revision: 3571
Added:
trunk/DBIx-Class/t/91merge_attr.t
Modified:
trunk/DBIx-Class/Makefile.PL
trunk/DBIx-Class/lib/DBIx/Class/ResultSet.pm
trunk/DBIx-Class/lib/DBIx/Class/Storage/DBI.pm
trunk/DBIx-Class/t/90join_torture.t
Log:
fixed attr merging problem
Modified: trunk/DBIx-Class/Makefile.PL
===================================================================
--- trunk/DBIx-Class/Makefile.PL 2007-07-04 18:57:31 UTC (rev 3570)
+++ trunk/DBIx-Class/Makefile.PL 2007-07-04 22:03:59 UTC (rev 3571)
@@ -22,6 +22,7 @@
requires 'Encode' => 0 if ($] <= 5.008000);
build_requires 'DBD::SQLite' => 1.11;
+build_requires 'Test::Builder' => 0.70;
install_script 'script/dbicadmin';
Modified: trunk/DBIx-Class/lib/DBIx/Class/ResultSet.pm
===================================================================
--- trunk/DBIx-Class/lib/DBIx/Class/ResultSet.pm 2007-07-04 18:57:31 UTC (rev 3570)
+++ trunk/DBIx-Class/lib/DBIx/Class/ResultSet.pm 2007-07-04 22:03:59 UTC (rev 3571)
@@ -1795,9 +1795,14 @@
my $join = ($attrs->{join}
? [ $attrs->{join}, $extra_join ]
: $extra_join);
+
+ # we need to take the prefetch the attrs into account before we
+ # ->resolve_join as otherwise they get lost - captainL
+ my $merged = $self->_merge_attr( $join, $attrs->{prefetch} );
+
$from = [
@$from,
- ($join ? $source->resolve_join($join, $attrs->{alias}, $seen) : ()),
+ ($join ? $source->resolve_join($merged, $attrs->{alias}, $seen) : ()),
];
return ($from,$seen);
@@ -1858,6 +1863,7 @@
$join = $self->_merge_attr(
$join, $attrs->{prefetch}
);
+
}
$attrs->{from} = # have to copy here to avoid corrupting the original
@@ -1865,6 +1871,7 @@
@{$attrs->{from}},
$source->resolve_join($join, $alias, { %{$attrs->{seen_join}||{}} })
];
+
}
$attrs->{group_by} ||= $attrs->{select} if delete $attrs->{distinct};
@@ -1901,48 +1908,108 @@
return $self->{_attrs} = $attrs;
}
+sub _rollout_attr {
+ my ($self, $attr) = @_;
+
+ if (ref $attr eq 'HASH') {
+ return $self->_rollout_hash($attr);
+ } elsif (ref $attr eq 'ARRAY') {
+ return $self->_rollout_array($attr);
+ } else {
+ return [$attr];
+ }
+}
+
+sub _rollout_array {
+ my ($self, $attr) = @_;
+
+ my @rolled_array;
+ foreach my $element (@{$attr}) {
+ if (ref $element eq 'HASH') {
+ push( @rolled_array, @{ $self->_rollout_hash( $element ) } );
+ } elsif (ref $element eq 'ARRAY') {
+ # XXX - should probably recurse here
+ push( @rolled_array, @{$self->_rollout_array($element)} );
+ } else {
+ push( @rolled_array, $element );
+ }
+ }
+ return \@rolled_array;
+}
+
+sub _rollout_hash {
+ my ($self, $attr) = @_;
+
+ my @rolled_array;
+ foreach my $key (keys %{$attr}) {
+ push( @rolled_array, { $key => $attr->{$key} } );
+ }
+ return \@rolled_array;
+}
+
+sub _calculate_score {
+ my ($self, $a, $b) = @_;
+
+ if (ref $b eq 'HASH') {
+ my ($b_key) = keys %{$b};
+ if (ref $a eq 'HASH') {
+ my ($a_key) = keys %{$a};
+ if ($a_key eq $b_key) {
+ return (1 + $self->_calculate_score( $a->{$a_key}, $b->{$b_key} ));
+ } else {
+ return 0;
+ }
+ } else {
+ return ($a eq $b_key) ? 1 : 0;
+ }
+ } else {
+ if (ref $a eq 'HASH') {
+ my ($a_key) = keys %{$a};
+ return ($b eq $a_key) ? 1 : 0;
+ } else {
+ return ($b eq $a) ? 1 : 0;
+ }
+ }
+}
+
sub _merge_attr {
my ($self, $a, $b) = @_;
+
return $b unless defined($a);
return $a unless defined($b);
- if (ref $b eq 'HASH' && ref $a eq 'HASH') {
- foreach my $key (keys %{$b}) {
- if (exists $a->{$key}) {
- $a->{$key} = $self->_merge_attr($a->{$key}, $b->{$key});
- } else {
- $a->{$key} = $b->{$key};
+ $a = $self->_rollout_attr($a);
+ $b = $self->_rollout_attr($b);
+
+ my $seen_keys;
+ foreach my $b_element ( @{$b} ) {
+ # find best candidate from $a to merge $b_element into
+ my $best_candidate = { position => undef, score => 0 }; my $position = 0;
+ foreach my $a_element ( @{$a} ) {
+ my $score = $self->_calculate_score( $a_element, $b_element );
+ if ($score > $best_candidate->{score}) {
+ $best_candidate->{position} = $position;
+ $best_candidate->{score} = $score;
}
+ $position++;
}
- return $a;
- } else {
- $a = [$a] unless ref $a eq 'ARRAY';
- $b = [$b] unless ref $b eq 'ARRAY';
-
- my $hash = {};
- my @array;
- foreach my $x ($a, $b) {
- foreach my $element (@{$x}) {
- if (ref $element eq 'HASH') {
- $hash = $self->_merge_attr($hash, $element);
- } elsif (ref $element eq 'ARRAY') {
- push(@array, @{$element});
- } else {
- push(@array, $element) unless $b == $x
- && grep { $_ eq $element } @array;
- }
+ my ($b_key) = ( ref $b_element eq 'HASH' ) ? keys %{$b_element} : ($b_element);
+ if ($best_candidate->{score} == 0 || exists $seen_keys->{$b_key}) {
+ push( @{$a}, $b_element );
+ } else {
+ $seen_keys->{$b_key} = 1; # don't merge the same key twice
+ my $a_best = $a->[$best_candidate->{position}];
+ # merge a_best and b_element together and replace original with merged
+ if (ref $a_best ne 'HASH') {
+ $a->[$best_candidate->{position}] = $b_element;
+ } elsif (ref $b_element eq 'HASH') {
+ my ($key) = keys %{$a_best};
+ $a->[$best_candidate->{position}] = { $key => $self->_merge_attr($a_best->{$key}, $b_element->{$key}) };
}
}
-
- @array = grep { !exists $hash->{$_} } @array;
+ }
- return keys %{$hash}
- ? ( scalar(@array)
- ? [$hash, @array]
- : $hash
- )
- : \@array;
- }
+ return $a;
}
sub result_source {
Modified: trunk/DBIx-Class/lib/DBIx/Class/Storage/DBI.pm
===================================================================
--- trunk/DBIx-Class/lib/DBIx/Class/Storage/DBI.pm 2007-07-04 18:57:31 UTC (rev 3570)
+++ trunk/DBIx-Class/lib/DBIx/Class/Storage/DBI.pm 2007-07-04 22:03:59 UTC (rev 3571)
@@ -121,11 +121,11 @@
return $$fields if $ref eq 'SCALAR';
if ($ref eq 'ARRAY') {
- return join(', ', map {
+ return join(', ', map {
$self->_recurse_fields($_)
- .(exists $self->{rownum_hack_count} && !($params && $params->{no_rownum_hack})
- ? ' AS col'.$self->{rownum_hack_count}++
- : '')
+ .(exists $self->{rownum_hack_count} && !($params && $params->{no_rownum_hack})
+ ? ' AS col'.$self->{rownum_hack_count}++
+ : '')
} @$fields);
} elsif ($ref eq 'HASH') {
foreach my $func (keys %$fields) {
@@ -142,7 +142,7 @@
if (ref $_[0] eq 'HASH') {
if (defined $_[0]->{group_by}) {
$ret = $self->_sqlcase(' group by ')
- .$self->_recurse_fields($_[0]->{group_by}, { no_rownum_hack => 1 });
+ .$self->_recurse_fields($_[0]->{group_by}, { no_rownum_hack => 1 });
}
if (defined $_[0]->{having}) {
my $frag;
Modified: trunk/DBIx-Class/t/90join_torture.t
===================================================================
--- trunk/DBIx-Class/t/90join_torture.t 2007-07-04 18:57:31 UTC (rev 3570)
+++ trunk/DBIx-Class/t/90join_torture.t 2007-07-04 22:03:59 UTC (rev 3571)
@@ -7,8 +7,33 @@
use Data::Dumper;
my $schema = DBICTest->init_schema();
-plan tests => 19;
+plan tests => 20;
+ {
+ my $rs = $schema->resultset( 'CD' )->search(
+ {
+ 'producer.name' => 'blah',
+ 'producer_2.name' => 'foo',
+ },
+ {
+ 'join' => [
+ { cd_to_producer => 'producer' },
+ { cd_to_producer => 'producer' },
+ ],
+ 'prefetch' => [
+ 'artist',
+ { cd_to_producer => 'producer' },
+ ],
+ }
+ );
+
+ eval {
+ my @rows = $rs->all();
+ };
+ is( $@, '' );
+ }
+
+
my @rs1a_results = $schema->resultset("Artist")->search_related('cds', {title => 'Forkful of bees'}, {order_by => 'title'});
is($rs1a_results[0]->title, 'Forkful of bees', "bare field conditions okay after search related");
my $rs1 = $schema->resultset("Artist")->search({ 'tags.tag' => 'Blue' }, { join => {'cds' => 'tracks'}, prefetch => {'cds' => 'tags'} });
@@ -16,14 +41,19 @@
cmp_ok(@artists, '==', 2, "Two artists returned");
my $rs2 = $rs1->search({ artistid => '1' }, { join => {'cds' => {'cd_to_producer' => 'producer'} } });
+use Data::Dumper; print "attrs: " . Dumper($rs1->{attrs}) ;
+use Data::Dumper; print "attrs: " . Dumper($rs2->{attrs}) ;
my @artists2 = $rs2->search({ 'producer.name' => 'Matt S Trout' });
my @cds = $artists2[0]->cds;
cmp_ok(scalar @cds, '==', 1, "condition based on inherited join okay");
-#this is wrong, should accept me.title really
my $rs3 = $rs2->search_related('cds');
+use Data::Dumper; print "attrs: " . Dumper($rs2->{attrs}) ;
+use Data::Dumper; print "attrs: " . Dumper($rs3->{attrs}) ;
cmp_ok(scalar($rs3->all), '==', 45, "All cds for artist returned");
+
+
cmp_ok($rs3->count, '==', 45, "All cds for artist returned via count");
my $rs4 = $schema->resultset("CD")->search({ 'artist.artistid' => '1' }, { join => ['tracks', 'artist'], prefetch => 'artist' });
Added: trunk/DBIx-Class/t/91merge_attr.t
===================================================================
--- trunk/DBIx-Class/t/91merge_attr.t (rev 0)
+++ trunk/DBIx-Class/t/91merge_attr.t 2007-07-04 22:03:59 UTC (rev 3571)
@@ -0,0 +1,128 @@
+use strict;
+use warnings;
+
+use Test::More;
+use lib qw(t/lib);
+use DBICTest;
+use Test::More;
+use Data::Dumper;
+
+plan tests => 14;
+
+my $schema = DBICTest->init_schema();
+my $rs = $schema->resultset( 'CD' );
+
+{
+ my $a = 'artist';
+ my $b = 'cd';
+ my $expected = [ 'artist', 'cd' ];
+ my $result = $rs->_merge_attr($a, $b);
+ is_deeply( $result, $expected );
+}
+
+{
+ my $a = [ 'artist' ];
+ my $b = [ 'cd' ];
+ my $expected = [ 'artist', 'cd' ];
+ my $result = $rs->_merge_attr($a, $b);
+ is_deeply( $result, $expected );
+}
+
+{
+ my $a = [ 'artist', 'cd' ];
+ my $b = [ 'cd' ];
+ my $expected = [ 'artist', 'cd' ];
+ my $result = $rs->_merge_attr($a, $b);
+ is_deeply( $result, $expected );
+}
+
+{
+ my $a = [ 'artist', 'artist' ];
+ my $b = [ 'artist', 'cd' ];
+ my $expected = [ 'artist', 'artist', 'cd' ];
+ my $result = $rs->_merge_attr($a, $b);
+ is_deeply( $result, $expected );
+}
+
+{
+ my $a = [ 'artist', 'cd' ];
+ my $b = [ 'artist', 'artist' ];
+ my $expected = [ 'artist', 'cd', 'artist' ];
+ my $result = $rs->_merge_attr($a, $b);
+ is_deeply( $result, $expected );
+}
+
+{
+ my $a = [ 'artist', 'cd', { 'artist' => 'manager' } ];
+ my $b = 'artist';
+ my $expected = [ 'artist', 'cd', { 'artist' => 'manager' } ];
+ my $result = $rs->_merge_attr($a, $b);
+ is_deeply( $result, $expected );
+}
+
+{
+ my $a = [ 'artist', 'cd', { 'artist' => 'manager' } ];
+ my $b = [ 'artist', 'cd' ];
+ my $expected = [ 'artist', 'cd', { 'artist' => 'manager' } ];
+ my $result = $rs->_merge_attr($a, $b);
+ is_deeply( $result, $expected );
+}
+
+{
+ my $a = [ 'artist', 'cd', { 'artist' => 'manager' } ];
+ my $b = { 'artist' => 'manager' };
+ my $expected = [ 'artist', 'cd', { 'artist' => [ 'manager' ] } ];
+ my $result = $rs->_merge_attr($a, $b);
+ is_deeply( $result, $expected );
+}
+
+{
+ my $a = [ 'artist', 'cd', { 'artist' => 'manager' } ];
+ my $b = { 'artist' => 'agent' };
+ my $expected = [ { 'artist' => 'agent' }, 'cd', { 'artist' => 'manager' } ];
+ my $result = $rs->_merge_attr($a, $b);
+ is_deeply( $result, $expected );
+}
+
+{
+ my $a = [ 'artist', 'cd', { 'artist' => 'manager' } ];
+ my $b = { 'artist' => { 'manager' => 'artist' } };
+ my $expected = [ 'artist', 'cd', { 'artist' => [ { 'manager' => 'artist' } ] } ];
+ my $result = $rs->_merge_attr($a, $b);
+ is_deeply( $result, $expected );
+}
+
+{
+ my $a = [ 'artist', 'cd', { 'artist' => 'manager' } ];
+ my $b = { 'artist' => { 'manager' => [ 'artist', 'label' ] } };
+ my $expected = [ 'artist', 'cd', { 'artist' => [ { 'manager' => [ 'artist', 'label' ] } ] } ];
+ my $result = $rs->_merge_attr($a, $b);
+ is_deeply( $result, $expected );
+}
+
+{
+ my $a = [ 'artist', 'cd', { 'artist' => 'manager' } ];
+ my $b = { 'artist' => { 'tour_manager' => [ 'venue', 'roadie' ] } };
+ my $expected = [ { 'artist' => { 'tour_manager' => [ 'venue', 'roadie' ] } }, 'cd', { 'artist' => 'manager' } ];
+ my $result = $rs->_merge_attr($a, $b);
+ is_deeply( $result, $expected );
+}
+
+{
+ my $a = [ 'artist', 'cd' ];
+ my $b = { 'artist' => { 'tour_manager' => [ 'venue', 'roadie' ] } };
+ my $expected = [ { 'artist' => { 'tour_manager' => [ 'venue', 'roadie' ] } }, 'cd' ];
+ my $result = $rs->_merge_attr($a, $b);
+ is_deeply( $result, $expected );
+}
+
+{
+ my $a = [ { 'artist' => 'manager' }, 'cd' ];
+ my $b = [ 'artist', { 'artist' => 'manager' } ];
+ my $expected = [ { 'artist' => 'manager' }, 'cd', { 'artist' => 'manager' } ];
+ my $result = $rs->_merge_attr($a, $b);
+ is_deeply( $result, $expected );
+}
+
+
+1;
More information about the Bast-commits
mailing list