Fixed #131 - 'file_name' is not needed anymore (aka 'about_resource'), the code will get the 'about_resource' from the 'about_file'

Author: chinyeungli

about_code_tool/MAPPING.CONFIG

@@ -21,18 +21,15 @@
 #   Assuming your input have columns "Resource", "Component", "file_name", "file_version"
 #   One should do:
 #
-#   about_resource: file_name
 #   about_file: Resource
 #   name: Component
 #   version: file_version
 #
 #   See http://www.dejacode.org/about_spec_v0.8.1.html for more information
 
 # Essential Fields
-about_resource: file_name
 about_file: Directory/Filename
 
-
 # Mandatory Fields
 name: Component
 version: Confirmed Version

about_code_tool/about.py

@@ -88,12 +88,15 @@ def repr_problem(obj):
 GENATTRIB = 'Attribution generation problem'
 
 MANDATORY_FIELDS = (
-    'about_resource',
+    #'about_resource',
+    #'about_file',
     'name',
     'version',
 )
 
 BASIC_FIELDS = (
+    'about_resource',
+    'about_resource_path', # Need to update spec
     'spec_version',
     'date',
     'description',

about_code_tool/genabout.py

@@ -23,7 +23,7 @@
 
 from collections import namedtuple
 from os import makedirs
-from os.path import exists, dirname, join, abspath, isdir, normpath
+from os.path import exists, dirname, join, abspath, isdir, normpath, basename
 
 import about
 
@@ -54,7 +54,7 @@
 logger.addHandler(handler)
 file_logger = logging.getLogger(__name__+'_file')
 
-ESSENTIAL_FIELDS = ('about_file', 'about_resource',)
+ESSENTIAL_FIELDS = ('about_file',)
 
 # The 'dje_license_key' will be removed and will use the 'dje_license' instead.
 SUPPORTED_FIELDS = about.OPTIONAL_FIELDS + about.MANDATORY_FIELDS + \
@@ -172,7 +172,7 @@ def validate_value_in_essential_fields(input_list):
     def validate_duplication(input_list):
         check_duplication = []
         for line in input_list:
-            component = line['about_file'] + line['about_resource']
+            component = line['about_file']
             if component in check_duplication:
                 return True
             check_duplication.append(component)
@@ -453,23 +453,38 @@ def pre_generation(self, gen_location, input_list, action_num, all_in_one):
         for line in input_list:
             component_list = []
             file_location = line['about_file']
+            # TODO: The following few line of code seems to change the value
+            # without checking the action num which is incorrect.
+            # Get the filename from the file_location and put it as the
+            # value for 'about_resource'
+            
             if file_location.startswith('/'):
                 file_location = file_location.partition('/')[2]
             if not file_location.endswith('.ABOUT'):
                 if file_location.endswith('/'):
                     file_location = dirname(file_location)
-                    file_location = join(file_location, os.path.basename(file_location))
+                    file_location = join(file_location, basename(file_location))
+                file_location += '.ABOUT'
+
+            """line['about_resource'] = basename(file_location)
+            if not file_location.startswith('/'):
+                line['about_resource_path'] = '/' + file_location
+            else:
+                line['about_resource_path'] = file_location
+                # Strip the first '/' for the later 'join'
+                file_location = file_location.partition('/')[2]
+            if not file_location.endswith('.ABOUT'):
+                if file_location.endswith('/'):
+                    file_location = dirname(file_location)
+                    file_location = join(file_location, basename(file_location))
                     # Since this is referencing everything in the current directory,
                     # we will use a '.' period to reference it.
                     line['about_resource'] = '.'
-                file_location += '.ABOUT'
+                file_location += '.ABOUT'"""
             if all_in_one:
                 # This is to get the filename instead of the file path
                 file_location = file_location.rpartition('/')[2]
             about_file_location = join(gen_location, file_location)
-            if not file_location.startswith('/'):
-                line['about_resource_path'] = '/'
-            line['about_resource_path'] += file_location
             dir = dirname(about_file_location)
             if not _exists(dir):
                 makedirs(dir)
@@ -491,13 +506,47 @@ def pre_generation(self, gen_location, input_list, action_num, all_in_one):
                     for field_name, value in about_object.parsed.items():
                         field_name = field_name.lower()
                         line[field_name] = value
-                # We don't need to do anything for the action_num = 3 as
-                # the original ABOUT file will be replaced in the write_output()
+                # We do not need to do anything if action_num is 3 as the 
+                # original ABOUT file will be replaced in the write_output()
+                elif action_num == 3:
+                    pass
+            # The following is to ensure the 'about_resource' and 
+            # 'about_resource_path' present. If those are existed already,
+            # the code will not touch it.
+            self.update_about_resource(line)
+            self.update_about_resource_path(line)
+
             component_list.append(about_file_location)
             component_list.append(line)
             output_list.append(component_list)
         return output_list
 
+    def update_about_resource(self, line):
+        # Check is 'about_resource' exist
+        try:
+            if line['about_resource']:
+                # Do nothing
+                pass
+        except:
+            # Add the 'about_resource' field
+            about_resource = line['about_file']
+            if about_resource.endswith('/'):
+                line['about_resource'] = '.'
+            else:
+                line['about_resource'] = basename(about_resource)
+
+    def update_about_resource_path(self, line):
+        # Check is 'about_resource_path' exist
+        try:
+            if line['about_resoure_path']:
+                pass
+        except:
+            file_path = line['about_file']
+            if not file_path.startswith('/'):
+                line['about_resource_path'] = '/' + file_path
+            else:
+                line['about_resource_path'] = file_path
+
     @staticmethod
     def gen_license_list(line):
         dje_license_name = line['dje_license_name']
@@ -527,7 +576,8 @@ def format_output(input_list):
                     # The purpose of the replace('\n', '\n ') is used to
                     # format the continuation strings
                     value = about_dict_list[item].replace('\n', '\n ')
-                    if (value or item in about.MANDATORY_FIELDS) and not item in about.ERROR_WARN_FIELDS:
+                    if (value or item in about.MANDATORY_FIELDS) and not item\
+                        in about.ERROR_WARN_FIELDS and not item == 'about_resource':
                         context += item + ': ' + value + '\n'
 
             component.append(about_file_location)

about_code_tool/tests/test_about.py

@@ -84,15 +84,15 @@ def test_return_path_is_not_abspath_and_contains_subdirs_on_dir(self):
             self.assertTrue(f.read().partition('\n')[2].startswith(expected))
 
     def test_header_row_in_csv_output(self):
-        expected_header = 'about_file,about_resource,name,version,' \
-        'spec_version,date,description,description_file,home_url,' \
-        'download_url,readme,readme_file,install,install_file,changelog,' \
-        'changelog_file,news,news_file,news_url,notes,notes_file,contact,' \
-        'owner,author,author_file,copyright,copyright_file,notice,' \
-        'notice_file,notice_url,license_text,license_text_file,license_url,' \
-        'license_spdx,redistribute,attribute,track_changes,vcs_tool,' \
-        'vcs_repository,vcs_path,vcs_tag,vcs_branch,vcs_revision,' \
-        'checksum_sha1,checksum_md5,checksum_sha256,dje_component,' \
+        expected_header = 'about_file,name,version,about_resource,'\
+        'about_resource_path,spec_version,date,description,description_file,'\
+        'home_url,download_url,readme,readme_file,install,install_file,changelog,'\
+        'changelog_file,news,news_file,news_url,notes,notes_file,contact,'\
+        'owner,author,author_file,copyright,copyright_file,notice,'\
+        'notice_file,notice_url,license_text,license_text_file,license_url,'\
+        'license_spdx,redistribute,attribute,track_changes,vcs_tool,'\
+        'vcs_repository,vcs_path,vcs_tag,vcs_branch,vcs_revision,'\
+        'checksum_sha1,checksum_md5,checksum_sha256,dje_component,'\
         'dje_license,dje_organization,dje_license_name,warnings,errors'
 
         input = "about_code_tool/tests/testdata/basic"
@@ -323,11 +323,12 @@ def test_validate_about_ref_testing_the_about_resource_field_is_present(self):
 
     def test_validate_about_ref_no_about_ref_key(self):
         about_file = about.AboutFile(join(TESTDATA_PATH, 'parser_tests/.ABOUT'))
-        expected_errors = [about.VALUE, 'about_resource']
-        self.assertTrue(len(about_file.errors) == 1, "This should throw 1 error")
-        for w in about_file.errors:
+        # We do not need 'about_resource' now, so no error should be thrown.
+        # expected_errors = [about.VALUE, 'about_resource']
+        self.assertTrue(len(about_file.errors) == 0, "No error should be thrown.")
+        """for w in about_file.errors:
             self.assertEqual(expected_errors[0], w.code)
-            self.assertEqual(expected_errors[1], w.field_name)
+            self.assertEqual(expected_errors[1], w.field_name)"""
 
     def test_validate_about_resource_error_thrown_when_file_referenced_by_about_file_does_not_exist(self):
         about_file = about.AboutFile(join(TESTDATA_PATH, 'parser_tests/missing_about_ref.ABOUT'))
@@ -339,19 +340,17 @@ def test_validate_about_resource_error_thrown_when_file_referenced_by_about_file
 
     def test_validate_mand_fields_name_and_version_and_about_resource_present(self):
         about_file = about.AboutFile(join(TESTDATA_PATH, 'parser_tests/missing_mand.ABOUT'))
-        expected_errors = [(about.VALUE, 'about_resource'),
-                           (about.VALUE, 'name'),
+        expected_errors = [(about.VALUE, 'name'),
                            (about.VALUE, 'version'), ]
-        self.assertTrue(len(about_file.errors) == 3, "This should throw 3 errors")
+        self.assertTrue(len(about_file.errors) == 2, "This should throw 2 errors.")
         for i, w in enumerate(about_file.errors):
             self.assertEqual(expected_errors[i][0], w.code)
             self.assertEqual(expected_errors[i][1], w.field_name)
 
         about_file = about.AboutFile(join(TESTDATA_PATH, 'parser_tests/missing_mand_values.ABOUT'))
         expected_errors = [(about.VALUE, 'name'),
-                             (about.VALUE, 'version'),
-                             (about.VALUE, 'about_resource')]
-        self.assertTrue(len(about_file.errors) == 3, "This should throw 3 errors")
+                             (about.VALUE, 'version')]
+        self.assertTrue(len(about_file.errors) == 2, "This should throw 2 errors.")
         for i, w in enumerate(about_file.errors):
             self.assertEqual(expected_errors[i][0], w.code)
             self.assertEqual(expected_errors[i][1], w.field_name)

about_code_tool/tests/test_genabout.py

@@ -63,7 +63,6 @@ def test_get_mapping_list(self):
         gen = genabout.GenAbout()
         expected_list = {'about_file': 'directory/filename',
                           'version': 'confirmed version',
-                           'about_resource': 'file_name',
                            'name': 'component',
                            'copyright': 'confirmed copyright'}
         output = gen.get_mapping_list()
@@ -93,7 +92,9 @@ def test_validate_value_in_essential_missing_about_resource(self):
         gen = genabout.GenAbout()
         input = [{'about_file': '/about.ABOUT', 'about_resource': '',
                        'name': 'ABOUT tool', 'version': '0.8.1'}]
-        self.assertFalse(gen.validate_value_in_essential_fields(input))
+        #self.assertFalse(gen.validate_value_in_essential_fields(input))
+        # This is now true as it doesn't depends on about_resource now
+        self.assertTrue(gen.validate_value_in_essential_fields(input))
 
     def test_validate_value_in_essential_missing_all(self):
         gen = genabout.GenAbout()
@@ -151,7 +152,9 @@ def test_validate_mandatory_fields_missing_about_resource(self):
         gen = genabout.GenAbout()
         input_list = [{'about_file': '/about.ABOUT', 'name': 'ABOUT tool',
                        'version': '0.8.1'}]
-        self.assertFalse(gen.validate_mandatory_fields(input_list))
+        #self.assertFalse(gen.validate_mandatory_fields(input_list))
+        # This is now True as it doesn't need about_resource
+        self.assertTrue(gen.validate_mandatory_fields(input_list))
 
     def test_get_non_supported_fields(self):
         gen = genabout.GenAbout()
@@ -299,7 +302,7 @@ def test_pre_generation_about_is_dir_exists_action_0(self):
                         'about_resource': '.', 'name': 'ABOUT tool'}]
         expected_output_list = [[join(TESTDATA_PATH, 'test_files_for_genabout/TESTCASE', 'TESTCASE.ABOUT'),
                                  {'about_file': '/TESTCASE/', 'version': '0.8.1',
-                                  'about_resource_path' : '/TESTCASE/TESTCASE.ABOUT',
+                                  'about_resource_path' : '/TESTCASE/',
                                   'about_resource': '.', 'name': 'ABOUT tool'}]]
         output_list = gen.pre_generation(gen_location, input_list, action_num, False)
         self.assertTrue(expected_output_list == output_list)
