mozilla
/
addons-server
зеркало из https://github.com/mozilla/addons-server.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Queue Cinder jobs on a queue per entity type (#21764) 

* Queue Cinder jobs on a queue per entity type

This changes the filtering the webhook did on the queue, relying on the
job_id to filter jobs we shouldn't be processing, except for jobs in the
special queue for CinderAddonHandledByReviewers.

This allows us to execute decisions from any queue the job might be in,
as long as we know the job_id and that the entity is still an AMO one,
which is useful as jobs might be bounced to a global queue not specific
to AMO.
This commit is contained in:
Mathieu Pillard 2024-01-31 11:17:28 +01:00 коммит произвёл GitHub
Родитель 5d35e7d543
Коммит a21ff5dfb4
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
6 изменённых файлов: 90 добавлений и 46 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

1
settings_test.py
Просмотреть файл

@ -101,6 +101,7 @@ ADDONS_FRONTEND_PROXY_PORT = None
VERIFY_FXA_ACCESS_TOKEN = False
CINDER_API_TOKEN = 'fake-test-token'
CINDER_QUEUE_PREFIX = 'amo-env-'
SOCKET_LABS_TOKEN = 'fake-test-token'
SOCKET_LABS_SERVER_ID = '12345'

22
src/olympia/abuse/cinder.py
Просмотреть файл

