From 47ca58fb47ea20cc43032768fc3e66e9bc010979 Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Tue, 6 Dec 2022 09:39:44 -0500 Subject: [PATCH 1/3] meta request: support for should_continue cancellation callback Add a boolean `continue_callback`, which polls a request-cancellation flag set by the user. This matches the use-case of the `ContinueRequestHandler` / `ContinueRequest` / `ShouldContinue` boolean functions used by `aws-sdk-cpp`. Passive polling is preferred over active cancellation, since the latter would require to store a reference to the s3 meta request. The polling function is evaluated at the following points during the lifetime of the meta request: 1. _Request preparation_ (`aws_s3_meta_request_prepare_request`). 2. _Request update_ (the `.update` handler invoked via `aws_s3_meta_request_update`). 3. _Http stream completion_ (`.on_complete` handler invoking `aws_s3_meta_request_send_request_finish_default`). 4. _Writing download data_ (`aws_s3_meta_request_stream_response_body_synced`). If a `continue_callback` is provided and if it returns `false` at any of these 4 evaluation times, the meta request completes with the `error_code` `AWS_ERROR_S3_CANCELED`. --- include/aws/s3/private/s3_meta_request_impl.h | 1 + include/aws/s3/s3_client.h | 11 ++ source/s3_meta_request.c | 7 +- tests/CMakeLists.txt | 2 + tests/s3_cancel_tests.c | 114 ++++++++++++++++++ tests/s3_tester.c | 14 +++ tests/s3_tester.h | 2 + 7 files changed, 149 insertions(+), 2 deletions(-) diff --git a/include/aws/s3/private/s3_meta_request_impl.h b/include/aws/s3/private/s3_meta_request_impl.h index 1f6164192..6964327ba 100644 --- a/include/aws/s3/private/s3_meta_request_impl.h +++ b/include/aws/s3/private/s3_meta_request_impl.h @@ -127,6 +127,7 @@ struct aws_s3_meta_request { /* Customer specified callbacks. */ aws_s3_meta_request_headers_callback_fn *headers_callback; + aws_s3_meta_request_should_continue_fn *continue_callback; aws_s3_meta_request_receive_body_callback_fn *body_callback; aws_s3_meta_request_finish_fn *finish_callback; aws_s3_meta_request_shutdown_fn *shutdown_callback; diff --git a/include/aws/s3/s3_client.h b/include/aws/s3/s3_client.h index b17e2a4da..25a1f07cf 100644 --- a/include/aws/s3/s3_client.h +++ b/include/aws/s3/s3_client.h @@ -110,6 +110,11 @@ typedef int(aws_s3_meta_request_receive_body_callback_fn)( /* User data specified by aws_s3_meta_request_options.*/ void *user_data); +/** + * Invoked to poll whether the user has canceled the meta request. + */ +typedef bool(aws_s3_meta_request_should_continue_fn)(void *user_data); + /** * Invoked when the entire meta request execution is complete. */ @@ -375,6 +380,12 @@ struct aws_s3_meta_request_options { */ aws_s3_meta_request_receive_body_callback_fn *body_callback; + /** + * Invoked to check whether the user has set the cancellation flag for this request. + * See `aws_s3_meta_request_should_continue_fn`. + */ + aws_s3_meta_request_should_continue_fn *continue_callback; + /** * Invoked when the entire meta request execution is complete. * See `aws_s3_meta_request_finish_fn`. diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c index b5e41b2fa..740f30562 100644 --- a/source/s3_meta_request.c +++ b/source/s3_meta_request.c @@ -231,6 +231,7 @@ int aws_s3_meta_request_init_base( meta_request->meta_request_level_running_response_sum = NULL; meta_request->user_data = options->user_data; + meta_request->continue_callback = options->continue_callback; meta_request->shutdown_callback = options->shutdown_callback; meta_request->progress_callback = options->progress_callback; @@ -364,10 +365,12 @@ bool aws_s3_meta_request_has_finish_result_synced(struct aws_s3_meta_request *me ASSERT_SYNCED_DATA_LOCK_HELD(meta_request); if (!meta_request->synced_data.finish_result_set) { - return false; + if (meta_request->continue_callback && !meta_request->continue_callback(meta_request->user_data)) { + aws_s3_meta_request_set_fail_synced(meta_request, NULL, AWS_ERROR_S3_CANCELED); + } } - return true; + return meta_request->synced_data.finish_result_set; } struct aws_s3_meta_request *aws_s3_meta_request_acquire(struct aws_s3_meta_request *meta_request) { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index bfbdae84c..0e5bb05ba 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -55,6 +55,8 @@ add_net_test_case(test_s3_cancel_mpd_head_object_completed) add_net_test_case(test_s3_cancel_mpd_get_without_range_sent) add_net_test_case(test_s3_cancel_mpd_get_without_range_completed) add_net_test_case(test_s3_cancel_prepare) +add_net_test_case(test_s3_continue_callback_ok) +add_net_test_case(test_s3_continue_callback_cancels) add_net_test_case(test_s3_get_object_tls_disabled) add_net_test_case(test_s3_get_object_tls_enabled) diff --git a/tests/s3_cancel_tests.c b/tests/s3_cancel_tests.c index e115165de..47b0ccf36 100644 --- a/tests/s3_cancel_tests.c +++ b/tests/s3_cancel_tests.c @@ -551,6 +551,11 @@ static int s_test_s3_cancel_prepare_meta_request_prepare_request( struct test_s3_cancel_prepare_user_data *test_user_data = tester->user_data; ++test_user_data->request_prepare_counters[request->request_tag]; + if (results->continue_callback != NULL) { + /* Cancel via s_test_continue_callback_cancels_after_first_part(). */ + return AWS_OP_SUCCESS; + } + /* Cancel after the first part is prepared, preventing any additional parts from being prepared. */ if (request->request_tag == AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_PART && test_user_data->request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_PART] == 1) { @@ -629,3 +634,112 @@ static int s_test_s3_cancel_prepare(struct aws_allocator *allocator, void *ctx) return 0; } + +static bool s_test_continue_callback_cancels_after_first_part(void *user_data) { + struct aws_s3_meta_request_test_results *results = user_data; + AWS_ASSERT(results != NULL); + + struct aws_s3_tester *tester = results->tester; + AWS_ASSERT(tester != NULL); + + /* Similar to s_test_s3_cancel_prepare_meta_request_prepare_request: cancel after the first part. */ + struct test_s3_cancel_prepare_user_data *test_user_data = tester->user_data; + return test_user_data->request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_PART] == 0; +} + +AWS_TEST_CASE(test_s3_continue_callback_cancels, s_test_s3_continue_callback_cancels) +static int s_test_s3_continue_callback_cancels(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_s3_tester tester; + ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester)); + + struct test_s3_cancel_prepare_user_data test_user_data; + AWS_ZERO_STRUCT(test_user_data); + tester.user_data = &test_user_data; + + struct aws_s3_client *client = NULL; + + struct aws_s3_tester_client_options client_options; + AWS_ZERO_STRUCT(client_options); + + ASSERT_SUCCESS(aws_s3_tester_client_new(&tester, &client_options, &client)); + + struct aws_s3_client_vtable *patched_client_vtable = aws_s3_tester_patch_client_vtable(&tester, client, NULL); + patched_client_vtable->meta_request_factory = s_test_s3_cancel_prepare_meta_request_factory; + + { + struct aws_s3_tester_meta_request_options options = { + .allocator = allocator, + .client = client, + .meta_request_type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT, + .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_FAILURE, + .continue_callback = s_test_continue_callback_cancels_after_first_part, + .put_options = + { + .ensure_multipart = true, + }, + }; + struct aws_s3_meta_request_test_results meta_request_test_results; + AWS_ZERO_STRUCT(meta_request_test_results); + + ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &options, &meta_request_test_results)); + ASSERT_TRUE(meta_request_test_results.finished_error_code == AWS_ERROR_S3_CANCELED); + + aws_s3_meta_request_test_results_clean_up(&meta_request_test_results); + } + + ASSERT_TRUE( + test_user_data.request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_CREATE_MULTIPART_UPLOAD] == 1); + ASSERT_TRUE(test_user_data.request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_PART] == 1); + ASSERT_TRUE( + test_user_data.request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_ABORT_MULTIPART_UPLOAD] == 1); + ASSERT_TRUE( + test_user_data.request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_COMPLETE_MULTIPART_UPLOAD] == 0); + + aws_s3_client_release(client); + aws_s3_tester_clean_up(&tester); + + return 0; +} + +static bool s_test_continue_callback_ok(void *user_data) { + (void)user_data; + return true; +} + +AWS_TEST_CASE(test_s3_continue_callback_ok, s_test_s3_continue_callback_ok) +static int s_test_s3_continue_callback_ok(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_s3_tester tester; + ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester)); + + struct aws_s3_client *client = NULL; + + struct aws_s3_tester_client_options client_options; + AWS_ZERO_STRUCT(client_options); + + ASSERT_SUCCESS(aws_s3_tester_client_new(&tester, &client_options, &client)); + + { + struct aws_s3_tester_meta_request_options options = { + .allocator = allocator, + .client = client, + .meta_request_type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT, + .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_SUCCESS, + .continue_callback = s_test_continue_callback_ok, + .put_options = + { + .ensure_multipart = true, + }, + }; + + ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &options, NULL)); + } + + aws_s3_client_release(client); + aws_s3_tester_clean_up(&tester); + + return 0; +} diff --git a/tests/s3_tester.c b/tests/s3_tester.c index d2383a099..fee3f58e0 100644 --- a/tests/s3_tester.c +++ b/tests/s3_tester.c @@ -92,6 +92,16 @@ static int s_s3_test_meta_request_header_callback( return AWS_OP_SUCCESS; } +static bool s_s3_test_meta_request_continue_callback(void *user_data) { + struct aws_s3_meta_request_test_results *meta_request_test_results = + (struct aws_s3_meta_request_test_results *)user_data; + + if (meta_request_test_results->continue_callback != NULL) { + return meta_request_test_results->continue_callback(user_data); + } + return true; +} + static int s_s3_test_meta_request_body_callback( struct aws_s3_meta_request *meta_request, const struct aws_byte_cursor *body, @@ -323,6 +333,9 @@ int aws_s3_tester_bind_meta_request( ASSERT_TRUE(options->headers_callback == NULL); options->headers_callback = s_s3_test_meta_request_header_callback; + ASSERT_TRUE(options->continue_callback == NULL); + options->continue_callback = s_s3_test_meta_request_continue_callback; + ASSERT_TRUE(options->body_callback == NULL); options->body_callback = s_s3_test_meta_request_body_callback; @@ -1366,6 +1379,7 @@ int aws_s3_tester_send_meta_request_with_options( } out_results->headers_callback = options->headers_callback; + out_results->continue_callback = options->continue_callback; out_results->body_callback = options->body_callback; out_results->finish_callback = options->finish_callback; diff --git a/tests/s3_tester.h b/tests/s3_tester.h index 5b18af556..d94a6fb57 100644 --- a/tests/s3_tester.h +++ b/tests/s3_tester.h @@ -151,6 +151,7 @@ struct aws_s3_tester_meta_request_options { struct aws_signing_config_aws *signing_config; aws_s3_meta_request_headers_callback_fn *headers_callback; + aws_s3_meta_request_should_continue_fn *continue_callback; aws_s3_meta_request_receive_body_callback_fn *body_callback; aws_s3_meta_request_finish_fn *finish_callback; @@ -189,6 +190,7 @@ struct aws_s3_meta_request_test_results { struct aws_s3_tester *tester; aws_s3_meta_request_headers_callback_fn *headers_callback; + aws_s3_meta_request_should_continue_fn *continue_callback; aws_s3_meta_request_receive_body_callback_fn *body_callback; aws_s3_meta_request_finish_fn *finish_callback; aws_s3_meta_request_shutdown_fn *shutdown_callback; From 8e599374cd19d7239357b13f9f78b77c99165c4c Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Mon, 2 Dec 2024 12:04:41 -0500 Subject: [PATCH 2/3] Back out of old changes --- include/aws/s3/private/s3_meta_request_impl.h | 1 - include/aws/s3/s3_client.h | 11 -- source/s3_meta_request.c | 7 +- tests/CMakeLists.txt | 2 - tests/s3_cancel_tests.c | 114 ------------------ tests/s3_tester.c | 14 --- tests/s3_tester.h | 2 - 7 files changed, 2 insertions(+), 149 deletions(-) diff --git a/include/aws/s3/private/s3_meta_request_impl.h b/include/aws/s3/private/s3_meta_request_impl.h index 6964327ba..1f6164192 100644 --- a/include/aws/s3/private/s3_meta_request_impl.h +++ b/include/aws/s3/private/s3_meta_request_impl.h @@ -127,7 +127,6 @@ struct aws_s3_meta_request { /* Customer specified callbacks. */ aws_s3_meta_request_headers_callback_fn *headers_callback; - aws_s3_meta_request_should_continue_fn *continue_callback; aws_s3_meta_request_receive_body_callback_fn *body_callback; aws_s3_meta_request_finish_fn *finish_callback; aws_s3_meta_request_shutdown_fn *shutdown_callback; diff --git a/include/aws/s3/s3_client.h b/include/aws/s3/s3_client.h index a6a99d300..021b5277f 100644 --- a/include/aws/s3/s3_client.h +++ b/include/aws/s3/s3_client.h @@ -110,11 +110,6 @@ typedef int(aws_s3_meta_request_receive_body_callback_fn)( /* User data specified by aws_s3_meta_request_options.*/ void *user_data); -/** - * Invoked to poll whether the user has canceled the meta request. - */ -typedef bool(aws_s3_meta_request_should_continue_fn)(void *user_data); - /** * Invoked when the entire meta request execution is complete. */ @@ -382,12 +377,6 @@ struct aws_s3_meta_request_options { */ aws_s3_meta_request_receive_body_callback_fn *body_callback; - /** - * Invoked to check whether the user has set the cancellation flag for this request. - * See `aws_s3_meta_request_should_continue_fn`. - */ - aws_s3_meta_request_should_continue_fn *continue_callback; - /** * Invoked when the entire meta request execution is complete. * See `aws_s3_meta_request_finish_fn`. diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c index 6f87b795e..a71e17a09 100644 --- a/source/s3_meta_request.c +++ b/source/s3_meta_request.c @@ -242,7 +242,6 @@ int aws_s3_meta_request_init_base( meta_request->meta_request_level_running_response_sum = NULL; meta_request->user_data = options->user_data; - meta_request->continue_callback = options->continue_callback; meta_request->shutdown_callback = options->shutdown_callback; meta_request->progress_callback = options->progress_callback; @@ -381,12 +380,10 @@ bool aws_s3_meta_request_has_finish_result_synced(struct aws_s3_meta_request *me ASSERT_SYNCED_DATA_LOCK_HELD(meta_request); if (!meta_request->synced_data.finish_result_set) { - if (meta_request->continue_callback && !meta_request->continue_callback(meta_request->user_data)) { - aws_s3_meta_request_set_fail_synced(meta_request, NULL, AWS_ERROR_S3_CANCELED); - } + return false; } - return meta_request->synced_data.finish_result_set; + return true; } struct aws_s3_meta_request *aws_s3_meta_request_acquire(struct aws_s3_meta_request *meta_request) { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 836cbfa2e..ce9282a51 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -57,8 +57,6 @@ add_net_test_case(test_s3_cancel_mpd_head_object_completed) add_net_test_case(test_s3_cancel_mpd_get_without_range_sent) add_net_test_case(test_s3_cancel_mpd_get_without_range_completed) add_net_test_case(test_s3_cancel_prepare) -add_net_test_case(test_s3_continue_callback_ok) -add_net_test_case(test_s3_continue_callback_cancels) add_net_test_case(test_s3_get_object_tls_disabled) add_net_test_case(test_s3_get_object_tls_enabled) diff --git a/tests/s3_cancel_tests.c b/tests/s3_cancel_tests.c index 275bc7c46..15d856203 100644 --- a/tests/s3_cancel_tests.c +++ b/tests/s3_cancel_tests.c @@ -552,11 +552,6 @@ static int s_test_s3_cancel_prepare_meta_request_prepare_request( struct test_s3_cancel_prepare_user_data *test_user_data = tester->user_data; ++test_user_data->request_prepare_counters[request->request_tag]; - if (results->continue_callback != NULL) { - /* Cancel via s_test_continue_callback_cancels_after_first_part(). */ - return AWS_OP_SUCCESS; - } - /* Cancel after the first part is prepared, preventing any additional parts from being prepared. */ if (request->request_tag == AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_PART && test_user_data->request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_PART] == 1) { @@ -635,112 +630,3 @@ static int s_test_s3_cancel_prepare(struct aws_allocator *allocator, void *ctx) return 0; } - -static bool s_test_continue_callback_cancels_after_first_part(void *user_data) { - struct aws_s3_meta_request_test_results *results = user_data; - AWS_ASSERT(results != NULL); - - struct aws_s3_tester *tester = results->tester; - AWS_ASSERT(tester != NULL); - - /* Similar to s_test_s3_cancel_prepare_meta_request_prepare_request: cancel after the first part. */ - struct test_s3_cancel_prepare_user_data *test_user_data = tester->user_data; - return test_user_data->request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_PART] == 0; -} - -AWS_TEST_CASE(test_s3_continue_callback_cancels, s_test_s3_continue_callback_cancels) -static int s_test_s3_continue_callback_cancels(struct aws_allocator *allocator, void *ctx) { - (void)ctx; - - struct aws_s3_tester tester; - ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester)); - - struct test_s3_cancel_prepare_user_data test_user_data; - AWS_ZERO_STRUCT(test_user_data); - tester.user_data = &test_user_data; - - struct aws_s3_client *client = NULL; - - struct aws_s3_tester_client_options client_options; - AWS_ZERO_STRUCT(client_options); - - ASSERT_SUCCESS(aws_s3_tester_client_new(&tester, &client_options, &client)); - - struct aws_s3_client_vtable *patched_client_vtable = aws_s3_tester_patch_client_vtable(&tester, client, NULL); - patched_client_vtable->meta_request_factory = s_test_s3_cancel_prepare_meta_request_factory; - - { - struct aws_s3_tester_meta_request_options options = { - .allocator = allocator, - .client = client, - .meta_request_type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT, - .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_FAILURE, - .continue_callback = s_test_continue_callback_cancels_after_first_part, - .put_options = - { - .ensure_multipart = true, - }, - }; - struct aws_s3_meta_request_test_results meta_request_test_results; - AWS_ZERO_STRUCT(meta_request_test_results); - - ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &options, &meta_request_test_results)); - ASSERT_TRUE(meta_request_test_results.finished_error_code == AWS_ERROR_S3_CANCELED); - - aws_s3_meta_request_test_results_clean_up(&meta_request_test_results); - } - - ASSERT_TRUE( - test_user_data.request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_CREATE_MULTIPART_UPLOAD] == 1); - ASSERT_TRUE(test_user_data.request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_PART] == 1); - ASSERT_TRUE( - test_user_data.request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_ABORT_MULTIPART_UPLOAD] == 1); - ASSERT_TRUE( - test_user_data.request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_COMPLETE_MULTIPART_UPLOAD] == 0); - - aws_s3_client_release(client); - aws_s3_tester_clean_up(&tester); - - return 0; -} - -static bool s_test_continue_callback_ok(void *user_data) { - (void)user_data; - return true; -} - -AWS_TEST_CASE(test_s3_continue_callback_ok, s_test_s3_continue_callback_ok) -static int s_test_s3_continue_callback_ok(struct aws_allocator *allocator, void *ctx) { - (void)ctx; - - struct aws_s3_tester tester; - ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester)); - - struct aws_s3_client *client = NULL; - - struct aws_s3_tester_client_options client_options; - AWS_ZERO_STRUCT(client_options); - - ASSERT_SUCCESS(aws_s3_tester_client_new(&tester, &client_options, &client)); - - { - struct aws_s3_tester_meta_request_options options = { - .allocator = allocator, - .client = client, - .meta_request_type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT, - .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_SUCCESS, - .continue_callback = s_test_continue_callback_ok, - .put_options = - { - .ensure_multipart = true, - }, - }; - - ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &options, NULL)); - } - - aws_s3_client_release(client); - aws_s3_tester_clean_up(&tester); - - return 0; -} diff --git a/tests/s3_tester.c b/tests/s3_tester.c index 0ca58dffe..29a36f0b2 100644 --- a/tests/s3_tester.c +++ b/tests/s3_tester.c @@ -95,16 +95,6 @@ static int s_s3_test_meta_request_header_callback( return AWS_OP_SUCCESS; } -static bool s_s3_test_meta_request_continue_callback(void *user_data) { - struct aws_s3_meta_request_test_results *meta_request_test_results = - (struct aws_s3_meta_request_test_results *)user_data; - - if (meta_request_test_results->continue_callback != NULL) { - return meta_request_test_results->continue_callback(user_data); - } - return true; -} - static int s_s3_test_meta_request_body_callback( struct aws_s3_meta_request *meta_request, const struct aws_byte_cursor *body, @@ -335,9 +325,6 @@ int aws_s3_tester_bind_meta_request( ASSERT_TRUE(options->headers_callback == NULL); options->headers_callback = s_s3_test_meta_request_header_callback; - ASSERT_TRUE(options->continue_callback == NULL); - options->continue_callback = s_s3_test_meta_request_continue_callback; - ASSERT_TRUE(options->body_callback == NULL); options->body_callback = s_s3_test_meta_request_body_callback; @@ -1400,7 +1387,6 @@ int aws_s3_tester_send_meta_request_with_options( } out_results->headers_callback = options->headers_callback; - out_results->continue_callback = options->continue_callback; out_results->body_callback = options->body_callback; out_results->finish_callback = options->finish_callback; diff --git a/tests/s3_tester.h b/tests/s3_tester.h index f6753f73a..1a3f183e2 100644 --- a/tests/s3_tester.h +++ b/tests/s3_tester.h @@ -156,7 +156,6 @@ struct aws_s3_tester_meta_request_options { struct aws_signing_config_aws *signing_config; aws_s3_meta_request_headers_callback_fn *headers_callback; - aws_s3_meta_request_should_continue_fn *continue_callback; aws_s3_meta_request_receive_body_callback_fn *body_callback; aws_s3_meta_request_finish_fn *finish_callback; @@ -198,7 +197,6 @@ struct aws_s3_meta_request_test_results { struct aws_s3_tester *tester; aws_s3_meta_request_headers_callback_fn *headers_callback; - aws_s3_meta_request_should_continue_fn *continue_callback; aws_s3_meta_request_receive_body_callback_fn *body_callback; aws_s3_meta_request_finish_fn *finish_callback; aws_s3_meta_request_shutdown_fn *shutdown_callback; From 9dbb8a316dbaa59742604baef5684ac2ff799ce4 Mon Sep 17 00:00:00 2001 From: Gerrit Renker Date: Mon, 2 Dec 2024 12:05:29 -0500 Subject: [PATCH 3/3] Restore updated changes --- include/aws/s3/private/s3_meta_request_impl.h | 1 + include/aws/s3/s3_client.h | 11 ++ source/s3_meta_request.c | 14 ++- tests/CMakeLists.txt | 2 + tests/s3_cancel_tests.c | 114 ++++++++++++++++++ tests/s3_tester.c | 14 +++ tests/s3_tester.h | 2 + 7 files changed, 156 insertions(+), 2 deletions(-) diff --git a/include/aws/s3/private/s3_meta_request_impl.h b/include/aws/s3/private/s3_meta_request_impl.h index 024680c8f..a5edf3623 100644 --- a/include/aws/s3/private/s3_meta_request_impl.h +++ b/include/aws/s3/private/s3_meta_request_impl.h @@ -169,6 +169,7 @@ struct aws_s3_meta_request { /* Customer specified callbacks. */ aws_s3_meta_request_headers_callback_fn *headers_callback; + aws_s3_meta_request_should_continue_fn *continue_callback; aws_s3_meta_request_receive_body_callback_fn *body_callback; aws_s3_meta_request_finish_fn *finish_callback; aws_s3_meta_request_shutdown_fn *shutdown_callback; diff --git a/include/aws/s3/s3_client.h b/include/aws/s3/s3_client.h index c765d79da..029a40dad 100644 --- a/include/aws/s3/s3_client.h +++ b/include/aws/s3/s3_client.h @@ -178,6 +178,11 @@ typedef int(aws_s3_meta_request_receive_body_callback_fn)( /* User data specified by aws_s3_meta_request_options.*/ void *user_data); +/** + * Invoked to poll whether the user has canceled the meta request. + */ +typedef bool(aws_s3_meta_request_should_continue_fn)(void *user_data); + /** * Invoked when the entire meta request execution is complete. */ @@ -767,6 +772,12 @@ struct aws_s3_meta_request_options { */ aws_s3_meta_request_receive_body_callback_fn *body_callback; + /** + * Invoked to check whether the user has set the cancellation flag for this request. + * See `aws_s3_meta_request_should_continue_fn`. + */ + aws_s3_meta_request_should_continue_fn *continue_callback; + /** * Invoked when the entire meta request execution is complete. * See `aws_s3_meta_request_finish_fn`. diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c index 2982bb40d..afaeebecf 100644 --- a/source/s3_meta_request.c +++ b/source/s3_meta_request.c @@ -306,6 +306,7 @@ int aws_s3_meta_request_init_base( meta_request->meta_request_level_running_response_sum = NULL; meta_request->user_data = options->user_data; + meta_request->continue_callback = options->continue_callback; meta_request->progress_callback = options->progress_callback; meta_request->telemetry_callback = options->telemetry_callback; meta_request->upload_review_callback = options->upload_review_callback; @@ -452,10 +453,19 @@ bool aws_s3_meta_request_has_finish_result_synced(struct aws_s3_meta_request *me ASSERT_SYNCED_DATA_LOCK_HELD(meta_request); if (!meta_request->synced_data.finish_result_set) { - return false; + /* + * Perform the equivalent of calling aws_s3_meta_request_cancel() if continue_callback() indicates + * that the request should be canceled. This is a work-around to enable canceling ongoing requests + * via a user-provided function. The work-around is needed until the AWS C++ SDK supports Cancel(). + * This function was chosen since it is evaluated several times during the lifetime of a request. + */ + if (meta_request->continue_callback && !meta_request->continue_callback(meta_request->user_data)) { + aws_s3_meta_request_set_fail_synced(meta_request, NULL, AWS_ERROR_S3_CANCELED); + aws_s3_meta_request_cancel_cancellable_requests_synced(meta_request, AWS_ERROR_S3_CANCELED); + } } - return true; + return meta_request->synced_data.finish_result_set; } struct aws_s3_meta_request *aws_s3_meta_request_acquire(struct aws_s3_meta_request *meta_request) { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 353b07318..86f26e6c3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -64,6 +64,8 @@ add_net_test_case(test_s3_cancel_mpd_empty_object_get_with_part_number_1_sent) add_net_test_case(test_s3_cancel_mpd_empty_object_get_with_part_number_1_completed) add_net_test_case(test_s3_cancel_mpd_pending_streaming) add_net_test_case(test_s3_cancel_prepare) +add_net_test_case(test_s3_continue_callback_ok) +add_net_test_case(test_s3_continue_callback_cancels) add_net_test_case(test_s3_get_object_tls_disabled) add_net_test_case(test_s3_get_object_tls_enabled) diff --git a/tests/s3_cancel_tests.c b/tests/s3_cancel_tests.c index 2ffc6b7a8..41d6be43e 100644 --- a/tests/s3_cancel_tests.c +++ b/tests/s3_cancel_tests.c @@ -705,6 +705,11 @@ static void s_test_s3_cancel_prepare_meta_request_prepare_request_on_original_do ++test_user_data->request_prepare_counters[request->request_tag]; + if (results->continue_callback != NULL) { + /* Cancel via s_test_continue_callback_cancels_after_first_part(). */ + return AWS_OP_SUCCESS; + } + /* Cancel after the first part is prepared, preventing any additional parts from being prepared. */ if (request->request_tag == AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_PART && test_user_data->request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_PART] == 1) { @@ -787,3 +792,112 @@ static int s_test_s3_cancel_prepare(struct aws_allocator *allocator, void *ctx) return 0; } + +static bool s_test_continue_callback_cancels_after_first_part(void *user_data) { + struct aws_s3_meta_request_test_results *results = user_data; + AWS_ASSERT(results != NULL); + + struct aws_s3_tester *tester = results->tester; + AWS_ASSERT(tester != NULL); + + /* Similar to s_test_s3_cancel_prepare_meta_request_prepare_request: cancel after the first part. */ + struct test_s3_cancel_prepare_user_data *test_user_data = tester->user_data; + return test_user_data->request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_PART] == 0; +} + +AWS_TEST_CASE(test_s3_continue_callback_cancels, s_test_s3_continue_callback_cancels) +static int s_test_s3_continue_callback_cancels(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_s3_tester tester; + ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester)); + + struct test_s3_cancel_prepare_user_data test_user_data; + AWS_ZERO_STRUCT(test_user_data); + tester.user_data = &test_user_data; + + struct aws_s3_client *client = NULL; + + struct aws_s3_tester_client_options client_options; + AWS_ZERO_STRUCT(client_options); + + ASSERT_SUCCESS(aws_s3_tester_client_new(&tester, &client_options, &client)); + + struct aws_s3_client_vtable *patched_client_vtable = aws_s3_tester_patch_client_vtable(&tester, client, NULL); + patched_client_vtable->meta_request_factory = s_test_s3_cancel_prepare_meta_request_factory; + + { + struct aws_s3_tester_meta_request_options options = { + .allocator = allocator, + .client = client, + .meta_request_type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT, + .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_FAILURE, + .continue_callback = s_test_continue_callback_cancels_after_first_part, + .put_options = + { + .ensure_multipart = true, + }, + }; + struct aws_s3_meta_request_test_results meta_request_test_results; + AWS_ZERO_STRUCT(meta_request_test_results); + + ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &options, &meta_request_test_results)); + ASSERT_TRUE(meta_request_test_results.finished_error_code == AWS_ERROR_S3_CANCELED); + + aws_s3_meta_request_test_results_clean_up(&meta_request_test_results); + } + + ASSERT_TRUE( + test_user_data.request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_CREATE_MULTIPART_UPLOAD] == 1); + ASSERT_TRUE(test_user_data.request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_PART] == 1); + ASSERT_TRUE( + test_user_data.request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_ABORT_MULTIPART_UPLOAD] == 1); + ASSERT_TRUE( + test_user_data.request_prepare_counters[AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_COMPLETE_MULTIPART_UPLOAD] == 0); + + aws_s3_client_release(client); + aws_s3_tester_clean_up(&tester); + + return 0; +} + +static bool s_test_continue_callback_ok(void *user_data) { + (void)user_data; + return true; +} + +AWS_TEST_CASE(test_s3_continue_callback_ok, s_test_s3_continue_callback_ok) +static int s_test_s3_continue_callback_ok(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_s3_tester tester; + ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester)); + + struct aws_s3_client *client = NULL; + + struct aws_s3_tester_client_options client_options; + AWS_ZERO_STRUCT(client_options); + + ASSERT_SUCCESS(aws_s3_tester_client_new(&tester, &client_options, &client)); + + { + struct aws_s3_tester_meta_request_options options = { + .allocator = allocator, + .client = client, + .meta_request_type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT, + .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_SUCCESS, + .continue_callback = s_test_continue_callback_ok, + .put_options = + { + .ensure_multipart = true, + }, + }; + + ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &options, NULL)); + } + + aws_s3_client_release(client); + aws_s3_tester_clean_up(&tester); + + return 0; +} diff --git a/tests/s3_tester.c b/tests/s3_tester.c index d879f3857..3d3e47bcd 100644 --- a/tests/s3_tester.c +++ b/tests/s3_tester.c @@ -114,6 +114,16 @@ static int s_s3_test_meta_request_header_callback( return AWS_OP_SUCCESS; } +static bool s_s3_test_meta_request_continue_callback(void *user_data) { + struct aws_s3_meta_request_test_results *meta_request_test_results = + (struct aws_s3_meta_request_test_results *)user_data; + + if (meta_request_test_results->continue_callback != NULL) { + return meta_request_test_results->continue_callback(user_data); + } + return true; +} + static int s_s3_test_meta_request_body_callback( struct aws_s3_meta_request *meta_request, const struct aws_byte_cursor *body, @@ -506,6 +516,9 @@ int aws_s3_tester_bind_meta_request( ASSERT_TRUE(options->headers_callback == NULL); options->headers_callback = s_s3_test_meta_request_header_callback; + ASSERT_TRUE(options->continue_callback == NULL); + options->continue_callback = s_s3_test_meta_request_continue_callback; + ASSERT_TRUE(options->body_callback == NULL); options->body_callback = s_s3_test_meta_request_body_callback; @@ -1732,6 +1745,7 @@ int aws_s3_tester_send_meta_request_with_options( } out_results->headers_callback = options->headers_callback; + out_results->continue_callback = options->continue_callback; out_results->body_callback = options->body_callback; out_results->finish_callback = options->finish_callback; out_results->progress_callback = options->progress_callback; diff --git a/tests/s3_tester.h b/tests/s3_tester.h index 03b1f1f6c..5b41e86e2 100644 --- a/tests/s3_tester.h +++ b/tests/s3_tester.h @@ -172,6 +172,7 @@ struct aws_s3_tester_meta_request_options { bool use_s3express_signing; aws_s3_meta_request_headers_callback_fn *headers_callback; + aws_s3_meta_request_should_continue_fn *continue_callback; aws_s3_meta_request_receive_body_callback_fn *body_callback; aws_s3_meta_request_finish_fn *finish_callback; aws_s3_meta_request_progress_fn *progress_callback; @@ -238,6 +239,7 @@ struct aws_s3_meta_request_test_results { struct aws_s3_tester *tester; aws_s3_meta_request_headers_callback_fn *headers_callback; + aws_s3_meta_request_should_continue_fn *continue_callback; aws_s3_meta_request_receive_body_callback_fn *body_callback; aws_s3_meta_request_finish_fn *finish_callback; aws_s3_meta_request_progress_fn *progress_callback;