Cloud Integration updates: renames in file metadata structure, remove path resolver workaround, partial fix for datalink resolver issue (#10050)

- Supersedes #9966 as I wanted to test these changes in one go.
- Fixes #10037 caused by lack of CI check and my oversight (forgot to run full tests after a minor change).
- Fixes a regression after [file metadata fields were renamed](c09d856ac8 (diff-9f59b6a0ee3155efecdc70c1ea0c90ab5cde00b5623d84363118b1793f941c46R2037)).
- Fixes handling of creating new datalinks and using them after cache was cleared (e.g. workflow restart).
- This was caused by troubles with path resolver.
- The fix addresses the most common issue and adds a test for it (test flushes the caches to ensure path resolver is used instead of the cached value).
- Some related issues were discovered on the cloud side, tracked by https://github.com/enso-org/cloud-v2/issues/1252
This commit is contained in:
Radosław Waśko 2024-05-23 18:05:48 +02:00 committed by GitHub
parent 094076a9e8
commit 9d75f79ff9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 48 additions and 44 deletions

View File

@ -126,7 +126,7 @@ type Enso_File
asset = Existing_Enso_Asset.get_asset_reference_for self
if asset.is_regular_file.not then Error.throw (Illegal_Argument.Error "`creation_time` can only be queried for files.") else
metadata = asset.get_file_description |> get_required_field "metadata"
Date_Time.parse (get_required_field "created_at" metadata expected_type=Text) Date_Time_Formatter.iso_offset_date_time
Date_Time.parse (get_required_field "createdAt" metadata expected_type=Text) Date_Time_Formatter.iso_offset_date_time
. catch Time_Error error-> Error.throw (Enso_Cloud_Error.Invalid_Response_Payload error)
## GROUP Metadata
@ -137,7 +137,7 @@ type Enso_File
asset = Existing_Enso_Asset.get_asset_reference_for self
if asset.is_regular_file.not then Error.throw (Illegal_Argument.Error "`last_modified_time` can only be queried for files.") else
metadata = asset.get_file_description |> get_required_field "metadata"
Date_Time.parse (get_required_field "modified_at" metadata expected_type=Text) Date_Time_Formatter.iso_offset_date_time
Date_Time.parse (get_required_field "modifiedAt" metadata expected_type=Text) Date_Time_Formatter.iso_offset_date_time
. catch Time_Error error-> Error.throw (Enso_Cloud_Error.Invalid_Response_Payload error)
## GROUP Metadata

View File

@ -1,6 +1,7 @@
private
import project.Any.Any
import project.Data.Index_Sub_Range.Index_Sub_Range
import project.Data.Json.Invalid_JSON
import project.Data.Json.JS_Object
import project.Data.Map.Map
@ -23,7 +24,7 @@ import project.System.File.File
import project.System.Output_Stream.Output_Stream
from project.Data.Boolean import Boolean, False, True
from project.Data.Text.Extensions import all
from project.Enso_Cloud.Data_Link_Helpers import data_link_encoding
from project.Enso_Cloud.Data_Link_Helpers import data_link_encoding, data_link_extension
from project.Enso_Cloud.Public_Utils import get_required_field
## PRIVATE
@ -44,7 +45,7 @@ upload_file (local_file : File) (destination : Enso_File) (replace_existing : Bo
The `create_action` function is called with the existing asset for the parent
directory and for the file, if it already exists, or `Nothing` otherwise, and
with a mapping of error handlers that may be added to the request.
generic_create_asset (destination : Enso_File) (allow_existing : Boolean) (create_action : Existing_Enso_Asset -> (Existing_Enso_Asset | Nothing) -> Any) -> Any =
generic_create_asset (destination : Enso_File) (allow_existing : Boolean) (create_action : Existing_Enso_Asset -> (Existing_Enso_Asset | Nothing) -> Map -> Any) -> Any =
parent_directory = destination.parent
if parent_directory.is_nothing then Error.throw (Illegal_Argument.Error "Please provide an asset name inside of the root directory.") else
parent_directory_asset = Existing_Enso_Asset.get_asset_reference_for parent_directory
@ -72,7 +73,7 @@ generic_create_asset (destination : Enso_File) (allow_existing : Boolean) (creat
where the first element is the request body and the second element is the result to be returned.
It is executed lazily, only after all pre-conditions are successfully met.
perform_upload (destination : Enso_File) (allow_existing : Boolean) (~generate_request_body_and_result) =
generic_create_asset destination allow_existing parent_directory_asset-> existing_asset->
generic_create_asset destination allow_existing parent_directory_asset-> existing_asset-> error_handlers->
if existing_asset.is_nothing.not && existing_asset.asset_type != Enso_Asset_Type.File then Error.throw (Illegal_Argument.Error "The destination must be a path to a file, not "+existing_asset.asset_type.to_text+".") else
existing_asset_id = existing_asset.if_not_nothing <| existing_asset.id
file_name = destination.name
@ -84,7 +85,7 @@ perform_upload (destination : Enso_File) (allow_existing : Boolean) (~generate_r
_ -> base_uri . add_query_argument "file_id" existing_asset_id
pair = generate_request_body_and_result
Asset_Cache.invalidate destination
response = Utils.http_request_as_json HTTP_Method.Post full_uri pair.first
response = Utils.http_request_as_json HTTP_Method.Post full_uri pair.first error_handlers=error_handlers
response.if_not_error <|
id = get_required_field "id" response expected_type=Text
Asset_Cache.update destination (Existing_Enso_Asset.from_id_and_title id file_name) . if_not_error <|
@ -110,23 +111,24 @@ create_directory_with_parents (target : Enso_File) -> Existing_Enso_Asset =
## PRIVATE
create_datalink_from_stream_action (destination : Enso_File) (allow_existing : Boolean) (stream_action : Output_Stream -> Any) =
generic_create_asset destination allow_existing parent_directory_asset-> existing_asset->
generic_create_asset destination allow_existing parent_directory_asset-> existing_asset-> error_handlers->
if existing_asset.is_nothing.not && existing_asset.asset_type != Enso_Asset_Type.Data_Link then Error.throw (Illegal_Argument.Error "The destination must be a path to a Data Link, not "+existing_asset.asset_type.to_text+".") else
# TODO once path resolver is updated to automatically add .datalink extension, we should strip the extension from the name
title = destination.name
stream_result = Output_Stream.with_memory_stream stream_action
raw_bytes = stream_result.first
action_result = stream_result.second
raw_json = Text.from_bytes raw_bytes data_link_encoding . parse_json . catch Invalid_JSON error->
Error.throw (Illegal_Argument.Error "A datalink can be created only with a valid JSON payload, but the written payload was invalid: "+error.to_display_text cause=error)
stream_result.if_not_error <|
payload = JS_Object.from_pairs <|
[["parentDirectoryId", parent_directory_asset.id], ["name", title], ["value", raw_json]]
+ (if existing_asset.is_nothing then [] else [["datalinkId", existing_asset.id]])
file_name = destination.name
if file_name.ends_with data_link_extension . not then Error.throw (Illegal_Argument.Error "A datalink must have a name ending with "+data_link_extension+", but the provided name was: "+file_name) else
title = file_name.drop (Index_Sub_Range.Last data_link_extension.length)
stream_result = Output_Stream.with_memory_stream stream_action
raw_bytes = stream_result.first
action_result = stream_result.second
raw_json = Text.from_bytes raw_bytes data_link_encoding . parse_json . catch Invalid_JSON error->
Error.throw (Illegal_Argument.Error "A datalink can be created only with a valid JSON payload, but the written payload was invalid: "+error.to_display_text cause=error)
stream_result.if_not_error <|
payload = JS_Object.from_pairs <|
[["parentDirectoryId", parent_directory_asset.id], ["name", title], ["value", raw_json]]
+ (if existing_asset.is_nothing then [] else [["datalinkId", existing_asset.id]])
Asset_Cache.invalidate destination
response = Utils.http_request_as_json HTTP_Method.Post Utils.datalinks_api payload
response.if_not_error <|
id = get_required_field "id" response expected_type=Text
Asset_Cache.update destination (Existing_Enso_Asset.from_id_and_title id title) . if_not_error <|
action_result
Asset_Cache.invalidate destination
response = Utils.http_request_as_json HTTP_Method.Post Utils.datalinks_api payload error_handlers=error_handlers
response.if_not_error <|
id = get_required_field "id" response expected_type=Text
Asset_Cache.update destination (Existing_Enso_Asset.from_id_and_title id title) . if_not_error <|
action_result

View File

@ -11,7 +11,6 @@ import project.Enso_Cloud.Cloud_Caching_Settings
import project.Enso_Cloud.Enso_File.Enso_Asset_Type
import project.Enso_Cloud.Enso_File.Enso_File
import project.Enso_Cloud.Enso_User.Enso_User
import project.Enso_Cloud.Errors.Enso_Cloud_Error
import project.Enso_Cloud.Internal.Utils
import project.Error.Error
import project.Errors.Common.Not_Found
@ -23,6 +22,7 @@ import project.Panic.Panic
import project.Runtime.Context
from project.Data.Boolean import Boolean, False, True
from project.Data.Text.Extensions import all
from project.Enso_Cloud.Data_Link_Helpers import data_link_extension
from project.Enso_Cloud.Public_Utils import get_required_field
## PRIVATE
@ -53,8 +53,8 @@ type Existing_Enso_Asset
## PRIVATE
name self -> Text =
added_extension = case self.asset_type of
Enso_Asset_Type.Data_Link -> data_link_extension
# TODO enable when using new path resolver that should support these
#Enso_Asset_Type.Data_Link -> ".datalink"
#Enso_Asset_Type.Secret -> ".secret"
#Enso_Asset_Type.Project -> ".enso"
_ -> ""
@ -82,22 +82,14 @@ type Existing_Enso_Asset
## PRIVATE
Resolves a path to an existing asset in the cloud.
resolve_path (path : Text) ~if_not_found =
handle_not_found _ = if_not_found
resolve_path (path : Text) ~if_not_found = path.if_not_error <|
handle_not_found _ = Error.throw Not_Found
error_handlers = Map.from_vector [["resource_missing", handle_not_found]]
uri = ((URI.from Utils.cloud_root_uri) / "path/resolve") . add_query_argument "path" path
r = Utils.http_request_as_json HTTP_Method.Get uri error_handlers=error_handlers
## Workaround to keep compatibility with old cloud API
TODO remove it after https://github.com/enso-org/cloud-v2/pull/1236 has been deployed
response = r.catch Enso_Cloud_Error error-> case error of
Enso_Cloud_Error.Unexpected_Service_Error _ status _ ->
if status.code != 404 then r else
old_uri = (URI.from Utils.cloud_root_uri) / "path/resolve"
old_payload = JS_Object.from_pairs [["path", path]]
Context.Output.with_enabled <| Utils.http_request_as_json HTTP_Method.Post old_uri old_payload error_handlers=error_handlers
_ -> r
response = Utils.http_request_as_json HTTP_Method.Get uri error_handlers=error_handlers
Existing_Enso_Asset.from_json response
. catch Not_Found _-> if_not_found
## PRIVATE
is_directory self = self.asset_type == Enso_Asset_Type.Directory

View File

@ -22,14 +22,14 @@ from project.Data.Boolean import Boolean, False, True
get_required_field (key : Text) js_object (show_value : Boolean = True) (expected_type = Any) = case js_object of
_ : JS_Object ->
handle_missing =
suffix = if show_value then " in "+js_object.to_display_text else ""
suffix = if show_value then " in "+js_object.to_text else ""
Error.throw (Enso_Cloud_Error.Invalid_Response_Payload "Missing required field `"+key+"`"+suffix+".")
result = js_object.get key if_missing=handle_missing
result.if_not_error <| if result.is_a expected_type then result else
representation = if show_value then result.to_display_text else Meta.type_of result . to_display_text
representation = if show_value then result.to_text else Meta.type_of result . to_display_text
Error.throw (Enso_Cloud_Error.Invalid_Response_Payload "Expected field `"+key+"` to be of type "+expected_type.to_display_text+", but got "+representation+".")
_ ->
representation = if show_value then js_object.to_display_text else Meta.type_of js_object . to_display_text
representation = if show_value then js_object.to_text else Meta.type_of js_object . to_display_text
Error.throw (Enso_Cloud_Error.Invalid_Response_Payload "Expected a JSON object, but got "+representation+".")
## PRIVATE
@ -53,10 +53,10 @@ get_optional_field (key : Text) js_object (~if_missing = Nothing) (show_value :
case result of
Nothing -> if_missing
_ -> if result.is_a expected_type then result else
representation = if show_value then result.to_display_text else Meta.type_of result . to_display_text
representation = if show_value then result.to_text else Meta.type_of result . to_display_text
Error.throw (Enso_Cloud_Error.Invalid_Response_Payload "Expected field `"+key+"` to be of type "+expected_type.to_display_text+", but got "+representation+".")
_ ->
representation = if show_value then js_object.to_display_text else Meta.type_of js_object . to_display_text
representation = if show_value then js_object.to_text else Meta.type_of js_object . to_display_text
Error.throw (Enso_Cloud_Error.Invalid_Response_Payload "Expected a JSON object, but got "+representation+".")
## PRIVATE

View File

@ -40,6 +40,16 @@ add_specs suite_builder setup:Cloud_Tests_Setup =
r.should_be_a Text
r.should_contain "Cupcake"
## Normally, once we created an asset, its information is cached for a brief period of time, so any tests will rely on this cache also.
Here we explicitly clear the cache to test the path resolver as well.
group_builder.specify "should be able to resolve data link by path after caches were cleared" <|
Cloud_Tests_Setup.reset
datalink = Enso_File.new (test_root.get.path + "/TestDataLink-HTTP.datalink")
datalink.asset_type.should_equal Enso_Asset_Type.Data_Link
datalink.exists.should_be_true
Test.with_retries <|
datalink.read . should_contain "Cupcake"
group_builder.specify "will report which library is missing if a datalink relying on other library is accessed" <|
datalink = test_root.get / "TestDataLink-S3.datalink"
Data_Link.write_raw_config datalink s3_data_link_content . should_succeed

View File

@ -940,7 +940,7 @@ add_data_link_specs suite_builder =
r.catch.to_display_text . should_contain "The Postgres Data Link cannot be saved to a file."
cloud_setup = Cloud_Tests_Setup.prepare
suite_builder.group "[PostgreSQL] Saving to DataLink" pending=cloud_setup.real_cloud_pending group_builder->
suite_builder.group "[PostgreSQL] Saving to DataLink" pending=(pending.if_nothing cloud_setup.real_cloud_pending) group_builder->
test_root = Temporary_Directory.make "Postgres-DataLinks"
group_builder.teardown test_root.cleanup
group_builder.specify "allows to save an established connection as a Data Link" <|