@ -16,15 +16,14 @@ from olympia.amo.utils import (
class CinderEntity:
# This queue is for reports that T&S / TaskUs look at
_queue = 'content-infringement'
queue_suffix = None # Needs to be defined by subclasses
type = None # Needs to be defined by subclasses
# Number of relationships to send by default in each Cinder request.
RELATIONSHIPS_BATCH_SIZE = 25
@classproperty
def queue(cls):
return f'{settings.CINDER_QUEUE_PREFIX}{cls._queue}'
@property
def queue(self):
return f'{settings.CINDER_QUEUE_PREFIX}{self.queue_suffix}'
@property
def id(self):
@ -173,6 +172,7 @@ class CinderEntity:
class CinderUser(CinderEntity):
type = 'amo_user'
queue_suffix = 'users'
def __init__(self, user):
self.user = user
@ -266,6 +266,10 @@ class CinderAddon(CinderEntity):
def id(self):
return self.get_str(self.addon.id)
@property
def queue_suffix(self):
return 'themes' if self.addon.type == amo.ADDON_STATICTHEME else 'listings'
def get_attributes(self):
# We look at the promoted group to tell whether or not the add-on has
# a badge, but we don't care about the promotion being approved for the
@ -354,6 +358,7 @@ class CinderAddon(CinderEntity):
class CinderRating(CinderEntity):
type = 'amo_rating'
queue_suffix = 'ratings'
def __init__(self, rating):
self.rating = rating
@ -390,6 +395,7 @@ class CinderRating(CinderEntity):
class CinderCollection(CinderEntity):
type = 'amo_collection'
queue_suffix = 'collections'
def __init__(self, collection):
self.collection = collection
@ -420,7 +426,11 @@ class CinderCollection(CinderEntity):
class CinderAddonHandledByReviewers(CinderAddon):
# This queue is not monitored on cinder - reports are resolved via AMO instead
_queue = 'addon-infringement'
queue_suffix = 'addon-infringement'
@classproperty # Special case, needs to be static for the webhook check.
def queue(cls):
return f'{settings.CINDER_QUEUE_PREFIX}{cls.queue_suffix}'
def flag_for_human_review(self, appeal=False):
from olympia.reviewers.models import NeedsHumanReview

61
src/olympia/abuse/tests/test_cinder.py
Просмотреть файл

@ -4,7 +4,6 @@ import random
from unittest import mock
from django.conf import settings
from django.test.utils import override_settings
import responses
@ -38,11 +37,17 @@ from ..cinder import (
class BaseTestCinderCase:
cinder_class = None # Override in child classes
expected_queue_suffix = None # Override in child classes
expected_queries_for_report = -1 # Override in child classes
@override_settings(CINDER_QUEUE_PREFIX='amo-env-')
def test_queue(self):
assert self.cinder_class.queue == 'amo-env-content-infringement'
target = self._create_dummy_target()
cinder_entity = self.cinder_class(target)
assert cinder_entity.queue_suffix == self.expected_queue_suffix
assert (
cinder_entity.queue
== f'{settings.CINDER_QUEUE_PREFIX}{cinder_entity.queue_suffix}'
)
def _create_dummy_target(self, **kwargs):
raise NotImplementedError
@ -158,10 +163,21 @@ class TestCinderAddon(BaseTestCinderCase, TestCase):
# and we have custom limits for batch-sending relationships)
# - Promoted add-on
expected_queries_for_report = 2
expected_queue_suffix = 'listings'
def _create_dummy_target(self, **kwargs):
return addon_factory(**kwargs)
def test_queue_theme(self):
target = self._create_dummy_target(type=amo.ADDON_STATICTHEME)
cinder_entity = self.cinder_class(target)
expected_queue_suffix = 'themes'
assert cinder_entity.queue_suffix == expected_queue_suffix
assert (
cinder_entity.queue
== f'{settings.CINDER_QUEUE_PREFIX}{cinder_entity.queue_suffix}'
)
def test_build_report_payload(self):
addon = self._create_dummy_target(
homepage='https://home.example.com',
@ -178,7 +194,7 @@ class TestCinderAddon(BaseTestCinderCase, TestCase):
report=CinderReport(abuse_report), reporter=None
)
assert data == {
'queue_slug': self.cinder_class.queue,
'queue_slug': cinder_addon.queue,
'entity_type': 'amo_addon',
'entity': {
'id': str(addon.pk),
@ -516,7 +532,7 @@ class TestCinderAddon(BaseTestCinderCase, TestCase):
reporter=None,
)
assert data == {
'queue_slug': self.cinder_class.queue,
'queue_slug': cinder_addon.queue,
'entity_type': 'amo_addon',
'entity': {
'id': str(addon.pk),
@ -612,7 +628,7 @@ class TestCinderAddon(BaseTestCinderCase, TestCase):
reporter=None,
)
assert data == {
'queue_slug': self.cinder_class.queue,
'queue_slug': cinder_addon.queue,
'entity_type': 'amo_addon',
'entity': {
'id': str(addon.pk),
@ -877,11 +893,25 @@ class TestCinderAddonHandledByReviewers(TestCinderAddon):
# - 13 Fetch Addon authors
# - 14 Fetch Promoted Addon
expected_queries_for_report = 14
expected_queue_suffix = 'addon-infringement'
@override_settings(CINDER_QUEUE_PREFIX='amo-env-')
def test_queue(self):
super().test_queue()
# For this class the property should be guaranteed to be static.
assert self.cinder_class.queue == 'amo-env-addon-infringement'
def test_queue_theme(self):
# Contrary to reports handled by Cinder moderators, for reports handled
# by AMO reviewers the queue should remain the same regardless of the
# addon-type.
target = self._create_dummy_target(type=amo.ADDON_STATICTHEME)
cinder_entity = self.cinder_class(target)
assert cinder_entity.queue_suffix == self.expected_queue_suffix
assert (
cinder_entity.queue
== f'{settings.CINDER_QUEUE_PREFIX}{cinder_entity.queue_suffix}'
)
def setUp(self):
user_factory(id=settings.TASK_USER_ID)
@ -1001,6 +1031,7 @@ class TestCinderUser(BaseTestCinderCase, TestCase):
# - Related add-ons
# - Number of listed add-ons
expected_queries_for_report = 2
expected_queue_suffix = 'users'
def _create_dummy_target(self, **kwargs):
return user_factory(**kwargs)
@ -1020,7 +1051,7 @@ class TestCinderUser(BaseTestCinderCase, TestCase):
report=CinderReport(abuse_report), reporter=None
)
assert data == {
'queue_slug': self.cinder_class.queue,
'queue_slug': 'amo-env-users',
'entity_type': 'amo_user',
'entity': {
'id': str(user.pk),
@ -1363,7 +1394,7 @@ class TestCinderUser(BaseTestCinderCase, TestCase):
report=CinderReport(abuse_report), reporter=None
)
assert data == {
'queue_slug': self.cinder_class.queue,
'queue_slug': 'amo-env-users',
'entity_type': 'amo_user',
'entity': {
'id': str(user.pk),
@ -1632,6 +1663,7 @@ class TestCinderUser(BaseTestCinderCase, TestCase):
class TestCinderRating(BaseTestCinderCase, TestCase):
cinder_class = CinderRating
expected_queries_for_report = 1 # For the author
expected_queue_suffix = 'ratings'
def setUp(self):
self.user = user_factory()
@ -1652,7 +1684,7 @@ class TestCinderRating(BaseTestCinderCase, TestCase):
report=CinderReport(abuse_report), reporter=None
)
assert data == {
'queue_slug': self.cinder_class.queue,
'queue_slug': 'amo-env-ratings',
'entity_type': 'amo_rating',
'entity': {
'id': str(rating.pk),
@ -1712,7 +1744,7 @@ class TestCinderRating(BaseTestCinderCase, TestCase):
report=CinderReport(abuse_report), reporter=CinderUser(user)
)
assert data == {
'queue_slug': self.cinder_class.queue,
'queue_slug': 'amo-env-ratings',
'entity_type': 'amo_rating',
'entity': {
'id': str(rating.pk),
@ -1785,7 +1817,7 @@ class TestCinderRating(BaseTestCinderCase, TestCase):
report=CinderReport(abuse_report), reporter=None
)
assert data == {
'queue_slug': self.cinder_class.queue,
'queue_slug': 'amo-env-ratings',
'entity_type': 'amo_rating',
'entity': {
'id': str(rating.pk),
@ -1853,6 +1885,7 @@ class TestCinderRating(BaseTestCinderCase, TestCase):
class TestCinderCollection(BaseTestCinderCase, TestCase):
cinder_class = CinderCollection
expected_queries_for_report = 1 # For the author
expected_queue_suffix = 'collections'
def setUp(self):
self.user = user_factory()
@ -1932,7 +1965,7 @@ class TestCinderCollection(BaseTestCinderCase, TestCase):
'slug': collection.slug,
},
'entity_type': 'amo_collection',
'queue_slug': self.cinder_class.queue,
'queue_slug': 'amo-env-collections',
'reasoning': message,
}
@ -2004,7 +2037,7 @@ class TestCinderCollection(BaseTestCinderCase, TestCase):
'slug': collection.slug,
},
'entity_type': 'amo_collection',
'queue_slug': self.cinder_class.queue,
'queue_slug': 'amo-env-collections',
'reasoning': message,
}

10
src/olympia/abuse/tests/test_tasks.py
Просмотреть файл

@ -249,7 +249,7 @@ def test_addon_report_to_cinder():
'version': str(addon.current_version.version),
},
'entity_type': 'amo_addon',
'queue_slug': 'amo-dev-content-infringement',
'queue_slug': 'amo-env-listings',
'reasoning': 'This is bad',
}
@ -327,7 +327,7 @@ def test_addon_report_to_cinder_different_locale():
'version': str(addon.current_version.version),
},
'entity_type': 'amo_addon',
'queue_slug': 'amo-dev-content-infringement',
'queue_slug': 'amo-env-listings',
'reasoning': 'This is bad',
}
@ -377,7 +377,7 @@ def test_addon_appeal_to_cinder_reporter():
},
'appealer_entity_type': 'amo_unauthenticated_reporter',
'decision_to_appeal_id': '4815162342-abc',
'queue_slug': 'amo-dev-content-infringement',
'queue_slug': 'amo-env-listings',
'reasoning': 'I appeal',
}
@ -432,7 +432,7 @@ def test_addon_appeal_to_cinder_authenticated_reporter():
},
'appealer_entity_type': 'amo_user',
'decision_to_appeal_id': '4815162342-abc',
'queue_slug': 'amo-dev-content-infringement',
'queue_slug': 'amo-env-listings',
'reasoning': 'I appeal',
}
@ -486,7 +486,7 @@ def test_addon_appeal_to_cinder_authenticated_author():
},
'appealer_entity_type': 'amo_user',
'decision_to_appeal_id': '4815162342-abc',
'queue_slug': 'amo-dev-content-infringement',
'queue_slug': 'amo-env-listings',
'reasoning': 'I appeal',
}

