Merge branch '403_invalid_info_for_false_boolean_field' into develop

Author: chinyeungli

src/attributecode/model.py

@@ -61,11 +61,13 @@
 from attributecode import saneyaml
 from attributecode import util
 from attributecode.util import add_unc
+from attributecode.util import boolean_fields
 from attributecode.util import copy_license_notice_files
 from attributecode.util import csv
 from attributecode.util import filter_errors
 from attributecode.util import is_valid_name
 from attributecode.util import on_windows
+from attributecode.util import wrap_boolean_value
 from attributecode.util import UNC_PREFIX
 from attributecode.util import ungroup_licenses
 from attributecode.util import unique
@@ -119,7 +121,7 @@ def validate(self, *args, **kwargs):
             # present fields should have content ...
             # The boolean value can be True, False and None
             # The value True or False is the content of boolean fields
-            if not self.has_content:
+            if not name in boolean_fields and not self.has_content:
                 # ... especially if required
                 if self.required:
                     msg = u'Field %(name)s is required and empty'
@@ -585,8 +587,7 @@ def _validate(self, *args, **kwargs):
             self.value = None
         elif flag is None:
             name = self.name
-            msg = (u'Field %(name)s: field is empty. '
-                   u'Defaulting flag to no.' % locals())
+            msg = (u'Field %(name)s: field is present but empty. ' % locals())
             errors.append(Error(INFO, msg))
             self.value = None
         else:
