Skip to content

Commit

Permalink
Compare to base filter when determining stash
Browse files Browse the repository at this point in the history
  • Loading branch information
KevinMind committed Nov 26, 2024
1 parent a2e3284 commit 7cfa66a
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/olympia/blocklist/cron.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def _upload_mlbf_to_remote_settings(*, force_base=False):
)

if create_stash:
mlbf.generate_and_write_stash(previous_filter)
mlbf.generate_and_write_stash(previous_filter, base_filter)

for block_type in base_filters_to_update:
mlbf.generate_and_write_filter(block_type)
Expand Down
8 changes: 5 additions & 3 deletions src/olympia/blocklist/mlbf.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,9 @@ def generate_diffs(
for block_type in BlockType
}

def generate_and_write_stash(self, previous_mlbf: 'MLBF' = None):
def generate_and_write_stash(
self, previous_mlbf: 'MLBF' = None, base_mlbf: 'MLBF' = None
):
"""
Generate and write the stash file representing changes between the
previous and current bloom filters. See:
Expand Down Expand Up @@ -313,14 +315,14 @@ def generate_and_write_stash(self, previous_mlbf: 'MLBF' = None):
blocked_added, blocked_removed, _ = diffs[BlockType.BLOCKED]
added_items = set(blocked_added)

if not self.should_upload_filter(BlockType.BLOCKED, previous_mlbf):
if not self.should_upload_filter(BlockType.BLOCKED, base_mlbf):
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, previous_mlbf):
if not self.should_upload_filter(BlockType.SOFT_BLOCKED, base_mlbf):
stash_json[STASH_KEYS[BlockType.SOFT_BLOCKED]] = soft_blocked_added
stash_json[UNBLOCKED_STASH_KEY].extend(soft_blocked_removed)

Expand Down
66 changes: 64 additions & 2 deletions src/olympia/blocklist/tests/test_mlbf.py
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,68 @@ def test_hard_to_soft_multiple(self):
'unblocked': [],
}

@mock.patch('olympia.blocklist.mlbf.BASE_REPLACE_THRESHOLD', 1)
def test_stash_is_empty_if_uploading_new_filter(self):
mlbf = MLBF.generate_from_db('test')

# No changes yet so no new filter and empty stash
assert not mlbf.should_upload_filter(BlockType.BLOCKED)
assert mlbf.generate_and_write_stash() == {
'blocked': [],
'softblocked': [],
'unblocked': [],
}

# One of each version produces a stash
addon, block = self._blocked_addon()
hard_block = self._block_version(block, self._version(addon), block_type=BlockType.BLOCKED)
soft_block = self._block_version(
block, self._version(addon), block_type=BlockType.SOFT_BLOCKED
)
hard_stash, soft_stash = MLBF.hash_filter_inputs([
(hard_block.block.guid, hard_block.version.version),
(soft_block.block.guid, soft_block.version.version),
])

next_mlbf = MLBF.generate_from_db('test_two')
# 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': [],
'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')

# When comparing to the base filter, the stash is empty
assert final_mlbf.generate_and_write_stash(previous_mlbf=next_mlbf, base_mlbf=mlbf) == {
'blocked': [],
'softblocked': [],
'unblocked': [],
}

# When comparing to the previous mlbf, the stash includes the hard blocked version
assert final_mlbf.generate_and_write_stash(
previous_mlbf=next_mlbf, base_mlbf=next_mlbf
) == {
'blocked': [soft_stash],
'softblocked': [],
'unblocked': [],
}

def test_diff_invalid_cache(self):
addon, block = self._blocked_addon(file_kw={'is_signed': True})
soft_blocked = self._block_version(
Expand Down Expand Up @@ -826,7 +888,7 @@ def test_generate_empty_stash_when_all_items_in_filter(self):

# If soft blocking is disabled, then softening a block is treated
# as an unblock.
assert mlbf.generate_and_write_stash(base_mlbf) == {
assert mlbf.generate_and_write_stash(base_mlbf, base_mlbf) == {
'blocked': [],
'softblocked': [],
'unblocked': [hard_block_hash],
Expand All @@ -849,7 +911,7 @@ def test_generate_empty_stash_when_all_items_in_filter(self):
# 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(base_mlbf) == {
assert mlbf.generate_and_write_stash(base_mlbf, base_mlbf) == {
'blocked': [],
'softblocked': [],
'unblocked': [],
Expand Down

0 comments on commit 7cfa66a

Please sign in to comment.