enso-org/enso
1
1
Fork 0
mirror of https://github.com/enso-org/enso.git synced 2025-01-09 01:26:59 +03:00
Code Issues Projects Releases Wiki Activity

Fix copy_to/move_to edge case when source and target are the same (#9798)

Browse Source 
- Adds tests for an edge case where the source and target of copy/move operation are the same.
- And fixes the behaviour to be as expected.
This commit is contained in:
Radosław Waśko 2024-04-29 14:29:07 +02:00 committed by GitHub
parent 210f0482a4
commit 16e1999040
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 69 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
distribution/lib/Standard/AWS/0.0.0-dev/src/S3/S3_File.enso
View File

@ -245,10 +245,11 @@ type S3_File
case destination_writable.file of
# Special shortcut for more efficient handling of S3 file copying (no need to move the data to our machine)
s3_destination : S3_File ->
if replace_existing.not && s3_destination.exists then Error.throw (File_Error.Already_Exists destination) else
destination_path = s3_destination.s3_path
translate_file_errors self <| S3.copy_object self.s3_path.bucket self.s3_path.key destination_path.bucket destination_path.key self.credentials
. if_not_error destination_writable.file_for_return
if s3_destination == self then (if self.exists then self else Error.throw (File_Error.Not_Found self)) else
if replace_existing.not && s3_destination.exists then Error.throw (File_Error.Already_Exists destination) else
destination_path = s3_destination.s3_path
translate_file_errors self <| S3.copy_object self.s3_path.bucket self.s3_path.key destination_path.bucket destination_path.key self.credentials
. if_not_error destination_writable.file_for_return
_ -> generic_copy self destination_writable replace_existing
## ICON data_output
@ -265,12 +266,14 @@ type S3_File
- destination: the destination to move the file to.
- replace_existing: specifies if the operation should proceed if the
destination file already exists. Defaults to `False`.
move_to : File_Like -> Boolean -> Nothing ! File_Error
move_to : File_Like -> Boolean -> Any ! File_Error
move_to self (destination : File_Like) (replace_existing : Boolean = False) = Data_Link_Helpers.disallow_links_in_move self destination <|
Context.Output.if_enabled disabled_message="File moving is forbidden as the Output context is disabled." panic=False <|
r = self.copy_to destination replace_existing=replace_existing
r.if_not_error <|
self.delete.if_not_error r
# If source and destination are the same, we do not want to delete the file
if destination.underlying == self then r else
self.delete.if_not_error r
## GROUP Standard.Base.Operators
ICON folder

11
distribution/lib/Standard/Base/0.0.0-dev/src/Enso_Cloud/Data_Link.enso
View File

@ -8,9 +8,11 @@ import project.Errors.File_Error.File_Error
import project.Errors.Illegal_Argument.Illegal_Argument
import project.Errors.Illegal_State.Illegal_State
import project.Errors.Problem_Behavior.Problem_Behavior
import project.Meta
import project.Nothing.Nothing
import project.Panic.Panic
import project.System.File.Data_Link_Access.Data_Link_Access
import project.System.File.File
import project.System.File.File_Access.File_Access
import project.System.File.Generic.File_Like.File_Like
import project.System.File.Generic.Writable_File.Writable_File
@ -92,8 +94,13 @@ type Data_Link
existing file. By default, the operation will fail if the file already
exists.
move (source : File_Like) (target : File_Like) (replace_existing : Boolean = False) -> Any ! File_Error =
Data_Link.copy source target replace_existing . if_not_error <|
source.underlying.delete . if_not_error target.underlying
is_same_file = case source.underlying of
f : File ->
target.underlying.is_a File && f.absolute.normalize.path == target.underlying.absolute.normalize.path
_ -> source.underlying == target.underlying
if is_same_file then Error.throw (Illegal_Argument.Error "Source and target files are the same.") else
Data_Link.copy source target replace_existing . if_not_error <|
source.underlying.delete . if_not_error target.underlying
## PRIVATE
Reads the raw configuration data of a data-link, as plain text.

6
distribution/lib/Standard/Base/0.0.0-dev/src/Enso_Cloud/Enso_File.enso
View File

@ -375,12 +375,14 @@ type Enso_File
- destination: the destination to move the file to.
- replace_existing: specifies if the operation should proceed if the
destination file already exists. Defaults to `False`.
move_to : File_Like -> Boolean -> Nothing ! File_Error
move_to : File_Like -> Boolean -> Any ! File_Error
move_to self (destination : File_Like) (replace_existing : Boolean = False) = Data_Link_Helpers.disallow_links_in_move self destination <|
# TODO we could have a fast path if Cloud will support renaming files
r = self.copy_to destination replace_existing
r.if_not_error <|
self.delete . if_not_error r
# If source and destination are the same, we do not want to delete the file
if destination.underlying == self then r else
self.delete . if_not_error r
## GROUP Operators
ICON folder

4
distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso
View File

@ -671,7 +671,7 @@ type File
- destination: the destination to move the file to.
- replace_existing: specifies if the operation should proceed if the
destination file already exists. Defaults to `False`.
copy_to : File_Like -> Boolean -> File ! File_Error
copy_to : File_Like -> Boolean -> Any ! File_Error
copy_to self (destination : File_Like) (replace_existing : Boolean = False) = Data_Link_Helpers.disallow_links_in_copy self destination <|
Context.Output.if_enabled disabled_message="File copying is forbidden as the Output context is disabled." panic=False <|
## We defer the `Writable_File` conversion after the 'disallow links' check,
@ -689,7 +689,7 @@ type File
- destination: the destination to move the file to.
- replace_existing: specifies if the operation should proceed if the
destination file already exists. Defaults to `False`.
move_to : File_Like -> Boolean -> File ! File_Error
move_to : File_Like -> Boolean -> Any ! File_Error
move_to self (destination : File_Like) (replace_existing : Boolean = False) = Data_Link_Helpers.disallow_links_in_move self destination <|
Context.Output.if_enabled disabled_message="File moving is forbidden as the Output context is disabled." panic=False <|
## We defer the `Writable_File` conversion after the 'disallow links' check,

12
distribution/lib/Standard/Base/0.0.0-dev/src/System/File/Generic/File_Write_Strategy.enso
View File

@ -152,8 +152,10 @@ dry_run_behavior file behavior:Existing_File_Behavior -> Dry_Run_File_Settings =
Generic `copy` implementation between two backends.
The files only need to support `with_input_stream` and `with_output_stream`.
generic_copy source (destination : Writable_File) replace_existing =
source.with_input_stream [File_Access.Read, Data_Link_Access.No_Follow] input_stream->
replace_options = if replace_existing then [File_Access.Create, File_Access.Truncate_Existing] else [File_Access.Create_New]
options = [File_Access.Write, Data_Link_Access.No_Follow] + replace_options
destination.with_output_stream options output_stream->
output_stream.write_stream input_stream . if_not_error destination.file_for_return
# If source and destination are the same - there is nothing to do.
if source == destination.file then (if source.exists then destination.file_for_return else Error.throw (File_Error.Not_Found source)) else
source.with_input_stream [File_Access.Read, Data_Link_Access.No_Follow] input_stream->
replace_options = if replace_existing then [File_Access.Create, File_Access.Truncate_Existing] else [File_Access.Create_New]
options = [File_Access.Write, Data_Link_Access.No_Follow] + replace_options
destination.with_output_stream options output_stream->
output_stream.write_stream input_stream . if_not_error destination.file_for_return

4
test/AWS_Tests/src/S3_Spec.enso
View File

@ -19,7 +19,7 @@ import enso_dev.Table_Tests.Util
from Standard.Test import all
import enso_dev.Base_Tests.Network.Enso_Cloud.Cloud_Tests_Setup.Cloud_Tests_Setup
import enso_dev.Base_Tests.System.File_Spec as Local_File_Spec
test_credentials -> AWS_Credential ! Illegal_State =
access_key_id = Environment.get "ENSO_LIB_S3_AWS_ACCESS_KEY_ID"
@ -521,6 +521,8 @@ add_specs suite_builder =
r.should_fail_with File_Error
r.catch.should_be_a File_Error.Not_Found
Local_File_Spec.add_copy_edge_cases_specs group_builder my_writable_dir file_from_path=(S3_File.new _ test_credentials)
suite_builder.group "DataLinks to S3_File" pending=api_pending group_builder->
group_builder.specify "should be able to read a data link of an S3 File" <| with_default_credentials <|
# It reads the datalink description and then reads the actual S3 file contents:

1
test/Base_Tests/src/Network/Enso_Cloud/Enso_File_Spec.enso
View File

@ -284,6 +284,7 @@ add_specs suite_builder setup:Cloud_Tests_Setup = suite_builder.group "Enso Clou
nested_file.is_writable . should_be_a Boolean
Local_File_Spec.add_create_and_delete_directory_specs group_builder test_root.get
Local_File_Spec.add_copy_edge_cases_specs group_builder test_root.get
main filter=Nothing =
setup = Cloud_Tests_Setup.prepare

34
test/Base_Tests/src/System/File_Spec.enso
View File

@ -227,6 +227,8 @@ add_specs suite_builder =
f.delete_if_exists
g.delete_if_exists
add_copy_edge_cases_specs group_builder (enso_project.data / "transient")
group_builder.specify "should handle exceptions when deleting a missing file" <|
file = File.new "does_not_exist.txt"
result = file.delete
@ -306,6 +308,7 @@ add_specs suite_builder =
contents_2 = File.new file . read_bytes
contents_2.take (First 6) . should_equal [67, 117, 112, 99, 97, 107]
suite_builder.group "Path Operations" group_builder->
group_builder.specify "should allow going above the first part of a relative directory by resolving it to absolute" <|
parent_file = (File.new '.').parent
parent_file.should_be_a File
@ -960,6 +963,37 @@ add_create_and_delete_directory_specs group_builder ~parent_dir =
dir.delete_if_exists . should_equal Nothing
add_copy_edge_cases_specs group_builder ~parent_dir file_from_path=File.new =
group_builder.specify "should allow copy/move edge case with source=target" <|
file = parent_dir / "src-target-in-one.txt"
file.delete_if_exists
# Same file but through a different path
same_file = file_from_path (parent_dir.path + "/foo/../src-target-in-one.txt")
# Source is not found
r_copy_not_found = same_file.copy_to file
r_copy_not_found.should_fail_with File_Error
r_copy_not_found.catch.should_be_a File_Error.Not_Found
r_move_not_found = same_file.move_to file
r_move_not_found.should_fail_with File_Error
r_move_not_found.catch.should_be_a File_Error.Not_Found
"Foo".write file on_existing_file=Existing_File_Behavior.Overwrite
[False, True].map replace_existing-> Test.with_clue "(replace_existing="+replace_existing.to_text+") " <|
# Copying the file to itself should succeed and just not change anything.
r_copy = same_file.copy_to file replace_existing=replace_existing
r_copy.should_equal file
r_copy.read . should_equal "Foo"
# Moving the file to itself should succeed and just not change anything.
r_move = same_file.move_to file replace_existing=replace_existing
r_move.should_equal file
file.exists . should_be_true
file.read . should_equal "Foo"
main filter=Nothing =
suite = Test.build suite_builder->
add_specs suite_builder