Add support for soft/hard block in upload_to_mllf_to_remote_settings cron:

- adds support to process fort blocks in the cron
- does not actually support soft blocks, we skip them always
- adds test proving we always skip them

This commit makes actually adding them more clear and easier to grok

src/olympia/blocklist/cron.py

@@ -1,11 +1,11 @@
 from datetime import datetime
+from typing import List
 
 import waffle
 from django_statsd.clients import statsd
 
 import olympia.core.logger
 from olympia.constants.blocklist import (
-    BASE_REPLACE_THRESHOLD,
     MLBF_BASE_ID_CONFIG_KEY,
     MLBF_TIME_CONFIG_KEY,
 )
@@ -13,7 +13,7 @@ from olympia.zadmin.models import get_config
 
 from .mlbf import MLBF
 from .models import Block, BlocklistSubmission, BlockType
-from .tasks import cleanup_old_files, process_blocklistsubmission, upload_filter
+from .tasks import process_blocklistsubmission, upload_filter
 from .utils import datetime_to_ts
 
 
@@ -28,9 +28,9 @@ def get_last_generation_time():
     return get_config(MLBF_TIME_CONFIG_KEY, None, json_value=True)
 
 
-def get_base_generation_time():
+def get_base_generation_time(block_type: BlockType):
     return get_config(
-        MLBF_BASE_ID_CONFIG_KEY(BlockType.BLOCKED, compat=True), None, json_value=True
+        MLBF_BASE_ID_CONFIG_KEY(block_type, compat=True), None, json_value=True
     )
 
 
@@ -66,48 +66,45 @@ def _upload_mlbf_to_remote_settings(*, force_base=False):
     # An add-on version/file from after this time can't be reliably asserted -
     # there may be false positives or false negatives.
     # https://github.com/mozilla/addons-server/issues/13695
-    generation_time = get_generation_time()
+    mlbf = MLBF.generate_from_db(get_generation_time())
-    # This timestamp represents the last time the MLBF was generated and uploaded.
+    previous_filter = MLBF.load_from_storage(
-    # It could have been a base filter or a stash.
+        # This timestamp represents the last time the MLBF was generated and uploaded.
-    last_generation_time = get_last_generation_time()
+        # It could have been a base filter or a stash.
-    # This timestamp represents the point in time when
+        get_last_generation_time()
-    # the base filter was generated and uploaded.
-    base_generation_time = get_base_generation_time()
-
-    mlbf = MLBF.generate_from_db(generation_time)
-
-    base_filter = (
-        MLBF.load_from_storage(base_generation_time)
-        if base_generation_time is not None
-        else None
     )
 
-    previous_filter = (
+    base_filters_to_update: List[BlockType] = []
-        # Only load previoous filter if there is a timestamp to use
+    create_stash = False
-        # and that timestamp is not the same as the base_filter
-        MLBF.load_from_storage(last_generation_time)
-        if last_generation_time is not None
-        and (base_filter is None or base_filter.created_at != last_generation_time)
-        else base_filter
-    )
 
-    changes_count = mlbf.blocks_changed_since_previous(
+    # Determine which base filters need to be re uploaded
-        BlockType.BLOCKED, previous_filter
+    # and whether a new stash needs to be created
-    )
+    for block_type in BlockType:
-    statsd.incr(
+        # This prevents us from updating a stash or filter based on new soft blocks
-        'blocklist.cron.upload_mlbf_to_remote_settings.blocked_changed', changes_count
+        if block_type == BlockType.SOFT_BLOCKED:
-    )
+            log.info(
-    need_update = (
+                'Skipping soft-blocks because enable-soft-blocking switch is inactive'
-        force_base
+            )
-        or base_filter is None
+            continue
-        or (
+
-            previous_filter is not None
+        base_filter = MLBF.load_from_storage(get_base_generation_time(block_type))
-            and previous_filter.created_at < get_blocklist_last_modified_time()
+
-        )
+        # add this block type to the list of filters to be re-uploaded
-        or changes_count > 0
+        if (
-    )
+            force_base
-    if not need_update:
+            or base_filter is None
+            or mlbf.should_upload_filter(block_type, base_filter)
+        ):
+            base_filters_to_update.append(block_type)
+        # only update the stash if we should AND if
+        # we aren't already reuploading the filter for this block type
+        elif mlbf.should_upload_stash(block_type, previous_filter or base_filter):
+            create_stash = True
+
+    skip_update = len(base_filters_to_update) == 0 and not create_stash
+    if skip_update:
         log.info('No new/modified/deleted Blocks in database; skipping MLBF generation')
+        # Delete the locally generated MLBF directory and files as they are not needed
+        mlbf.delete()
         return
 
     statsd.incr(
@@ -119,27 +116,24 @@ def _upload_mlbf_to_remote_settings(*, force_base=False):
         len(mlbf.data.not_blocked_items),
     )
 
-    make_base_filter = (
+    # Until we are ready to enable soft blocking, it should not be possible
-        force_base
+    # to create a stash and a filter at the same iteration
-        or base_filter is None
+    if create_stash and len(base_filters_to_update) > 0:
-        or previous_filter is None
+        raise Exception(
-        or mlbf.blocks_changed_since_previous(BlockType.BLOCKED, base_filter)
+            'Cannot upload stash and filter without implementing soft blocking'
-        > BASE_REPLACE_THRESHOLD
+        )
-    )
 
-    if make_base_filter:
+    if create_stash:
-        mlbf.generate_and_write_filter()
-    else:
         mlbf.generate_and_write_stash(previous_filter)
 
-    upload_filter.delay(
+    for block_type in base_filters_to_update:
-        generation_time,
+        mlbf.generate_and_write_filter(block_type)
-        filter_list=[BlockType.BLOCKED.name] if make_base_filter else [],
-        create_stash=not make_base_filter,
-    )
 
-    if base_filter:
+    upload_filter.delay(
-        cleanup_old_files.delay(base_filter_id=base_filter.created_at)
+        mlbf.created_at,
+        filter_list=[key.name for key in base_filters_to_update],
+        create_stash=create_stash,
+    )
 
 
 def process_blocklistsubmissions():

src/olympia/blocklist/mlbf.py

@@ -14,6 +14,7 @@ import olympia.core.logger
 from olympia.amo.utils import SafeStorage
 from olympia.blocklist.models import BlockType, BlockVersion
 from olympia.blocklist.utils import datetime_to_ts
+from olympia.constants.blocklist import BASE_REPLACE_THRESHOLD
 from olympia.versions.models import Version
 
 
@@ -79,7 +80,8 @@ class BaseMLBFLoader:
     def __init__(self, storage: SafeStorage):
         self.storage = storage
 
-    def data_type_key(self, key: MLBFDataType) -> str:
+    @classmethod
+    def data_type_key(cls, key: MLBFDataType) -> str:
         return key.name.lower()
 
     @cached_property
@@ -207,13 +209,21 @@ class MLBF:
             for (guid, version) in input_list
         ]
 