32
src/olympia/abuse/tests/test_views.py
Просмотреть файл

@ -916,7 +916,7 @@ class TestCinderWebhook(TestCase):
CinderJob.DECISION_ACTIONS.AMO_APPROVE,
]
def _test_process_decision_called(self, data, override):
def _test_process_decision_called(self, data, *, override):
abuse_report = self._setup_reports()
addon_factory(guid=abuse_report.guid)
req = self.get_request(data=data)
@ -933,14 +933,14 @@ class TestCinderWebhook(TestCase):
assert response.status_code == 201
assert response.data == {'amo': {'received': True, 'handled': True}}
def test_process_decision_called_not_overide(self):
def test_process_decision_called_not_override(self):
data = self.get_data()
return self._test_process_decision_called(data, False)
return self._test_process_decision_called(data, override=False)
def test_process_decision_called_for_override(self):
data = self.get_data()
data['payload']['source']['decision']['type'] = 'override'
return self._test_process_decision_called(data, True)
return self._test_process_decision_called(data, override=True)
def test_process_decision_called_for_appeal_confirm_approve(self):
data = self.get_data(filename='cinder_webhook_appeal_confirm_approve.json')
@ -996,8 +996,16 @@ class TestCinderWebhook(TestCase):
assert response.status_code == 201
assert response.data == {'amo': {'received': True, 'handled': True}}
def _test_wrong_queue(self, data):
self._setup_reports()
def test_queue_does_not_matter_non_reviewer_case(self):
data = self.get_data()
data['payload']['source']['job']['queue']['slug'] = 'amo-another-queue'
return self._test_process_decision_called(data, override=False)
def test_queue_handled_reviewer_queue_ignored(self):
data = self.get_data()
data['payload']['source']['job']['queue']['slug'] = 'amo-addon-infringement'
abuse_report = self._setup_reports()
addon_factory(guid=abuse_report.guid)
req = self.get_request(data=data)
with mock.patch.object(CinderJob, 'process_decision') as process_mock:
response = cinder_webhook(req)
@ -1007,20 +1015,10 @@ class TestCinderWebhook(TestCase):
'amo': {
'received': True,
'handled': False,
'not_handled_reason': 'Not from a queue we process',
'not_handled_reason': 'Queue handled by AMO reviewers',
}
}
def test_wrong_queue_slug(self):
data = self.get_data()
data['payload']['source']['job']['queue']['slug'] = 'amo-another-queue'
self._test_wrong_queue(data)
@override_settings(CINDER_QUEUE_PREFIX='amo-stage')
def test_wrong_queue_prefix(self):
data = self.get_data()
self._test_wrong_queue(data)
def test_not_decision_event(self):
self._setup_reports()
data = self.get_data()

