Repository: sciunto-org/python-bibtexparser

Issue:
BlockMiddlewareTransformer does not handle `None` or collections
According to documentation, transformers implementing `BlockMiddlewareTransformer` can return `None`, a single entry, or a collection of entries for the `transform_block` function. But returning anything other than a single entry results in a crash. The problem seems to stem from this code in `middleware.py`:

```python3
    def transform(self, library: "Library") -> "Library":
        # TODO Multiprocessing (only for large library and if allow_multi..)
        blocks = [self.transform_block(b, library) for b in library.blocks]
        return Library(blocks=blocks)
```

It doesn't handle cases where `transform_block` does not return an entry.


Discussion:
mlutze: @MiWeiss Is there anything more needed before this can be merged?
MiWeiss: Please apologize my delay, I had not noticed the PR previously.  Will try to do a review within the next couple of days.

Thanks already for opening the PR :rocket: 
MiWeiss: Great,  thanks! 

I guess the three new test cases could be refactored into a single `@pytest.mark.parameterized` test function, thus keeping the test suite simpler to read and maintain. Or am I missing something?

Would you mind doing this refactoring? Otherwise I can take care of it. 

Afterwards,  this will be ready to be merged...
MiWeiss: Thanks a lot @mlutze for the PR. Nice work!

Patch:
diff --git a/bibtexparser/middlewares/middleware.py b/bibtexparser/middlewares/middleware.py
index d7e0d16..2329eb9 100644
--- a/bibtexparser/middlewares/middleware.py
+++ b/bibtexparser/middlewares/middleware.py
@@ -74,7 +74,29 @@ class BlockMiddleware(Middleware, abc.ABC):
     # docstr-coverage: inherited
     def transform(self, library: "Library") -> "Library":
         # TODO Multiprocessing (only for large library and if allow_multi..)
-        blocks = [self.transform_block(b, library) for b in library.blocks]
+        blocks = []
+        for b in library.blocks:
+            transformed = self.transform_block(b, library)
+            # Case 1: None. Skip it.
+            if transformed is None:
+                pass
+            # Case 2: A single block. Add it to the list.
+            elif isinstance(transformed, Block):
+                blocks.append(transformed)
+            # Case 3: A collection. Append all the elements.
+            elif isinstance(transformed, Collection):
+                # check that all the items are indeed blocks
+                for item in transformed:
+                    if not isinstance(item, Block):
+                        raise TypeError(
+                            f"Non-Block type found in transformed collection: {type(item)}"
+                        )
+                blocks.extend(transformed)
+            # Case 4: Something else. Error.
+            else:
+                raise TypeError(
+                    f"Illegal output type from transform_block: {type(transformed)}"
+                )
         return Library(blocks=blocks)
 
     def transform_block(


Test patch:
diff --git a/tests/middleware_tests/test_block_middleware.py b/tests/middleware_tests/test_block_middleware.py
new file mode 100644
index 0000000..eef9cce
--- /dev/null
+++ b/tests/middleware_tests/test_block_middleware.py
@@ -0,0 +1,115 @@
+import pytest
+
+from bibtexparser import Library
+from bibtexparser.middlewares.middleware import BlockMiddleware
+from bibtexparser.model import Entry, ExplicitComment, ImplicitComment, Preamble, String
+
+BLOCKS = [
+    ExplicitComment("explicit_comment_a"),
+    String("string_b", "value_b"),
+    String("string_a", "value_a"),
+    ImplicitComment("% implicit_comment_a"),
+    ExplicitComment("explicit_comment_b"),
+    Entry("article", "entry_a", fields=[]),
+    ImplicitComment("% implicit_comment_b"),
+    Entry("article", "entry_b", fields=[]),
+    Entry("article", "entry_d", fields=[]),
+    Entry("article", "entry_c", fields=[]),
+    Preamble("preamble_a"),
+    ImplicitComment("% implicit_comment_c"),
+]
+
+
+class ConstantBlockMiddleware(BlockMiddleware):
+    """A middleware that always returns the same result for every block."""
+
+    def __init__(self, const):
+        self._const = const
+        super().__init__(allow_parallel_execution=True, allow_inplace_modification=True)
+
+    def transform_block(self, block, library):
+        return self._const
+
+    def metadata_key():
+        return "ConstantBlockMiddleware"
+
+
+class LambdaBlockMiddleware(BlockMiddleware):
+    """A middleware that applies a lambda to the input block"""
+
+    def __init__(self, f):
+        self._f = f
+        super().__init__(allow_parallel_execution=True, allow_inplace_modification=True)
+
+    def transform_block(self, block, library):
+        return self._f(block)
+
+    def metadata_key():
+        return "LambdaBlockMiddleware"
+
+
+@pytest.mark.parametrize(
+    ("middleware", "expected"),
+    [
+        (ConstantBlockMiddleware(None), []),
+        (ConstantBlockMiddleware([]), []),
+        (LambdaBlockMiddleware(lambda b: [b]), BLOCKS),
+    ],
+)
+def test_successful_transform(middleware, expected):
+    library = Library(blocks=BLOCKS)
+    library = middleware.transform(library)
+
+    assert library.blocks == expected
+
+
+def test_returning_list_adds_all():
+    library = Library(blocks=BLOCKS)
+    library = LambdaBlockMiddleware(
+        lambda b: [ImplicitComment("% Block"), b]
+    ).transform(library)
+
+    expected = [
+        ImplicitComment("% Block"),
+        ExplicitComment("explicit_comment_a"),
+        ImplicitComment("% Block"),
+        String("string_b", "value_b"),
+        ImplicitComment("% Block"),
+        String("string_a", "value_a"),
+        ImplicitComment("% Block"),
+        ImplicitComment("% implicit_comment_a"),
+        ImplicitComment("% Block"),
+        ExplicitComment("explicit_comment_b"),
+        ImplicitComment("% Block"),
+        Entry("article", "entry_a", fields=[]),
+        ImplicitComment("% Block"),
+        ImplicitComment("% implicit_comment_b"),
+        ImplicitComment("% Block"),
+        Entry("article", "entry_b", fields=[]),
+        ImplicitComment("% Block"),
+        Entry("article", "entry_d", fields=[]),
+        ImplicitComment("% Block"),
+        Entry("article", "entry_c", fields=[]),
+        ImplicitComment("% Block"),
+        Preamble("preamble_a"),
+        ImplicitComment("% Block"),
+        ImplicitComment("% implicit_comment_c"),
+    ]
+
+    assert library.blocks == expected
+
+
+@pytest.mark.parametrize(
+    "middleware",
+    [
+        ConstantBlockMiddleware(True),
+        ConstantBlockMiddleware([True]),
+        LambdaBlockMiddleware(
+            lambda block: (b for b in [block])
+        ),  # generators are not collections
+    ],
+)
+def test_returning_invalid_raises_error(middleware):
+    library = Library(blocks=BLOCKS)
+    with pytest.raises(TypeError):
+        middleware.transform(library)