From 32072e0a0b1981299253311e6de5fb7c7315267b Mon Sep 17 00:00:00 2001 From: Kevin Meinhardt Date: Wed, 13 Nov 2024 15:33:03 +0100 Subject: [PATCH] Fallback to empty list when block type is not present (#22846) * Revert "Revert "Fallback to empty list when block type is not present on cache.json"" This reverts commit 2bd0783eb5218008b7420eb86c49b8a043de5c3e. * Add regression test + comment * Format * Validate diff adds and omits from corupted cache. --- src/olympia/blocklist/mlbf.py | 6 ++-- src/olympia/blocklist/tests/test_mlbf.py | 42 ++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/olympia/blocklist/mlbf.py b/src/olympia/blocklist/mlbf.py index ce3a9f30bda5..43eede4a427e 100644 --- a/src/olympia/blocklist/mlbf.py +++ b/src/olympia/blocklist/mlbf.py @@ -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): diff --git a/src/olympia/blocklist/tests/test_mlbf.py b/src/olympia/blocklist/tests/test_mlbf.py index d61067ee7fcd..d087b9b011a3 100644 --- a/src/olympia/blocklist/tests/test_mlbf.py +++ b/src/olympia/blocklist/tests/test_mlbf.py @@ -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): @@ -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 = [