fixed a bug where paths and classes in Mojolicious::Static and Mojoli…

…cious::Renderer would have the wrong precedence
mojolicious · Mar 26, 2012 · 5e82fb2 · 5e82fb2
1 parent b57372c
commit 5e82fb2
Show file tree

Hide file tree

Showing 16 changed files with 100 additions and 33 deletions.
diff --git a/Changes b/Changes
@@ -6,6 +6,8 @@ This file documents the revision history for Perl extension Mojolicious.
   - Improved Mojo::DOM::CSS performance.
   - Improved documentation.
   - Improved Mojo::Reactor tests to be less strict.
+  - Fixed a bug where paths and classes in Mojolicious::Static and
+    Mojolicious::Renderer would have the wrong precedence.
   - Fixed small bug where Hypnotoad would ignore the MOJO_REVERSE_PROXY
     environment variable.
 

diff --git a/lib/Mojo/Exception.pm b/lib/Mojo/Exception.pm
@@ -69,7 +69,7 @@ sub _detect {
   }
 
   # Search for context
-  foreach my $frame (reverse @trace) {
+  for my $frame (reverse @trace) {
     next unless -r $frame->[0];
     my $handle = IO::File->new($frame->[0], '<:utf8');
     $self->_parse_context($frame->[1], [[<$handle>]]);

diff --git a/lib/Mojo/Headers.pm b/lib/Mojo/Headers.pm
@@ -155,7 +155,7 @@ sub to_hash {
 
   # Build
   my $hash = {};
-  foreach my $header (@{$self->names}) {
+  for my $header (@{$self->names}) {
     my @headers = $self->header($header);
 
     # Nested arrayrefs
@@ -165,7 +165,7 @@ sub to_hash {
     else {
 
       # Turn single value arrayrefs into strings
-      foreach my $h (@headers) { $h = $h->[0] if @$h == 1 }
+      for my $h (@headers) { $h = $h->[0] if @$h == 1 }
       $hash->{$header} = @headers > 1 ? [@headers] : $headers[0];
     }
   }

diff --git a/lib/Mojo/Loader.pm b/lib/Mojo/Loader.pm
@@ -41,7 +41,7 @@ sub search {
   # Scan
   my $modules = [];
   my %found;
-  foreach my $directory (exists $INC{'blib.pm'} ? grep {/blib/} @INC : @INC) {
+  for my $directory (exists $INC{'blib.pm'} ? grep {/blib/} @INC : @INC) {
     my $path = catdir $directory, (split /::/, $namespace);
     next unless (-e $path && -d $path);
 

diff --git a/lib/Mojo/UserAgent/Transactor.pm b/lib/Mojo/UserAgent/Transactor.pm
@@ -78,7 +78,7 @@ sub form {
 
     # Parts
     my @parts;
-    foreach my $name (sort keys %$form) {
+    for my $name (sort keys %$form) {
       my $part = Mojo::Content::Single->new;
       my $h    = $part->headers;
       my $f    = $form->{$name};

diff --git a/lib/Mojolicious/Command/routes.pm b/lib/Mojolicious/Command/routes.pm
@@ -50,7 +50,7 @@ sub _draw {
   }
 
   # Draw
-  foreach my $node (@$routes) {
+  for my $node (@$routes) {
     my @parts;
 
     # Pattern

diff --git a/lib/Mojolicious/Commands.pm b/lib/Mojolicious/Commands.pm
@@ -100,14 +100,14 @@ sub run {
   # Make list
   my $list = [];
   my $max  = 0;
-  foreach my $command (@$commands) {
+  for my $command (@$commands) {
     my $len = length $command->[0];
     $max = $len if $len > $max;
     push @$list, [$command->[0], $command->[1]->new->description];
   }
 
   # Print list
-  foreach my $command (@$list) {
+  for my $command (@$list) {
     my ($name, $description) = @$command;
     print "  $name" . (' ' x ($max - length $name)) . "   $description";
   }

diff --git a/lib/Mojolicious/Renderer.pm b/lib/Mojolicious/Renderer.pm
@@ -195,7 +195,7 @@ sub template_path {
   return unless my $name = $self->template_name(shift);
 
   # Search all paths
-  foreach my $path (@{$self->paths}) {
+  for my $path (@{$self->paths}) {
     my $file = catfile($path, split '/', $name);
     return $file if -r $file;
   }
@@ -209,7 +209,7 @@ sub _data_templates {
 
   # Index DATA templates
   return $self->{data_templates} if $self->{data_templates};
-  for my $class (@{$self->classes}) {
+  for my $class (reverse @{$self->classes}) {
     $self->{data_templates}->{$_} = $class
       for keys %{Mojo::Command->new->get_all_data($class) || {}};
   }
@@ -298,7 +298,8 @@ Renderer cache, defaults to a L<Mojo::Cache> object.
   my $classes = $renderer->classes;
   $renderer   = $renderer->classes(['main']);
 
-Classes to use for finding templates in C<DATA> section, defaults to C<main>.
+Classes to use for finding templates in C<DATA> section, first one has the
+highest precedence, defaults to C<main>.
 
   # Add another class with templates in DATA section
   push @{$renderer->classes}, 'Mojolicious::Plugin::Fun';
@@ -345,7 +346,7 @@ Registered helpers.
   my $paths = $renderer->paths;
   $renderer = $renderer->paths(['/foo/bar/templates']);
 
-Directories to look for templates in.
+Directories to look for templates in, first one has the highest precedence.
 
   # Add another "templates" directory
   push @{$renderer->paths}, '/foo/bar/templates';

diff --git a/lib/Mojolicious/Static.pm b/lib/Mojolicious/Static.pm
@@ -72,17 +72,14 @@ sub serve {
   my $modified = $self->{modified} ||= time;
   my $res      = $c->res;
   for my $path (@{$self->paths}) {
-    my $file = catfile($path, split('/', $rel));
-    next unless my $data = $self->_get_file($file);
-
-    # Forbidded
-    unless (@$data) {
-      $c->app->log->debug(qq/File "$rel" is forbidden./);
-      $res->code(403) and return;
-    }
+    next unless my $data = $self->_get_file(catfile $path, split('/', $rel));
 
     # Exists
-    ($asset, $size, $modified) = @$data;
+    last if ($asset, $size, $modified) = @$data;
+
+    # Forbidded
+    $c->app->log->debug(qq/File "$rel" is forbidden./);
+    $res->code(403) and return;
   }
 
   # Search DATA
@@ -157,7 +154,7 @@ sub _get_data_file {
   # Index DATA files
   unless ($self->{data_files}) {
     $self->{data_files} = {};
-    for my $class (@{$self->classes}) {
+    for my $class (reverse @{$self->classes}) {
       $self->{data_files}->{$_} = $class
         for keys %{Mojo::Command->new->get_all_data($class) || {}};
     }
@@ -202,7 +199,8 @@ L<Mojolicious::Static> implements the following attributes.
   my $classes = $static->classes;
   $static     = $static->classes(['main']);
 
-Classes to use for finding files in C<DATA> section, defaults to C<main>.
+Classes to use for finding files in C<DATA> section, first one has the
+highest precedence, defaults to C<main>.
 
   # Add another class with static files in DATA section
   push @{$static->classes}, 'Mojolicious::Plugin::Fun';
@@ -212,7 +210,7 @@ Classes to use for finding files in C<DATA> section, defaults to C<main>.
   my $paths = $static->paths;
   $static   = $static->paths(['/foo/bar/public']);
 
-Directories to serve static files from.
+Directories to serve static files from, first one has the highest precedence.
 
   # Add another "public" directory
   push @{$static->paths}, '/foo/bar/public';

diff --git a/t/mojolicious/app.t b/t/mojolicious/app.t
@@ -7,7 +7,7 @@ BEGIN {
   $ENV{MOJO_REACTOR}    = 'Mojo::Reactor::Poll';
 }
 
-use Test::More tests => 284;
+use Test::More tests => 294;
 
 use FindBin;
 use lib "$FindBin::Bin/lib";
@@ -250,6 +250,18 @@ $t->get_ok('/hello.txt', {'If-Modified-Since' => $mtime})->status_is(304)
   ->header_is(Server         => 'Mojolicious (Perl)')
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')->content_is('');
 
+# Embedded development static file
+$t->get_ok('/some/static/file.txt')->status_is(200)
+  ->header_is(Server         => 'Mojolicious (Perl)')
+  ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
+  ->content_is("Development static file with high precedence.\n");
+
+# Embedded development template
+$t->get_ok('/just/some/template')->status_is(200)
+  ->header_is(Server         => 'Mojolicious (Perl)')
+  ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
+  ->content_is("Development template with high precedence.\n");
+
 # Check develpment mode log level
 my $app = Mojolicious->new;
 is $app->log->level, 'debug', 'right log level';

diff --git a/t/mojolicious/lib/MojoliciousTest.pm b/t/mojolicious/lib/MojoliciousTest.pm
@@ -1,17 +1,27 @@
 package MojoliciousTest;
 use Mojo::Base 'Mojolicious';
 
+use MojoliciousTest::Foo;
+
 sub development_mode {
   my $self = shift;
 
+  # Template and static file class with higher precedence for development
+  unshift @{$self->static->classes},   'MojoliciousTest::Foo';
+  unshift @{$self->renderer->classes}, 'MojoliciousTest::Foo';
+
   # Static root for development
-  $self->static->paths->[0] = $self->home->rel_dir('public_dev');
+  unshift @{$self->static->paths}, $self->home->rel_dir('public_dev');
 }
 
 # "Let's face it, comedy's a dead art form. Tragedy, now that's funny."
 sub startup {
   my $self = shift;
 
+  # Template and static file class with lower precedence for production
+  push @{$self->static->classes},   'MojoliciousTest';
+  push @{$self->renderer->classes}, 'MojoliciousTest';
+
   # Plugins in custom namespace
   unshift @{$self->plugins->namespaces},
     $self->routes->namespace . '::Plugin';
@@ -134,6 +144,16 @@ sub startup {
 
   # /*/* (the default route)
   $r->route('/(controller)/(action)')->to(action => 'index');
+
+  # /just/some/template (embedded template)
+  $r->route('/just/some/template')->to(template => 'just/some/template');
 }
 
 1;
+__DATA__
+
+@@ some/static/file.txt
+Production static file with low precedence.
+
+@@ just/some/template.html.ep
+Production template with low precedence.
diff --git a/t/mojolicious/lib/MojoliciousTest/Foo.pm b/t/mojolicious/lib/MojoliciousTest/Foo.pm
@@ -81,3 +81,10 @@ sub withblock { shift->render(template => 'withblock') }
 sub withlayout { shift->stash(template => 'withlayout') }
 
 1;
+__DATA__
+
+@@ just/some/template.html.ep
+Development template with high precedence.
+
+@@ some/static/file.txt
+Development static file with high precedence.
diff --git a/t/mojolicious/multipath_lite_app.t b/t/mojolicious/multipath_lite_app.t
@@ -8,21 +8,24 @@ BEGIN {
 
 # "You are hereby conquered.
 #  Please line up in order of how much beryllium it takes to kill you."
-use Test::More tests => 20;
+use Test::More tests => 30;
 
 use Mojolicious::Lite;
 use Test::Mojo;
 
-# More paths
-push @{app->renderer->paths}, app->home->rel_dir('templates2');
-push @{app->static->paths},   app->home->rel_dir('public2');
+# More paths with higher precedence
+unshift @{app->renderer->paths}, app->home->rel_dir('templates2');
+unshift @{app->static->paths},   app->home->rel_dir('public2');
 
 # GET /twenty_three
 get '/twenty_three' => '23';
 
 # GET /fourty_two
 get '/fourty_two' => '42';
 
+# GET /yada
+get '/yada' => {template => 'foo/yada'};
+
 my $t = Test::Mojo->new;
 
 # GET /twenty_three (templates)
@@ -36,14 +39,24 @@ $t->get_ok('/fourty_two')->status_is(200)
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
   ->content_is("The answer is 42.\n");
 
-# GET /hello.txt (public)
+# GET /hello.txt (public2)
 $t->get_ok('/hello.txt')->status_is(200)
   ->header_is(Server         => 'Mojolicious (Perl)')
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
-  ->content_is("Hello Mojo from a static file!\n");
+  ->content_is("Also higher precedence!\n");
+
+# GET /hello2.txt (public)
+$t->get_ok('/hello2.txt')->status_is(200)
+  ->header_is(Server         => 'Mojolicious (Perl)')
+  ->header_is('X-Powered-By' => 'Mojolicious (Perl)')->content_is("X");
 
 # GET /hello3.txt (public2)
 $t->get_ok('/hello3.txt')->status_is(200)
   ->header_is(Server         => 'Mojolicious (Perl)')
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
   ->content_is("Hello Mojo from... ALL GLORY TO THE HYPNOTOAD!\n");
+
+# GET /yada (templates2)
+$t->get_ok('/yada')->status_is(200)->header_is(Server => 'Mojolicious (Perl)')
+  ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
+  ->content_is("Higher precedence!\n");
diff --git a/t/mojolicious/production_app.t b/t/mojolicious/production_app.t
@@ -7,7 +7,7 @@ BEGIN {
   $ENV{MOJO_REACTOR}    = 'Mojo::Reactor::Poll';
 }
 
-use Test::More tests => 69;
+use Test::More tests => 79;
 
 use FindBin;
 use lib "$FindBin::Bin/lib";
@@ -105,3 +105,15 @@ $t->get_ok('/../../mojolicious/secret.txt')->status_is(404)
   ->header_is(Server         => 'Mojolicious (Perl)')
   ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
   ->content_like(qr/Page not found/);
+
+# Embedded production static file
+$t->get_ok('/some/static/file.txt')->status_is(200)
+  ->header_is(Server         => 'Mojolicious (Perl)')
+  ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
+  ->content_is("Production static file with low precedence.\n\n");
+
+# Embedded production template
+$t->get_ok('/just/some/template')->status_is(200)
+  ->header_is(Server         => 'Mojolicious (Perl)')
+  ->header_is('X-Powered-By' => 'Mojolicious (Perl)')
+  ->content_is("Production template with low precedence.\n");
diff --git a/t/mojolicious/public2/hello.txt b/t/mojolicious/public2/hello.txt
@@ -0,0 +1 @@
+Also higher precedence!
diff --git a/t/mojolicious/templates2/foo/yada.html.ep b/t/mojolicious/templates2/foo/yada.html.ep
@@ -0,0 +1 @@
+Higher precedence!