Repository: CODAIT/exchange-metadata-converter

Issue:
Support optional properties
Currently all `{{...}}` placeholders in https://github.com/CODAIT/exchange-metadata-converter/tree/main/templates are considered to be required and therefore each placeholder input file must define them. Annotations would solve this issue. Investigate what it takes to support something like this, where `@annotation_key` serves as a hint to the processing engine and doesn't invalidate the YAML file because it is specified as a comment.

```
property: '{{value}}'         #  just a comment
another:
  property2: '{{value2}}'     # @optional and a comment
third_property:               #  comment only
fourth_property:
fifth_property: '{{value6}}'  # @annotation_only
```

Discussion:

Patch:
diff --git a/metadata_converter/apply.py b/metadata_converter/apply.py
index f20d451..34cd14f 100644
--- a/metadata_converter/apply.py
+++ b/metadata_converter/apply.py
@@ -16,6 +16,8 @@
 from argparse import ArgumentParser
 from pathlib import Path
 from ruamel.yaml import YAML
+from ruamel.yaml.tokens import CommentToken
+from ruamel.yaml.comments import CommentedMap
 import re
 import sys
 
@@ -48,11 +50,70 @@ def replace(yaml_dict, template_dict):
     :rtype: dict
     """
 
+    def get_metadata(template_dict, key):
+        # If d is an instance of ruamel.yaml.comments.CommentedMap
+        # this method returns the metadata associated with the
+        # provided property key
+
+        if not isinstance(template_dict, CommentedMap):
+            return None
+
+        d = template_dict
+
+        metadata = {
+            'comment': None,
+            'comment_text': None,
+            'annotations': set()
+        }
+        # raw comment, e.g. "# @optional another comment\n"
+        if key in d.ca.items:
+            if (len(d.ca.items.get(key)) > 2) and \
+               isinstance(d.ca.items.get(key)[2], CommentToken):
+                metadata['comment'] = d.ca.items.get(key)[2].value
+        # comment text, e.g. "another comment"
+        metadata['comment_text'] = metadata['comment']
+        # extract embedded annotations, e.g. "@optional"
+        if metadata['comment'] is not None:
+            # Extract @... annotations
+            for match in re.finditer('@([a-zA-Z]+)',
+                                     metadata['comment'],
+                                     re.I):
+                metadata['annotations'].add(match.group(1).lower())
+            # Remove noise from the comment, such as annotations
+            # comment characters and whitespace characters
+            #  - Remove annotations
+            for annotation in metadata['annotations']:
+                metadata['comment_text'] =\
+                    metadata['comment_text'].replace('@{}'.format(annotation),
+                                                     '')
+            #  - Remove '#' and whitespaces from the beginning of each line
+            #    This needs to be done twice to handle empty comments properly.
+            metadata['comment_text'] = re.sub(r'^\s*#\s*',
+                                              '',
+                                              metadata['comment_text'],
+                                              flags=re.MULTILINE)
+            metadata['comment_text'] = re.sub(r'^\s*#\s*',
+                                              '',
+                                              metadata['comment_text'],
+                                              flags=re.MULTILINE)
+            #  - remove whitespace chars from the end of each line
+            metadata['comment_text'] = re.sub(r'\s*$',
+                                              '',
+                                              metadata['comment_text'],
+                                              flags=re.MULTILINE)
+
+            #  - remove newline from the end
+            metadata['comment_text'] = metadata['comment_text'].strip()
+
+            if len(metadata['comment_text']) == 0:
+                metadata['comment_text'] = None
+        return metadata
+
     # pre-compile the {{...}} placeholder search expression
     # outside `do_replace` to avoid doing it multiple times
     p = re.compile(r'{{([\w\.]+)}}', re.VERBOSE)
 
-    def process_string(yaml_dict, str):
+    def process_string(yaml_dict, str, metadata):
         # determine whether {{...}} placeholder
         # replacement is required
         r = p.search(str)
@@ -63,19 +124,25 @@ def replace(yaml_dict, template_dict):
             for property in r.group(1).split('.'):
                 root = root.get(property)
                 if root is None:
-                    raise PlaceholderNotFoundError(r.group(1))
+                    # check annotations to determine appropriate
+                    # behavior
+                    if metadata is not None and\
+                       'optional' in metadata['annotations']:
+                        return None
+                    else:
+                        raise PlaceholderNotFoundError(r.group(1))
             return root
 
-    def process_list(yaml_dict, list_in):
+    def process_list(yaml_dict, list_in, metadata):
         list_out = []
         for v in list_in:
             # if v is a string process it
             if(isinstance(v, str)):
-                list_out.append(process_string(yaml_dict, v))
+                list_out.append(process_string(yaml_dict, v, metadata))
             elif(isinstance(v, dict)):
                 list_out.append(do_replace(yaml_dict, v))
             elif(isinstance(v, list)):
-                list_out.append(process_list(yaml_dict, v))
+                list_out.append(process_list(yaml_dict, v, metadata))
             else:
                 raise NotImplementedError(
                         'Support for properties of type {} in lists '
@@ -103,18 +170,23 @@ def replace(yaml_dict, template_dict):
         for key in template_dict.keys():
             val = template_dict[key]
             # print('Key: {} Value: {} Type: {}'.format(key, val, type(val)))
+            metadata = get_metadata(template_dict, key)
             if val is None:
                 # NoneType - no value
                 template_out[key] = None
             elif isinstance(val, str):
                 # process string
-                template_out[key] = process_string(yaml_dict, val)
+                template_out[key] = process_string(yaml_dict,
+                                                   val,
+                                                   metadata)
             elif isinstance(val, dict):
                 # process the dictionary recursively
                 template_out[key] = do_replace(yaml_dict, val)
             elif isinstance(val, list):
                 # process each list item
-                template_out[key] = process_list(yaml_dict, val)
+                template_out[key] = process_list(yaml_dict,
+                                                 val,
+                                                 metadata)
             else:
                 raise NotImplementedError(
                         'Support for properties of type {} is not implemented.'
diff --git a/setup.py b/setup.py
index d3a2bc7..5ab9ac3 100644
--- a/setup.py
+++ b/setup.py
@@ -22,7 +22,7 @@ with open('README.md') as readme:
 setup(
   name='exchange-metadata-converter',
   packages=find_packages(),
-  version='0.0.6',
+  version='0.0.7',
   license='Apache-2.0',
   description='exchange metadata converters',
   long_description=README,


Test patch:
diff --git a/tests/inputs/annotations.yaml b/tests/inputs/annotations.yaml
new file mode 100644
index 0000000..84678c1
--- /dev/null
+++ b/tests/inputs/annotations.yaml
@@ -0,0 +1,6 @@
+# This YAML document does not define the following properties:
+# noprop1value, noprop2value, noprop3value, noprop3value,
+# noprop4value, noprop5value, noprop6value
+unused_property: True
+hey:
+  we_are_a_match: 'Then let us pair up!'
\ No newline at end of file
diff --git a/tests/templates/annotations.yaml b/tests/templates/annotations.yaml
new file mode 100644
index 0000000..890c6e3
--- /dev/null
+++ b/tests/templates/annotations.yaml
@@ -0,0 +1,46 @@
+# test level-0 properties; each one is annotated with
+# <at>optional and should therefore not raise an error
+property1: '{{noprop1value}}' # @optional
+property2: '{{noprop2value}}' # comment @optional
+property3: '{{noprop3value}}' # more comments @optional
+property4: '{{noprop4value}}' # more @optional comments
+property5: '{{noprop5value}}' # more @bogus comments @optional
+property6: '{{noprop6value}}' # more 
+                              # @bogus comments
+                              #  
+                              # @optional
+property7:
+  property8: '{{noprop8value}}' # property8 is
+                                #  
+                                # an @optional property
+                                # we are @very @very @lucky
+property9:                           # annotations work in dicts
+  property10:
+    property11: '{{noprop11value}}'  # this @optional property
+                                     # is @fine
+property12:                           # annotations work in lists
+  - property12a: '{{hey.we_are_a_match}}'   # @optional does not apply
+    property12b: '{{noprop12bvalue}}' # sweet, it's @optional
+    property12c: a_constant!
+property13:                             # annotations are case insensitive
+  property14: 
+    property15:
+      property16a: '{{noprop16avalue}}' # @OPTIONAL
+      property16b: '{{noprop16bvalue}}' # an @Optional property
+      property16c: '{{noprop16cvalue}}' # another @OptioNAl property
+    property17:  all is good            # Nothing @opTioNAL here
+    property18: '{{hey.we_are_a_match}}'# nothing @OPTIONal here either
+---
+property1: '{{noprop1value}}' #
+---
+property1: '{{noprop1value}}' # just a comment
+---
+property1: '{{noprop1value}}' # @notarecognizedannotation
+---
+property1: '{{noprop1value}}' # more
+                              # is not always @better
+                              #
+---
+property1:                          # @optional does not apply
+  property1a: '{{noprop1avalue}}'   # because we fail here
+
diff --git a/tests/test_annotations.py b/tests/test_annotations.py
new file mode 100644
index 0000000..e11616d
--- /dev/null
+++ b/tests/test_annotations.py
@@ -0,0 +1,114 @@
+#
+# Copyright 2020 IBM Corporation
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+from metadata_converter.apply import replace
+from metadata_converter.apply import PlaceholderNotFoundError
+from pathlib import Path
+from ruamel.yaml import YAML
+import unittest
+
+yaml = YAML()
+yaml.default_flow_style = False
+
+#
+# Tests template annotations: @optional
+#
+
+
+class TestAnnotations(unittest.TestCase):
+
+    def setUp(self):
+
+        self.placeholder_file = 'tests/inputs/annotations.yaml'
+
+        self.in_yamls = yaml.load(Path(self.placeholder_file))
+
+        self.template_file = 'tests/templates/annotations.yaml'
+
+        self.template_yamls = list(yaml.load_all(Path(self.template_file)))
+        self.assertEqual(len(self.template_yamls), 6)
+
+    def test_optional_annotation(self):
+
+        try:
+            out_dict = replace(self.in_yamls, self.template_yamls[0])
+            self.assertTrue(out_dict is not None)
+            self.assertTrue(isinstance(out_dict, dict))
+            self.assertIsNone(out_dict['property1'])
+            self.assertIsNone(out_dict['property2'])
+            self.assertIsNone(out_dict['property3'])
+            self.assertIsNone(out_dict['property4'])
+            self.assertIsNone(out_dict['property5'])
+            self.assertIsNone(out_dict['property6'])
+            self.assertIsNone(out_dict['property7']['property8'])
+            self.assertIsNone(
+                out_dict['property9']['property10']['property11'])
+            self.assertEqual(
+                out_dict['property12'][0]['property12a'],
+                self.in_yamls['hey']['we_are_a_match'])
+            self.assertIsNone(
+                out_dict['property12'][0]['property12b'])
+            self.assertEqual(
+                self.template_yamls[0]['property12'][0]['property12c'],
+                out_dict['property12'][0]['property12c'])
+            self.assertIsNone(
+                out_dict['property13']['property14']
+                        ['property15']['property16a'])
+            self.assertIsNone(
+                out_dict['property13']['property14']
+                        ['property15']['property16b'])
+            self.assertIsNone(
+                out_dict['property13']['property14']
+                        ['property15']['property16c'])
+            self.assertEqual(out_dict['property13']
+                             ['property14']['property17'],
+                             self.template_yamls[0]['property13']['property14']
+                             ['property17'])
+            self.assertEqual(out_dict['property13']
+                             ['property14']['property18'],
+                             self.in_yamls['hey']['we_are_a_match'])
+        except Exception as e:
+            self.assertTrue(False, e)
+
+    def test_without_optional_annotation(self):
+
+        # self.template_yamls contains multipe documents with
+        # references to an undefined placeholder
+        for index in [1, 2, 3, 4]:
+            try:
+                # should fail
+                replace(self.in_yamls, self.template_yamls[index])
+                self.assertTrue(False)
+            except PlaceholderNotFoundError as pnfe:
+                self.assertEqual('{{' + pnfe.placeholder + '}}',
+                                 self.template_yamls[index]['property1'])
+            except Exception as e:
+                self.assertTrue(False, e)
+
+        for index in [5]:
+            try:
+                # should fail
+                replace(self.in_yamls, self.template_yamls[index])
+                self.assertTrue(False)
+            except PlaceholderNotFoundError as pnfe:
+                self.assertEqual('{{' + pnfe.placeholder + '}}',
+                                 self.template_yamls[index]
+                                 .get('property1').get('property1a'))
+            except Exception as e:
+                self.assertTrue(False, e)
+
+
+if __name__ == '__main__':
+    unittest.main()