Configure but don't connect to DB in init_db()

This allows us to start up QP even if e.g. Redis is down

lib/Qpsmtpd/DB/File/DBM.pm

@@ -180,15 +180,23 @@ sub flush {
 }
 
 sub dir {
-    my ( $self, @candidate_dirs ) = @_;
-    return $self->{dir} if $self->{dir} and ! @candidate_dirs;
-    push @candidate_dirs, ( $self->qphome . '/var/db', $self->qphome . '/config' );
-    for my $d ( @candidate_dirs ) {
+    my ( $self, @arg ) = @_;
+    return $self->{dir} if $self->{dir} and ! @arg;
+    for my $d ( $self->candidate_dirs(@arg) ) {
         next if ! $self->validate_dir($d);
         return $self->{dir} = $d; # first match wins
     }
 }
 
+sub candidate_dirs {
+    my ( $self, @arg ) = @_;
+    return @{ $self->{candidate_dirs} } if $self->{candidate_dirs} && ! @arg;
+    $self->{candidate_dirs} = \@arg if @arg;
+    push @{ $self->{candidate_dirs} },
+      $self->qphome . '/var/db', $self->qphome . '/config';
+    return @{ $self->{candidate_dirs} };
+}
+
 sub validate_dir {
     my ( $self, $d ) = @_;
     return 0 if ! $d;

lib/Qpsmtpd/Plugin.pm

@@ -347,10 +347,16 @@ sub _register_standard_hooks {
     }
 }
 
+sub db_args {
+    my ( $self, %arg ) = @_;
+    $self->{db_args} = \%arg if %arg;
+    $self->{db_args}{name} ||= $self->plugin_name;
+    return %{ $self->{db_args} };
+}
+
 sub db {
     my ( $self, %arg ) = @_;
-    $arg{name} ||= $self->plugin_name;
-    return $self->{db} ||= Qpsmtpd::DB->new(%arg);
+    return $self->{db} ||= Qpsmtpd::DB->new( $self->db_args(%arg) );
 }
 
 1;

plugins/greylisting

@@ -211,7 +211,7 @@ sub register {
         $config->{reject} = $config->{mode} =~ /testonly|off/i ? 0 : 1;
     }
     $self->{_args} = $config;
-    $self->init_db() or return;
+    $self->init_db();
     $self->register_hooks();
     $self->prune_db();
     if ($self->{_args}{upgrade}) {
@@ -233,22 +233,20 @@ sub register_hooks {
 
 sub init_db {
     my ($self) = @_;
-    return $self->init_redis if $self->{_args}{redis};
-    return $self->init_dbm;
+    if ( $self->{_args}{redis} ) {
+        $self->init_redis;
+    } else {
+        $self->init_dbm;
+    }
 }
 
 sub init_redis {
     my ($self) = @_;
-    eval {
-        $self->db(
-            name   => 'greylist',
-            class  => 'Qpsmtpd::DB::Redis',
-            server => $self->parse_redis_server,
-        ) or die 'Unknown error';
-    };
-    return 1 if ! $@;
-    $self->log(LOGCRIT, "Unable to connect to redis, GREYLISTING DISABLED: $@");
-    return 0;
+    $self->db_args(
+        name   => 'greylist',
+        class  => 'Qpsmtpd::DB::Redis',
+        server => $self->parse_redis_server,
+    );
 }
 
 sub parse_redis_server {
@@ -263,19 +261,18 @@ sub init_dbm {
     $self->db(
         name  => 'greylist',
         class => 'Qpsmtpd::DB::File::DBM'
-    ) or return 0;
+    );
     my $cdir = $self->{_args}{db_dir};
     $cdir = $1 if $cdir and $cdir =~ m{^([-a-zA-Z0-9./_]+)$};
     # greylisting-specific hints for where to store the greylist DB
     my $db_dir = $self->db->dir( $cdir, '/var/lib/qpsmtpd/greylisting' );
     $self->db->nfs_locking( $self->{_args}{nfslock} );
-    
+
     # Work around old DBM filename
-    return 1 if -f "$db_dir/greylist.dbm";
     my $oldname = 'denysoft_greylist';
-    return 1 if ! -f "$db_dir/$oldname.dbm";
-    $self->db->name($oldname);
-    return 1;
+    if ( ! -f "$db_dir/greylist.dbm" && -f "$db_dir/$oldname.dbm" ) {
+        $self->db->name($oldname);
+    }
 }
 
 sub load_exclude_files {

t/plugin_tests/greylisting

@@ -333,19 +333,26 @@ sub rc {
 sub test_init_redis {
     my ($self) = @_;
     delete $self->{db};
-    $self->{_args}{redis} = 'bogusserverasdfqwerty.:6379';
-    ok( ! $self->init_redis, 'init_redis() fails on bogus server' );
-    eval { Qpsmtpd::DB::Redis->new };
-    return if $@;
-    $self->{_args}{redis} = 'localhost';
-    ok( $self->init_redis, 'init_redis() succeeds when redis is up' );
+    $self->{_args}{redis} = 'testredis';
+    $self->init_db;
+    is( keyvals($self->db_args),
+      'class=Qpsmtpd::DB::Redis;name=greylist;server=testredis:6379',
+      'init_redis() sets redis args' );
+}
+
+sub keyvals {
+    my ( %h ) = @_;
+    return join ";", map { "$_=$h{$_}" } sort keys %h;
 }
 
 sub test_init_dbm {
     my ($self) = @_;
     delete $self->{db};
     delete $self->{_args}{redis};
-    ok( $self->init_db, 'init_db() works for DBM' );
+    $self->init_db;
+    is( $self->db->name, 'greylist', 'init_dbm() sets correct db name' );
+    is( $self->db->path, 't/config/greylist.dbm', 'init_dbm() sets correct path' );
+    is( ref $self->db, 'Qpsmtpd::DB::File::DBM', 'init_dbm() gives DBM object' );
 }
 
 sub test_parse_redis_server {

t/qpsmtpd-db-file-dbm.t

@@ -19,6 +19,7 @@ __get_keys();
 __size();
 __flush();
 __qphome();
+__candidate_dirs();
 __validate_dir();
 __dir();
 __untie_gotcha();
@@ -108,6 +109,15 @@ sub __qphome {
     is( $db->qphome, 't', 'qphome()' );
 }
 
+sub __candidate_dirs {
+    is( join('|', $db->candidate_dirs), 't/var/db|t/config',
+      'candidate_dirs() default ' );
+    is( join('|', $db->candidate_dirs('foo')), 'foo|t/var/db|t/config',
+      'candidate_dirs() passed args plus defaults' );
+    is( join('|', $db->candidate_dirs), 'foo|t/var/db|t/config',
+      'candidate_dirs() cached values' );
+}
+
 sub __validate_dir {
     is( $db->validate_dir(),      0, 'validate_dir(): false on no input' );
     is( $db->validate_dir(undef), 0, 'validate_dir(): false on undef' );

t/qpsmtpd-plugin.t

@@ -9,11 +9,33 @@ use Test::Qpsmtpd;
 
 use_ok('Qpsmtpd::Plugin');
 
+__db_args();
 __db();
 __register_hook();
 
 done_testing();
 
+sub __db_args {
+    my $plugin = FakePlugin->new;
+    is( keyvals($plugin->db_args),
+      'name=___MockHook___',
+      'default db args populated' );
+    is( keyvals($plugin->db_args( arg1 => 1 )),
+      'arg1=1;name=___MockHook___',
+      'passed args in addition to defaults' );
+    is( keyvals($plugin->db_args( name => 'bob', arg2 => 2 )),
+      'arg2=2;name=bob',
+      'passed args override defaults' );
+    is( keyvals($plugin->db_args),
+      'arg2=2;name=bob',
+      'get previous args' );
+}
+
+sub keyvals {
+    my ( %h ) = @_;
+    return join ";", map { "$_=$h{$_}" } sort keys %h;
+}
+
 sub __db {
     my $plugin = FakePlugin->new;
     my $db = $plugin->db( class => 'FakeDB', name => 'testfoo' );