@@ -896,7 +897,6 @@ def process(self, fields, about_file_path, running_inventory=False,
         afp = self.about_file_path
 
         errors = self.hydrate(fields)
-
         # We want to copy the license_files before the validation
         if reference_dir:
             copy_license_notice_files(
@@ -926,6 +926,10 @@ def load(self, location):
             loc = add_unc(loc)
             with io.open(loc, encoding='utf-8') as txt:
                 input_text = txt.read()
+            # The 'Yes' and 'No' will be converted to 'True' and 'False' in the yaml.load()
+            # Therefore, we need to wrap the original value in quote to prevent
+            # the conversion
+            input = wrap_boolean_value(input_text)
             # FIXME: this should be done in the commands, not here
             """
             The running_inventory defines if the current process is 'inventory' or not.
@@ -937,7 +941,7 @@ def load(self, location):
             and then join with the 'about_resource'
             """
             running_inventory = True
-            data = saneyaml.load(input_text, allow_duplicate_keys=False)
+            data = saneyaml.load(input, allow_duplicate_keys=False)
             errs = self.load_dict(data, base_dir, running_inventory)
             errors.extend(errs)
         except Exception as e:
@@ -979,6 +983,7 @@ def load_dict(self, fields_dict, base_dir, running_inventory=False, reference_di
                 # 'Field licenses is a custom field.'
                 licenses_field = (key, value)
                 fields.remove(licenses_field)
+
         errors = self.process(
             fields=fields,
             about_file_path=self.about_file_path,
@@ -1009,9 +1014,9 @@ def dumps(self):
         license_file = []
         license_url = []
         file_fields = ['about_resource', 'notice_file', 'changelog_file', 'author_file']
-        bool_fields = ['redistribute', 'attribute', 'track_changes', 'modified']
+        bool_fields = ['redistribute', 'attribute', 'track_changes', 'modified', 'internal_use_only']
         for field in self.all_fields():
-            if not field.value:
+            if not field.value and not field.name in bool_fields:
                 continue
 
             if field.name == 'license_key' and field.value:
@@ -1028,8 +1033,10 @@ def dumps(self):
             # value of 'about_resource'
             elif field.name in file_fields and field.value:
                 data[field.name] = list(field.value.keys())[0]
+            elif field.name in bool_fields and not field.value == None:
+                data[field.name] = field.value
             else:
-                if field.value or (field.name in bool_fields and not field.value == None):
+                if field.value:
                     data[field.name] = field.value
 
         # Group the same license information in a list
@@ -1127,7 +1134,6 @@ def collect_inventory(location):
             msg = (about_file_path + ": " + message)
             errors.append(Error(severity, msg))
         abouts.append(about)
-
     return unique(errors), abouts
 
 

src/attributecode/util.py

@@ -51,6 +51,8 @@
 
 on_windows = 'win32' in sys.platform
 
+# boolean field name
+boolean_fields = ['redistribute', 'attribute', 'track_change', 'modified', 'internal_use_only']
 
 def to_posix(path):
     """
@@ -123,6 +125,20 @@ def check_file_names(paths):
             seen[path] = orig_path
     return errors
 
+def wrap_boolean_value(context):
+    updated_context = ''
+    for line in context.splitlines():
+        """
+        wrap the boolean value in quote
+        """
+        key = line.partition(':')[0]
+        value = line.partition(':')[2].strip()
+        value = '"' + value + '"'
+        if key in boolean_fields and not value == "":
+            updated_context += key + ': ' + value + '\n'
+        else:
+            updated_context += line + '\n'
+    return updated_context
 
 # TODO: rename to normalize_path
 def get_absolute(location):

tests/test_gen.py

@@ -199,3 +199,18 @@ def test_generate_not_overwrite_original_license_file(self):
             'licenses:\n'
             '    -   file: this.LICENSE\n')
         assert expected == result
+
+    def test_boolean_value_not_lost(self):
+        location = get_test_loc('test_gen/inv6.csv')
+        base_dir = get_temp_dir()
+
+        _errors, abouts = gen.generate(location, base_dir)
+
+        in_mem_result = [a.dumps() for a in abouts][0]
+        expected = (u'about_resource: .\n'
+                    u'name: AboutCode\n'
+                    u'version: 0.11.0\n'
+                    u'redistribute: yes\n'
+                    u'attribute: yes\n'
+                    u'modified: no\n')
+        assert expected == in_mem_result

tests/test_model.py

@@ -578,6 +578,27 @@ def test_About_invalid_boolean_value(self):
         expected_msg = "Field modified: Invalid flag value: 'blah'"
         assert expected_msg in a.errors[0].message
 
+    def test_About_boolean_value(self):
+        test_file = get_test_loc('test_model/parse/boolean_data.about')
+        a = model.About(test_file)
+        expected_msg = "Field track_changes is present but empty."
+        assert expected_msg in a.errors[0].message
+        # Context of the test file
+        """
+        about_resource: .
+        name: boolean_data
+        attribute: False
+        modified: true
+        internal_use_only: no
+        redistribute: yes
+        track_changes:
+        """
+        assert a.attribute.value is False
+        assert a.modified.value is True
+        assert a.internal_use_only.value is False
+        assert a.redistribute.value is True
+        assert a.track_changes.value is None
+
     def test_About_contains_about_file_path(self):
         test_file = get_test_loc('test_model/serialize/about.ABOUT')
         # TODO: I am not sure this override of the about_file_path makes sense
@@ -681,6 +702,8 @@ def test_About_dumps_does_all_non_empty_present_fields(self):
             Error(INFO, 'Field custom2 is a custom field.'),
             Error(INFO, 'Field custom2 is present but empty.')
         ]
+        print("########################################")
+        print(a.errors)
         assert sorted(expected_error) == sorted(a.errors)
 
         expected = '''about_resource: .

tests/testdata/test_gen/inv6.csv

@@ -0,0 +1,2 @@
+about_file_path,about_resource,name,version,download_url,description,homepage_url,notes,license_name,license_file,license_url,copyright,notice_file,notice_url,redistribute,attribute,track_change,modified,changelog_file,owner,owner_url,contact,author,vcs_tool,vcs_repository,vcs_path,vcs_tag,vcs_branch,vcs_revision,checksum_md5,checksum_sha1,spec_version
+inv/this.ABOUT,.,AboutCode,0.11.0,,,,,,,,,,,Yes,Y,,N,,,,,,,,,,,,,,

tests/testdata/test_model/parse/boolean_data.about

@@ -0,0 +1,7 @@
+about_resource: .
+name: boolean_data
+attribute: False
+modified: true
+internal_use_only: no
+redistribute: yes
+track_changes: