Fixed #327 and #341

 * Correct the boolean output behavior
 * Support 'x' as the "True" value for boolean field
 * Add test code

Signed-off-by: Chin Yeung Li <[email protected]>

Author: chinyeungli

src/attributecode/model.py

@@ -75,6 +75,7 @@ class Field(object):
     An ABOUT file field. The initial value is a string. Subclasses can and
     will alter the value type as needed.
     """
+
     def __init__(self, name=None, value=None, required=False, present=False):
         # normalized names are lowercased per specification
         self.name = name
@@ -113,6 +114,8 @@ def validate(self, *args, **kwargs):
                 return errors
         else:
             # 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:
                 # ... especially if required
                 if self.required:
@@ -542,9 +545,9 @@ class BooleanField(SingleLineField):
     def default_value(self):
         return None
 
-    flags = {'yes': True, 'y': True, 'no': False, 'n': False, 'true' : True, 'false': False, }
-
-    flag_values = ', '.join(flags)
+    true_flags = ('yes', 'y', 'true', 'x')
+    false_flags = ('no', 'n', 'false')
+    flag_values = true_flags + false_flags
 
     def _validate(self, *args, **kwargs):
         """
@@ -568,7 +571,10 @@ def _validate(self, *args, **kwargs):
             errors.append(Error(INFO, msg))
             self.value = None
         else:
-            self.value = self.flags.get(flag)
+            if flag == u'yes' or flag is True:
+                self.value = True
+            else:
+                self.value = False
         return errors
 
     def get_flag(self, value):
@@ -590,8 +596,10 @@ def get_flag(self, value):
 
                 value = value.lower()
                 if value in self.flag_values:
-                    # of of yes, no, true, etc.
-                    return value
+                    if value in self.true_flags:
+                        return u'yes'
+                    else:
+                        return u'no'
                 else:
                     return False
             else:
@@ -793,6 +801,7 @@ def all_fields(self, with_absent=True, with_empty=True):
         If with_empty, include empty fields.
         """
         all_fields = []
+        bool_fields = ['redistribute', 'attribute', 'track_changes', 'modified']
         for field in list(self.fields.values()) + list(self.custom_fields.values()):
             if field.required:
                 all_fields.append(field)
@@ -805,7 +814,10 @@ def all_fields(self, with_absent=True, with_empty=True):
                         all_fields.append(field)
                     elif field.present and field.value:
                         all_fields.append(field)
-
+                    elif field.name in bool_fields:
+                        # Check is the value valid
+                        if not field.value == None:
+                            all_fields.append(field)
                 else:
                     if field.present:
                         if not field.value:
@@ -1005,7 +1017,10 @@ def load(self, location, use_mapping=False, mapping_file=None):
             and then join with the 'about_resource_path'
             """
             running_inventory = True
-            errs = self.load_dict(saneyaml.load(input_text), base_dir, running_inventory, use_mapping, mapping_file)
+            # wrap the value of the boolean field in quote to avoid 
+            # automatically convertion from yaml.load
+            input = util.wrap_boolean_value(input_text)
+            errs = self.load_dict(saneyaml.load(input), base_dir, running_inventory, use_mapping, mapping_file)
             errors.extend(errs)
         except Exception as e:
             msg = 'Cannot load invalid ABOUT file: %(location)r: %(e)r\n' + str(e)
