Repository: vmware/tern
Pull request #560 (2020-02-28 12:25:06)

classes: Add FileData object list to package class
 This PR adds files property to package class. It adds add_file, remove_file, get_file_paths methods. Tests have been added for the changes made. The to_dict method has also been changed to account for the new files property

Closes https://github.com/vmware/tern/issues/559

Signed-off-by: PrajwalM2212 <prajwalmmath@gmail.com>

nishakm: @PrajwalM2212 I asked for a small change in the code. Other than that, it looks good to me.
PrajwalM2212: @nishakm Changes made

Patch:
diff --git a/tern/classes/package.py b/tern/classes/package.py
index 390cd4f..96afa34 100644
--- a/tern/classes/package.py
+++ b/tern/classes/package.py
@@ -2,7 +2,7 @@
 #
 # Copyright (c) 2017-2019 VMware, Inc. All Rights Reserved.
 # SPDX-License-Identifier: BSD-2-Clause
-
+from tern.classes.file_data import FileData
 from tern.classes.notice import Notice
 from tern.classes.origins import Origins
 from tern.utils.general import prop_names
@@ -19,11 +19,16 @@ class Package:
         download_url: package download url
         origins: a list of NoticeOrigin objects
         checksum: checksum as package property
+        files: list of files in a package
 
     methods:
         to_dict: returns a dict representation of the instance
         fill: instantiates a package object given a dict representation.
-        is_equal: compares two package objects.'''
+        is_equal: compares two package objects.
+        get_file_paths: Returns a list of file paths
+        add_file: Add a file to list of files
+        remove_file: Removes a file from a list of files
+        '''
     def __init__(self, name):
         self.__name = name
         self.__version = ''
@@ -33,11 +38,16 @@ class Package:
         self.__download_url = ''
         self.__checksum = ''
         self.__origins = Origins()
+        self.__files = []
 
     @property
     def name(self):
         return self.__name
 
+    @property
+    def files(self):
+        return self.__files
+
     @property
     def version(self):
         return self.__version
@@ -90,11 +100,33 @@ class Package:
     def checksum(self, checksum):
         self.__checksum = checksum
 
+    def get_file_paths(self):
+        """Return a list of paths of all the files in a package"""
+        return [f.path for f in self.__files]
+
+    def add_file(self, file):
+        """Add a file to the list of files present in a package"""
+        if isinstance(file, FileData):
+            if file.path not in self.get_file_paths():
+                self.files.append(file)
+        else:
+            raise TypeError('Object type is {0}, should be FileData'.format(
+                type(file)))
+
+    def remove_file(self, file_path):
+        """Remove a file from the list of files present in a package"""
+        for file in self.__files:
+            if file.path == file_path:
+                self.__files.remove(file)
+                return True
+        return False
+
     def to_dict(self, template=None):
         '''Return a dictionary version of the Package object
         If given an object which is a subclass of Template then map
         the keys to the Package class properties'''
         pkg_dict = {}
+        file_list = [f.to_dict(template) for f in self.__files]
         if template:
             # loop through object properties
             for key, prop in prop_names(self):
@@ -106,11 +138,16 @@ class Package:
             if 'origins' in template.package().keys():
                 pkg_dict.update(
                     {template.package()['origins']: self.origins.to_dict()})
+            # update the 'files' part if it exists in the mapping
+            if 'files' in template.package().keys():
+                pkg_dict.update(
+                    {template.package()['files']: file_list})
         else:
             # don't map, just use the property name as the key
             for key, prop in prop_names(self):
                 pkg_dict.update({prop: self.__dict__[key]})
             pkg_dict.update({'origins': self.origins.to_dict()})
+            pkg_dict.update({'files': file_list})
         return pkg_dict
 
     def __fill_properties(self, package_dict):


Test patch:
diff --git a/tests/test_class_package.py b/tests/test_class_package.py
index f3aa5d1..6509ad0 100644
--- a/tests/test_class_package.py
+++ b/tests/test_class_package.py
@@ -5,6 +5,7 @@
 
 import unittest
 
+from tern.classes.file_data import FileData
 from tern.classes.package import Package
 from test_fixtures import TestTemplate1
 from test_fixtures import TestTemplate2
@@ -60,7 +61,42 @@ class TestClassPackage(unittest.TestCase):
         self.assertEqual(self.p1.download_url, 'https://github.com')
         self.assertEqual(self.p1.checksum, '123abc456')
 
