deprecate no-arg, 1-result model()/view() calls; croak on no-arg, multi-result calls.

diff --git a/Changes b/Changes
index b17fa35..07778eb 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,17 @@
 # This file documents the revision history for Perl extension Catalyst.
 
+TO BE RELEASED
+
+  Deprecated features:
+   - Calling model() and view() with no args when only one model or view is
+     available has been deprecated. You should explicitly pass the name of 
+     the component.
+
+  Removed features:
+   - Calling model() and view() with no args when more than one model or view
+     was available no longer returns a random model/view and will croak() 
+     instead.
+
 5.90007 - 2011-11-22 20:35:00
 
   New features:

diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm
index 6c32146..faf1da5 100644 (file)
--- a/lib/Catalyst.pm
+++ b/lib/Catalyst.pm
@@ -676,15 +676,19 @@ sub model {
 
     my( $comp, $rest ) = $c->_comp_search_prefixes( undef, qw/Model M/);
 
-    if( $rest ) {
-        $c->log->warn( Carp::shortmess('Calling $c->model() will return a random model unless you specify one of:') );
-        $c->log->warn( '* $c->config(default_model => "the name of the default model to use")' );
-        $c->log->warn( '* $c->stash->{current_model} # the name of the model to use for this request' );
-        $c->log->warn( '* $c->stash->{current_model_instance} # the instance of the model to use for this request' );
-        $c->log->warn( 'NB: in version 5.81, the "random" behavior will not work at all.' );
-    }
-
-    return $c->_filter_component( $comp );
+    if( !$rest ) {
+        ( my $name = ref $comp ) =~ s{$appclass\::M(odel)?::}{};
+        $c->log->warn( Carp::shortmess('Calling $c->model() with no arguments has been deprecated and will be removed.') );
+        $c->log->warn( "You could change the method call to \$c->model('$name') to retrieve this component." );
+        return $c->_filter_component( $comp );
+    }
+
+    croak( join( "\n", 
+        'Calling $c->model() will fail unless you specify one of:',
+        '* $c->config(default_model => "the name of the default model to use")',
+        '* $c->stash->{current_model} # the name of the model to use for this request',
+        '* $c->stash->{current_model_instance} # the instance of the model to use for this request' )
+    );
 }
 
 
@@ -740,15 +744,19 @@ sub view {
 
     my( $comp, $rest ) = $c->_comp_search_prefixes( undef, qw/View V/);
 
-    if( $rest ) {
-        $c->log->warn( 'Calling $c->view() will return a random view unless you specify one of:' );
-        $c->log->warn( '* $c->config(default_view => "the name of the default view to use")' );
-        $c->log->warn( '* $c->stash->{current_view} # the name of the view to use for this request' );
-        $c->log->warn( '* $c->stash->{current_view_instance} # the instance of the view to use for this request' );
-        $c->log->warn( 'NB: in version 5.81, the "random" behavior will not work at all.' );
+    if( !$rest ) {
+        ( my $name = ref $comp ) =~ s{$appclass\::V(iew)?\::}{};
+        $c->log->warn( Carp::shortmess('Calling $c->view() with no arguments has been deprecated and will be removed.') );
+        $c->log->warn( "You could change the method call to \$c->view('$name') to retrieve this component." );
+        return $c->_filter_component( $comp );
     }
 
-    return $c->_filter_component( $comp );
+    croak( join( "\n", 
+        'Calling $c->view() will fail unless you specify one of:',
+        '* $c->config(default_view => "the name of the default view to use")',
+        '* $c->stash->{current_view} # the name of the view to use for this request',
+        '* $c->stash->{current_view_instance} # the instance of the view to use for this request' )
+    );
 }
 
 =head2 $c->controllers

diff --git a/t/aggregate/unit_core_mvc.t b/t/aggregate/unit_core_mvc.t
index b04c3a3..a910c0b 100644 (file)
--- a/t/aggregate/unit_core_mvc.t
+++ b/t/aggregate/unit_core_mvc.t
@@ -1,4 +1,4 @@
-use Test::More tests => 51;
+use Test::More tests => 48;
 use strict;
 use warnings;
 
@@ -61,12 +61,8 @@ is_deeply( [ sort MyMVCTestApp->models ],
            'models ok');
 
 {
-    my $warnings = 0;
-    no warnings 'redefine';
-    local *Catalyst::Log::warn = sub { $warnings++ };
-
-    like (MyMVCTestApp->view , qr/^MyMVCTestApp\::(V|View)\::/ , 'view() with no defaults returns *something*');
-    ok( $warnings, 'view() w/o a default is random, warnings thrown' );
+    eval { MyMVCTestApp->view; };
+    ok( $@, 'view() with no defaults and multiple choices dies' );
 }
 
 is ( bless ({stash=>{current_view=>'V'}}, 'MyMVCTestApp')->view , 'MyMVCTestApp::View::V', 'current_view ok');
@@ -78,17 +74,8 @@ is ( bless ({stash=>{current_view_instance=> $view, current_view=>'MyMVCTestApp:
   'current_view_instance precedes current_view ok');
 
 {
-    my $warnings = 0;
-    no warnings 'redefine';
-    local *Catalyst::Log::warn = sub { $warnings++ };
-
-    ok( my $model = MyMVCTestApp->model );
-
-    ok( (($model =~ /^MyMVCTestApp\::(M|Model)\::/) ||
-        $model->isa('Some::Test::Object')),
-        'model() with no defaults returns *something*' );
-
-    ok( $warnings, 'model() w/o a default is random, warnings thrown' );
+    eval { MyMVCTestApp->model; };
+    ok( $@, 'model() with no defaults and multiple choices dies' );
 }
 
 is ( bless ({stash=>{current_model=>'M'}}, 'MyMVCTestApp')->model , 'MyMVCTestApp::Model::M', 'current_model ok');

diff --git a/t/lib/TestAppOneView/Controller/Root.pm b/t/lib/TestAppOneView/Controller/Root.pm
index 61a9bf5..44b0222 100644 (file)
--- a/t/lib/TestAppOneView/Controller/Root.pm
+++ b/t/lib/TestAppOneView/Controller/Root.pm
@@ -8,7 +8,13 @@ __PACKAGE__->config->{namespace} = '';
 sub view_no_args : Local {
     my ( $self, $c ) = @_;
 
-    my $v = $c->view;
+    my $v;
+    {
+        # silence warning for test suite
+        no warnings 'redefine';
+        local *Catalyst::Log::warn = sub { };
+        $v = $c->view;
+    }
 
     $c->res->body(Scalar::Util::blessed($v));
 }