-    def filter_path(self, _block_type: BlockType = BlockType.BLOCKED):
+    def filter_path(self, block_type: BlockType):
-        return self.storage.path('filter')
+        # TODO: explain / test
+        if block_type == BlockType.BLOCKED:
+            return self.storage.path('filter')
+        return self.storage.path(f'filter-{BaseMLBFLoader.data_type_key(block_type)}')
 
     @property
     def stash_path(self):
         return self.storage.path('stash.json')
 
+    def delete(self):
+        if self.storage.exists(self.storage.base_location):
+            self.storage.rm_stored_dir(self.storage.base_location)
+            log.info(f'Deleted {self.storage.base_location}')
+
     def generate_and_write_filter(self, block_type: BlockType = BlockType.BLOCKED):
         stats = {}
 
@@ -297,6 +307,26 @@ class MLBF:
         _, _, changed_count = self.generate_diffs(previous_mlbf)[block_type]
         return changed_count
 
+    def should_upload_filter(
+        self, block_type: BlockType = BlockType.BLOCKED, previous_mlbf: 'MLBF' = None
+    ):
+        return (
+            self.blocks_changed_since_previous(
+                block_type=block_type, previous_mlbf=previous_mlbf
+            )
+            > BASE_REPLACE_THRESHOLD
+        )
+
+    def should_upload_stash(
+        self, block_type: BlockType = BlockType.BLOCKED, previous_mlbf: 'MLBF' = None
+    ):
+        return (
+            self.blocks_changed_since_previous(
+                block_type=block_type, previous_mlbf=previous_mlbf
+            )
+            > 0
+        )
+
     @classmethod
     def load_from_storage(
         cls, created_at: str = datetime_to_ts(), error_on_missing: bool = False

src/olympia/blocklist/tests/test_commands.py

@@ -9,6 +9,7 @@ from olympia.amo.tests import (
     version_factory,
 )
 from olympia.blocklist.mlbf import MLBF
+from olympia.blocklist.models import BlockType
 
 
 class TestExportBlocklist(TestCase):
@@ -38,4 +39,4 @@ class TestExportBlocklist(TestCase):
 
         call_command('export_blocklist', '1')
         mlbf = MLBF.load_from_storage(1)
-        assert mlbf.storage.exists(mlbf.filter_path())
+        assert mlbf.storage.exists(mlbf.filter_path(BlockType.BLOCKED))

src/olympia/blocklist/tests/test_cron.py

@@ -1,6 +1,7 @@
 import json
 import uuid
 from datetime import datetime, timedelta
+from typing import List, Union
 from unittest import mock
 
 from django.conf import settings
@@ -26,6 +27,7 @@ from olympia.blocklist.cron import (
 )
 from olympia.blocklist.mlbf import MLBF
 from olympia.blocklist.models import Block, BlocklistSubmission, BlockType, BlockVersion
+from olympia.blocklist.tasks import upload_filter
 from olympia.blocklist.utils import datetime_to_ts
 from olympia.constants.blocklist import MLBF_BASE_ID_CONFIG_KEY, MLBF_TIME_CONFIG_KEY
 from olympia.zadmin.models import set_config
@@ -45,7 +47,6 @@ class TestUploadToRemoteSettings(TestCase):
         self.mocks: dict[str, mock.Mock] = {}
         for mock_name in (
             'olympia.blocklist.cron.statsd.incr',
-            'olympia.blocklist.cron.cleanup_old_files.delay',
             'olympia.blocklist.cron.upload_filter.delay',
             'olympia.blocklist.cron.get_generation_time',
             'olympia.blocklist.cron.get_last_generation_time',
@@ -58,9 +59,9 @@ class TestUploadToRemoteSettings(TestCase):
         self.base_time = datetime_to_ts(self.block.modified)
         self.last_time = datetime_to_ts(self.block.modified + timedelta(seconds=1))
         self.current_time = datetime_to_ts(self.block.modified + timedelta(seconds=2))
-        self.mocks[
+        self.mocks['olympia.blocklist.cron.get_base_generation_time'].side_effect = (
-            'olympia.blocklist.cron.get_base_generation_time'
+            lambda _block_type: self.base_time
-        ].return_value = self.base_time
+        )
         self.mocks[
             'olympia.blocklist.cron.get_last_generation_time'
         ].return_value = self.last_time
@@ -84,49 +85,104 @@ class TestUploadToRemoteSettings(TestCase):
             block=block, version=version, block_type=block_type
         )
 
-    def test_skip_update_unless_force_base(self):
+    def _test_skip_update_unless_force_base(self, enable_soft_blocking=False):
         """
         skip update unless force_base is true
         """
-        upload_mlbf_to_remote_settings(force_base=False)
-
         # We skip update at this point because there is no reason to update.
+        upload_mlbf_to_remote_settings(force_base=False)
         assert not self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
 
-        # But if we force the base filter, we update.
+        filter_list = [BlockType.BLOCKED.name]
-        upload_mlbf_to_remote_settings(force_base=True)
 
-        assert self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
+        with override_switch('enable-soft-blocking', active=enable_soft_blocking):
+            upload_mlbf_to_remote_settings(force_base=True)
 
-        # Check that a filter was created on the second attempt
+            assert (
-        mlbf = MLBF.load_from_storage(self.current_time)
+                mock.call(
-        assert mlbf.storage.exists(mlbf.filter_path())
+                    self.current_time,
-        assert not mlbf.storage.exists(mlbf.stash_path)
+                    filter_list=filter_list,
+                    create_stash=False,
+                )
+            ) in self.mocks['olympia.blocklist.cron.upload_filter.delay'].call_args_list
 
-    def test_skip_update_unless_no_base_mlbf(self):
+            # Check that both filters were created on the second attempt
+            mlbf = MLBF.load_from_storage(self.current_time)
+            self.assertTrue(
+                mlbf.storage.exists(mlbf.filter_path(BlockType.BLOCKED)),
+            )
+            self.assertEqual(
+                mlbf.storage.exists(mlbf.filter_path(BlockType.SOFT_BLOCKED)),
+                # Until we are ready to enable soft blocking
+                # there should never be a soft block filter.
+                False,
+            )
+            assert not mlbf.storage.exists(mlbf.stash_path)
+
+    def test_skip_update_unless_forced_soft_blocking_disabled(self):
+        self._test_skip_update_unless_force_base(enable_soft_blocking=False)
+
+    def test_skip_update_unless_forced_soft_blocking_enabled(self):
+        self._test_skip_update_unless_force_base(enable_soft_blocking=True)
+
+    def _test_skip_update_unless_no_base_mlbf(
+        self, block_type: BlockType, filter_list: Union[List[BlockType], None] = None
+    ):
         """
-        skip update unless there is no base mlbf
+        skip update unless there is no base mlbf for the given block type
         """
         # We skip update at this point because there is a base filter.
         upload_mlbf_to_remote_settings(force_base=False)
         assert not self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
 
-        self.mocks[
+        self.mocks['olympia.blocklist.cron.get_base_generation_time'].side_effect = (
-            'olympia.blocklist.cron.get_base_generation_time'
+            lambda _block_type: None if _block_type == block_type else self.base_time
-        ].return_value = None
+        )
         upload_mlbf_to_remote_settings(force_base=False)
 
-        assert self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
+        if filter_list is None:
+            assert not self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
+        else:
+            assert (
+                mock.call(
+                    self.current_time,
+                    filter_list=filter_list,
+                    create_stash=False,
+                )
+            ) in self.mocks['olympia.blocklist.cron.upload_filter.delay'].call_args_list
+
+    def test_skip_update_unless_no_base_mlbf_for_blocked(self):
+        self._test_skip_update_unless_no_base_mlbf(
+            BlockType.BLOCKED, filter_list=[BlockType.BLOCKED.name]
+        )
+
+    @override_switch('enable-soft-blocking', active=True)
+    def test_skip_update_unless_no_base_mlbf_for_soft_blocked_with_switch_enabled(self):
+        self._test_skip_update_unless_no_base_mlbf(
+            # Until we enable soft blocking even if there is no soft block base filter
+            # and the switch is active, no update expected
+            BlockType.SOFT_BLOCKED,
+            filter_list=None,
+        )
+
+    def test_skip_update_unless_no_base_mlbf_for_soft_blocked_with_switch_disabled(
+        self,
+    ):
+        self._test_skip_update_unless_no_base_mlbf(
+            BlockType.SOFT_BLOCKED, filter_list=None
+        )
 
     def test_missing_last_filter_uses_base_filter(self):
         """
         When there is a base filter and no last filter,
         fallback to using the base filter
         """
-        self._block_version(is_signed=True)
+        block_version = self._block_version(is_signed=True)
-        # Re-created the last filter created after the new block
+        # Re-create the last filter so we ensure
+        # the block is already processed comparing to previous
         MLBF.generate_from_db(self.last_time)
 
+        assert datetime_to_ts(block_version.modified) < self.last_time
         # We skip the update at this point because the new last filter already
         # accounted for the new block.
         upload_mlbf_to_remote_settings(force_base=False)
@@ -138,48 +194,74 @@ class TestUploadToRemoteSettings(TestCase):
             'olympia.blocklist.cron.get_last_generation_time'
         ].return_value = None
         upload_mlbf_to_remote_settings(force_base=False)
