Skip to content

Commit

Permalink
[s3] Skip move/copy operation if parent path of source is equal to de…
Browse files Browse the repository at this point in the history
…stination path (#3478)

What changes were proposed in this pull request?
- Whenever someone attempt to copy multiple files on Hue File Browser, they can encounter dataloss if they do not wait for the model pop-up to fully load.

RCA
- Found the root cause and it's because of a bad call to Hue server which is sending wrong destination path.
- When the modal pops up to select the destination path, it tries to list down the content of the directory chosen and that takes some extra seconds but the button to move is clickable still. When someone clicks the 'Move' button but the path has not loaded correctly, it sends the wrong path (either the source path or default user home path) instead of actual destination path with the request.
- This messes up the move operation in the backend where it actually doesn't move stuff to the destination because the path was incorrect or the keys were already present in the wrong path so it skips them, and afterwards cleanup the keys from the source path.

Fix
- Adding guardrail checks to skip the operation when the parent path of source is equal to destination for copy and move operations.

How was this patch tested?
- Tested manually.
  • Loading branch information
Harshg999 authored Oct 3, 2023
1 parent 093da4a commit 68d4816
Showing 1 changed file with 20 additions and 2 deletions.
22 changes: 20 additions & 2 deletions desktop/libs/aws/src/aws/s3/s3fs.py
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,10 @@ def _copy(self, src, dst, recursive, use_src_basename):
if src_st.isDir and dst_st and not dst_st.isDir:
raise S3FileSystemException("Cannot overwrite non-directory '%s' with directory '%s'" % (dst, src))

# Skip operation if destination path is same as source path
if self._check_key_parent_path(src, dst):
raise S3FileSystemException('Destination path is same as the source path, skipping the operation.')

src_bucket, src_key = s3.parse_uri(src)[:2]
dst_bucket, dst_key = s3.parse_uri(dst)[:2]

Expand Down Expand Up @@ -492,8 +496,22 @@ def _copy(self, src, dst, recursive, use_src_basename):
@auth_error_handler
def rename(self, old, new):
new = s3.abspath(old, new)
self.copy(old, new, recursive=True)
self.rmtree(old, skipTrash=True)

# Skip operation if destination path is same as source path
if not self._check_key_parent_path(old, new):
self.copy(old, new, recursive=True)
self.rmtree(old, skipTrash=True)
else:
raise S3FileSystemException('Destination path is same as source path, skipping the operation.')

@translate_s3_error
@auth_error_handler
def _check_key_parent_path(self, src, dst):
# Return True if parent path of source is same as destination path.
if S3FileSystem.parent_path(src) == dst:
return True
else:
return False

@translate_s3_error
@auth_error_handler
Expand Down

0 comments on commit 68d4816

Please sign in to comment.