From d347212289e0427f06a6d08097a453c3f0449e4b Mon Sep 17 00:00:00 2001 From: Ran Ziv Date: Thu, 16 Feb 2017 16:38:51 +0200 Subject: [PATCH] ARIA-108 Add API for deleting a resource for resource storage --- aria/storage/api.py | 15 ++++- aria/storage/filesystem_rapi.py | 41 ++++++++---- tests/storage/test_resource_storage.py | 90 +++++++++++++++++++++++++- 3 files changed, 129 insertions(+), 17 deletions(-) diff --git a/aria/storage/api.py b/aria/storage/api.py index 09a4dd9c..ed8a2ff7 100644 --- a/aria/storage/api.py +++ b/aria/storage/api.py @@ -135,7 +135,7 @@ def name(self): """ return self._name - def read(self, entry_id, path=None, **kwargs): + def read(self, entry_id, path, **kwargs): """ Get a bytesteam from the storage. @@ -144,7 +144,18 @@ def read(self, entry_id, path=None, **kwargs): :param kwargs: :return: """ - raise NotImplementedError('Subclass must implement abstract data method') + raise NotImplementedError('Subclass must implement abstract read method') + + def delete(self, entry_id, path, **kwargs): + """ + Delete a resource from the storage. + + :param entry_id: + :param path: + :param kwargs: + :return: + """ + raise NotImplementedError('Subclass must implement abstract delete method') def download(self, entry_id, destination, path=None, **kwargs): """ diff --git a/aria/storage/filesystem_rapi.py b/aria/storage/filesystem_rapi.py index 6693dbdf..3ddc5205 100644 --- a/aria/storage/filesystem_rapi.py +++ b/aria/storage/filesystem_rapi.py @@ -13,7 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. """ -SQLalchemy based RAPI +File system based RAPI """ import os import shutil @@ -92,14 +92,13 @@ def create(self, **kwargs): except (OSError, IOError): pass - def read(self, entry_id, path=None, **_): + def read(self, entry_id, path, **_): """ Retrieve the content of a file system storage resource. - :param str entry_type: the type of the entry. :param str entry_id: the id of the entry. - :param str path: a path to a specific resource. - :return: the content of the file + :param str path: a path to the specific resource to read. + :return: the content of the file. :rtype: bytes """ resource_relative_path = os.path.join(self.name, entry_id, path or '') @@ -110,7 +109,9 @@ def read(self, entry_id, path=None, **_): if not os.path.isfile(resource): resources = os.listdir(resource) if len(resources) != 1: - raise exceptions.StorageError('No resource in path: {0}'.format(resource)) + raise exceptions.StorageError( + 'Failed to read {0}; Reading a directory is ' + 'only allowed when it contains a single resource'.format(resource)) resource = os.path.join(resource, resources[0]) with open(resource, 'rb') as resource_file: return resource_file.read() @@ -119,10 +120,9 @@ def download(self, entry_id, destination, path=None, **_): """ Download a specific file or dir from the file system resource storage. - :param str entry_type: the name of the entry. - :param str entry_id: the id of the entry - :param str destination: the destination of the files. - :param str path: a path on the remote machine relative to the root of the entry. + :param str entry_id: the id of the entry. + :param str destination: the destination to download to + :param str path: the path to download relative to the root of the entry (otherwise all). """ resource_relative_path = os.path.join(self.name, entry_id, path or '') resource = os.path.join(self.directory, resource_relative_path) @@ -138,9 +138,8 @@ def upload(self, entry_id, source, path=None, **_): """ Uploads a specific file or dir to the file system resource storage. - :param str entry_type: the name of the entry. - :param str entry_id: the id of the entry - :param source: the source of the files to upload. + :param str entry_id: the id of the entry. + :param source: the source of the files to upload. :param path: the destination of the file/s relative to the entry root dir. """ resource_directory = os.path.join(self.directory, self.name, entry_id) @@ -151,3 +150,19 @@ def upload(self, entry_id, source, path=None, **_): shutil.copy2(source, destination) else: dir_util.copy_tree(source, destination) # pylint: disable=no-member + + def delete(self, entry_id, path=None, **_): + """ + Deletes a file system storage resource. + + :param str entry_id: the id of the entry. + :param str path: a path to delete relative to the root of the entry (otherwise all). + """ + destination = os.path.join(self.directory, self.name, entry_id, path or '') + if os.path.exists(destination): + if os.path.isfile(destination): + os.remove(destination) + else: + shutil.rmtree(destination) + return True + return False diff --git a/tests/storage/test_resource_storage.py b/tests/storage/test_resource_storage.py index 9b5f7826..4d01a887 100644 --- a/tests/storage/test_resource_storage.py +++ b/tests/storage/test_resource_storage.py @@ -111,7 +111,9 @@ def test_data_file(self): tmpfile_path = tempfile.mkstemp(suffix=self.__class__.__name__, dir=self.path)[1] self._upload(storage, tmpfile_path, 'blueprint_id') - assert storage.blueprint.read(entry_id='blueprint_id') == 'fake context' + assert storage.blueprint.read( + entry_id='blueprint_id', + path=os.path.basename(tmpfile_path)) == 'fake context' def test_upload_dir(self): storage = self._create_storage() @@ -188,4 +190,88 @@ def test_data_dir(self): storage.blueprint.upload(entry_id='blueprint_id', source=tmp_dir) with pytest.raises(exceptions.StorageError): - storage.blueprint.read(entry_id='blueprint_id') + storage.blueprint.read(entry_id='blueprint_id', path='') + + def test_delete_resource(self): + storage = self._create_storage() + self._create(storage) + tmpfile_path = tempfile.mkstemp(suffix=self.__class__.__name__, dir=self.path)[1] + self._upload(storage, tmpfile_path, 'blueprint_id') + tmpfile2_path = tempfile.mkstemp(suffix=self.__class__.__name__, dir=self.path)[1] + self._upload(storage, tmpfile2_path, 'blueprint_id') + + # deleting the first resource and expecting an error on read + storage.blueprint.delete(entry_id='blueprint_id', path=os.path.basename(tmpfile_path)) + with pytest.raises(exceptions.StorageError): + storage.blueprint.read(entry_id='blueprint_id', path=os.path.basename(tmpfile_path)) + # the second resource should still be available for reading + assert storage.blueprint.read( + entry_id='blueprint_id', + path=os.path.basename(tmpfile2_path)) == 'fake context' + + def test_delete_directory(self): + storage = self._create_storage() + self._create(storage) + temp_destination_dir = tempfile.mkdtemp(dir=self.path) + + tmp_dir = tempfile.mkdtemp(suffix=self.__class__.__name__, dir=self.path) + second_level_tmp_dir = tempfile.mkdtemp(dir=tmp_dir) + tmp_filename = tempfile.mkstemp(dir=second_level_tmp_dir)[1] + self._upload_dir(storage, tmp_dir, tmp_filename, id='blueprint_id') + file_path_in_dir = os.path.join( + os.path.basename(second_level_tmp_dir), + os.path.basename(tmp_filename)) + + # should be able to read the file and download the directory.. + assert storage.blueprint.read( + entry_id='blueprint_id', + path=file_path_in_dir) == 'fake context' + storage.blueprint.download( + entry_id='blueprint_id', + path=os.path.basename(second_level_tmp_dir), + destination=temp_destination_dir) + + # after deletion, the file and directory should both be gone + storage.blueprint.delete( + entry_id='blueprint_id', + path=os.path.basename(second_level_tmp_dir)) + with pytest.raises(exceptions.StorageError): + assert storage.blueprint.read( + entry_id='blueprint_id', + path=file_path_in_dir) == 'fake context' + with pytest.raises(exceptions.StorageError): + storage.blueprint.download( + entry_id='blueprint_id', + path=os.path.basename(second_level_tmp_dir), + destination=temp_destination_dir) + + def test_delete_all_resources(self): + storage = self._create_storage() + self._create(storage) + temp_destination_dir = tempfile.mkdtemp(dir=self.path) + + tmp_dir = tempfile.mkdtemp(suffix=self.__class__.__name__, dir=self.path) + second_level_tmp_dir = tempfile.mkdtemp(dir=tmp_dir) + tmp_filename = tempfile.mkstemp(dir=second_level_tmp_dir)[1] + self._upload_dir(storage, tmp_dir, tmp_filename, id='blueprint_id') + file_path_in_dir = os.path.join( + os.path.basename(second_level_tmp_dir), + os.path.basename(tmp_filename)) + + # deleting without specifying a path - delete all resources of this entry + storage.blueprint.delete(entry_id='blueprint_id') + with pytest.raises(exceptions.StorageError): + assert storage.blueprint.read( + entry_id='blueprint_id', + path=file_path_in_dir) == 'fake context' + with pytest.raises(exceptions.StorageError): + storage.blueprint.download( + entry_id='blueprint_id', + path=os.path.basename(second_level_tmp_dir), + destination=temp_destination_dir) + + def test_delete_nonexisting_resource(self): + storage = self._create_storage() + self._create(storage) + # deleting a nonexisting resource - no effect is expected to happen + assert storage.blueprint.delete(entry_id='blueprint_id', path='fake-file') is False