Skip to content

Commit

Permalink
Fallback to empty list when block type is not present (#22846)
Browse files Browse the repository at this point in the history
* Revert "Revert "Fallback to empty list when block type is not present on cache.json""

This reverts commit 2bd0783.

* Add regression test + comment

* Format

* Validate diff adds and omits from corupted cache.
  • Loading branch information
KevinMind committed Nov 13, 2024
1 parent b3d0a22 commit 32072e0
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/olympia/blocklist/mlbf.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,15 @@ def __init__(self, storage: SafeStorage):

@cached_property
def blocked_items(self) -> List[str]:
return self._data.get(self.data_type_key(MLBFDataType.BLOCKED))
return self._data.get(self.data_type_key(MLBFDataType.BLOCKED), [])

@cached_property
def soft_blocked_items(self) -> List[str]:
return self._data.get(self.data_type_key(MLBFDataType.SOFT_BLOCKED))
return self._data.get(self.data_type_key(MLBFDataType.SOFT_BLOCKED), [])

@cached_property
def not_blocked_items(self) -> List[str]:
return self._data.get(self.data_type_key(MLBFDataType.NOT_BLOCKED))
return self._data.get(self.data_type_key(MLBFDataType.NOT_BLOCKED), [])


class MLBFDataBaseLoader(BaseMLBFLoader):
Expand Down
42 changes: 42 additions & 0 deletions src/olympia/blocklist/tests/test_mlbf.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,17 @@ def test_loads_data_from_file(self):
loader = MLBFStorageLoader(self.storage)
assert loader._raw == self._data

def test_fallback_to_empty_list_for_missing_key(self):
for key in self._data.keys():
new_data = self._data.copy()
new_data.pop(key)
# Generate a corrupted `cache.json` file
# (we do this for each key).
with self.storage.open('cache.json', 'w') as f:
json.dump(new_data, f)
loader = MLBFStorageLoader(self.storage)
assert loader._raw == {**new_data, key: []}


class TestMLBFDataBaseLoader(_MLBFBase):
def test_load_returns_expected_data(self):
Expand Down Expand Up @@ -452,6 +463,37 @@ def test_diff_block_soft_to_hard(self):
),
}

def test_diff_invalid_cache(self):
addon, block = self._blocked_addon(file_kw={'is_signed': True})
soft_blocked = self._block_version(
block, self._version(addon), block_type=BlockType.SOFT_BLOCKED
)
base = MLBF.generate_from_db()
# Overwrite the cache file removing the soft blocked version
with base.storage.open(base.data._cache_path, 'r+') as f:
data = json.load(f)
del data['soft_blocked']
f.seek(0)
json.dump(data, f)
f.truncate()

previous_mlbf = MLBF.load_from_storage(base.created_at)

mlbf = MLBF.generate_from_db()

# The diff should include the soft blocked version because it was removed
# and should not include the blocked version because it was not changed
assert mlbf.generate_diffs(previous_mlbf=previous_mlbf) == {
BlockType.BLOCKED: ([], [], 0),
BlockType.SOFT_BLOCKED: (
MLBF.hash_filter_inputs(
[(soft_blocked.block.guid, soft_blocked.version.version)]
),
[],
1,
),
}

def test_generate_stash_returns_expected_stash(self):
addon, block = self._blocked_addon()
block_versions = [
Expand Down

0 comments on commit 32072e0

Please sign in to comment.