Require a human review for all auto-accepts

lib/Cavil/Model/Packages.pm

@@ -135,6 +135,11 @@ sub generate_spdx_report ($self, $id, $options = {}) {
     if $minion->lock("spdx_$id", 172800);
 }
 
+sub has_human_review ($self, $name) {
+  return !!$self->pg->db->query('SELECT COUNT(*) FROM bot_packages WHERE name = ? AND reviewing_user IS NOT NULL',
+    $name)->array->[0];
+}
+
 sub has_spdx_report ($self, $id) {
   return -f $self->spdx_report_path($id);
 }

lib/Cavil/Task/Analyze.pm

@@ -99,7 +99,6 @@ sub _analyzed ($job, $id) {
   my $reports = $app->reports;
   my $pkgs    = $app->packages;
 
-
   # Protect from race conditions
   return $job->finish("Package $id is not indexed yet") unless $pkgs->is_indexed($id);
   return $job->finish("Package $id is already being processed")
@@ -111,6 +110,12 @@ sub _analyzed ($job, $id) {
   return unless $pkg->{indexed};
   return if $pkg->{state} ne 'new' && $pkg->{state} ne 'acceptable';
 
+  # Every package needs a human review before future versions can be auto-accepted (still gets a diff)
+  unless ($pkgs->has_human_review($pkg->{name})) {
+    _look_for_smallest_delta($app, $pkg, 0) if $pkg->{state} eq 'new';
+    return;
+  }
+
   # Fast-track packages that are configured to always be acceptable
   my $name                = $pkg->{name};
   my $acceptable_packages = $config->{acceptable_packages} || [];
@@ -168,10 +173,10 @@ sub _analyzed ($job, $id) {
     $pkgs->update($pkg);
   }
 
-  _look_for_smallest_delta($app, $pkg) if $pkg->{state} eq 'new';
+  _look_for_smallest_delta($app, $pkg, 1) if $pkg->{state} eq 'new';
 }
 
-sub _look_for_smallest_delta ($app, $pkg) {
+sub _look_for_smallest_delta ($app, $pkg, $allow_accept) {
   my $reports = $app->reports;
   my $pkgs    = $app->packages;
 
@@ -185,15 +190,18 @@ sub _look_for_smallest_delta ($app, $pkg) {
     next if $checked{$old->{checksum}};
     my $old_summary = $reports->summary($old->{id});
     my $score       = summary_delta_score($old_summary, $new_summary);
-    if (!$score) {
 
-      # don't look further
-      $pkg->{state}            = 'acceptable';
-      $pkg->{review_timestamp} = 1;
-      $pkg->{result}           = "Not found any signficant difference against $old->{id}";
+    # don't look further
+    if (!$score) {
+      if ($allow_accept) {
+        $pkg->{state}            = 'acceptable';
+        $pkg->{review_timestamp} = 1;
+      }
+      $pkg->{result} = "Not found any signficant difference against $old->{id}";
       $pkgs->update($pkg);
       return;
     }
+
     $checked{$old->{checksum}} = 1;
     if (!$best || $score < $best_score) {
       $best       = $old_summary;

t/cleanup.t

@@ -91,11 +91,16 @@ $t->app->patterns->create(pattern => 'The Artistic License 2.0', license => "Art
 $t->app->minion->enqueue(unpack => [$_]) for ($one_id, $two_id, $three_id);
 $t->app->minion->perform_jobs;
 
+is $t->app->packages->find($one_id)->{state}, 'new', 'not previously reviewed by a human';
+$t->app->packages->update({id => $one_id, state => 'acceptable', reviewing_user => 1, result => 'Human review ok'});
+$t->app->minion->enqueue('reindex_all');
+$t->app->minion->perform_jobs;
+
 # First package
 # fake import date
 $t->app->pg->db->query('update bot_packages set imported = ? where id=?', '2017-12-24', $one_id);
-is $t->app->packages->find($one_id)->{state},  'acceptable',                       'right state';
-is $t->app->packages->find($one_id)->{result}, 'Accepted because of low risk (2)', 'right result';
+is $t->app->packages->find($one_id)->{state},  'acceptable',      'right state';
+is $t->app->packages->find($one_id)->{result}, 'Human review ok', 'right result';
 ok !$t->app->packages->find($one_id)->{obsolete},                                               'not obsolete';
 ok -e $dir->child(@one),                                                                        'checkout exists';
 ok $t->app->pg->db->select('bot_reports', [\'count(*)'], {package => $one_id})->array->[0],     'has reports';
@@ -129,7 +134,7 @@ ok $t->app->pg->db->select('emails', [\'count(*)'], {package => $three_id})->arr
 ok $t->app->pg->db->select('urls', [\'count(*)'], {package => $three_id})->array->[0],   'has URLs';
 
 # Upgrade from acceptable to correct by reindexing
-$t->app->packages->update({id => $two_id, state => 'correct'});
+$t->app->packages->update({id => $two_id, state => 'correct', reviewing_user => 1});
 $t->app->minion->enqueue(analyzed => [$one_id]);
 $t->app->minion->perform_jobs;
 is $t->app->packages->find($one_id)->{state},  'correct',                                             'right state';

t/index.t

@@ -205,7 +205,7 @@ $t->app->pg->db->query("update license_patterns set pattern = 'powerful' where i
 $t->app->patterns->expire_cache;
 $list = $t->app->minion->backend->list_jobs(0, 10, {tasks => ['index_later']});
 is $list->{total}, 1, 'one index_later jobs';
-$reindex_id = $t->app->minion->enqueue('reindex_all');
+$t->app->minion->enqueue('reindex_all');
 $t->app->minion->perform_jobs;
 $list = $t->app->minion->backend->list_jobs(0, 10, {tasks => ['index_later']});
 is $list->{total},          3,          'three index_later jobs';
@@ -232,13 +232,35 @@ is $pkg->{state}, 'new', 'still snippets left';
 
 # now 'classify'
 $db->update('snippets', {classified => 1, license => 0});
-$reindex_id = $t->app->minion->enqueue('reindex_all');
-$t->app->minion->perform_jobs;
 
-# Accepted because of low risk
-$pkg = $t->app->packages->find(1);
-is $pkg->{state},  'acceptable',                       'automatically accepted';
-is $pkg->{result}, 'Accepted because of low risk (5)', 'because of low risk';
+subtest 'Accepted because of low risk (with human review)' => sub {
+  $t->app->minion->enqueue('reindex_all');
+  $t->app->minion->perform_jobs;
+
+  my $pkg = $t->app->packages->find(1);
+  is $pkg->{state}, 'new', 'not previously reviewed by a human';
+
+  my $acceptable_id = $t->app->packages->add(
+    name            => 'perl-Mojolicious',
+    checkout_dir    => 'c7cfdab0e71b0bebfdf8b2dc3badfecd',
+    api_url         => 'https://api.opensuse.org',
+    requesting_user => 1,
+    project         => 'devel:languages:perl',
+    package         => 'perl-Mojolicious',
+    srcmd5          => 'bd91c36647a5d3dd883d490da2140401',
+    priority        => 5
+  );
+  $t->app->packages->imported($acceptable_id);
+  $t->app->packages->unpacked($acceptable_id);
+  $t->app->packages->indexed($acceptable_id);
+  $t->app->packages->update({id => $acceptable_id, state => 'acceptable', reviewing_user => 2, obsolete => 1});
+  $t->app->minion->enqueue('reindex_all');
+  $t->app->minion->perform_jobs;
+
+  $pkg = $t->app->packages->find(1);
+  is $pkg->{state},  'acceptable',                       'automatically accepted';
+  is $pkg->{result}, 'Accepted because of low risk (5)', 'because of low risk';
+};
 
 subtest 'Accept package because of its name' => sub {
   $db->update('bot_packages', {state => 'new'}, {id => 1});