hydra-notify: move BuildFinished processing to an Event

src/lib/Hydra/Event/BuildFinished.pm

@@ -18,8 +18,48 @@ sub parse :prototype(@) {
 }
 
 sub new {
-    my ($self, $build_id, $dependencies) = @_;
+    my ($self, $build_id, $dependent_ids) = @_;
-    return bless { "build_id" => $build_id, "dependencies" => $dependencies }, $self;
+    return bless {
+        "build_id" => $build_id,
+        "dependent_ids" => $dependent_ids,
+        "build" => undef,
+        "dependents" => [],
+    }, $self;
+}
+
+sub load {
+    my ($self, $db) = @_;
+
+    if (!defined($self->{"build"})) {
+        $self->{"build"} = $db->resultset('Builds')->find($self->{"build_id"})
+            or die "build $self->{'build_id'} does not exist\n";
+
+        foreach my $id (@{$self->{"dependent_ids"}}) {
+            my $dep = $db->resultset('Builds')->find($id)
+                or die "dependent build $id does not exist\n";
+            push @{$self->{"dependents"}}, $dep;
+        }
+    }
+}
+
+sub execute {
+    my ($self, $db, $plugin) = @_;
+
+    $self->load($db);
+
+    $plugin->buildFinished($self->{"build"}, $self->{"dependents"});
+
+    # Mark the build and all dependents as having their notifications "finished".
+    #
+    # Otherwise, the dependent builds will remain with notificationpendingsince set
+    # until hydra-notify is started, as buildFinished is never emitted for them.
+    foreach my $b ($self->{"build"}, @{$self->{"dependents"}}) {
+        if ($b->finished && defined($b->notificationpendingsince)) {
+            $b->update({ notificationpendingsince => undef })
+        }
+    }
+
+    return 1;
 }
 
 1;

src/script/hydra-notify

@@ -2,10 +2,11 @@
 
 use strict;
 use utf8;
+use Hydra::Event::BuildFinished;
+use Hydra::Helper::AddBuilds;
+use Hydra::Helper::Nix;
 use Hydra::Plugin;
 use Hydra::PostgresListener;
-use Hydra::Helper::Nix;
-use Hydra::Helper::AddBuilds;
 use Getopt::Long;
 
 STDERR->autoflush(1);
@@ -44,44 +45,15 @@ sub runPluginsForEvent {
     }
 }
 
-sub buildFinished {
-    my ($buildId, @deps) = @_;
-
-    my $build = $db->resultset('Builds')->find($buildId)
-    or die "build $buildId does not exist\n";
-
-    my @dependents;
-    foreach my $id (@deps) {
-        my $dep = $db->resultset('Builds')->find($id)
-            or die "build $id does not exist\n";
-        push @dependents, $dep;
-    }
-
-    foreach my $plugin (@plugins) {
-        eval {
-            $plugin->buildFinished($build, [@dependents]);
-            1;
-        } or do {
-            print STDERR "error with $plugin->buildFinished: $@\n";
-        }
-    }
-
-    # We have to iterate through all dependents as well, and if they are finished
-    # to mark their notificationpendingsince.
-    # Otherwise, the dependent builds will remain with notificationpendingsince set
-    # until hydra-notify is started, as buildFinished is never emitted for them.
-    foreach my $b ($build, @dependents) {
-        $b->update({ notificationpendingsince => undef }) if $b->finished;
-    }
-}
-
 # Process builds that finished while hydra-notify wasn't running.
 for my $build ($db->resultset('Builds')->search(
                    { notificationpendingsince => { '!=', undef } }))
 {
-    my $buildId = $build->id;
+    print STDERR "sending notifications for build $build->id...\n";
-    print STDERR "sending notifications for build ${\$buildId}...\n";
+
-    buildFinished($build->id);
+
+    my $event = Hydra::Event::BuildFinished->new($build->id);
+    runPluginsForEvent($event);
 }
 
 
