byterock

Its nip a bug day here in the Moose-Pen.

Over the past few days I have been slowly cleaning up my tests and now I am going to fix one more little problem I was having with Database::Accessor; Setting the 'view/table' of an element/field that is being passed down into a DAD.

I do have this working for most of the parts that I am sending down to the DAD but I am still missing this functionality on the 'sort/order by' so lets look at that test as a start;

my $tests = [{

    key  =>'sorts',

    sorts => [

             {name => 'last_name',

            },

            {

              name => 'first_name',

            },

            ],

    caption => "Order by ",

    sql     => "SELECT people.first_name, people.last_name, people.user_id FROM people ORDER BY people.last_name, people.first_name",

}];

This time I have taken the 'view' off the two elements in the sort so it will fail with this SQL

"ORDER BY .last_name, .first_name",

simple enough to fix by just adding in a little more iteration but I want to make my app a little better designed that just brute force each time I have to do something.

One thing I included Accessor.pm was a check for properly closed parentheses which I currently only do on the 'conditions' attributes, which is only partial coverage for this check as one can have parentheses anywhere an 'element', 'expression' or 'function' can be used. A good example is this valid SQL clause;

'ORDER BY (pay/tax_rate)+100-(overage*1.02)'

This means I have to that a parentheses check on all my attributes and the only way to accomplish this is to iterate over all of them and while I am doing that I might as well do the '_view' check as well.

The first thing I have to do is modify the '_parentheses_check' sub so it will go over all of the Accesspr.pm attributes and that is acompliehd by gathering them all up in an array;

   private_method _parentheses_check => sub {

        my $self = shift;

        my ($action) = @_;

        $self->_reset_parens();

--        $self->_count_parentheses( @{ $self->conditions },

--            @{ $self->dynamic_conditions } ); 

++        my @items;

++        push(@items,@{ $self->conditions }

++            ,@{ $self->dynamic_conditions },

++            ,@{ $self->sorts}

++            ,@{ $self->dynamic_sorts}

++            ,@{ $self->elements}

++            );

++        if ($self->gather()){

++            push(@items,(@{$self->gather->conditions}

++                        ,@{ $self->gather->elements}));

++        }  

++        $self->_count_parentheses(@items);

  

        die " Database::Accessor->"

          . lc($action)

          . " Unbalanced parentheses in your conditions and dynamic_conditions. Please check them!"

          if ( $self->_parens_are_open() );

…

The only thing specail with the above is I have to check to see if there is a 'Gather' class set on the DA and then use the conditions and elements from that class.

Next I had to make a change in the '_count_parentheses' sub by getting rid of it and adding it back into the '_parentheses_check' sub as I will not be using it anywhere else. As I did this I did a quick modification to so it will handle more that just 'conditions'

--       $self->_count_parentheses(@items);

        foreach my $condition (@items) {

            if ( $condition->can('predicates') ) {

                my $predicate_count = 0;

                foreach my $predicate ( @{ $condition->predicates() } ) {

                    $self->_inc_parens()

                      if ( $predicate->open_parentheses() );

                    $self->_dec_parens()

                      if ( $predicate->close_parentheses() );

                    $predicate->condition(Database::Accessor::Constants::AND)

                      if ( $predicate_count and !$predicate->condition() );

                    $predicate_count = 1;

                    $self->check_view( $predicate->right )

                      if (

                        ref( $predicate->right ) eq

                        'Database::Accessor::Element' );

                    $self->check_view( $predicate->left )

                      if (

                        ref( $predicate->left ) eq

                        'Database::Accessor::Element' );

                }

            }

            else {

                $self->check_view($condition);

            }

        }

You might notice if you look at the change history I also fixed only other but in the above code. Originally I had;

 foreach my $predicate ( @{ $condition->{predicates} } ) {

which should have be;

 foreach my $predicate ( @{ $condition->predicates() } ) {

again the same problem I had in another post, old style direct call of an attribute, I guess old coding habits are hard to break, at least my re factoring found it.

I can even get rid of more code as this call

 $self->check_view($element);

in the 'get_dad_elements' sub is no longer needed. Now it is time to get greedy as I noticed that on my 'execute' sub I had

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

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

which I think I can now replace with just

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

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

and it worked so that is another sub gone 'check_predicates' and more iteration saved.

Now to have a look at my test suite and see what needs updating there, but that is another post.