From 1eeaba441d2dc6a3b98cba548727df3fee4eddb8 Mon Sep 17 00:00:00 2001
From: Oliver Gorwits <oliver@cpan.org>
Date: Tue, 7 Nov 2017 22:30:55 +0000
Subject: [PATCH] finish refactor to new desired behaviour (buggy?)

---
 lib/App/Netdisco/Backend/Job.pm             | 33 ++++++++++++++-------
 lib/App/Netdisco/Worker/Plugin.pm           |  4 +--
 lib/App/Netdisco/Worker/Plugin/Test.pm      | 20 ++++++-------
 lib/App/Netdisco/Worker/Plugin/Test/Core.pm | 26 +++++++++++-----
 lib/App/Netdisco/Worker/Runner.pm           | 12 ++++----
 lib/App/Netdisco/Worker/Status.pm           |  2 +-
 6 files changed, 60 insertions(+), 37 deletions(-)

diff --git a/lib/App/Netdisco/Backend/Job.pm b/lib/App/Netdisco/Backend/Job.pm
index f14692d0..c92efbdf 100644
--- a/lib/App/Netdisco/Backend/Job.pm
+++ b/lib/App/Netdisco/Backend/Job.pm
@@ -20,7 +20,7 @@ foreach my $slot (qw/
       userip
       log
 
-      _last_phase
+      _current_phase
       _last_namespace
       _last_priority
     /) {
@@ -66,6 +66,8 @@ sub finalise_status {
   $job->log('failed to report from any worker!');
 
   foreach my $status (@{ $job->_statuslist }) {
+    next unless $status->phase 
+      and $status->phase =~ m/^(?:check|early|main)$/;
     if ($status->level >= $max_level) {
       $job->status( $status->status );
       $job->log( $status->log );
@@ -84,6 +86,7 @@ C<done>.
 sub check_passed {
   my $job = shift;
   foreach my $status (@{ $job->_statuslist }) {
+    next unless $status->phase and $status->phase eq 'check';
     return true if $status->is_ok;
   }
   return false;
@@ -91,8 +94,8 @@ sub check_passed {
 
 =head2 namespace_passed( \%workerconf )
 
-Returns true when, for the namespace specified in the passed configuration,
-all workers of a higher priority level have succeeded.
+Returns true when, for the namespace specified in the passed configuration, a
+worker of a higher priority level has already succeeded.
 
 =cut
 
@@ -101,33 +104,41 @@ sub namespace_passed {
 
   if ($job->_last_namespace) {
     foreach my $status (@{ $job->_statuslist }) {
-      next unless ($workerconf->{phase} eq $job->_last_phase)
+      next unless ($status->phase and $status->phase eq $workerconf->{phase})
               and ($workerconf->{namespace} eq $job->_last_namespace)
-              and ($workerconf->{priority} != $job->_last_priority);
+              and ($workerconf->{priority} > $job->_last_priority);
       return true if $status->is_ok;
     }
   }
 
-  # reset the internal status cache when the phase changes
-  $job->_statuslist([]) if $job->_last_phase
-    and $job->_last_phase ne $workerconf->{phase};
-
-  $job->_last_phase( $workerconf->{phase} );
   $job->_last_namespace( $workerconf->{namespace} );
   $job->_last_priority( $workerconf->{priority} );
   return false;
 }
 
+=head2 enter_phase( $phase )
+
+Pass the name of the phase being entered.
+
+=cut
+
+sub enter_phase {
+  my ($job, $phase) = @_;
+  $job->_current_phase( $phase );
+  debug "=> running workers for phase: $phase";
+}
+
 =head2 add_status
 
 Passed an L<App::Netdisco::Worker::Status> will add it to this job's internal
-status cache.
+status cache. Phase slot of the Status will be set to the current phase.
 
 =cut
 
 sub add_status {
   my ($job, $status) = @_;
   return unless ref $status eq 'App::Netdisco::Worker::Status';
+  $status->phase( $job->_current_phase );
   push @{ $job->_statuslist }, $status;
   debug $status->log if $status->log;
 }
diff --git a/lib/App/Netdisco/Worker/Plugin.pm b/lib/App/Netdisco/Worker/Plugin.pm
index a3cc3337..53af07cd 100644
--- a/lib/App/Netdisco/Worker/Plugin.pm
+++ b/lib/App/Netdisco/Worker/Plugin.pm
@@ -43,7 +43,7 @@ register 'register_worker' => sub {
       my $no   = (exists $workerconf->{no}   ? $workerconf->{no}   : undef);
       my $only = (exists $workerconf->{only} ? $workerconf->{only} : undef);
 
-      return $job->defer('worker is not applicable to this device')
+      return $job->add_status( Status->defer('worker is not applicable to this device') )
         if ($no and check_acl_no($job->device, $no))
            or ($only and not check_acl_only($job->device, $only));
 
@@ -59,7 +59,7 @@ register 'register_worker' => sub {
       }
 
       # per-device action but no device creds available
-      return $job->defer('deferred job with no device creds')
+      return $job->add_status( Status->defer('deferred job with no device creds') )
         if 0 == scalar @newuserconf;
     }
 
diff --git a/lib/App/Netdisco/Worker/Plugin/Test.pm b/lib/App/Netdisco/Worker/Plugin/Test.pm
index 75ba809a..96a7c6f6 100644
--- a/lib/App/Netdisco/Worker/Plugin/Test.pm
+++ b/lib/App/Netdisco/Worker/Plugin/Test.pm
@@ -4,24 +4,24 @@ use Dancer ':syntax';
 use App::Netdisco::Worker::Plugin;
 use aliased 'App::Netdisco::Worker::Status';
 
-register_worker({ phase => 'main' }, sub {
-  my ($job, $workerconf) = @_;
-  return Status->done('Test (main) ran successfully (1).');
-});
-
-register_worker({ phase => 'check' }, sub {
+register_worker({ phase => 'check', driver => 'snmp' }, sub {
   my ($job, $workerconf) = @_;
   return Status->done('Test (check) ran successfully.');
 });
 
-register_worker({ phase => 'early' }, sub {
+register_worker({ phase => 'check', priority => 100 }, sub {
   my ($job, $workerconf) = @_;
-  return Status->error('Test (early) ran successfully.');
+  return Status->done('Test (check 100) ran successfully.');
 });
 
-register_worker(sub {
+register_worker({ phase => 'check', priority => 120 }, sub {
   my ($job, $workerconf) = @_;
-  return Status->noop('Test (undefined) ran successfully.');
+  return Status->done('Test (check 120) ran successfully.');
+});
+
+register_worker({ phase => 'check', driver => 'eapi' }, sub {
+  my ($job, $workerconf) = @_;
+  return Status->done('Test (check eapi) ran successfully.');
 });
 
 true;
diff --git a/lib/App/Netdisco/Worker/Plugin/Test/Core.pm b/lib/App/Netdisco/Worker/Plugin/Test/Core.pm
index e64fb280..619ad4c6 100644
--- a/lib/App/Netdisco/Worker/Plugin/Test/Core.pm
+++ b/lib/App/Netdisco/Worker/Plugin/Test/Core.pm
@@ -4,14 +4,9 @@ use Dancer ':syntax';
 use App::Netdisco::Worker::Plugin;
 use aliased 'App::Netdisco::Worker::Status';
 
-register_worker({ phase => 'main' }, sub {
+register_worker({ phase => 'early' }, sub {
   my ($job, $workerconf) = @_;
-  return Status->done('Test (main) ran successfully (2).');
-});
-
-register_worker({ phase => 'check' }, sub {
-  my ($job, $workerconf) = @_;
-  return Status->done('Test (check) ran successfully.');
+  return Status->error('Test (early) ran successfully.');
 });
 
 register_worker({ phase => 'early' }, sub {
@@ -19,9 +14,24 @@ register_worker({ phase => 'early' }, sub {
   return Status->done('Test (early) ran successfully.');
 });
 
+register_worker({ phase => 'main' }, sub {
+  my ($job, $workerconf) = @_;
+  return Status->done('Test (main) ran successfully (1).');
+});
+
+register_worker({ phase => 'main' }, sub {
+  my ($job, $workerconf) = @_;
+  return Status->done('Test (main) ran successfully (2).');
+});
+
 register_worker(sub {
   my ($job, $workerconf) = @_;
-  return Status->error('Test (undefined) ran successfully.');
+  return Status->noop('Test (user) ran successfully.');
+});
+
+register_worker(sub {
+  my ($job, $workerconf) = @_;
+  return Status->error('Test (user) ran successfully.');
 });
 
 true;
diff --git a/lib/App/Netdisco/Worker/Runner.pm b/lib/App/Netdisco/Worker/Runner.pm
index 91114c10..a251f6ff 100644
--- a/lib/App/Netdisco/Worker/Runner.pm
+++ b/lib/App/Netdisco/Worker/Runner.pm
@@ -5,6 +5,7 @@ use Dancer::Factory::Hook;
 
 use App::Netdisco::Util::Permission qw/check_acl_no check_acl_only/;
 use App::Netdisco::Util::Device 'get_device';
+use aliased 'App::Netdisco::Worker::Status';
 
 use Try::Tiny;
 use Module::Load ();
@@ -43,7 +44,7 @@ sub run {
     }
 
     # per-device action but no device creds available
-    return $job->defer('deferred job with no device creds')
+    return $job->add_status( Status->defer('deferred job with no device creds') )
       if 0 == scalar @newuserconf;
   }
 
@@ -51,7 +52,7 @@ sub run {
   my $configguard = guard { set(device_auth => \@userconf) };
   set(device_auth => \@newuserconf);
 
-  # finalise job status when we exit XXX FIXME
+  # finalise job status when we exit
   my $statusguard = guard { $job->finalise_status };
 
   # run check phase and if there are workers then one MUST be successful
@@ -65,19 +66,20 @@ sub run {
 sub run_workers {
   my $self = shift;
   my $job  = $self->job or die error 'no job in worker job slot';
-  my $hook = shift or return $job->error('missing hook param'); # XXX FIXME
+  my $hook = shift
+    or return $job->add_status( Status->error('missing hook param') );
 
   my $store = Dancer::Factory::Hook->instance();
   (my $phase = $hook) =~ s/^nd2_core_//;
 
   return unless scalar @{ $store->get_hooks_for($hook) };
-  debug "=> running workers for phase: $phase";
+  $job->enter_phase($phase);
 
   foreach my $worker (@{ $store->get_hooks_for($hook) }) {
     try { $job->add_status( $worker->($job) ) }
     catch {
       debug "=> $_" if $_;
-      $job->error($_);
+      $job->add_status( Status->error($_) );
     };
   }
 }
diff --git a/lib/App/Netdisco/Worker/Status.pm b/lib/App/Netdisco/Worker/Status.pm
index 75d3d738..b8234f75 100644
--- a/lib/App/Netdisco/Worker/Status.pm
+++ b/lib/App/Netdisco/Worker/Status.pm
@@ -13,7 +13,7 @@ has 'status' => (
   default => undef,
 );
 
-has 'log' => (
+has [qw/log phase/] => (
   is => 'rw',
   default => '',
 );