diff --git a/bibtexparser/library.py b/bibtexparser/library.py index 296b7fc..db01748 100644 --- a/bibtexparser/library.py +++ b/bibtexparser/library.py @@ -73,19 +73,19 @@ def replace( except ValueError: raise ValueError("Block to replace is not in library.") - self._blocks.insert(index, new_block) block_after_add = self._add_to_dicts(new_block) + self._blocks.insert(index, block_after_add) if ( new_block is not block_after_add - and isinstance(new_block, DuplicateBlockKeyBlock) + and isinstance(block_after_add, DuplicateBlockKeyBlock) and fail_on_duplicate_key ): # Revert changes to old_block # Don't fail on duplicate key, as this would lead to an infinite recursion # (should never happen for a clean library, but could happen if the user # tampered with the internals of the library). - self.replace(new_block, old_block, fail_on_duplicate_key=False) + self.replace(block_after_add, old_block, fail_on_duplicate_key=False) raise ValueError("Duplicate key found.") @staticmethod @@ -160,7 +160,9 @@ def strings_dict(self) -> Dict[str, String]: @property def entries(self) -> List[Entry]: """All entry (@article, ...) blocks in the library, preserving order of insertion.""" - return list(self._entries_by_key.values()) + # Note: Taking this from the entries dict would be faster, but does not preserve order + # e.g. in cases where `replace` has been called. + return [b for b in self._blocks if isinstance(b, Entry)] @property def entries_dict(self) -> Dict[str, Entry]: diff --git a/tests/test_library.py b/tests/test_library.py new file mode 100644 index 0000000..239f76f --- /dev/null +++ b/tests/test_library.py @@ -0,0 +1,66 @@ +from copy import deepcopy + +import pytest + +from bibtexparser import Library +from bibtexparser.model import Entry, Field + + +def get_dummy_entry(): + return Entry( + entry_type="article", + key="duplicateKey", + fields=[ + Field(key="title", value="A title"), + Field(key="author", value="An author"), + ], + ) + + +def test_replace_with_duplicates(): + """Test that replace() works when there are duplicate values. See issue 404.""" + library = Library() + library.add(get_dummy_entry()) + library.add(get_dummy_entry()) + # Test precondition + assert len(library.blocks) == 2 + assert len(library.failed_blocks) == 1 + + replacement_entry = get_dummy_entry() + replacement_entry.fields_dict["title"].value = "A new title" + + library.replace( + library.failed_blocks[0], replacement_entry, fail_on_duplicate_key=False + ) + assert len(library.blocks) == 2 + assert len(library.failed_blocks) == 1 + assert library.failed_blocks[0].ignore_error_block["title"] == "A new title" + + replacement_entry_2 = get_dummy_entry() + replacement_entry_2.fields_dict["title"].value = "Another new title" + + library.replace( + library.entries[0], replacement_entry_2, fail_on_duplicate_key=False + ) + assert len(library.blocks) == 2 + assert len(library.failed_blocks) == 1 + # The new block replaces the previous "non-duplicate" and should thus not become a duplicate itself + assert library.entries[0].fields_dict["title"].value == "Another new title" + + +def test_replace_fail_on_duplicate(): + library = Library() + replaceable_entry = get_dummy_entry() + replaceable_entry.key = "Just a regular entry, to be replaced" + future_duplicate_entry = get_dummy_entry() + library.add([replaceable_entry, future_duplicate_entry]) + + with pytest.raises(ValueError): + library.replace( + replaceable_entry, get_dummy_entry(), fail_on_duplicate_key=True + ) + + assert len(library.blocks) == 2 + assert len(library.failed_blocks) == 0 + assert library.entries[0].key == "Just a regular entry, to be replaced" + assert library.entries[1].key == "duplicateKey"