separate out is_*able last_* checks

lib/App/Netdisco/Backend/Worker/Poller/Arpnip.pm

@@ -1,7 +1,7 @@
 package App::Netdisco::Backend::Worker::Poller::Arpnip;
 
 use App::Netdisco::Core::Arpnip 'do_arpnip';
-use App::Netdisco::Util::Device 'is_arpnipable';
+use App::Netdisco::Util::Device 'is_arpnipable_now';
 
 use Role::Tiny;
 use namespace::clean;
@@ -9,7 +9,7 @@ use namespace::clean;
 with 'App::Netdisco::Backend::Worker::Poller::Common';
 
 sub arpnip_action { \&do_arpnip }
-sub arpnip_filter { \&is_arpnipable }
+sub arpnip_filter { \&is_arpnipable_now }
 sub arpnip_layer { 3 }
 
 sub arpwalk { (shift)->_walk_body('arpnip', @_) }

lib/App/Netdisco/Backend/Worker/Poller/Device.pm

@@ -3,7 +3,7 @@ package App::Netdisco::Backend::Worker::Poller::Device;
 use Dancer qw/:moose :syntax :script/;
 
 use App::Netdisco::Util::SNMP 'snmp_connect';
-use App::Netdisco::Util::Device qw/get_device is_discoverable/;
+use App::Netdisco::Util::Device qw/get_device is_discoverable_now/;
 use App::Netdisco::Core::Discover ':all';
 use App::Netdisco::Backend::Util ':all';
 use App::Netdisco::JobQueue qw/jq_queued jq_insert/;
@@ -54,7 +54,7 @@ sub discover {
       return job_done("discover skipped: $host is pseudo-device");
   }
 
