Ensure blocks of type X are excluded from stash if filter of type X i…

…s also being uploaded

src/olympia/blocklist/cron.py

@@ -6,7 +6,6 @@
 
 import olympia.core.logger
 from olympia.constants.blocklist import (
-    BASE_REPLACE_THRESHOLD,
     MLBF_BASE_ID_CONFIG_KEY,
     MLBF_TIME_CONFIG_KEY,
 )
@@ -90,26 +89,18 @@ def _upload_mlbf_to_remote_settings(*, force_base=False):
 
         base_filter = MLBF.load_from_storage(get_base_generation_time(block_type))
 
-        should_update_filter = (
+        # add this block type to the list of filters to be re-uploaded
+        if (
             force_base
             or base_filter is None
-            or mlbf.blocks_changed_since_previous(block_type, base_filter)
-            > BASE_REPLACE_THRESHOLD
-        )
-
-        should_update_stash = (
-            mlbf.blocks_changed_since_previous(
-                block_type, previous_filter or base_filter
-            )
-            > 0
-        )
-
-        # add this block type to the list of filters to be re-uploaded
-        if should_update_filter:
+            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 should_update_stash:
+        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

src/olympia/blocklist/mlbf.py

@@ -14,6 +14,7 @@
 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
 
 
@@ -296,6 +297,10 @@ def generate_and_write_stash(self, previous_mlbf: 'MLBF' = None):
         in the "unblocked" stash (like for hard blocked items) as this would
         result in the version being in both blocked and unblocked stashes.
         """
+        STASH_KEYS = {
+            'blocked': BlockType.BLOCKED,
+            'softblocked': BlockType.SOFT_BLOCKED,
+        }
         diffs = self.generate_diffs(previous_mlbf)
         blocked_added, blocked_removed, _ = diffs[BlockType.BLOCKED]
         stash_json = {
@@ -312,6 +317,16 @@ def generate_and_write_stash(self, previous_mlbf: 'MLBF' = None):
                 if unblocked not in blocked_added
             ]
 
+        # Filter out any stash keys where we should instead upload the filter
+        for key, block_type in STASH_KEYS.items():
+            # If the change_count for the given block type is
+            # greater than the base replace threshold, we should
+            # upload the filter instead of the stash
+            if self.should_upload_filter(
+                block_type=block_type, previous_mlbf=previous_mlbf
+            ):
+                stash_json[key] = []
+
         # write stash
         stash_path = self.stash_path
         with self.storage.open(stash_path, 'w') as json_file:
@@ -325,6 +340,26 @@ def blocks_changed_since_previous(
         _, _, 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_cron.py

@@ -489,7 +489,7 @@ def test_upload_stash_unless_missing_base_filter(self):
                 )
             ) in self.mocks['olympia.blocklist.cron.upload_filter.delay'].call_args_list
 
-    @mock.patch('olympia.blocklist.cron.BASE_REPLACE_THRESHOLD', 1)
+    @mock.patch('olympia.blocklist.mlbf.BASE_REPLACE_THRESHOLD', 1)
     def test_upload_stash_unless_enough_changes(self):
         """
         When there are new blocks, upload either a stash or a filter depending on
@@ -529,7 +529,7 @@ def test_upload_stash_unless_enough_changes(self):
         assert new_mlbf.storage.exists(new_mlbf.filter_path(BlockType.BLOCKED))
         assert not new_mlbf.storage.exists(new_mlbf.stash_path)
 
-    @mock.patch('olympia.blocklist.cron.BASE_REPLACE_THRESHOLD', 1)
+    @mock.patch('olympia.blocklist.mlbf.BASE_REPLACE_THRESHOLD', 1)
     def test_upload_stash_even_if_filter_is_updated(self):
         """
         If enough changes of one type are made, update the filter, but still upload
@@ -548,6 +548,9 @@ def test_upload_stash_even_if_filter_is_updated(self):
                 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)
 
         with override_switch('enable-soft-blocking', active=True):
             self._block_version(is_signed=True, block_type=BlockType.BLOCKED)
@@ -558,6 +561,13 @@ def test_upload_stash_even_if_filter_is_updated(self):
                 filter_list=[BlockType.BLOCKED.name],
                 create_stash=True,
             )
+            mlbf = MLBF.load_from_storage(self.current_time)
+            assert mlbf.storage.exists(mlbf.filter_path(BlockType.BLOCKED))
+            assert mlbf.storage.exists(mlbf.stash_path)
+            with mlbf.storage.open(mlbf.stash_path, 'r') as f:
+                data = json.load(f)
+                assert len(data['blocked']) == 0
+                assert len(data['softblocked']) == 1
 
     def test_remove_storage_if_no_update(self):
         """