From b2dcd9faaa4b8a146790bf5f679ee7d3d2a90612 Mon Sep 17 00:00:00 2001 From: Josh Bird Date: Wed, 11 Apr 2018 15:55:14 -0400 Subject: [PATCH] Finish work on orphan overwrites --- tests/core/test_provider.py | 49 ++++--- tests/providers/box/test_provider.py | 123 +++++++++++------- tests/providers/dropbox/test_provider.py | 84 ++++++------ tests/providers/googledrive/test_provider.py | 29 ++--- tests/providers/owncloud/test_provider.py | 12 +- waterbutler/core/exceptions.py | 4 +- waterbutler/core/provider.py | 102 +++++++++------ waterbutler/providers/box/provider.py | 6 +- waterbutler/providers/googledrive/provider.py | 22 ++-- 9 files changed, 252 insertions(+), 179 deletions(-) diff --git a/tests/core/test_provider.py b/tests/core/test_provider.py index 7717d1e84..1171fc973 100644 --- a/tests/core/test_provider.py +++ b/tests/core/test_provider.py @@ -40,32 +40,44 @@ def test_can_intra_move(self, provider2): @pytest.mark.asyncio async def test_will_orphan_dest_is_file(self, provider1): - src_path = WaterButlerPath('/folder1/folder1/', - _ids=['root', 'folder', 'Folder'], - folder=True) - dest_path = WaterButlerPath('/folder1/', - _ids=['root','folder'], - folder=False) + src_path = WaterButlerPath( + '/folder1/folder1/', + _ids=['root', 'folder', 'Folder'], + folder=True + ) + dest_path = WaterButlerPath( + '/folder1/', + _ids=['root','folder'], + folder=False + ) assert provider1.replace_will_orphan(src_path, dest_path) == False @pytest.mark.asyncio async def test_will_orphan_dest_different_names(self, provider1): - src_path = WaterButlerPath('/folder1/folder1/', - _ids=['root', 'folder', 'Folder'], - folder=True) - dest_path = WaterButlerPath('/folder2/', - _ids=['root','folder'], - folder=True) + src_path = WaterButlerPath( + '/folder1/folder1/', + _ids=['root', 'folder', 'Folder'], + folder=True + ) + dest_path = WaterButlerPath( + '/folder2/', + _ids=['root','folder'], + folder=True + ) assert provider1.replace_will_orphan(src_path, dest_path) == False @pytest.mark.asyncio async def test_will_orphan_dest_different_branch(self, provider1): - src_path = WaterButlerPath('/other_folder/folder1/', - _ids=['root', 'other_folder', 'Folder'], - folder=True) - dest_path = WaterButlerPath('/folder1/', - _ids=['root','folder'], - folder=True) + src_path = WaterButlerPath( + '/other_folder/folder1/', + _ids=['root', 'other_folder', 'Folder'], + folder=True + ) + dest_path = WaterButlerPath( + '/folder1/', + _ids=['root','folder'], + folder=True + ) assert provider1.replace_will_orphan(src_path, dest_path) == False @pytest.mark.asyncio @@ -482,6 +494,7 @@ async def test_intra_move_folder_orphan(self, provider1): with pytest.raises(exceptions.OrphanSelfError) as exc: await provider1.move(provider1, src_path, dest_path) + assert exc.value.code == 400 assert exc.typename == 'OrphanSelfError' diff --git a/tests/providers/box/test_provider.py b/tests/providers/box/test_provider.py index fdee5c791..8e11aa4da 100644 --- a/tests/providers/box/test_provider.py +++ b/tests/providers/box/test_provider.py @@ -164,13 +164,17 @@ async def test_validate_path_bad_path(self, provider): async def test_validate_path(self, provider, root_provider_fixtures): provider.folder = '0' folder_id = '0' + response_body = root_provider_fixtures['revalidate_metadata'] + + good_url = provider.build_url( + 'folders', + folder_id, + 'items', + fields='id,name,type', + limit=1000 + ) - good_url = provider.build_url('folders', folder_id, 'items', - fields='id,name,type', limit=1000) - - aiohttpretty.register_json_uri('GET', good_url, - body=root_provider_fixtures['revalidate_metadata'], - status=200) + aiohttpretty.register_json_uri('GET', good_url, body=response_body, status=200) result = await provider.validate_path('/bulbasaur') assert result == WaterButlerPath('/bulbasaur', folder=False) @@ -377,22 +381,23 @@ async def test_delete_root_no_confirm(self, provider, root_provider_fixtures): @pytest.mark.aiohttpretty async def test_delete_root(self, provider, root_provider_fixtures): item = root_provider_fixtures['file_metadata']['entries'][0] + list_response_body = root_provider_fixtures['one_entry_folder_list_metadata'] path = WaterButlerPath('/newfile', _ids=(provider.folder, item['id'])) root_path = WaterButlerPath('/', _ids=('0')) - url = provider.build_url('folders', root_path.identifier, 'items', - fields='id,name,size,modified_at,etag,total_count', - offset=(0), limit=1000) - aiohttpretty.register_json_uri( - 'GET', - url, - body=root_provider_fixtures['one_entry_folder_list_metadata'] + list_url = provider.build_url( + 'folders', + root_path.identifier, + 'items', + fields='id,name,size,modified_at,etag,total_count', + offset=(0), + limit=1000 ) - url = provider.build_url('files', item['id'], fields='id,name,path_collection') delete_url = provider.build_url('files', path.identifier) - aiohttpretty.register_json_uri('get', url, - body=root_provider_fixtures['file_metadata']['entries'][0]) + + aiohttpretty.register_json_uri('GET', list_url, body=list_response_body) + aiohttpretty.register_json_uri('GET', url, body=item) aiohttpretty.register_json_uri('DELETE', delete_url, status=204) await provider.delete(root_path, 1) @@ -647,9 +652,10 @@ async def test_intra_copy_folder(self, provider, intra_fixtures, root_provider_f expected_folder = BoxFolderMetadata(item, dest_path) expected_folder._children = [] for child_item in list_metadata['entries']: - child_path = dest_path.child(child_item['name'], - folder=(child_item['type'] == 'folder')) - + child_path = dest_path.child( + child_item['name'], + folder=(child_item['type'] == 'folder') + ) serialized_child = provider._serialize_item(child_item, child_path) expected_folder._children.append(serialized_child) expected = (expected_folder, True) @@ -660,10 +666,12 @@ async def test_intra_copy_folder(self, provider, intra_fixtures, root_provider_f @pytest.mark.asyncio @pytest.mark.aiohttpretty - async def test_intra_copy_folder_replace(self, - provider, - intra_fixtures, - root_provider_fixtures): + async def test_intra_copy_folder_replace( + self, + provider, + intra_fixtures, + root_provider_fixtures + ): item = intra_fixtures['intra_folder_metadata'] list_metadata = root_provider_fixtures['folder_list_metadata'] @@ -672,9 +680,14 @@ async def test_intra_copy_folder_replace(self, file_url = provider.build_url('folders', src_path.identifier, 'copy') delete_url = provider.build_url('folders', dest_path.identifier, recursive=True) - list_url = provider.build_url('folders', item['id'], 'items', - fields='id,name,size,modified_at,etag,total_count', - offset=0, limit=1000) + list_url = provider.build_url( + 'folders', + item['id'], + 'items', + fields='id,name,size,modified_at,etag,total_count', + offset=0, + limit=1000 + ) aiohttpretty.register_json_uri('GET', list_url, body=list_metadata) aiohttpretty.register_uri('DELETE', delete_url, status=204) @@ -683,9 +696,10 @@ async def test_intra_copy_folder_replace(self, expected_folder = BoxFolderMetadata(item, dest_path) expected_folder._children = [] for child_item in list_metadata['entries']: - child_path = dest_path.child(child_item['name'], - folder=(child_item['type'] == 'folder')) - + child_path = dest_path.child( + child_item['name'], + folder=(child_item['type'] == 'folder') + ) serialized_child = provider._serialize_item(child_item, child_path) expected_folder._children.append(serialized_child) expected = (expected_folder, False) @@ -718,8 +732,10 @@ async def test_intra_move_file(self, provider, root_provider_fixtures): async def test_intra_move_file_replace(self, provider, root_provider_fixtures): item = root_provider_fixtures['file_metadata']['entries'][0] src_path = WaterButlerPath('/name.txt', _ids=(provider, item['id'])) - dest_path = WaterButlerPath('/charmander/name.txt', - _ids=(provider, item['id'], 'YgzZejrj834j')) + dest_path = WaterButlerPath( + '/charmander/name.txt', + _ids=(provider, item['id'], 'YgzZejrj834j') + ) file_url = provider.build_url('files', src_path.identifier) delete_url = provider.build_url('files', dest_path.identifier) @@ -765,21 +781,31 @@ async def test_intra_move_folder(self, provider, intra_fixtures, root_provider_f @pytest.mark.asyncio @pytest.mark.aiohttpretty - async def test_intra_move_folder_replace(self, - provider, - intra_fixtures, - root_provider_fixtures): + async def test_intra_move_folder_replace( + self, + provider, + intra_fixtures, + root_provider_fixtures + ): item = intra_fixtures['intra_folder_metadata'] list_metadata = root_provider_fixtures['folder_list_metadata'] src_path = WaterButlerPath('/name/', _ids=(provider, item['id'])) - dest_path = WaterButlerPath('/charmander/name/', _ids=(provider, item['id'], '7759994812')) + dest_path = WaterButlerPath( + '/charmander/name/', + _ids=(provider, item['id'], '7759994812') + ) file_url = provider.build_url('folders', src_path.identifier) delete_url = provider.build_url('folders', dest_path.identifier, recursive=True) - list_url = provider.build_url('folders', item['id'], 'items', - fields='id,name,size,modified_at,etag,total_count', - offset=0, limit=1000) + list_url = provider.build_url( + 'folders', + item['id'], + 'items', + fields='id,name,size,modified_at,etag,total_count', + offset=0, + limit=1000 + ) aiohttpretty.register_json_uri('PUT', file_url, body=item) aiohttpretty.register_uri('DELETE', delete_url, status=204) @@ -788,9 +814,10 @@ async def test_intra_move_folder_replace(self, expected_folder = BoxFolderMetadata(item, dest_path) expected_folder._children = [] for child_item in list_metadata['entries']: - child_path = dest_path.child(child_item['name'], - folder=(child_item['type'] == 'folder')) - + child_path = dest_path.child( + child_item['name'], + folder=(child_item['type'] == 'folder') + ) serialized_child = provider._serialize_item(child_item, child_path) expected_folder._children.append(serialized_child) expected = (expected_folder, False) @@ -875,10 +902,14 @@ async def test_returns_metadata(self, provider, root_provider_fixtures): class TestOperations: def test_will_self_overwrite(self, provider, other_provider): - src_path = WaterButlerPath('/50 shades of nope.txt', - _ids=(provider.folder, '12231')) - dest_path = WaterButlerPath('/50 shades of nope2223.txt', - _ids=(provider.folder, '2342sdfsd')) + src_path = WaterButlerPath( + '/50 shades of nope.txt', + _ids=(provider.folder, '12231') + ) + dest_path = WaterButlerPath( + '/50 shades of nope2223.txt', + _ids=(provider.folder, '2342sdfsd') + ) result = provider.will_self_overwrite(other_provider, src_path, dest_path) assert result is False diff --git a/tests/providers/dropbox/test_provider.py b/tests/providers/dropbox/test_provider.py index ccb179109..e7330bfaf 100644 --- a/tests/providers/dropbox/test_provider.py +++ b/tests/providers/dropbox/test_provider.py @@ -659,45 +659,45 @@ async def test_intra_copy_replace_file( provider_fixtures, error_fixtures ): - url = provider.build_url('files', 'delete_v2') - path = await provider.validate_path('/The past') - data = {'path': path.full_path} - aiohttpretty.register_json_uri('POST', url, data=data, status=HTTPStatus.OK) + delete_path = await provider.validate_path('/The past') + delete_data = {'path': path.full_path} + delete_url = provider.build_url('files', 'delete_v2') + aiohttpretty.register_json_uri( + 'POST', + delete_url, + data=delete_data, + status=HTTPStatus.OK + ) src_path = WaterButlerPath('/pfile', prepend=provider.folder) dest_path = WaterButlerPath('/pfile_renamed', prepend=provider.folder) - - url = provider.build_url('files', 'copy_v2') - data = { + rename_conflict_folder_metadata = error_fixtures['rename_conflict_folder_metadata'] + imc_metadata = provider_fixtures['intra_move_copy_file_metadata_v2'] + copy_data = { 'from_path': src_path.full_path.rstrip('/'), 'to_path': dest_path.full_path.rstrip('/') } - aiohttpretty.register_json_uri('POST', url, **{ - "responses": [ - { - 'headers': {'Content-Type': 'application/json'}, - 'data': data, - 'body': json.dumps(error_fixtures['rename_conflict_folder_metadata']).encode('utf-8'), - 'status': HTTPStatus.CONFLICT - }, - { - 'headers': {'Content-Type': 'application/json'}, - 'data': data, - 'body': json.dumps(provider_fixtures['intra_move_copy_file_metadata_v2']).encode('utf-8') - }, - ] - }) + copy_responses = [ + { + 'headers': {'Content-Type': 'application/json'}, + 'data': copy_data, + 'body': json.dumps(rename_conflict_folder_metadata).encode('utf-8'), + 'status': HTTPStatus.CONFLICT + }, + { + 'headers': {'Content-Type': 'application/json'}, + 'data': copy_data, + 'body': json.dumps(imc_metadata).encode('utf-8') + }, + ] + copy_url = provider.build_url('files', 'copy_v2') + aiohttpretty.register_json_uri('POST', copy_url, responses=copy_responses) + + expected = (DropboxFileMetadata(imc_metadata['metadata'], provider.folder), False) result = await provider.intra_copy(provider, src_path, dest_path) - expected = ( - DropboxFileMetadata( - provider_fixtures['intra_move_copy_file_metadata_v2']['metadata'], - provider.folder - ), - False - ) - assert expected == result + assert result == expected @pytest.mark.asyncio @pytest.mark.aiohttpretty @@ -919,17 +919,27 @@ async def test_intra_move_casing_change(self, provider): class TestOperations: def test_will_self_overwrite(self, provider, other_provider): - src_path = WaterButlerPath('/50 shades of nope.txt', - _ids=(provider.folder, '12231')) - dest_path = WaterButlerPath('/50 shades of nope2223.txt', - _ids=(provider.folder, '2342sdfsd')) - - result = provider.will_self_overwrite(other_provider, src_path, dest_path) - assert result is False + src_path = WaterButlerPath( + '/50 shades of nope.txt', + _ids=(provider.folder, '12231') + ) result = provider.will_self_overwrite(other_provider, src_path, src_path) assert result is True + def test_wont_self_overwrite(self, provider, other_provider): + src_path = WaterButlerPath( + '/50 shades of nope.txt', + _ids=(provider.folder, '12231') + ) + dest_path = WaterButlerPath( + '/50 shades of nope2223.txt', + _ids=(provider.folder, '2342sdfsd') + ) + + result = provider.will_self_overwrite(other_provider, src_path, dest_path) + assert result is False + def test_can_intra_copy(self, provider): assert provider.can_intra_copy(provider) diff --git a/tests/providers/googledrive/test_provider.py b/tests/providers/googledrive/test_provider.py index 6c856fc76..ce1f62c6f 100644 --- a/tests/providers/googledrive/test_provider.py +++ b/tests/providers/googledrive/test_provider.py @@ -1492,8 +1492,10 @@ async def test_returns_metadata(self, provider, root_provider_fixtures): @pytest.mark.asyncio @pytest.mark.aiohttpretty async def test_raises_non_404(self, provider): - path = WaterButlerPath('/hugo/kim/pins/', _ids=(provider.folder['id'], - 'something', 'something', None)) + path = WaterButlerPath( + '/hugo/kim/pins/', + _ids=(provider.folder['id'], 'something', 'something', None) + ) url = provider.build_url('files') aiohttpretty.register_json_uri('POST', url, status=418) @@ -1517,14 +1519,11 @@ class TestIntraFunctions: async def test_intra_move_file(self, provider, root_provider_fixtures): item = root_provider_fixtures['docs_file_metadata'] src_path = WaterButlerPath('/unsure.txt', _ids=('0', item['id'])) - dest_path = WaterButlerPath('/really/unsure.txt', _ids=('0', - 'yy42kcj', 'rrjk42k')) + dest_path = WaterButlerPath('/really/unsure.txt', _ids=('0', 'yy42kcj', 'rrjk42k')) url = provider.build_url('files', src_path.identifier) data = json.dumps({ - 'parents': [{ - 'id': dest_path.parent.identifier - }], + 'parents': [{'id': dest_path.parent.identifier}], 'title': dest_path.name }), aiohttpretty.register_json_uri('PATCH', url, data=data, body=item) @@ -1533,8 +1532,8 @@ async def test_intra_move_file(self, provider, root_provider_fixtures): del_url_body = json.dumps({'labels': {'trashed': 'true'}}) aiohttpretty.register_uri('PUT', delete_url, body=del_url_body, status=200) - result, created = await provider.intra_move(provider, src_path, dest_path) expected = GoogleDriveFileMetadata(item, dest_path) + result, _ = await provider.intra_move(provider, src_path, dest_path) assert result == expected assert aiohttpretty.has_call(method='PUT', uri=delete_url) @@ -1544,14 +1543,11 @@ async def test_intra_move_file(self, provider, root_provider_fixtures): async def test_intra_move_folder(self, provider, root_provider_fixtures): item = root_provider_fixtures['folder_metadata'] src_path = WaterButlerPath('/unsure/', _ids=('0', item['id'])) - dest_path = WaterButlerPath('/really/unsure/', _ids=('0', - '42jdkerf', '7ejGjeajr')) + dest_path = WaterButlerPath('/really/unsure/', _ids=('0', '42jdkerf', '7ejGjeajr')) url = provider.build_url('files', src_path.identifier) data = json.dumps({ - 'parents': [{ - 'id': dest_path.parent.identifier - }], + 'parents': [{'id': dest_path.parent.identifier}], 'title': dest_path.name }), aiohttpretty.register_json_uri('PATCH', url, data=data, body=item) @@ -1580,13 +1576,10 @@ async def test_intra_move_folder(self, provider, root_provider_fixtures): async def test_intra_copy_file(self, provider, root_provider_fixtures): item = root_provider_fixtures['docs_file_metadata'] src_path = WaterButlerPath('/unsure.txt', _ids=('0', item['id'])) - dest_path = WaterButlerPath('/really/unsure.txt', _ids=('0', - '312kjfe', '4ckk2lkl3')) + dest_path = WaterButlerPath('/really/unsure.txt', _ids=('0', '312kjfe', '4ckk2lkl3')) url = provider.build_url('files', src_path.identifier, 'copy') data = json.dumps({ - 'parents': [{ - 'id': dest_path.parent.identifier - }], + 'parents': [{'id': dest_path.parent.identifier}], 'title': dest_path.name }), aiohttpretty.register_json_uri('POST', url, data=data, body=item) diff --git a/tests/providers/owncloud/test_provider.py b/tests/providers/owncloud/test_provider.py index bf36767fb..9cdfd148b 100644 --- a/tests/providers/owncloud/test_provider.py +++ b/tests/providers/owncloud/test_provider.py @@ -375,10 +375,14 @@ async def test_revisions(self, provider, file_metadata): class TestOperations: def test_will_self_overwrite(self, provider): - src_path = WaterButlerPath('/50 shades of nope.txt', - _ids=(provider.folder, '12231')) - dest_path = WaterButlerPath('/50 shades of nope2223.txt', - _ids=(provider.folder, '2342sdfsd')) + src_path = WaterButlerPath( + '/50 shades of nope.txt', + _ids=(provider.folder, '12231') + ) + dest_path = WaterButlerPath( + '/50 shades of nope2223.txt', + _ids=(provider.folder, '2342sdfsd') + ) result = provider.will_self_overwrite(provider, src_path, dest_path) assert result is False diff --git a/waterbutler/core/exceptions.py b/waterbutler/core/exceptions.py index c84a10491..d513a3356 100644 --- a/waterbutler/core/exceptions.py +++ b/waterbutler/core/exceptions.py @@ -220,8 +220,8 @@ class OrphanSelfError(InvalidParameters): in removal of a directory containing the file or folder to be copied or moved. """ def __init__(self, path): - super().__init__('Unable to move or copy \'{}\'. Moving or copying a folder onto parent ' - 'folder of same name with "replace" is not supported.'.format(path)) + super().__init__('''Unable to move or copy \'{}\'. Moving or copying a folder onto parent + folder of same name with "replace" is not supported.'''.format(path)) class UnsupportedOperationError(ProviderError): diff --git a/waterbutler/core/provider.py b/waterbutler/core/provider.py index 62f7b9953..de3e190ce 100644 --- a/waterbutler/core/provider.py +++ b/waterbutler/core/provider.py @@ -196,13 +196,15 @@ async def make_request(self, method: str, url: str, *args, **kwargs) -> aiohttp. def request(self, *args, **kwargs): return RequestHandlerContext(self.make_request(*args, **kwargs)) - async def move(self, - dest_provider: 'BaseProvider', - src_path: wb_path.WaterButlerPath, - dest_path: wb_path.WaterButlerPath, - rename: str=None, - conflict: str='replace', - handle_conflicts: bool=True) -> typing.Tuple[wb_metadata.BaseMetadata, bool]: + async def move( + self, + dest_provider: 'BaseProvider', + src_path: wb_path.WaterButlerPath, + dest_path: wb_path.WaterButlerPath, + rename: str=None, + conflict: str='replace', + handle_conflicts: bool=True + ) -> typing.Tuple[wb_metadata.BaseMetadata, bool]: """Moves a file or folder from the current provider to the specified one Performs a copy and then a delete. Calls :func:`BaseProvider.intra_move` if possible. @@ -224,7 +226,6 @@ async def move(self, }) if handle_conflicts: - dest_path = await dest_provider.handle_conflicts( self, src_path, @@ -250,13 +251,25 @@ async def move(self, return meta_data, created - async def copy(self, - dest_provider: 'BaseProvider', - src_path: wb_path.WaterButlerPath, - dest_path: wb_path.WaterButlerPath, - rename: str=None, conflict: str='replace', - handle_conflicts: bool=True) \ - -> typing.Tuple[wb_metadata.BaseMetadata, bool]: + async def copy( + self, + dest_provider: 'BaseProvider', + src_path: wb_path.WaterButlerPath, + dest_path: wb_path.WaterButlerPath, + rename: str=None, + conflict: str='replace', + handle_conflicts: bool=True + ) -> typing.Tuple[wb_metadata.BaseMetadata, bool]: + """CoreProvider.copy + Performs a copy + + :param dest_provider: ( :class:`BaseProvider`) The provider to copy to + :param src_path: ( :class:`WaterButlerPath` ) Path to where the resource can be found + :param dest_path: ( :class:`WaterButlerPath` ) Path to there the resource will be moved + :param rename: ( :class:`str` ) The desired name of the resulting path, may be incremented + :param conflict: ( :class:`str' ) What to do in the event of a name conflict; ``replace`` or ``keep`` + :param handle_conflicts: ( :class:`bool` ) If true, will run the handle conflicts + """ args = (dest_provider, src_path, dest_path) kwargs = {'rename': rename, 'conflict': conflict, 'handle_conflicts': handle_conflicts} @@ -267,7 +280,6 @@ async def copy(self, }) if handle_conflicts: - dest_path = await dest_provider.handle_conflicts( self, src_path, @@ -342,7 +354,7 @@ async def _folder_file_op(self, dest_provider, # TODO figure out a way to cut down on all the requests made here (await self.revalidate_path(src_path, item.name, folder=item.is_folder)), - # I don't think dest_provider needs to be revalidated at this point. + # `dest_provider` may not need to be revalidated at this point. # 'def move' and 'def copy are the only methods calling _folder_file_op # both move and copy already call handle_conflicts # which is why the next line down sets handle_conflicts=False @@ -365,12 +377,14 @@ async def _folder_file_op(self, return folder, created - async def handle_conflicts(self, - src_provider, - src_path: wb_path.WaterButlerPath, - dest_path: wb_path.WaterButlerPath, - rename: str=None, - conflict: str='replace') -> wb_path.WaterButlerPath: + async def handle_conflicts( + self, + src_provider, + src_path: wb_path.WaterButlerPath, + dest_path: wb_path.WaterButlerPath, + rename: str=None, + conflict: str='replace' + ) -> wb_path.WaterButlerPath: """Given a :class:`.WaterButlerPath` and the desired name, handle any potential naming issues. i.e.: @@ -392,10 +406,11 @@ async def handle_conflicts(self, :rtype: :class:`.WaterButlerPath` - self is the destination provider - dest_path may be 1. The folder path into which the src will be moved/copied/uploaded - 2. The file path that will be overwritten with new content - 3. None in the case of unvalidated path for ID based provider + ``self`` is the destination provider + ``dest_path`` may be: + 1. The folder path into which the src will be moved/copied/uploaded + 2. The file path that will be overwritten with new content + 3. None in the case of unvalidated path for ID based provider """ if src_path.is_dir and dest_path.is_file: @@ -414,8 +429,7 @@ async def handle_conflicts(self, if self.will_self_overwrite(src_provider, src_path, dest_path): raise exceptions.OverwriteSelfError(src_path) - # files and folders shouldn't overwrite themselves - # TODO: how is this different from above will_self_overwrite call? + # Files and folders shouldn't overwrite themselves if ( self.shares_storage_root(src_provider) and src_path.materialized_path == dest_path.materialized_path @@ -423,9 +437,9 @@ async def handle_conflicts(self, raise exceptions.OverwriteSelfError(src_path) if ( - self == src_provider and - conflict == 'replace' and - self.replace_will_orphan(src_path, dest_path) + self == src_provider + and conflict == 'replace' + and self.replace_will_orphan(src_path, dest_path) ): raise exceptions.OrphanSelfError(src_path) @@ -433,13 +447,15 @@ async def handle_conflicts(self, return dest_path - def will_self_overwrite(self, - dest_provider: 'BaseProvider', - src_path: wb_path.WaterButlerPath, - dest_path: wb_path.WaterButlerPath) -> bool: - """ Return wether a move or copy operation will result in a self-overwrite. + def will_self_overwrite( + self, + dest_provider: 'BaseProvider', + src_path: wb_path.WaterButlerPath, + dest_path: wb_path.WaterButlerPath + ) -> bool: + """Return wether a move or copy operation will result in a self-overwrite. - .. note:: + .. note:: Defaults to False Overridden by providers that need to run this check @@ -516,16 +532,18 @@ async def intra_move(self, await self.delete(src_path) return data, created - def replace_will_orphan(self, - src_path: wb_path.WaterButlerPath, - dest_path: wb_path.WaterButlerPath) -> bool: + def replace_will_orphan( + self, + src_path: wb_path.WaterButlerPath, + dest_path: wb_path.WaterButlerPath + ) -> bool: """Check if copy/move conflict=replace will orphan src_path. Assumes src_provider == dest_provider and conflict == 'replace' have already been checked. This can be an expensive operation. Should be used as last resort. Should be overridden if provider has a cheaper option. """ - if not dest_path.kind == 'folder': + if not dest_path.is_dir: return False if src_path.name != dest_path.name: return False diff --git a/waterbutler/providers/box/provider.py b/waterbutler/providers/box/provider.py index 872bbf685..2137e080f 100644 --- a/waterbutler/providers/box/provider.py +++ b/waterbutler/providers/box/provider.py @@ -180,7 +180,8 @@ def shares_storage_root(self, other: provider.BaseProvider) -> bool: """Box settings include the root folder id, which is unique across projects for subfolders. But the root folder of a Box account always has an ID of 0. This means that the root folders of two separate Box accounts would incorrectly test as being the same storage root. - Add a comparison of credentials to avoid this.""" + Add a comparison of credentials to avoid this. + """ return super().shares_storage_root(other) and self.credentials == other.credentials def will_self_overwrite(self, dest_provider, src_path, dest_path): @@ -202,8 +203,7 @@ async def intra_copy( src_path: WaterButlerPath, dest_path: WaterButlerPath ) -> typing.Tuple[typing.Union[BoxFileMetadata, BoxFolderMetadata], bool]: - """ - Copy a file if the src and dest are both on Box. + """Copy a file if the src and dest are both on Box. """ if src_path.identifier == dest_path.identifier: diff --git a/waterbutler/providers/googledrive/provider.py b/waterbutler/providers/googledrive/provider.py index dba43078d..e397b3ed4 100644 --- a/waterbutler/providers/googledrive/provider.py +++ b/waterbutler/providers/googledrive/provider.py @@ -144,15 +144,22 @@ def can_intra_move( return self == other def can_intra_copy(self, other: provider.BaseProvider, path=None) -> bool: - # gdrive doesn't support intra-copy on folders + # Google Drive doesn't support intra-copy on folders return self == other and (path and path.is_file) - async def intra_move(self, # type: ignore - dest_provider: provider.BaseProvider, - src_path: WaterButlerPath, - dest_path: WaterButlerPath + async def intra_move( + self, # type: ignore + dest_provider: provider.BaseProvider, + src_path: WaterButlerPath, + dest_path: WaterButlerPath ) -> Tuple[BaseGoogleDriveMetadata, bool]: + """Move a file where the source and destination are both on Google Drive + :param dest_provider: ( :class:`.BaseProvider` ) The provider to check against + :param src_path: ( :class:`.WaterButlerPath` ) The move/copy source path + :param dest_path: ( :class:`.WaterButlerPath` ) The move/copy destination path + :rtype: :class:`Tuple[BaseGoogleDriveMetadata, bool]` + """ if src_path.identifier == dest_path.identifier: raise exceptions.IntraCopyError( "Cannot overwrite a file with itself", @@ -196,10 +203,7 @@ async def intra_copy( src_path: WaterButlerPath, dest_path: WaterButlerPath ) -> Tuple[GoogleDriveFileMetadata, bool]: - """ - Copy file where src and dest are both on Google Drive. - """ - + """Copy file where src and dest are both on Google Drive.""" if src_path.identifier == dest_path.identifier: raise exceptions.IntraCopyError( "Cannot overwrite a file with itself",