-        assert self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
         assert (
             mock.call(
-                'blocklist.cron.upload_mlbf_to_remote_settings.blocked_changed', 1
+                self.current_time,
+                filter_list=[],
+                create_stash=True,
             )
-            in self.mocks['olympia.blocklist.cron.statsd.incr'].call_args_list
+        ) in self.mocks['olympia.blocklist.cron.upload_filter.delay'].call_args_list
-        )
 
-    def test_skip_update_unless_recent_modified_blocks(self):
+    @override_switch('enable-soft-blocking', active=True)
+    def test_skip_update_if_unsigned_blocks_added(self):
         """
-        skip update unless there are recent modified blocks
+        skip update if there are only unsigned new blocks
         """
+        self._block_version(block_type=BlockType.BLOCKED, is_signed=False)
+        self._block_version(block_type=BlockType.SOFT_BLOCKED, is_signed=False)
+
         upload_mlbf_to_remote_settings(force_base=False)
         assert not self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
 
-        # Now the last filter is older than the most recently modified block.
+    def _test_skip_update_unless_new_blocks(
-        older_last_time = datetime_to_ts(self.block.modified - timedelta(seconds=1))
+        self, block_type: BlockType, enable_soft_blocking=False, expect_update=False
-        self.mocks[
+    ):
-            'olympia.blocklist.cron.get_last_generation_time'
-        ].return_value = older_last_time
-        MLBF.generate_from_db(older_last_time)
-
-        upload_mlbf_to_remote_settings(force_base=False)
-        assert self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
-
-    def test_skip_update_unless_new_blocks(self):
         """
         skip update unless there are new blocks
         """