@@ -525,3 +528,43 @@ def test_process_dje_licenses(self):
         expected_output = [[join(u'/test', 'test_key.LICENSE'), 'This is a test license.']]
         output = gen.process_dje_licenses(test_license_list, test_license_dict, test_path)
         self.assertTrue(output == expected_output)
+
+    def test_update_about_resource_exist(self):
+        gen = genabout.GenAbout()
+        input_dict = {'about_resource': 'test.c', 'about_file': '/tmp/test.c'}
+        gen.update_about_resource(input_dict)
+        self.assertTrue(input_dict == input_dict, "The dict should not be changed.")
+
+    def test_update_about_resource_not_exist_isFile(self):
+        gen = genabout.GenAbout()
+        input_dict = {'about_file': '/tmp/test.c'}
+        expected_output = {'about_file': '/tmp/test.c', 'about_resource': 'test.c'}
+        gen.update_about_resource(input_dict)
+        self.assertTrue(input_dict == expected_output)
+
+    def test_update_about_resource_not_exist_isdir(self):
+        gen = genabout.GenAbout()
+        input_dict = {'about_file': '/tmp/test/'}
+        expected_output = {'about_file': '/tmp/test/', 'about_resource': '.'}
+        gen.update_about_resource(input_dict)
+        self.assertTrue(input_dict == expected_output)
+
+    def test_update_about_resource_path_exist(self):
+        gen = genabout.GenAbout()
+        input_dict = {'about_resource_path': '/tmp/test.c', 'about_file': '/tmp/test.c'}
+        gen.update_about_resource_path(input_dict)
+        self.assertTrue(input_dict == input_dict, "The dict should not be changed.")
+
+    def test_update_about_resource_path_not_exist_isFile(self):
+        gen = genabout.GenAbout()
+        input_dict = {'about_file': '/tmp/test.c'}
+        expected_output = {'about_file': '/tmp/test.c', 'about_resource_path': '/tmp/test.c'}
+        gen.update_about_resource_path(input_dict)
+        self.assertTrue(input_dict == expected_output)
+
+    def test_update_about_resource_path_not_exist_isDir(self):
+        gen = genabout.GenAbout()
+        input_dict = {'about_file': '/tmp/test/'}
+        expected_output = {'about_file': '/tmp/test/', 'about_resource_path': '/tmp/test/'}
+        gen.update_about_resource_path(input_dict)
+        self.assertTrue(input_dict == expected_output)