diff --git a/.changelog/5150.yml b/.changelog/5150.yml new file mode 100644 index 00000000000..38a051bdeab --- /dev/null +++ b/.changelog/5150.yml @@ -0,0 +1,9 @@ +changes: +- description: | + Fixed id_set generation crash when processing lists with separate metadata + and data files. Lists can store metadata (with 'id' field) and data in + separate files. The fix skips data-only files that lack the 'id' field, + preventing AttributeError during sorting while correctly including list + metadata files in the id_set. + type: fix +pr_number: 5150 diff --git a/demisto_sdk/commands/common/tests/update_id_set_test.py b/demisto_sdk/commands/common/tests/update_id_set_test.py index 16fc52d7d28..c833220ae9a 100644 --- a/demisto_sdk/commands/common/tests/update_id_set_test.py +++ b/demisto_sdk/commands/common/tests/update_id_set_test.py @@ -36,6 +36,7 @@ get_indicator_type_data, get_layout_data, get_layout_rule_data, + get_list_data, get_mapper_data, get_modeling_rule_data, get_pack_metadata_data, @@ -4151,3 +4152,71 @@ def test_add_item_to_exclusion_dict(): add_item_to_exclusion_dict(excluded_items_from_id_set, file_path, "Cortex XDR") assert IsEqualFunctions.is_dicts_equal(expected_result, excluded_items_from_id_set) + + +def test_get_list_data_with_separate_metadata_and_data_files(): + """ + Given: + - A unified list file with 'id' field + - A list metadata file with 'id' field + - A list data file without 'id' field + + When: + - Calling get_list_data() on each file + + Then: + - Unified list returns dict with id and metadata + - List metadata file returns dict with id and metadata + - List data file returns empty dict (skipped as it has no 'id') + + This test ensures that list data files (which contain only data, no metadata) + are correctly skipped during id_set generation, preventing errors. + """ + + # Test data + unified_list_data = { + "id": "test-list-unified", + "name": "Test List Unified", + "type": "plain_text", + "data": "item1\nitem2\nitem3" + } + + list_metadata_data = { + "id": "test-list-with-separate-data", + "name": "Test List With Separate Data", + "type": "plain_text", + "data": "-" # Placeholder when data is in separate file + } + + # Data file - contains only data, no 'id' field + list_data_file_content = { + "data": ["item1", "item2", "item3"] + } + + # Create temp files for testing + with tempfile.TemporaryDirectory() as temp_dir: + # Test unified list + unified_list_path = os.path.join(temp_dir, "UnifiedList.json") + with open(unified_list_path, "w") as f: + json.dump(unified_list_data, f) + + unified_result = get_list_data(unified_list_path) + assert "test-list-unified" in unified_result + assert unified_result["test-list-unified"]["name"] == "Test List Unified" + + # Test list metadata file (has 'id') + metadata_path = os.path.join(temp_dir, "ListWithSeparateData.json") + with open(metadata_path, "w") as f: + json.dump(list_metadata_data, f) + + metadata_result = get_list_data(metadata_path) + assert "test-list-with-separate-data" in metadata_result + assert metadata_result["test-list-with-separate-data"]["name"] == "Test List With Separate Data" + + # Test list data file (no 'id' - should be skipped) + data_path = os.path.join(temp_dir, "ListWithSeparateData_data.json") + with open(data_path, "w") as f: + json.dump(list_data_file_content, f) + + data_result = get_list_data(data_path) + assert data_result == {} # Should return empty dict for files without 'id' diff --git a/demisto_sdk/commands/common/update_id_set.py b/demisto_sdk/commands/common/update_id_set.py index 782c9de0913..13cf8ffad9d 100755 --- a/demisto_sdk/commands/common/update_id_set.py +++ b/demisto_sdk/commands/common/update_id_set.py @@ -2170,7 +2170,10 @@ def process_general_items( return [], excluded_items_from_id_set if print_logs: logger.info(f"adding {file_path} to id_set") - res.append(data_extraction_func(file_path, packs=packs)) + data = data_extraction_func(file_path, packs=packs) + # Only add non-empty results (e.g., skip list data files without 'id') + if data: + res.append(data) else: package_name = Path(file_path).name file_path = os.path.join(file_path, f"{package_name}.{suffix}") @@ -2187,7 +2190,10 @@ def process_general_items( return [], excluded_items_from_id_set if print_logs: logger.info(f"adding {file_path} to id_set") - res.append(data_extraction_func(file_path, packs=packs)) + data = data_extraction_func(file_path, packs=packs) + # Only add non-empty results (e.g., skip list data files without 'id') + if data: + res.append(data) except Exception as exp: logger.info(f"failed to process {file_path}, Error: {str(exp)}") raise @@ -2457,6 +2463,17 @@ def get_generic_module_data(path, packs: Dict[str, Dict] = None): def get_list_data(path: str, packs: Dict[str, Dict] = None): json_data = get_json(path) + + # Skip list data files (e.g., ListName_data.json) without 'id' field. + # Lists can store metadata and data separately: + # - ListName.json: metadata file with 'id' field + # - ListName_data.{txt|json|etc}: data-only file + # For id_set generation, we only need the metadata file. + list_id = json_data.get("id") + if not list_id: + logger.debug(f"Skipping file without 'id' field (likely list data file): {path}") + return {} + marketplaces = get_item_marketplaces(path, item_data=json_data, packs=packs) data = create_common_entity_data( path=path, @@ -2468,7 +2485,7 @@ def get_list_data(path: str, packs: Dict[str, Dict] = None): marketplaces=marketplaces, ) - return {json_data.get("id"): data} + return {list_id: data} def get_wizard_data(path: str, packs: Dict[str, Dict] = None): diff --git a/demisto_sdk/commands/create_id_set/tests/create_id_set_test.py b/demisto_sdk/commands/create_id_set/tests/create_id_set_test.py index e7ad7f98498..c22577317a9 100644 --- a/demisto_sdk/commands/create_id_set/tests/create_id_set_test.py +++ b/demisto_sdk/commands/create_id_set/tests/create_id_set_test.py @@ -588,3 +588,79 @@ def test_generic_command_that_uses_a_specific_brand(repo): playbook = id_set_creator.id_set["playbooks"][0]["Playbook"] assert playbook["command_to_integration"]["send-notification"] == "Slack" + + +def test_create_id_set_with_lists_having_separate_data_files(repo): + """ + Given: + - A pack with unified lists and lists with separate metadata/data files + + When: + - Creating an id_set for the pack + + Then: + - Ensure both unified lists and list metadata files are in id_set + - Ensure list data files (without 'id') are skipped + - Ensure the id_set contains the correct list IDs + """ + pack = repo.create_pack("TestListPack") + pack.pack_metadata.write_json(METADATA) + + lists_dir = pack._lists_path + lists_dir.mkdir(exist_ok=True, parents=True) + + # Create unified list + unified_list_data = { + "id": "unified-test-list", + "name": "Unified Test List", + "type": "plain_text", + "data": "item1\nitem2\nitem3", + "fromVersion": "6.10.0", + } + unified_list_path = lists_dir / "UnifiedList.json" + with open(unified_list_path, "w") as f: + json.dump(unified_list_data, f) + + # Create list metadata file (data in separate file) + list_metadata_data = { + "id": "list-with-separate-data", + "name": "List With Separate Data", + "type": "plain_text", + "data": "-", # Placeholder when data is in separate file + "fromVersion": "6.10.0", + } + list_metadata_path = lists_dir / "ListWithSeparateData.json" + with open(list_metadata_path, "w") as f: + json.dump(list_metadata_data, f) + + # Create list data file (should be skipped - no 'id' field) + list_data_file = { + "data": ["data-item1", "data-item2", "data-item3"] + } + list_data_path = lists_dir / "ListWithSeparateData_data.json" + with open(list_data_path, "w") as f: + json.dump(list_data_file, f) + + # Create id_set + id_set_creator = IDSetCreator(output=None, input=str(pack.path)) + id_set, _, _ = id_set_creator.create_id_set() + + # Verify unified list is in id_set + assert "Lists" in id_set + list_ids = [list(item.keys())[0] for item in id_set["Lists"]] + assert "unified-test-list" in list_ids, "Unified list should be in id_set" + + # Verify list metadata file is in id_set + assert "list-with-separate-data" in list_ids, "List metadata should be in id_set" + + # Verify exactly 2 lists (not 3 - data file should be skipped) + assert len(id_set["Lists"]) == 2, "Should have 2 lists, data file skipped" + + # Verify the list entries have correct names + unified_entry = next((item for item in id_set["Lists"] if "unified-test-list" in item), None) + assert unified_entry is not None + assert unified_entry["unified-test-list"]["name"] == "Unified Test List" + + metadata_entry = next((item for item in id_set["Lists"] if "list-with-separate-data" in item), None) + assert metadata_entry is not None + assert metadata_entry["list-with-separate-data"]["name"] == "List With Separate Data"