Added per-category field validation and improved some validation messages (#174)

* Improved error messages likely to occur when authoring IAM policies.
* Additional required fields checked per category.

Author: Laren-AWS

aws_doc_sdk_examples_tools/doc_gen.py

@@ -9,6 +9,9 @@
 from functools import reduce
 from pathlib import Path
 from typing import Dict, Iterable, Optional, Set, Tuple, List, Any
+from yaml.parser import ParserError
+
+from yaml import YAMLError
 
 # from os import glob
 
@@ -28,6 +31,7 @@
     NameFormat,
     ActionNameFormat,
     ServiceNameFormat,
+    YamlParseError,
 )
 from .metadata_validator import validate_metadata
 from .project_validator import ValidationConfig
@@ -211,25 +215,28 @@ def find_and_process_metadata(self, metadata_path: Path):
     def process_metadata(self, path: Path) -> "DocGen":
         if path in self._loaded:
             return self
-        with open(path) as file:
-            examples, errs = parse_examples(
-                path,
-                yaml.safe_load(file),
-                self.sdks,
-                self.services,
-                self.standard_categories,
-                self.cross_blocks,
-                self.validation,
-            )
-            self.extend_examples(examples, self.errors)
-            self.errors.extend(errs)
-            for example in examples:
-                for lang in example.languages:
-                    language = example.languages[lang]
-                    for version in language.versions:
-                        for excerpt in version.excerpts:
-                            self.snippet_files.update(excerpt.snippet_files)
-        self._loaded.add(path)
+        try:
+            with open(path) as file:
+                examples, errs = parse_examples(
+                    path,
+                    yaml.safe_load(file),
+                    self.sdks,
+                    self.services,
+                    self.standard_categories,
+                    self.cross_blocks,
+                    self.validation,
+                )
+                self.extend_examples(examples, self.errors)
+                self.errors.extend(errs)
+                for example in examples:
+                    for lang in example.languages:
+                        language = example.languages[lang]
+                        for version in language.versions:
+                            for excerpt in version.excerpts:
+                                self.snippet_files.update(excerpt.snippet_files)
+            self._loaded.add(path)
+        except ParserError as e:
+            self.errors.append(YamlParseError(file=path, parser_error=str(e)))
         return self
 
     @classmethod

aws_doc_sdk_examples_tools/metadata_errors.py

@@ -93,6 +93,21 @@ class MetadataErrors(ErrorsList[MetadataError]):
     pass
 
 
+@dataclass
+class YamlParseError(MetadataError):
+    parser_error: Optional[str] = None
+
+    def prefix(self):
+        prefix = f"In {self.file},"
+        return prefix
+
+    def message(self) -> str:
+        return (
+            f"incorrect YAML format {self.parser_error}.\n"
+            f"\tThis indicates a problem with the YAML itself. Use a YAML authoring tool to help diagnose further."
+        )
+
+
 @dataclass
 class MetadataParseError(MetadataError):
     id: Optional[str] = None
@@ -172,7 +187,7 @@ class FieldError(MetadataParseError):
 @dataclass
 class MissingField(FieldError):
     def message(self):
-        return f"missing field {self.field}"
+        return f"missing field '{self.field}'"
 
 
 @dataclass
@@ -213,7 +228,8 @@ def message(self) -> str:
 class UnknownLanguage(LanguageError):
     def message(self):
         return (
-            f"contains {self.language} as a language, which is not listed in sdks.yaml."
+            f"contains '{self.language}' as a language, which is not listed in sdks.yaml. Typically, this indicates a "
+            f"typo in the name of the language."
         )
 
 
@@ -333,7 +349,10 @@ class UnknownService(MetadataParseError):
     service: str = ""
 
     def message(self):
-        return f"has unknown service {self.service}"
+        return (
+            f"has unknown service '{self.service}'. Typically this indicates a typo in the name of the service, "
+            f"which must match a key used in services.yaml."
+        )
 
 
 @dataclass
@@ -384,7 +403,10 @@ class PersonMissingField(SdkVersionError):
     alias: str = ""
 
     def message(self):
-        return f"person is missing a field: name: {self.name}, alias: {self.alias}"
+        return (
+            f"person is missing a field: name: {self.name}, alias: {self.alias}. A person must have both a name "
+            f"and an alias."
+        )
 
 
 @dataclass

aws_doc_sdk_examples_tools/metadata_test.py

@@ -500,6 +500,62 @@ def test_parse_cross():
     assert actual[0] == example
 
 
