diff --git a/bibtexparser/library.py b/bibtexparser/library.py index 296b7fc..78f4e7c 100644 --- a/bibtexparser/library.py +++ b/bibtexparser/library.py @@ -11,6 +11,7 @@ String, ) + # TODO Use functools.lru_cache for library properties (which create lists when called) @@ -57,7 +58,7 @@ def remove(self, blocks: Union[List[Block], Block]): del self._strings_by_key[block.key] def replace( - self, old_block: Block, new_block: Block, fail_on_duplicate_key: bool = True + self, old_block: Block, new_block: Block, fail_on_duplicate_key: bool = True ): """Replace a block with another block, at the same position. @@ -73,24 +74,24 @@ 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 fail_on_duplicate_key + new_block is not block_after_add + 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 def _cast_to_duplicate( - prev_block_with_same_key: Union[Entry, String], duplicate: Union[Entry, String] + prev_block_with_same_key: Union[Entry, String], duplicate: Union[Entry, String] ): assert isinstance(prev_block_with_same_key, type(duplicate)) or isinstance( duplicate, type(prev_block_with_same_key) @@ -102,7 +103,7 @@ def _cast_to_duplicate( ) assert ( - prev_block_with_same_key.key == duplicate.key + prev_block_with_same_key.key == duplicate.key ), "Internal BibtexParser Error. Duplicate blocks have different keys." return DuplicateBlockKeyBlock( @@ -160,7 +161,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..8f2d946 --- /dev/null +++ b/tests/test_library.py @@ -0,0 +1,56 @@ +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"