-  unless (is_discoverable($device->ip)) {
+  unless (is_discoverable_now($device->ip)) {
       return job_defer("discover deferred: $host is not discoverable");
   }
 

lib/App/Netdisco/Backend/Worker/Poller/Macsuck.pm

@@ -1,7 +1,7 @@
 package App::Netdisco::Backend::Worker::Poller::Macsuck;
 
 use App::Netdisco::Core::Macsuck 'do_macsuck';
-use App::Netdisco::Util::Device 'is_macsuckable';
+use App::Netdisco::Util::Device 'is_macsuckable_now';
 
 use Role::Tiny;
 use namespace::clean;
@@ -9,7 +9,7 @@ use namespace::clean;
 with 'App::Netdisco::Backend::Worker::Poller::Common';
 
 sub macsuck_action { \&do_macsuck }
-sub macsuck_filter { \&is_macsuckable }
+sub macsuck_filter { \&is_macsuckable_now }
 sub macsuck_layer { 2 }
 
 sub macwalk { (shift)->_walk_body('macsuck', @_) }

lib/App/Netdisco/Backend/Worker/Poller/Nbtstat.pm

@@ -5,7 +5,7 @@ use Dancer::Plugin::DBIC 'schema';
 
 use App::Netdisco::Core::Nbtstat qw/nbtstat_resolve_async store_nbt/;
 use App::Netdisco::Util::Node 'is_nbtstatable';
-use App::Netdisco::Util::Device qw/get_device is_discoverable/;
+use App::Netdisco::Util::Device qw/get_device is_macsuckable/;
 use App::Netdisco::Backend::Util ':all';
 
 use NetAddr::IP::Lite ':lower';
@@ -29,8 +29,8 @@ sub nbtstat  {
     or job_error("nbtstat failed: unable to interpret device parameter");
   my $host = $device->ip;
 
-  unless (is_discoverable($device->ip)) {
+  unless (is_macsuckable($device->ip)) {
-      return job_defer("nbtstat deferred: $host is not discoverable");
+      return job_defer("nbtstat deferred: $host is not macsuckable");
   }
 
   # get list of nodes on device

lib/App/Netdisco/Util/Device.pm

@@ -11,9 +11,9 @@ our @EXPORT_OK = qw/
   delete_device
   renumber_device
   match_devicetype
-  is_discoverable
+  is_discoverable is_discoverable_now
-  is_arpnipable
+  is_arpnipable   is_arpnipable_now
-  is_macsuckable
+  is_macsuckable  is_macsuckable_now
 /;
 our %EXPORT_TAGS = (all => \@EXPORT_OK);
 
@@ -144,6 +144,8 @@ sub match_devicetype {
                         @{setting($setting_name) || []});
 }
 
+sub _bail_msg { debug $_[0]; return 0; }
+
 =head2 is_discoverable( $ip, $device_type? )
 
 Given an IP address, returns C<true> if Netdisco on this host is permitted by
@@ -159,8 +161,6 @@ Returns false if the host is not permitted to discover the target device.
 
 =cut
 
-sub _bail_msg { debug $_[0]; return 0; }
-
 sub is_discoverable {
   my ($ip, $remote_type) = @_;
   my $device = get_device($ip) or return 0;
@@ -175,16 +175,32 @@ sub is_discoverable {
   return _bail_msg("is_discoverable: device failed to match discover_only")
     unless check_acl_only($device, 'discover_only');
 
-  # cannot check last_discover for as yet undiscovered devices :-)
+  return 1;
-  return 1 if not $device->in_storage;
+}
 
-  if ($device->since_last_discover and setting('discover_min_age')
+=head2 is_discoverable_now( $ip, $device_type? )
-      and $device->since_last_discover < setting('discover_min_age')) {
 
-      return _bail_msg("is_discoverable: time since last discover less than discover_min_age");
+Same as C<is_discoverable>, but also checks the last_discover field if the
+device is in storage, and returns false if that host has been too recently
+discovered.
+
+Returns false if the host is not permitted to discover the target device.
+
+=cut
+
+sub is_discoverable_now {
+  my ($ip, $remote_type) = @_;
+  my $device = get_device($ip) or return 0;
+
+  if ($device->in_storage) {
+    if ($device->since_last_discover and setting('discover_min_age')
+        and $device->since_last_discover < setting('discover_min_age')) {
+
+        return _bail_msg("is_discoverable: time since last discover less than discover_min_age");
+    }
   }
 
-  return 1;
+  return is_discoverable(@_);
 }
 
 =head2 is_arpnipable( $ip )
@@ -203,12 +219,32 @@ sub is_arpnipable {
   my $ip = shift;
   my $device = get_device($ip) or return 0;
 
+  return _bail_msg("is_arpnipable: cannot arpnip an undiscovered device")
+    if not $device->in_storage;
+
   return _bail_msg("is_arpnipable: device matched arpnip_no")
     if check_acl_no($device, 'arpnip_no');
 
   return _bail_msg("is_arpnipable: device failed to match arpnip_only")
     unless check_acl_only($device, 'arpnip_only');
 
+  return 1;
+}
+
+=head2 is_arpnipable_now( $ip )
+
+Same as C<is_arpnipable>, but also checks the last_arpnip field if the
+device is in storage, and returns false if that host has been too recently
+arpnipped.
+
+Returns false if the host is not permitted to arpnip the target device.
+
+=cut
+
+sub is_arpnipable_now {
+  my $ip = shift;
+  my $device = get_device($ip) or return 0;
+
   return _bail_msg("is_arpnipable: cannot arpnip an undiscovered device")
     if not $device->in_storage;
 
@@ -218,7 +254,7 @@ sub is_arpnipable {
       return _bail_msg("is_arpnipable: time since last arpnip less than arpnip_min_age");
   }
 
-  return 1;
+  return is_arpnipable(@_);
 }
 
 =head2 is_macsuckable( $ip )
@@ -237,13 +273,33 @@ sub is_macsuckable {
   my $ip = shift;
   my $device = get_device($ip) or return 0;
 
+  return _bail_msg("is_macsuckable: cannot macksuck an undiscovered device")
+    if not $device->in_storage;
+
   return _bail_msg("is_macsuckable: device matched macsuck_no")
     if check_acl_no($device, 'macsuck_no');
 
   return _bail_msg("is_macsuckable: device failed to match macsuck_only")
     unless check_acl_only($device, 'macsuck_only');
 
-  return _bail_msg("is_macsuckable: cannot macsuck an undiscovered device")
+  return 1;
+}
+
+=head2 is_macsuckable_now( $ip )
+
+Same as C<is_macsuckable>, but also checks the last_macsuck field if the
+device is in storage, and returns false if that host has been too recently
+macsucked.
+
+Returns false if the host is not permitted to macsuck the target device.
+
+=cut
+
+sub is_macsuckable_now {
+  my $ip = shift;
+  my $device = get_device($ip) or return 0;
+
+  return _bail_msg("is_macsuckable: cannot macksuck an undiscovered device")
     if not $device->in_storage;
 
   if ($device->since_last_macsuck and setting('macsuck_min_age')
@@ -252,7 +308,7 @@ sub is_macsuckable {
       return _bail_msg("is_macsuckable: time since last macsuck less than macsuck_min_age");
   }
 
-  return 1;
+  return is_macsuckable(@_);
 }
 
 1;