Fixed #349

 * Format the key ordering in the generated ABOUT file.
 	The current ordering is:

	about_resource
	name
	version <-- if any

	and the rest is the order from the mapping.config file (if any);
	otherwise alphabetical order.

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

Author: chinyeungli

docs/CHANGELOG.rst

@@ -5,6 +5,7 @@
     * New UrlListField introduced for list of urls
     * The UrlField is now only taking single URL value
     * The owner is now a StringField instead of ListField
+    * Format the ordering of the genreated ABOUT file (See https://github.com/nexB/aboutcode-toolkit/issues/349#issuecomment-438871444)
 
 
 2018-10-23

src/attributecode/gen.py

@@ -302,7 +302,7 @@ def generate(location, base_dir, license_notice_text_location=None,
                             about.license_name.present = True
 
             # Write the ABOUT files
-            about.dump(dump_loc, with_empty=with_empty, with_absent=with_absent)
+            about.dump(dump_loc, use_mapping=use_mapping, mapping_file=mapping_file, with_empty=with_empty, with_absent=with_absent)
             for e in not_exist_errors:
                 errors.append(Error(ERROR, e))
         except Exception as e:

src/attributecode/model.py

@@ -1091,7 +1091,7 @@ def load_dict(self, fields_dict, base_dir, running_inventory=False,
         return errors
 
 
-    def dumps(self, with_absent=False, with_empty=True):
+    def dumps(self, use_mapping=False, mapping_file=False, with_absent=False, with_empty=True):
         """
         Return self as a formatted ABOUT string.
         If with_absent, include absent (not present) fields.
@@ -1138,9 +1138,10 @@ def dumps(self, with_absent=False, with_empty=True):
             if lic_group[3]:
                 lic_dict['url'] = lic_group[3] 
             about_data.setdefault('licenses', []).append(lic_dict)
-        return saneyaml.dump(about_data)
+        formatted_about_data = util.format_output(about_data, use_mapping, mapping_file)
+        return saneyaml.dump(formatted_about_data)
 
-    def dump(self, location, with_absent=False, with_empty=True):
+    def dump(self, location, use_mapping=False, mapping_file=False, with_absent=False, with_empty=True):
         """
         Write formatted ABOUT representation of self to location.
         If with_absent, include absent (not present) fields.
@@ -1160,7 +1161,7 @@ def dump(self, location, with_absent=False, with_empty=True):
         if on_windows:
             about_file_path = add_unc(about_file_path)
         with codecs.open(about_file_path, mode='wb', encoding='utf-8') as dumped:
-            dumped.write(self.dumps(with_absent, with_empty))
+            dumped.write(self.dumps(use_mapping, mapping_file, with_absent, with_empty))
 
     def dump_lic(self, location, license_dict):
         """

src/attributecode/util.py

@@ -330,7 +330,7 @@ def get_mapping(location=None):
     if not os.path.exists(location):
         return {}
 
-    mapping = {}
+    mapping = collections.OrderedDict()
     try:
         with open(location) as mapping_file:
             for line in mapping_file:
@@ -412,6 +412,52 @@ def apply_mapping(abouts, alternate_mapping=None):
         mapped_abouts.append(mapped_about)
     return mapped_abouts
 
+def get_mapping_key_order(mapping_file):
+    """
+    Get the mapping key order and return as a list
+    """
+    if mapping_file:
+        mapping = get_mapping(mapping_file)
+    else:
+        mapping = get_mapping()
+    return mapping.keys()
+
+def format_output(about_data, use_mapping, mapping_file):
+    """
+    Convert the about_data dictionary to an ordered dictionary for saneyaml.dump()
+    The ordering should be:
+
+    about_resource
+    name
+    version <-- if any
+    and the rest is the order from the mapping.config file (if any); otherwise alphabetical order.
+    """
+    mapping_key_order = []
+    if use_mapping or mapping_file:
+        mapping_key_order = get_mapping_key_order(mapping_file)
+    priority_keys = [u'about_resource', u'name', u'version']
+    about_data_keys = []
+    order_dict = collections.OrderedDict()
+    for key in about_data:
+        about_data_keys.append(key)
+    if u'about_resource' in about_data_keys:
+        order_dict['about_resource'] = about_data['about_resource']
+    if u'name' in about_data_keys:
+        order_dict['name'] = about_data['name']
+    if u'version' in about_data_keys:
+        order_dict['version'] = about_data['version']
+    if not mapping_key_order:
+        for other_key in sorted(about_data_keys):
+            if not other_key in priority_keys:
+                order_dict[other_key] = about_data[other_key]
+    else:
+        for key in mapping_key_order:
+            if not key in priority_keys and key in about_data_keys:
+                order_dict[key] = about_data[key]
+        for other_key in sorted(about_data_keys):
+            if not other_key in priority_keys and not other_key in mapping_key_order:
+                order_dict[other_key] = about_data[other_key]
+    return order_dict
 
 def get_about_file_path(location, use_mapping=False, mapping_file=None):
     """

tests/test_gen.py

@@ -62,12 +62,12 @@ def test_load_inventory(self):
         assert expected_errors == errors
 
         expected = [u'about_resource: .\n'
+                    u'name: AboutCode\n'
+                    u'version: 0.11.0\n'
                     u'description: |-\n'
                     u'    multi\n'
-                    u'    line\n'
-                    u'name: AboutCode\n'
-                    u'version: 0.11.0\n']
-        result = [a.dumps(with_absent=False, with_empty=False)
+                    u'    line\n']
+        result = [a.dumps(use_mapping=False, mapping_file=False, with_absent=False, with_empty=False)
                         for a in abouts]
         assert expected == result
 
@@ -86,15 +86,15 @@ def test_load_inventory_with_mapping(self):
         assert sorted(expected_errors) == sorted(errors)
 
         expected = [u'about_resource: .\n'
+                    u'name: AboutCode\n'
+                    u'version: 0.11.0\n'
                     u'copyright: Copyright (c) nexB, Inc.\n'
+                    u'resource: this.ABOUT\n'
                     u'description: |-\n'
                     u'    multi\n'
                     u'    line\n'
-                    u'name: AboutCode\n'
-                    u'resource: this.ABOUT\n'
-                    u'version: 0.11.0\n'
                     ]
-        result = [a.dumps(with_absent=False, with_empty=False)
+        result = [a.dumps(use_mapping, mapping_file=False, with_absent=False, with_empty=False)
                         for a in abouts]
         assert expected == result
 
@@ -148,15 +148,15 @@ def test_generate(self):
         assert msg1 in errors[0].message
         assert msg2 in errors[1].message
 
-        in_mem_result = [a.dumps(with_absent=False, with_empty=False)
+        in_mem_result = [a.dumps(use_mapping=False, mapping_file=False, with_absent=False, with_empty=False)
                         for a in abouts][0]
         expected = (u'about_resource: .\n'
+                    u'name: AboutCode\n'
+                    u'version: 0.11.0\n'
                     u'about_resource_path: .\n'
                     u'description: |-\n'
                     u'    multi\n'
-                    u'    line\n'
-                    u'name: AboutCode\n'
-                    u'version: 0.11.0\n')
+                    u'    line\n')
         assert expected == in_mem_result
 
     def test_generate_not_overwrite_original_license_file(self):
@@ -167,14 +167,14 @@ def test_generate_not_overwrite_original_license_file(self):
 
         errors, abouts = gen.generate(location, base_dir, license_notice_text_location, fetch_license)
 
-        in_mem_result = [a.dumps(with_absent=False, with_empty=False)
+        in_mem_result = [a.dumps(use_mapping=False, mapping_file=False, with_absent=False, with_empty=False)
                         for a in abouts][0]
         expected = (u'about_resource: .\n'
+                    u'name: AboutCode\n'
+                    u'version: 0.11.0\n'
                     u'about_resource_path: .\n'
                     u'licenses:\n'
-                    u'    -   file: this.LICENSE\n'
-                    u'name: AboutCode\n'
-                    u'version: 0.11.0\n')
+                    u'    -   file: this.LICENSE\n')
         assert expected == in_mem_result
 
     def test_deduplicate(self):

tests/test_model.py

@@ -707,29 +707,29 @@ def test_About_dumps(self):
         assert [] == a.errors
 
         expected = u'''about_resource: .
+name: AboutCode
+version: 0.11.0
+copyright: Copyright (c) 2013-2014 nexB Inc.
+license_expression: apache-2.0
 author:
     - Jillian Daguil
     - Chin Yeung Li
     - Philippe Ombredanne
     - Thomas Druez
-copyright: Copyright (c) 2013-2014 nexB Inc.
 description: |-
     AboutCode is a tool
     to process ABOUT files.
     An ABOUT file is a file.
 homepage_url: http://dejacode.org
-license_expression: apache-2.0
 licenses:
     -   file: apache-2.0.LICENSE
         key: apache-2.0
-name: AboutCode
 notice_file: NOTICE
 owner: nexB Inc.
 vcs_repository: https://github.com/dejacode/about-code-tool.git
 vcs_tool: git
-version: 0.11.0
 '''
-        result = a.dumps()
+        result = a.dumps(use_mapping=True)
         assert expected == result
 
     # We do not support with_absent and with_empty staring in version 3.2.0.
@@ -811,7 +811,7 @@ def test_About_dumps_with_different_boolean_value(self):
 modified: yes
 '''
 
-        result = a.dumps()
+        result = a.dumps(use_mapping=False, mapping_file=False)
         assert set(expected) == set(result)
 
 
@@ -827,7 +827,7 @@ def test_About_dumps_does_not_dump_present__empty_with_absent_False(self):
 name: AboutCode
 version: 0.11.0
 '''
-        result = a.dumps(with_absent=False, with_empty=False)
+        result = a.dumps(use_mapping=False, mapping_file=False, with_absent=False, with_empty=False)
         assert expected == result
 
     def test_About_as_dict_contains_special_paths(self):
@@ -940,7 +940,7 @@ def test_load_dump_is_idempotent(self):
         a = model.About()
         a.load(test_file)
         dumped_file = get_temp_file('that.ABOUT')
-        a.dump(dumped_file, with_absent=False, with_empty=False)
+        a.dump(dumped_file, use_mapping=False, mapping_file=False, with_absent=False, with_empty=False)
 
         expected = get_unicode_content(test_file).splitlines()
         result = get_unicode_content(dumped_file).splitlines()

tests/test_util.py

@@ -266,6 +266,17 @@ def test_get_relative_path_with_same_path_twice(self):
             result = util.get_relative_path(loc, loc)
             assert expected == result
 
+    def test_get_mapping_key_order(self):
+        expected = ['about_file_path', 'name', 'version', 'copyright', 'license_expression', 'resource']
+        result = util.get_mapping_key_order(mapping_file=False)
+        assert expected == result
+
+    def test_get_mapping_key_order_with_mapping_file(self):
+        expected = ['about_file_path', 'name', 'version', 'description', 'license_expression', 'copyright']
+        test_mapping_file = get_test_loc('mapping_config/mapping.config')
+        result = util.get_mapping_key_order(test_mapping_file)
+        assert expected == result
+
     def test_load_csv_without_mapping(self):
         test_file = get_test_loc('util/about.csv')
         expected = [OrderedDict(

tests/testdata/mapping_config/mapping.config

@@ -0,0 +1,8 @@
+about_file_path: about_file
+name: Component
+version: Confirmed Version
+
+# Optional Fields
+description: description
+license_expression: dje_license_key
+copyright: Confirmed Copyright