Download_Mode for File, S3_File and Enso_File (#12017)

Author: GregoryTravis

CHANGELOG.md

@@ -167,6 +167,7 @@
 - [Added `add_group_number` to the in-memory database.[11818]
 - [The reload button clears the HTTP cache.][11673]
 - [SQL Server Support for Aggregate][11811]
+- [Added `Download_Mode` parameter to `Data.download`.][12017]
 
 [11235]: https://github.com/enso-org/enso/pull/11235
 [11255]: https://github.com/enso-org/enso/pull/11255
@@ -178,6 +179,7 @@
 [11818]: https://github.com/enso-org/enso/pull/11818
 [11673]: https://github.com/enso-org/enso/pull/11673
 [11811]: https://github.com/enso-org/enso/pull/11811
+[12017]: https://github.com/enso-org/enso/pull/12017
 
 #### Enso Language & Runtime
 

distribution/lib/Standard/Base/0.0.0-dev/src/Data.enso

@@ -1,4 +1,5 @@
 import project.Any.Any
+import project.Data.Download.Download_Mode.Download_Mode
 import project.Data.Pair.Pair
 import project.Data.Read.Many_Files_List.Many_Files_List
 import project.Data.Read.Return_As.Return_As
@@ -437,23 +438,33 @@ post (uri:(URI | Text)=(Missing_Argument.throw "uri")) (body:Request_Body=..Empt
    Arguments:
    - uri: The URI to fetch.
    - file: The file to write the response to.
+   - replace_existing: Determines when `download` should proceed with the
+     download, if the destination file already exists. (If the destination file
+     does not exist, the download always proceeds).
+     Options include:
+         - `If_Not_Exists`: Download the file if it does not already exist on
+           disk. (The default.)
+         - `If_Older_Than age`: Download the file if the existing file is older
+           than the specified age.
+         - `Always`: Always download.
    - method: The HTTP method to use. Must be one of `HTTP_Method.Get`,
      `HTTP_Method.Head`, `HTTP_Method.Delete`, `HTTP_Method.Options`.
      Defaults to `HTTP_Method.Get`.
    - headers: The headers to send with the request. Defaults to an empty vector.
 @uri (Text_Input display=..Always)
 @headers Header.default_widget
-download : (URI | Text) -> Writable_File -> HTTP_Method -> Vector (Header | Pair Text Text) -> File ! Request_Error | HTTP_Error
-download (uri:(URI | Text)=(Missing_Argument.throw "uri")) file:Writable_File (method:HTTP_Method=..Get) (headers:(Vector (Header | Pair Text Text))=[]) =
+download : (URI | Text) -> Writable_File -> Download_Mode -> HTTP_Method -> Vector (Header | Pair Text Text) -> File ! Request_Error | HTTP_Error
+download (uri:(URI | Text)=(Missing_Argument.throw "uri")) file:Writable_File (replace_existing:Download_Mode=..If_Not_Exists) (method:HTTP_Method=..Get) (headers:(Vector (Header | Pair Text Text))=[]) =
     Context.Output.if_enabled disabled_message="As writing is disabled, cannot download to a file. Press the Write button ▶ to perform the operation." panic=False <|
-        response = HTTP.fetch uri method headers cache_policy=Cache_Policy.No_Cache
-        case Data_Link.is_data_link_from_metadata response.body.metadata of
-            True ->
-                # If the resource was a data link, we follow it, download the target data and try to write it to a file.
-                data_link = Data_Link_Helpers.interpret_json_as_data_link response.decode_as_json
-                Data_Link_Helpers.save_data_link_to_file data_link file
-            False ->
-                response.write file
+        if replace_existing.should_download file then
+            response = HTTP.fetch uri method headers cache_policy=Cache_Policy.No_Cache
+            case Data_Link.is_data_link_from_metadata response.body.metadata of
+                True ->
+                    # If the resource was a data link, we follow it, download the target data and try to write it to a file.
+                    data_link = Data_Link_Helpers.interpret_json_as_data_link response.decode_as_json
+                    Data_Link_Helpers.save_data_link_to_file data_link file
+                False ->
+                    response.write file
 
 ## If the `format` is set to `Raw_Response`, a raw HTTP `Response` is returned
    that can be then processed further manually.

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Download/Download_Mode.enso

@@ -0,0 +1,27 @@
+import project.Data.Time.Date_Time.Date_Time
+import project.Data.Time.Duration.Duration
+import project.Nothing.Nothing
+import project.System.File.Generic.Writable_File.Writable_File
+from project.Data.Boolean import Boolean, False, True
+
+type Download_Mode
+    ## Download the file if it does not already exist on disk.
+    If_Not_Exists
+
+    ## Download the file if it is older than the specified age.
+    If_Older_Than age:Duration
+
+    ## Always download.
+    Always
+
+    ## PRIVATE
+       Determine if a file should be downloaded, based on the file type,
+       download mode, and file age.
+    should_download self (file:Writable_File) -> Boolean =
+        case self of
+            Download_Mode.If_Not_Exists ->
+                file.file.exists.not
+            Download_Mode.If_Older_Than age ->
+                file.file.exists.not || file.file.last_modified_time < (Date_Time.now - age)
+            Download_Mode.Always ->
+                True

distribution/lib/Standard/Base/0.0.0-dev/src/Main.enso

@@ -8,6 +8,7 @@ export project.Data.Boolean.Boolean.True
 export project.Data.Decimal.dec
 export project.Data.Decimal.Decimal
 export project.Data.Dictionary.Dictionary
+export project.Data.Download.Download_Mode.Download_Mode
 export project.Data.Filter_Condition.Filter_Action
 export project.Data.Filter_Condition.Filter_Condition
 export project.Data.Hashset.Hashset

std-bits/table/src/main/java/org/enso/table/excel/ExcelConnectionPool.java

@@ -201,6 +201,11 @@ private String getKeyForFile(File file) throws IOException {
   }
 
   void release(ReadOnlyExcelConnection excelConnection) throws IOException {
+    System.out.println("AAAo");
+    System.err.println("AAAo");
+    new Exception().printStackTrace();
+    System.out.println("AAAo2");
+    System.err.println("AAAo2");
     synchronized (this) {
       excelConnection.record.refCount--;
       if (excelConnection.record.refCount <= 0) {

test/AWS_Tests/src/S3_Spec.enso

@@ -18,6 +18,7 @@ import enso_dev.Table_Tests.Util
 
 from Standard.Test import all
 
+import enso_dev.Base_Tests.Data.Data_Spec
 import enso_dev.Base_Tests.Network.Enso_Cloud.Cloud_Tests_Setup.Cloud_Tests_Setup
 import enso_dev.Base_Tests.Network.Enso_Cloud.Cloud_Tests_Setup.Temporary_Directory
 import enso_dev.Base_Tests.System.File_Spec as Local_File_Spec
@@ -70,6 +71,9 @@ replace_username_in_data_link base_file =
    past we had a bug precisely due to relying on the defaults: https://github.com/enso-org/enso/issues/9284
 add_specs suite_builder =
     cloud_setup = Cloud_Tests_Setup.prepare
+
+    add_data_specs suite_builder writable_root
+
     suite_builder.group "S3 Path handling" group_builder->
         # Testing just path handling can be done on instances without any credentials:
         root = S3_File.new "s3://"+bucket_name+"/"
@@ -661,6 +665,10 @@ create_data_link_to_s3 credential:AWS_Credential link_location target:S3_File =
         . replace "<SECRET_KEY>" keys.second
     Data_Link.write_raw_config link_location new_content replace_existing=True
 
+add_data_specs suite_builder root =
+    file_maker path_element:Text = root / "data_spec" / (path_element+".txt")
+    Data_Spec.add_specs "(S3_File) " suite_builder file_maker
+
 main filter=Nothing =
     suite = Test.build suite_builder->
         add_specs suite_builder

test/Base_Tests/src/Data/Data_Spec.enso

@@ -0,0 +1,59 @@
+from Standard.Base import all
+
+from project.Network.Http.Http_Test_Setup import base_url_with_slash, pending_has_url
+
+from Standard.Test import all
+import Standard.Test.Test_Environment
+
+polyglot java import java.lang.Thread
+
+with_test_file f ~action =
+    f.delete_if_exists
+    Panic.with_finalizer (f.delete_if_exists) <|
+        action f
+
+## file_maker should take a path component and return a full file path, for example:
+   "if_not_exist"
+     =>
+   (enso_project.data / "transient" / "if_not_exist.txt")
+add_specs prefix suite_builder file_maker =
+    suite_builder.group prefix+"Download Mode" pending=pending_has_url group_builder->
+        url_n_bytes n = base_url_with_slash+'test_download?length='+n.to_text
+
+        group_builder.specify prefix+"Will always download a file for mode Always" <|
+            with_test_file (file_maker "always") file->
+                file.exists . should_be_false
+                Data.download (url_n_bytes 10) replace_existing=..Always file
+                first_contents = file.read
+                Data.download (url_n_bytes 11) replace_existing=..Always file
+                second_contents = file.read
+                first_contents . should_not_equal second_contents
+
+        group_builder.specify prefix+"Will download a file if it does not exist for default mode If_Not_Exists" <|
+            with_test_file (file_maker "if_not_exist") file->
+                file.exists . should_be_false
+                Data.download (url_n_bytes 10) file
+                first_contents = file.read
+                Data.download (url_n_bytes 11) file
+                second_contents = file.read
+                first_contents . should_equal second_contents
+
+        # With retries because it relies on a short `sleep`
+        group_builder.specify prefix+"Will download a file if it is older than a specified duration for mode If_Older_Than" <| Test.with_retries <|
+            with_test_file (file_maker "if_older_than") file->
+                sleep_duration_secs = 3
+
+                file.exists . should_be_false
+
+                Data.download (url_n_bytes 10) file
+                first_contents = file.read
+
+                Data.download (url_n_bytes 11) (replace_existing=..If_Older_Than (Duration.new seconds=sleep_duration_secs)) file
+                second_contents = file.read
+                first_contents . should_equal second_contents
+
+                Thread.sleep ((sleep_duration_secs + 0.5) * 1000)
+
+                Data.download (url_n_bytes 12) (replace_existing=..If_Older_Than (Duration.new seconds=sleep_duration_secs)) file
+                third_contents = file.read
+                first_contents . should_not_equal third_contents

test/Base_Tests/src/Network/Enso_Cloud/Enso_File_Spec.enso

@@ -11,6 +11,7 @@ import Standard.Base.Runtime.Context
 from Standard.Test import all
 import Standard.Test.Test_Environment
 
+import project.Data.Data_Spec
 import project.Network.Enso_Cloud.Cloud_Tests_Setup.Cloud_Tests_Setup
 import project.Network.Enso_Cloud.Cloud_Tests_Setup.Temporary_Directory
 
@@ -26,6 +27,8 @@ add_specs suite_builder setup:Cloud_Tests_Setup = suite_builder.group "Enso Clou
         Panic.rethrow <| "Hello Another!".write (sub / "another.txt")
     group_builder.teardown test_root.cleanup
 
+    add_data_specs suite_builder test_root.get
+
     group_builder.specify "should be able to list a directory" <|
         assets = Enso_File.home.list
         # We don't a priori know the contents, so we can only check very generic properties
@@ -394,6 +397,12 @@ add_specs suite_builder setup:Cloud_Tests_Setup = suite_builder.group "Enso Clou
     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
 
+add_data_specs suite_builder root =
+    my_dir = (root / "data_specs-").create_directory
+    my_dir.should_succeed
+    file_maker path_element:Text = my_dir / (path_element+".txt")
+    Data_Spec.add_specs "(Enso_File) " suite_builder file_maker
+
 main filter=Nothing =
     setup = Cloud_Tests_Setup.prepare
     suite = Test.build suite_builder->

test/Base_Tests/src/System/File_Spec.enso

@@ -13,6 +13,7 @@ polyglot java import org.enso.base_test_helpers.FileSystemHelper
 from Standard.Test import all
 import Standard.Test.Test_Environment
 
+import project.Data.Data_Spec
 import project.Network.Enso_Cloud.Cloud_Tests_Setup.Cloud_Tests_Setup
 
 ## We rely on a less strict equality for `File` as it is fine if a relative file gets resolved to absolute.
@@ -38,6 +39,8 @@ add_specs suite_builder =
         Platform.OS.Windows -> Nothing
         _ -> "This test runs only on Windows."
 
+    add_data_specs suite_builder
+
     suite_builder.group "File Operations" group_builder->
         group_builder.specify "should get name of the root" <|
             root = File.new "/"
@@ -1055,6 +1058,10 @@ add_create_and_delete_directory_specs group_builder ~parent_dir =
 
         dir.delete_if_exists . should_equal Nothing
 
+add_data_specs suite_builder =
+    file_maker path_element:Text = enso_project.data / "transient" / (path_element+".txt")
+    Data_Spec.add_specs "(File) " suite_builder file_maker
+
 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"