#364 - Remove the about_file_path key/column from input/output

 - It will not output the `about_file_path` key/column in the output
(which include the generated about_files and the csv/json file).
 - the `about_resource` is now an essential/required field

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

Author: chinyeungli

src/attributecode/gen.py

@@ -78,21 +78,21 @@ def check_duplicated_columns(location):
     return unique(errors)
 
 
-def check_duplicated_about_file_path(inventory_dict):
+def check_duplicated_about_resource(inventory_dict):
     """
-    Return a list of errors for duplicated about_file_path in a CSV file at location.
+    Return a list of errors for duplicated about_resource in a CSV file at location.
     """
-    afp_list = []
+    arp_list = []
     errors = []
     for component in inventory_dict:
         # Ignore all the empty path
-        if component['about_file_path']:
-            if component['about_file_path'] in afp_list:
-                msg = ("The input has duplicated values in 'about_file_path' "
-                       "field: " + component['about_file_path'])
+        if component['about_resource']:
+            if component['about_resource'] in arp_list:
+                msg = ("The input has duplicated values in 'about_resource' "
+                       "field: " + component['about_resource'])
                 errors.append(Error(CRITICAL, msg))
             else:
-                afp_list.append(component['about_file_path'])
+                arp_list.append(component['about_resource'])
     return errors
 
 
@@ -122,13 +122,13 @@ def load_inventory(location, base_dir, reference_dir=None):
 
     try:
         # FIXME: this should not be done here.
-        dup_about_paths_err = check_duplicated_about_file_path(inventory)
-        if dup_about_paths_err:
-            errors.extend(dup_about_paths_err)
+        dup_about_resource_err = check_duplicated_about_resource(inventory)
+        if dup_about_resource_err:
+            errors.extend(dup_about_resource_err)
             return errors, abouts
     except Exception as e:
         # TODO: why catch ALL Exception
-        msg = "The essential field 'about_file_path' is not found in the <input>"
+        msg = "The essential field 'about_resource' is not found in the <input>"
         errors.append(Error(CRITICAL, msg))
         return errors, abouts
 
@@ -141,7 +141,7 @@ def load_inventory(location, base_dir, reference_dir=None):
                 msg = "Required field: %(f)r not found in the <input>" % locals()
                 errors.append(Error(ERROR, msg))
                 return errors, abouts
-        afp = fields.get(model.About.ABOUT_FILE_PATH_ATTR)
+        afp = fields.get(model.About.ABOUT_RESOURCE_ATTR)
 
         # FIXME: this should not be a failure condition
         if not afp or not afp.strip():
@@ -154,24 +154,39 @@ def load_inventory(location, base_dir, reference_dir=None):
         about = model.About(about_file_path=afp)
         about.location = loc
 
+        # Update value for 'about_resource'
+        # keep only the filename or '.' if it's a directory
+        if 'about_resource' in fields:
+            updated_resource_value = u''
+            resource_path = fields['about_resource']
+            if resource_path.endswith(u'/'):
+                updated_resource_value = u'.'
+            else:
+                updated_resource_value = basename(resource_path)
+            fields['about_resource'] = updated_resource_value
+        
         ld_errors = about.load_dict(
             fields,
             base_dir,
             running_inventory=False,
             reference_dir=reference_dir,
         )
+        """
         # 'about_resource' field will be generated during the process.
         # No error need to be raise for the missing 'about_resource'.
         for e in ld_errors:
             if e.message == 'Field about_resource is required':
                 ld_errors.remove(e)
+        """
         for e in ld_errors:
             if not e in errors:
                 errors.extend(ld_errors)
         abouts.append(about)
 
     return unique(errors), abouts
 
