From 4d230093f771917de79cb2c5f1851dcbfc2b8b12 Mon Sep 17 00:00:00 2001
From: Michael Joyce <Michael.J.Joyce@jpl.nasa.gov>
Date: Fri, 5 Jan 2024 13:59:09 -0800
Subject: [PATCH] Issue #507 - Fix invalid content validation check in base
 val.py classes

Update base Validator.content_val to return a proper default value.
Validation that doesn't include a custom `content_val` implementation
relies on this default implementation during validation checks. By
default, that includes EVR and Limit dictionary checks. The current
implementation incorrect returns a garbage value (None) for the default
implementation.

Documentation of the default `content_val` method has been updated to
elaborate on why it's making the current check and what a more thorough
implementation looks like.

Tangentially related, error handling around schema validation was a bit
lacking. This also includes minor updates around the `schema_val` checks
in `Validator` to better handle and log exceptions related to that.
Additionally, validation is short circuited if failures occur there to
avoid confusing error messages in unrelated `content_val` checks.
---
 ait/core/val.py | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/ait/core/val.py b/ait/core/val.py
index 03c39262..874ff12f 100644
--- a/ait/core/val.py
+++ b/ait/core/val.py
@@ -456,19 +456,27 @@ def validate(self, ymldata=None, messages=None):
 
         """
         content_val_list = []
-        schema_val = self.schema_val(messages)
+        try:
+            schema_val = self.schema_val(messages)
+        except yaml.YAMLError as e:
+            log.error(
+                "Unable to validate file due to YAML error. "
+                f"Verify that the YAML file is well formed.\n\nError: {e}"
+            )
+            return False
+        except jsonschema.SchemaError as e:
+            log.error(
+                "Unable to validate file due to error with schema file. "
+                f"Verify availability and validity of the schema file.\n\nError: {e}"
+            )
+            return False
 
         # Loop through the list of all the tested yaml files
         for yaml_file in self.yml_files_to_validate:
             log.info(f"Validating: {yaml_file}")
             content_val_list.append(self.content_val(yaml_file, messages=messages))
 
-        if all(content_val_list):  # Test that all tested files returned True
-            content_val = True
-        else:
-            content_val = False
-
-        return schema_val and content_val
+        return schema_val and all(content_val_list)
 
     def schema_val(self, messages=None):
         """Perform validation with processed YAML and Schema"""
@@ -520,8 +528,19 @@ def schema_val(self, messages=None):
         return valid
 
     def content_val(self, yaml_file, ymldata=None, messages=None):
-        """Simple base content_val method - needed for unit tests"""
+        """Simple base content_val method - needed for unit tests
+
+        The base content_val method doesn't check any useful elements of the
+        loaded file. Instead it just returns whether the file was successfully
+        loaded by the YAMLProcessor.
+
+        Child classes should overwrite this to implement checks for specific
+        content requirements that cannot be easily encoded in a schema file.
+        For examples, see :meth:`CmdValidator.content_val` or
+        :meth:`TlmValidator.content_val`
+        """
         self._ymlproc = YAMLProcessor(yaml_file, False)
+        return self._ymlproc.loaded
 
 
 class CmdValidator(Validator):