From: André Walker Date: Thu, 7 Jul 2011 22:27:18 +0000 (-0300) Subject: removed component resolution regex fallback X-Git-Tag: 5.80033~10 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=f723794a21b143f495fc00a4584efcfee4c14fa7 removed component resolution regex fallback --- diff --git a/Changes b/Changes index cecff5e..a365537 100644 --- a/Changes +++ b/Changes @@ -1,5 +1,9 @@ # This file documents the revision history for Perl extension Catalyst. + Deprecations: + - Removed component resolution regexp fallback. Now it always warns + when the component is not found, and returns undef. + Bug fixes: - Fix the disable_component_resolution_regex_fallback config setting to also work in the $c->component method. diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm index 9e41ec6..d5f5ec8 100644 --- a/lib/Catalyst.pm +++ b/lib/Catalyst.pm @@ -560,48 +560,9 @@ sub _comp_names_search_prefixes { return @result if @result; - # if we were given a regexp to search against, we're done. - return if ref $name; - - # skip regexp fallback if configured - return - if $appclass->config->{disable_component_resolution_regex_fallback}; - - # regexp fallback - $query = qr/$name/i; - @result = grep { $eligible{ $_ } =~ m{$query} } keys %eligible; - - # no results? try against full names - if( !@result ) { - @result = grep { m{$query} } keys %eligible; - } - - # don't warn if we didn't find any results, it just might not exist - if( @result ) { - # Disgusting hack to work out correct method name - my $warn_for = lc $prefixes[0]; - my $msg = "Used regexp fallback for \$c->${warn_for}('${name}'), which found '" . - (join '", "', @result) . "'. Relying on regexp fallback behavior for " . - "component resolution is unreliable and unsafe."; - my $short = $result[0]; - # remove the component namespace prefix - $short =~ s/.*?(Model|Controller|View):://; - my $shortmess = Carp::shortmess(''); - if ($shortmess =~ m#Catalyst/Plugin#) { - $msg .= " You probably need to set '$short' instead of '${name}' in this " . - "plugin's config"; - } elsif ($shortmess =~ m#Catalyst/lib/(View|Controller)#) { - $msg .= " You probably need to set '$short' instead of '${name}' in this " . - "component's config"; - } else { - $msg .= " You probably meant \$c->${warn_for}('$short') instead of \$c->${warn_for}('${name}'), " . - "but if you really wanted to search, pass in a regexp as the argument " . - "like so: \$c->${warn_for}(qr/${name}/)"; - } - $c->log->warn( "${msg}$shortmess" ); - } + $c->log->warn("Looking for '$name', but nothing was found."); - return @result; + return; } # Find possible names for a prefix @@ -832,12 +793,6 @@ should be used instead. If C<$name> is a regexp, a list of components matched against the full component name will be returned. -If Catalyst can't find a component by name, it will fallback to regex -matching by default. To disable this behaviour set -disable_component_resolution_regex_fallback to a true value. - - __PACKAGE__->config( disable_component_resolution_regex_fallback => 1 ); - =cut sub component { @@ -860,21 +815,9 @@ sub component { my( $comp ) = $c->_comp_search_prefixes( $name, qw/Model M Controller C View V/ ); return $c->_filter_component( $comp, @args ) if $comp; } - - return - if $c->config->{disable_component_resolution_regex_fallback}; - - # This is here so $c->comp( '::M::' ) works - my $query = ref $name ? $name : qr{$name}i; - - my @result = grep { m{$query} } keys %{ $c->components }; - return map { $c->_filter_component( $_, @args ) } @result if ref $name; - - if( $result[ 0 ] ) { - $c->log->warn( Carp::shortmess(qq(Found results for "${name}" using regexp fallback)) ); - $c->log->warn( 'Relying on the regexp fallback behavior for component resolution' ); - $c->log->warn( 'is unreliable and unsafe. You have been warned' ); - return $c->_filter_component( $result[ 0 ], @args ); + else { + my @result = grep { m{$name} } keys %{ $c->components }; + return map { $c->_filter_component( $_, @args ) } @result; } # I would expect to return an empty list here, but that breaks back-compat @@ -2926,14 +2869,6 @@ C - The default view to be rendered or returned when C<< $c->view =item * -C - Turns -off the deprecated component resolution functionality so -that if any of the component methods (e.g. C<< $c->controller('Foo') >>) -are called then regex search will not be attempted on string values and -instead C will be returned. - -=item * - C - The application home directory. In an uninstalled application, this is the top level application directory. In an installed application, this will be the directory containing C<< MyApp.pm >>. diff --git a/t/aggregate/unit_core_component.t b/t/aggregate/unit_core_component.t index 69ac6c0..8ef6051 100644 --- a/t/aggregate/unit_core_component.t +++ b/t/aggregate/unit_core_component.t @@ -1,4 +1,4 @@ -use Test::More tests => 22; +use Test::More; use strict; use warnings; @@ -30,31 +30,16 @@ is_deeply([ MyApp->comp('Foo') ], \@complist, 'Fallthrough return ok'); # regexp behavior { - is_deeply( [ MyApp->comp( qr{Model} ) ], [ 'MyApp::M::Model'], 'regexp ok' ); - is_deeply( [ MyApp->comp('MyApp::V::View$') ], [ 'MyApp::V::View' ], 'Explicit return ok'); - is_deeply( [ MyApp->comp('MyApp::C::Controller$') ], [ 'MyApp::C::Controller' ], 'Explicit return ok'); - is_deeply( [ MyApp->comp('MyApp::M::Model$') ], [ 'MyApp::M::Model' ], 'Explicit return ok'); - - # a couple other varieties for regexp fallback - is_deeply( [ MyApp->comp('M::Model') ], [ 'MyApp::M::Model' ], 'Explicit return ok'); + is_deeply( [ MyApp->comp( qr{Model} ) ], [ 'MyApp::M::Model' ], 'regexp ok' ); { my $warnings = 0; no warnings 'redefine'; local *Catalyst::Log::warn = sub { $warnings++ }; - is_deeply( [ MyApp->comp('::M::Model') ], [ 'MyApp::M::Model' ], 'Explicit return ok'); - ok( $warnings, 'regexp fallback warnings' ); - - $warnings = 0; - is_deeply( [ MyApp->comp('Mode') ], [ 'MyApp::M::Model' ], 'Explicit return ok'); + is_deeply( [ MyApp->comp('::M::Model') ], \@complist, 'no results for regexp fallback'); ok( $warnings, 'regexp fallback warnings' ); - - $warnings = 0; - is(MyApp->comp('::M::'), 'MyApp::M::Model', 'Regex return ok'); - ok( $warnings, 'regexp fallback for comp() warns' ); } - } # multiple returns @@ -86,8 +71,6 @@ is_deeply([ MyApp->comp('Foo') ], \@complist, 'Fallthrough return ok'); $c->component('M::Model', qw/foo2 bar2/); is_deeply($args, [qw/foo2 bar2/], 'args passed to ACCEPT_CONTEXT ok'); - - $c->component('Mode', qw/foo3 bar3/); - is_deeply($args, [qw/foo3 bar3/], 'args passed to ACCEPT_CONTEXT ok'); } +done_testing; diff --git a/t/aggregate/unit_core_mvc.t b/t/aggregate/unit_core_mvc.t index b04c3a3..7ef265d 100644 --- 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; use strict; use warnings; @@ -123,13 +123,13 @@ is ( MyMVCTestApp->model , 'MyMVCTestApp::Model::M', 'default_model in class met local *Catalyst::Log::warn = sub { $warnings++ }; # object w/ regexp fallback - is_deeply( [ MyMVCTestApp->model( 'Test' ) ], [ MyMVCTestApp->components->{'MyMVCTestApp::Model::Test::Object'} ], 'Object returned' ); + is_deeply( MyMVCTestApp->model( 'Test' ), undef, 'no regexp fallback' ); ok( $warnings, 'regexp fallback warnings' ); } - is_deeply( [ MyMVCTestApp->view('MyMVCTestApp::V::View$') ], [ 'MyMVCTestApp::V::View' ], 'Explicit return ok'); - is_deeply( [ MyMVCTestApp->controller('MyMVCTestApp::C::Controller$') ], [ 'MyMVCTestApp::C::Controller' ], 'Explicit return ok'); - is_deeply( [ MyMVCTestApp->model('MyMVCTestApp::M::Model$') ], [ 'MyMVCTestApp::M::Model' ], 'Explicit return ok'); + is( MyMVCTestApp->view('MyMVCTestApp::V::View$'), undef, 'no regexp fallback'); + is( MyMVCTestApp->controller('MyMVCTestApp::C::Controller$'), undef, 'no regexp fallback'); + is( MyMVCTestApp->model('MyMVCTestApp::M::Model$'), undef, 'no regexp fallback'); } { @@ -175,25 +175,6 @@ is ( MyMVCTestApp->model , 'MyMVCTestApp::Model::M', 'default_model in class met my $x = $c->view('V', qw/foo2 bar2/); is_deeply($args, [qw/foo2 bar2/], '$c->view args passed to ACCEPT_CONTEXT ok'); - # regexp fallback - $c->view('::View::V', qw/foo3 bar3/); - is_deeply($args, [qw/foo3 bar3/], 'args passed to ACCEPT_CONTEXT ok'); - - -} - -{ - my $warn = ''; - no warnings 'redefine'; - local *Catalyst::Log::warn = sub { $warn .= $_[1] }; - - is_deeply (MyMVCTestApp->controller('MyMVCTestApp::Controller::C'), - MyMVCTestApp->components->{'MyMVCTestApp::Controller::C'}, - 'controller by fully qualified name ok'); - - # You probably meant $c->controller('C') instead of $c->controller({'MyMVCTestApp::Controller::C'}) - my ($suggested_comp_name, $orig_comp_name) = $warn =~ /You probably meant (.*) instead of (.*) /; - isnt($suggested_comp_name, $orig_comp_name, 'suggested fix in warning for fully qualified component names makes sense' ); } { @@ -223,5 +204,6 @@ is ( MyMVCTestApp->model , 'MyMVCTestApp::Model::M', 'default_model in class met # try to get nonexisting object w/o regexp fallback is( MyApp::WithoutRegexFallback->controller('Foo'), undef, 'no controller Foo found'); - ok( !$warnings, 'no regexp fallback warnings' ); } + +done_testing;