+    def testAddFile(self):
+        p1 = Package('package')
+        f1 = FileData('test.java', 'abc/pqr/test.java')
+        f2 = FileData('test.c', 'abc/pqr/test.c')
+        p1.add_file(f1)
+        p1.add_file(f2)
+        self.assertEqual(len(p1.files), 2)
+        with self.assertRaises(TypeError):
+            p1.add_file('sample.cpp')
+
+    def testRemoveFile(self):
+        p1 = Package('package')
+        f1 = FileData('test.java', 'abc/pqr/test.java')
+        f2 = FileData('test.c', 'abc/pqr/test.c')
+        p1.add_file(f1)
+        p1.add_file(f2)
+        self.assertTrue(p1.remove_file('abc/pqr/test.java'))
+        self.assertEqual(len(p1.files), 1)
+        self.assertTrue(p1.remove_file('abc/pqr/test.c'))
+        self.assertEqual(len(p1.files), 0)
+        self.assertFalse(p1.remove_file('abc'))
+
+    def testGetFilePaths(self):
+        p1 = Package('package')
+        f1 = FileData('test.java', 'abc/pqr/test.java')
+        f2 = FileData('test.c', 'abc/pqr/test.c')
+        p1.add_file(f1)
+        p1.add_file(f2)
+        self.assertEqual(p1.get_file_paths(),
+                         ['abc/pqr/test.java',
+                          'abc/pqr/test.c'
+                          ])
+
     def testToDict(self):
+        f1 = FileData('test.java', 'abc/pqr/test.java')
+        self.p1.add_file(f1)
         a_dict = self.p1.to_dict()
         self.assertEqual(a_dict['name'], 'p1')
         self.assertEqual(a_dict['version'], '1.0')
@@ -70,17 +106,25 @@ class TestClassPackage(unittest.TestCase):
         self.assertEqual(a_dict['download_url'], 'https://github.com')
         self.assertFalse(a_dict['origins'])
         self.assertEqual(a_dict['checksum'], '123abc456')
+        self.assertEqual(a_dict['files'][0]['name'], 'test.java')
+        self.assertEqual(a_dict['files'][0]['path'], 'abc/pqr/test.java')
 
     def testToDictTemplate(self):
         template1 = TestTemplate1()
         template2 = TestTemplate2()
+        f1 = FileData('test.java', 'abc/pqr/test.java')
+        self.p1.add_file(f1)
         dict1 = self.p1.to_dict(template1)
         dict2 = self.p1.to_dict(template2)
-        self.assertEqual(len(dict1.keys()), 3)
+        self.assertEqual(len(dict1.keys()), 4)
         self.assertEqual(dict1['package.name'], 'p1')
+        self.assertEqual(dict1['package.files'][0]['file.name'],
+                         'test.java')
         self.assertEqual(dict1['package.version'], '1.0')
         self.assertEqual(dict1['package.license'], 'Apache 2.0')
-        self.assertEqual(len(dict2.keys()), 5)
+        self.assertEqual(dict1['package.files'][0]['file.path'],
+                         'abc/pqr/test.java')
+        self.assertEqual(len(dict2.keys()), 6)
         self.assertFalse(dict2['notes'])
 
     def testIsEqual(self):
@@ -112,7 +156,7 @@ class TestClassPackage(unittest.TestCase):
         self.assertFalse(p.proj_url)
         self.assertEqual(len(p.origins.origins), 1)
         self.assertEqual(p.origins.origins[0].origin_str, 'p1')
-        self.assertEqual(len(p.origins.origins[0].notices), 3)
+        self.assertEqual(len(p.origins.origins[0].notices), 4)
         self.assertEqual(p.origins.origins[0].notices[0].message,
                          "No metadata for key: copyright")
         self.assertEqual(p.origins.origins[0].notices[0].level, 'warning')
diff --git a/tests/test_fixtures.py b/tests/test_fixtures.py
index e43bd78..5c48bdd 100644
--- a/tests/test_fixtures.py
+++ b/tests/test_fixtures.py
@@ -36,7 +36,8 @@ class TestTemplate1(Template):
     def package(self):
         return {'name': 'package.name',
                 'version': 'package.version',
-                'pkg_license': 'package.license'}
+                'pkg_license': 'package.license',
+                'files': 'package.files'}
 
     def image_layer(self):
         return {'diff_id': 'layer.diff',
@@ -63,7 +64,8 @@ class TestTemplate2(Template):
         mapping = {'name': 'package.name',
                    'version': 'package.version',
                    'pkg_license': 'package.license',
-                   'proj_url': 'package.url'}
+                   'proj_url': 'package.url',
+                   'files': 'package.files'}
         # we update the mapping with another defined mapping
         mapping.update(self.origins())
         return mapping