I expressed some doubts about various aspects of the refactoring work on
Maypole.pm. So here's a concrete suggestion for one of those aspects -
refactoring is_applicable.
I think the goal was to make the code more readable by changing
is_applicable to return a truth value instead of an Apache return code.
But the proposal was to add an extra subroutine, which IMHO doesn't move
us closer to that goal. I think the attached patch does move us towards
the goal and also fixes a couple of other niggles in the method.
Cheers, Dave
--- /usr/lib/perl5/site_perl/5.8.6/Maypole.pm 2005-09-17 15:28:15.000000000
+0100
+++ Maypole.pm 2005-10-05 06:49:39.000000000 +0100
@@ -7,7 +7,7 @@
use Maypole::Constants;
use Maypole::Headers;
-our $VERSION = '2.10';
+our $VERSION = '2.10 + is_model_applicable';
__PACKAGE__->mk_classdata($_) for qw( config init_done view_object );
__PACKAGE__->mk_accessors(
@@ -82,8 +82,8 @@
my $r = shift;
$r->model_class( $r->config->model->class_of( $r, $r->{table} ) );
- my $applicable = $r->is_applicable;
- unless ( $applicable == OK ) {
+ my $applicable = $r->is_model_applicable;
+ unless ( $applicable ) {
# It's just a plain template
delete $r->{model_class};
@@ -109,7 +109,7 @@
# We run additional_data for every request
$r->additional_data;
- if ( $applicable == OK ) {
+ if ( $applicable ) {
eval { $r->model_class->process($r) };
if ( my $error = $@ ) {
$status = $r->call_exception($error);
@@ -133,23 +133,40 @@
else { return OK; }
}
-sub is_applicable {
+sub is_model_applicable {
my $self = shift;
+
+ # cater for applications that are using obsolete version
+ if ($self->can('is_applicable')) {
+ warn "DEPRECATION WARNING:
+ rewrite is_applicable to the interface of Maypole::is_model_applicable\n";
+ return $self->is_applicable == OK;
+ }
+
+ # Establish which tables should be processed by the model
my $config = $self->config;
$config->ok_tables || $config->ok_tables( $config->display_tables );
$config->ok_tables( { map { $_ => 1 } @{ $config->ok_tables } } )
if ref $config->ok_tables eq "ARRAY";
- warn "We don't have that table ($self->{table}).\n"
- . "Available tables are: "
- . join( ",", @{ $config->{display_tables} } )
- if $self->debug
- and not $config->ok_tables->{ $self->{table} }
- and $self->{action};
- return DECLINED() unless exists $config->ok_tables->{ $self->{table} };
-
- # Is it public?
- return DECLINED unless $self->model_class->is_public( $self->{action} );
- return OK();
+ my $ok_tables = $config->ok_tables;
+
+ # Does this request concern a table to be processed by the model?
+ my $table = $self->table;
+ warn "We don't have that table ($table).\n" .
+ "Available tables are: " . join( ',', keys %$ok_tables )
+ if $self->debug and not $ok_tables->{ $table };
+ return 0 unless exists $ok_tables->{ $table };
+
+ # Is the proposed action a public method of the model?
+ if ($self->model_class->is_public( $self->action )) {
+ return 1 ;
+ }
+ else {
+ warn 'The action ' . $self->action .
+ 'is not applicable to the table ' . $table
+ if $self->debug;
+ return 0;
+ }
}
sub call_authenticate {
@@ -372,11 +389,17 @@
=head3 is_applicable
-Returns a Maypole::Constant to indicate whether the request is valid.
+This method is deprecated as of version 2.11. If you have overridden it,
+please override C<is_model_applicable> instead
+(i.e. change the return type from Maypole:Constants to true/false).
+
+=head3 is_model_applicable
+
+Returns true or false to indicate whether the request is valid.
-The default implementation checks that C<$r-E<gt>table> is publicly
-accessible
-and that the model class is configured to handle the C<$r-E<gt>action>
+The default implementation checks that C<<$r->table>> is publicly
+accessible and that the model class is configured to handle the
+C<<$r->action>>.
=head3 authenticate
