Fix package function update with same md5

Do not update or delete the function package if updating the function
with the same package but not providing the md5.

Change-Id: Iea24978106984111b61b0b19336cf1eb9ded9ed6
Story: 2003482
Task: 24749

qinling/api/controllers/v1/function.py

@@ -224,7 +224,7 @@ class FunctionsController(rest.RestController):
             if store:
                 try:
                     ctx = context.get_ctx()
-                    actual_md5 = self.storage_provider.store(
+                    _, actual_md5 = self.storage_provider.store(
                         ctx.projectid, func_db.id, data, md5sum=md5sum
                     )
                     values['code'].update({"md5sum": actual_md5})
@@ -394,7 +394,7 @@ class FunctionsController(rest.RestController):
 
                 # Package type function. 'code' and 'entry' make sense only if
                 # 'package' is provided
-                update_package = False
+                package_updated = False
                 if (pre_source == constants.PACKAGE_FUNCTION and
                         values.get('package') is not None):
                     if md5sum and md5sum == pre_md5sum:
@@ -404,7 +404,7 @@ class FunctionsController(rest.RestController):
 
                     # Update the package data.
                     data = values['package'].file.read()
-                    md5sum = self.storage_provider.store(
+                    package_updated, md5sum = self.storage_provider.store(
                         ctx.projectid,
                         id,
                         data,
@@ -414,7 +414,6 @@ class FunctionsController(rest.RestController):
                         {"md5sum": md5sum, "source": pre_source}
                     )
                     values.pop('package')
-                    update_package = True
 
                 # Swift type function
                 if pre_source == constants.SWIFT_FUNCTION:
@@ -429,7 +428,7 @@ class FunctionsController(rest.RestController):
                 func_db = db_api.update_function(id, values)
 
             # Delete the old function package if needed
-            if update_package:
+            if package_updated:
                 self.storage_provider.delete(ctx.projectid, id, pre_md5sum)
 
         pecan.response.status = 200

qinling/storage/base.py

@@ -35,7 +35,7 @@ class PackageStorage(object):
         :param data: Package file content.
         :param kwargs: A dict may including
             - md5sum: The MD5 provided by the user.
-        :return: MD5 value of the package.
+        :return: A tuple (if the package is updated, MD5 value of the package)
         """
         raise NotImplementedError
 

qinling/storage/file_system.py

@@ -44,7 +44,7 @@ class FileSystemStorage(base.PackageStorage):
         :param function: Function ID.
         :param data: Package file content.
         :param md5sum: The MD5 provided by the user.
-        :return: MD5 value of the package.
+        :return: A tuple (if the package is updated, MD5 value of the package)
         """
         LOG.debug(
             'Store package, function: %s, project: %s', function, project_id
@@ -58,12 +58,15 @@ class FileSystemStorage(base.PackageStorage):
         if md5sum and md5_actual != md5sum:
             raise exc.InputException("Package md5 mismatch.")
 
-        # The md5 is contained in the package path.
-        new_func_zip = os.path.join(project_path, '%s.zip.new' % function)
-        func_zip = os.path.join(project_path,
-                                PACKAGE_NAME_TEMPLATE % (function, md5_actual))
+        func_zip = os.path.join(
+            project_path,
+            PACKAGE_NAME_TEMPLATE % (function, md5_actual)
+        )
+        if os.path.exists(func_zip):
+            return False, md5_actual
 
-        # Store package
+        # Save package
+        new_func_zip = os.path.join(project_path, '%s.zip.new' % function)
         with open(new_func_zip, 'wb') as fd:
             fd.write(data)
 
@@ -72,7 +75,8 @@ class FileSystemStorage(base.PackageStorage):
             raise exc.InputException("Package is not a valid ZIP package.")
 
         os.rename(new_func_zip, func_zip)
-        return md5_actual
+
+        return True, md5_actual
 
     def retrieve(self, project_id, function, md5sum, version=0):
         """Get function package data.

qinling/tests/unit/api/controllers/v1/test_function.py

@@ -38,7 +38,7 @@ class TestFunctionController(base.APITest):
 
     @mock.patch('qinling.storage.file_system.FileSystemStorage.store')
     def test_post(self, mock_store):
-        mock_store.return_value = 'fake_md5'
+        mock_store.return_value = (True, 'fake_md5')
 
         with tempfile.NamedTemporaryFile() as f:
             body = {
@@ -184,7 +184,7 @@ class TestFunctionController(base.APITest):
     def test_put_package(self, mock_delete_func, mock_delete, mock_store,
                          mock_etcd_del):
         db_func = self.create_function(runtime_id=self.runtime_id)
-        mock_store.return_value = "fake_md5_changed"
+        mock_store.return_value = (True, "fake_md5_changed")
 
         with tempfile.NamedTemporaryFile() as f:
             resp = self.app.put(
@@ -202,7 +202,30 @@ class TestFunctionController(base.APITest):
         mock_delete.assert_called_once_with(unit_base.DEFAULT_PROJECT_ID,
                                             db_func.id, "fake_md5")
 
-    def test_put_package_same_md5(self):
+    @mock.patch('qinling.storage.file_system.FileSystemStorage.store')
+    @mock.patch('qinling.rpc.EngineClient.delete_function')
+    @mock.patch('qinling.utils.etcd_util.delete_function')
+    @mock.patch('qinling.storage.file_system.FileSystemStorage.delete')
+    def test_put_package_md5_not_change(self, file_delete_mock,
+                                        etcd_delete_mock, function_delete_mock,
+                                        store_mock):
+        db_func = self.create_function(runtime_id=self.runtime_id)
+        store_mock.return_value = (False, "fake_md5")
+
+        with tempfile.NamedTemporaryFile() as f:
+            resp = self.app.put(
+                '/v1/functions/%s' % db_func.id,
+                params={},
+                upload_files=[('package', f.name, f.read())]
+            )
+
+        self.assertEqual(200, resp.status_int)
+        self.assertEqual('fake_md5', resp.json['code'].get('md5sum'))
+        function_delete_mock.assert_called_once_with(db_func.id)
+        etcd_delete_mock.assert_called_once_with(db_func.id)
+        self.assertFalse(file_delete_mock.called)
+
+    def test_put_package_same_md5_provided(self):
         db_func = self.create_function(runtime_id=self.runtime_id)
 
         with tempfile.NamedTemporaryFile() as f:

qinling/tests/unit/storage/test_file_system.py

@@ -49,7 +49,12 @@ class TestFileSystemStorage(base.BaseTest):
         function_data = "Some data".encode('utf8')
         md5 = common.md5(content=function_data)
 
-        self.storage.store(self.project_id, function, function_data)
+        package_updated, ret_md5 = self.storage.store(
+            self.project_id, function, function_data
+        )
+
+        self.assertTrue(package_updated)
+        self.assertEqual(md5, ret_md5)
 
         temp_package_path = os.path.join(FAKE_STORAGE_PATH, self.project_id,
                                          '%s.zip.new' % function)
@@ -65,6 +70,29 @@ class TestFileSystemStorage(base.BaseTest):
         is_zipfile_mock.assert_called_once_with(temp_package_path)
         rename_mock.assert_called_once_with(temp_package_path, package_path)
 
+    @mock.patch('oslo_utils.fileutils.ensure_tree')
+    @mock.patch('os.path.exists')
+    def test_store_zip_exists(self, exists_mock, ensure_tree_mock):
+        function = self.rand_name('function', prefix='TestFileSystemStorage')
+        function_data = "Some data".encode('utf8')
+        md5 = common.md5(content=function_data)
+        exists_mock.return_value = True
+
+        package_updated, ret_md5 = self.storage.store(
+            self.project_id, function, function_data
+        )
+
+        self.assertFalse(package_updated)
+        self.assertEqual(md5, ret_md5)
+
+        package_path = os.path.join(
+            FAKE_STORAGE_PATH,
+            file_system.PACKAGE_PATH_TEMPLATE % (self.project_id, function,
+                                                 md5)
+        )
+
+        exists_mock.assert_called_once_with(package_path)
+
     @mock.patch('oslo_utils.fileutils.ensure_tree')
     def test_store_md5_mismatch(self, ensure_tree_mock):
         function = self.rand_name('function', prefix='TestFileSystemStorage')