[Catalyst] Making Controllers thin and Models thick

[Kee Hinckley]

[Was: Re: [Catalyst] Command-line utility using Controller - examples?]

On Jul 17, 2007, at 9:21 AM, Ronald J Kimball wrote:
> (The Controller in this case is probably not as thin as it should  
> be. :)

The issue I had is that Catalyst makes it very easy to do the  
database part of the Model, but it's not immediately obvious where to  
put the rest of the business logic.  So it ends up going in the  
Controller.

I finally resolved that by taking advantage of the ability to specify  
the return class of the ResultSet.  That allows me to create a custom  
class for each table (along with a common super-class for generic and  
cross-table stuff).  The Controller has easy access to the ResultSet  
class, and the ResultSet class has easy access to the database, so it  
seemed like an ideal place to put the heavy logic.

The outline follows below.  I'd be interested in what people think of  
it, since this is my first Catalyst app.

----------
#
# Define the database portion of the Model
#
package Somewhere::Schema::MDB::Persona;
use base qw/Somewhere::Schema::Super::Class/;
__PACKAGE__->load_components(qw/ UTF8Columns Core /);
__PACKAGE__->table('persona', extra => { mysql_charset => 'utf8' });
__PACKAGE__->add_columns(
  ...
#
# Note that if you are defining the DB here, it's handy to have some  
helper routines for common types like
# boolean and foreign-key
#
...
#
# Now the critical part.  Specify that the result set returned for  
this table has the following class.
#
__PACKAGE__->resultset_class(__PACKAGE__ . '::ResultSet');


#
# Now define the business logic of the Model,
#
# This could be in a separate file, but it seemed to make sense to  
keep it here, so I just keep right on going.
# That makes it real easy to update the biz-logic if the db changes.
#
package Somewhere::Schema::MDB::Persona::ResultSet;
use base 'Somewhere::Schema::Super::ResultSet';
use Error qw(:try);
use Error::Throws;
use Somewhere::Util;
use NEXT;
throws ::Error::InUse, -format => 'The [_1] "[_2]" is already in use.';

------------

Now I can add helper methods to be called by the Controller.  And  
since they are attached to the ResultSet, they have all the necessary  
context to do their work.  It should never be necessary to pass $c to  
them, since that would break their independence. In addition, I can  
override update/insert to add validation.  Because really the  
Controller should only be doing validation specific to the View-- 
things like checking to make sure that both of the password fields  
match when the user enters their initial password.  Checking for  
things like field length and correct types of data belong in the Model.

Another way to look at it is to consider any non-web utility programs  
you write.  Presumably you can't trust them not to make mistakes  
either--but they won't be calling the controller, so....  In fact, I  
define a separate validate function in the Model for each table.  The  
Controller calls it before calling the actual create/update  
operations.  By default it continues checking everything even if it  
gets an error, and then it returns a list of errors.  Each error is  
flagged with the field or fields it applies to.  So the Controller  
can pass the error objects back to the View, which then uses the  
field info to highlight the appropriate form fields.  If the  
validation routine returns no errors, then the create/update  
operation is called.  It calls the validate operation *again* (don't  
rely on the caller doing the right thing!), but this time telling it  
to throw an exception as soon as it sees any error.

Also on the trust side, it's the Controller's responsibility to  
ensure that only the appropriate arguments are passed to the Model.   
No passing every single variable given by the web browser to the  
view.  Aside from security issues, this also provides an opportunity  
to remap argument names if necessary (E.g. db changes, ui hasn't).

So here's a piece of Controller code for doing signup for a service.

------------------
sub signup : Local {
     my ($self, $c) = @_;
     my (@errors);

     # If the form hasn't been submitted yet, just display it
     if (!$c->request->params->{'-signup'}) {
	$c->template('to/signup.html');
	return 1;
     }

     # Set some shortcut variables
     my $params = $c->request->params();
     my $acctparams = $params->{account};

     # Check to see if the user typed things correctly
     if ($acctparams->{email} ne $acctparams->{email2}) {
	push(@errors, record Error::DoubleCheck(
	    -fields	=> [qw( account.email account.email2 )],
	    -text	=> 'email addresses',
	));
     }
     if ($acctparams->{password} ne $acctparams->{password2}) {
	push(@errors, record Error::DoubleCheck(
	    -fields => [qw( account.password account.password2 )],
	    -text	=> 'passwords',
	));
     }

     #
     # All the other error checking needs to be handled at the  
bizlogic level.
     #

     # $persona and $account are result sets, I've started improving  
my naming conventions, but
     # haven't backfilled to this file yet.
     my ($persona, $account);
     my ($acctslice, $persslice);

     # Pull out only the required fields
     $acctslice = hashslice($params->{account}, qw( login password  
email ));
     $persslice = hashslice($params->{persona}, qw( name uri ));

     # Get a handle on the resultset classes
     $persona = $c->model('MDB::Persona');
     $account = $c->model('MDB::Account');

     # Validate everything and store the errors.  -fieldprefix is  
used to tell the validator how to munge
     # field names to that the View will understand them.  -action  
specifies the kind of checking to be done
     push(@errors, $account->validate($acctslice, -fieldprefix =>  
'account.', -action => 'create'));
     push(@errors, $persona->validate($persslice, -fieldprefix =>  
'persona.', -action => 'create'));

     if (!@errors) {
         # If there are no errors, try and do the work.  At some  
point I need to make a DB-specific version
         # of try that handles transactions automatically.
	try {
	    $account->txn_do(sub {	
		my $acct = $account->create($acctslice);
		my $pers = $acct->create_related('personas', $persslice);
	    });

             # If we made it here, there were no errors, log the user  
in.  The login action will do a redirect.
	    $c->forward('login', $acctslice->{login}, $acctslice->{password})
		or throw Error::Unexpected(qq/The account "$acctslice->{login}" was  
created, but we were unable to log you in./);
	    return 1;
	} otherwise {
             # Something went wrong, push the error on the list.
	    push(@errors, $@);
	} default Somewhere::Controller::To::Error::Unexpected;	#TODO Make  
shortcuts work with default
     }

     # This helper routine will merge these errors with any to be  
found in $c->error().  Save them in the stash for
     # the view to handle, clear them, and then set the template to  
the provided name.  (A second template name
     # can be provided for cases when there are no errors, but this  
time we know there are.)
     return $c->cleanup(\@errors, 'to/signup.html');
}
------------------

and here's the validate routine for account.


------------------
sub validate {
     my $self = shift;
     my ($params, %options) = @_;
     my (@errors, $fast);

     #TODO Does DBix::Class provide anything for validation?
     #TODO If not, make something that at least allows us to do  
length/type validation automatically

     # The -fast option tells us to bail out as soon as we see an  
error.  If -throw is set, we'll throw rather
     # than return, but our superclass will take care of that.
     $fast = $options{-fast};
     do {
         # If we're doing a create, make sure we have all the  
required non-empty fields.
	if ($options{-action} eq 'create') {
	    if (my @missing = hasvalues($params, qw(login password email))) {
		push(@errors, record Error::Required(-args => ['create', join(', ',  
@missing)], -fields => \@missing));
		last if ($fast);
	    }
	}
	# TODO When we validate update's we'll need to diff between  
identifying fields and ones to change

         # Make sure the login and email address aren't already in use.
	if ($self->single({ login => $params->{login} })) {
	    push(@errors, record Error::InUse(-args => ['account name',  
$params->{login}], -fields => ['login']));
	    last if ($fast);
	}
	if ($self->single({ email => $params->{email} })) {
	    push(@errors, record Error::InUse(-args => ['email address',  
$params->{email}], -fields => ['email']));
	    last if ($fast);
	}

     };
     # Let the superclass do the necessary cleanup work.
     return $self->NEXT::validate($params, %options, -errors =>  
\@errors);
}
------------------

The validate routine in Super::ResultSet does some generic work, and  
then this.


------------------
     if (@errors && ($options{-throw})) {
	$errors[0]->rethrow() if (@errors == 1);
	throw Error::Multiple(-args => [@errors]);
     }
     return @errors;
------------------

Just to take things all the way back to the view, here's the code  
that handles flagging form fields that correspond to errors.  Note  
that I'm using Embperl::Object for the View. (I have a complete  
rewrite of the plugin which has far tighter integration than the one  
on CPAN.  I hope to release that in a few weeks.)  The following code  
assumes the Prototype javascript library has been loaded.   
Technically this should be called "onload", but I haven't gotten  
around to it yet, and it will (usually) work if added after the form  
in question has been completely displayed.

------------------
<script lang="javascript">
     [$ if ($errors) $]
	[$ foreach my $err (@$errors) $]
	    [$ foreach my $f (@{ $err->{-fields} }) $]
		$("[+ $f +]").addClassName('error-field');
	    [$ endforeach $]
	[$ endforeach $]
     [$ endif $]
</script>
------------------

Comments?  Thoughts?