diff --git a/src/olympia/blocklist/cron.py b/src/olympia/blocklist/cron.py index dbbc7c89d2a0..0917e784e13b 100644 --- a/src/olympia/blocklist/cron.py +++ b/src/olympia/blocklist/cron.py @@ -81,16 +81,6 @@ def _upload_mlbf_to_remote_settings(*, force_base=False): # Determine which base filters need to be re uploaded # and whether a new stash needs to be created. for block_type in BlockType: - # This prevents us from updating a stash or filter based on new soft blocks - # until we are ready to enable soft blocking. - if block_type == BlockType.SOFT_BLOCKED and not waffle.switch_is_active( - 'enable-soft-blocking' - ): - log.info( - 'Skipping soft-blocks because enable-soft-blocking switch is inactive' - ) - continue - base_filter = MLBF.load_from_storage(get_base_generation_time(block_type)) base_filters[block_type] = base_filter diff --git a/src/olympia/blocklist/mlbf.py b/src/olympia/blocklist/mlbf.py index e8968592b048..87b2c625734f 100644 --- a/src/olympia/blocklist/mlbf.py +++ b/src/olympia/blocklist/mlbf.py @@ -7,7 +7,6 @@ from django.utils.functional import cached_property -import waffle from filtercascade import FilterCascade from filtercascade.fileformats import HashAlgorithm @@ -352,14 +351,13 @@ def generate_and_write_stash( stash_json[STASH_KEYS[BlockType.BLOCKED]] = blocked_added stash_json[UNBLOCKED_STASH_KEY] = blocked_removed - if waffle.switch_is_active('enable-soft-blocking'): - soft_blocked_added, soft_blocked_removed, _ = diffs[BlockType.SOFT_BLOCKED] - added_items.update(soft_blocked_added) - if not self.should_upload_filter( - BlockType.SOFT_BLOCKED, soft_blocked_base_filter - ): - stash_json[STASH_KEYS[BlockType.SOFT_BLOCKED]] = soft_blocked_added - stash_json[UNBLOCKED_STASH_KEY].extend(soft_blocked_removed) + soft_blocked_added, soft_blocked_removed, _ = diffs[BlockType.SOFT_BLOCKED] + added_items.update(soft_blocked_added) + if not self.should_upload_filter( + BlockType.SOFT_BLOCKED, soft_blocked_base_filter + ): + stash_json[STASH_KEYS[BlockType.SOFT_BLOCKED]] = soft_blocked_added + stash_json[UNBLOCKED_STASH_KEY].extend(soft_blocked_removed) # Remove any items that were added to a block type. stash_json[UNBLOCKED_STASH_KEY] = [ diff --git a/src/olympia/blocklist/tests/test_cron.py b/src/olympia/blocklist/tests/test_cron.py index 0f98be43e79d..d4d0cb4f1711 100644 --- a/src/olympia/blocklist/tests/test_cron.py +++ b/src/olympia/blocklist/tests/test_cron.py @@ -43,7 +43,6 @@ @freeze_time('2020-01-01 12:34:56') @override_switch('blocklist_mlbf_submit', active=True) -@override_switch('enable-soft-blocking', active=False) class TestUploadToRemoteSettings(TestCase): def setUp(self): self.user = user_factory() @@ -256,7 +255,6 @@ def test_upload_stash_unless_force_base(self): ) @mock.patch('olympia.blocklist.mlbf.get_base_replace_threshold') - @override_switch('enable-soft-blocking', active=True) def test_upload_stash_unless_enough_changes(self, mock_get_base_replace_threshold): """ When there are new blocks, upload a stash unless @@ -303,12 +301,10 @@ def test_upload_stash_unless_enough_changes(self, mock_get_base_replace_threshol def _test_upload_stash_and_filter( self, - enable_soft_blocking: bool, expected_actions: List[BlockListAction], ): set_config(BASE_REPLACE_THRESHOLD_KEY, 1) - with override_switch('enable-soft-blocking', active=enable_soft_blocking): - upload_mlbf_to_remote_settings() + upload_mlbf_to_remote_settings() # Generation time is set to current time so we can load the MLBF. mlbf = MLBF.load_from_storage(self.current_time) @@ -346,23 +342,11 @@ def test_upload_blocked_stash_and_softblock_filter(self): self._block_version(is_signed=True, block_type=BlockType.SOFT_BLOCKED) self._test_upload_stash_and_filter( - # Even though there are enough soft blocks, soft blocking is disabled - # So we only upload a stash for the blocked version - enable_soft_blocking=False, - expected_actions=[ - BlockListAction.UPLOAD_STASH.name, - ], - ) - - # Now try again with soft blocking enabled - self._test_upload_stash_and_filter( - # Soft blocking is enabled, so we expect the same stash and a new filter - enable_soft_blocking=True, - expected_actions=[ + [ BlockListAction.UPLOAD_BLOCKED_FILTER.name, BlockListAction.UPLOAD_SOFT_BLOCKED_FILTER.name, BlockListAction.CLEAR_STASH.name, - ], + ] ) def test_upload_soft_blocked_stash_and_blocked_filter(self): @@ -377,15 +361,13 @@ def test_upload_soft_blocked_stash_and_blocked_filter(self): self._block_version(is_signed=True, block_type=BlockType.BLOCKED) self._block_version(is_signed=True, block_type=BlockType.BLOCKED) - for enable_soft_blocking in [False, True]: - self._test_upload_stash_and_filter( - enable_soft_blocking=enable_soft_blocking, - expected_actions=[ - BlockListAction.UPLOAD_BLOCKED_FILTER.name, - BlockListAction.UPLOAD_SOFT_BLOCKED_FILTER.name, - BlockListAction.CLEAR_STASH.name, - ], - ) + self._test_upload_stash_and_filter( + [ + BlockListAction.UPLOAD_BLOCKED_FILTER.name, + BlockListAction.UPLOAD_SOFT_BLOCKED_FILTER.name, + BlockListAction.CLEAR_STASH.name, + ] + ) def test_upload_blocked_and_softblocked_filter(self): """ @@ -399,15 +381,13 @@ def test_upload_blocked_and_softblocked_filter(self): self._block_version(is_signed=True, block_type=BlockType.SOFT_BLOCKED) self._block_version(is_signed=True, block_type=BlockType.SOFT_BLOCKED) - for enable_soft_blocking in [False, True]: - self._test_upload_stash_and_filter( - enable_soft_blocking=enable_soft_blocking, - expected_actions=[ - BlockListAction.UPLOAD_BLOCKED_FILTER.name, - BlockListAction.UPLOAD_SOFT_BLOCKED_FILTER.name, - BlockListAction.CLEAR_STASH.name, - ], - ) + self._test_upload_stash_and_filter( + [ + BlockListAction.UPLOAD_BLOCKED_FILTER.name, + BlockListAction.UPLOAD_SOFT_BLOCKED_FILTER.name, + BlockListAction.CLEAR_STASH.name, + ] + ) def test_upload_blocked_and_softblocked_stash(self): """ @@ -419,13 +399,11 @@ def test_upload_blocked_and_softblocked_stash(self): # Enough soft blocks for a stash self._block_version(is_signed=True, block_type=BlockType.SOFT_BLOCKED) - for enable_soft_blocking in [False, True]: - self._test_upload_stash_and_filter( - enable_soft_blocking=enable_soft_blocking, - expected_actions=[ - BlockListAction.UPLOAD_STASH.name, - ], - ) + self._test_upload_stash_and_filter( + [ + BlockListAction.UPLOAD_STASH.name, + ] + ) def test_remove_storage_if_no_update(self): """ @@ -494,7 +472,6 @@ def _test_dont_skip_update_if_all_blocked_or_not_blocked( ], ) - @override_switch('enable-soft-blocking', active=True) def test_dont_skip_update_if_all_blocked_or_not_blocked_for_soft_blocked(self): self._test_dont_skip_update_if_all_blocked_or_not_blocked( block_type=BlockType.SOFT_BLOCKED diff --git a/src/olympia/blocklist/tests/test_mlbf.py b/src/olympia/blocklist/tests/test_mlbf.py index 4c2cab0f6f56..188bc69f3449 100644 --- a/src/olympia/blocklist/tests/test_mlbf.py +++ b/src/olympia/blocklist/tests/test_mlbf.py @@ -2,8 +2,6 @@ from functools import cached_property from unittest import mock -from waffle.testutils import override_switch - from olympia import amo from olympia.addons.models import GUID_REUSE_FORMAT from olympia.amo.tests import ( @@ -474,15 +472,9 @@ def test_block_added(self): } assert next_mlbf.generate_and_write_stash(previous_mlbf=base_mlbf) == { 'blocked': [new_block_hash], - 'softblocked': [], + 'softblocked': [new_soft_block_hash], 'unblocked': [], } - with override_switch('enable-soft-blocking', active=True): - assert next_mlbf.generate_and_write_stash(previous_mlbf=base_mlbf) == { - 'blocked': [new_block_hash], - 'softblocked': [new_soft_block_hash], - 'unblocked': [], - } def test_block_removed(self): addon, block = self._blocked_addon() @@ -570,15 +562,9 @@ def test_block_hard_to_soft(self): } assert next_mlbf.generate_and_write_stash(previous_mlbf=base_mlbf) == { 'blocked': [], - 'softblocked': [], - 'unblocked': [block_hash], + 'softblocked': [block_hash], + 'unblocked': [], } - with override_switch('enable-soft-blocking', active=True): - assert next_mlbf.generate_and_write_stash(previous_mlbf=base_mlbf) == { - 'blocked': [], - 'softblocked': [block_hash], - 'unblocked': [], - } def test_block_soft_to_hard(self): addon, block = self._blocked_addon() @@ -634,17 +620,10 @@ def test_hard_to_soft_multiple(self, mock_get_base_replace_threshold): assert next_mlbf.generate_and_write_stash(previous_mlbf=base_mlbf) == { 'blocked': [], - 'softblocked': [], - 'unblocked': block_hashes, + 'softblocked': block_hashes, + 'unblocked': [], } - with override_switch('enable-soft-blocking', active=True): - assert next_mlbf.generate_and_write_stash(previous_mlbf=base_mlbf) == { - 'blocked': [], - 'softblocked': block_hashes, - 'unblocked': [], - } - @mock.patch('olympia.blocklist.mlbf.get_base_replace_threshold') def test_stash_is_empty_if_uploading_new_filter( self, mock_get_base_replace_threshold @@ -679,21 +658,12 @@ def test_stash_is_empty_if_uploading_new_filter( # No new filter yet assert not next_mlbf.should_upload_filter(BlockType.BLOCKED, mlbf) - # Only the hard blocked version is included in the stash assert next_mlbf.generate_and_write_stash(mlbf) == { 'blocked': [hard_stash], - 'softblocked': [], + 'softblocked': [soft_stash], 'unblocked': [], } - # With soft blocking enabled, both versions are included in the stash - with override_switch('enable-soft-blocking', active=True): - assert next_mlbf.generate_and_write_stash(mlbf) == { - 'blocked': [hard_stash], - 'softblocked': [soft_stash], - 'unblocked': [], - } - # Harden the soft blocked version soft_block.update(block_type=BlockType.BLOCKED) final_mlbf = MLBF.generate_from_db('test_three') @@ -809,21 +779,12 @@ def test_diff_all_possible_changes(self): BlockType.SOFT_BLOCKED: ([three_hash, four_hash], [], 2), } - # The first time we generate the stash, we expect 3 to 6 to be in the stash - # as they have some kind of block applied. assert first_mlbf.generate_and_write_stash() == { 'blocked': [five_hash, six_hash], - 'softblocked': [], + 'softblocked': [three_hash, four_hash], 'unblocked': [], } - with override_switch('enable-soft-blocking', active=True): - assert first_mlbf.generate_and_write_stash() == { - 'blocked': [five_hash, six_hash], - 'softblocked': [three_hash, four_hash], - 'unblocked': [], - } - # Update the existing blocks, and create new ones for # the versions "one" and "two". @@ -865,21 +826,12 @@ def test_diff_all_possible_changes(self): assert second_mlbf.generate_and_write_stash(previous_mlbf=first_mlbf) == { 'blocked': [three_hash, two_hash], - # Soft blocked is empty because the waffle switch is off - 'softblocked': [], - # 4 is omitted because it's soft blocked and the waffle switch is off - 'unblocked': [five_hash, six_hash], + 'softblocked': [five_hash, one_hash], + # 3 and 5 are omitted because they transitioned + # from one block type to another + 'unblocked': [six_hash, four_hash], } - with override_switch('enable-soft-blocking', active=True): - assert second_mlbf.generate_and_write_stash(previous_mlbf=first_mlbf) == { - 'blocked': [three_hash, two_hash], - 'softblocked': [five_hash, one_hash], - # 3 and 5 are omitted because they transitioned - # from one block type to another - 'unblocked': [six_hash, four_hash], - } - def test_generate_stash_returns_expected_stash(self): addon, block = self._blocked_addon() block_versions = [ @@ -901,34 +853,23 @@ def test_generate_stash_returns_expected_stash(self): 'unblocked': [], } - with override_switch('enable-soft-blocking', active=True): - assert mlbf.generate_and_write_stash() == { - 'blocked': MLBF.hash_filter_inputs(expected_blocked), - 'softblocked': [], - 'unblocked': [], - } + assert mlbf.generate_and_write_stash() == { + 'blocked': MLBF.hash_filter_inputs(expected_blocked), + 'softblocked': [], + 'unblocked': [], + } # Remove the last block version block_versions[-1].delete() expected_unblocked = expected_blocked[-1:] next_mlbf = MLBF.generate_from_db('test_two') - next_mlbf.generate_and_write_stash(previous_mlbf=mlbf) - - with next_mlbf.storage.open(next_mlbf.stash_path, 'r') as f: - assert json.load(f) == { - 'blocked': [], - # Soft blocked is empty because the waffle switch is off - 'softblocked': [], - 'unblocked': MLBF.hash_filter_inputs(expected_unblocked), - } - with override_switch('enable-soft-blocking', active=True): - assert next_mlbf.generate_and_write_stash(previous_mlbf=mlbf) == { - 'blocked': [], - 'softblocked': [], - 'unblocked': MLBF.hash_filter_inputs(expected_unblocked), - } + assert next_mlbf.generate_and_write_stash(previous_mlbf=mlbf) == { + 'blocked': [], + 'softblocked': [], + 'unblocked': MLBF.hash_filter_inputs(expected_unblocked), + } @mock.patch('olympia.blocklist.mlbf.get_base_replace_threshold') def test_generate_empty_stash_when_all_items_in_filter( @@ -960,49 +901,27 @@ def test_generate_empty_stash_when_all_items_in_filter( mlbf = MLBF.generate_from_db('test') - # We should create a soft blocked filter and a stash assert mlbf.should_upload_filter(BlockType.SOFT_BLOCKED, base_mlbf) + # We have a softened block so we should upload stash, even though + # it will be empty since the block will be handled by the filter assert mlbf.should_upload_stash(BlockType.BLOCKED, base_mlbf) - # If soft blocking is disabled, then softening a block is treated - # as an unblock. + # If soft blocking is enabled, then we will expect a new soft block filter + # and no softblock stash since the blocks will be handled by the filter + # similarly we do not include the blocked version in the unblocked stash + # because it is now soft blocked. + # We actually would like this to result in no stash being created + # Bug: https://github.com/mozilla/addons/issues/15202 assert mlbf.generate_and_write_stash( previous_mlbf=base_mlbf, blocked_base_filter=base_mlbf, - soft_blocked_base_filter=None, + soft_blocked_base_filter=base_mlbf, ) == { 'blocked': [], 'softblocked': [], - 'unblocked': [hard_block_hash], + 'unblocked': [], } - # Recreate the mlbf and re-run with softblocking enabled - mlbf.delete() - - with override_switch('enable-soft-blocking', active=True): - mlbf.generate_from_db('test') - - assert mlbf.should_upload_filter(BlockType.SOFT_BLOCKED, base_mlbf) - # We have a softened block so we should upload stash, even though - # it will be empty since the block will be handled by the filter - assert mlbf.should_upload_stash(BlockType.BLOCKED, base_mlbf) - - # If soft blocking is enabled, then we will expect a new soft block filter - # and no softblock stash since the blocks will be handled by the filter - # similarly we do not include the blocked version in the unblocked stash - # because it is now soft blocked. - # We actually would like this to result in no stash being created - # Bug: https://github.com/mozilla/addons/issues/15202 - assert mlbf.generate_and_write_stash( - previous_mlbf=base_mlbf, - blocked_base_filter=base_mlbf, - soft_blocked_base_filter=base_mlbf, - ) == { - 'blocked': [], - 'softblocked': [], - 'unblocked': [], - } - def test_generate_filter_returns_expected_data(self): addon, block = self._blocked_addon() not_blocked = self._version(addon) diff --git a/src/olympia/blocklist/tests/test_tasks.py b/src/olympia/blocklist/tests/test_tasks.py index 1bf305f6e53c..4dc48bb3453e 100644 --- a/src/olympia/blocklist/tests/test_tasks.py +++ b/src/olympia/blocklist/tests/test_tasks.py @@ -9,7 +9,6 @@ from django.test.testcases import TransactionTestCase import pytest -from waffle.testutils import override_switch from olympia.amo.tests import ( addon_factory,