diff --git a/tests/core/test_provider.py b/tests/core/test_provider.py index 123888ca0..7717d1e84 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,36 @@ 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 +203,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 +216,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 +227,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 +240,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 +268,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 +285,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 +330,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 +382,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 +410,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 +427,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 +472,19 @@ 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 +521,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 +541,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 47e8d55ab..32bd3690a 100644 --- a/tests/providers/box/test_provider.py +++ b/tests/providers/box/test_provider.py @@ -165,7 +165,9 @@ async def test_validate_path(self, provider, root_provider_fixtures): provider.folder = '0' folder_id = '0' - 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) @@ -295,7 +297,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) @@ -360,8 +362,11 @@ async def test_delete_root(self, provider, root_provider_fixtures): 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']) + aiohttpretty.register_json_uri( + 'GET', + url, + body=root_provider_fixtures['one_entry_folder_list_metadata'] + ) url = provider.build_url('files', item['id'], fields='id,name,path_collection') delete_url = provider.build_url('files', path.identifier) @@ -568,6 +573,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): @@ -588,7 +594,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) @@ -620,7 +626,9 @@ 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) @@ -631,12 +639,15 @@ 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) @@ -651,7 +662,9 @@ 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) @@ -684,7 +697,8 @@ 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) @@ -716,7 +730,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) @@ -725,15 +742,17 @@ 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) @@ -748,7 +767,9 @@ 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) @@ -832,25 +853,29 @@ 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 01f1b6c00..649853eda 100644 --- a/tests/providers/dropbox/test_provider.py +++ b/tests/providers/dropbox/test_provider.py @@ -290,8 +290,12 @@ async def test_folder_with_subdirectory_metadata(self, provider, root_provider_f path = await provider.validate_path('/') url = provider.build_url('files', 'list_folder') data = {'path': path.full_path} - aiohttpretty.register_json_uri('POST', url, data=data, - body=root_provider_fixtures['folder_with_subdirectory_metadata']) + aiohttpretty.register_json_uri( + 'POST', + url, + data=data, + body=root_provider_fixtures['folder_with_subdirectory_metadata'] + ) result = await provider.metadata(path) assert isinstance(result, list) @@ -308,8 +312,12 @@ async def test_folder_with_hasmore_metadata(self, provider, root_provider_fixtur data = {'path': path.full_path} aiohttpretty.register_json_uri('POST', url, data=data, body=root_provider_fixtures['folder_with_hasmore_metadata']) - aiohttpretty.register_json_uri('POST', url + '/continue', data=data, - body=root_provider_fixtures['folder_with_subdirectory_metadata']) + aiohttpretty.register_json_uri( + 'POST', + url + '/continue', + data=data, + body=root_provider_fixtures['folder_with_subdirectory_metadata'] + ) result = await provider.metadata(path) @@ -544,7 +552,9 @@ async def test_intra_copy_replace_file(self, provider, root_provider_fixtures, e { 'headers': {'Content-Type': 'application/json'}, 'data': data, - 'body': json.dumps(error_fixtures['rename_conflict_folder_metadata']).encode('utf-8'), + 'body': json.dumps( + error_fixtures['rename_conflict_folder_metadata'] + ).encode('utf-8'), 'status': 409 }, { @@ -574,8 +584,12 @@ async def test_intra_copy_file_different_provider(self, provider, other_provider url1 = provider.build_url('files', 'copy_reference', 'save') data1 = {'copy_reference': 'test', 'path': dest_path.full_path.rstrip('/')} - aiohttpretty.register_json_uri('POST', url1, data=data1, - body=intra_copy_fixtures['intra_copy_other_provider_file_metadata']) + aiohttpretty.register_json_uri( + 'POST', + url1, + data=data1, + body=intra_copy_fixtures['intra_copy_other_provider_file_metadata'] + ) result = await provider.intra_copy(other_provider, src_path, dest_path) expected = (DropboxFileMetadata( @@ -648,7 +662,9 @@ async def test_intra_move_replace_file(self, provider, root_provider_fixtures, e { 'headers': {'Content-Type': 'application/json'}, 'data': data, - 'body': json.dumps(error_fixtures['rename_conflict_file_metadata']).encode('utf-8'), + 'body': json.dumps( + error_fixtures['rename_conflict_file_metadata'] + ).encode('utf-8'), 'status': 409 }, { @@ -689,7 +705,9 @@ async def test_intra_move_replace_folder(self, provider, root_provider_fixtures, { 'headers': {'Content-Type': 'application/json'}, 'data': data, - 'body': json.dumps(error_fixtures['rename_conflict_folder_metadata']).encode('utf-8'), + 'body': json.dumps( + error_fixtures['rename_conflict_folder_metadata'] + ).encode('utf-8'), 'status': 409 }, { @@ -722,6 +740,18 @@ 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 + + result = provider.will_self_overwrite(other_provider, src_path, src_path) + assert result is True + def test_can_intra_copy(self, provider): assert provider.can_intra_copy(provider) @@ -732,7 +762,7 @@ def test_can_intra_move(self, provider): assert provider.can_intra_move(provider) def test_cannot_intra_move_other(self, provider, other_provider): - assert provider.can_intra_move(other_provider) == False + assert provider.can_intra_move(other_provider) is False def test_conflict_error_handler_not_found(self, provider, error_fixtures): error_path = '/Photos/folder/file' diff --git a/tests/providers/googledrive/test_provider.py b/tests/providers/googledrive/test_provider.py index 81624f5a5..eeee379c1 100644 --- a/tests/providers/googledrive/test_provider.py +++ b/tests/providers/googledrive/test_provider.py @@ -1488,9 +1488,9 @@ 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({ @@ -1501,7 +1501,7 @@ async def test_intra_move_file(self, provider, root_provider_fixtures): }), 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) @@ -1515,9 +1515,9 @@ 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({ @@ -1528,11 +1528,11 @@ async def test_intra_move_folder(self, provider, root_provider_fixtures): }), 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) @@ -1551,10 +1551,9 @@ 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': [{ @@ -1564,7 +1563,7 @@ async def test_intra_copy_file(self, provider, root_provider_fixtures): }), 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) @@ -1577,33 +1576,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'])) @@ -1640,7 +1639,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 e0e6d52da..5fa100beb 100644 --- a/tests/providers/osfstorage/test_provider.py +++ b/tests/providers/osfstorage/test_provider.py @@ -341,7 +341,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 3ef3cb5d9..e9f3ffce4 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) @@ -360,6 +361,18 @@ 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 24c5f76c6..339b44053 100644 --- a/waterbutler/core/exceptions.py +++ b/waterbutler/core/exceptions.py @@ -208,6 +208,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 ba443e33f..62f7b9953 100644 --- a/waterbutler/core/provider.py +++ b/waterbutler/core/provider.py @@ -202,29 +202,31 @@ async def move(self, dest_path: wb_path.WaterButlerPath, rename: str=None, conflict: str='replace', - handle_naming: bool=True) -> typing.Tuple[wb_metadata.BaseMetadata, bool]: + 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, @@ -233,13 +235,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) @@ -248,7 +243,8 @@ 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) @@ -259,18 +255,21 @@ async def copy(self, src_path: wb_path.WaterButlerPath, dest_path: wb_path.WaterButlerPath, rename: str=None, conflict: str='replace', - handle_naming: bool=True) \ + handle_conflicts: bool=True) \ -> typing.Tuple[wb_metadata.BaseMetadata, bool]: 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, @@ -279,13 +278,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) @@ -350,8 +342,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)), + # I don't think dest_provider needs 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, ) )) @@ -368,11 +365,12 @@ 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.: @@ -386,13 +384,20 @@ 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') @@ -405,11 +410,46 @@ 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 + # TODO: how is this different from above will_self_overwrite call? + 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: @@ -476,6 +516,28 @@ 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.kind == 'folder': + 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 a8b8f8096..a8096661b 100644 --- a/waterbutler/providers/box/provider.py +++ b/waterbutler/providers/box/provider.py @@ -182,6 +182,9 @@ def shares_storage_root(self, other: provider.BaseProvider) -> bool: 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): + return self.NAME == dest_provider.NAME and src_path.identifier == dest_path.identifier + def can_intra_move(self, other: provider.BaseProvider, path: wb_path.WaterButlerPath=None) -> bool: return self == other @@ -195,6 +198,7 @@ async def intra_copy(self, # type: ignore src_path: wb_path.WaterButlerPath, dest_path: wb_path.WaterButlerPath) \ -> typing.Tuple[typing.Union[BoxFileMetadata, BoxFolderMetadata], bool]: + if dest_path.identifier is not None: await dest_provider.delete(dest_path) @@ -223,6 +227,7 @@ async def intra_move(self, # type: ignore dest_provider: provider.BaseProvider, src_path: wb_path.WaterButlerPath, dest_path: wb_path.WaterButlerPath) -> typing.Tuple[BaseBoxMetadata, bool]: + if dest_path.identifier is not None and str(dest_path).lower() != str(src_path).lower(): await dest_provider.delete(dest_path) diff --git a/waterbutler/providers/dropbox/provider.py b/waterbutler/providers/dropbox/provider.py index 3353770f5..40cdb1435 100644 --- a/waterbutler/providers/dropbox/provider.py +++ b/waterbutler/providers/dropbox/provider.py @@ -151,6 +151,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( @@ -192,6 +193,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 exceptions.InvalidPathError( @@ -370,6 +372,9 @@ async def create_folder(self, path: WaterButlerPath, **kwargs) -> DropboxFolderM ) return DropboxFolderMetadata(data, 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 73712d601..92d3b56c2 100644 --- a/waterbutler/providers/googledrive/provider.py +++ b/waterbutler/providers/googledrive/provider.py @@ -135,6 +135,9 @@ def can_duplicate_names(self) -> bool: def default_headers(self) -> dict: return {'authorization': 'Bearer {}'.format(self.token)} + 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: @@ -151,6 +154,7 @@ async def intra_move(self, # type: ignore src_path: wb_path.WaterButlerPath, dest_path: wb_path.WaterButlerPath) \ -> typing.Tuple[BaseGoogleDriveMetadata, bool]: + self.metrics.add('intra_move.destination_exists', dest_path.identifier is not None) if dest_path.identifier: await dest_provider.delete(dest_path) @@ -187,6 +191,7 @@ async def intra_copy(self, src_path: wb_path.WaterButlerPath, dest_path: wb_path.WaterButlerPath) \ -> typing.Tuple[GoogleDriveFileMetadata, bool]: + 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