comments addressed

include/aws/s3/s3.h

@@ -46,8 +46,8 @@ enum aws_s3_errors {
     AWS_ERROR_S3EXPRESS_CREATE_SESSION_FAILED,
     AWS_ERROR_S3_INTERNAL_PART_SIZE_MISMATCH_RETRYING_WITH_RANGE,
     AWS_ERROR_S3_REQUEST_HAS_COMPLETED,
-    AWS_ERROR_S3_RECV_FILE_EXISTS,
-    AWS_ERROR_S3_RECV_FILE_NOT_EXISTS,
+    AWS_ERROR_S3_RECV_FILE_ALREADY_EXISTS,
+    AWS_ERROR_S3_RECV_FILE_NOT_FOUND,
 
     AWS_ERROR_S3_END_RANGE = AWS_ERROR_ENUM_END_RANGE(AWS_C_S3_PACKAGE_ID)
 };

include/aws/s3/s3_client.h

@@ -249,27 +249,27 @@ enum aws_s3_checksum_location {
     AWS_SCL_TRAILER,
 };
 
-enum aws_s3_recv_file_options {
+enum aws_s3_recv_file_option {
     /**
      * Create a new file if it doesn't exist, otherwise replace the existing file.
      */
-    AWS_RECV_FILE_CREATE_OR_REPLACE = 0,
+    AWS_S3_RECV_FILE_CREATE_OR_REPLACE = 0,
     /**
-     * Always create a new file. If the file already exists, AWS_ERROR_S3_RECV_FILE_EXISTS will be raised.
+     * Always create a new file. If the file already exists, AWS_ERROR_S3_RECV_FILE_ALREADY_EXISTS will be raised.
      */
-    AWS_RECV_FILE_CREATE_NEW,
+    AWS_S3_RECV_FILE_CREATE_NEW,
     /**
      * Create a new file if it doesn't exist, otherwise append to the existing file.
      */
-    AWS_RECV_FILE_CREATE_OR_APPEND,
+    AWS_S3_RECV_FILE_CREATE_OR_APPEND,
 
     /**
      * Write to an existing file at the specified position, defined by the `recv_file_position`.
-     * If the file does not exist, AWS_ERROR_S3_RECV_FILE_NOT_EXISTS will be raised.
+     * If the file does not exist, AWS_ERROR_S3_RECV_FILE_NOT_FOUND will be raised.
      * If `recv_file_position` is not configured, start overwriting data at the beginning of the
      * file (byte 0).
      */
-    AWS_RECV_FILE_WRITE_TO_POSITION,
+    AWS_S3_RECV_FILE_WRITE_TO_POSITION,
 };
 /**
  * Info about a single part, for you to review before the upload completes.
@@ -659,21 +659,26 @@ struct aws_s3_meta_request_options {
      * If set, the received data will be written into this file.
      * the `body_callback` will NOT be invoked.
      * This gives a better performance when receiving data to write to a file.
-     * See `aws_s3_recv_file_options` for the configuration on the receive file.
+     * See `aws_s3_recv_file_option` for the configuration on the receive file.
      */
-    struct aws_byte_cursor receive_filepath;
+    struct aws_byte_cursor recv_filepath;
 
     /**
      * Optional.
-     * Default to AWS_RECV_FILE_CREATE_OR_REPLACE.
-     * This only works with receive_filepath set.
-     * See `aws_s3_recv_file_options`.
+     * Default to AWS_S3_RECV_FILE_CREATE_OR_REPLACE.
+     * This only works with recv_filepath set.
+     * See `aws_s3_recv_file_option`.
+     */
+    enum aws_s3_recv_file_option recv_file_option;
+    /**
+     * Optional.
+     * The specified position to start writing at for the recv file when `recv_file_option` is set to
+     * AWS_S3_RECV_FILE_WRITE_TO_POSITION, ignored otherwise.
      */
-    enum aws_s3_recv_file_options recv_file_options;
     uint64_t recv_file_position;
     /**
      * Set it to be true to delete the receive file on failure, otherwise, the file will be left as-is.
-     * This only works with receive_filepath set.
+     * This only works with recv_filepath set.
      */
     bool recv_file_delete_on_failure;
 

source/s3.c

@@ -49,8 +49,8 @@ static struct aws_error_info s_errors[] = {
     AWS_DEFINE_ERROR_INFO_S3(AWS_ERROR_S3EXPRESS_CREATE_SESSION_FAILED, "CreateSession call failed when signing with S3 Express."),
     AWS_DEFINE_ERROR_INFO_S3(AWS_ERROR_S3_INTERNAL_PART_SIZE_MISMATCH_RETRYING_WITH_RANGE, "part_size mismatch, possibly due to wrong object_size_hint. Retrying with Range instead of partNumber."),
     AWS_DEFINE_ERROR_INFO_S3(AWS_ERROR_S3_REQUEST_HAS_COMPLETED, "Request has already completed, action cannot be performed."),
-    AWS_DEFINE_ERROR_INFO_S3(AWS_ERROR_S3_RECV_FILE_EXISTS, "The receive file already exist, cannot override as configuration required."),
-    AWS_DEFINE_ERROR_INFO_S3(AWS_ERROR_S3_RECV_FILE_NOT_EXISTS, "The receive file doesn't exist, cannot create as configuration required."),
+    AWS_DEFINE_ERROR_INFO_S3(AWS_ERROR_S3_RECV_FILE_ALREADY_EXISTS, "The receive file already exist, cannot override as configuration required."),
+    AWS_DEFINE_ERROR_INFO_S3(AWS_ERROR_S3_RECV_FILE_NOT_FOUND, "The receive file doesn't exist, cannot create as configuration required."),
 };
 /* clang-format on */
 

source/s3_meta_request.c

@@ -234,47 +234,50 @@ int aws_s3_meta_request_init_base(
     /* Keep original message around, for headers, method, and synchronous body-stream (if any) */
     meta_request->initial_request_message = aws_http_message_acquire(options->message);
 
-    if (options->receive_filepath.len > 0) {
+    if (options->recv_filepath.len > 0) {
 
-        meta_request->recv_filepath = aws_string_new_from_cursor(allocator, &options->receive_filepath);
-        switch (options->recv_file_options) {
-            case AWS_RECV_FILE_CREATE_OR_REPLACE:
+        meta_request->recv_filepath = aws_string_new_from_cursor(allocator, &options->recv_filepath);
+        switch (options->recv_file_option) {
+            case AWS_S3_RECV_FILE_CREATE_OR_REPLACE:
                 meta_request->recv_file = aws_fopen(aws_string_c_str(meta_request->recv_filepath), "wb");
                 break;
 
-            case AWS_RECV_FILE_CREATE_NEW:
+            case AWS_S3_RECV_FILE_CREATE_NEW:
                 if (aws_path_exists(meta_request->recv_filepath)) {
                     AWS_LOGF_ERROR(
                         AWS_LS_S3_META_REQUEST,
-                        "id=%p The receive file already exists, but required to be not.",
+                        "id=%p Cannot receive file via CREATE_NEW: file already exists",
                         (void *)meta_request);
-                    aws_raise_error(AWS_ERROR_S3_RECV_FILE_EXISTS);
+                    aws_raise_error(AWS_ERROR_S3_RECV_FILE_ALREADY_EXISTS);
                     break;
                 } else {
                     meta_request->recv_file = aws_fopen(aws_string_c_str(meta_request->recv_filepath), "wb");
                     break;
                 }
-            case AWS_RECV_FILE_CREATE_OR_APPEND:
+            case AWS_S3_RECV_FILE_CREATE_OR_APPEND:
                 meta_request->recv_file = aws_fopen(aws_string_c_str(meta_request->recv_filepath), "ab");
                 break;
-            case AWS_RECV_FILE_WRITE_TO_POSITION:
+            case AWS_S3_RECV_FILE_WRITE_TO_POSITION:
                 if (!aws_path_exists(meta_request->recv_filepath)) {
                     AWS_LOGF_ERROR(
                         AWS_LS_S3_META_REQUEST,
-                        "id=%p The receive file doesn't exist, but required to be.",
+                        "id=%p Cannot receive file via WRITE_TO_POSITION: file not found.",
                         (void *)meta_request);
-                    aws_raise_error(AWS_ERROR_S3_RECV_FILE_NOT_EXISTS);
+                    aws_raise_error(AWS_ERROR_S3_RECV_FILE_NOT_FOUND);
                     break;
                 } else {
                     meta_request->recv_file = aws_fopen(aws_string_c_str(meta_request->recv_filepath), "r+");
-                    if (aws_fseek(meta_request->recv_file, options->recv_file_position, SEEK_SET) != AWS_OP_SUCCESS) {
+                    if (meta_request->recv_file &&
+                        aws_fseek(meta_request->recv_file, options->recv_file_position, SEEK_SET) != AWS_OP_SUCCESS) {
                         /* error out. */
                         goto error;
                     }
                     break;
                 }
+
             default:
                 AWS_ASSERT(false);
+                aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
                 break;
         }
         if (!meta_request->recv_file) {

tests/s3_data_plane_tests.c

@@ -1465,14 +1465,14 @@ static int s_test_s3_get_object_file_path_create_new(struct aws_allocator *alloc
             {
                 .object_path = object_path,
                 .file_on_disk = true,
-                .recv_file_options = AWS_RECV_FILE_CREATE_NEW,
+                .recv_file_option = AWS_S3_RECV_FILE_CREATE_NEW,
                 .pre_exist_file_length = 10,
                 .recv_file_delete_on_failure = true,
             },
     };
 
     ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &get_options, NULL));
-    ASSERT_UINT_EQUALS(AWS_ERROR_S3_RECV_FILE_EXISTS, aws_last_error());
+    ASSERT_UINT_EQUALS(AWS_ERROR_S3_RECV_FILE_ALREADY_EXISTS, aws_last_error());
 
     /* The request failed without invoking the finish callback. Reset the finish and shutdown count */
     aws_s3_tester_lock_synced_data(&tester);
@@ -1519,7 +1519,7 @@ static int s_test_s3_get_object_file_path_append(struct aws_allocator *allocator
             {
                 .object_path = object_path,
                 .file_on_disk = true,
-                .recv_file_options = AWS_RECV_FILE_CREATE_OR_APPEND,
+                .recv_file_option = AWS_S3_RECV_FILE_CREATE_OR_APPEND,
                 .pre_exist_file_length = pre_exist_file_length,
             },
     };
@@ -1561,13 +1561,13 @@ static int s_test_s3_get_object_file_path_to_position(struct aws_allocator *allo
             {
                 .object_path = object_path,
                 .file_on_disk = true,
-                .recv_file_options = AWS_RECV_FILE_WRITE_TO_POSITION,
+                .recv_file_option = AWS_S3_RECV_FILE_WRITE_TO_POSITION,
                 .pre_exist_file_length = 0,
             },
     };
 
     ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &get_options, NULL));
-    ASSERT_UINT_EQUALS(AWS_ERROR_S3_RECV_FILE_NOT_EXISTS, aws_last_error());
+    ASSERT_UINT_EQUALS(AWS_ERROR_S3_RECV_FILE_NOT_FOUND, aws_last_error());
 
     /* The request failed without invoking the finish callback. Reset the finish and shutdown count */
     aws_s3_tester_lock_synced_data(&tester);

tests/s3_tester.c

@@ -1538,8 +1538,8 @@ int aws_s3_tester_send_meta_request_with_options(
                 } else {
                     filepath_str = aws_s3_tester_create_file(allocator, options->get_options.object_path, NULL);
                 }
-                meta_request_options.receive_filepath = aws_byte_cursor_from_string(filepath_str);
-                meta_request_options.recv_file_options = options->get_options.recv_file_options;
+                meta_request_options.recv_filepath = aws_byte_cursor_from_string(filepath_str);
+                meta_request_options.recv_file_option = options->get_options.recv_file_option;
                 meta_request_options.recv_file_position = options->get_options.recv_file_position;
             }
             meta_request_options.message = message;
@@ -1778,7 +1778,7 @@ int aws_s3_tester_send_meta_request_with_options(
                 FILE *file = aws_fopen(aws_string_c_str(filepath_str), "rb");
                 ASSERT_NOT_NULL(file);
                 ASSERT_SUCCESS(aws_file_get_length(file, &out_results->received_file_size));
-                if (options->get_options.recv_file_options == AWS_RECV_FILE_CREATE_OR_REPLACE) {
+                if (options->get_options.recv_file_option == AWS_S3_RECV_FILE_CREATE_OR_REPLACE) {
                     ASSERT_UINT_EQUALS(out_results->progress.total_bytes_transferred, out_results->received_file_size);
                 }
                 fclose(file);

tests/s3_tester.h

@@ -189,7 +189,7 @@ struct aws_s3_tester_meta_request_options {
         /* Get the part from S3, starts from 1. 0 means not set. */
         int part_number;
         bool file_on_disk;
-        enum aws_s3_recv_file_options recv_file_options;
+        enum aws_s3_recv_file_option recv_file_option;
         uint64_t recv_file_position;
         bool recv_file_delete_on_failure;
         /* If larger than 0, create a pre-exist file with the length */