fix: add filter to dependency datasource (#3949)

- when add_filter to component (parser or combiner),
  add the filters to the datasource on which the component
  directly depends
- add relevant tests

Signed-off-by: Xiangce Liu <xiangceliu@redhat.com>
(cherry picked from commit 26abad6)

insights/core/filters.py

@@ -66,9 +66,9 @@ def add_filter(component, patterns):
        patterns (str, [str]): A string, list of strings, or set of strings to
             add to the datasource's filters.
     """
-    def inner(component, patterns):
-        if component in _CACHE:
-            del _CACHE[component]
+    def inner(comp, patterns):
+        if comp in _CACHE:
+            del _CACHE[comp]
 
         types = six.string_types + (list, set)
         if not isinstance(patterns, types):
@@ -81,18 +81,25 @@ def inner(component, patterns):
 
         for pat in patterns:
             if not pat:
-                raise Exception("Filter patterns must not be empy.")
+                raise Exception("Filter patterns must not be empty.")
 
-        FILTERS[component] |= patterns
+        FILTERS[comp] |= patterns
+
+    def get_dependency_datasources(comp):
+        """"Get (all) the first depended datasource"""
+        dep_ds = set()
+        if plugins.is_datasource(comp):
+            dep_ds.add(comp)
+            return dep_ds
+        for dep in dr.get_dependencies(comp):
+            dep_ds.update(get_dependency_datasources(dep))
+        return dep_ds
 
     if not plugins.is_datasource(component):
-        deps = dr.run_order(dr.get_dependency_graph(component))
+        deps = get_dependency_datasources(component)
         if deps:
-            filterable_deps = [
-                dep for dep in deps if (plugins.is_datasource(dep) and
-                                        dr.get_delegate(dep).filterable)
-            ]
-            # At least one dependency component be filterable
+            filterable_deps = [dep for dep in deps if dr.get_delegate(dep).filterable]
+            # At least one dependency datasource be filterable
             if not filterable_deps:
                 raise Exception("Filters aren't applicable to %s." % dr.get_name(component))
 

insights/tests/test_filters.py → insights/tests/core/test_filters.py

@@ -3,10 +3,10 @@
 
 from collections import defaultdict
 
-from insights import datasource
+from insights import parser
 from insights.combiners.hostname import Hostname
 from insights.core import filters
-from insights.core.spec_factory import DatasourceProvider, RegistryPoint, SpecSet
+from insights.core.spec_factory import RegistryPoint, SpecSet, simple_file
 from insights.parsers.ps import PsAux, PsAuxcww
 from insights.specs import Specs
 from insights.specs.default import DefaultSpecs
@@ -18,10 +18,27 @@ class MySpecs(SpecSet):
 
 
 class LocalSpecs(MySpecs):
-    # The has_filters depends on no_filters
-    @datasource(MySpecs.no_filters)
-    def has_filters(broker):
-        return DatasourceProvider("", "the_data")
+    no_filters = simple_file('no_filters')
+    has_filters = simple_file('has_filters')
+
+
+@parser(MySpecs.has_filters)
+class MySpecsHasFilters(object):
+    pass
+
+
+@parser(LocalSpecs.has_filters)
+class LocalSpecsHasFilters(object):
+    pass
+
+
+@parser(MySpecs.no_filters)
+class LocalSpecsNoFilters(object):
+    pass
+
+#
+# TEST
+#
 
 
 def setup_function(func):
@@ -37,6 +54,7 @@ def setup_function(func):
 
 
 def teardown_function(func):
+    filters._CACHE = {}
     if func is test_get_filter:
         del filters.FILTERS[Specs.ps_aux]
 
@@ -47,8 +65,16 @@ def teardown_function(func):
     if func is test_filter_dumps_loads:
         del filters.FILTERS[Specs.ps_aux]
 
-    if func is test_add_filter_to_parser:
+    if func in [
+        test_add_filter_to_MySpecsHasFilters,
+        test_add_filter_to_LocalSpecsHasFilters,
+    ]:
+        del filters.FILTERS[MySpecs.has_filters]
+        del filters.FILTERS[LocalSpecs.has_filters]
+
+    if func is test_add_filter_to_PsAux:
         del filters.FILTERS[Specs.ps_aux]
+        del filters.FILTERS[DefaultSpecs.ps_aux]
 
     if func is test_add_filter_to_parser_patterns_list:
         del filters.FILTERS[Specs.ps_aux]
@@ -87,12 +113,58 @@ def test_get_filter_registry_point():
     assert "MEM" not in f
 
 
-def test_add_filter_to_parser():
+# Local Parser
+def test_add_filter_to_MySpecsHasFilters():
+    """
+    "filters" added to MySpecs.x will also add to LocalSpecs.x
+    """
+    filter_string = "bash"
+
+    # Local Parser depends on MySpecs (Specs)
+    filters.add_filter(MySpecsHasFilters, filter_string)
+
+    myspecs_filters = filters.get_filters(MySpecs.has_filters)
+    assert filter_string in myspecs_filters
+    assert filter_string in filters.FILTERS[MySpecs.has_filters]
+
+    localspecs_filters = filters.get_filters(LocalSpecs.has_filters)
+    assert filter_string in localspecs_filters
+    # but there is no key in FILTERS for the LocalSpecs.x
+    assert filter_string not in filters.FILTERS[LocalSpecs.has_filters]
+
+
+def test_add_filter_to_LocalSpecsHasFilters():
+    """
+    "filters" added to LocalSpecs.x will NOT add to Specs.x
+    """
+    filter_string = "bash"
+    filters.add_filter(LocalSpecsHasFilters, filter_string)
+
+    myspecs_filters = filters.get_filters(MySpecs.has_filters)
+    assert filter_string not in myspecs_filters
+    assert filter_string not in filters.FILTERS[MySpecs.has_filters]
+
+    localspecs_filters = filters.get_filters(LocalSpecs.has_filters)
+    assert filter_string in localspecs_filters
+    assert filter_string in filters.FILTERS[LocalSpecs.has_filters]
+
+
+# General Parser
+def test_add_filter_to_PsAux():
+    """
+    "filters" added to Specs.x will add to DefaultSpecs.x
+    """
     filter_string = "bash"
     filters.add_filter(PsAux, filter_string)
 
     spec_filters = filters.get_filters(Specs.ps_aux)
     assert filter_string in spec_filters
+    assert filter_string in filters.FILTERS[Specs.ps_aux]
+
+    default_spec_filters = filters.get_filters(DefaultSpecs.ps_aux)
+    assert filter_string in default_spec_filters  # get_filters() works
+    # but there is no key in FILTERS for the LocalSpecs.x
+    assert filter_string not in filters.FILTERS[DefaultSpecs.ps_aux]  # empty in FILTERS
 
     parser_filters = filters.get_filters(PsAux)
     assert not parser_filters
@@ -109,7 +181,7 @@ def test_add_filter_to_parser_patterns_list():
     assert not parser_filters
 
 
-def test_add_filter_exception_not_filterable():
+def test_add_filter_exception_spec_not_filterable():
     with pytest.raises(Exception):
         filters.add_filter(Specs.ps_auxcww, "bash")
 
@@ -118,6 +190,9 @@ def test_add_filter_exception_parser_non_filterable():
     with pytest.raises(Exception):
         filters.add_filter(PsAuxcww, 'bash')
 
+    with pytest.raises(Exception):
+        filters.add_filter(LocalSpecsNoFilters, 'bash')
+
 
 def test_add_filter_exception_combiner_non_filterable():
     with pytest.raises(Exception):