[Dbix-class] Enforcing read-only objects

[Ian Docherty (icydee)]

On 11/08/2010 20:59, Bill Moseley wrote:
<snip>

>
> So, just to be clear, you are suggesting in each business layer method
> that might update do something like:
>
> sub set_active {
>       my $foo = shift;
>
>       die "$foo is readonly" if $foo->is_readonly;
>
>       $foo->active( 1 );
>       $foo->update;
> }
>
> If that's not what you meant then could you show a simple code example?

This example looks like a function acting on a DBIx::Class row object.

If you are going to do this then better to create methods to do it. e.g.

---
package MyApp::DB::Result::Label;

sub set_active {
     my ($self) = @_;

     die "Is Readonly" if $self->is_readonly;
     $self->active(1);
     $self->update;
}

---
(or perhaps that was what you really meant?)
Either way you are exposing your ORM layer to your application code.

This is not 'wrong' and so long as you only use the result or resultset 
methods that you create for your business logic then it can work.

>
>
> In the past we had code like that, but it turns out it sometimes the
> tests are forgotten.  Thus, we were looking for a way to enforce the
> access control.   So, if every object has an "is_readonly" method and
> update() is a common method used by all other methods, then the update
> method is a central place to test instead of repeating the same code in
> many methods.

When you say 'the tests are forgotten' I assume you mean that people 
accessed and modified the DBIC objects directly from within the 
application? (For example, by forgetting to call the $foo->set_active 
method but calling the $foo->active(1) method instead?)

You could try encapsulating the DBIx::Class objects in some business 
objects. The programmers would then have less opportunity to make this 
mistake. You do rather lose the flexibility of having direct access to 
your ORM but the benefit is that you end up with a smaller set of 
business layer methods that actually mean something to your business and 
which can be tested more easily.

I have done exactly this, but it is difficult to give a consise example 
since the business logic was rather complex.

There would not necessarily be a direct one-one relationship between 
your business layer objects and the DBIC objects. In your example you 
probably want a 'Label' business object that would access the 'label', 
'user_access' and 'users' tables to carry out it's magic.

A further advantage is that you are protecting your application from 
changes made to your database (for example normalising the data) which 
would otherwise require application code changes. You would just make a 
change to the business layer to compensate leaving the interface to the 
application unchanged. (which also answers your earlier question on 
20/4/2010 about alias names for columns)

Ignoring for the moment the artists, cds, tracks etc. I could see it 
something like the following. (top of my head, first impressions, 
probably totally borked)

In your application code.
----

my $factory = MyApp::Business->new({schema => $schema});
my $label   = $factory->find_label($id);

$label->set_active;

----
The factory code might have something like the following
----
package MyApp::Business;
use Moose;

has 'schema' => (is => 'ro', required => 1);

sub find_label {
     my ($self, $id) = @_;

     return $self->schema->resultset('DB::Label')->find($id);
}
----
Your Label class would be something like.
----
package MyApp::Business::Label;
use Moose;

has 'dbic_obj' => (is => 'ro', required => 1);

sub set_active {
     my ($self) = @_;

     die "Is Readonly" if $self->dbic_obj->is_readonly;

     $self->dbic_obj->active( 1 );
     $self->dbic_obj->update;
}
----

This is not all that different from your example, but it would be fairly 
obvious if someone was accessing the ORM directly from your application 
since you would see the use of the 'dbic_obj' accessor.

Without too much trouble it should be possible to make the 'dbic_obj' a 
private attribute so preventing it's use directly in the application.

Note. The above code is totally untested and likely to be broken and is 
only an indication of the general method! Please don't flame me for 
syntax errors but feel free to suggest alternative ways of doing this. I 
am still learning too!