reverted a previous change to Mojo::Loader that allowed for load_clas…

…s to be called on the same class multiple times, since it had very bad side effects
mojolicious · Sep 2, 2017 · 6017309 · 6017309
1 parent 19502a0
commit 6017309
Show file tree

Hide file tree

Showing 11 changed files with 67 additions and 64 deletions.
diff --git a/Changes b/Changes
@@ -1,6 +1,11 @@
 
-7.44  2017-08-20
+7.44  2017-09-02
+  - Reverted a previous change to Mojo::Loader that allowed for load_class to be
+    called on the same class multiple times, since it had very bad side effects.
   - Improved Mojo::IOLoop::TLS to use a little less memory.
+  - Fixed a bug where Mojolicious controllers like "MyApp::Controller::Foo::Bar"
+    would disappear if loading the controller "MyApp::Controller::Foo" was
+    attempted but failed.
 
 7.43  2017-08-18
   - Improved Mojo::Base role support with the ability to rebless objects.

diff --git a/lib/Mojo/Loader.pm b/lib/Mojo/Loader.pm
@@ -40,11 +40,8 @@ sub load_class {
   # Invalid class name
   return 1 if ($class || '') !~ /^\w(?:[\w:']*\w)?$/;
 
-  # Already loaded
-  return undef if $class->can('new');
-
-  # Success
-  eval "require $class; 1" ? return undef : Mojo::Util::_teardown($class);
+  # Load if not already loaded
+  return undef if $class->can('new') || eval "require $class; 1";
 
   # Does not exist
   return 1 if $@ =~ /^Can't locate \Q@{[class_to_path $class]}\E in \@INC/;
@@ -173,7 +170,8 @@ Search for modules in a namespace non-recursively.
 Load a class and catch exceptions, returns a false value if loading was
 successful, a true value if the class was not found, or a L<Mojo::Exception>
 object if loading failed. Note that classes are checked for a C<new> method to
-see if they are already loaded.
+see if they are already loaded, so trying to load the same class multiple time
+may yield different results.
 
   # Handle exceptions
   if (my $e = load_class 'Foo::Bar') {

diff --git a/lib/Mojo/Reactor.pm b/lib/Mojo/Reactor.pm
@@ -5,12 +5,15 @@ use Carp 'croak';
 use Config;
 use Mojo::Loader 'load_class';
 
+my $DETECTED;
+
 sub again { croak 'Method "again" not implemented by subclass' }
 
 sub detect {
   my $default = 'Mojo::Reactor::' . ($Config{d_pseudofork} ? 'Poll' : 'EV');
   my $try = $ENV{MOJO_REACTOR} || $default;
-  return load_class($try) ? 'Mojo::Reactor::Poll' : $try;
+  return $DETECTED if $DETECTED;
+  return $DETECTED = load_class($try) ? 'Mojo::Reactor::Poll' : $try;
 }
 
 sub io         { croak 'Method "io" not implemented by subclass' }

diff --git a/t/mojo/ioloop.t b/t/mojo/ioloop.t
@@ -9,22 +9,8 @@ use Mojo::IOLoop::Delay;
 use Mojo::IOLoop::Server;
 use Mojo::IOLoop::Stream;
 
-# Custom reactor
-package MyReactor;
-use Mojo::Base 'Mojo::Reactor::Poll';
-
-package main;
-
-# Reactor detection
-$ENV{MOJO_REACTOR} = 'MyReactorDoesNotExist';
-my $loop = Mojo::IOLoop->new;
-is ref $loop->reactor, 'Mojo::Reactor::Poll', 'right class';
-$ENV{MOJO_REACTOR} = 'MyReactor';
-$loop = Mojo::IOLoop->new;
-is ref $loop->reactor, 'MyReactor', 'right class';
-
 # Defaults
-$loop = Mojo::IOLoop->new;
+my $loop = Mojo::IOLoop->new;
 is $loop->max_connections, 1000, 'right default';
 $loop = Mojo::IOLoop->new(max_connections => 10);
 is $loop->max_connections, 10, 'right value';
@@ -332,7 +318,7 @@ ok !$accepting[1], 'connection limit reached';
   my $loop = Mojo::IOLoop->new;
   $loop->timer(0 => sub { die 'Bye!' });
   $loop->start;
-  like $err, qr/^MyReactor:.*Bye!/, 'right error';
+  like $err, qr/^Mojo::Reactor::Poll:.*Bye!/, 'right error';
 }
 
 # Defaults

diff --git a/t/mojo/lib/Mojo/LoaderException.pm b/t/mojo/lib/Mojo/LoaderException.pm
@@ -2,6 +2,8 @@ package Mojo::LoaderException;
 
 use Mojo::Base -base;
 
+sub new {}
+
 foo {
 
 1;
diff --git a/t/mojo/loader.t b/t/mojo/loader.t
@@ -24,25 +24,20 @@ ok !!UNIVERSAL::can(B => 'svref_2object'), 'method found';
 my $e = load_class 'Mojo::LoaderException';
 isa_ok $e, 'Mojo::Exception', 'right exception';
 like $e->message, qr/Missing right curly/, 'right message';
-is $e->lines_before->[0][0], 2,                       'right number';
-is $e->lines_before->[0][1], '',                      'right line';
-is $e->lines_before->[1][0], 3,                       'right number';
-is $e->lines_before->[1][1], 'use Mojo::Base -base;', 'right line';
-is $e->lines_before->[2][0], 4,                       'right number';
-is $e->lines_before->[2][1], '',                      'right line';
-is $e->lines_before->[3][0], 5,                       'right number';
-is $e->lines_before->[3][1], 'foo {',                 'right line';
-is $e->lines_before->[4][0], 6,                       'right number';
-is $e->lines_before->[4][1], '',                      'right line';
-is $e->line->[0], 7,    'right number';
+is $e->lines_before->[0][0], 4,            'right number';
+is $e->lines_before->[0][1], '',           'right line';
+is $e->lines_before->[1][0], 5,            'right number';
+is $e->lines_before->[1][1], 'sub new {}', 'right line';
+is $e->lines_before->[2][0], 6,            'right number';
+is $e->lines_before->[2][1], '',           'right line';
+is $e->lines_before->[3][0], 7,            'right number';
+is $e->lines_before->[3][1], 'foo {',      'right line';
+is $e->lines_before->[4][0], 8,            'right number';
+is $e->lines_before->[4][1], '',           'right line';
+is $e->line->[0], 9,    'right number';
 is $e->line->[1], "1;", 'right line';
 like "$e", qr/Missing right curly/, 'right message';
 
-# Exception again
-$e = load_class 'Mojo::LoaderException';
-isa_ok $e, 'Mojo::Exception', 'right exception';
-like $e->message, qr/Attempt to reload/, 'right message';
-
 # Complicated exception
 $e = load_class 'Mojo::LoaderException2';
 isa_ok $e, 'Mojo::Exception', 'right exception';

diff --git a/t/mojo/reactor_detect.t b/t/mojo/reactor_detect.t
@@ -0,0 +1,22 @@
+use Mojo::Base -strict;
+
+use Test::More;
+use Mojo::Reactor::Poll;
+
+# Dummy reactor
+package Mojo::Reactor::Test;
+use Mojo::Base 'Mojo::Reactor::Poll';
+
+package main;
+
+# Detection (env)
+{
+  local $ENV{MOJO_REACTOR} = 'Mojo::Reactor::Test';
+  is(Mojo::Reactor->detect, 'Mojo::Reactor::Test', 'right class');
+}
+
+# Event loop detection
+require Mojo::IOLoop;
+is ref Mojo::IOLoop->new->reactor, 'Mojo::Reactor::Test', 'right class';
+
+done_testing();
diff --git a/t/mojo/reactor_ev.t b/t/mojo/reactor_ev.t
@@ -253,18 +253,6 @@ is $timer, 1, 'timer was triggered once';
 # Detection
 is(Mojo::Reactor->detect, 'Mojo::Reactor::EV', 'right class');
 
-# Dummy reactor
-package Mojo::Reactor::Test;
-use Mojo::Base 'Mojo::Reactor::Poll';
-
-package main;
-
-# Detection (env)
-{
-  local $ENV{MOJO_REACTOR} = 'Mojo::Reactor::Test';
-  is(Mojo::Reactor->detect, 'Mojo::Reactor::Test', 'right class');
-}
-
 # EV in control
 is ref Mojo::IOLoop->singleton->reactor, 'Mojo::Reactor::EV', 'right object';
 ok !Mojo::IOLoop->is_running, 'loop is not running';

diff --git a/t/mojo/reactor_poll.t b/t/mojo/reactor_poll.t
@@ -250,18 +250,6 @@ is $timer, 1, 'timer was triggered once';
 # Detection
 is(Mojo::Reactor::Poll->detect, 'Mojo::Reactor::Poll', 'right class');
 
-# Dummy reactor
-package Mojo::Reactor::Test;
-use Mojo::Base 'Mojo::Reactor::Poll';
-
-package main;
-
-# Detection (env)
-{
-  local $ENV{MOJO_REACTOR} = 'Mojo::Reactor::Test';
-  is(Mojo::Reactor->detect, 'Mojo::Reactor::Test', 'right class');
-}
-
 # Reactor in control
 is ref Mojo::IOLoop->singleton->reactor, 'Mojo::Reactor::Poll', 'right object';
 ok !Mojo::IOLoop->is_running, 'loop is not running';

diff --git a/t/mojolicious/app.t b/t/mojolicious/app.t
@@ -599,6 +599,16 @@ $t->get_ok('/foo/session')->status_is(200)
 $t->get_ok('/rss.xml')->status_is(200)->content_type_is('application/rss+xml')
   ->content_like(qr!<\?xml version="1.0" encoding="UTF-8"\?><rss />!);
 
+# Missing controller has no side effects
+$t->get_ok('/side_effects-test/index')->status_is(200)
+  ->header_is(Server => 'Mojolicious (Perl)')->content_is('pass');
+$t->get_ok('/side_effects/index')->status_is(404)
+  ->header_is(Server => 'Mojolicious (Perl)');
+$t->get_ok('/side_effects/index')->status_is(404)
+  ->header_is(Server => 'Mojolicious (Perl)');
+$t->get_ok('/side_effects-test/index')->status_is(200)
+  ->header_is(Server => 'Mojolicious (Perl)')->content_is('pass');
+
 # Connection already closed
 eval { Mojolicious::Controller->new->finish };
 like $@, qr/Connection already closed/, 'right error';

diff --git a/t/mojolicious/lib/MojoliciousTest/SideEffects/Test.pm b/t/mojolicious/lib/MojoliciousTest/SideEffects/Test.pm
@@ -0,0 +1,6 @@
+package MojoliciousTest::SideEffects::Test;
+use Mojo::Base 'Mojolicious::Controller';
+
+sub index { shift->render(text => 'pass') }
+
+1;