[Catalyst] setup_home() bug

[Adam Herzog]

On Jul 9, 2007, at 3:05 PM, Matt S Trout wrote:
> Patch looks fantastic, but it strongly suggests what we really need  
> is an
> addition to Catalyst::Utils called if_env_exists (or equivalent  
> better name :)
>
> What do you think? Fancy having a go at cleaning this up once and  
> for all?

Okay. I added Catalyst::Utils::env_value(), and updated Catalyst.pm  
to use the util function in all of the places that were checking  
CATALYST_ and MYAPP_ (with MYAPP_ winning out if they are both defined.)

setup_engine and setup_dispatcher have had their behavior changed,  
slightly. In an application named MyApp::Web, they would have checked  
for MYAPP::WEB_ENGINE instead of their new behavior of checking  
MYAPP_WEB_ENGINE. I'm assuming it was a bug, but I couldn't find and  
documentation that would indicate what was correct and I'd never seen  
it before.

Thanks,

-A


Index: t/unit_utils_env_value.t
===================================================================
--- t/unit_utils_env_value.t    (revision 0)
+++ t/unit_utils_env_value.t    (revision 0)
@@ -0,0 +1,44 @@
+use strict;
+use warnings;
+
+use Test::More tests => 5;
+
+BEGIN { use_ok("Catalyst::Utils") }
+
+####################################################################### 
#######
+### No env vars defined
+####################################################################### 
#######
+{
+    ok( !Catalyst::Utils::env_value( 'MyApp', 'Key' ),
+        'No env values defined returns false'
+    );
+}
+
+####################################################################### 
#######
+### App env var defined
+####################################################################### 
#######
+{
+    $ENV{'MYAPP2_KEY'} = 'Env value 2';
+    is( Catalyst::Utils::env_value( 'MyApp2', 'Key' ),
+        'Env value 2', 'Got the right value from the application  
var' );
+}
+
+####################################################################### 
#######
+### Catalyst env var defined
+####################################################################### 
#######
+{
+    $ENV{'CATALYST_KEY'} = 'Env value 3';
+    is( Catalyst::Utils::env_value( 'MyApp3', 'Key' ),
+        'Env value 3', 'Got the right value from the catalyst var' );
+}
+
+####################################################################### 
#######
+### Catalyst and Application env vars defined
+####################################################################### 
#######
+{
+    $ENV{'CATALYST_KEY'} = 'Env value bad';
+    $ENV{'MYAPP4_KEY'}   = 'Env value 4';
+    is( Catalyst::Utils::env_value( 'MyApp4', 'Key' ),
+        'Env value 4', 'Got the right value from the application  
var' );
+}
+
Index: lib/Catalyst.pm
===================================================================
--- lib/Catalyst.pm     (revision 6518)
+++ lib/Catalyst.pm     (working copy)
@@ -1923,15 +1923,10 @@
          $dispatcher = 'Catalyst::Dispatcher::' . $dispatcher;
      }
-    if ( $ENV{CATALYST_DISPATCHER} ) {
-        $dispatcher = 'Catalyst::Dispatcher::' . $ENV 
{CATALYST_DISPATCHER};
+    if ( my $env = Catalyst::Utils::env_value( $class,  
'DISPATCHER' ) ) {
+        $dispatcher = 'Catalyst::Dispatcher::' . $env;
      }
-    if ( $ENV{ uc($class) . '_DISPATCHER' } ) {
-        $dispatcher =
-          'Catalyst::Dispatcher::' . $ENV{ uc($class) .  
'_DISPATCHER' };
-    }
-
      unless ($dispatcher) {
          $dispatcher = $class->dispatcher_class;
      }
@@ -1957,14 +1952,10 @@
          $engine = 'Catalyst::Engine::' . $engine;
      }
-    if ( $ENV{CATALYST_ENGINE} ) {
-        $engine = 'Catalyst::Engine::' . $ENV{CATALYST_ENGINE};
+    if ( my $env = Catalyst::Utils::env_value( $class, 'ENGINE' ) ) {
+        $engine = 'Catalyst::Engine::' . $env;
      }
-    if ( $ENV{ uc($class) . '_ENGINE' } ) {
-        $engine = 'Catalyst::Engine::' . $ENV{ uc($class) .  
'_ENGINE' };
-    }
-
      if ( $ENV{MOD_PERL} ) {
          # create the apache method
@@ -2077,15 +2068,10 @@
sub setup_home {
      my ( $class, $home ) = @_;
-    if ( $ENV{CATALYST_HOME} ) {
-        $home = $ENV{CATALYST_HOME};
+    if ( my $env = Catalyst::Utils::env_value( $class, 'HOME' ) ) {
+        $home = $env;
      }
-    if ( $ENV{ uc($class) . '_HOME' } ) {
-        $class =~ s/::/_/g;
-        $home = $ENV{ uc($class) . '_HOME' };
-    }
-
      unless ($home) {
          $home = Catalyst::Utils::home($class);
      }
@@ -2109,14 +2095,8 @@
          $class->log( Catalyst::Log->new );
      }
-    my $app_flag = Catalyst::Utils::class2env($class) . '_DEBUG';
-
-    if (
-          ( defined( $ENV{CATALYST_DEBUG} ) || defined( $ENV 
{$app_flag} ) )
-        ? ( $ENV{CATALYST_DEBUG} || $ENV{$app_flag} )
-        : $debug
-      )
-    {
+    my $env_debug = Catalyst::Utils::env_value( $class, 'DEBUG' );
+    if ( defined($env_debug) ? $env_debug : $debug ) {
          no strict 'refs';
          *{"$class\::debug"} = sub { 1 };
          $class->log->debug('Debug messages enabled');
Index: lib/Catalyst/Utils.pm
===================================================================
--- lib/Catalyst/Utils.pm       (revision 6518)
+++ lib/Catalyst/Utils.pm       (working copy)
@@ -288,7 +288,29 @@
      return \%merged;
}
+=head2 env_value($class, $key)
+Checks for and returns an environment value. For instance, if $key is
+'home', then this method will check for and return the first value  
it finds,
+looking at $ENV{MYAPP_HOME} and $ENV{CATALYST_HOME}.
+
+=cut
+
+sub env_value {
+    my ( $class, $key ) = @_;
+
+    $key = uc($key);
+    my @prefixes = ( class2env($class), 'CATALYST' );
+
+    for my $prefix (@prefixes) {
+        if ( defined( my $value = $ENV{"${prefix}_${key}"} ) ) {
+            return $value;
+        }
+    }
+
+    return;
+}
+
=head1 AUTHOR
Sebastian Riedel, C<sri at cpan.org>