-        upload_mlbf_to_remote_settings(force_base=False)
+        with override_switch('enable-soft-blocking', active=enable_soft_blocking):
-        assert not self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
+            upload_mlbf_to_remote_settings(force_base=False)
+            assert not self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
 
-        # Now there is a new blocked version
+            # Now there is a new blocked version
-        self._block_version(is_signed=True)
+            self._block_version(block_type=block_type, is_signed=True)
-        upload_mlbf_to_remote_settings(force_base=False)
+            upload_mlbf_to_remote_settings(force_base=False)
-        assert self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
+
+            self.assertEqual(
+                expect_update,
+                self.mocks['olympia.blocklist.cron.upload_filter.delay'].called,
+            )
+
+    def test_skip_update_unless_new_blocks_for_blocked(self):
+        self._test_skip_update_unless_new_blocks(
+            block_type=BlockType.BLOCKED,
+            expect_update=True,
+        )
+
+    def test_skip_update_unless_new_blocks_for_soft_blocked_with_switch_disabled(self):
+        self._test_skip_update_unless_new_blocks(
+            block_type=BlockType.SOFT_BLOCKED,
+            enable_soft_blocking=False,
+            expect_update=False,
+        )
+
+    def test_skip_update_unless_new_blocks_for_soft_blocked_with_switch_enabled(self):
+        self._test_skip_update_unless_new_blocks(
+            block_type=BlockType.SOFT_BLOCKED,
+            enable_soft_blocking=True,
+            # Until we enable soft blocking
+            # even if there is a new soft block
+            # and switch is active, expect no update
+            expect_update=False,
+        )
 
     def test_send_statsd_counts(self):
         """
-        Send statsd counts for the number of blocked and not blocked items.
+        Send statsd counts for the number of blocked,
+        soft blocked, and not blocked items.
         """
