Merge pull request #1215 from matsduf/redefine-batch_status-api

Redefines batch_status to replace get_batch_job_result

Author: matsduf

lib/Zonemaster/Backend/DB.pm

@@ -765,6 +765,60 @@ sub get_batch_job_result {
     return \%result;
 }
 
+=head2 batch_status
+
+Returns number of tests per category (finished, running, waiting) for the given
+batch, provided as C<batch_id>.
+
+If one or more of parameters C<list_running_tests>, C<list_finished_tests> or
+C<list_waiting_tests> are included with true value, the C<hash_id> values for
+that category is also included.
+
+=cut
+
+# Standard SQL, can be here
+sub batch_status {
+    my ( $self, $test_params ) = @_;
+
+    my $batch_id = $test_params->{batch_id};
+
+    die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Unknown batch", data => { batch_id => $batch_id } )
+        unless defined $self->batch_exists_in_db( $batch_id );
+
+    my $dbh = $self->dbh;
+
+    my %result;
+    $result{waiting_count} = 0;
+    $result{running_count} = 0;
+    $result{finished_count} = 0;
+
+    my $query = "
+        SELECT hash_id, progress
+        FROM test_results
+        WHERE batch_id=?";
+
+    my $sth1 = $dbh->prepare( $query );
+    $sth1->execute( $batch_id );
+
+    while ( my $h = $sth1->fetchrow_hashref ) {
+        if ( $h->{progress} eq '0' ) {
+            $result{waiting_count}++;
+            push(@{$result{waiting_tests}}, $h->{hash_id}) if $test_params->{list_waiting_tests};
+        }
+        elsif ( $h->{progress} eq '100' ) {
+            $result{finished_count}++;
+            push(@{$result{finished_tests}}, $h->{hash_id}) if $test_params->{list_finished_tests};
+        }
+        else {
+            $result{running_count}++;
+            push(@{$result{running_tests}}, $h->{hash_id}) if $test_params->{list_running_tests};
+        }
+    }
+
+    return \%result;
+}
+
+
 =head2 process_unfinished_tests($queue_label, $test_run_timeout)
 
 Append a new log entry C<BACKEND_TEST_AGENT:UNABLE_TO_FINISH_TEST> to all the

lib/Zonemaster/Backend/RPCAPI.pm

@@ -737,9 +737,11 @@ sub batch_create {
     return $result;
 }
 
+# Deprecated to be removed in v2025.2.
 $json_schemas{get_batch_job_result} = joi->object->strict->props(
     batch_id => $zm_validator->batch_id->required
 );
