diff --git a/waterbutler/core/provider.py b/waterbutler/core/provider.py index 8b15e2a5c..003a419f0 100644 --- a/waterbutler/core/provider.py +++ b/waterbutler/core/provider.py @@ -197,12 +197,8 @@ 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', + 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]: """Moves a file or folder from the current provider to the specified one Performs a copy and then a delete. @@ -224,8 +220,9 @@ async def move(self, 'got_rename': rename is not None, }) - # This does not mean the `dest_path` is a folder, at this point it could just be the parent - # of the actual dest_path, or have a blank name + # This does not mean that the `dest_path` is a folder, at this point it could just be the + # parent of the actual destination file/folder, or have a blank name. Use `not .is_file` + # in the if statement for less confusion. if not dest_path.is_file: temp_path = await self.revalidate_path( dest_path, @@ -236,7 +233,6 @@ async def move(self, raise exceptions.OverwriteSelfError(src_path) if handle_naming: - dest_path = await dest_provider.handle_naming( src_path, dest_path, @@ -248,8 +244,8 @@ async def move(self, # files and folders shouldn't overwrite themselves if ( - self.shares_storage_root(dest_provider) and - src_path.materialized_path == dest_path.materialized_path + self.shares_storage_root(dest_provider) and + src_path.materialized_path == dest_path.materialized_path ): raise exceptions.OverwriteSelfError(src_path) @@ -267,13 +263,10 @@ async def move(self, return meta_data, created - async def copy(self, - dest_provider: 'BaseProvider', - src_path: wb_path.WaterButlerPath, - dest_path: wb_path.WaterButlerPath, - rename: str=None, conflict: str='replace', - handle_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_naming: bool=True) -> typing.Tuple[wb_metadata.BaseMetadata, bool]: + args = (dest_provider, src_path, dest_path) kwargs = {'rename': rename, 'conflict': conflict, 'handle_naming': handle_naming} @@ -283,8 +276,9 @@ async def copy(self, 'got_rename': rename is not None, }) - # This does not mean the `dest_path` is a folder, at this point it could just be the parent - # of the actual dest_path, or have a blank name + # This does not mean that the `dest_path` is a folder, at this point it could just be the + # parent of the actual destination file/folder, or have a blank name. Use `not .is_file` + # in the if statement for less confusion. if not dest_path.is_file: temp_path = await self.revalidate_path( dest_path, @@ -295,7 +289,6 @@ async def copy(self, raise exceptions.OverwriteSelfError(src_path) if handle_naming: - dest_path = await dest_provider.handle_naming( src_path, dest_path, @@ -399,7 +392,8 @@ async def handle_naming(self, 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. + """Given a :class:`.WaterButlerPath` and the desired name, handle any potential naming + issues. i.e.: @@ -413,24 +407,23 @@ async def handle_naming(self, :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 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` """ - # This function can either take a file path, or a parent path to make a file path out of. - + # Can't copy a directory to a file 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') - # This is confusing. `dest_path` at this point can refer to the parent or root of - # the file we want to move/copy. So even if moving/copying a file, this code will run if not dest_path.is_file: - # Directories always are going to be copied into - # cp /folder1/ /folder2/ -> /folder1/folder2/ + # At this point, `dest_path` must be the parent or root folder into which we want to + # move/copy the source. The file/folder is always going to be copied into the parent. + # For example: + # cp /file.txt /folder/ -> /folder/file.txt + # cp /folder1/ /folder2/ -> /folder1/folder2/ dest_path = await self.revalidate_path( dest_path, rename or src_path.name, @@ -438,7 +431,6 @@ async def handle_naming(self, ) dest_path, _ = await self.handle_name_conflict(dest_path, conflict=conflict) - return dest_path def will_self_overwrite(self,