Remove obsolete validator caching (#22627)

* Remove obsolete validator caching

This was only really relevant when validation could be triggered arbitrarily by
reviewers for an existing File(Upload) that was laying around unvalidated.

That confusing and dangerous functionality no longer exists, FileUpload
validation is always done once at upload and can't be triggered later.

When we still had post-request-task, that caching was broken for years
and nobody noticed because the code path where we pulled the task id
from the cache was never used.

* Remove obsolete tests

src/olympia/devhub/tasks.py

@@ -9,7 +9,6 @@ from functools import wraps
 from zipfile import BadZipFile
 
 from django.conf import settings
-from django.core.cache import cache
 from django.core.files.storage import default_storage as storage
 from django.core.validators import ValidationError
 from django.db import transaction
@@ -19,7 +18,6 @@ from django.utils.encoding import force_str
 from django.utils.translation import gettext
 
 import waffle
-from celery.result import AsyncResult
 from django_statsd.clients import statsd
 
 import olympia.core.logger
@@ -67,16 +65,8 @@ def validate(file_, *, final_task=None, theme_specific=False):
     from .utils import Validator
 
     validator = Validator(file_, theme_specific=theme_specific, final_task=final_task)
-
-    task_id = cache.get(validator.cache_key)
-
-    if task_id:
-        return AsyncResult(task_id)
-    else:
-        task = validator.get_task()
-        task_id = task.freeze().id
-        cache.set(validator.cache_key, task_id, 5 * 60)
-        return task.delay()
+    task = validator.get_task()
+    return task.delay()
 
 
 def validate_and_submit(*, addon, upload, client_info, theme_specific=False):

src/olympia/devhub/tests/test_utils.py

@@ -8,7 +8,6 @@ from django.test.utils import override_settings
 
 import pytest
 from celery import chord
-from celery.result import AsyncResult
 
 from olympia import amo
 from olympia.addons.models import Addon
@@ -100,54 +99,6 @@ class TestAddonsLinterListed(UploadMixin, TestCase):
         assert not repack_fileupload.called
         assert not validate_upload.called
 
-    @mock.patch.object(utils.Validator, 'get_task')
-    def test_run_once_per_file(self, get_task_mock):
-        """Tests that only a single validation task is run for a given file."""
-        get_task_mock.return_value.freeze.return_value = mock.Mock(id='42')
-
-        assert isinstance(tasks.validate(self.file), mock.Mock)
-        assert get_task_mock.return_value.delay.call_count == 1
-
-        assert isinstance(tasks.validate(self.file), AsyncResult)
-        assert get_task_mock.return_value.delay.call_count == 1
-
-        new_version = version_factory(addon=self.addon, version='0.0.2')
-        assert isinstance(tasks.validate(new_version.file), mock.Mock)
-        assert get_task_mock.return_value.delay.call_count == 2
-
-    @mock.patch.object(utils.Validator, 'get_task')
-    def test_run_once_file_upload(self, get_task_mock):
-        """Tests that only a single validation task is run for a given file
-        upload."""
-        get_task_mock.return_value.freeze.return_value = mock.Mock(id='42')
-
-        assert isinstance(tasks.validate(self.file_upload), mock.Mock)
-        assert get_task_mock.return_value.delay.call_count == 1
-
-        assert isinstance(tasks.validate(self.file_upload), AsyncResult)
-        assert get_task_mock.return_value.delay.call_count == 1
-
-    def test_cache_key(self):
-        """Tests that the correct cache key is generated for a given object."""
-
-        assert (
-            utils.Validator(self.file).cache_key
-            == f'validation-task:files.File:{self.file.pk}:2'
-        )
-
-        assert utils.Validator(
-            self.file_upload
-        ).cache_key == 'validation-task:files.FileUpload:{}:2'.format(
-            self.file_upload.pk
-        )
-
-        self.file_upload.update(channel=amo.CHANNEL_UNLISTED)
-        assert utils.Validator(
-            self.file_upload
-        ).cache_key == 'validation-task:files.FileUpload:{}:1'.format(
-            self.file_upload.pk
-        )
-
 
 class TestLimitAddonsLinterResults(TestCase):
     """Test that higher priority messages are truncated last."""

src/olympia/devhub/utils.py

@@ -255,13 +255,6 @@ class Validator:
 
         self.task = chain(*validation_tasks)
 
-        # Create a cache key for the task, so multiple requests to validate the
-        # same object do not result in duplicate tasks.
-        opts = file_._meta
-        self.cache_key = 'validation-task:{}.{}:{}:{}'.format(
-            opts.app_label, opts.object_name, file_.pk, channel
-        )
-
     def get_task(self):
         """Return task chain to execute to trigger validation."""
         return self.task