+CATEGORY_FIELD_ERRORS = """
+medical-imaging_TestRequiredCategoryFields:
+  title: Delete Topic
+  title_abbrev: Delete topic
+  synopsis: this is a synopsis.
+  category: IAMPolicy
+  languages:
+   PHP:
+     versions:
+       - sdk_version: 1
+         excerpts:
+         - snippet_files:
+           - snippet_files/AccountControlApiDoc/iam-policies.AccountControlApiDoc.src.manage-acct-update-contact-alternate.xml.1.xml  
+  services:
+    medical-imaging:
+"""
+
+
+def test_parse_required_category_fields_errors():
+    meta = yaml.safe_load(CATEGORY_FIELD_ERRORS)
+    _, errors = parse_examples(
+        Path("test_meta.yaml"),
+        meta,
+        SDKS,
+        SERVICES,
+        STANDARD_CATS,
+        set(),
+        ValidationConfig(strict_titles=True),
+    )
+    expected = [
+        metadata_errors.MissingField(
+            file=Path("test_meta.yaml"),
+            id="medical-imaging_TestRequiredCategoryFields",
+            language="PHP",
+            sdk_version=1,
+            field="owner",
+        ),
+        metadata_errors.MissingField(
+            file=Path("test_meta.yaml"),
+            id="medical-imaging_TestRequiredCategoryFields",
+            language="PHP",
+            sdk_version=1,
+            field="source",
+        ),
+        metadata_errors.MissingField(
+            file=Path("test_meta.yaml"),
+            id="medical-imaging_TestRequiredCategoryFields",
+            language="PHP",
+            sdk_version=1,
+            field="authors",
+        ),
+    ]
+    for e in expected:
+        assert e in errors
+
+
 def test_verify_load_successful():
     actual, errors = load(
         TEST_RESOURCES_PATH / "valid_metadata.yaml",

aws_doc_sdk_examples_tools/metadata_validator.py

@@ -14,6 +14,7 @@
 import re
 import xml.etree.ElementTree as xml_tree
 import yaml
+import yaml.parser
 from dataclasses import dataclass, field
 from pathlib import Path
 from typing import Any, Dict, Iterable, List, Optional, Set
@@ -200,6 +201,8 @@ def validate_files(
             data = yamale.make_data(meta_name)
             yamale.validate(schema, data, strict=strict)
             print(f"{meta_name.resolve()} validation success! 👍")
+        except yaml.parser.ParserError as e:
+            pass  # YAML parse errors are found and reported by the DocGen validator so we won't report them here.
         except YamaleError as e:
             errors.append(ValidateYamaleError(file=meta_name, yamale_error=e))
     return errors

aws_doc_sdk_examples_tools/snippets.py

@@ -261,7 +261,10 @@ class MissingSnippetFile(MetadataError):
     snippet_file: Optional[str] = None
 
     def message(self):
-        return f"missing snippet_file {self.snippet_file}"
+        return (
+            f"missing snippet_file {self.snippet_file}. The relative path to the snippet_file listed in the "
+            f"metadata for this example must map to an actual file in the file system."
+        )
 
 
 @dataclass

aws_doc_sdk_examples_tools/yaml_mapper.py

@@ -18,6 +18,9 @@
 from .metadata_validator import StringExtension
 
 
+CATEGORY_REQUIRED_FIELDS = {"IAMPolicy": {"version": {"authors", "owner", "source"}}}
+
+
 def example_from_yaml(
     yaml: Any,
     sdks: Dict[str, Sdk],
@@ -83,7 +86,7 @@ def example_from_yaml(
     else:
         for name in yaml_languages:
             language, errs = language_from_yaml(
-                name, yaml_languages[name], sdks, blocks, is_action
+                name, yaml_languages[name], sdks, blocks, is_action, category
             )
             languages[language.name] = language
             errors.extend(errs)
@@ -146,6 +149,7 @@ def language_from_yaml(
     sdks: Dict[str, Sdk],
     blocks: Set[str],
     is_action: bool,
+    category: str,
 ) -> Tuple[Language, MetadataErrors]:
     errors = MetadataErrors()
     if name not in sdks:
@@ -161,7 +165,7 @@ def language_from_yaml(
 
     versions: List[Version] = []
     for version in yaml_versions:
-        vers, version_errors = version_from_yaml(version, blocks, is_action)
+        vers, version_errors = version_from_yaml(version, blocks, is_action, category)
         errors.extend(version_errors)
         versions.append(vers)
 
@@ -222,9 +226,14 @@ def version_from_yaml(
     yaml: Dict[str, Any],
     cross_content_blocks: Set[str],
     is_action: bool,
+    category: str,
 ) -> Tuple["Version", MetadataErrors]:
     errors = MetadataErrors()
 
+    for field in CATEGORY_REQUIRED_FIELDS.get(category, {}).get("version", {}):
+        if not yaml.get(field):
+            errors.append(metadata_errors.MissingField(field=field))
+
     sdk_version = int(yaml.get("sdk_version", 0))
     if sdk_version == 0:
         errors.append(metadata_errors.MissingField(field="sdk_version"))

aws_doc_sdk_examples_tools/yaml_writer_test.py

@@ -18,7 +18,10 @@ def test_doc_gen(sample_doc_gen: DocGen):
     writes = prepare_write(sample_doc_gen.examples)
     assert writes
 
-    writes = {k.replace(str(ROOT) + "/", ""): v for k, v in writes.items()}
+    writes = {
+        Path(k.replace(str(ROOT), "")).as_posix().lstrip("/"): v
+        for k, v in writes.items()
+    }
 
     expected_writes = {
         ".doc_gen/metadata/aws_entity_metadata.yaml": """sns_EntitySuccesses: