Stop passing redundant operation-name to request_new().

In the 1 case where the name differed from the type, we'll just set that afterwards.

include/aws/s3/private/s3_request.h

@@ -157,14 +157,17 @@ struct aws_s3_request {
      * by the derived type.  Request tags do not necessarily map 1:1 with actual S3 API requests. (For example, they can
      * be more contextual, like "first part" instead of just "part".) */
 
-    /* TODO: this should be a union type to make it clear that this could be one of two enums for puts, and gets. */
+    /* TODO: Eliminate the concept of "request tag" and just use request_type.
+     * It's confusing having 2 concepts that are so similar.
+     * There's only 1 case where 2 tags used the same type,
+     * we can use some other bool/flag to differentiate this 1 case. */
     int request_tag;
 
     /* Actual S3 type for the single request (may be AWS_S3_REQUEST_TYPE_UNKNOWN) */
     enum aws_s3_request_type request_type;
 
     /* S3 operation name for the single request (e.g. "CompleteMultipartUpload") (NULL if unknown) */
-    const char *operation_name;
+    struct aws_string *operation_name;
 
     /* Members of this structure will be repopulated each time the request is sent. If the request fails, and needs to
      * be retried, then the members of this structure will be cleaned up and re-populated on the next send.
@@ -237,7 +240,6 @@ struct aws_s3_request *aws_s3_request_new(
     struct aws_s3_meta_request *meta_request,
     int request_tag,
     enum aws_s3_request_type request_type,
-    const char *operation_name,
     uint32_t part_number,
     uint32_t flags);
 

source/s3_auto_ranged_get.c

@@ -162,7 +162,6 @@ static bool s_s3_auto_ranged_get_update(
                             meta_request,
                             AWS_S3_AUTO_RANGE_GET_REQUEST_TYPE_HEAD_OBJECT,
                             AWS_S3_REQUEST_TYPE_HEAD_OBJECT,
-                            "HeadObject",
                             0 /*part_number*/,
                             AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
 
@@ -185,7 +184,6 @@ static bool s_s3_auto_ranged_get_update(
                         meta_request,
                         AWS_S3_AUTO_RANGE_GET_REQUEST_TYPE_PART,
                         AWS_S3_REQUEST_TYPE_GET_OBJECT,
-                        "GetObject",
                         1 /*part_number*/,
                         AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS | AWS_S3_REQUEST_FLAG_PART_SIZE_RESPONSE_BODY);
 
@@ -213,7 +211,6 @@ static bool s_s3_auto_ranged_get_update(
                     meta_request,
                     AWS_S3_AUTO_RANGE_GET_REQUEST_TYPE_INITIAL_MESSAGE,
                     AWS_S3_REQUEST_TYPE_GET_OBJECT,
-                    "GetObject",
                     0 /*part_number*/,
                     AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
 
@@ -264,7 +261,6 @@ static bool s_s3_auto_ranged_get_update(
                     meta_request,
                     AWS_S3_AUTO_RANGE_GET_REQUEST_TYPE_PART,
                     AWS_S3_REQUEST_TYPE_GET_OBJECT,
-                    "GetObject",
                     auto_ranged_get->synced_data.num_parts_requested + 1 /*part_number*/,
                     AWS_S3_REQUEST_FLAG_PART_SIZE_RESPONSE_BODY);
 

source/s3_auto_ranged_put.c

@@ -463,7 +463,6 @@ static bool s_s3_auto_ranged_put_update(
                     meta_request,
                     AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_LIST_PARTS,
                     AWS_S3_REQUEST_TYPE_LIST_PARTS,
-                    "ListParts",
                     0 /*part_number*/,
                     AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
 
@@ -479,7 +478,6 @@ static bool s_s3_auto_ranged_put_update(
                     meta_request,
                     AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_LIST_PARTS,
                     AWS_S3_REQUEST_TYPE_LIST_PARTS,
-                    "ListParts",
                     0 /*part_number*/,
                     AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
                 auto_ranged_put->synced_data.list_parts_state.continues = false;
@@ -497,7 +495,6 @@ static bool s_s3_auto_ranged_put_update(
                     meta_request,
                     AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_CREATE_MULTIPART_UPLOAD,
                     AWS_S3_REQUEST_TYPE_CREATE_MULTIPART_UPLOAD,
-                    "CreateMultipartUpload",
                     0 /*part_number*/,
                     AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
 
@@ -553,7 +550,6 @@ static bool s_s3_auto_ranged_put_update(
                         meta_request,
                         AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_PART,
                         AWS_S3_REQUEST_TYPE_UPLOAD_PART,
-                        "UploadPart",
                         0 /*part_number*/,
                         AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS | AWS_S3_REQUEST_FLAG_PART_SIZE_REQUEST_BODY);
 
@@ -601,7 +597,6 @@ static bool s_s3_auto_ranged_put_update(
                     meta_request,
                     AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_COMPLETE_MULTIPART_UPLOAD,
                     AWS_S3_REQUEST_TYPE_COMPLETE_MULTIPART_UPLOAD,
-                    "CompleteMultipartUpload",
                     0 /*part_number*/,
                     AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
 
@@ -668,7 +663,6 @@ static bool s_s3_auto_ranged_put_update(
                     meta_request,
                     AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_ABORT_MULTIPART_UPLOAD,
                     AWS_S3_REQUEST_TYPE_ABORT_MULTIPART_UPLOAD,
-                    "AbortMultipartUpload",
                     0 /*part_number*/,
                     AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS | AWS_S3_REQUEST_FLAG_ALWAYS_SEND);
 

source/s3_copy_object.c

@@ -137,7 +137,6 @@ static bool s_s3_copy_object_update(
                 meta_request,
                 AWS_S3_COPY_OBJECT_REQUEST_TAG_GET_OBJECT_SIZE,
                 AWS_S3_REQUEST_TYPE_HEAD_OBJECT,
-                "HeadObject",
                 0 /*part_number*/,
                 AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
 
@@ -158,7 +157,6 @@ static bool s_s3_copy_object_update(
                     meta_request,
                     AWS_S3_COPY_OBJECT_REQUEST_TAG_BYPASS,
                     AWS_S3_REQUEST_TYPE_COPY_OBJECT,
-                    "CopyObject",
                     1 /*part_number*/,
                     AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
 
@@ -187,7 +185,6 @@ static bool s_s3_copy_object_update(
                 meta_request,
                 AWS_S3_COPY_OBJECT_REQUEST_TAG_CREATE_MULTIPART_UPLOAD,
                 AWS_S3_REQUEST_TYPE_CREATE_MULTIPART_UPLOAD,
-                "CreateMultipartUpload",
                 0 /*part_number*/,
                 AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
 
@@ -219,7 +216,6 @@ static bool s_s3_copy_object_update(
                 meta_request,
                 AWS_S3_COPY_OBJECT_REQUEST_TAG_MULTIPART_COPY,
                 AWS_S3_REQUEST_TYPE_UPLOAD_PART_COPY,
-                "UploadPartCopy",
                 copy_object->threaded_update_data.next_part_number,
                 AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
 
@@ -248,7 +244,6 @@ static bool s_s3_copy_object_update(
                 meta_request,
                 AWS_S3_COPY_OBJECT_REQUEST_TAG_COMPLETE_MULTIPART_UPLOAD,
                 AWS_S3_REQUEST_TYPE_COMPLETE_MULTIPART_UPLOAD,
-                "CompleteMultipartUpload",
                 0 /*part_number*/,
                 AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
 
@@ -303,7 +298,6 @@ static bool s_s3_copy_object_update(
                 meta_request,
                 AWS_S3_COPY_OBJECT_REQUEST_TAG_ABORT_MULTIPART_UPLOAD,
                 AWS_S3_REQUEST_TYPE_ABORT_MULTIPART_UPLOAD,
-                "AbortMultipartUpload",
                 0 /*part_number*/,
                 AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS | AWS_S3_REQUEST_FLAG_ALWAYS_SEND);
 

source/s3_default_meta_request.c

@@ -167,11 +167,16 @@ static bool s_s3_meta_request_default_update(
                     meta_request,
                     0 /*request_tag*/,
                     meta_request_default->request_type,
-                    meta_request_default->operation_name ? aws_string_c_str(meta_request_default->operation_name)
-                                                         : NULL,
                     1 /*part_number*/,
                     AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
 
+                /* Default meta-request might know operation name, despite not knowing valid request_type.
+                 * If so, pass the name along. */
+                if (request->operation_name == NULL && meta_request_default->operation_name != NULL) {
+                    request->operation_name =
+                        aws_string_new_from_string(meta_request->allocator, meta_request_default->operation_name);
+                }
+
                 AWS_LOGF_DEBUG(
                     AWS_LS_S3_META_REQUEST,
                     "id=%p: Meta Request Default created request %p",

source/s3_meta_request.c

@@ -1808,7 +1808,7 @@ void aws_s3_meta_request_result_setup(
 
         if (failed_request->operation_name != NULL) {
             result->error_response_operation_name =
-                aws_string_new_from_c_str(meta_request->allocator, failed_request->operation_name);
+                aws_string_new_from_string(meta_request->allocator, failed_request->operation_name);
         }
     }
 

source/s3_request.c

@@ -17,13 +17,11 @@ struct aws_s3_request *aws_s3_request_new(
     struct aws_s3_meta_request *meta_request,
     int request_tag,
     enum aws_s3_request_type request_type,
-    const char *operation_name,
     uint32_t part_number,
     uint32_t flags) {
 
     AWS_PRECONDITION(meta_request);
     AWS_PRECONDITION(meta_request->allocator);
-    AWS_PRECONDITION(operation_name == NULL || operation_name[0] != '\0'); /* should be NULL or non-empty */
 
     struct aws_s3_request *request = aws_mem_calloc(meta_request->allocator, 1, sizeof(struct aws_s3_request));
 
@@ -34,7 +32,12 @@ struct aws_s3_request *aws_s3_request_new(
 
     request->request_tag = request_tag;
     request->request_type = request_type;
-    request->operation_name = operation_name;
+
+    const char *operation_name = aws_s3_request_type_operation_name(request_type);
+    if (operation_name[0] != '\0') {
+        request->operation_name = aws_string_new_from_c_str(request->allocator, operation_name);
+    }
+
     request->part_number = part_number;
     request->record_response_headers = (flags & AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS) != 0;
     request->has_part_size_response_body = (flags & AWS_S3_REQUEST_FLAG_PART_SIZE_RESPONSE_BODY) != 0;
@@ -171,7 +174,7 @@ struct aws_s3_request_metrics *aws_s3_request_metrics_new(
     metrics->req_resp_info_metrics.request_type = request->request_type;
 
     if (request->operation_name != NULL) {
-        metrics->req_resp_info_metrics.operation_name = aws_string_new_from_c_str(allocator, request->operation_name);
+        metrics->req_resp_info_metrics.operation_name = aws_string_new_from_string(allocator, request->operation_name);
     }
 
     metrics->time_metrics.start_timestamp_ns = -1;

tests/s3_data_plane_tests.c

@@ -368,8 +368,7 @@ static int s_test_s3_request_create_destroy(struct aws_allocator *allocator, voi
     (void)ctx;
 
     const int request_tag = 1234;
-    const enum aws_s3_request_type request_type = (enum aws_s3_request_type)9876;
-    const char *operation_name = "BlibbityBlob";
+    const enum aws_s3_request_type request_type = AWS_S3_REQUEST_TYPE_LIST_PARTS;
     const uint32_t part_number = 5678;
 
     struct aws_s3_tester tester;
@@ -387,20 +386,15 @@ static int s_test_s3_request_create_destroy(struct aws_allocator *allocator, voi
     ASSERT_TRUE(request_message != NULL);
 
     struct aws_s3_request *request = aws_s3_request_new(
-        meta_request,
-        request_tag,
-        request_type,
-        operation_name,
-        part_number,
-        AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
+        meta_request, request_tag, request_type, part_number, AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
 
     ASSERT_TRUE(request != NULL);
 
     ASSERT_TRUE(request->meta_request == meta_request);
     ASSERT_TRUE(request->part_number == part_number);
     ASSERT_TRUE(request->request_tag == request_tag);
     ASSERT_TRUE(request->request_type == request_type);
-    ASSERT_STR_EQUALS(operation_name, request->operation_name);
+    ASSERT_STR_EQUALS("ListParts", aws_string_c_str(request->operation_name));
     ASSERT_TRUE(request->record_response_headers == true);
 
     aws_s3_request_setup_send_data(request, request_message);
@@ -503,7 +497,7 @@ static int s_test_s3_meta_request_body_streaming(struct aws_allocator *allocator
     {
         for (uint32_t part_number = part_range0_start; part_number <= part_range0_end; ++part_number) {
             struct aws_s3_request *request = aws_s3_request_new(
-                meta_request, 0 /*request_tag*/, AWS_S3_REQUEST_TYPE_GET_OBJECT, "GetObject", part_number, 0 /*flags*/);
+                meta_request, 0 /*request_tag*/, AWS_S3_REQUEST_TYPE_GET_OBJECT, part_number, 0 /*flags*/);
 
             aws_s3_get_part_range(
                 0ULL,
@@ -540,7 +534,7 @@ static int s_test_s3_meta_request_body_streaming(struct aws_allocator *allocator
 
         for (uint32_t part_number = part_range1_start + 1; part_number <= part_range1_end; ++part_number) {
             struct aws_s3_request *request = aws_s3_request_new(
-                meta_request, 0 /*request_tag*/, AWS_S3_REQUEST_TYPE_GET_OBJECT, "GetObject", part_number, 0 /*flags*/);
+                meta_request, 0 /*request_tag*/, AWS_S3_REQUEST_TYPE_GET_OBJECT, part_number, 0 /*flags*/);
 
             aws_s3_get_part_range(
                 0ULL,
@@ -569,12 +563,7 @@ static int s_test_s3_meta_request_body_streaming(struct aws_allocator *allocator
     /* Stream the last part of the body, which should flush the priority queue. */
     {
         struct aws_s3_request *request = aws_s3_request_new(
-            meta_request,
-            0 /*request_tag*/,
-            AWS_S3_REQUEST_TYPE_GET_OBJECT,
-            "GetObject",
-            part_range1_start,
-            0 /*flags*/);
+            meta_request, 0 /*request_tag*/, AWS_S3_REQUEST_TYPE_GET_OBJECT, part_range1_start, 0 /*flags*/);
 
         aws_s3_get_part_range(
             0ULL,
@@ -624,15 +613,15 @@ static int s_test_s3_client_queue_requests(struct aws_allocator *allocator, void
     struct aws_s3_meta_request *mock_meta_request = aws_s3_tester_mock_meta_request_new(&tester);
     mock_meta_request->client = aws_s3_client_acquire(mock_client);
 
-    struct aws_s3_request *pivot_request = aws_s3_request_new(mock_meta_request, 0, 0, NULL, 0, 0);
+    struct aws_s3_request *pivot_request = aws_s3_request_new(mock_meta_request, 0, 0, 0, 0);
 
     struct aws_linked_list pivot_request_list;
     aws_linked_list_init(&pivot_request_list);
 
     struct aws_s3_request *requests[] = {
-        aws_s3_request_new(mock_meta_request, 0, 0, NULL, 0, 0),
-        aws_s3_request_new(mock_meta_request, 0, 0, NULL, 0, 0),
-        aws_s3_request_new(mock_meta_request, 0, 0, NULL, 0, 0),
+        aws_s3_request_new(mock_meta_request, 0, 0, 0, 0),
+        aws_s3_request_new(mock_meta_request, 0, 0, 0, 0),
+        aws_s3_request_new(mock_meta_request, 0, 0, 0, 0),
     };
 
     const uint32_t num_requests = AWS_ARRAY_SIZE(requests);
@@ -745,7 +734,7 @@ static bool s_s3_test_work_meta_request_update(
 
     if (out_request) {
         if (user_data->has_work_remaining) {
-            *out_request = aws_s3_request_new(meta_request, 0, 0, NULL, 0, 0);
+            *out_request = aws_s3_request_new(meta_request, 0, 0, 0, 0);
         }
     }
 
@@ -1006,7 +995,7 @@ static int s_test_s3_client_update_connections_finish_result(struct aws_allocato
 
     /* Verify that the request does not get sent because the meta request has finish-result. */
     {
-        struct aws_s3_request *request = aws_s3_request_new(mock_meta_request, 0, 0, NULL, 0, 0);
+        struct aws_s3_request *request = aws_s3_request_new(mock_meta_request, 0, 0, 0, 0);
         aws_linked_list_push_back(&mock_client->threaded_data.request_queue, &request->node);
         ++mock_client->threaded_data.request_queue_size;
 
@@ -1028,7 +1017,7 @@ static int s_test_s3_client_update_connections_finish_result(struct aws_allocato
     /* Verify that a request with the 'always send' flag still gets sent when the meta request has a finish-result. */
     {
         struct aws_s3_request *request =
-            aws_s3_request_new(mock_meta_request, 0, 0, NULL, 0, AWS_S3_REQUEST_FLAG_ALWAYS_SEND);
+            aws_s3_request_new(mock_meta_request, 0, 0, 0, AWS_S3_REQUEST_FLAG_ALWAYS_SEND);
         aws_linked_list_push_back(&mock_client->threaded_data.request_queue, &request->node);
         ++mock_client->threaded_data.request_queue_size;