-        self._block_version(is_signed=True)
+        self._block_version(block_type=BlockType.BLOCKED)
+        self._block_version(block_type=BlockType.SOFT_BLOCKED)
         upload_mlbf_to_remote_settings()
 
         statsd_calls = self.mocks['olympia.blocklist.cron.statsd.incr'].call_args_list
@@ -206,27 +288,172 @@ class TestUploadToRemoteSettings(TestCase):
         upload_mlbf_to_remote_settings(bypass_switch=True)
         assert self.mocks['olympia.blocklist.cron.statsd.incr'].called
 
-    def test_upload_stash_unless_force_base(self):
+    def _test_upload_stash_unless_force_base(
+        self,
+        block_types: List[BlockType],
+        expect_stash: bool,
+        filter_list: Union[List[BlockType], None],
+        enable_soft_blocking: bool,
+    ):
         """
         Upload a stash unless force_base is true. When there is a new block,
         We expect to upload a stash, unless the force_base is true, in which case
         we upload a new filter.
         """
-        force_base = False
+        for block_type in block_types:
-        self._block_version(is_signed=True)
+            self._block_version(block_type=block_type)
-        upload_mlbf_to_remote_settings(force_base=force_base)
+
-        assert self.mocks[
+        with override_switch('enable-soft-blocking', active=enable_soft_blocking):
-            'olympia.blocklist.cron.upload_filter.delay'
+            upload_mlbf_to_remote_settings(force_base=False)
-        ].call_args_list == [
+
-            mock.call(
+            self.assertEqual(
-                self.current_time,
+                expect_stash,
-                filter_list=[BlockType.BLOCKED.name] if force_base else [],
+                mock.call(
-                create_stash=not force_base,
+                    self.current_time,
+                    filter_list=[],
+                    create_stash=True,
+                )
+                in self.mocks[
+                    'olympia.blocklist.cron.upload_filter.delay'
+                ].call_args_list,
             )
-        ]
+
-        mlbf = MLBF.load_from_storage(self.current_time)
+            mlbf = MLBF.load_from_storage(self.current_time)
-        assert mlbf.storage.exists(mlbf.filter_path()) == force_base
+
-        assert mlbf.storage.exists(mlbf.stash_path) != force_base
+            if expect_stash:
+                assert mlbf.storage.exists(mlbf.stash_path)
+
+                for block_type in BlockType:
+                    assert not mlbf.storage.exists(mlbf.filter_path(block_type))
+            else:
+                assert mlbf is None
+
+            upload_mlbf_to_remote_settings(force_base=True)
+            next_mlbf = MLBF.load_from_storage(self.current_time)
+            expected_block_types = []
+
+            for block_type in filter_list:
+                assert next_mlbf.storage.exists(next_mlbf.filter_path(block_type))
+                expected_block_types.append(block_type.name)
+
+            assert (
+                mock.call(
+                    self.current_time,
+                    filter_list=expected_block_types,
+                    create_stash=False,
+                )
+                in self.mocks[
+                    'olympia.blocklist.cron.upload_filter.delay'
+                ].call_args_list
+            )
+
+    def test_upload_stash_unless_force_base_for_blocked_with_switch_disabled(self):
+        """
+        When force base is false, it uploads a stash because there is a new hard blocked
+        version. When force base is true, it uploads the blocked filter for the same
+        reason.
+        """
+        self._test_upload_stash_unless_force_base(
+            block_types=[BlockType.BLOCKED],
+            expect_stash=True,
+            filter_list=[BlockType.BLOCKED],
+            enable_soft_blocking=False,
+        )
+
+    def test_upload_stash_unless_force_base_for_blocked_with_switch_enabled(self):
+        """
+        When force base is false, it uploads a stash because soft block is enabled
+        and there is a new hard blocked version. When force base is true, it uploads
+        both blocked and soft blocked filters for the previous reason and because
+        soft blocking is enabled.
+        """
+        self._test_upload_stash_unless_force_base(
+            block_types=[BlockType.BLOCKED],
+            expect_stash=True,
+            # Even if updating a soft block and the switch is active
+            # don't expect a soft block filter update until we
+            # implement support for that
+            filter_list=[BlockType.BLOCKED],
+            enable_soft_blocking=True,
+        )
+
+    def test_upload_stash_unless_force_base_for_soft_blocked_with_switch_disabled(self):
+        """
+        When force base is false, it does not upload a stash even when there is a new
+        soft blocked version, because soft blocking is disabled.
+        When force base is true, it uploads only the blocked filter
+        for the same reason.
+        """
+        self._test_upload_stash_unless_force_base(
+            block_types=[BlockType.SOFT_BLOCKED],
+            expect_stash=False,
+            filter_list=[BlockType.BLOCKED],
+            enable_soft_blocking=False,
+        )
+
+    def test_upload_stash_unless_force_base_for_soft_blocked_with_switch_enabled(self):
+        """
+        When force base is false, it uploads a stash because soft block is enabled
+        and there is a new soft blocked version. When force base is true, it uploads
+        both blocked and soft blocked filters.
+        """
+        self._test_upload_stash_unless_force_base(
+            block_types=[BlockType.SOFT_BLOCKED],
+            # Even if updating a soft block and the switch is active
+            # don't expect a soft block filter update until we
+            # implement support for that
+            expect_stash=False,
+            filter_list=[BlockType.BLOCKED],
+            enable_soft_blocking=True,
+        )
+
+    def test_upload_stash_unless_force_base_for_both_blocked_with_switch_disabled(self):
+        """
+        When force base is false, it uploads a stash even though soft blocking disabled
+        because there is a hard blocked version. When force base is true,
+        it uploads only the blocked filter for the same reason.
+        """
+        self._test_upload_stash_unless_force_base(
+            block_types=[BlockType.BLOCKED, BlockType.SOFT_BLOCKED],
+            expect_stash=True,
+            filter_list=[BlockType.BLOCKED],
+            enable_soft_blocking=False,
+        )
+
+    def test_upload_stash_unless_force_base_for_both_blocked_with_switch_enabled(self):
+        """
+        When force base is false, it uploads a stash because there are new hard and soft
+        blocked versions. When force base is true,
+        it uploads both blocked + soft blocked filters for the same reason.
+        """
+        self._test_upload_stash_unless_force_base(
+            block_types=[BlockType.BLOCKED, BlockType.SOFT_BLOCKED],
+            expect_stash=True,
+            # Even if updating a soft block and the switch is active
+            # don't expect a soft block filter update until we
+            # implement support for that
+            filter_list=[BlockType.BLOCKED],
+            enable_soft_blocking=True,
+        )
+
+    def test_dont_upload_stash_unless_force_base_for_both_blocked_with_switch_enabled(
+        self,
+    ):
+        """
+        When force base is false, it does not upload a stash because
+        there are no new versions.When force base is true,
+        it uploads both blocked and soft blocked filters because
+        soft blocking is enabled.
+        """
+        self._test_upload_stash_unless_force_base(
+            block_types=[],
+            expect_stash=False,
+            # Even if updating a soft block and the switch is active
+            # don't expect a soft block filter update until we
+            # implement support for that
+            filter_list=[BlockType.BLOCKED],
+            enable_soft_blocking=True,
+        )
 
     def test_upload_stash_unless_missing_base_filter(self):
         """
@@ -244,12 +471,13 @@ class TestUploadToRemoteSettings(TestCase):
             )
         ]
         mlbf = MLBF.load_from_storage(self.current_time)
-        assert not mlbf.storage.exists(mlbf.filter_path())
+        assert not mlbf.storage.exists(mlbf.filter_path(BlockType.BLOCKED))
+        assert not mlbf.storage.exists(mlbf.filter_path(BlockType.SOFT_BLOCKED))
         assert mlbf.storage.exists(mlbf.stash_path)
 
-        self.mocks[
+        self.mocks['olympia.blocklist.cron.get_base_generation_time'].side_effect = (
-            'olympia.blocklist.cron.get_base_generation_time'
+            lambda _block_type: None
-        ].return_value = None
+        )
         upload_mlbf_to_remote_settings()
         assert (
             mock.call(
@@ -259,15 +487,34 @@ class TestUploadToRemoteSettings(TestCase):
             )
             in self.mocks['olympia.blocklist.cron.upload_filter.delay'].call_args_list
         )
-        assert mlbf.storage.exists(mlbf.filter_path())
+        assert mlbf.storage.exists(mlbf.filter_path(BlockType.BLOCKED))
 
-    @mock.patch('olympia.blocklist.cron.BASE_REPLACE_THRESHOLD', 1)
+        with override_switch('enable-soft-blocking', active=True):
+            upload_mlbf_to_remote_settings()
+            assert not mlbf.storage.exists(mlbf.filter_path(BlockType.SOFT_BLOCKED))
+            assert (
+                mock.call(
+                    self.current_time,
+                    # Even if updating a soft block and the switch is active
+                    # don't expect a soft block filter update until we
+                    # implement support for that
+                    filter_list=[BlockType.BLOCKED.name],
+                    create_stash=False,
+                )
+            ) in self.mocks['olympia.blocklist.cron.upload_filter.delay'].call_args_list
+
+    # TODO: add test for soft blocks and ensure stash/filter compatibility
+    @mock.patch('olympia.blocklist.mlbf.BASE_REPLACE_THRESHOLD', 1)
+    @override_switch('enable-soft-blocking', active=True)
     def test_upload_stash_unless_enough_changes(self):
+        block_type = BlockType.BLOCKED
         """
         When there are new blocks, upload either a stash or a filter depending on
         whether we have surpased the BASE_REPLACE_THRESHOLD amount.
         """
-        self._block_version(is_signed=True)
+        for _block_type in BlockType:
+            self._block_version(is_signed=True, block_type=_block_type)
+
         upload_mlbf_to_remote_settings()
         assert self.mocks[
             'olympia.blocklist.cron.upload_filter.delay'
@@ -279,44 +526,71 @@ class TestUploadToRemoteSettings(TestCase):
             )
         ]
         mlbf = MLBF.load_from_storage(self.current_time)
-        assert not mlbf.storage.exists(mlbf.filter_path())
+        assert not mlbf.storage.exists(mlbf.filter_path(block_type))
         assert mlbf.storage.exists(mlbf.stash_path)
 
-        self._block_version(is_signed=True)
+        # delete the mlbf so we can test again with different conditions
-        # Create a new current time so we can test that the stash is not created
+        mlbf.delete()
-        self.current_time = datetime_to_ts(self.block.modified + timedelta(seconds=4))
+
-        self.mocks[
+        self._block_version(is_signed=True, block_type=block_type)
-            'olympia.blocklist.cron.get_generation_time'
+
-        ].return_value = self.current_time
         upload_mlbf_to_remote_settings()
         assert (
             mock.call(
                 self.current_time,
-                filter_list=[BlockType.BLOCKED.name],
+                filter_list=[block_type.name],
                 create_stash=False,
             )
             in self.mocks['olympia.blocklist.cron.upload_filter.delay'].call_args_list
         )
         new_mlbf = MLBF.load_from_storage(self.current_time)
-        assert new_mlbf.storage.exists(new_mlbf.filter_path())
+        assert new_mlbf.storage.exists(new_mlbf.filter_path(block_type))
         assert not new_mlbf.storage.exists(new_mlbf.stash_path)
 
-    def test_cleanup_old_files(self):
+    @mock.patch('olympia.blocklist.mlbf.BASE_REPLACE_THRESHOLD', 1)
+    def test_upload_stash_even_if_filter_is_updated(self):
         """
-        Cleanup old files only if a base filter already exists.
+        If enough changes of one type are made, update the filter, but still upload
+        a stash if there are changes of other types.
         """
-        upload_mlbf_to_remote_settings(force_base=True)
+        self._block_version(is_signed=True, block_type=BlockType.BLOCKED)
+        self._block_version(is_signed=True, block_type=BlockType.BLOCKED)
+        self._block_version(is_signed=True, block_type=BlockType.SOFT_BLOCKED)
+        upload_mlbf_to_remote_settings()
         assert self.mocks[
-            'olympia.blocklist.cron.cleanup_old_files.delay'
+            'olympia.blocklist.cron.upload_filter.delay'
-        ].call_args_list == [mock.call(base_filter_id=self.base_time)]
+        ].call_args_list == [
+            mock.call(
+                self.current_time,
+                filter_list=[BlockType.BLOCKED.name],
+                create_stash=False,
+            )
+        ]
+        mlbf = MLBF.load_from_storage(self.current_time)
+        assert mlbf.storage.exists(mlbf.filter_path(BlockType.BLOCKED))
+        assert not mlbf.storage.exists(mlbf.stash_path)
 
-        self.mocks[
+        with override_switch('enable-soft-blocking', active=True):
-            'olympia.blocklist.cron.get_base_generation_time'
+            self._block_version(is_signed=True, block_type=BlockType.BLOCKED)
-        ].return_value = None
+            self._block_version(is_signed=True, block_type=BlockType.BLOCKED)
-        upload_mlbf_to_remote_settings(force_base=True)
+            upload_mlbf_to_remote_settings()
-        assert (
+            self.mocks['olympia.blocklist.cron.upload_filter.delay'].assert_called_with(
-            self.mocks['olympia.blocklist.cron.cleanup_old_files.delay'].call_count == 1
+                self.current_time,
-        )
+                filter_list=[BlockType.BLOCKED.name],
+                create_stash=False,
+            )
+            mlbf = MLBF.load_from_storage(self.current_time)
+            mlbf = MLBF.load_from_storage(self.current_time)
+            assert mlbf.storage.exists(mlbf.filter_path(BlockType.BLOCKED))
+            assert not mlbf.storage.exists(mlbf.stash_path)
+
+    def test_remove_storage_if_no_update(self):
+        """
+        If there is no update, remove the storage used by the current mlbf.
+        """
+        upload_mlbf_to_remote_settings(force_base=False)
+        assert not self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
+        assert MLBF.load_from_storage(self.current_time) is None
 
     def test_creates_base_filter_if_base_generation_time_invalid(self):
         """
@@ -327,36 +601,47 @@ class TestUploadToRemoteSettings(TestCase):
         upload_mlbf_to_remote_settings(force_base=True)
         assert self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
 
-    def test_creates_base_filter_if_last_generation_time_invalid(self):
+    def test_compares_against_base_filter_if_missing_previous_filter(self):
         """
-        When a last_generation_time is provided, but no filter exists for it,
+        When no previous filter is found, compare blocks against the base filter
-        raise no filter found.
+        of that block type.
         """
-        self.mocks['olympia.blocklist.cron.get_last_generation_time'].return_value = 1
+        # Hard block version is accounted for in the base filter
-        upload_mlbf_to_remote_settings(force_base=True)
+        self._block_version(block_type=BlockType.BLOCKED)
-        assert self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
+        MLBF.generate_from_db(self.base_time)
+        # Soft block version is not accounted for in the base filter
+        # but accounted for in the last filter
+        self._block_version(block_type=BlockType.SOFT_BLOCKED)
+        MLBF.generate_from_db(self.last_time)
 
+        upload_mlbf_to_remote_settings(force_base=False)
+        assert not self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
+
+        # delete the last filter, now the base filter will be used to compare
+        MLBF.load_from_storage(self.last_time).delete()
+        upload_mlbf_to_remote_settings(force_base=False)
+        # We expect to not upload anything as soft blocking is disabled
+        # and only the soft blocked version is missing from the base filter
+        assert not self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
+
+    @override_switch('enable-soft-blocking', active=True)
     def test_dont_skip_update_if_all_blocked_or_not_blocked(self):
         """
         If all versions are either blocked or not blocked, skip the update.
         """
-        version = self._block_version(is_signed=True)
+        for _ in range(0, 10):
-        upload_mlbf_to_remote_settings(force_base=True)
+            self._block_version(block_type=BlockType.BLOCKED)
-        assert self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
+
-        version.update(block_type=BlockType.SOFT_BLOCKED)
+        upload_mlbf_to_remote_settings()
-        upload_mlbf_to_remote_settings(force_base=True)
         assert self.mocks['olympia.blocklist.cron.upload_filter.delay'].called
 
     def test_invalid_cache_results_in_diff(self):
         self._block_version(block_type=BlockType.BLOCKED)
 
-        # First we re-create the last filter including the blocked version
+        # First we create the current filter including the blocked version
-        self.mocks[
-            'olympia.blocklist.cron.get_generation_time'
-        ].return_value = self.last_time
         upload_mlbf_to_remote_settings()
 
-        base_mlbf = MLBF.load_from_storage(self.last_time)
+        base_mlbf = MLBF.load_from_storage(self.current_time)
 
         # Remove the blocked version from the cache.json file so we can test that
         # the next generation includes the blocked version.
@@ -367,24 +652,37 @@ class TestUploadToRemoteSettings(TestCase):
             json.dump(data, f)
             f.truncate()
 
-        # Reset the generation time to the current time so we can test that the
+        # Set the generation time to after the current time so we can test that the
         # diff includes the blocked version after it is removed from the cache.json
+        next_time = self.current_time + 1
         self.mocks[
             'olympia.blocklist.cron.get_generation_time'
-        ].return_value = self.current_time
+        ].return_value = next_time
         upload_mlbf_to_remote_settings()
 
         # We expect to upload a stash because the cache.json we are comparing against
         # is missing the blocked version.
         assert (
             mock.call(
-                self.current_time,
+                next_time,
                 filter_list=[],
                 create_stash=True,
             )
             in self.mocks['olympia.blocklist.cron.upload_filter.delay'].call_args_list
         )
 
+    def test_pass_correct_arguments_to_upload_filter(self):
+        self.mocks['olympia.blocklist.cron.upload_filter.delay'].stop()
+        with mock.patch(
+            'olympia.blocklist.cron.upload_filter.delay', wraps=upload_filter.delay
+        ) as spy_delay:
+            upload_mlbf_to_remote_settings(force_base=True)
+            spy_delay.assert_called_with(
+                self.current_time,
+                filter_list=[BlockType.BLOCKED.name],
+                create_stash=False,
+            )
+
 
 class TestTimeMethods(TestCase):
     @freeze_time('2024-10-10 12:34:56')
@@ -398,9 +696,10 @@ class TestTimeMethods(TestCase):
         assert get_last_generation_time() == 1
 
     def test_get_base_generation_time(self):
-        assert get_base_generation_time() is None
+        for block_type in BlockType:
-        set_config(MLBF_BASE_ID_CONFIG_KEY(BlockType.BLOCKED, compat=True), 1)
+            assert get_base_generation_time(block_type) is None
-        assert get_base_generation_time() == 1
+            set_config(MLBF_BASE_ID_CONFIG_KEY(block_type, compat=True), 1)
+            assert get_base_generation_time(block_type) == 1
 
 
 @pytest.mark.django_db