byterock

Still API clean up day here in the Moose-Pen

It has been a long running battle between myself,m the API and the the 'Link' objects. A number of posts over the past few weeks have been dealing with how to handle the 'view' for the various elements on link.

Today I started down the path of enforcing the API rule that all 'elements' in a link must have a view. So given this link;

      links => {

                type       => 'LEFT',

                to         => { name => 'address' },

                conditions => [

                    {

                        left  => { name => 'id',

                                    },

                        right => {

                            name => 'user_id',

                            view => 'address'

                        }

                    }

                ]

            },

I should get a die as the 'left' in the first condition does not have a view. In the end I did not like this as it was inconstant with the other API parts of Database::Accessor. There fore I am going to modify the API so that in a 'Link' the 'To' view is the one that is assigned rather than the primary 'view'

Of course I will need a test first and I think I can re-use 11_link.t for my evil purposes.

First I will change what I am passing into the Link

my $link = Database::Accessor::Link->new(

    {

        type       => 'left',

        to         => { name => 'test' },

        conditions => [

            {

                left => {

                    name => 'field-1',

                    view => 'table-1'

                },

                right => {

                    name => 'field-2',

--                 view => 'table-1'

                },

                operator => '='

            }

        ]

    }

);

Now I just need a check to see that the 'view' on that 'right' comes out as 'test';

ok( $link->conditions()->[0]->predicates()->right()->view eq 'test',

    "Right view is test " );

I will also have to add another test to check what happenes when an alias is present on the 'to';

  

$link = Database::Accessor::Link->new(

    {

        type       => 'left',

        to         => { name => 'test',

                             alias=> 'test_2' },

        conditions => [

            {

                left => {

                    name => 'field-1',

                    view => 'table-1'

                },

                right => {

                    name => 'field-2',

                },

                operator => '='

            }

        ]

    }

);


ok( $link->conditions()->[0]->predicates()->left()->view eq 'table-1',

    "Left view is table-1" );

ok( $link->conditions()->[0]->predicates()->right()->view eq 'test_2',

    "Right view is test_2" );

Now to code for the above. Unlike all my previous work with views and elements I am going to do this one directly on the 'Link' class by checking and changing the 'build' arguments sent down to the class. Moose lets us do that with the 'around BUILDARGS' call; Here is my first take on it;

    package 

        Database::Accessor::Link;

    use Moose;

    extends 'Database::Accessor::Base';


    sub _check_elements {

        my $self = shift;

        my ( $ops, $view_name ) = @_;

        if ( exists( $ops->{name} ) ) {

            $ops->{view} = $view_name;

        }

        else {

            $self->_check_elements( $ops->{right}, $view_name )

              if exists( $ops->{right} );

            $self->_check_elements( $ops->{left}, $view_name )

              if exists( $ops->{left} );

        }

    }

    around BUILDARGS => sub {

        my $orig  = shift;

        my $class = shift;

        my $ops   = shift(@_);

        my $view_name = $ops->{to}->{name};

        $view_name = $ops->{to}->{alias}

          if ( exists( $ops->{to}->{alias} ) );

        foreach my $condition ( @{ $ops->{conditions} } ) {

            $class->_check_elements( $condition->{left},  $view_name );

            $class->_check_elements( $condition->{right}, $view_name );

        }

        return $class->$orig($ops);

    };

Again I have taken an recursive approach to handling this. I first loop around all the 'conditions' on the 'link' calling the '_check_elements' sub for both 'left' and 'right' attributes. In that sub I check to see if the passed in item is an 'Element' by looking for the 'name' key and then I set the 'view' otherwise I call the sub again on the 'left' and 'right' if they exists.

On the first run I get;

…

ok 9 - Right view is test 

not ok 10 - Left view is table-1

ok 11 - Right view is test_2

A little problem here as that 10 is checking this

     left => {

                    name => 'field-1',

                    view => 'table-1'

                },

and that should remain 'table-1' maybe I was a little aggressive on my application. This little patch should fix that;

 if ( exists( $ops->{name} ) ) {

            $ops->{view} = $view_name

++               unless($ops->{view});

        }

        else {

and it does;

…

ok 9 - Right view is test 

ok 10 - Left view is table-1

ok 11 - Right view is test_2

Now that looks ok but before I finish for the I am going to take out a little code first from the '_elements_check' sub

  foreach my $link ((@{ $self->links },@{ $self->dynamic_links })){
++          my $view = $link->to;
++            $self->_check_element($link->conditions,0,$view->name);
           push(@items,$link->conditions);
        } 

and I am going to the '$alias' out of the params on the '_check_element' sub

        my $self = shift;
--      my ($element,$right, $alias) = @_;
++        my ($element,$right) = @_;

but there is no reason to put all those changes here.

Well onto something else now.