byterock

Its another step back day here in the Moose-Pen

Yesterday I added in a little code to Database::Accessor to stop users from tying to enter an empty 'elements' attribute like this;

my $da = Database::Accessor->new( { view => { name => 'test' } },

                                    elements=>[] );

and the worked fine but then as I started to work on Driver::DBI today I ran into this;

Can't use an undefined value as an ARRAY reference 

at GitHub\database-accessor\lib/Database/Accessor.pm line 715.

Having a look at the offending code in the '_elements_check' sub of Accessor.pm;

 push( @items,

            @{ $self->conditions },

            @{ $self->dynamic_conditions },

            @{ $self->sorts },

            @{ $self->dynamic_sorts },

            @{ $self->elements } );

now I see why I had added those 'default =>sub{ [] }' calls in some of my attributes to get that sub to work. I guess I will have to account for that now.

First thing I did was a quick code review to see if I have any more similar problems about as I know I use that 'default =>sub{ [] }' in other places.

After looking at my code and the original spec the only place I require this empty check is on the 'elements' attribute of my Database::Accessor class and the 'elements' attribute of my 'Gather' class but I see I also see that I use it in the 'sort' attribute of the ' Database::Accessor' as well.

Well there is no real bother here I just had a look in 'Database::Accessor::Types' and saw that I have this type all ready;

subtype 'ArrayRefofParams' => 
         as 'ArrayRef[Element|Param|Function|Expression]';

So I just added back in the 'default' and changed the type of the 'sort' and 'dynamic_sort' attributes;

 has sorts => (
            is  => 'ro',
--          isa => 'ArrayRefofElements',
++          isa => 'ArrayRefofParams',
            traits  => ['Array'],
            handles => { sort_count => 'count', },
++          default => sub { [] },
        );
…
 has dynamic_sorts => (
--      isa         => 'ArrayRefofElements',
++      isa         => 'ArrayRefofParams',
++      default  => sub { [] },
        traits      => [ 'Array', 'MooseX::MetaDescription::Meta::Trait' ],
        description => { not_in_DAD => 1 },
        is          => 'rw',
        init_arg    => undef,
        handles     => {
            reset_sorts        => 'clear',
            add_sort           => 'push',
            dynamic_sort_count => 'count',
        },
    );

tried my test case again, however I got this error;

Attribute (elements) does not pass the type constraint because: ArrayRefofElements can not be an empty array ref 

which I eventually traced back to the when I was doing a 'delete' operation. This was the offending code;

 my $dad = $driver->new(
            {
                view               => $self->view,
                elements           => ($action ne Database::Accessor::Constants::DELETE) ? $self->get_dad_elements($action,$opt):[],
…

seems on a delete I was not sending down any elements to the DAD and hence the the error for an empty 'elements' array-ref.

Now this got me thinking, it is not really the place of the Database::Driver to limit what is sent down for action by the DAD, who knows there may be a DB out there someplace that needs the fields for a delete so I think I can just remove that code and while I am at it I see I have the same sort of code for sorts so I will drop that as well;

       my $dad = $driver->new(

            {

                view               => $self->view,

--              elements           => ($action ne Database::Accessor::Constants::DELETE) ? $self->get_dad_elements($action,$opt):[],

++                elements           =>$self->get_dad_elements($action,$opt),

                conditions         => [@{$self->conditions},@{$self->dynamic_conditions}],

                links              => [@{$self->links},@{$self->dynamic_links}],

                gather             => $gather,

--              sorts              => ($action eq Database::Accessor::Constants::RETRIEVE) ? [@{ $self->sorts }  ,@{ $self->dynamic_sorts   }] : [],

++               sorts              =>[@{ $self->sorts }  ,@{ $self->dynamic_sorts   }] ,


                da_compose_only    => $self->da_compose_only,

                da_no_effect       => $self->da_no_effect,

                da_warning         => $self->da_warning,

                da_raise_error_off => $self->da_raise_error_off,

                

            }

        );

 

and now I am getting a full pass on that test case. So onto other things.