@@ -1043,22 +1058,6 @@ def load_dict(self, fields_dict, base_dir, running_inventory=False,
                 # 'Field licenses is not a supported field and is ignored.'
                 licenses_field = (key, value)
                 fields.remove(licenses_field)
-        """
-        # Generate about_resource_path if not present so that it can be validate
-        # the existence of the about_resource
-        arp_present = False 
-        about_resource_value = u''
-        about_parent = u''
-        for n, v in fields:
-            about_parent = dirname(self.about_file_path)
-            if n == u'about_resource_path':
-                arp_present = True
-            elif n == u'about_resource':
-                about_resource_value = v
-        if not arp_present:
-            #about_parent = dirname(self.about_file_path)
-            fields.append((u'about_resource_path', u'.' + posixpath.join(about_parent, about_resource_value)))
-        """
         errors = self.process(
             fields, about_file_path, running_inventory, base_dir, 
             license_notice_text_location, use_mapping, mapping_file)
@@ -1079,7 +1078,7 @@ def dumps(self, with_absent=False, with_empty=True):
         license_file = []
         license_url = []
         file_fields = ['about_resource_path', 'notice_file', 'changelog_file', 'author_file']
-
+        bool_fields = ['redistribute', 'attribute', 'track_changes', 'modified']
         for field in self.all_fields(with_absent, with_empty):
             if field.name == 'license_key' and field.value:
                 license_key = field.value
@@ -1097,7 +1096,9 @@ def dumps(self, with_absent=False, with_empty=True):
             elif field.name in file_fields and field.value:
                 about_data[field.name] = list(field.value.keys())[0]
             else:
-                about_data[field.name] = field.value
+                if field.value or (field.name in bool_fields and not field.value == None):
+                    about_data[field.name] = field.value
+
         # Group the same license information in a list
         license_group = list(zip_longest(license_key, license_name, license_file, license_url))
         for lic_group in license_group:

src/attributecode/util.py

@@ -151,6 +151,21 @@ def check_duplicate_keys_about_file(context):
     return dup_keys
 
 
+def wrap_boolean_value(context):
+    bool_fields = ['redistribute', 'attribute', 'track_changes', 'modified']
+    input = []
+    for line in context.splitlines():
+        key = line.partition(':')[0]
+        if key in bool_fields:
+            value = "'" + line.partition(':')[2].strip() + "'"
+            updated_line = key + ': ' + value
+            input.append(updated_line)
+        else:
+            input.append(line)
+    updated_context = '\n'.join(input)
+    return updated_context
+
+
 def get_absolute(location):
     """
     Return an absolute normalized location.

tests/test_model.py

@@ -706,7 +706,6 @@ def test_About_dumps(self):
         a = model.About(test_file)
         assert [] == a.errors
 
-# Since the licenses group was not processed, 
         expected = u'''about_resource: .
 author:
     - Jillian Daguil
@@ -797,6 +796,27 @@ def test_About_dumps_does_not_dump_not_present_with_absent_False(self):
         result = a.dumps(with_absent=False)
         assert set(expected) == set(result)
 
+    def test_About_dumps_with_different_boolean_value(self):
+        test_file = get_test_loc('parse/complete2/about2.ABOUT')
+        a = model.About(test_file)
+        expected_error_msg = "Field track_changes: Invalid flag value: 'blah' is not one of"
+        assert len(a.errors) == 1
+        assert expected_error_msg in a.errors[0].message
+
+        expected = u'''about_resource: .
+
+name: AboutCode
+version: 0.11.0
+
+redistribute: no
+attribute: yes
+modified: yes
+'''
+
+        result = a.dumps()
+        assert set(expected) == set(result)
+
+
     def test_About_dumps_does_not_dump_present__empty_with_absent_False(self):
         test_file = get_test_loc('parse/complete2/about.ABOUT')
         a = model.About(test_file)

tests/test_util.py

@@ -457,6 +457,25 @@ def test_check_duplicate_keys_about_file(self):
         expected = ['notes']
         assert expected == util.check_duplicate_keys_about_file(test)
 
+    def test_wrap_boolean_value(self):
+        test = '''
+name: test
+notes: some notes
+
+license_expression: mit
+modified: yes
+track_changes: no
+            '''
+        expected = '''
+name: test
+notes: some notes
+
+license_expression: mit
+modified: 'yes'
+track_changes: 'no'
+            '''
+        assert expected == util.wrap_boolean_value(test)
+
     def test_check_duplicate_keys_about_file_with_multiline(self):
         test = '''
 name: test

tests/testdata/parse/complete2/about2.ABOUT

@@ -0,0 +1,9 @@
+about_resource: .
+
+name: AboutCode
+version: 0.11.1
+
+redistribute: n
+attribute: yes
+track_changes: blah
+modified: True