+def update_about_resource(self):
+    pass
 
 def generate(location, base_dir, reference_dir=None, fetch_license=False):
     """

src/attributecode/model.py

@@ -693,9 +693,12 @@ class About(object):
 
     # name of the attribute containing the resolved relative Resources paths
     about_resource_path_attr = 'about_resource_path'
+    
+    # name of the attribute containing the resolved relative Resources paths
+    ABOUT_RESOURCE_ATTR = 'about_resource'    
 
     # Required fields
-    required_fields = ['name']
+    required_fields = ['name', ABOUT_RESOURCE_ATTR]
 
     def get_required_fields(self):
         return [f for f in self.fields if f.required]
@@ -913,7 +916,6 @@ def process(self, fields, about_file_path, running_inventory=False,
             self.base_dir,
             self.reference_dir)
         errors.extend(validation_errors)
-
         return errors
 
     def load(self, location):
@@ -1130,7 +1132,6 @@ def collect_inventory(location):
             msg = (about_file_path + ": " + message)
             errors.append(Error(severity, msg))
         abouts.append(about)
-
     return unique(errors), abouts
 
 
@@ -1184,16 +1185,24 @@ def about_object_to_list_of_dictionary(abouts):
         ad = about.as_dict()
         # Update the 'about_resource' field with the relative path
         # from the output location
-        if 'about_file_path' in ad.keys():
-            afp = ad['about_file_path']
-            afp_parent = posixpath.dirname(afp)
-            afp_parent = '/' + afp_parent if not afp_parent.startswith('/') else afp_parent
-            about_resource = ad['about_resource']
-            for resource in about_resource:
-                updated_about_resource = posixpath.join(afp_parent, resource)
-            ad['about_resource'] = OrderedDict([(updated_about_resource, None)])
-            del ad['about_file_path']
-        serialized.append(ad)
+        try:
+            if ad['about_resource']:
+                if 'about_file_path' in ad.keys():
+                    afp = ad['about_file_path']
+                    afp_parent = posixpath.dirname(afp)
+                    afp_parent = '/' + afp_parent if not afp_parent.startswith('/') else afp_parent
+                    about_resource = ad['about_resource']
+                    for resource in about_resource:
+                        updated_about_resource = posixpath.normpath(posixpath.join(afp_parent, resource))
+                        if resource == u'.':
+                            updated_about_resource = updated_about_resource + '/'
+                    ad['about_resource'] = OrderedDict([(updated_about_resource, None)])
+                    del ad['about_file_path']
+                serialized.append(ad)
+        except Exception as e:
+            # The missing required field, about_resource, has already been checked
+            # and the error has already been logged.
+            pass
     return serialized
 
 

tests/test_cmd.py

@@ -2,7 +2,7 @@
 # -*- coding: utf8 -*-
 
 # ============================================================================
-#  Copyright (c) 2014-2017 nexB Inc. http://www.nexb.com/ - All rights reserved.
+#  Copyright (c) 2014-2019 nexB Inc. http://www.nexb.com/ - All rights reserved.
 #  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

tests/test_gen.py

@@ -2,7 +2,7 @@
 # -*- coding: utf8 -*-
 
 # ============================================================================
-#  Copyright (c) 2014-2017 nexB Inc. http://www.nexb.com/ - All rights reserved.
+#  Copyright (c) 2014-2019 nexB Inc. http://www.nexb.com/ - All rights reserved.
 #  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
@@ -45,15 +45,15 @@ def test_check_duplicated_columns_handles_lower_upper_case(self):
         result = gen.check_duplicated_columns(test_file)
         assert expected == result
 
-    def test_check_duplicated_about_file_path(self):
+    def test_check_duplicated_about_resource(self):
         test_dict = [
-            {'about_file_path': '/test/test.c', 'version': '1.03', 'name': 'test.c'},
-            {'about_file_path': '/test/abc/', 'version': '1.0', 'name': 'abc'},
-            {'about_file_path': '/test/test.c', 'version': '1.04', 'name': 'test1.c'}]
+            {'about_resource': '/test/test.c', 'version': '1.03', 'name': 'test.c'},
+            {'about_resource': '/test/abc/', 'version': '1.0', 'name': 'abc'},
+            {'about_resource': '/test/test.c', 'version': '1.04', 'name': 'test1.c'}]
         expected = [
             Error(CRITICAL,
-                  "The input has duplicated values in 'about_file_path' field: /test/test.c")]
-        result = gen.check_duplicated_about_file_path(test_dict)
+                  "The input has duplicated values in 'about_resource' field: /test/test.c")]
+        result = gen.check_duplicated_about_resource(test_dict)
         assert expected == result
 
     def test_load_inventory(self):

tests/test_model.py

@@ -609,11 +609,10 @@ def test_get_field_names_only_returns_non_empties(self):
         b.custom_fields['g'] = model.StringField(
             name='g', value='1', present=True)
         abouts = [a, b]
-        # ensure that custom fields and about file path are collected
-        # and that all fields are in the correct order
+        # ensure all fields (including custom fields) and
+        # about_resource are collected in the correct order
         expected = [
-            model.About.ABOUT_FILE_PATH_ATTR,
-            'about_resource', 'name', 'f', 'g'
+            model.About.ABOUT_RESOURCE_ATTR, 'name', 'f', 'g'
         ]
         result = model.get_field_names(abouts)
         assert expected == result
@@ -630,11 +629,9 @@ def test_get_field_names_does_not_return_duplicates_custom_fields(self):
         b.custom_fields['cf'] = model.StringField(name='cf', value='2',
                                                  present=True)
         abouts = [a, b]
-        # ensure that custom fields and about file path are collected
-        # and that all fields are in the correct order
-        # FIXME: this is not USED
+        # ensure all fields (including custom fields) and
+        # about_resource are collected in the correct order
         expected = [
-            'about_file_path',
             'about_resource',
             'name',
             'cf',
@@ -1069,9 +1066,6 @@ def test_collect_inventory_with_no_about_resource_from_directory(self):
         expected_errors = [Error(CRITICAL, 'about/about.ABOUT: Field about_resource is required')]
         assert expected_errors == errors
 
-        expected = get_test_loc('test_model/inventory/no_about_resource_key/expected.csv')
-        check_csv(expected, result)
-
     def test_collect_inventory_complex_from_directory(self):
         location = get_test_loc('test_model/inventory/complex')
         result = get_temp_file()

tests/testdata/test_cmd/geninventory.csv

@@ -1,23 +1,23 @@
-"about_file_path","name","version"
-"PyJWT-1.6.4-py2.py3-none-any.whl","PyJWT","1.6.4"
-"pyldap-2.4.45.tar.gz","pyldap","2.4.45"
-"xmlcatmgr-2.2_2.txz","xmlcatmgr","v 2.2_2"
-"sdocbook-xml-1.1_2.2.txz","sdocbook-xml","v 1.1_2,2"
-"Pygments-2.2.0-py2.py3-none-any.whl","Pygments","2.2.0"
-"ruby22-gems-2.6.4.txz","ruby22-gems","v 2.6.4"
-"xorg-macros-1.19.0.txz","xorg-macros","v 1.19.0"
-"samba42-4.2.14.txz","samba42","v 4.2.14"
-"xmlto-0.0.26_2.txz","xmlto","v 0.0.26_2"
-"xtrans-1.3.5.txz","xtrans","v 1.3.5"
-"w3m-0.5.3_4.txz","w3m","v 0.5.3_4"
-"pyaml-17.12.1-py2.py3-none-any.whl","pyaml","17.12.1"
-"siproxd-0.8.1.txz","siproxd","v 0.8.1"
-"xcb-util-0.4.0_1.1.txz","xcb-util","v 0.4.0_1,1"
-"requests-2.17.3-py2.py3-none-any.whl","requests","2.17.3"
-"setuptools-36.6.0-py2.py3-none-any.whl","setuptools","36.6.0"
-"ruby-2.2.5_1.1.txz","ruby","v 2.2.5_1,1"
-"packageurl_python-0.8.1-py2.py3-none-any.whl","packageurl-python","0.8.1"
-"certifi-2017.4.17-py2.py3-none-any.whl","certifi","2017.4.17"
-"psycopg2-2.7.5.tar.gz","psycopg2","2.7.5"
-"xorg-fonts-truetype-7.7_1.txz","xorg-fonts-truetype","v 7.7_1"
-"xerces-c3-3.1.4.txz","xerces-c3","v 3.1.4"
+about_resource,name,version
+PyJWT-1.6.4-py2.py3-none-any.whl,PyJWT,1.6.4
+pyldap-2.4.45.tar.gz,pyldap,2.4.45
+xmlcatmgr-2.2_2.txz,xmlcatmgr,v 2.2_2
+sdocbook-xml-1.1_2.2.txz,sdocbook-xml,"v 1.1_2,2"
+Pygments-2.2.0-py2.py3-none-any.whl,Pygments,2.2.0
+ruby22-gems-2.6.4.txz,ruby22-gems,v 2.6.4
+xorg-macros-1.19.0.txz,xorg-macros,v 1.19.0
+samba42-4.2.14.txz,samba42,v 4.2.14
+xmlto-0.0.26_2.txz,xmlto,v 0.0.26_2
+xtrans-1.3.5.txz,xtrans,v 1.3.5
+w3m-0.5.3_4.txz,w3m,v 0.5.3_4
+pyaml-17.12.1-py2.py3-none-any.whl,pyaml,17.12.1
+siproxd-0.8.1.txz,siproxd,v 0.8.1
+xcb-util-0.4.0_1.1.txz,xcb-util,"v 0.4.0_1,1"
+requests-2.17.3-py2.py3-none-any.whl,requests,2.17.3
+setuptools-36.6.0-py2.py3-none-any.whl,setuptools,36.6.0
+ruby-2.2.5_1.1.txz,ruby,"v 2.2.5_1,1"
+packageurl_python-0.8.1-py2.py3-none-any.whl,packageurl-python,0.8.1
+certifi-2017.4.17-py2.py3-none-any.whl,certifi,2017.4.17
+psycopg2-2.7.5.tar.gz,psycopg2,2.7.5
+xorg-fonts-truetype-7.7_1.txz,xorg-fonts-truetype,v 7.7_1
+xerces-c3-3.1.4.txz,xerces-c3,v 3.1.4

tests/testdata/test_gen/inv.csv

@@ -1,4 +1,4 @@
-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_changes,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,custom1
-inv/this.ABOUT,.,AboutCode,0.11.0,,"multi
+about_resource,name,version,download_url,description,homepage_url,notes,license_name,license_file,license_url,copyright,notice_file,notice_url,redistribute,attribute,track_changes,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,custom1
+/inv/,AboutCode,0.11.0,,"multi
 line",,,,,,,,,,,,,,,,,,,,,,,,,,,"multi
 line"

tests/testdata/test_gen/inv2.csv

@@ -1,2 +1,2 @@
-about_file_path,name,version
-inv/,AboutCode,0.11.0
+about_resource,name,version
+/inv/,AboutCode,0.11.0

tests/testdata/test_gen/inv3.csv

@@ -1,2 +1,2 @@
-about_file_path,name,version
+about_resource,name,version
 inv/test.tar.gz,AboutCode,0.11.0

tests/testdata/test_gen/inv4.csv

@@ -1,3 +1,3 @@
-about_file_path,about_resource,name,version,description,Confirmed Copyright,Resource,test
-inv/this.ABOUT,.,AboutCode,0.11.0,"multi
+about_resource,name,version,description,Confirmed Copyright,Resource,test
+inv/,AboutCode,0.11.0,"multi
 line","Copyright (c) nexB, Inc.",this.ABOUT,This is a test