From 1e0351c37c100f191526ba31730bbfd28db233b8 Mon Sep 17 00:00:00 2001 From: Christopher Grebs Date: Tue, 30 Jul 2019 20:05:53 +0200 Subject: [PATCH] Recalculate risk score for already confirmed add-ons regularly. (#11950) * Recalculate risk score for already confirmed add-ons regularly. Fixes #11790 * Update task to be run on Sunday, 3pm * Cleanup, don't take deleted reports into account --- scripts/crontab/crontab.tpl | 2 + .../management/commands/process_addons.py | 33 ++++- src/olympia/addons/tests/test_commands.py | 134 +++++++++++++++++- 3 files changed, 167 insertions(+), 2 deletions(-) diff --git a/scripts/crontab/crontab.tpl b/scripts/crontab/crontab.tpl index 75b7c80c3d..4ef3879cab 100644 --- a/scripts/crontab/crontab.tpl +++ b/scripts/crontab/crontab.tpl @@ -40,6 +40,8 @@ HOME=/tmp # Once per week 1 9 * * 1 %(django)s review_reports +0 15 * * 6 %(django)s process_addons --task=constantly_recalculate_post_review_weight + # Do not put crons below this line diff --git a/src/olympia/addons/management/commands/process_addons.py b/src/olympia/addons/management/commands/process_addons.py index e2c210132a..dc350ae2ee 100644 --- a/src/olympia/addons/management/commands/process_addons.py +++ b/src/olympia/addons/management/commands/process_addons.py @@ -1,7 +1,7 @@ from datetime import datetime from django.core.management.base import BaseCommand, CommandError -from django.db.models import Q +from django.db.models import Q, F from celery import chord, group @@ -18,6 +18,7 @@ from olympia.addons.tasks import ( recreate_theme_previews, repack_themes_for_69, ) +from olympia.abuse.models import AbuseReport from olympia.amo.utils import chunked from olympia.devhub.tasks import get_preview_sizes, recreate_previews from olympia.lib.crypto.tasks import sign_addons @@ -32,6 +33,30 @@ firefox_56_star = version_int('56.*') current_autoapprovalsummary = '_current_version__autoapprovalsummary__' +def get_recalc_needed_filters(): + summary_modified = F('_current_version__autoapprovalsummary__modified') + # We don't take deleted reports into account + valid_abuse_report_states = ( + AbuseReport.STATES.UNTRIAGED, AbuseReport.STATES.VALID, + AbuseReport.STATES.SUSPICIOUS) + return [ + # Only recalculate add-ons that received recent abuse reports + # possibly through their authors. + Q( + abuse_reports__state__in=valid_abuse_report_states, + abuse_reports__created__gte=summary_modified + ) | + Q( + authors__abuse_reports__state__in=valid_abuse_report_states, + authors__abuse_reports__created__gte=summary_modified + ) | + + # And check ratings that have a rating of 3 or less + Q(_current_version__ratings__created__gte=summary_modified, + _current_version__ratings__rating__lte=3) + ] + + tasks = { 'find_inconsistencies_between_es_and_db': { 'method': find_inconsistencies_between_es_and_db, 'qs': []}, @@ -42,6 +67,12 @@ tasks = { Q(**{current_autoapprovalsummary + 'verdict': amo.AUTO_APPROVED}) & ~Q(**{current_autoapprovalsummary + 'confirmed': True}) ]}, + 'constantly_recalculate_post_review_weight': { + # This re-calculates the whole post-review weight which can be costly + # so take that into account. We may want to optimize that later + # in case we notice things are slower than needed - cgrebs 20190730 + 'method': recalculate_post_review_weight, + 'qs': get_recalc_needed_filters()}, 'resign_addons_for_cose': { 'method': sign_addons, 'qs': [ diff --git a/src/olympia/addons/tests/test_commands.py b/src/olympia/addons/tests/test_commands.py index 7ad25a9f7e..29b0dbf274 100644 --- a/src/olympia/addons/tests/test_commands.py +++ b/src/olympia/addons/tests/test_commands.py @@ -1,5 +1,5 @@ from contextlib import contextmanager -from datetime import datetime +from datetime import datetime, timedelta from django.core import mail from django.core.management import call_command @@ -15,6 +15,7 @@ from olympia.activity.models import ActivityLog from olympia.addons.management.commands import process_addons from olympia.addons.models import ( Addon, AddonApprovalsCounter, MigratedLWT, ReusedGUID, GUID_REUSE_FORMAT) +from olympia.abuse.models import AbuseReport from olympia.amo.tests import ( TestCase, addon_factory, user_factory, version_factory) from olympia.files.models import FileValidation, WebextPermission @@ -227,6 +228,137 @@ class RecalculateWeightTestCase(TestCase): assert summary.weight == 10 +class ConstantlyRecalculateWeightTestCase(TestCase): + def test_affects_correct_addons(self): + # *not considered* - Non auto-approved add-on + addon_factory() + + # *not considered* - Non auto-approved add-on that has an + # AutoApprovalSummary entry + AutoApprovalSummary.objects.create( + version=addon_factory().current_version, + verdict=amo.NOT_AUTO_APPROVED) + + # *not considered* -Add-on with the current version not auto-approved + extra_addon = addon_factory() + AutoApprovalSummary.objects.create( + version=extra_addon.current_version, verdict=amo.AUTO_APPROVED) + extra_addon.current_version.update(created=self.days_ago(1)) + version_factory(addon=extra_addon) + + # *not considered* - current version is auto-approved but doesn't + # have recent abuse reports or low ratings + auto_approved_addon = addon_factory() + AutoApprovalSummary.objects.create( + version=auto_approved_addon.current_version, + verdict=amo.AUTO_APPROVED) + + # *considered* - current version is auto-approved and + # has a recent rating with rating <= 3 + auto_approved_addon1 = addon_factory() + summary = AutoApprovalSummary.objects.create( + version=auto_approved_addon1.current_version, + verdict=amo.AUTO_APPROVED) + Rating.objects.create( + created=summary.modified + timedelta(days=3), + addon=auto_approved_addon1, + version=auto_approved_addon1.current_version, + rating=2, body='Apocalypse', user=user_factory()), + + # *not considered* - current version is auto-approved but + # has a recent rating with rating > 3 + auto_approved_addon2 = addon_factory() + summary = AutoApprovalSummary.objects.create( + version=auto_approved_addon2.current_version, + verdict=amo.AUTO_APPROVED) + Rating.objects.create( + created=summary.modified + timedelta(days=3), + addon=auto_approved_addon2, + version=auto_approved_addon2.current_version, + rating=4, body='Apocalypse', user=user_factory()), + + # *not considered* - current version is auto-approved but + # has a recent rating with rating > 3 + auto_approved_addon3 = addon_factory() + summary = AutoApprovalSummary.objects.create( + version=auto_approved_addon3.current_version, + verdict=amo.AUTO_APPROVED) + Rating.objects.create( + created=summary.modified + timedelta(days=3), + addon=auto_approved_addon3, + version=auto_approved_addon3.current_version, + rating=4, body='Apocalypse', user=user_factory()), + + # *not considered* - current version is auto-approved but + # has a low rating that isn't recent enough + auto_approved_addon4 = addon_factory() + summary = AutoApprovalSummary.objects.create( + version=auto_approved_addon4.current_version, + verdict=amo.AUTO_APPROVED) + Rating.objects.create( + created=summary.modified - timedelta(days=3), + addon=auto_approved_addon4, + version=auto_approved_addon4.current_version, + rating=1, body='Apocalypse', user=user_factory()), + + # *considered* - current version is auto-approved and + # has a recent abuse report + auto_approved_addon5 = addon_factory() + summary = AutoApprovalSummary.objects.create( + version=auto_approved_addon5.current_version, + verdict=amo.AUTO_APPROVED) + AbuseReport.objects.create( + addon=auto_approved_addon5, + created=summary.modified + timedelta(days=3)) + + # *not considered* - current version is auto-approved but + # has an abuse report that isn't recent enough + auto_approved_addon6 = addon_factory() + summary = AutoApprovalSummary.objects.create( + version=auto_approved_addon6.current_version, + verdict=amo.AUTO_APPROVED) + AbuseReport.objects.create( + addon=auto_approved_addon6, + created=summary.modified - timedelta(days=3)) + + # *considered* - current version is auto-approved and + # has an abuse report through it's author that is recent enough + author = user_factory() + auto_approved_addon7 = addon_factory(users=[author]) + summary = AutoApprovalSummary.objects.create( + version=auto_approved_addon7.current_version, + verdict=amo.AUTO_APPROVED) + AbuseReport.objects.create( + user=author, + created=summary.modified + timedelta(days=3)) + + # *not considered* - current version is auto-approved and + # has an abuse report through it's author that is recent enough + # BUT the abuse report is deleted. + author = user_factory() + auto_approved_addon8 = addon_factory(users=[author]) + summary = AutoApprovalSummary.objects.create( + version=auto_approved_addon8.current_version, + verdict=amo.AUTO_APPROVED) + AbuseReport.objects.create( + user=author, + state=AbuseReport.STATES.DELETED, + created=summary.modified + timedelta(days=3)) + + with count_subtask_calls( + process_addons.recalculate_post_review_weight) as calls: + call_command( + 'process_addons', + task='constantly_recalculate_post_review_weight') + + assert len(calls) == 1 + assert calls[0]['kwargs']['args'] == [[ + auto_approved_addon1.pk, + auto_approved_addon5.pk, + auto_approved_addon7.pk, + ]] + + class TestExtractWebextensionsToGitStorage(TestCase): @mock.patch('olympia.addons.tasks.index_addons.delay', autospec=True) @mock.patch(