check for target attribute in template link

Author: lukaspie

src/pynxtools/dataconverter/helpers.py

@@ -171,9 +171,12 @@ def _log(self, path: str, log_type: ValidationProblem, value: Optional[Any], *ar
         elif log_type == ValidationProblem.BrokenLink:
             logger.warning(f"Broken link at {path} to {value}.")
         elif log_type == ValidationProblem.MissingTargetAttribute:
-            logger.info(
+            log_text = (
                 f"A link was used for {path}, but no '@target' attribute was found."
             )
+            if value is not None:
+                log_text += f" The link target was automatically set to {value}."
+            logger.info(log_text)
         elif log_type == ValidationProblem.TargetAttributeMismatch:
             logger.info(
                 f"A link was used for {path}, but its @target attribute '{value}' "
@@ -252,6 +255,8 @@ def collect_and_log(
         if log_type in (
             ValidationProblem.UnitWithoutDocumentation,
             ValidationProblem.OpenEnumWithNewItem,
+            ValidationProblem.MissingTargetAttribute,
+            ValidationProblem.TargetAttributeMismatch,
             ValidationProblem.CompressionStrengthZero,
         ):
             if self.logging and message not in self.data["info"]:

src/pynxtools/dataconverter/validation.py

@@ -872,7 +872,7 @@ def visititems(group: h5py.Group, path: str = "", filename: str = ""):
 
                 if "target" not in group[name].attrs:
                     collector.collect_and_log(
-                        full_path, ValidationProblem.MissingTargetAttribute, ""
+                        full_path, ValidationProblem.MissingTargetAttribute, None
                     )
                 else:
                     attr_target = group[name].attrs["target"]
@@ -1229,7 +1229,7 @@ def remove_from_not_visited(path: str) -> str:
         return path
 
     def _follow_link(
-        keys: Optional[Mapping[str, Any]], prev_path: str, p=False
+        keys: Optional[Mapping[str, Any]], prev_path: str
     ) -> Optional[Any]:
         """
         Resolves internal dictionary "links" by replacing any keys containing a
@@ -1284,9 +1284,30 @@ def _follow_link(
                         "key",
                     )
                     keys_to_remove.append(key_path)
+                    keys_to_remove.append(f"{key_path}/@target")
                     del resolved_keys[key]
                 else:
                     resolved_keys[key] = current_keys
+
+                    if f"{key_path}/@target" not in mapping:
+                        collector.collect_and_log(
+                            key_path,
+                            ValidationProblem.MissingTargetAttribute,
+                            value["link"],
+                        )
+                        mapping[f"{key_path}/@target"] = value["link"]
+                    else:
+                        attr_target = mapping[f"{key_path}/@target"]
+                        remove_from_not_visited(f"{key_path}/@target")
+                        target_path = value["link"]
+                        if attr_target != target_path:
+                            collector.collect_and_log(
+                                key_path,
+                                ValidationProblem.TargetAttributeMismatch,
+                                attr_target,
+                                target_path,
+                            )
+
         return resolved_keys
 
     def handle_field(node: NexusNode, keys: Mapping[str, Any], prev_path: str):
@@ -2135,18 +2156,13 @@ def check_reserved_prefix(
     keys = _follow_link(nested_keys, "")
     recurse_tree(tree, nested_keys)
 
-    check_attributes_of_nonexisting_field(tree)
+    # check_attributes_of_nonexisting_field(tree)
 
     for not_visited_key in not_visited:
         if mapping.get(not_visited_key) is None:
             # This value is not really set. Skip checking its validity.
             continue
 
-        # TODO: remove again if "@target"/"@reference" is sorted out by NIAC
-        always_allowed_attributes = ("@target", "@reference")
-        if not_visited_key.endswith(always_allowed_attributes):
-            # If we want to support this in the future, we could check that the targetted field exists.
-            continue
         if not_visited_key.endswith("/@units"):
             # check that parent exists
             if not_visited_key.rsplit("/", 1)[0] not in mapping.keys():
@@ -2247,7 +2263,8 @@ def check_reserved_prefix(
 
     # remove keys that are incorrect
     for key in set(keys_to_remove):
-        del mapping[key]
+        if key in mapping:
+            del mapping[key]
 
     return not collector.has_validation_problems()
 

tests/dataconverter/test_validation.py

@@ -991,16 +991,24 @@ def format_error_message(msg: str) -> str:
         pytest.param(
             alter_dict(
                 alter_dict(
-                    remove_from_dict(
-                        TEMPLATE,
-                        "/ENTRY[my_entry]/required_group/description",
-                        "optional",
+                    alter_dict(
+                        alter_dict(
+                            remove_from_dict(
+                                TEMPLATE,
+                                "/ENTRY[my_entry]/required_group/description",
+                                "optional",
+                            ),
+                            "/ENTRY[my_entry]/required_group",
+                            {"link": "/my_entry/required_group2"},
+                        ),
+                        "/ENTRY[my_entry]/required_group/@target",
+                        "/my_entry/required_group2",
                     ),
-                    "/ENTRY[my_entry]/required_group",
-                    {"link": "/my_entry/required_group2"},
+                    "/ENTRY[my_entry]/OPTIONAL_group[some_group]/required_field",
+                    {"link": "/my_entry/specified_group/specified_field"},
                 ),
-                "/ENTRY[my_entry]/OPTIONAL_group[some_group]/required_field",
-                {"link": "/my_entry/specified_group/specified_field"},
+                "/ENTRY[my_entry]/OPTIONAL_group[some_group]/required_field/@target",
+                "/my_entry/specified_group/specified_field",
             ),
             [],
             id="appdef-links-with-matching-nexus-types",
@@ -1025,9 +1033,13 @@ def format_error_message(msg: str) -> str:
         pytest.param(
             alter_dict(
                 alter_dict(
-                    TEMPLATE,
-                    "/ENTRY[my_entry]/SAMPLE[my_sample]",
-                    {"link": "/my_entry/some_group"},
+                    alter_dict(
+                        TEMPLATE,
+                        "/ENTRY[my_entry]/SAMPLE[my_sample]",
+                        {"link": "/my_entry/some_group"},
+                    ),
+                    "/ENTRY[my_entry]/SAMPLE[my_sample]/@target",
+                    "/my_entry/some_group",
                 ),
                 "/ENTRY[my_entry]/SAMPLE[my_sample2]/name",
                 {"link": "/my_entry/specified_group/some_field223"},