diff --git a/lib/Cavil/Model/Packages.pm b/lib/Cavil/Model/Packages.pm index 2f13666d6..1d6be65cf 100644 --- a/lib/Cavil/Model/Packages.pm +++ b/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); } diff --git a/lib/Cavil/Task/Analyze.pm b/lib/Cavil/Task/Analyze.pm index 1e49b076d..fb57cd0a4 100644 --- a/lib/Cavil/Task/Analyze.pm +++ b/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; diff --git a/t/cleanup.t b/t/cleanup.t index c076767bd..582543236 100644 --- a/t/cleanup.t +++ b/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'; diff --git a/t/index.t b/t/index.t index 71a4119e3..d2449f180 100644 --- a/t/index.t +++ b/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});