+# Deprecated to be removed in v2025.2.
 sub get_batch_job_result {
     my ( $self, $params ) = @_;
 
@@ -756,15 +758,29 @@ sub get_batch_job_result {
     return $result;
 }
 
-# Experimental
-$json_schemas{batch_status} = $json_schemas{get_batch_job_result};
+
+$json_schemas{batch_status} = {
+    type => 'object',
+    additionalProperties => 0,
+    required => [ 'batch_id' ],
+    properties => {
+        batch_id => $zm_validator->batch_id->required,
+        list_waiting_tests => joi->boolean->compile,
+        list_running_tests => joi->boolean->compile,
+        list_finished_tests => joi->boolean->compile,
+    }
+};
 sub batch_status {
-    my $result = get_batch_job_result( @_ );
-    return {
-        running_count    => $result->{nb_running},
-        finished_count   => $result->{nb_finished},
-        finished_job_ids => $result->{finished_job_ids},
+    my ( $self, $params ) = @_;
+
+    my $result;
+    eval {
+        $result = $self->{db}->batch_status($params);
     };
+    if ($@) {
+        handle_exception( $@ );
+    }
+    return $result;
 }
 
 sub _get_locale {

script/zmb

@@ -584,8 +584,7 @@ sub cmd_add_batch_job {
     );
 }
 
-
-=head2 get_batch_job_result
+=head2 get_batch_job_result (** Deprecated to be removed in Zonemaster version v2025.2. Use C<batch_status> instead. **)
 
  zmb [GLOBAL OPTIONS] get_batch_job_result [OPTIONS]
 
@@ -613,6 +612,80 @@ sub cmd_get_batch_job_result {
 }
 
 
+=head2 batch_status
+
+ zmb [GLOBAL OPTIONS] batch_status [OPTIONS]
+
+ Options:
+   --batch-id BATCH-ID|--bi BATCH-ID
+   --list-waiting-tests true|false|null
+   --list-running-tests true|false|null
+   --list-finished-tests true|false|null
+
+   --lw # Same as "--list-waiting-tests true"
+   --lr # Same as "--list-running-tests true"
+   --lf # Same as "--list-finished-tests true"
+
+"--batch-id" is mandatory.
+
+The command provides the number of tests waiting to be run, tests running and
+test finished, respectively, for the batch.
+
+"--list-waiting-tests", "--list-running-tests" and "--list-finished-tests" are
+optional. If given the test IDs of tests waiting to be run, tests running
+and test finished, respectively, are listed.
+
+"--lw", "--lr" and "--lf" are option.
+
+"--lw" must not be combined with "--list-waiting-tests". "--lr" must not be
+combined with "--list-running-tests". "--lf" must not be combined with
+"--list-finished-tests".
+=cut
+
+sub cmd_batch_status {
+    my @opts = @_;
+
+    my $opt_batch_id;
+    my $opt_list_waiting_tests;
+    my $opt_lw;
+    my $opt_list_running_tests;
+    my $opt_lr;
+    my $opt_list_finished_tests;
+    my $opt_lf;
+
+    GetOptionsFromArray(
+        \@opts,
+        'batch-id|bi=s'         => \$opt_batch_id,
+        'list-waiting-tests=s'  => \$opt_list_waiting_tests,
+        'lw'                    => \$opt_lw,
+        'list-running-tests=s'  => \$opt_list_running_tests,
+        'lr'                    => \$opt_lr,
+        'list-finished-tests=s' => \$opt_list_finished_tests,
+        'lf'                    => \$opt_lf,
+    ) or pod2usage( 2 );
+
+    pod2usage( "'--lw' and '--list-waiting-test' must not be combined" )  if defined $opt_list_waiting_tests and $opt_lw;
+    pod2usage( "'--lr' and '--list-running-test' must not be combined" )  if defined $opt_list_running_tests and $opt_lr;
+    pod2usage( "'--lf' and '--list-finished-test' must not be combined" ) if defined $opt_list_finished_tests and $opt_lf;
+    
+    my %params;
+    $params{batch_id} = $opt_batch_id;
+    $params{list_waiting_tests} = json_tern( $opt_list_waiting_tests ) if $opt_list_waiting_tests and json_tern( $opt_list_waiting_tests );
+    $params{list_running_tests} = json_tern( $opt_list_running_tests ) if $opt_list_running_tests and json_tern( $opt_list_running_tests );
+    $params{list_finished_tests} = json_tern( $opt_list_finished_tests ) if $opt_list_finished_tests and json_tern( $opt_list_finished_tests );
+
+    $params{list_waiting_tests} = JSON::PP::true if $opt_lw;
+    $params{list_running_tests} = JSON::PP::true if $opt_lr;
+    $params{list_finished_tests} = JSON::PP::true if $opt_lf;
+
+    return to_jsonrpc(
+        id     => 1,
+        method => 'batch_status',
+        params => \%params,
+    );
+}
+
+
 sub show_commands {
     my %specials = (
         man                 => 'Show the full manual page.',
@@ -643,8 +716,10 @@ sub get_commands {
       grep { $_ =~ /^cmd_/ } grep { defined &{"main\::$_"} } keys %{"main\::"};
 }
 
+
 sub json_tern {
     my $value = shift;
+
     if ( $value eq 'true' ) {
         return JSON::PP::true;
     }

script/zonemaster_backend_rpcapi.psgi

@@ -169,12 +169,12 @@ my $router = router {
         action => "domain_history"
     };
 
+    # Deprecated to be removed v2025.2
     connect "get_batch_job_result" => {
         handler => $handler,
         action => "get_batch_job_result"
     };
 
-    # Experimental
     connect "batch_status" => {
         handler => $handler,
         action => "batch_status"

t/batches.t

@@ -8,6 +8,7 @@ use POSIX qw( strftime );
 use Time::Local qw( timelocal_modern );
 use Test::Exception;
 use Test::More;    # see done_testing()
+use Test::Differences;
 
 my $t_path;
 BEGIN {
@@ -150,7 +151,8 @@ subtest 'RPCAPI add_batch_job' => sub {
     };
 };
 
-subtest 'RPCAPI get_batch_job_result' => sub {
+subtest 'RPCAPI get_batch_job_result and batch_status' => sub {
+    # "get_batch_job_result deprecated to be removed by v2025.2
     my $config = Zonemaster::Backend::Config->parse( $config );
     my $rpcapi = init_backend( $config );
     subtest 'batch job exists' => sub {
@@ -166,13 +168,26 @@ subtest 'RPCAPI get_batch_job_result' => sub {
 
         is( $batch_id, 1, 'correct batch job id returned' );
 
-        my $res = $rpcapi->get_batch_job_result( { batch_id => $batch_id } );
+        {
+            my $res = $rpcapi->get_batch_job_result( { batch_id => $batch_id } );
+
+            is( $res->{nb_running}, scalar @domains, 'correct number of runninng tests' );
+            is( $res->{nb_finished}, 0, 'correct number of finished tests' );
+        }
 
-        is( $res->{nb_running}, @domains, 'correct number of runninng tests' );
-        is( $res->{nb_finished}, 0, 'correct number of finished tests' );
+        {
+            my $res = $rpcapi->batch_status( { batch_id => $batch_id } );
+
+            is( $res->{waiting_count}, scalar @domains, 'correct number of runninng tests' );
+            is( $res->{running_count}, 0, 'correct number of finished tests' );
+            is( $res->{finished_count}, 0, 'correct number of finished tests' );
+            ok( !exists $res->{waiting_tests}, 'list of waiting tests expected to be absent' );
+            ok( !exists $res->{running_tests}, 'list of running tests expected to be absent' );
+            ok( !exists $res->{finished_tests}, 'list of finished tests to be absent' );
+        }
     };
 
-    subtest 'unknown batch' => sub {
+    subtest 'unknown batch (get_batch_job_result)' => sub {
         my $unknown_batch = 10;
         dies_ok {
             $rpcapi->get_batch_job_result( { batch_id => $unknown_batch } );
@@ -182,6 +197,17 @@ subtest 'RPCAPI get_batch_job_result' => sub {
         is( $res->{message}, 'Unknown batch', 'correct error message' );
         is( $res->{data}->{batch_id}, $unknown_batch, 'correct data type returned' );
     };
+
+    subtest 'unknown batch (batch_status)' => sub {
+        my $unknown_batch = 10;
+        dies_ok {
+            $rpcapi->batch_status( { batch_id => $unknown_batch } );
+        } 'getting results for an unknown batch_id should die';
+        my $res = $@;
+        is( $res->{error}, 'Zonemaster::Backend::Error::ResourceNotFound', 'correct error type' );
+        is( $res->{message}, 'Unknown batch', 'correct error message' );
+        is( $res->{data}->{batch_id}, $unknown_batch, 'correct data type returned' );
+    };
 };
 
 subtest 'batch with several domains' => sub {
@@ -201,11 +227,32 @@ subtest 'batch with several domains' => sub {
 
     is( $res, 1, 'correct batch job id returned' );
 
+    # "get_batch_job_result deprecated to be removed by v2025.2
     $res = $rpcapi->get_batch_job_result( { batch_id => 1 } );
 
     is( $res->{nb_running}, @domains, 'correct number of runninng tests' );
     is( $res->{nb_finished}, 0, 'correct number of finished tests' );
 
+    # No lists of test IDs requested
+    $res = $rpcapi->batch_status( { batch_id => 1 } );
+
+    is( $res->{waiting_count}, scalar @domains, 'correct number of running tests' );
+    is( $res->{running_count}, 0, 'correct number of finished tests' );
+    is( $res->{finished_count}, 0, 'correct number of finished tests' );
+    ok( !exists $res->{waiting_tests}, 'list of waiting tests expected to be absent' );
+    ok( !exists $res->{running_tests}, 'list of running tests expected to be absent' );
+    ok( !exists $res->{finished_tests}, 'list of finished tests expected to be absent' );
+
+    # List of waiting test IDs requested
+    $res = $rpcapi->batch_status( { batch_id => 1, list_waiting_tests => 1 } );
+
+    is( $res->{waiting_count}, scalar @domains, 'correct number of runninng tests' );
+    is( $res->{running_count}, 0, 'correct number of finished tests' );
+    is( $res->{finished_count}, 0, 'correct number of finished tests' );
+    is( scalar @{ $res->{waiting_tests} }, scalar @domains, 'correct number of elements in waiting_tests' );
+    ok( !exists $res->{running_tests}, 'list of running tests expected to be absent' );
+    ok( !exists $res->{finished_tests}, 'list of finished tests expected to be absent' );
+
     subtest 'table "test_results" contains 2 entries' => sub {
         my ( $count ) = $dbh->selectrow_array( q[ SELECT count(*) FROM test_results ] );
         is( $count, @domains, 'two rows in table' );