[core][fix] Simplify and fix owner lookup (#2128)

Author: aquamatthias

fixcore/fixcore/db/arango_query_rewrite.py

@@ -37,8 +37,10 @@ def combine_term_if_possible(term: Term, predicates: List[Predicate]) -> Term:
                 for res in query_model.owners(pred.name):
                     if res.fqn not in predefined_kinds_by_name:
                         kinds.add(res.fqn)
-        kinds.discard("resource")  # all resources have this base kind - ignore it
-        return IsTerm(kinds=sorted(kinds)).and_term(term) if kinds else term
+        if not kinds or "resource" in kinds:  # all resources have this base kind - ignore it
+            return term
+        else:
+            return IsTerm(kinds=sorted(kinds)).and_term(term)
 
     def change_term(term: Term) -> Term:
         if isinstance(term, CombinedTerm) and term.op == "or":

fixcore/fixcore/model/model.py

@@ -181,6 +181,10 @@ def __init__(self, path: Sequence[Optional[str]], str_rep: Optional[str] = None)
     def root(self) -> bool:
         return not bool(self.path)
 
+    @property
+    def single(self) -> bool:
+        return len(self.path) == 1
+
     def child(self, part: Optional[str]) -> PropertyPath:
         update = list(self.path)
         update.append(part)
@@ -189,6 +193,10 @@ def child(self, part: Optional[str]) -> PropertyPath:
     def unescaped_parts(self) -> List[str]:
         return [p.rstrip("[]").strip("`") for p in self.path if p is not None]
 
+    @property
+    def first_part(self) -> Optional[str]:
+        return self.path[0] if self.path else None
+
     @property
     def last_part(self) -> Optional[str]:
         return self.path[-1] if self.path else None
@@ -926,9 +934,8 @@ def __init__(
         self.__resolved_props: Dict[str, Tuple[Property, Kind]] = {}
         self.__resolved_direct_props: Dict[str, Tuple[Property, Kind]] = {}
         self.__resolved_bases: Dict[str, ComplexKind] = {}
-        self.__owner_lookup: Dict[PropertyPath, ComplexKind] = {}
         self.__all_props: List[Property] = list(self.properties)
-        self.__resolved_hierarchy: Set[str] = {fqn}
+        self.__resolved_hierarchy: Dict[str, ComplexKind] = {fqn: self}
         self.__property_by_path: List[ResolvedPropertyPath] = []
         self.__synthetic_props: List[ResolvedPropertyPath] = []
 
@@ -999,7 +1006,7 @@ def resolve(self, model: Dict[str, Kind]) -> None:
                         self.__resolved_hierarchy.update(base.__resolved_hierarchy)
 
             # property path -> kind
-            self.__property_by_path, self.__owner_lookup = ComplexKind.resolve_properties(self, model)
+            self.__property_by_path = ComplexKind.resolve_properties(self, model)
             self.__synthetic_props = [p for p in self.__property_by_path if p.prop.synthetic]
 
             # resolve predecessor kinds
@@ -1054,6 +1061,9 @@ def is_root(self) -> bool:
         return not self.bases or (len(self.bases) == 1 and self.bases[0] == self.fqn)
 
     def kind_hierarchy(self) -> Set[str]:
+        return set(self.__resolved_hierarchy.keys())
+
+    def resolved_hierarchy(self) -> Dict[str, ComplexKind]:
         return self.__resolved_hierarchy
 
     def is_a(self, kind: str) -> bool:
@@ -1062,9 +1072,6 @@ def is_a(self, kind: str) -> bool:
     def resolved_property_paths(self) -> List[ResolvedPropertyPath]:
         return self.__property_by_path
 
-    def owned_paths(self) -> Dict[PropertyPath, ComplexKind]:
-        return self.__owner_lookup
-
     def resolved_bases(self) -> Dict[str, ComplexKind]:
         return self.__resolved_bases
 
@@ -1257,14 +1264,10 @@ def walk_element(
         return walk_element(elem, self, initial_level, overrides=overrides)
 
     @staticmethod
-    def resolve_properties(
-        complex_kind: ComplexKind, model: Dict[str, Kind]
-    ) -> Tuple[List[ResolvedPropertyPath], Dict[PropertyPath, ComplexKind]]:
+    def resolve_properties(complex_kind: ComplexKind, model: Dict[str, Kind]) -> List[ResolvedPropertyPath]:
         result: List[ResolvedPropertyPath] = []
-        owner_lookup: Dict[PropertyPath, ComplexKind] = {}
 
         def path_for(
-            owner: ComplexKind,
             prop: Property,
             kind: Kind,
             path: PropertyPath,
@@ -1284,30 +1287,24 @@ def path_for(
             kind.resolve(model)
             if isinstance(kind, SimpleKind):
                 result.append(ResolvedPropertyPath(relative, prop, kind))
-                owner_lookup[relative] = owner
             elif isinstance(kind, ArrayKind):
                 if name := relative.last_part:
                     result.append(ResolvedPropertyPath(relative, Property(name, kind.fqn), kind))
-                    owner_lookup[relative] = owner
-                path_for(owner, prop, kind.inner, path, visited_kinds, True)
+                path_for(prop, kind.inner, path, visited_kinds, True)
             elif isinstance(kind, DictionaryKind):
                 child = relative.child(None)
                 if name := relative.last_part:
                     result.append(ResolvedPropertyPath(relative, Property(name, kind.fqn), kind))
-                    owner_lookup[relative] = owner
                     # Any child path accessing this dictionary will get a property of value kind.
                     value = kind.value_kind
                     result.append(ResolvedPropertyPath(child, Property("any", value.fqn), value))
-                    owner_lookup[child] = owner
-                path_for(owner, prop, kind.value_kind, child, visited_kinds, add_prop_to_path=False)
+                path_for(prop, kind.value_kind, child, visited_kinds, add_prop_to_path=False)
             elif isinstance(kind, ComplexKind):
                 if name := relative.last_part:
                     result.append(ResolvedPropertyPath(relative, Property(name, kind.fqn), kind))
-                    owner_lookup[relative] = owner
-                for_complex_kind(owner, kind, relative, visited_kinds)
+                for_complex_kind(kind, relative, visited_kinds)
 
         def for_complex_kind(
-            owner: ComplexKind,
             current: ComplexKind,
             relative: PropertyPath,
             visited_kinds: Optional[Dict[str, Set[str]]] = None,
@@ -1318,12 +1315,10 @@ def for_complex_kind(
                 if isinstance(cpx := model.get(cpx_fqn), ComplexKind):
                     cpx.resolve(model)
                     for prop in cpx.properties:
-                        path_for(
-                            owner, prop, cpx.__resolved_props[prop.name][1], relative, visited_kinds or defaultdict(set)
-                        )
+                        path_for(prop, cpx.__resolved_props[prop.name][1], relative, visited_kinds or defaultdict(set))
 
-        for_complex_kind(complex_kind, complex_kind, PropertyPath([], ""))
-        return result, owner_lookup
+        for_complex_kind(complex_kind, PropertyPath([], ""))
+        return result
 
 
 string_kind = StringKind("string")
@@ -1411,26 +1406,30 @@ def from_kinds(kinds: List[Kind]) -> Model:
         prop_kinds_by_path = {}
         # lookup map to get the aggregate root that defined a specific property path
         # Example: instance_cores
-        complex_by_path_distinct: Dict[PropertyPath, Dict[str, ComplexKind]] = defaultdict(dict)
+        complex_by_prop_distinct: Dict[str, Dict[str, ComplexKind]] = defaultdict(dict)
         for c in all_kinds:
             if isinstance(c, ComplexKind) and c.aggregate_root:
                 for r in c.resolved_property_paths():
                     prop_kinds_by_path[r.path] = r
-                for path, cpl in c.owned_paths().items():
-                    complex_by_path_distinct[path][cpl.fqn] = cpl
+                for cpl in c.resolved_hierarchy().values():
+                    for prop in cpl.properties:
+                        complex_by_prop_distinct[prop.name][cpl.fqn] = cpl
 
-        complex_by_path = {k: list(v.values()) for k, v in complex_by_path_distinct.items()}
-        return Model(kind_dict, list(prop_kinds_by_path.values()), complex_by_path)
+        complex_by_prop = {k: list(v.values()) for k, v in complex_by_prop_distinct.items()}
+        return Model(kind_dict, list(prop_kinds_by_path.values()), complex_by_prop)
 
     def __init__(
         self,
         kinds: Dict[str, Kind],
         property_kind_by_path: List[ResolvedPropertyPath],
-        complex_kinds_by_path: Dict[PropertyPath, List[ComplexKind]],
+        complex_kinds_by_root_prop: Dict[str, List[ComplexKind]],
     ):
+        # all kinds of this model
         self.kinds = kinds
+        # all possible paths in the model to efficiently look up the kind of property
         self.__property_kind_by_path: List[ResolvedPropertyPath] = property_kind_by_path
-        self.__complex_kinds_by_path: Dict[PropertyPath, List[ComplexKind]] = complex_kinds_by_path
+        # only the root paths point to the owners: foo.bla[*].bar -> lookup foo
+        self.__complex_kinds_by_root_prop: Dict[str, List[ComplexKind]] = complex_kinds_by_root_prop
 
     def __contains__(self, name_or_object: Union[str, Json]) -> bool:
         if isinstance(name_or_object, str):
@@ -1481,8 +1480,8 @@ def kind_by_path(self, path: Union[str, List[str]]) -> Kind:
         return self.property_by_path(path).kind
 
     def owners_by_path(self, path_: Union[str, List[str]]) -> List[ComplexKind]:
-        path = PropertyPath.from_string(path_) if isinstance(path_, str) else PropertyPath.from_list(path_)
-        return self.__complex_kinds_by_path.get(path, [])
+        prop_path = PropertyPath.from_string(path_) if isinstance(path_, str) else PropertyPath.from_list(path_)
+        return self.__complex_kinds_by_root_prop.get(p, []) if (p := prop_path.first_part) else []
 
     def coerce(self, js: Json) -> Json:
         try:
@@ -1669,7 +1668,7 @@ def all_predecessor_kinds(kind: ComplexKind) -> Dict[EdgeType, List[str]]:
                 )
             else:
                 result[kind.fqn] = kind
-        return Model(result, self.__property_kind_by_path, self.__complex_kinds_by_path)
+        return Model(result, self.__property_kind_by_path, self.__complex_kinds_by_root_prop)
 
     def filter_complex(
         self, filter_fn: Callable[[ComplexKind], bool], with_bases: bool = True, with_prop_types: bool = True
@@ -1699,7 +1698,7 @@ def add_kind(cpl: ComplexKind) -> None:
             if isinstance(kind, ComplexKind) and filter_fn(kind):
                 add_kind(kind)
 
-        return Model(kinds, self.__property_kind_by_path, self.__complex_kinds_by_path)
+        return Model(kinds, self.__property_kind_by_path, self.__complex_kinds_by_root_prop)
 
     def complete_path(
         self,

fixcore/tests/fixcore/db/model_test.py

@@ -16,3 +16,10 @@ def test_graph_update() -> None:
     gu2 = GraphUpdate(6, 5, 4, 3, 2, 1)
     assert gu1.all_changes() == gu2.all_changes() == 21
     assert gu1 + gu2 == GraphUpdate(7, 7, 7, 7, 7, 7)
+
+
+def test_owner(person_model: Model) -> None:
+    model = QueryModel(Query.by("test"), person_model)
+    assert {a.fqn for a in model.owners("mtime")} == {"Base"}
+    assert {a.fqn for a in model.owners("tags")} == {"Base", "graph_root"}
+    assert {a.fqn for a in model.owners("other_addresses.test.city")} == {"Person"}

fixcore/tests/fixcore/model/model_test.py

@@ -337,6 +337,15 @@ def test_property_path_on_model(person_model: Model) -> None:
     assert person_model.kind_by_path("tags.owner") == person_model["string"]
 
 
+def test_owned_path(person_model: Model) -> None:
+    assert {a.fqn for a in person_model.owners_by_path("name")} == {"Person", "graph_root"}
+    assert {a.fqn for a in person_model.owners_by_path("city")} == {"Address"}
+    assert {a.fqn for a in person_model.owners_by_path("list")} == {"Base"}
+    assert {a.fqn for a in person_model.owners_by_path("mtime")} == {"Base"}
+    assert {a.fqn for a in person_model.owners_by_path("test")} == {"any_foo"}
+    assert len(person_model.owners_by_path("does_not_exist")) == 0
+
+
 def test_metadata_on_update(person_model: Model) -> None:
     person = cast(ComplexKind, person_model["Person"])
     person_23 = deepcopy(person)

plugins/azure/fix_plugin_azure/resource/base.py

@@ -354,13 +354,12 @@ class AzureUsageName:
 class AzureBaseUsage:
     kind: ClassVar[str] = "azure_usage"
     mapping: ClassVar[Dict[str, Bender]] = {
-        "name": S("name", "value"),
+        "name": S("name", "value"),  # inherited by BaseResource - name already defined there
         "usage_name": S("name") >> Bend(AzureUsageName.mapping),
         "current_value": S("currentValue"),
         "limit": S("limit"),
         "unit": S("unit"),
     }
-    name: Optional[str] = field(default=None, metadata={"description": "The name of the resource"})
     usage_name: Optional[AzureUsageName] = field(default=None, metadata={"description": "The name of the type of usage."})  # fmt: skip
     current_value: Optional[int] = field(default=None, metadata={"description": "The current value of the usage."})
     limit: Optional[int] = field(default=None, metadata={"description": "The limit of usage."})