Fix bug when overriding metadata

Author: gordonwatts

func_adl/ast/meta_data.py

@@ -59,6 +59,7 @@ def extract_metadata(a: ast.AST) -> Tuple[ast.AST, List[Dict[str, str]]]:
 
 def remove_empty_metadata(a: ast.AST) -> ast.AST:
     """Returns a new ast with any empty `MetaData` clauses removed.
+    The old ast is not modified.
 
     Args:
         a (ast.AST): The AST of the query to clean up.
@@ -75,7 +76,7 @@ def visit_Call(self, node: ast.Call):
                 if len(n.args) == 2:
                     d = ast.literal_eval(n.args[1])
                     if isinstance(d, dict) and len(d) == 0:
-                        return self.visit(n.args[0])
+                        return n.args[0]
             return n
 
     return _cleaner().visit(a)
@@ -103,10 +104,14 @@ def found(self) -> Optional[Any]:
 
         def generic_visit(self, node: ast.AST):
             q_metadata = getattr(node, "_q_metadata", None)
+            found = False
             if q_metadata is not None:
                 if metadata_name in q_metadata:
+                    found = True
                     self._found = q_metadata[metadata_name]
-            return super().generic_visit(node)
+
+            if not found:
+                super().generic_visit(node)
 
     ds_f = _finder()
     ds_f.visit(q.query_ast)

func_adl/object_stream.py

@@ -199,16 +199,20 @@ def QMetaData(self, metadata: Dict[str, Any]) -> ObjectStream[T]:
         base_ast = self.query_ast
         for k, v in metadata.items():
             found_md = lookup_query_metadata(self, k)
+            add_md = False
             if found_md is None:
+                add_md = True
+            elif found_md != v:
+                logging.getLogger(__name__).info(
+                    f'Overwriting metadata "{k}" from its old value of "{found_md}" to "{v}"'
+                )
+                add_md = True
+            if add_md:
                 if first:
                     first = False
                     base_ast = self.MetaData({}).query_ast
                     base_ast._q_metadata = {}  # type: ignore
                 base_ast._q_metadata[k] = v  # type: ignore
-            elif found_md != v:
-                logging.getLogger(__name__).warning(
-                    f'Overwriting metadata "{k}" from its old value of "{found_md}" to "{v}"'
-                )
 
         return ObjectStream[T](base_ast, self.item_type)
 

tests/ast/test_meta_data.py

@@ -88,6 +88,17 @@ def test_query_metadata_burried():
     assert lookup_query_metadata(r, "three") == "forks"
 
 
+def test_query_metadata_updated():
+    'This is testing code in QMetaData, but we need lookup_query_metadata which we are testing in this file'
+    r = (
+        my_event()
+        .QMetaData({"one": "two"})
+        .QMetaData({"one": "three"})
+    )
+
+    assert lookup_query_metadata(r, "one") == "three"
+
+
 def test_remove_empty_metadata_empty():
     r = remove_empty_metadata(my_event().MetaData({}).value())
     assert "MetaData" not in ast.dump(r)
@@ -96,3 +107,19 @@ def test_remove_empty_metadata_empty():
 def test_remove_empty_metadata_not_empty():
     r = remove_empty_metadata(my_event().MetaData({"hi": "there"}).value())
     assert "MetaData" in ast.dump(r)
+
+
+def test_remove_metadata_no_change():
+    "Removing metadata should not alter original query"
+    orig = my_event().MetaData({})
+    remove_empty_metadata(orig.query_ast)
+
+    assert "MetaData" in ast.dump(orig.query_ast)
+
+
+def test_remove_metadata_no_change_2_levels():
+    "Removing metadata should not alter original query"
+    orig = my_event().MetaData({}).MetaData({})
+    remove_empty_metadata(orig.query_ast)
+
+    assert "MetaData" in ast.dump(orig.query_ast)

tests/test_object_stream.py

@@ -1,6 +1,7 @@
 # Test the object stream
 import ast
 import asyncio
+import logging
 from typing import Any, Iterable, Optional, Tuple, TypeVar
 
 import pytest
@@ -173,6 +174,7 @@ def test_query_metadata_dup(caplog):
 
 
 def test_query_metadata_dup_update(caplog):
+    caplog.set_level(logging.INFO)
     r = (
         my_event()
         .QMetaData({"one": "two", "two": "three"})
@@ -189,6 +191,7 @@ def test_query_metadata_dup_update(caplog):
 
 
 def test_query_metadata_composable(caplog):
+    caplog.set_level(logging.INFO)
     r_base = my_event().QMetaData({"one": "1"})
 
     # Each of these is a different base and should not interfear.