diff --git a/tests/core/test_provider.py b/tests/core/test_provider.py index 123888ca0..1171fc973 100644 --- a/tests/core/test_provider.py +++ b/tests/core/test_provider.py @@ -4,6 +4,7 @@ from unittest import mock from waterbutler.core import metadata from waterbutler.core import exceptions +from waterbutler.core.path import WaterButlerPath @pytest.fixture @@ -37,6 +38,48 @@ def test_cant_intra_move(self, provider1): def test_can_intra_move(self, provider2): assert provider2.can_intra_move(provider2) is True + @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 + ) + 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 + ) + 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 + ) + assert provider1.replace_will_orphan(src_path, dest_path) == False + @pytest.mark.asyncio async def test_exists(self, provider1): ret = await provider1.exists('somepath') @@ -172,7 +215,7 @@ async def test_no_problem(self, provider1): dest_path = await provider1.validate_path('/test/path/') provider1.exists = utils.MockCoroutine(return_value=False) - handled = await provider1.handle_naming(src_path, dest_path) + handled = await provider1.handle_conflicts(provider1, src_path, dest_path) assert handled == src_path.child('path', folder=True) assert handled.is_dir is True @@ -185,7 +228,7 @@ async def test_rename_via_path(self, provider1): dest_path = await provider1.validate_path('/test/name2') provider1.exists = utils.MockCoroutine(return_value=False) - handled = await provider1.handle_naming(src_path, dest_path) + handled = await provider1.handle_conflicts(provider1, src_path, dest_path) assert handled.name == 'name2' assert handled.is_file is True @@ -196,24 +239,11 @@ async def test_rename_explicit(self, provider1): src_path = await provider1.validate_path('/test/name1') provider1.exists = utils.MockCoroutine(return_value=False) - handled = await provider1.handle_naming(src_path, dest_path, rename='name2') + handled = await provider1.handle_conflicts(provider1, src_path, dest_path, rename='name2') assert handled.name == 'name2' assert handled.is_file is True - @pytest.mark.asyncio - async def test_no_problem_file(self, provider1): - src_path = await provider1.validate_path('/test/path') - dest_path = await provider1.validate_path('/test/path') - provider1.exists = utils.MockCoroutine(return_value=False) - - handled = await provider1.handle_naming(src_path, dest_path) - - assert handled == dest_path # == not is - assert handled.is_file is True - assert len(handled.parts) == 3 # Includes root - assert handled.name == 'path' - class TestCopy: @@ -222,22 +252,23 @@ async def test_handles_naming_false(self, provider1): src_path = await provider1.validate_path('/source/path') dest_path = await provider1.validate_path('/destination/path') - provider1.handle_naming = utils.MockCoroutine() + provider1.handle_conflicts = utils.MockCoroutine() - await provider1.copy(provider1, src_path, dest_path, handle_naming=False) + await provider1.copy(provider1, src_path, dest_path, handle_conflicts=False) - assert provider1.handle_naming.called is False + assert provider1.handle_conflicts.called is False @pytest.mark.asyncio async def test_handles_naming(self, provider1): src_path = await provider1.validate_path('/source/path') dest_path = await provider1.validate_path('/destination/path') - provider1.handle_naming = utils.MockCoroutine() + provider1.handle_conflicts = utils.MockCoroutine() await provider1.copy(provider1, src_path, dest_path) - provider1.handle_naming.assert_called_once_with( + provider1.handle_conflicts.assert_called_once_with( + provider1, src_path, dest_path, rename=None, @@ -249,11 +280,12 @@ async def test_passes_on_conflict(self, provider1): src_path = await provider1.validate_path('/source/path') dest_path = await provider1.validate_path('/destination/path') - provider1.handle_naming = utils.MockCoroutine() + provider1.handle_conflicts = utils.MockCoroutine() await provider1.copy(provider1, src_path, dest_path, conflict='keep') - provider1.handle_naming.assert_called_once_with( + provider1.handle_conflicts.assert_called_once_with( + provider1, src_path, dest_path, rename=None, @@ -265,17 +297,27 @@ async def test_passes_on_rename(self, provider1): src_path = await provider1.validate_path('/source/path') dest_path = await provider1.validate_path('/destination/path') - provider1.handle_naming = utils.MockCoroutine() + provider1.handle_conflicts = utils.MockCoroutine() await provider1.copy(provider1, src_path, dest_path, rename='Baz') - provider1.handle_naming.assert_called_once_with( + provider1.handle_conflicts.assert_called_once_with( + provider1, src_path, dest_path, rename='Baz', conflict='replace', ) + @pytest.mark.asyncio + async def test_copy_will_self_overwrite(self, provider1): + src_path = await provider1.validate_path('/source/path') + dest_path = await provider1.validate_path('/destination/') + provider1.will_self_overwrite = utils.MockCoroutine() + + with pytest.raises(exceptions.OverwriteSelfError): + await provider1.copy(provider1, src_path, dest_path) + @pytest.mark.asyncio async def test_checks_can_intra_copy(self, provider1): provider1.can_intra_copy = mock.Mock(return_value=False) @@ -300,6 +342,19 @@ async def test_calls_intra_copy(self, provider1): provider1.can_intra_copy.assert_called_once_with(provider1, src_path) provider1.intra_copy.assert_called_once_with(provider1, src_path, dest_path) + @pytest.mark.asyncio + @pytest.mark.aiohttpretty + async def test_intra_copy_folder_orphan(self, provider1): + src_path = await provider1.validate_path('/folder1/folder1/') + dest_path = await provider1.validate_path('/') + + provider1.can_intra_copy = mock.Mock(return_value=True) + + with pytest.raises(exceptions.OrphanSelfError) as exc: + await provider1.copy(provider1, src_path, dest_path) + assert exc.value.code == 400 + assert exc.typename == 'OrphanSelfError' + @pytest.mark.asyncio async def test_calls_folder_op_on_dir(self, provider1): src_path = await provider1.validate_path('/source/path/') @@ -339,22 +394,23 @@ async def test_handles_naming_false(self, provider1): src_path = await provider1.validate_path('/source/path') dest_path = await provider1.validate_path('/destination/path') - provider1.handle_naming = utils.MockCoroutine() + provider1.handle_conflicts = utils.MockCoroutine() - await provider1.move(provider1, src_path, dest_path, handle_naming=False) + await provider1.move(provider1, src_path, dest_path, handle_conflicts=False) - assert provider1.handle_naming.called is False + assert provider1.handle_conflicts.called is False @pytest.mark.asyncio async def test_handles_naming(self, provider1): src_path = await provider1.validate_path('/source/path') dest_path = await provider1.validate_path('/destination/path') - provider1.handle_naming = utils.MockCoroutine() + provider1.handle_conflicts = utils.MockCoroutine() await provider1.move(provider1, src_path, dest_path) - provider1.handle_naming.assert_called_once_with( + provider1.handle_conflicts.assert_called_once_with( + provider1, src_path, dest_path, rename=None, @@ -366,11 +422,12 @@ async def test_passes_on_conflict(self, provider1): src_path = await provider1.validate_path('/source/path') dest_path = await provider1.validate_path('/destination/path') - provider1.handle_naming = utils.MockCoroutine() + provider1.handle_conflicts = utils.MockCoroutine() await provider1.move(provider1, src_path, dest_path, conflict='keep') - provider1.handle_naming.assert_called_once_with( + provider1.handle_conflicts.assert_called_once_with( + provider1, src_path, dest_path, rename=None, @@ -382,17 +439,27 @@ async def test_passes_on_rename(self, provider1): src_path = await provider1.validate_path('/source/path') dest_path = await provider1.validate_path('/destination/path') - provider1.handle_naming = utils.MockCoroutine() + provider1.handle_conflicts = utils.MockCoroutine() await provider1.move(provider1, src_path, dest_path, rename='Baz') - provider1.handle_naming.assert_called_once_with( + provider1.handle_conflicts.assert_called_once_with( + provider1, src_path, dest_path, rename='Baz', conflict='replace', ) + @pytest.mark.asyncio + async def test_move_will_self_overwrite(self, provider1): + src_path = await provider1.validate_path('/source/path') + dest_path = await provider1.validate_path('/destination/') + provider1.will_self_overwrite = utils.MockCoroutine() + + with pytest.raises(exceptions.OverwriteSelfError): + await provider1.move(provider1, src_path, dest_path) + @pytest.mark.asyncio async def test_checks_can_intra_move(self, provider1): provider1.can_intra_move = mock.Mock(return_value=False) @@ -417,6 +484,20 @@ async def test_calls_intra_move(self, provider1): provider1.can_intra_move.assert_called_once_with(provider1, src_path) provider1.intra_move.assert_called_once_with(provider1, src_path, dest_path) + @pytest.mark.asyncio + @pytest.mark.aiohttpretty + async def test_intra_move_folder_orphan(self, provider1): + src_path = await provider1.validate_path('/folder1/folder1/') + dest_path = await provider1.validate_path('/') + + provider1.can_intra_move = mock.Mock(return_value=True) + + with pytest.raises(exceptions.OrphanSelfError) as exc: + await provider1.move(provider1, src_path, dest_path) + + assert exc.value.code == 400 + assert exc.typename == 'OrphanSelfError' + @pytest.mark.asyncio async def test_calls_folder_op_on_dir_and_delete(self, provider1): src_path = await provider1.validate_path('/source/path/') @@ -453,7 +534,7 @@ async def test_calls_copy_and_delete(self, provider1): provider1, src_path, dest_path, - handle_naming=False + handle_conflicts=False ) @pytest.mark.asyncio @@ -473,7 +554,7 @@ async def test_no_delete_on_copy_error(self, provider1): provider1, src_path, dest_path, - handle_naming=False + handle_conflicts=False ) def test_build_range_header(self, provider1): diff --git a/tests/providers/box/test_provider.py b/tests/providers/box/test_provider.py index 384e9e7dc..0635a3660 100644 --- a/tests/providers/box/test_provider.py +++ b/tests/providers/box/test_provider.py @@ -77,10 +77,13 @@ async def test_validate_v1_path_file(self, provider, root_provider_fixtures): good_url = provider.build_url('files', file_id, fields='id,name,path_collection') bad_url = provider.build_url('folders', file_id, fields='id,name,path_collection') - aiohttpretty.register_json_uri('get', good_url, - body=root_provider_fixtures['file_metadata']['entries'][0], - status=200) - aiohttpretty.register_uri('get', bad_url, status=404) + aiohttpretty.register_json_uri( + 'GET', + good_url, + body=root_provider_fixtures['file_metadata']['entries'][0], + status=200 + ) + aiohttpretty.register_uri('GET', bad_url, status=404) try: wb_path_v1 = await provider.validate_v1_path('/' + file_id) @@ -105,10 +108,14 @@ async def test_validate_v1_path_folder(self, provider, root_provider_fixtures): good_url = provider.build_url('folders', folder_id, fields='id,name,path_collection') bad_url = provider.build_url('files', folder_id, fields='id,name,path_collection') - aiohttpretty.register_json_uri('get', good_url, - body=root_provider_fixtures['folder_object_metadata'], - status=200) - aiohttpretty.register_uri('get', bad_url, status=404) + aiohttpretty.register_json_uri( + 'GET', + good_url, + body=root_provider_fixtures['folder_object_metadata'], + status=200 + ) + aiohttpretty.register_uri('GET', bad_url, status=404) + try: wb_path_v1 = await provider.validate_v1_path('/' + folder_id + '/') except Exception as exc: @@ -164,11 +171,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) @@ -316,7 +329,7 @@ async def test_upload_checksum_mismatch(self, provider, root_provider_fixtures, aiohttpretty.register_json_uri('POST', upload_url, status=201, body=root_provider_fixtures['checksum_mismatch_metadata']) - with pytest.raises(exceptions.UploadChecksumMismatchError) as exc: + with pytest.raises(exceptions.UploadChecksumMismatchError): await provider.upload(file_stream, path) assert aiohttpretty.has_call(method='POST', uri=upload_url) @@ -375,19 +388,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) @@ -589,6 +606,7 @@ async def test_get_revisions_free_account(self, provider, root_provider_fixtures class TestIntraCopy: + @pytest.mark.asyncio @pytest.mark.aiohttpretty async def test_intra_copy_file(self, provider, root_provider_fixtures): @@ -609,7 +627,7 @@ async def test_intra_copy_file(self, provider, root_provider_fixtures): async def test_intra_copy_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'], item['id'])) + dest_path = WaterButlerPath('/charmander/name.txt', _ids=(provider, item['id'], 'cats77831')) file_url = provider.build_url('files', src_path.identifier, 'copy') delete_url = provider.build_url('files', dest_path.identifier) @@ -641,7 +659,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) @@ -652,18 +673,28 @@ 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'] src_path = WaterButlerPath('/name/', _ids=(provider, item['id'])) - dest_path = WaterButlerPath('/charmander/name/', _ids=(provider, item['id'], item['id'])) + dest_path = WaterButlerPath('/charmander/name/', _ids=(provider, item['id'], '4jkmrm4zzerj')) 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) @@ -672,7 +703,10 @@ async def test_intra_copy_folder_replace(self, provider, intra_fixtures, root_pr 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) @@ -705,7 +739,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'], item['id'])) + 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) @@ -737,7 +774,10 @@ async def test_intra_move_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) @@ -746,21 +786,33 @@ async def test_intra_move_folder(self, provider, intra_fixtures, root_provider_f assert result == expected - @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'], item['id'])) + 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) @@ -769,7 +821,10 @@ async def test_intra_move_folder_replace(self, provider, intra_fixtures, root_pr 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) @@ -853,25 +908,33 @@ async def test_returns_metadata(self, provider, root_provider_fixtures): class TestOperations: - @pytest.mark.asyncio - @pytest.mark.aiohttpretty - async def test_can_duplicate_names(self, provider): + 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 + + result = provider.will_self_overwrite(other_provider, src_path, src_path) + assert result is True + + def test_can_duplicate_names(self, provider): assert provider.can_duplicate_names() is False - @pytest.mark.asyncio - @pytest.mark.aiohttpretty - async def test_shares_storage_root(self, provider, other_provider): + def test_shares_storage_root(self, provider, other_provider): assert provider.shares_storage_root(other_provider) is False assert provider.shares_storage_root(provider) is True - @pytest.mark.asyncio - @pytest.mark.aiohttpretty - async def test_can_intra_move(self, provider, other_provider): + def test_can_intra_move(self, provider, other_provider): assert provider.can_intra_move(other_provider) is False assert provider.can_intra_move(provider) is True - @pytest.mark.asyncio - @pytest.mark.aiohttpretty - async def test_can_intra_copy(self, provider, other_provider): + def test_can_intra_copy(self, provider, other_provider): assert provider.can_intra_copy(other_provider) is False assert provider.can_intra_copy(provider) is True diff --git a/tests/providers/dropbox/test_provider.py b/tests/providers/dropbox/test_provider.py index 787af4a31..35755c3ea 100644 --- a/tests/providers/dropbox/test_provider.py +++ b/tests/providers/dropbox/test_provider.py @@ -371,6 +371,7 @@ async def test_folder_with_more_metadata(self, provider, provider_fixtures): data=data, body=provider_fixtures['folder_with_more_metadata'] ) + aiohttpretty.register_json_uri( 'POST', url + '/continue', @@ -658,49 +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': delete_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 @@ -921,6 +918,28 @@ 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') + ) + + 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 6c80a507e..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) @@ -1516,25 +1518,22 @@ class TestIntraFunctions: @pytest.mark.aiohttpretty async def test_intra_move_file(self, provider, root_provider_fixtures): item = root_provider_fixtures['docs_file_metadata'] - src_path = WaterButlerPath('/unsure.txt', _ids=(provider.folder['id'], item['id'])) - dest_path = WaterButlerPath('/really/unsure.txt', _ids=(provider.folder['id'], - item['id'], item['id'])) + src_path = WaterButlerPath('/unsure.txt', _ids=('0', item['id'])) + 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) - delete_url = provider.build_url('files', item['id']) + delete_url = provider.build_url('files', dest_path.identifier) 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) @@ -1543,24 +1542,21 @@ async def test_intra_move_file(self, provider, root_provider_fixtures): @pytest.mark.aiohttpretty async def test_intra_move_folder(self, provider, root_provider_fixtures): item = root_provider_fixtures['folder_metadata'] - src_path = WaterButlerPath('/unsure/', _ids=(provider.folder['id'], item['id'])) - dest_path = WaterButlerPath('/really/unsure/', _ids=(provider.folder['id'], - item['id'], item['id'])) + src_path = WaterButlerPath('/unsure/', _ids=('0', item['id'])) + 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) - delete_url = provider.build_url('files', item['id']) + delete_url = provider.build_url('files', dest_path.identifier) del_url_body = json.dumps({'labels': {'trashed': 'true'}}) aiohttpretty.register_uri('PUT', delete_url, body=del_url_body, status=200) - children_query = provider._build_query(dest_path.identifier) + children_query = provider._build_query(src_path.identifier) children_url = provider.build_url('files', q=children_query, alt='json', maxResults=1000) children_list = generate_list(3, **root_provider_fixtures['folder_metadata']) aiohttpretty.register_json_uri('GET', children_url, body=children_list) @@ -1579,20 +1575,16 @@ async def test_intra_move_folder(self, provider, root_provider_fixtures): @pytest.mark.aiohttpretty async def test_intra_copy_file(self, provider, root_provider_fixtures): item = root_provider_fixtures['docs_file_metadata'] - src_path = WaterButlerPath('/unsure.txt', _ids=(provider.folder['id'], item['id'])) - dest_path = WaterButlerPath('/really/unsure.txt', _ids=(provider.folder['id'], - item['id'], item['id'])) - + src_path = WaterButlerPath('/unsure.txt', _ids=('0', item['id'])) + 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) - delete_url = provider.build_url('files', item['id']) + delete_url = provider.build_url('files', dest_path.identifier) del_url_body = json.dumps({'labels': {'trashed': 'true'}}) aiohttpretty.register_uri('PUT', delete_url, body=del_url_body, status=200) @@ -1605,33 +1597,33 @@ async def test_intra_copy_file(self, provider, root_provider_fixtures): class TestOperationsOrMisc: - @pytest.mark.asyncio - @pytest.mark.aiohttpretty - async def test_can_duplicate_names(self, provider): + def test_will_self_overwrite(self, provider, other_provider): + src_path = GoogleDrivePath('/root/Gear1.stl', _ids=['0', '10', '11']) + dest_path = GoogleDrivePath('/root/Gear23123.stl', _ids=['0', '10', '12']) + + result = provider.will_self_overwrite(other_provider, src_path, dest_path) + assert result is False + + result = provider.will_self_overwrite(other_provider, src_path, src_path) + assert result is True + + def test_can_duplicate_names(self, provider): assert provider.can_duplicate_names() is True - @pytest.mark.asyncio - @pytest.mark.aiohttpretty - async def test_shares_storage_root(self, provider, other_provider): + def test_shares_storage_root(self, provider, other_provider): assert provider.shares_storage_root(other_provider) is True assert provider.shares_storage_root(provider) is True - @pytest.mark.asyncio - @pytest.mark.aiohttpretty - async def test_can_intra_move(self, provider, other_provider): + def test_can_intra_move(self, provider, other_provider): assert provider.can_intra_move(other_provider) is False assert provider.can_intra_move(provider) is True - @pytest.mark.asyncio - @pytest.mark.aiohttpretty - async def test__serialize_item_raw(self, provider, root_provider_fixtures): + def test__serialize_item_raw(self, provider, root_provider_fixtures): item = root_provider_fixtures['docs_file_metadata'] assert provider._serialize_item(None, item, True) == item - @pytest.mark.asyncio - @pytest.mark.aiohttpretty - async def test_can_intra_copy(self, provider, other_provider, root_provider_fixtures): + def test_can_intra_copy(self, provider, other_provider, root_provider_fixtures): item = root_provider_fixtures['list_file']['items'][0] path = WaterButlerPath('/birdie.jpg', _ids=(provider.folder['id'], item['id'])) @@ -1668,7 +1660,7 @@ async def test_revalidate_path_file_error(self, provider, root_provider_fixtures body=error_fixtures['parts_file_missing_metadata']) with pytest.raises(exceptions.MetadataError) as e: - result = await provider._resolve_path_to_ids(file_name) + await provider._resolve_path_to_ids(file_name) assert e.value.message == '{} not found'.format(str(path)) assert e.value.code == 404 diff --git a/tests/providers/osfstorage/test_provider.py b/tests/providers/osfstorage/test_provider.py index d5b0b9748..bcf545bd7 100644 --- a/tests/providers/osfstorage/test_provider.py +++ b/tests/providers/osfstorage/test_provider.py @@ -327,7 +327,6 @@ async def test_intra_copy_folder(self, provider_and_mock, provider_and_mock2, src_mock._children_metadata.assert_not_called() src_mock.validate_v1_path.assert_not_called() - @pytest.mark.asyncio @pytest.mark.aiohttpretty async def test_intra_copy_file(self, provider_and_mock, provider_and_mock2, diff --git a/tests/providers/owncloud/test_provider.py b/tests/providers/owncloud/test_provider.py index d83a33479..0fc695e9f 100644 --- a/tests/providers/owncloud/test_provider.py +++ b/tests/providers/owncloud/test_provider.py @@ -36,6 +36,7 @@ def file_like(file_content): return io.BytesIO(file_content) + @pytest.fixture def file_stream(file_like): return streams.FileStreamReader(file_like) @@ -375,6 +376,22 @@ 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') + ) + + result = provider.will_self_overwrite(provider, src_path, dest_path) + assert result is False + + result = provider.will_self_overwrite(provider, src_path, src_path) + assert result is True + def test_can_intra_copy(self, provider, provider_different_credentials): assert provider.can_intra_copy(provider) assert not provider.can_intra_copy(provider_different_credentials) diff --git a/waterbutler/core/exceptions.py b/waterbutler/core/exceptions.py index 081dff26b..c3d60ccde 100644 --- a/waterbutler/core/exceptions.py +++ b/waterbutler/core/exceptions.py @@ -215,6 +215,15 @@ def __init__(self, path): 'folder onto itself is not supported.'.format(path)) +class OrphanSelfError(InvalidParameters): + """Error for conflict=replace sent to methods copy or move, when action would result + 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)) + + class UnsupportedOperationError(ProviderError): def __init__(self, message, code=HTTPStatus.FORBIDDEN, is_user_error=True): if not message: diff --git a/waterbutler/core/provider.py b/waterbutler/core/provider.py index 84e44d8dd..e2e7763f7 100644 --- a/waterbutler/core/provider.py +++ b/waterbutler/core/provider.py @@ -197,35 +197,38 @@ 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_naming: 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. - :param dest_provider: ( :class:`.BaseProvider` ) The provider to move to + :param dest_provider: ( :class:`BaseProvider` ) The provider to move to :param src_path: ( :class:`.WaterButlerPath` ) Path to where the resource can be found :param dest_path: ( :class:`.WaterButlerPath` ) Path to where 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_naming: ( :class:`bool` ) If a naming conflict is detected, should it be automatically handled? + :param handle_conflicts: ( :class:`bool` ) If true will be run through handle_conflicts method. """ args = (dest_provider, src_path, dest_path) kwargs = {'rename': rename, 'conflict': conflict} self.provider_metrics.add('move', { - 'got_handle_naming': handle_naming, + 'got_handle_conflicts': handle_conflicts, 'conflict': conflict, 'got_rename': rename is not None, }) - if handle_naming: - dest_path = await dest_provider.handle_naming( + if handle_conflicts: + dest_path = await dest_provider.handle_conflicts( + self, src_path, dest_path, rename=rename, @@ -234,13 +237,6 @@ async def move(self, args = (dest_provider, src_path, dest_path) kwargs = {} - # files and folders shouldn't overwrite themselves - if ( - self.shares_storage_root(dest_provider) and - src_path.materialized_path == dest_path.materialized_path - ): - raise exceptions.OverwriteSelfError(src_path) - self.provider_metrics.add('move.can_intra_move', False) if self.can_intra_move(dest_provider, src_path): self.provider_metrics.add('move.can_intra_move', True) @@ -249,29 +245,44 @@ async def move(self, if src_path.is_dir: meta_data, created = await self._folder_file_op(self.move, *args, **kwargs) # type: ignore else: - meta_data, created = await self.copy(*args, handle_naming=False, **kwargs) # type: ignore + # handle_conflicts false because we've already been through handle_conflicts once. + meta_data, created = await self.copy(*args, handle_conflicts=False, **kwargs) # type: ignore await self.delete(src_path) 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_naming: 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_naming': handle_naming} + kwargs = {'rename': rename, 'conflict': conflict, 'handle_conflicts': handle_conflicts} self.provider_metrics.add('copy', { - 'got_handle_naming': handle_naming, + 'got_handle_conflicts': handle_conflicts, 'conflict': conflict, 'got_rename': rename is not None, }) - if handle_naming: - dest_path = await dest_provider.handle_naming( + + if handle_conflicts: + dest_path = await dest_provider.handle_conflicts( + self, src_path, dest_path, rename=rename, @@ -280,13 +291,6 @@ async def copy(self, args = (dest_provider, src_path, dest_path) kwargs = {} - # files and folders shouldn't overwrite themselves - if ( - self.shares_storage_root(dest_provider) and - src_path.materialized_path == dest_path.materialized_path - ): - raise exceptions.OverwriteSelfError(src_path) - self.provider_metrics.add('copy.can_intra_copy', False) if self.can_intra_copy(dest_provider, src_path): self.provider_metrics.add('copy.can_intra_copy', True) @@ -351,8 +355,13 @@ 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)), + # `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 + # which in turn already calls dest_provider.revalidate_path (await dest_provider.revalidate_path(dest_path, item.name, folder=item.is_folder)), - handle_naming=False, + handle_conflicts=False, ) )) @@ -369,11 +378,14 @@ async def _folder_file_op(self, return folder, created - async def handle_naming(self, - 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.: @@ -387,13 +399,21 @@ async def handle_naming(self, cp /file.txt /folder/doc.txt -> /folder/doc.txt + :param src_provider: ( :class:`.BaseProvider` ) The source provider :param src_path: ( :class:`.WaterButlerPath` ) The object that is being copied :param dest_path: ( :class:`.WaterButlerPath` ) The path that is being copied to or into :param rename: ( :class:`str` ) The desired name of the resulting path, may be incremented :param conflict: ( :class:`str` ) The conflict resolution strategy, ``replace`` or ``keep`` :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 """ + if src_path.is_dir and dest_path.is_file: # Cant copy a directory to a file raise ValueError('Destination must be a directory if the source is') @@ -406,11 +426,47 @@ async def handle_naming(self, rename or src_path.name, folder=src_path.is_dir ) + if conflict != 'keep': + if self.will_self_overwrite(src_provider, src_path, dest_path): + raise exceptions.OverwriteSelfError(src_path) + + # Files and folders shouldn't overwrite themselves + if ( + self.shares_storage_root(src_provider) and + src_path.materialized_path == dest_path.materialized_path + ): + raise exceptions.OverwriteSelfError(src_path) + + if ( + self == src_provider and + conflict == 'replace' and + self.replace_will_orphan(src_path, dest_path) + ): + raise exceptions.OrphanSelfError(src_path) dest_path, _ = await self.handle_name_conflict(dest_path, conflict=conflict) 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. + + .. note:: + Defaults to False + Overridden by providers that need to run this check + + :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:`bool` + """ + return False + def can_intra_copy(self, other: 'BaseProvider', path: wb_path.WaterButlerPath=None) -> bool: @@ -477,6 +533,30 @@ 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: + """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.is_dir: + return False + if src_path.name != dest_path.name: + return False + + incr_path = src_path + while incr_path.materialized_path != '/': + incr_path = incr_path.parent + if incr_path.materialized_path == dest_path.materialized_path: + return True + + return False + async def exists(self, path: wb_path.WaterButlerPath, **kwargs) \ -> typing.Union[bool, wb_metadata.BaseMetadata, typing.List[wb_metadata.BaseMetadata]]: """Check for existence of WaterButlerPath diff --git a/waterbutler/providers/box/provider.py b/waterbutler/providers/box/provider.py index fc2533c89..887d50286 100644 --- a/waterbutler/providers/box/provider.py +++ b/waterbutler/providers/box/provider.py @@ -53,7 +53,7 @@ async def validate_v1_path(self, path: str, **kwargs) -> WaterButlerPath: raise exceptions.NotFoundError(str(path)) response = await self.make_request( - 'get', + 'GET', self.build_url(files_or_folders, obj_id, fields='id,name,path_collection'), expects=(200, 404,), throws=exceptions.MetadataError, @@ -97,7 +97,7 @@ async def validate_path(self, path: str, **kwargs) -> WaterButlerPath: response = None if obj_id.isdecimal(): response = await self.make_request( - 'get', + 'GET', self.build_url(files_or_folders, obj_id, fields='id,name,path_collection'), expects=(200, 404, 405), throws=exceptions.MetadataError, @@ -180,18 +180,38 @@ 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 can_intra_move(self, other: provider.BaseProvider, path: WaterButlerPath=None) -> bool: + def will_self_overwrite(self, dest_provider, src_path, dest_path): + return self.NAME == dest_provider.NAME and src_path.identifier == dest_path.identifier + + def can_intra_move( + self, + other: provider.BaseProvider, + path: WaterButlerPath=None + ) -> bool: return self == other def can_intra_copy(self, other: provider.BaseProvider, path: WaterButlerPath=None) -> bool: return self == other - async def intra_copy(self, # type: ignore - dest_provider: provider.BaseProvider, src_path: WaterButlerPath, - dest_path: WaterButlerPath) -> Tuple[BaseBoxMetadata, bool]: + async def intra_copy( + self, # type: ignore + dest_provider: provider.BaseProvider, + src_path: WaterButlerPath, + dest_path: WaterButlerPath + ) -> Tuple[Union[BoxFileMetadata, BoxFolderMetadata], bool]: + """Copy a file if the src and dest are both on Box. + """ + + if src_path.identifier == dest_path.identifier: + raise exceptions.IntraCopyError( + "Cannot overwrite a file with itself", + code=HTTPStatus.CONFLICT + ) + if dest_path.identifier is not None: await dest_provider.delete(dest_path) @@ -216,9 +236,19 @@ async def intra_copy(self, # type: ignore return await self._intra_move_copy_metadata(dest_path, data) - async def intra_move(self, # type: ignore - dest_provider: provider.BaseProvider, src_path: WaterButlerPath, - dest_path: WaterButlerPath) -> Tuple[BaseBoxMetadata, bool]: + async def intra_move( + self, # type: ignore + dest_provider: provider.BaseProvider, + src_path: WaterButlerPath, + dest_path: WaterButlerPath + ) -> Tuple[BaseBoxMetadata, bool]: + + if src_path.identifier == dest_path.identifier: + raise exceptions.IntraCopyError( + "Cannot overwrite a file with itself", + code=HTTPStatus.CONFLICT + ) + if dest_path.identifier is not None and str(dest_path).lower() != str(src_path).lower(): await dest_provider.delete(dest_path) @@ -259,7 +289,6 @@ async def download(self, # type: ignore **kwargs) -> streams.ResponseStreamReader: if path.identifier is None: raise exceptions.DownloadError('"{}" not found'.format(str(path)), code=404) - query = {} if revision and revision != path.identifier: query['version'] = revision diff --git a/waterbutler/providers/dropbox/provider.py b/waterbutler/providers/dropbox/provider.py index 7c4cecdae..5bc674782 100644 --- a/waterbutler/providers/dropbox/provider.py +++ b/waterbutler/providers/dropbox/provider.py @@ -152,6 +152,7 @@ async def intra_copy(self, # type: ignore dest_path: WaterButlerPath) \ -> typing.Tuple[typing.Union[DropboxFileMetadata, DropboxFolderMetadata], bool]: dest_folder = dest_provider.folder + try: if self == dest_provider: data = await self.dropbox_request( @@ -193,6 +194,7 @@ async def intra_move(self, # type: ignore dest_provider: 'DropboxProvider', src_path: WaterButlerPath, dest_path: WaterButlerPath) -> typing.Tuple[BaseDropboxMetadata, bool]: + if dest_path.full_path.lower() == src_path.full_path.lower(): # Dropbox does not support changing the casing in a file name raise core_exceptions.InvalidPathError( @@ -372,6 +374,9 @@ async def create_folder(self, path: WaterButlerPath, **kwargs) -> DropboxFolderM ) return DropboxFolderMetadata(data['metadata'], self.folder) + def will_self_overwrite(self, dest_provider, src_path, dest_path): + return self.NAME == dest_provider.NAME and dest_path.full_path == src_path.full_path + def can_intra_copy(self, dest_provider: provider.BaseProvider, path: WaterButlerPath=None) -> bool: return type(self) == type(dest_provider) diff --git a/waterbutler/providers/googledrive/provider.py b/waterbutler/providers/googledrive/provider.py index d254bfd75..e397b3ed4 100644 --- a/waterbutler/providers/googledrive/provider.py +++ b/waterbutler/providers/googledrive/provider.py @@ -133,17 +133,39 @@ def can_duplicate_names(self) -> bool: def default_headers(self) -> dict: return {'authorization': 'Bearer {}'.format(self.token)} - def can_intra_move(self, other: provider.BaseProvider, path: WaterButlerPath=None) -> bool: + def will_self_overwrite(self, dest_provider, src_path, dest_path): + return self.NAME == dest_provider.NAME and src_path.identifier == dest_path.identifier + + def can_intra_move( + self, + other: provider.BaseProvider, + path=None + ) -> bool: 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) -> Tuple[BaseGoogleDriveMetadata, bool]: + 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", + code=HTTPStatus.CONFLICT + ) + self.metrics.add('intra_move.destination_exists', dest_path.identifier is not None) if dest_path.identifier: await dest_provider.delete(dest_path) @@ -175,11 +197,21 @@ async def intra_move(self, # type: ignore else: return GoogleDriveFileMetadata(data, dest_path), created # type: ignore - async def intra_copy(self, - dest_provider: provider.BaseProvider, - src_path: WaterButlerPath, - dest_path: WaterButlerPath) -> Tuple[GoogleDriveFileMetadata, bool]: + async def intra_copy( + self, + dest_provider: provider.BaseProvider, + src_path: WaterButlerPath, + dest_path: WaterButlerPath + ) -> Tuple[GoogleDriveFileMetadata, bool]: + """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", + code=HTTPStatus.CONFLICT + ) + self.metrics.add('intra_copy.destination_exists', dest_path.identifier is not None) + if dest_path.identifier: await dest_provider.delete(dest_path) diff --git a/waterbutler/providers/owncloud/provider.py b/waterbutler/providers/owncloud/provider.py index 29cc2b10e..d183129c1 100644 --- a/waterbutler/providers/owncloud/provider.py +++ b/waterbutler/providers/owncloud/provider.py @@ -266,6 +266,9 @@ async def create_folder(self, path, **kwargs): def can_duplicate_names(self): return True + def will_self_overwrite(self, dest_provider, src_path, dest_path): + return self.NAME == dest_provider.NAME and src_path.identifier == dest_path.identifier + def can_intra_copy(self, dest_provider, path=None): return self == dest_provider