diff --git a/src/olympia/blocklist/tests/test_cron.py b/src/olympia/blocklist/tests/test_cron.py index 6febb5a2547..3e45a8deb60 100644 --- a/src/olympia/blocklist/tests/test_cron.py +++ b/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 @@ -37,6 +38,7 @@ @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() @@ -58,9 +60,9 @@ def setUp(self): 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[ - 'olympia.blocklist.cron.get_base_generation_time' - ].return_value = self.base_time + self.mocks['olympia.blocklist.cron.get_base_generation_time'].side_effect = ( + lambda _block_type: self.base_time + ) self.mocks[ 'olympia.blocklist.cron.get_last_generation_time' ].return_value = self.last_time @@ -84,44 +86,94 @@ def _block_version( 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. - upload_mlbf_to_remote_settings(force_base=True) + filter_list = [BlockType.BLOCKED.name] - assert self.mocks['olympia.blocklist.cron.upload_filter.delay'].called - - # Check that both filters were created on the second attempt - mlbf = MLBF.load_from_storage(self.current_time) - assert mlbf.storage.exists(mlbf.filter_path(BlockType.BLOCKED)) - assert not mlbf.storage.exists(mlbf.filter_path(BlockType.SOFT_BLOCKED)) - assert not mlbf.storage.exists(mlbf.stash_path) + if enable_soft_blocking: + filter_list.append(BlockType.SOFT_BLOCKED.name) - with override_switch('enable-soft-blocking', active=True): + with override_switch('enable-soft-blocking', active=enable_soft_blocking): upload_mlbf_to_remote_settings(force_base=True) - assert mlbf.storage.exists(mlbf.filter_path(BlockType.SOFT_BLOCKED)) - def test_skip_update_unless_no_base_mlbf(self): + assert ( + mock.call( + self.current_time, + filter_list=filter_list, + upload_stash=False, + ) + ) in self.mocks['olympia.blocklist.cron.upload_filter.delay'].call_args_list + + # 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)), + enable_soft_blocking, + ) + 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[ - 'olympia.blocklist.cron.get_base_generation_time' - ].return_value = None + self.mocks['olympia.blocklist.cron.get_base_generation_time'].side_effect = ( + lambda _block_type: None if _block_type == block_type else self.base_time + ) 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, + upload_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( + BlockType.SOFT_BLOCKED, filter_list=[BlockType.SOFT_BLOCKED.name] + ) + + 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): """ @@ -153,32 +205,61 @@ def test_missing_last_filter_uses_base_filter(self): ) ) in self.mocks['olympia.blocklist.cron.upload_filter.delay'].call_args_list + @override_switch('enable-soft-blocking', active=True) def test_skip_update_if_unsigned_blocks_added(self): """ skip update if there are only unsigned new blocks """ - self._block_version(is_signed=False) + 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 - def test_skip_update_unless_new_blocks(self): + def _test_skip_update_unless_new_blocks( + self, block_type: BlockType, enable_soft_blocking=False, expect_update=False + ): """ skip update unless there are new blocks """ - upload_mlbf_to_remote_settings(force_base=False) - assert not 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=False) + assert not self.mocks['olympia.blocklist.cron.upload_filter.delay'].called - # Now there is a new blocked version - self._block_version(is_signed=True) - upload_mlbf_to_remote_settings(force_base=False) - assert self.mocks['olympia.blocklist.cron.upload_filter.delay'].called + # Now there is a new blocked version + self._block_version(block_type=block_type, is_signed=True) + upload_mlbf_to_remote_settings(force_base=False) + + 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, + expect_update=True, + ) 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() @@ -212,49 +293,148 @@ def test_skip_upload_if_switch_is_disabled(self): 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. """ - self._block_version(is_signed=True) - upload_mlbf_to_remote_settings(force_base=False) - assert self.mocks[ - 'olympia.blocklist.cron.upload_filter.delay' - ].call_args_list == [ - mock.call( - self.current_time, - filter_list=[], - upload_stash=True, - ) - ] + for block_type in block_types: + self._block_version(block_type=block_type) - mlbf = MLBF.load_from_storage(self.current_time) - assert not mlbf.storage.exists(mlbf.filter_path(BlockType.BLOCKED)) - assert mlbf.storage.exists(mlbf.stash_path) + with override_switch('enable-soft-blocking', active=enable_soft_blocking): + upload_mlbf_to_remote_settings(force_base=False) - upload_mlbf_to_remote_settings(force_base=True) - assert mlbf.storage.exists(mlbf.filter_path(BlockType.BLOCKED)) - assert ( - mock.call( - self.current_time, - filter_list=[BlockType.BLOCKED.name], - upload_stash=False, + self.assertEqual( + expect_stash, + mock.call( + self.current_time, + filter_list=[], + upload_stash=True, + ) in self.mocks['olympia.blocklist.cron.upload_filter.delay'].call_args_list ) - ) in self.mocks['olympia.blocklist.cron.upload_filter.delay'].call_args_list - with override_switch('enable-soft-blocking', active=True): + mlbf = MLBF.load_from_storage(self.current_time) + + if expect_stash: + for block_type in BlockType: + assert not mlbf.storage.exists(mlbf.filter_path(block_type)) + assert mlbf.storage.exists(mlbf.stash_path) + else: + assert mlbf is None + upload_mlbf_to_remote_settings(force_base=True) - assert mlbf.storage.exists(mlbf.filter_path(BlockType.SOFT_BLOCKED)) - assert ( - mock.call( - self.current_time, - filter_list=[BlockType.BLOCKED.name, BlockType.SOFT_BLOCKED.name], - upload_stash=False, - ) + 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, + upload_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, + filter_list=[BlockType.BLOCKED, BlockType.SOFT_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], + expect_stash=True, + filter_list=[BlockType.BLOCKED, BlockType.SOFT_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 is 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 and soft blocked + filters 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, BlockType.SOFT_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, + filter_list=[BlockType.BLOCKED, BlockType.SOFT_BLOCKED], + enable_soft_blocking=True, + ) + def test_upload_stash_unless_missing_base_filter(self): """ Upload a stash unless there is no base filter. @@ -277,7 +457,7 @@ def test_upload_stash_unless_missing_base_filter(self): self.mocks[ 'olympia.blocklist.cron.get_base_generation_time' - ].return_value = None + ].side_effect = lambda _block_type: None upload_mlbf_to_remote_settings() assert ( mock.call( @@ -387,26 +567,55 @@ def test_creates_base_filter_if_base_generation_time_invalid(self): 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, - raise no filter found. + When no previous filter is found, compare blocks against the base filter + of that block type. """ - self.mocks['olympia.blocklist.cron.get_last_generation_time'].return_value = 1 - upload_mlbf_to_remote_settings(force_base=True) - assert self.mocks['olympia.blocklist.cron.upload_filter.delay'].called + # Hard block version is accounted for in the base filter + self._block_version(block_type=BlockType.BLOCKED) + 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 + + # now with softblocking enabled we can account for the soft blocked version + with override_switch('enable-soft-blocking', active=True): + upload_mlbf_to_remote_settings(force_base=False) + assert self.mocks['olympia.blocklist.cron.upload_filter.delay'].called - def test_dont_skip_update_if_all_blocked_or_not_blocked(self): + @override_switch('enable-soft-blocking', active=True) + def _test_dont_skip_update_if_all_blocked_or_not_blocked(self, block_type: BlockType): """ If all versions are either blocked or not blocked, skip the update. """ - version = self._block_version(is_signed=True) - upload_mlbf_to_remote_settings(force_base=True) - assert self.mocks['olympia.blocklist.cron.upload_filter.delay'].called - version.update(block_type=BlockType.SOFT_BLOCKED) - upload_mlbf_to_remote_settings(force_base=True) + for _ in range(0, 10): + self._block_version(block_type=block_type) + + upload_mlbf_to_remote_settings() assert self.mocks['olympia.blocklist.cron.upload_filter.delay'].called + 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 + ) + + def test_dont_skip_update_if_all_blocked_or_not_blocked_for_blocked(self): + self._test_dont_skip_update_if_all_blocked_or_not_blocked( + block_type=BlockType.BLOCKED + ) + def test_invalid_cache_results_in_diff(self): self._block_version(block_type=BlockType.BLOCKED)