10
src/olympia/abuse/views.py
Просмотреть файл

@ -21,6 +21,7 @@ from rest_framework.response import Response
from rest_framework.viewsets import GenericViewSet
import olympia.core.logger
from olympia.abuse.cinder import CinderAddonHandledByReviewers
from olympia.abuse.tasks import appeal_to_cinder
from olympia.accounts.utils import redirect_for_login
from olympia.accounts.views import AccountViewSet
@ -31,7 +32,6 @@ from olympia.core import set_user
from olympia.ratings.views import RatingViewSet
from olympia.users.models import UserProfile
from .cinder import CinderEntity
from .forms import AbuseAppealEmailForm, AbuseAppealForm
from .models import AbuseReport, CinderJob
from .serializers import (
@ -213,9 +213,11 @@ def cinder_webhook(request):
source = payload.get('source', {})
job = source.get('job', {})
if (queue_name := job.get('queue', {}).get('slug')) != CinderEntity.queue:
log.info('Payload from other queue: %s', queue_name)
raise ValidationError('Not from a queue we process')
if (
queue_name := job.get('queue', {}).get('slug')
) == CinderAddonHandledByReviewers.queue:
log.info('Payload from queue handled by reviewers: %s', queue_name)
raise ValidationError('Queue handled by AMO reviewers')
log.info('Valid Payload from AMO queue: %s', payload)
job_id = job.get('id', '')