byterock

Its re-think things day here in the Moose-Pen

It seems I am getting close to finishing off Database::Accessor and Driver::DBI. I have a very complete test suite for both and an extensive practical test suite for Driver::DBI and all three are running at 100% pass. I should be happy.

Well I am not. The practical testing did show me one major API change that I should make.

If one of the main advantages of Database::Accessor is that it can protect the data in your DB from injection attacks and from backdoor queries that can find things out about your Data or even change it.

It is serves to reason that anyone who is going to try and do this sort of malfeasance at the code level really wouldn't bother to use Datadate::Accessor. Given that the calling format for Database::Accessor is;

$da->retrieve($db_connection);

The '$db_connection' is right there so what is stopping a programmer from doing this

$db_connection->('delete * from users');

Funny that never stood out for me until now. I guess it never stood out for anyone else as I have gotten this far without anyone pointing it.

Therefore I was thinking to take that '$db_connection' out off there and place it in Database::Accessor as a private attribute.

Now this change will invalidate 99.987% of my tests and a good deal or re-writing. Anyone want to have another years work of Database::Accesor posts??.

I could just leave things as is and if the end user wanted to bury the $dbh some-place else in the code they could try this;

package Xtest::DA::Address;

use Database::Accessor;

use Moose;

use MooseX::Privacy;


around BUILDARGS => sub {

        my $self = shift;

        my $orig  = shift;

        my $class = shift;

        my $ops   = shift(@_);

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

...

      $ops->{da} = $da;

      my $dbh = My::DBIx::Connector->dbh();

      $ops->{dbh} = $dbh;

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

};

…

private_method da => (

    is      => 'ro',

    isa     => 'Database::Accessor',

);

private_method dbh => (

    is      => 'ro',

    isa     => 'Database::Accessor',

);

sub retrieve {

 my $self = shift;

 $self->da->retrieve($self->dbh);

}

...

Now things are not looking much better as the above is setting me up for a very large anti-patten if I have to create new 'CRUD' methods for each of the classes that I want to use Database::Accessor on.

A real sticky wicket when something like this pop-ups so late in the game. It did get me to re-read my initial POD for Accessor and I do make this point there;

'Though it can be used directly it is best used within another abstracted class as below ...'

I think what I have to do is greatly expand on that one line above and add in more details on how Database::Accessor can protect access to your DB when in an abstracted class.

I also went out an looked a few projects that used the original DBIx::DA to remind myself how it was used in production. I looked at a number of differing projects and never saw DBIx::DA being called directly it was always wrapped in another class in some way.

Ok I am going to keep the API as it is for now but I will make sure to add into the POD a much larger section on why it should not use it directly.