Skip to content

Commit

Permalink
Remove Waffle Switch for Soft Blocking and Simplify MLBF Logic
Browse files Browse the repository at this point in the history
This commit removes the Waffle switch for soft blocking and updates the MLBF (Machine Learning Bloom Filter) logic to always process soft-blocked items. Key changes include:

- Removed all references to the 'enable-soft-blocking' Waffle switch
- Simplified test cases to remove conditional logic based on the switch
- Updated MLBF generation and stash creation to always include soft-blocked items
- Removed conditional checks in cron and MLBF processing that were dependent on the switch

The changes streamline the blocklist filter generation process and remove unnecessary complexity around soft blocking configuration.
  • Loading branch information
KevinMind committed Jan 31, 2025
1 parent 015afb7 commit f7bdada
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 177 deletions.
10 changes: 0 additions & 10 deletions src/olympia/blocklist/cron.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 7 additions & 9 deletions src/olympia/blocklist/mlbf.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

from django.utils.functional import cached_property

import waffle
from filtercascade import FilterCascade
from filtercascade.fileformats import HashAlgorithm

Expand Down Expand Up @@ -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] = [
Expand Down
67 changes: 22 additions & 45 deletions src/olympia/blocklist/tests/test_cron.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand All @@ -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):
"""
Expand All @@ -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):
"""
Expand All @@ -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):
"""
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit f7bdada

Please sign in to comment.