@@ -93,18 +65,11 @@ while (!$queued_only) {
         my $channelName = $message->{"channel"};
         my $pid = $message->{"pid"};
         my $payload = $message->{"payload"};
-        #print STDERR "got '$channelName' from $pid: $payload\n";
-
-        my @payload = split /\t/, $payload;
 
         eval {
-            if ($channelName eq "build_started" || $channelName eq "step_finished" ) {
             my $event = Hydra::Event::new_event($channelName, $message->{"payload"});
             runPluginsForEvent($event);
-            } elsif ($channelName eq "build_finished") {
+
-                my $buildId = int($payload[0]);
-                buildFinished($buildId, @payload[1..$#payload]);
-            }
             1;
         } or do {
             print STDERR "error processing message '$payload' on channel '$channelName': $@\n";

t/Event.t

@@ -1,6 +1,5 @@
 use strict;
 use Hydra::Event;
-use Hydra::Event::BuildFinished;
 
 use Test2::V0;
 use Test2::Tools::Exception;
@@ -12,41 +11,6 @@ subtest "Event: new event" => sub {
     is($event->{'event'}->{'build_id'}, 19);
 };
 
-
-subtest "Payload type: build_finished" => sub {
-    like(
-        dies { Hydra::Event::parse_payload("build_finished", "") },
-        qr/at least one argument/,
-        "empty payload"
-    );
-    like(
-        dies { Hydra::Event::parse_payload("build_finished", "abc123") },
-        qr/should be integers/,
-        "build ID should be an integer"
-    );
-    like(
-        dies { Hydra::Event::parse_payload("build_finished", "123\tabc123") },
-        qr/should be integers/,
-        "dependent ID should be an integer"
-    );
-    is(
-        Hydra::Event::parse_payload("build_finished", "123"),
-        Hydra::Event::BuildFinished->new(123, []),
-        "no dependent builds"
-    );
-    is(
-        Hydra::Event::parse_payload("build_finished", "123\t456"),
-        Hydra::Event::BuildFinished->new(123, [456]),
-        "one dependent build"
-    );
-    is(
-        Hydra::Event::parse_payload("build_finished", "123\t456\t789\t012\t345"),
-        Hydra::Event::BuildFinished->new(123, [456, 789, 12, 345]),
-        "four dependent builds"
-    );
-};
-
-
 subtest "Payload type: bogus" => sub {
     like(
         dies { Hydra::Event::parse_payload("bogus", "") },

t/Event/BuildFinished.t

@@ -0,0 +1,92 @@
+use strict;
+use Setup;
+
+my %ctx = test_init();
+
+require Hydra::Schema;
+require Hydra::Model::DB;
+use Hydra::Event;
+use Hydra::Event::BuildFinished;
+
+use Test2::V0;
+use Test2::Tools::Exception;
+use Test2::Tools::Mock qw(mock_obj);
+
+my $db = Hydra::Model::DB->new;
+hydra_setup($db);
+
+subtest "Parsing" => sub {
+    like(
+        dies { Hydra::Event::parse_payload("build_finished", "") },
+        qr/at least one argument/,
+        "empty payload"
+    );
+    like(
+        dies { Hydra::Event::parse_payload("build_finished", "abc123") },
+        qr/should be integers/,
+        "build ID should be an integer"
+    );
+    like(
+        dies { Hydra::Event::parse_payload("build_finished", "123\tabc123") },
+        qr/should be integers/,
+        "dependent ID should be an integer"
+    );
+    is(
+        Hydra::Event::parse_payload("build_finished", "123"),
+        Hydra::Event::BuildFinished->new(123, []),
+        "no dependent builds"
+    );
+    is(
+        Hydra::Event::parse_payload("build_finished", "123\t456"),
+        Hydra::Event::BuildFinished->new(123, [456]),
+        "one dependent build"
+    );
+    is(
+        Hydra::Event::parse_payload("build_finished", "123\t456\t789\t012\t345"),
+        Hydra::Event::BuildFinished->new(123, [456, 789, 12, 345]),
+        "four dependent builds"
+    );
+};
+
+my $project = $db->resultset('Projects')->create({name => "tests", displayname => "", owner => "root"});
+my $jobset = createBaseJobset("basic", "basic.nix", $ctx{jobsdir});
+ok(evalSucceeds($jobset),               "Evaluating jobs/basic.nix should exit with return code 0");
+is(nrQueuedBuildsForJobset($jobset), 3, "Evaluating jobs/basic.nix should result in 3 builds");
+
+subtest "load" => sub {
+    my ($build, $dependent_a, $dependent_b) = $db->resultset('Builds')->search(
+      { },
+      { limit => 3 }
+    )->all;
+
+    my $event = Hydra::Event::BuildFinished->new($build->id, [$dependent_a->id, $dependent_b->id]);
+
+    $event->load($db);
+
+    is($event->{"build"}->id, $build->id, "The build record matches.");
+    is($event->{"dependents"}[0]->id, $dependent_a->id, "The dependent_a record matches.");
+    is($event->{"dependents"}[1]->id, $dependent_b->id, "The dependent_b record matches.");
+
+    # Create a fake "plugin" with a buildFinished sub, the sub sets this
+    # global passedBuild and passedDependents variables for verifying.
+    my $passedBuild;
+    my $passedDependents;
+    my $plugin = {};
+    my $mock = mock_obj $plugin => (
+        add => [
+            "buildFinished" => sub {
+                my ($self, $build, $dependents) = @_;
+                $passedBuild = $build;
+                $passedDependents = $dependents;
+            }
+        ]
+    );
+
+    $event->execute($db, $plugin);
+
+    is($passedBuild->id, $build->id, "The plugin's buildFinished hook is called with a matching build");
+    is($passedDependents->[0]->id, $dependent_a->id, "The plugin's buildFinished hook is called with a matching dependent_a");
+    is($passedDependents->[1]->id, $dependent_b->id, "The plugin's buildFinished hook is called with a matching dependent_b");
+};
+
+done_testing;