load config file immediately when set

This allows us to detect errors in the file and reject it.
haarg · Sep 14, 2009 · f9c3658 · f9c3658
1 parent 001347e
commit f9c3658
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 11 deletions.
diff --git a/lib/WGDev.pm b/lib/WGDev.pm
@@ -82,6 +82,7 @@ sub config_file {
     my $self = shift;
     if (@_) {
         my $path = shift;
+        require Config::JSON;
         if ( -f $path ) {
         }
         elsif (
@@ -108,9 +109,18 @@ sub config_file {
                     ) );
             };
         }
+        my $path_abs = File::Spec->rel2abs($path);
+        my $config;
+        if ( ! eval { $config = Config::JSON->new($path_abs); 1 } ) {
+            WGDev::X::BadParameter->throw(
+                'parameter' => 'WebGUI config file',
+                'value'     => $path
+            );
+        }
         $self->close_session;
         $self->close_config;
-        $self->{config_file} = File::Spec->rel2abs($path);
+        $self->{config_file} = $path_abs;
+        $self->{config} = $config;
         delete $self->{config_file_relative};
     }
     return $self->{config_file};

diff --git a/t/01_WGDev.t b/t/01_WGDev.t
@@ -1,8 +1,7 @@
 use strict;
 use warnings;
 
-use Test::More tests => 32;
-use Test::NoWarnings;
+use Test::More;
 use Test::Exception;
 
 use File::Spec::Functions qw(catdir catfile catpath rel2abs splitpath);
@@ -15,6 +14,12 @@ use constant TEST_DIR => catpath( ( splitpath(__FILE__) )[ 0, 1 ], '' );
 
 use WGDev ();
 
+use constant HAS_DONE_TESTING => Test::More->can('done_testing') ? 1 : undef;
+# use done_testing if possible
+if ( ! HAS_DONE_TESTING ) {
+    plan 'no_plan';
+}
+
 my $wgd = WGDev->new;
 
 isa_ok $wgd, 'WGDev', 'WGDev->new returns WGDev object';
@@ -45,6 +50,13 @@ my $config = catfile( $etc, 'www.example.com.conf' );
 copy catfile( $test_data, 'www.example.com.conf' ), $config;
 copy catfile( $test_data, 'www.example.com.conf' ),
     catfile( $etc, 'WebGUI.conf.original' );
+my $config_broken = catfile( $etc, 'www.broken.com.conf' );
+{
+    open my $fh, '>', $config_broken;
+    print {$fh} 'garbage data';
+    close $fh;
+}
+
 
 my $module = catfile( $lib, 'WebGUI.pm' );
 copy catfile( $test_data, 'WebGUI.pm' ), $module;
@@ -130,6 +142,9 @@ is $wgd->root, $root_abs, 'Root not modified after failed attempts to set';
 throws_ok { $wgd->config_file('nonexistant') } 'WGDev::X::BadParameter',
     'Error thrown when trying to set config to nonexistant file with root set';
 
+throws_ok { $wgd->config_file($config_broken) } 'WGDev::X::BadParameter',
+    'Error thrown when trying to set config to broken config file';
+
 $wgd = WGDev->new;
 
 throws_ok { $wgd->config_file('nonexistant') } 'WGDev::X::BadParameter',
@@ -143,6 +158,9 @@ is realpath( $wgd->root ), realpath($root_abs),
 
 ok scalar( grep { $_ eq $wgd->lib } @INC ), 'WebGUI lib path added to @INC';
 
+ok $wgd->close_config, 'Can close config';
+lives_and { isa_ok $wgd->config, 'Config::JSON' } 'Call to config reopens config as needed';
+
 open my $fh, '>', catfile( $sbin, 'preload.custom' );
 print {$fh} $sbin . "\n"
     . catdir( $root, 'nonexistant' ) . "\n"
@@ -165,3 +183,7 @@ $wgd = WGDev->new($config);
 is_deeply [ map { realpath($_) } $wgd->lib ], [ map { realpath($_) } ($lib) ],
     'Unreadable preload.custom silently ignored';
 
+if ( HAS_DONE_TESTING ) {
+    done_testing;
+}
+
diff --git a/t/03_WGDev-Command.t b/t/03_WGDev-Command.t
@@ -1,8 +1,7 @@
 use strict;
 use warnings;
 
-use Test::More 'no_plan';
-use Test::NoWarnings;
+use Test::More;
 use Test::Exception;
 use Test::Warn;
 use Test::MockObject;
@@ -32,6 +31,12 @@ use WGDev::Help                    ();
 
 BEGIN { $INC{'WGDev/Command/_tester.pm'} = $INC{'WGDev_tester_command.pm'} }
 
+use constant HAS_DONE_TESTING => Test::More->can('done_testing') ? 1 : undef;
+# use done_testing if possible
+if ( ! HAS_DONE_TESTING ) {
+    plan 'no_plan';
+}
+
 my $test_data = catdir( TEST_DIR, 'testdata' );
 
 # we don't want the user's configuration interfering with the test
@@ -177,6 +182,12 @@ my $config = catfile( $etc, 'www.example.com.conf' );
 copy catfile( $test_data, 'www.example.com.conf' ), $config;
 my $config_in_empty = catfile( $emptydir, 'www.example.com.conf' );
 copy catfile( $test_data, 'www.example.com.conf' ), $config_in_empty;
+my $config_broken = catfile( $etc, 'www.broken.com.conf' );
+{
+    open my $fh, '>', $config_broken;
+    print {$fh} 'garbage data';
+    close $fh;
+}
 
 my $module = catfile( $lib, 'WebGUI.pm' );
 copy catfile( $test_data, 'WebGUI.pm' ), $module;
@@ -443,11 +454,6 @@ my $config2_abs = catfile($etc, 'www.example2.com.conf');
     }
     'guess_webgui_paths finds config file when given shortened sitename';
 
-    {
-        open my $fh, '>', catfile($etc, 'www.broken.com.conf');
-        print {$fh} 'garbage data';
-        close $fh;
-    }
     lives_and {
         is_path +WGDev::Command->guess_webgui_paths(
             wgd => WGDev->new,
@@ -456,7 +462,6 @@ my $config2_abs = catfile($etc, 'www.example2.com.conf');
         )->config_file, $config2_abs;
     }
     q{broken config file doesn't interfere with sitename search};
-    unlink catfile($etc, 'www.broken.com.conf');
 
     throws_ok {
         WGDev::Command->guess_webgui_paths( wgd => WGDev->new, root => $root, sitename => 'www.example.com' )
@@ -534,6 +539,11 @@ my $config2_abs = catfile($etc, 'www.example2.com.conf');
     # TODO: Add more tests for sitename/config conflicts
 }
 
+throws_ok {
+    my $wgd = WGDev->new;
+    WGDev::Command->guess_webgui_paths(wgd => $wgd, config_file => $config_broken);
+} 'WGDev::X::BadParameter', 'throws when given a broken config file';
+
 # TODO: test cwd in valid WebGUI root and specified config in different valid WebGUI root
 
 throws_ok {
@@ -623,3 +633,7 @@ throws_ok {
     is $return, 'magic', '... and returns value from object directly';
 }
 
+if ( HAS_DONE_TESTING ) {
+    done_testing;
+}
+