From 0bfd64e96e0f349371af01272a5ddc097804f630 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Thu, 16 Jan 2025 15:55:01 -0800 Subject: [PATCH 01/29] Outline --- .../request_response_client.c | 115 ++++++++++++++++-- .../request_response_client_tests.c | 27 ++-- 2 files changed, 121 insertions(+), 21 deletions(-) diff --git a/source/request-response/request_response_client.c b/source/request-response/request_response_client.c index 448f7c98..86037e80 100644 --- a/source/request-response/request_response_client.c +++ b/source/request-response/request_response_client.c @@ -352,6 +352,8 @@ struct aws_mqtt_request_response_client { */ struct aws_hash_table streaming_operation_subscription_lists; + struct aws_hash_table streaming_operation_wildcards_subscription_lists; + /* * Map from cursor (topic) -> request response path (topic, correlation token json path) * @@ -1067,6 +1069,67 @@ static void s_aws_rr_client_protocol_adapter_incoming_publish_callback( AWS_BYTE_CURSOR_PRI(publish_event->topic)); s_apply_publish_to_streaming_operation_list(subscription_filter_element->value, publish_event); + } else { + AWS_LOGF_INFO( + AWS_LS_MQTT_REQUEST_RESPONSE, + "= Looking subscription for topic '" PRInSTR "'", + AWS_BYTE_CURSOR_PRI(publish_event->topic)); + for (struct aws_hash_iter iter = + aws_hash_iter_begin(&rr_client->streaming_operation_wildcards_subscription_lists); + !aws_hash_iter_done(&iter); + aws_hash_iter_next(&iter)) { + struct aws_rr_operation_list_topic_filter_entry *entry = iter.element.value; + AWS_LOGF_INFO( + AWS_LS_MQTT_REQUEST_RESPONSE, + "= Checking subscription with topic filter " PRInSTR, + AWS_BYTE_CURSOR_PRI(entry->topic_filter_cursor)); + + struct aws_byte_cursor subscription_topic_filter_segment; + AWS_ZERO_STRUCT(subscription_topic_filter_segment); + + struct aws_byte_cursor topic_segment; + AWS_ZERO_STRUCT(topic_segment); + + bool match = true; + + while (aws_byte_cursor_next_split(&entry->topic_filter_cursor, '/', &subscription_topic_filter_segment)) { + AWS_LOGF_INFO( + AWS_LS_MQTT_REQUEST_RESPONSE, + "=== subscription topic filter segment is '" PRInSTR "'", + AWS_BYTE_CURSOR_PRI(subscription_topic_filter_segment)); + + if (!aws_byte_cursor_next_split(&publish_event->topic, '/', &topic_segment)) { + AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== topic segment is NULL"); + match = false; + break; + } + + AWS_LOGF_INFO( + AWS_LS_MQTT_REQUEST_RESPONSE, + "======= topic segment is '" PRInSTR "'", + AWS_BYTE_CURSOR_PRI(topic_segment)); + + if (!aws_byte_cursor_eq_c_str(&subscription_topic_filter_segment, "+") && + !aws_byte_cursor_eq_ignore_case(&topic_segment, &subscription_topic_filter_segment)) { + AWS_LOGF_INFO( + AWS_LS_MQTT_REQUEST_RESPONSE, + "======= topic segment differs", + AWS_BYTE_CURSOR_PRI(topic_segment)); + match = false; + break; + } + } + + if (aws_byte_cursor_next_split(&publish_event->topic, '/', &topic_segment)) { + match = false; + } + + if (match) { + AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== found subscription match"); + } else { + AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== this is not the right subscription"); + } + } } /* Request-Response handling */ @@ -1180,6 +1243,15 @@ static struct aws_mqtt_request_response_client *s_aws_mqtt_request_response_clie NULL, s_aws_rr_operation_list_topic_filter_entry_hash_element_destroy); + aws_hash_table_init( + &rr_client->streaming_operation_wildcards_subscription_lists, + allocator, + 10, // TODO + aws_hash_byte_cursor_ptr, + aws_mqtt_byte_cursor_hash_equality, + NULL, + s_aws_rr_operation_list_topic_filter_entry_hash_element_destroy); + aws_hash_table_init( &rr_client->request_response_paths, allocator, @@ -1288,20 +1360,45 @@ static int s_add_streaming_operation_to_subscription_topic_filter_table( struct aws_byte_cursor topic_filter_cursor = operation->storage.streaming_storage.options.topic_filter; + bool is_topic_with_wildcard = false; + if (memchr(topic_filter_cursor.ptr, '+', topic_filter_cursor.len)) { + is_topic_with_wildcard = true; + } + struct aws_hash_element *element = NULL; - if (aws_hash_table_find(&client->streaming_operation_subscription_lists, &topic_filter_cursor, &element)) { - return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); + if (is_topic_with_wildcard) { + if (aws_hash_table_find( + &client->streaming_operation_wildcards_subscription_lists, &topic_filter_cursor, &element)) { + return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); + } + } else { + if (aws_hash_table_find(&client->streaming_operation_subscription_lists, &topic_filter_cursor, &element)) { + return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); + } } struct aws_rr_operation_list_topic_filter_entry *entry = NULL; if (element == NULL) { - entry = s_aws_rr_operation_list_topic_filter_entry_new(client->allocator, topic_filter_cursor); - aws_hash_table_put(&client->streaming_operation_subscription_lists, &entry->topic_filter_cursor, entry, NULL); - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "id=%p: request-response client adding topic filter '" PRInSTR "' to streaming subscriptions table", - (void *)client, - AWS_BYTE_CURSOR_PRI(topic_filter_cursor)); + if (is_topic_with_wildcard) { + entry = s_aws_rr_operation_list_topic_filter_entry_new(client->allocator, topic_filter_cursor); + aws_hash_table_put( + &client->streaming_operation_wildcards_subscription_lists, &entry->topic_filter_cursor, entry, NULL); + AWS_LOGF_DEBUG( + AWS_LS_MQTT_REQUEST_RESPONSE, + "id=%p: request-response client adding wildcard topic filter '" PRInSTR + "' to streaming subscriptions table", + (void *)client, + AWS_BYTE_CURSOR_PRI(topic_filter_cursor)); + } else { + entry = s_aws_rr_operation_list_topic_filter_entry_new(client->allocator, topic_filter_cursor); + aws_hash_table_put( + &client->streaming_operation_subscription_lists, &entry->topic_filter_cursor, entry, NULL); + AWS_LOGF_DEBUG( + AWS_LS_MQTT_REQUEST_RESPONSE, + "id=%p: request-response client adding topic filter '" PRInSTR "' to streaming subscriptions table", + (void *)client, + AWS_BYTE_CURSOR_PRI(topic_filter_cursor)); + } } else { entry = element->value; } diff --git a/tests/request-response/request_response_client_tests.c b/tests/request-response/request_response_client_tests.c index a12e2bf9..1e80aaba 100644 --- a/tests/request-response/request_response_client_tests.c +++ b/tests/request-response/request_response_client_tests.c @@ -1219,7 +1219,7 @@ static int s_init_fixture_streaming_operation_success( struct aws_mqtt_request_response_client_options rr_client_options = { .max_request_response_subscriptions = 2, - .max_streaming_subscriptions = 1, + .max_streaming_subscriptions = 2, .operation_timeout_seconds = 2, }; @@ -1249,8 +1249,9 @@ static int s_rrc_streaming_operation_success_single_fn(struct aws_allocator *all ASSERT_SUCCESS(s_init_fixture_streaming_operation_success(&fixture, &client_test_options, allocator, NULL, NULL)); struct aws_byte_cursor record_key1 = aws_byte_cursor_from_c_str("key1"); + struct aws_byte_cursor topic = aws_byte_cursor_from_c_str("topic/+"); struct aws_byte_cursor topic_filter1 = aws_byte_cursor_from_c_str("topic/1"); - struct aws_mqtt_rr_client_operation *operation = s_create_streaming_operation(&fixture, record_key1, topic_filter1); + struct aws_mqtt_rr_client_operation *operation = s_create_streaming_operation(&fixture, record_key1, topic); s_rrc_wait_for_n_streaming_subscription_events(&fixture, record_key1, 1); @@ -1994,12 +1995,14 @@ static int s_rrc_streaming_operation_failure_exceeds_subscription_budget_fn( &fixture, &client_test_options, allocator, s_rrc_unsubscribe_success_config, NULL)); struct aws_byte_cursor record_key1 = aws_byte_cursor_from_c_str("key1"); - struct aws_byte_cursor topic_filter1 = aws_byte_cursor_from_c_str("topic/1"); + struct aws_byte_cursor topic_filter1 = aws_byte_cursor_from_c_str("topic/1/+"); + struct aws_byte_cursor topic1 = aws_byte_cursor_from_c_str("topic/1/abc"); struct aws_mqtt_rr_client_operation *operation1 = s_create_streaming_operation(&fixture, record_key1, topic_filter1); struct aws_byte_cursor record_key2 = aws_byte_cursor_from_c_str("key2"); - struct aws_byte_cursor topic_filter2 = aws_byte_cursor_from_c_str("topic/2"); + struct aws_byte_cursor topic_filter2 = aws_byte_cursor_from_c_str("/topic/2/+"); + struct aws_byte_cursor topic2 = aws_byte_cursor_from_c_str("/topic/2/def"); struct aws_mqtt_rr_client_operation *operation2 = s_create_streaming_operation(&fixture, record_key2, topic_filter2); @@ -2022,24 +2025,24 @@ static int s_rrc_streaming_operation_failure_exceeds_subscription_budget_fn( }, }; ASSERT_SUCCESS(s_rrc_verify_streaming_record_subscription_events( - &fixture, record_key2, AWS_ARRAY_SIZE(expected_failure_events), expected_failure_events)); + &fixture, record_key2, AWS_ARRAY_SIZE(expected_success_events), expected_success_events)); // two publishes on the mqtt client that get reflected into our subscription topic1 struct aws_byte_cursor payload1 = aws_byte_cursor_from_c_str("Payload1"); struct aws_byte_cursor payload2 = aws_byte_cursor_from_c_str("Payload2"); - ASSERT_SUCCESS(s_rrc_protocol_client_publish(&fixture, topic_filter1, payload1)); - ASSERT_SUCCESS(s_rrc_protocol_client_publish(&fixture, topic_filter1, payload2)); + ASSERT_SUCCESS(s_rrc_protocol_client_publish(&fixture, topic1, payload1)); + ASSERT_SUCCESS(s_rrc_protocol_client_publish(&fixture, topic1, payload2)); s_rrc_wait_for_n_streaming_publishes(&fixture, record_key1, 2); struct aws_rr_client_fixture_publish_message_view expected_publishes[] = { { payload1, - topic_filter1, + topic1, }, { payload2, - topic_filter1, + topic1, }, }; ASSERT_SUCCESS(s_rrc_verify_streaming_publishes(&fixture, record_key1, 2, expected_publishes)); @@ -2058,7 +2061,7 @@ static int s_rrc_streaming_operation_failure_exceeds_subscription_budget_fn( // make a third using topic filter 2 struct aws_byte_cursor record_key3 = aws_byte_cursor_from_c_str("key3"); struct aws_mqtt_rr_client_operation *operation3 = - s_create_streaming_operation(&fixture, record_key3, topic_filter2); + s_create_streaming_operation(&fixture, record_key3, topic2); s_rrc_wait_for_n_streaming_subscription_events(&fixture, record_key3, 1); ASSERT_SUCCESS(s_rrc_verify_streaming_record_subscription_events( @@ -2066,7 +2069,7 @@ static int s_rrc_streaming_operation_failure_exceeds_subscription_budget_fn( // publish again struct aws_byte_cursor payload3 = aws_byte_cursor_from_c_str("payload3"); - ASSERT_SUCCESS(s_rrc_protocol_client_publish(&fixture, topic_filter2, payload3)); + ASSERT_SUCCESS(s_rrc_protocol_client_publish(&fixture, topic2, payload3)); // verify third operation got the new publish s_rrc_wait_for_n_streaming_publishes(&fixture, record_key3, 1); @@ -2074,7 +2077,7 @@ static int s_rrc_streaming_operation_failure_exceeds_subscription_budget_fn( struct aws_rr_client_fixture_publish_message_view third_expected_publishes[] = { { payload3, - topic_filter2, + topic2, }, }; ASSERT_SUCCESS(s_rrc_verify_streaming_publishes( From fd912fa1f80a50515355984f05a569abe0ca5bf3 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Fri, 17 Jan 2025 09:34:14 -0800 Subject: [PATCH 02/29] Extract search into separate source --- .../request_response_subscription_set.h | 32 ++++++++ .../request_response_client.c | 74 +------------------ .../request_response_subscription_set.c | 67 +++++++++++++++++ 3 files changed, 101 insertions(+), 72 deletions(-) create mode 100644 include/aws/mqtt/private/request-response/request_response_subscription_set.h create mode 100644 source/request-response/request_response_subscription_set.c diff --git a/include/aws/mqtt/private/request-response/request_response_subscription_set.h b/include/aws/mqtt/private/request-response/request_response_subscription_set.h new file mode 100644 index 00000000..bd7db110 --- /dev/null +++ b/include/aws/mqtt/private/request-response/request_response_subscription_set.h @@ -0,0 +1,32 @@ +#ifndef AWS_MQTT_PRIVATE_REQUEST_RESPONSE_REQUEST_RESPONSE_SUBSCRIPTION_SET_H +#define AWS_MQTT_PRIVATE_REQUEST_RESPONSE_REQUEST_RESPONSE_SUBSCRIPTION_SET_H + +/** + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +#include +#include +#include +#include + +/* + * This is the (key and) value in hash table (4) above. + */ +struct aws_rr_operation_list_topic_filter_entry { + struct aws_allocator *allocator; + + struct aws_byte_cursor topic_filter_cursor; + struct aws_byte_buf topic_filter; + + struct aws_linked_list operations; +}; + +AWS_EXTERN_C_BEGIN + +AWS_MQTT_API void search(const struct aws_hash_table *subscriptions, const struct aws_byte_cursor *topic); + +AWS_EXTERN_C_END + +#endif /* AWS_MQTT_PRIVATE_REQUEST_RESPONSE_REQUEST_RESPONSE_SUBSCRIPTION_SET_H */ diff --git a/source/request-response/request_response_client.c b/source/request-response/request_response_client.c index 86037e80..0ac16cf9 100644 --- a/source/request-response/request_response_client.c +++ b/source/request-response/request_response_client.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -136,18 +137,6 @@ state, removed on operation completion/destruction */ -/* - * This is the (key and) value in hash table (4) above. - */ -struct aws_rr_operation_list_topic_filter_entry { - struct aws_allocator *allocator; - - struct aws_byte_cursor topic_filter_cursor; - struct aws_byte_buf topic_filter; - - struct aws_linked_list operations; -}; - static struct aws_rr_operation_list_topic_filter_entry *s_aws_rr_operation_list_topic_filter_entry_new( struct aws_allocator *allocator, struct aws_byte_cursor topic_filter) { @@ -1070,66 +1059,7 @@ static void s_aws_rr_client_protocol_adapter_incoming_publish_callback( s_apply_publish_to_streaming_operation_list(subscription_filter_element->value, publish_event); } else { - AWS_LOGF_INFO( - AWS_LS_MQTT_REQUEST_RESPONSE, - "= Looking subscription for topic '" PRInSTR "'", - AWS_BYTE_CURSOR_PRI(publish_event->topic)); - for (struct aws_hash_iter iter = - aws_hash_iter_begin(&rr_client->streaming_operation_wildcards_subscription_lists); - !aws_hash_iter_done(&iter); - aws_hash_iter_next(&iter)) { - struct aws_rr_operation_list_topic_filter_entry *entry = iter.element.value; - AWS_LOGF_INFO( - AWS_LS_MQTT_REQUEST_RESPONSE, - "= Checking subscription with topic filter " PRInSTR, - AWS_BYTE_CURSOR_PRI(entry->topic_filter_cursor)); - - struct aws_byte_cursor subscription_topic_filter_segment; - AWS_ZERO_STRUCT(subscription_topic_filter_segment); - - struct aws_byte_cursor topic_segment; - AWS_ZERO_STRUCT(topic_segment); - - bool match = true; - - while (aws_byte_cursor_next_split(&entry->topic_filter_cursor, '/', &subscription_topic_filter_segment)) { - AWS_LOGF_INFO( - AWS_LS_MQTT_REQUEST_RESPONSE, - "=== subscription topic filter segment is '" PRInSTR "'", - AWS_BYTE_CURSOR_PRI(subscription_topic_filter_segment)); - - if (!aws_byte_cursor_next_split(&publish_event->topic, '/', &topic_segment)) { - AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== topic segment is NULL"); - match = false; - break; - } - - AWS_LOGF_INFO( - AWS_LS_MQTT_REQUEST_RESPONSE, - "======= topic segment is '" PRInSTR "'", - AWS_BYTE_CURSOR_PRI(topic_segment)); - - if (!aws_byte_cursor_eq_c_str(&subscription_topic_filter_segment, "+") && - !aws_byte_cursor_eq_ignore_case(&topic_segment, &subscription_topic_filter_segment)) { - AWS_LOGF_INFO( - AWS_LS_MQTT_REQUEST_RESPONSE, - "======= topic segment differs", - AWS_BYTE_CURSOR_PRI(topic_segment)); - match = false; - break; - } - } - - if (aws_byte_cursor_next_split(&publish_event->topic, '/', &topic_segment)) { - match = false; - } - - if (match) { - AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== found subscription match"); - } else { - AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== this is not the right subscription"); - } - } + search(&rr_client->streaming_operation_wildcards_subscription_lists, &publish_event->topic); } /* Request-Response handling */ diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c new file mode 100644 index 00000000..503bbf54 --- /dev/null +++ b/source/request-response/request_response_subscription_set.c @@ -0,0 +1,67 @@ +/** + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +#include + +#include +#include + +void search(const struct aws_hash_table *subscriptions, const struct aws_byte_cursor *topic) { + AWS_LOGF_INFO( + AWS_LS_MQTT_REQUEST_RESPONSE, "= Looking subscription for topic '" PRInSTR "'", AWS_BYTE_CURSOR_PRI(*topic)); + + for (struct aws_hash_iter iter = aws_hash_iter_begin(subscriptions); !aws_hash_iter_done(&iter); + aws_hash_iter_next(&iter)) { + struct aws_rr_operation_list_topic_filter_entry *entry = iter.element.value; + AWS_LOGF_INFO( + AWS_LS_MQTT_REQUEST_RESPONSE, + "= Checking subscription with topic filter " PRInSTR, + AWS_BYTE_CURSOR_PRI(entry->topic_filter_cursor)); + + struct aws_byte_cursor subscription_topic_filter_segment; + AWS_ZERO_STRUCT(subscription_topic_filter_segment); + + struct aws_byte_cursor topic_segment; + AWS_ZERO_STRUCT(topic_segment); + + bool match = true; + + while (aws_byte_cursor_next_split(&entry->topic_filter_cursor, '/', &subscription_topic_filter_segment)) { + AWS_LOGF_INFO( + AWS_LS_MQTT_REQUEST_RESPONSE, + "=== subscription topic filter segment is '" PRInSTR "'", + AWS_BYTE_CURSOR_PRI(subscription_topic_filter_segment)); + + if (!aws_byte_cursor_next_split(topic, '/', &topic_segment)) { + AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== topic segment is NULL"); + match = false; + break; + } + + AWS_LOGF_INFO( + AWS_LS_MQTT_REQUEST_RESPONSE, + "======= topic segment is '" PRInSTR "'", + AWS_BYTE_CURSOR_PRI(topic_segment)); + + if (!aws_byte_cursor_eq_c_str(&subscription_topic_filter_segment, "+") && + !aws_byte_cursor_eq_ignore_case(&topic_segment, &subscription_topic_filter_segment)) { + AWS_LOGF_INFO( + AWS_LS_MQTT_REQUEST_RESPONSE, "======= topic segment differs", AWS_BYTE_CURSOR_PRI(topic_segment)); + match = false; + break; + } + } + + if (aws_byte_cursor_next_split(topic, '/', &topic_segment)) { + match = false; + } + + if (match) { + AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== found subscription match"); + } else { + AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== this is not the right subscription"); + } + } +} From ba11a74108f818b2b84b57b5ed276645adefe97e Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Fri, 17 Jan 2025 11:26:21 -0800 Subject: [PATCH 03/29] Move subscriptions match --- .../request_response_subscription_set.h | 45 +++++++++- .../request_response_client.c | 88 +++++++------------ .../request_response_subscription_set.c | 47 +++++++++- 3 files changed, 121 insertions(+), 59 deletions(-) diff --git a/include/aws/mqtt/private/request-response/request_response_subscription_set.h b/include/aws/mqtt/private/request-response/request_response_subscription_set.h index bd7db110..e3912ac2 100644 --- a/include/aws/mqtt/private/request-response/request_response_subscription_set.h +++ b/include/aws/mqtt/private/request-response/request_response_subscription_set.h @@ -10,6 +10,28 @@ #include #include #include +#include + +/* Holds subscriptions for request-response client. */ +struct aws_request_response_subscriptions { + /* + * Map from cursor (topic filter) -> list of streaming operations using that filter + */ + struct aws_hash_table streaming_operation_subscription_lists; + + /* + * Map from cursor (topic filter with wildcards) -> list of streaming operations using that filter + */ + struct aws_hash_table streaming_operation_wildcards_subscription_lists; + + /* + * Map from cursor (topic) -> request response path (topic, correlation token json path) + * + * We don't garbage collect this table over the course of normal client operation. We only clean it up + * when the client is shutting down. + */ + struct aws_hash_table request_response_paths; +}; /* * This is the (key and) value in hash table (4) above. @@ -23,9 +45,30 @@ struct aws_rr_operation_list_topic_filter_entry { struct aws_linked_list operations; }; +struct aws_mqtt_request_response_client; +struct aws_rr_response_path_entry; + +typedef void(aws_mqtt_stream_operation_subscription_match_fn)( + struct aws_rr_operation_list_topic_filter_entry *entry, + const struct aws_protocol_adapter_incoming_publish_event *publish_event); + +typedef void(aws_mqtt_request_operation_subscription_match_fn)( + struct aws_mqtt_request_response_client *rr_client, + struct aws_rr_response_path_entry *entry, + const struct aws_protocol_adapter_incoming_publish_event *publish_event); + AWS_EXTERN_C_BEGIN -AWS_MQTT_API void search(const struct aws_hash_table *subscriptions, const struct aws_byte_cursor *topic); +AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_destroy( + struct aws_request_response_subscriptions *subscriptions); + +AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_match( + const struct aws_request_response_subscriptions *subscriptions, + const struct aws_byte_cursor *topic, + aws_mqtt_stream_operation_subscription_match_fn *on_stream_operation_subscription_match, + aws_mqtt_request_operation_subscription_match_fn *on_request_operation_subscription_match, + const struct aws_protocol_adapter_incoming_publish_event *publish_event, + struct aws_mqtt_request_response_client *rr_client); AWS_EXTERN_C_END diff --git a/source/request-response/request_response_client.c b/source/request-response/request_response_client.c index 0ac16cf9..5fec0808 100644 --- a/source/request-response/request_response_client.c +++ b/source/request-response/request_response_client.c @@ -336,20 +336,7 @@ struct aws_mqtt_request_response_client { */ struct aws_priority_queue operations_by_timeout; - /* - * Map from cursor (topic filter) -> list of streaming operations using that filter - */ - struct aws_hash_table streaming_operation_subscription_lists; - - struct aws_hash_table streaming_operation_wildcards_subscription_lists; - - /* - * Map from cursor (topic) -> request response path (topic, correlation token json path) - * - * We don't garbage collect this table over the course of normal client operation. We only clean it up - * when the client is shutting down. - */ - struct aws_hash_table request_response_paths; + struct aws_request_response_subscriptions subscriptions; /* * Map from cursor (correlation token) -> request operation @@ -397,8 +384,8 @@ static void s_mqtt_request_response_client_final_destroy(struct aws_mqtt_request aws_hash_table_clean_up(&client->operations); aws_priority_queue_clean_up(&client->operations_by_timeout); - aws_hash_table_clean_up(&client->streaming_operation_subscription_lists); - aws_hash_table_clean_up(&client->request_response_paths); + + aws_mqtt_request_response_client_subscriptions_destroy(&client->subscriptions); aws_hash_table_clean_up(&client->operations_by_correlation_tokens); aws_mem_release(client->allocator, client); @@ -1045,36 +1032,13 @@ static void s_aws_rr_client_protocol_adapter_incoming_publish_callback( return; } - /* Streaming operation handling */ - struct aws_hash_element *subscription_filter_element = NULL; - if (aws_hash_table_find( - &rr_client->streaming_operation_subscription_lists, &publish_event->topic, &subscription_filter_element) == - AWS_OP_SUCCESS && - subscription_filter_element != NULL) { - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "id=%p: request-response client incoming publish on topic '" PRInSTR "' matches streaming topic", - (void *)rr_client, - AWS_BYTE_CURSOR_PRI(publish_event->topic)); - - s_apply_publish_to_streaming_operation_list(subscription_filter_element->value, publish_event); - } else { - search(&rr_client->streaming_operation_wildcards_subscription_lists, &publish_event->topic); - } - - /* Request-Response handling */ - struct aws_hash_element *response_path_element = NULL; - if (aws_hash_table_find(&rr_client->request_response_paths, &publish_event->topic, &response_path_element) == - AWS_OP_SUCCESS && - response_path_element != NULL) { - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "id=%p: request-response client incoming publish on topic '" PRInSTR "' matches response path", - (void *)rr_client, - AWS_BYTE_CURSOR_PRI(publish_event->topic)); - - s_apply_publish_to_response_path_entry(rr_client, response_path_element->value, publish_event); - } + aws_mqtt_request_response_client_subscriptions_match( + &rr_client->subscriptions, + &publish_event->topic, + s_apply_publish_to_streaming_operation_list, + s_apply_publish_to_response_path_entry, + publish_event, + rr_client); } static void s_aws_rr_client_protocol_adapter_terminate_callback(void *user_data) { @@ -1165,7 +1129,7 @@ static struct aws_mqtt_request_response_client *s_aws_mqtt_request_response_clie s_compare_rr_operation_timeouts); aws_hash_table_init( - &rr_client->streaming_operation_subscription_lists, + &rr_client->subscriptions.streaming_operation_subscription_lists, allocator, MQTT_RR_CLIENT_OPERATION_TABLE_DEFAULT_SIZE, aws_hash_byte_cursor_ptr, @@ -1174,7 +1138,7 @@ static struct aws_mqtt_request_response_client *s_aws_mqtt_request_response_clie s_aws_rr_operation_list_topic_filter_entry_hash_element_destroy); aws_hash_table_init( - &rr_client->streaming_operation_wildcards_subscription_lists, + &rr_client->subscriptions.streaming_operation_wildcards_subscription_lists, allocator, 10, // TODO aws_hash_byte_cursor_ptr, @@ -1183,7 +1147,7 @@ static struct aws_mqtt_request_response_client *s_aws_mqtt_request_response_clie s_aws_rr_operation_list_topic_filter_entry_hash_element_destroy); aws_hash_table_init( - &rr_client->request_response_paths, + &rr_client->subscriptions.request_response_paths, allocator, MQTT_RR_CLIENT_RESPONSE_TABLE_DEFAULT_SIZE, aws_hash_byte_cursor_ptr, @@ -1298,11 +1262,14 @@ static int s_add_streaming_operation_to_subscription_topic_filter_table( struct aws_hash_element *element = NULL; if (is_topic_with_wildcard) { if (aws_hash_table_find( - &client->streaming_operation_wildcards_subscription_lists, &topic_filter_cursor, &element)) { + &client->subscriptions.streaming_operation_wildcards_subscription_lists, + &topic_filter_cursor, + &element)) { return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); } } else { - if (aws_hash_table_find(&client->streaming_operation_subscription_lists, &topic_filter_cursor, &element)) { + if (aws_hash_table_find( + &client->subscriptions.streaming_operation_subscription_lists, &topic_filter_cursor, &element)) { return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); } } @@ -1312,7 +1279,10 @@ static int s_add_streaming_operation_to_subscription_topic_filter_table( if (is_topic_with_wildcard) { entry = s_aws_rr_operation_list_topic_filter_entry_new(client->allocator, topic_filter_cursor); aws_hash_table_put( - &client->streaming_operation_wildcards_subscription_lists, &entry->topic_filter_cursor, entry, NULL); + &client->subscriptions.streaming_operation_wildcards_subscription_lists, + &entry->topic_filter_cursor, + entry, + NULL); AWS_LOGF_DEBUG( AWS_LS_MQTT_REQUEST_RESPONSE, "id=%p: request-response client adding wildcard topic filter '" PRInSTR @@ -1322,7 +1292,10 @@ static int s_add_streaming_operation_to_subscription_topic_filter_table( } else { entry = s_aws_rr_operation_list_topic_filter_entry_new(client->allocator, topic_filter_cursor); aws_hash_table_put( - &client->streaming_operation_subscription_lists, &entry->topic_filter_cursor, entry, NULL); + &client->subscriptions.streaming_operation_subscription_lists, + &entry->topic_filter_cursor, + entry, + NULL); AWS_LOGF_DEBUG( AWS_LS_MQTT_REQUEST_RESPONSE, "id=%p: request-response client adding topic filter '" PRInSTR "' to streaming subscriptions table", @@ -1363,7 +1336,7 @@ static int s_add_request_operation_to_response_path_table( aws_array_list_get_at(paths, &path, i); struct aws_hash_element *element = NULL; - if (aws_hash_table_find(&client->request_response_paths, &path.topic, &element)) { + if (aws_hash_table_find(&client->subscriptions.request_response_paths, &path.topic, &element)) { return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); } @@ -1375,7 +1348,7 @@ static int s_add_request_operation_to_response_path_table( struct aws_rr_response_path_entry *entry = s_aws_rr_response_path_entry_new(client->allocator, path.topic, path.correlation_token_json_path); - if (aws_hash_table_put(&client->request_response_paths, &entry->topic_cursor, entry, NULL)) { + if (aws_hash_table_put(&client->subscriptions.request_response_paths, &entry->topic_cursor, entry, NULL)) { return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); } } @@ -1874,7 +1847,8 @@ static void s_remove_operation_from_client_tables(struct aws_mqtt_rr_client_oper aws_array_list_get_at(paths, &path, i); struct aws_hash_element *element = NULL; - if (aws_hash_table_find(&client->request_response_paths, &path.topic, &element) || element == NULL) { + if (aws_hash_table_find(&client->subscriptions.request_response_paths, &path.topic, &element) || + element == NULL) { AWS_LOGF_ERROR( AWS_LS_MQTT_REQUEST_RESPONSE, "id=%p: internal state error removing reference to response path for topic " PRInSTR, @@ -1892,7 +1866,7 @@ static void s_remove_operation_from_client_tables(struct aws_mqtt_rr_client_oper "id=%p: removing last reference to response path for topic " PRInSTR, (void *)client, AWS_BYTE_CURSOR_PRI(path.topic)); - aws_hash_table_remove(&client->request_response_paths, &path.topic, NULL, NULL); + aws_hash_table_remove(&client->subscriptions.request_response_paths, &path.topic, NULL, NULL); } else { AWS_LOGF_DEBUG( AWS_LS_MQTT_REQUEST_RESPONSE, diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index 503bbf54..372f26db 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -8,7 +8,13 @@ #include #include -void search(const struct aws_hash_table *subscriptions, const struct aws_byte_cursor *topic) { +void aws_mqtt_request_response_client_subscriptions_destroy(struct aws_request_response_subscriptions *subscriptions) { + aws_hash_table_clean_up(&subscriptions->streaming_operation_subscription_lists); + aws_hash_table_clean_up(&subscriptions->streaming_operation_wildcards_subscription_lists); + aws_hash_table_clean_up(&subscriptions->request_response_paths); +} + +void s_match_wildcard_subscriptions(const struct aws_hash_table *subscriptions, const struct aws_byte_cursor *topic) { AWS_LOGF_INFO( AWS_LS_MQTT_REQUEST_RESPONSE, "= Looking subscription for topic '" PRInSTR "'", AWS_BYTE_CURSOR_PRI(*topic)); @@ -65,3 +71,42 @@ void search(const struct aws_hash_table *subscriptions, const struct aws_byte_cu } } } + +void aws_mqtt_request_response_client_subscriptions_match( + const struct aws_request_response_subscriptions *subscriptions, + const struct aws_byte_cursor *topic, + aws_mqtt_stream_operation_subscription_match_fn *on_stream_operation_subscription_match, + aws_mqtt_request_operation_subscription_match_fn *on_request_operation_subscription_match, + const struct aws_protocol_adapter_incoming_publish_event *publish_event, + struct aws_mqtt_request_response_client *rr_client) { + /* Streaming operation handling */ + struct aws_hash_element *subscription_filter_element = NULL; + if (aws_hash_table_find( + &subscriptions->streaming_operation_subscription_lists, topic, &subscription_filter_element) == + AWS_OP_SUCCESS && + subscription_filter_element != NULL) { + AWS_LOGF_DEBUG( + AWS_LS_MQTT_REQUEST_RESPONSE, + "id=%p: request-response client incoming publish on topic '" PRInSTR "' matches streaming topic", + (void *)rr_client, + AWS_BYTE_CURSOR_PRI(*topic)); + + on_stream_operation_subscription_match(subscription_filter_element->value, publish_event); + } + + s_match_wildcard_subscriptions(&subscriptions->streaming_operation_wildcards_subscription_lists, topic); + + /* Request-Response handling */ + struct aws_hash_element *response_path_element = NULL; + if (aws_hash_table_find(&subscriptions->request_response_paths, &publish_event->topic, &response_path_element) == + AWS_OP_SUCCESS && + response_path_element != NULL) { + AWS_LOGF_DEBUG( + AWS_LS_MQTT_REQUEST_RESPONSE, + "id=%p: request-response client incoming publish on topic '" PRInSTR "' matches response path", + (void *)rr_client, + AWS_BYTE_CURSOR_PRI(publish_event->topic)); + + on_request_operation_subscription_match(rr_client, response_path_element->value, publish_event); + } +} From af0bb88bbf92ac41047b9520b105461d4e84f52f Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Fri, 17 Jan 2025 11:41:26 -0800 Subject: [PATCH 04/29] Move init --- .../request_response_subscription_set.h | 20 +++++- .../request_response_client.c | 70 +------------------ .../request_response_subscription_set.c | 66 ++++++++++++++++- 3 files changed, 84 insertions(+), 72 deletions(-) diff --git a/include/aws/mqtt/private/request-response/request_response_subscription_set.h b/include/aws/mqtt/private/request-response/request_response_subscription_set.h index e3912ac2..bc3ca7ac 100644 --- a/include/aws/mqtt/private/request-response/request_response_subscription_set.h +++ b/include/aws/mqtt/private/request-response/request_response_subscription_set.h @@ -12,6 +12,8 @@ #include #include +struct aws_mqtt_request_response_client; + /* Holds subscriptions for request-response client. */ struct aws_request_response_subscriptions { /* @@ -45,8 +47,16 @@ struct aws_rr_operation_list_topic_filter_entry { struct aws_linked_list operations; }; -struct aws_mqtt_request_response_client; -struct aws_rr_response_path_entry; +struct aws_rr_response_path_entry { + struct aws_allocator *allocator; + + size_t ref_count; + + struct aws_byte_cursor topic_cursor; + struct aws_byte_buf topic; + + struct aws_byte_buf correlation_token_json_path; +}; typedef void(aws_mqtt_stream_operation_subscription_match_fn)( struct aws_rr_operation_list_topic_filter_entry *entry, @@ -59,7 +69,11 @@ typedef void(aws_mqtt_request_operation_subscription_match_fn)( AWS_EXTERN_C_BEGIN -AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_destroy( +AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_init( + struct aws_request_response_subscriptions *subscriptions, + struct aws_allocator *allocator); + +AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_cleanup( struct aws_request_response_subscriptions *subscriptions); AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_match( diff --git a/source/request-response/request_response_client.c b/source/request-response/request_response_client.c index 5fec0808..86c14e52 100644 --- a/source/request-response/request_response_client.c +++ b/source/request-response/request_response_client.c @@ -19,7 +19,6 @@ #include #define MQTT_RR_CLIENT_OPERATION_TABLE_DEFAULT_SIZE 50 -#define MQTT_RR_CLIENT_RESPONSE_TABLE_DEFAULT_SIZE 50 struct aws_mqtt_request_operation_storage { struct aws_mqtt_request_operation_options options; @@ -152,31 +151,6 @@ static struct aws_rr_operation_list_topic_filter_entry *s_aws_rr_operation_list_ return entry; } -static void s_aws_rr_operation_list_topic_filter_entry_destroy(struct aws_rr_operation_list_topic_filter_entry *entry) { - if (entry == NULL) { - return; - } - - aws_byte_buf_clean_up(&entry->topic_filter); - - aws_mem_release(entry->allocator, entry); -} - -static void s_aws_rr_operation_list_topic_filter_entry_hash_element_destroy(void *value) { - s_aws_rr_operation_list_topic_filter_entry_destroy(value); -} - -struct aws_rr_response_path_entry { - struct aws_allocator *allocator; - - size_t ref_count; - - struct aws_byte_cursor topic_cursor; - struct aws_byte_buf topic; - - struct aws_byte_buf correlation_token_json_path; -}; - static struct aws_rr_response_path_entry *s_aws_rr_response_path_entry_new( struct aws_allocator *allocator, struct aws_byte_cursor topic, @@ -193,21 +167,6 @@ static struct aws_rr_response_path_entry *s_aws_rr_response_path_entry_new( return entry; } -static void s_aws_rr_response_path_entry_destroy(struct aws_rr_response_path_entry *entry) { - if (entry == NULL) { - return; - } - - aws_byte_buf_clean_up(&entry->topic); - aws_byte_buf_clean_up(&entry->correlation_token_json_path); - - aws_mem_release(entry->allocator, entry); -} - -static void s_aws_rr_response_path_table_hash_element_destroy(void *value) { - s_aws_rr_response_path_entry_destroy(value); -} - struct aws_mqtt_rr_client_operation { struct aws_allocator *allocator; @@ -385,7 +344,7 @@ static void s_mqtt_request_response_client_final_destroy(struct aws_mqtt_request aws_priority_queue_clean_up(&client->operations_by_timeout); - aws_mqtt_request_response_client_subscriptions_destroy(&client->subscriptions); + aws_mqtt_request_response_client_subscriptions_cleanup(&client->subscriptions); aws_hash_table_clean_up(&client->operations_by_correlation_tokens); aws_mem_release(client->allocator, client); @@ -1128,32 +1087,7 @@ static struct aws_mqtt_request_response_client *s_aws_mqtt_request_response_clie sizeof(struct aws_mqtt_rr_client_operation *), s_compare_rr_operation_timeouts); - aws_hash_table_init( - &rr_client->subscriptions.streaming_operation_subscription_lists, - allocator, - MQTT_RR_CLIENT_OPERATION_TABLE_DEFAULT_SIZE, - aws_hash_byte_cursor_ptr, - aws_mqtt_byte_cursor_hash_equality, - NULL, - s_aws_rr_operation_list_topic_filter_entry_hash_element_destroy); - - aws_hash_table_init( - &rr_client->subscriptions.streaming_operation_wildcards_subscription_lists, - allocator, - 10, // TODO - aws_hash_byte_cursor_ptr, - aws_mqtt_byte_cursor_hash_equality, - NULL, - s_aws_rr_operation_list_topic_filter_entry_hash_element_destroy); - - aws_hash_table_init( - &rr_client->subscriptions.request_response_paths, - allocator, - MQTT_RR_CLIENT_RESPONSE_TABLE_DEFAULT_SIZE, - aws_hash_byte_cursor_ptr, - aws_mqtt_byte_cursor_hash_equality, - NULL, - s_aws_rr_response_path_table_hash_element_destroy); + aws_mqtt_request_response_client_subscriptions_init(&rr_client->subscriptions, allocator); aws_hash_table_init( &rr_client->operations_by_correlation_tokens, diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index 372f26db..c01cee7f 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -7,8 +7,72 @@ #include #include +#include -void aws_mqtt_request_response_client_subscriptions_destroy(struct aws_request_response_subscriptions *subscriptions) { +#define MQTT_RR_CLIENT_RESPONSE_TABLE_DEFAULT_SIZE 50 +#define MQTT_RR_CLIENT_OPERATION_TABLE_DEFAULT_SIZE 50 + +static void s_aws_rr_operation_list_topic_filter_entry_destroy(struct aws_rr_operation_list_topic_filter_entry *entry) { + if (entry == NULL) { + return; + } + + aws_byte_buf_clean_up(&entry->topic_filter); + + aws_mem_release(entry->allocator, entry); +} + +static void s_aws_rr_operation_list_topic_filter_entry_hash_element_destroy(void *value) { + s_aws_rr_operation_list_topic_filter_entry_destroy(value); +} + +static void s_aws_rr_response_path_entry_destroy(struct aws_rr_response_path_entry *entry) { + if (entry == NULL) { + return; + } + + aws_byte_buf_clean_up(&entry->topic); + aws_byte_buf_clean_up(&entry->correlation_token_json_path); + + aws_mem_release(entry->allocator, entry); +} + +static void s_aws_rr_response_path_table_hash_element_destroy(void *value) { + s_aws_rr_response_path_entry_destroy(value); +} + +void aws_mqtt_request_response_client_subscriptions_init( + struct aws_request_response_subscriptions *subscriptions, + struct aws_allocator *allocator) { + aws_hash_table_init( + &subscriptions->streaming_operation_subscription_lists, + allocator, + MQTT_RR_CLIENT_OPERATION_TABLE_DEFAULT_SIZE, + aws_hash_byte_cursor_ptr, + aws_mqtt_byte_cursor_hash_equality, + NULL, + s_aws_rr_operation_list_topic_filter_entry_hash_element_destroy); + + aws_hash_table_init( + &subscriptions->streaming_operation_wildcards_subscription_lists, + allocator, + MQTT_RR_CLIENT_OPERATION_TABLE_DEFAULT_SIZE, + aws_hash_byte_cursor_ptr, + aws_mqtt_byte_cursor_hash_equality, + NULL, + s_aws_rr_operation_list_topic_filter_entry_hash_element_destroy); + + aws_hash_table_init( + &subscriptions->request_response_paths, + allocator, + MQTT_RR_CLIENT_RESPONSE_TABLE_DEFAULT_SIZE, + aws_hash_byte_cursor_ptr, + aws_mqtt_byte_cursor_hash_equality, + NULL, + s_aws_rr_response_path_table_hash_element_destroy); +} + +void aws_mqtt_request_response_client_subscriptions_cleanup(struct aws_request_response_subscriptions *subscriptions) { aws_hash_table_clean_up(&subscriptions->streaming_operation_subscription_lists); aws_hash_table_clean_up(&subscriptions->streaming_operation_wildcards_subscription_lists); aws_hash_table_clean_up(&subscriptions->request_response_paths); From d2f5325e1e9f5f9b15d937c8ef127b250673365f Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Fri, 17 Jan 2025 13:21:55 -0800 Subject: [PATCH 05/29] Move add_stream_subscription --- .../request_response_subscription_set.h | 11 +++ .../request_response_client.c | 72 ++----------------- .../request_response_subscription_set.c | 63 +++++++++++++++- 3 files changed, 77 insertions(+), 69 deletions(-) diff --git a/include/aws/mqtt/private/request-response/request_response_subscription_set.h b/include/aws/mqtt/private/request-response/request_response_subscription_set.h index bc3ca7ac..f3c9e2f9 100644 --- a/include/aws/mqtt/private/request-response/request_response_subscription_set.h +++ b/include/aws/mqtt/private/request-response/request_response_subscription_set.h @@ -16,6 +16,8 @@ struct aws_mqtt_request_response_client; /* Holds subscriptions for request-response client. */ struct aws_request_response_subscriptions { + struct aws_allocator *allocator; + /* * Map from cursor (topic filter) -> list of streaming operations using that filter */ @@ -76,6 +78,15 @@ AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_init( AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_cleanup( struct aws_request_response_subscriptions *subscriptions); +AWS_MQTT_API struct aws_rr_operation_list_topic_filter_entry * + aws_mqtt_request_response_client_subscriptions_add_stream_subscription( + struct aws_mqtt_request_response_client *client, + struct aws_request_response_subscriptions *subscriptions, + const struct aws_byte_cursor *topic_filter); + +AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_add_request_subscription( + struct aws_request_response_subscriptions *subscriptions); + AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_match( const struct aws_request_response_subscriptions *subscriptions, const struct aws_byte_cursor *topic, diff --git a/source/request-response/request_response_client.c b/source/request-response/request_response_client.c index 86c14e52..dd1deba8 100644 --- a/source/request-response/request_response_client.c +++ b/source/request-response/request_response_client.c @@ -136,21 +136,6 @@ state, removed on operation completion/destruction */ -static struct aws_rr_operation_list_topic_filter_entry *s_aws_rr_operation_list_topic_filter_entry_new( - struct aws_allocator *allocator, - struct aws_byte_cursor topic_filter) { - struct aws_rr_operation_list_topic_filter_entry *entry = - aws_mem_calloc(allocator, 1, sizeof(struct aws_rr_operation_list_topic_filter_entry)); - - entry->allocator = allocator; - aws_byte_buf_init_copy_from_cursor(&entry->topic_filter, allocator, topic_filter); - entry->topic_filter_cursor = aws_byte_cursor_from_buf(&entry->topic_filter); - - aws_linked_list_init(&entry->operations); - - return entry; -} - static struct aws_rr_response_path_entry *s_aws_rr_response_path_entry_new( struct aws_allocator *allocator, struct aws_byte_cursor topic, @@ -1188,60 +1173,13 @@ static int s_add_streaming_operation_to_subscription_topic_filter_table( struct aws_byte_cursor topic_filter_cursor = operation->storage.streaming_storage.options.topic_filter; - bool is_topic_with_wildcard = false; - if (memchr(topic_filter_cursor.ptr, '+', topic_filter_cursor.len)) { - is_topic_with_wildcard = true; - } - - struct aws_hash_element *element = NULL; - if (is_topic_with_wildcard) { - if (aws_hash_table_find( - &client->subscriptions.streaming_operation_wildcards_subscription_lists, - &topic_filter_cursor, - &element)) { - return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); - } - } else { - if (aws_hash_table_find( - &client->subscriptions.streaming_operation_subscription_lists, &topic_filter_cursor, &element)) { - return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); - } - } - - struct aws_rr_operation_list_topic_filter_entry *entry = NULL; - if (element == NULL) { - if (is_topic_with_wildcard) { - entry = s_aws_rr_operation_list_topic_filter_entry_new(client->allocator, topic_filter_cursor); - aws_hash_table_put( - &client->subscriptions.streaming_operation_wildcards_subscription_lists, - &entry->topic_filter_cursor, - entry, - NULL); - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "id=%p: request-response client adding wildcard topic filter '" PRInSTR - "' to streaming subscriptions table", - (void *)client, - AWS_BYTE_CURSOR_PRI(topic_filter_cursor)); - } else { - entry = s_aws_rr_operation_list_topic_filter_entry_new(client->allocator, topic_filter_cursor); - aws_hash_table_put( - &client->subscriptions.streaming_operation_subscription_lists, - &entry->topic_filter_cursor, - entry, - NULL); - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "id=%p: request-response client adding topic filter '" PRInSTR "' to streaming subscriptions table", - (void *)client, - AWS_BYTE_CURSOR_PRI(topic_filter_cursor)); - } - } else { - entry = element->value; + struct aws_rr_operation_list_topic_filter_entry *entry = + aws_mqtt_request_response_client_subscriptions_add_stream_subscription( + client, &client->subscriptions, &topic_filter_cursor); + if (entry == NULL) { + return AWS_OP_ERR; } - AWS_FATAL_ASSERT(entry != NULL); - if (aws_linked_list_node_is_in_list(&operation->node)) { aws_linked_list_remove(&operation->node); } diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index c01cee7f..8f875374 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -44,6 +44,9 @@ static void s_aws_rr_response_path_table_hash_element_destroy(void *value) { void aws_mqtt_request_response_client_subscriptions_init( struct aws_request_response_subscriptions *subscriptions, struct aws_allocator *allocator) { + + subscriptions->allocator = allocator; + aws_hash_table_init( &subscriptions->streaming_operation_subscription_lists, allocator, @@ -78,7 +81,62 @@ void aws_mqtt_request_response_client_subscriptions_cleanup(struct aws_request_r aws_hash_table_clean_up(&subscriptions->request_response_paths); } -void s_match_wildcard_subscriptions(const struct aws_hash_table *subscriptions, const struct aws_byte_cursor *topic) { +static struct aws_rr_operation_list_topic_filter_entry *s_aws_rr_operation_list_topic_filter_entry_new( + struct aws_allocator *allocator, + struct aws_byte_cursor topic_filter) { + struct aws_rr_operation_list_topic_filter_entry *entry = + aws_mem_calloc(allocator, 1, sizeof(struct aws_rr_operation_list_topic_filter_entry)); + + entry->allocator = allocator; + aws_byte_buf_init_copy_from_cursor(&entry->topic_filter, allocator, topic_filter); + entry->topic_filter_cursor = aws_byte_cursor_from_buf(&entry->topic_filter); + + aws_linked_list_init(&entry->operations); + + return entry; +} + +struct aws_rr_operation_list_topic_filter_entry *aws_mqtt_request_response_client_subscriptions_add_stream_subscription( + struct aws_mqtt_request_response_client *client, + struct aws_request_response_subscriptions *subscriptions, + const struct aws_byte_cursor *topic_filter) { + + bool is_topic_with_wildcard = + (memchr(topic_filter->ptr, '+', topic_filter->len) || memchr(topic_filter->ptr, '#', topic_filter->len)); + + struct aws_hash_table *subscription_lists = is_topic_with_wildcard + ? &subscriptions->streaming_operation_wildcards_subscription_lists + : &subscriptions->streaming_operation_subscription_lists; + + struct aws_hash_element *element = NULL; + if (aws_hash_table_find(subscription_lists, topic_filter, &element)) { + aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); + return NULL; + } + + struct aws_rr_operation_list_topic_filter_entry *entry = NULL; + if (element == NULL) { + entry = s_aws_rr_operation_list_topic_filter_entry_new(subscriptions->allocator, *topic_filter); + aws_hash_table_put(subscription_lists, &entry->topic_filter_cursor, entry, NULL); + AWS_LOGF_DEBUG( + AWS_LS_MQTT_REQUEST_RESPONSE, + "id=%p: request-response client adding wildcard topic filter '" PRInSTR + "' to streaming subscriptions table", + (void *)client, + AWS_BYTE_CURSOR_PRI(*topic_filter)); + } else { + entry = element->value; + } + + AWS_FATAL_ASSERT(entry != NULL); + + return entry; +} + +static void s_match_wildcard_stream_subscriptions( + const struct aws_hash_table *subscriptions, + const struct aws_byte_cursor *topic) { + AWS_LOGF_INFO( AWS_LS_MQTT_REQUEST_RESPONSE, "= Looking subscription for topic '" PRInSTR "'", AWS_BYTE_CURSOR_PRI(*topic)); @@ -143,6 +201,7 @@ void aws_mqtt_request_response_client_subscriptions_match( aws_mqtt_request_operation_subscription_match_fn *on_request_operation_subscription_match, const struct aws_protocol_adapter_incoming_publish_event *publish_event, struct aws_mqtt_request_response_client *rr_client) { + /* Streaming operation handling */ struct aws_hash_element *subscription_filter_element = NULL; if (aws_hash_table_find( @@ -158,7 +217,7 @@ void aws_mqtt_request_response_client_subscriptions_match( on_stream_operation_subscription_match(subscription_filter_element->value, publish_event); } - s_match_wildcard_subscriptions(&subscriptions->streaming_operation_wildcards_subscription_lists, topic); + s_match_wildcard_stream_subscriptions(&subscriptions->streaming_operation_wildcards_subscription_lists, topic); /* Request-Response handling */ struct aws_hash_element *response_path_element = NULL; From 72639939e993dd24fad7e1a4fd6b20b2b9e26879 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Fri, 17 Jan 2025 13:58:46 -0800 Subject: [PATCH 06/29] Move add_request_subscription --- .../request_response_subscription_set.h | 5 +- .../request_response_client.c | 41 +--------------- .../request_response_subscription_set.c | 48 +++++++++++++++++++ 3 files changed, 52 insertions(+), 42 deletions(-) diff --git a/include/aws/mqtt/private/request-response/request_response_subscription_set.h b/include/aws/mqtt/private/request-response/request_response_subscription_set.h index f3c9e2f9..48180def 100644 --- a/include/aws/mqtt/private/request-response/request_response_subscription_set.h +++ b/include/aws/mqtt/private/request-response/request_response_subscription_set.h @@ -84,8 +84,9 @@ AWS_MQTT_API struct aws_rr_operation_list_topic_filter_entry * struct aws_request_response_subscriptions *subscriptions, const struct aws_byte_cursor *topic_filter); -AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_add_request_subscription( - struct aws_request_response_subscriptions *subscriptions); +AWS_MQTT_API int aws_mqtt_request_response_client_subscriptions_add_request_subscription( + struct aws_request_response_subscriptions *subscriptions, + const struct aws_array_list *paths); AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_match( const struct aws_request_response_subscriptions *subscriptions, diff --git a/source/request-response/request_response_client.c b/source/request-response/request_response_client.c index dd1deba8..93fa2fb2 100644 --- a/source/request-response/request_response_client.c +++ b/source/request-response/request_response_client.c @@ -136,22 +136,6 @@ state, removed on operation completion/destruction */ -static struct aws_rr_response_path_entry *s_aws_rr_response_path_entry_new( - struct aws_allocator *allocator, - struct aws_byte_cursor topic, - struct aws_byte_cursor correlation_token_json_path) { - struct aws_rr_response_path_entry *entry = aws_mem_calloc(allocator, 1, sizeof(struct aws_rr_response_path_entry)); - - entry->allocator = allocator; - entry->ref_count = 1; - aws_byte_buf_init_copy_from_cursor(&entry->topic, allocator, topic); - entry->topic_cursor = aws_byte_cursor_from_buf(&entry->topic); - - aws_byte_buf_init_copy_from_cursor(&entry->correlation_token_json_path, allocator, correlation_token_json_path); - - return entry; -} - struct aws_mqtt_rr_client_operation { struct aws_allocator *allocator; @@ -1202,30 +1186,7 @@ static int s_add_request_operation_to_response_path_table( struct aws_mqtt_rr_client_operation *operation) { struct aws_array_list *paths = &operation->storage.request_storage.operation_response_paths; - size_t path_count = aws_array_list_length(paths); - for (size_t i = 0; i < path_count; ++i) { - struct aws_mqtt_request_operation_response_path path; - aws_array_list_get_at(paths, &path, i); - - struct aws_hash_element *element = NULL; - if (aws_hash_table_find(&client->subscriptions.request_response_paths, &path.topic, &element)) { - return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); - } - - if (element != NULL) { - struct aws_rr_response_path_entry *entry = element->value; - ++entry->ref_count; - continue; - } - - struct aws_rr_response_path_entry *entry = - s_aws_rr_response_path_entry_new(client->allocator, path.topic, path.correlation_token_json_path); - if (aws_hash_table_put(&client->subscriptions.request_response_paths, &entry->topic_cursor, entry, NULL)) { - return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); - } - } - - return AWS_OP_SUCCESS; + return aws_mqtt_request_response_client_subscriptions_add_request_subscription(&client->subscriptions, paths); } static int s_add_request_operation_to_correlation_token_table( diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index 8f875374..1f090676 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -8,6 +8,7 @@ #include #include #include +#include #define MQTT_RR_CLIENT_RESPONSE_TABLE_DEFAULT_SIZE 50 #define MQTT_RR_CLIENT_OPERATION_TABLE_DEFAULT_SIZE 50 @@ -133,6 +134,53 @@ struct aws_rr_operation_list_topic_filter_entry *aws_mqtt_request_response_clien return entry; } +static struct aws_rr_response_path_entry *s_aws_rr_response_path_entry_new( + struct aws_allocator *allocator, + struct aws_byte_cursor topic, + struct aws_byte_cursor correlation_token_json_path) { + struct aws_rr_response_path_entry *entry = aws_mem_calloc(allocator, 1, sizeof(struct aws_rr_response_path_entry)); + + entry->allocator = allocator; + entry->ref_count = 1; + aws_byte_buf_init_copy_from_cursor(&entry->topic, allocator, topic); + entry->topic_cursor = aws_byte_cursor_from_buf(&entry->topic); + + aws_byte_buf_init_copy_from_cursor(&entry->correlation_token_json_path, allocator, correlation_token_json_path); + + return entry; +} + +int aws_mqtt_request_response_client_subscriptions_add_request_subscription( + struct aws_request_response_subscriptions *subscriptions, + const struct aws_array_list *paths) { + + size_t path_count = aws_array_list_length(paths); + for (size_t i = 0; i < path_count; ++i) { + struct aws_mqtt_request_operation_response_path path; + aws_array_list_get_at(paths, &path, i); + + struct aws_hash_element *element = NULL; + if (aws_hash_table_find(&subscriptions->request_response_paths, &path.topic, &element)) { + return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); + } + + if (element != NULL) { + struct aws_rr_response_path_entry *entry = element->value; + ++entry->ref_count; + continue; + } + + struct aws_rr_response_path_entry *entry = + s_aws_rr_response_path_entry_new(subscriptions->allocator, path.topic, path.correlation_token_json_path); + if (aws_hash_table_put(&subscriptions->request_response_paths, &entry->topic_cursor, entry, NULL)) { + s_aws_rr_response_path_entry_destroy(entry); + return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); + } + } + + return AWS_OP_SUCCESS; +} + static void s_match_wildcard_stream_subscriptions( const struct aws_hash_table *subscriptions, const struct aws_byte_cursor *topic) { From f093b1dbe4bf1f5a29c6bcf916d663381493a410 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Fri, 17 Jan 2025 14:16:20 -0800 Subject: [PATCH 07/29] Move remove_request_subscription --- .../request_response_subscription_set.h | 10 ++-- .../request_response_client.c | 42 ++------------- .../request_response_subscription_set.c | 53 ++++++++++++++++--- 3 files changed, 57 insertions(+), 48 deletions(-) diff --git a/include/aws/mqtt/private/request-response/request_response_subscription_set.h b/include/aws/mqtt/private/request-response/request_response_subscription_set.h index 48180def..3115052f 100644 --- a/include/aws/mqtt/private/request-response/request_response_subscription_set.h +++ b/include/aws/mqtt/private/request-response/request_response_subscription_set.h @@ -17,6 +17,7 @@ struct aws_mqtt_request_response_client; /* Holds subscriptions for request-response client. */ struct aws_request_response_subscriptions { struct aws_allocator *allocator; + struct aws_mqtt_request_response_client *client; /* * Map from cursor (topic filter) -> list of streaming operations using that filter @@ -73,6 +74,7 @@ AWS_EXTERN_C_BEGIN AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_init( struct aws_request_response_subscriptions *subscriptions, + struct aws_mqtt_request_response_client *client, struct aws_allocator *allocator); AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_cleanup( @@ -80,7 +82,6 @@ AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_cleanup( AWS_MQTT_API struct aws_rr_operation_list_topic_filter_entry * aws_mqtt_request_response_client_subscriptions_add_stream_subscription( - struct aws_mqtt_request_response_client *client, struct aws_request_response_subscriptions *subscriptions, const struct aws_byte_cursor *topic_filter); @@ -88,13 +89,16 @@ AWS_MQTT_API int aws_mqtt_request_response_client_subscriptions_add_request_subs struct aws_request_response_subscriptions *subscriptions, const struct aws_array_list *paths); +AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_remove_request_subscription( + struct aws_request_response_subscriptions *subscriptions, + const struct aws_array_list *paths); + AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_match( const struct aws_request_response_subscriptions *subscriptions, const struct aws_byte_cursor *topic, aws_mqtt_stream_operation_subscription_match_fn *on_stream_operation_subscription_match, aws_mqtt_request_operation_subscription_match_fn *on_request_operation_subscription_match, - const struct aws_protocol_adapter_incoming_publish_event *publish_event, - struct aws_mqtt_request_response_client *rr_client); + const struct aws_protocol_adapter_incoming_publish_event *publish_event); AWS_EXTERN_C_END diff --git a/source/request-response/request_response_client.c b/source/request-response/request_response_client.c index 93fa2fb2..7530c8da 100644 --- a/source/request-response/request_response_client.c +++ b/source/request-response/request_response_client.c @@ -965,8 +965,7 @@ static void s_aws_rr_client_protocol_adapter_incoming_publish_callback( &publish_event->topic, s_apply_publish_to_streaming_operation_list, s_apply_publish_to_response_path_entry, - publish_event, - rr_client); + publish_event); } static void s_aws_rr_client_protocol_adapter_terminate_callback(void *user_data) { @@ -1056,7 +1055,7 @@ static struct aws_mqtt_request_response_client *s_aws_mqtt_request_response_clie sizeof(struct aws_mqtt_rr_client_operation *), s_compare_rr_operation_timeouts); - aws_mqtt_request_response_client_subscriptions_init(&rr_client->subscriptions, allocator); + aws_mqtt_request_response_client_subscriptions_init(&rr_client->subscriptions, rr_client, allocator); aws_hash_table_init( &rr_client->operations_by_correlation_tokens, @@ -1159,7 +1158,7 @@ static int s_add_streaming_operation_to_subscription_topic_filter_table( struct aws_rr_operation_list_topic_filter_entry *entry = aws_mqtt_request_response_client_subscriptions_add_stream_subscription( - client, &client->subscriptions, &topic_filter_cursor); + &client->subscriptions, &topic_filter_cursor); if (entry == NULL) { return AWS_OP_ERR; } @@ -1674,41 +1673,8 @@ static void s_remove_operation_from_client_tables(struct aws_mqtt_rr_client_oper NULL); struct aws_array_list *paths = &operation->storage.request_storage.operation_response_paths; - size_t path_count = aws_array_list_length(paths); - for (size_t i = 0; i < path_count; ++i) { - struct aws_mqtt_request_operation_response_path path; - aws_array_list_get_at(paths, &path, i); - struct aws_hash_element *element = NULL; - if (aws_hash_table_find(&client->subscriptions.request_response_paths, &path.topic, &element) || - element == NULL) { - AWS_LOGF_ERROR( - AWS_LS_MQTT_REQUEST_RESPONSE, - "id=%p: internal state error removing reference to response path for topic " PRInSTR, - (void *)client, - AWS_BYTE_CURSOR_PRI(path.topic)); - continue; - } - - struct aws_rr_response_path_entry *entry = element->value; - --entry->ref_count; - - if (entry->ref_count == 0) { - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "id=%p: removing last reference to response path for topic " PRInSTR, - (void *)client, - AWS_BYTE_CURSOR_PRI(path.topic)); - aws_hash_table_remove(&client->subscriptions.request_response_paths, &path.topic, NULL, NULL); - } else { - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "id=%p: removing reference to response path for topic " PRInSTR ", %zu references remain", - (void *)client, - AWS_BYTE_CURSOR_PRI(path.topic), - entry->ref_count); - } - } + aws_mqtt_request_response_client_subscriptions_remove_request_subscription(&client->subscriptions, paths); } static void s_mqtt_rr_client_destroy_operation(struct aws_task *task, void *arg, enum aws_task_status status) { diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index 1f090676..26601864 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -44,9 +44,11 @@ static void s_aws_rr_response_path_table_hash_element_destroy(void *value) { void aws_mqtt_request_response_client_subscriptions_init( struct aws_request_response_subscriptions *subscriptions, + struct aws_mqtt_request_response_client *client, struct aws_allocator *allocator) { subscriptions->allocator = allocator; + subscriptions->client = client; aws_hash_table_init( &subscriptions->streaming_operation_subscription_lists, @@ -98,7 +100,6 @@ static struct aws_rr_operation_list_topic_filter_entry *s_aws_rr_operation_list_ } struct aws_rr_operation_list_topic_filter_entry *aws_mqtt_request_response_client_subscriptions_add_stream_subscription( - struct aws_mqtt_request_response_client *client, struct aws_request_response_subscriptions *subscriptions, const struct aws_byte_cursor *topic_filter) { @@ -123,7 +124,7 @@ struct aws_rr_operation_list_topic_filter_entry *aws_mqtt_request_response_clien AWS_LS_MQTT_REQUEST_RESPONSE, "id=%p: request-response client adding wildcard topic filter '" PRInSTR "' to streaming subscriptions table", - (void *)client, + (void *)subscriptions->client, AWS_BYTE_CURSOR_PRI(*topic_filter)); } else { entry = element->value; @@ -181,6 +182,45 @@ int aws_mqtt_request_response_client_subscriptions_add_request_subscription( return AWS_OP_SUCCESS; } +void aws_mqtt_request_response_client_subscriptions_remove_request_subscription( + struct aws_request_response_subscriptions *subscriptions, + const struct aws_array_list *paths) { + size_t path_count = aws_array_list_length(paths); + for (size_t i = 0; i < path_count; ++i) { + struct aws_mqtt_request_operation_response_path path; + aws_array_list_get_at(paths, &path, i); + + struct aws_hash_element *element = NULL; + if (aws_hash_table_find(&subscriptions->request_response_paths, &path.topic, &element) || element == NULL) { + AWS_LOGF_ERROR( + AWS_LS_MQTT_REQUEST_RESPONSE, + "id=%p: internal state error removing reference to response path for topic " PRInSTR, + (void *)subscriptions->client, + AWS_BYTE_CURSOR_PRI(path.topic)); + continue; + } + + struct aws_rr_response_path_entry *entry = element->value; + --entry->ref_count; + + if (entry->ref_count == 0) { + AWS_LOGF_DEBUG( + AWS_LS_MQTT_REQUEST_RESPONSE, + "id=%p: removing last reference to response path for topic " PRInSTR, + (void *)subscriptions->client, + AWS_BYTE_CURSOR_PRI(path.topic)); + aws_hash_table_remove(&subscriptions->request_response_paths, &path.topic, NULL, NULL); + } else { + AWS_LOGF_DEBUG( + AWS_LS_MQTT_REQUEST_RESPONSE, + "id=%p: removing reference to response path for topic " PRInSTR ", %zu references remain", + (void *)subscriptions->client, + AWS_BYTE_CURSOR_PRI(path.topic), + entry->ref_count); + } + } +} + static void s_match_wildcard_stream_subscriptions( const struct aws_hash_table *subscriptions, const struct aws_byte_cursor *topic) { @@ -247,8 +287,7 @@ void aws_mqtt_request_response_client_subscriptions_match( const struct aws_byte_cursor *topic, aws_mqtt_stream_operation_subscription_match_fn *on_stream_operation_subscription_match, aws_mqtt_request_operation_subscription_match_fn *on_request_operation_subscription_match, - const struct aws_protocol_adapter_incoming_publish_event *publish_event, - struct aws_mqtt_request_response_client *rr_client) { + const struct aws_protocol_adapter_incoming_publish_event *publish_event) { /* Streaming operation handling */ struct aws_hash_element *subscription_filter_element = NULL; @@ -259,7 +298,7 @@ void aws_mqtt_request_response_client_subscriptions_match( AWS_LOGF_DEBUG( AWS_LS_MQTT_REQUEST_RESPONSE, "id=%p: request-response client incoming publish on topic '" PRInSTR "' matches streaming topic", - (void *)rr_client, + (void *)subscriptions->client, AWS_BYTE_CURSOR_PRI(*topic)); on_stream_operation_subscription_match(subscription_filter_element->value, publish_event); @@ -275,9 +314,9 @@ void aws_mqtt_request_response_client_subscriptions_match( AWS_LOGF_DEBUG( AWS_LS_MQTT_REQUEST_RESPONSE, "id=%p: request-response client incoming publish on topic '" PRInSTR "' matches response path", - (void *)rr_client, + (void *)subscriptions->client, AWS_BYTE_CURSOR_PRI(publish_event->topic)); - on_request_operation_subscription_match(rr_client, response_path_element->value, publish_event); + on_request_operation_subscription_match(subscriptions->client, response_path_element->value, publish_event); } } From 8fdc4fce9a32b0561acde0a4db5d7c5ff99cdd75 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Fri, 17 Jan 2025 14:25:45 -0800 Subject: [PATCH 08/29] Style fixes --- .../request_response_subscription_set.h | 8 ++- .../request_response_subscription_set.c | 62 +++++++++---------- 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/include/aws/mqtt/private/request-response/request_response_subscription_set.h b/include/aws/mqtt/private/request-response/request_response_subscription_set.h index 3115052f..eaeead07 100644 --- a/include/aws/mqtt/private/request-response/request_response_subscription_set.h +++ b/include/aws/mqtt/private/request-response/request_response_subscription_set.h @@ -17,6 +17,9 @@ struct aws_mqtt_request_response_client; /* Holds subscriptions for request-response client. */ struct aws_request_response_subscriptions { struct aws_allocator *allocator; + + /* Convenient access to request-response client instance. Lifetime of aws_request_response_subscriptions is bound to + * client, so no ref-counting is required here. */ struct aws_mqtt_request_response_client *client; /* @@ -39,7 +42,7 @@ struct aws_request_response_subscriptions { }; /* - * This is the (key and) value in hash table (4) above. + * This is the (key and) value in stream subscriptions tables. */ struct aws_rr_operation_list_topic_filter_entry { struct aws_allocator *allocator; @@ -50,6 +53,9 @@ struct aws_rr_operation_list_topic_filter_entry { struct aws_linked_list operations; }; +/* + * Value in request subscriptions table. + */ struct aws_rr_response_path_entry { struct aws_allocator *allocator; diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index 26601864..f2a0f86c 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -13,6 +13,21 @@ #define MQTT_RR_CLIENT_RESPONSE_TABLE_DEFAULT_SIZE 50 #define MQTT_RR_CLIENT_OPERATION_TABLE_DEFAULT_SIZE 50 +static struct aws_rr_operation_list_topic_filter_entry *s_aws_rr_operation_list_topic_filter_entry_new( + struct aws_allocator *allocator, + struct aws_byte_cursor topic_filter) { + struct aws_rr_operation_list_topic_filter_entry *entry = + aws_mem_calloc(allocator, 1, sizeof(struct aws_rr_operation_list_topic_filter_entry)); + + entry->allocator = allocator; + aws_byte_buf_init_copy_from_cursor(&entry->topic_filter, allocator, topic_filter); + entry->topic_filter_cursor = aws_byte_cursor_from_buf(&entry->topic_filter); + + aws_linked_list_init(&entry->operations); + + return entry; +} + static void s_aws_rr_operation_list_topic_filter_entry_destroy(struct aws_rr_operation_list_topic_filter_entry *entry) { if (entry == NULL) { return; @@ -27,6 +42,22 @@ static void s_aws_rr_operation_list_topic_filter_entry_hash_element_destroy(void s_aws_rr_operation_list_topic_filter_entry_destroy(value); } +static struct aws_rr_response_path_entry *s_aws_rr_response_path_entry_new( + struct aws_allocator *allocator, + struct aws_byte_cursor topic, + struct aws_byte_cursor correlation_token_json_path) { + struct aws_rr_response_path_entry *entry = aws_mem_calloc(allocator, 1, sizeof(struct aws_rr_response_path_entry)); + + entry->allocator = allocator; + entry->ref_count = 1; + aws_byte_buf_init_copy_from_cursor(&entry->topic, allocator, topic); + entry->topic_cursor = aws_byte_cursor_from_buf(&entry->topic); + + aws_byte_buf_init_copy_from_cursor(&entry->correlation_token_json_path, allocator, correlation_token_json_path); + + return entry; +} + static void s_aws_rr_response_path_entry_destroy(struct aws_rr_response_path_entry *entry) { if (entry == NULL) { return; @@ -84,21 +115,6 @@ void aws_mqtt_request_response_client_subscriptions_cleanup(struct aws_request_r aws_hash_table_clean_up(&subscriptions->request_response_paths); } -static struct aws_rr_operation_list_topic_filter_entry *s_aws_rr_operation_list_topic_filter_entry_new( - struct aws_allocator *allocator, - struct aws_byte_cursor topic_filter) { - struct aws_rr_operation_list_topic_filter_entry *entry = - aws_mem_calloc(allocator, 1, sizeof(struct aws_rr_operation_list_topic_filter_entry)); - - entry->allocator = allocator; - aws_byte_buf_init_copy_from_cursor(&entry->topic_filter, allocator, topic_filter); - entry->topic_filter_cursor = aws_byte_cursor_from_buf(&entry->topic_filter); - - aws_linked_list_init(&entry->operations); - - return entry; -} - struct aws_rr_operation_list_topic_filter_entry *aws_mqtt_request_response_client_subscriptions_add_stream_subscription( struct aws_request_response_subscriptions *subscriptions, const struct aws_byte_cursor *topic_filter) { @@ -135,22 +151,6 @@ struct aws_rr_operation_list_topic_filter_entry *aws_mqtt_request_response_clien return entry; } -static struct aws_rr_response_path_entry *s_aws_rr_response_path_entry_new( - struct aws_allocator *allocator, - struct aws_byte_cursor topic, - struct aws_byte_cursor correlation_token_json_path) { - struct aws_rr_response_path_entry *entry = aws_mem_calloc(allocator, 1, sizeof(struct aws_rr_response_path_entry)); - - entry->allocator = allocator; - entry->ref_count = 1; - aws_byte_buf_init_copy_from_cursor(&entry->topic, allocator, topic); - entry->topic_cursor = aws_byte_cursor_from_buf(&entry->topic); - - aws_byte_buf_init_copy_from_cursor(&entry->correlation_token_json_path, allocator, correlation_token_json_path); - - return entry; -} - int aws_mqtt_request_response_client_subscriptions_add_request_subscription( struct aws_request_response_subscriptions *subscriptions, const struct aws_array_list *paths) { From 4757825344bfa48cb3550b66cfff0a2cfe31fc47 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Fri, 17 Jan 2025 14:37:44 -0800 Subject: [PATCH 09/29] Call on_publish cb for wildcarded topics --- .../request_response_subscription_set.h | 5 ++-- .../request_response_client.c | 5 ++-- .../request_response_subscription_set.c | 29 ++++++++++++------- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/include/aws/mqtt/private/request-response/request_response_subscription_set.h b/include/aws/mqtt/private/request-response/request_response_subscription_set.h index eaeead07..4beb72dc 100644 --- a/include/aws/mqtt/private/request-response/request_response_subscription_set.h +++ b/include/aws/mqtt/private/request-response/request_response_subscription_set.h @@ -101,10 +101,9 @@ AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_remove_request_ AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_match( const struct aws_request_response_subscriptions *subscriptions, - const struct aws_byte_cursor *topic, + const struct aws_protocol_adapter_incoming_publish_event *publish_event, aws_mqtt_stream_operation_subscription_match_fn *on_stream_operation_subscription_match, - aws_mqtt_request_operation_subscription_match_fn *on_request_operation_subscription_match, - const struct aws_protocol_adapter_incoming_publish_event *publish_event); + aws_mqtt_request_operation_subscription_match_fn *on_request_operation_subscription_match); AWS_EXTERN_C_END diff --git a/source/request-response/request_response_client.c b/source/request-response/request_response_client.c index 7530c8da..26c60df9 100644 --- a/source/request-response/request_response_client.c +++ b/source/request-response/request_response_client.c @@ -962,10 +962,9 @@ static void s_aws_rr_client_protocol_adapter_incoming_publish_callback( aws_mqtt_request_response_client_subscriptions_match( &rr_client->subscriptions, - &publish_event->topic, + publish_event, s_apply_publish_to_streaming_operation_list, - s_apply_publish_to_response_path_entry, - publish_event); + s_apply_publish_to_response_path_entry); } static void s_aws_rr_client_protocol_adapter_terminate_callback(void *user_data) { diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index f2a0f86c..91f7a2c6 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -223,10 +223,13 @@ void aws_mqtt_request_response_client_subscriptions_remove_request_subscription( static void s_match_wildcard_stream_subscriptions( const struct aws_hash_table *subscriptions, - const struct aws_byte_cursor *topic) { + const struct aws_protocol_adapter_incoming_publish_event *publish_event, + aws_mqtt_stream_operation_subscription_match_fn *on_stream_operation_subscription_match) { AWS_LOGF_INFO( - AWS_LS_MQTT_REQUEST_RESPONSE, "= Looking subscription for topic '" PRInSTR "'", AWS_BYTE_CURSOR_PRI(*topic)); + AWS_LS_MQTT_REQUEST_RESPONSE, + "= Looking subscription for topic '" PRInSTR "'", + AWS_BYTE_CURSOR_PRI(publish_event->topic)); for (struct aws_hash_iter iter = aws_hash_iter_begin(subscriptions); !aws_hash_iter_done(&iter); aws_hash_iter_next(&iter)) { @@ -250,7 +253,7 @@ static void s_match_wildcard_stream_subscriptions( "=== subscription topic filter segment is '" PRInSTR "'", AWS_BYTE_CURSOR_PRI(subscription_topic_filter_segment)); - if (!aws_byte_cursor_next_split(topic, '/', &topic_segment)) { + if (!aws_byte_cursor_next_split(&publish_event->topic, '/', &topic_segment)) { AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== topic segment is NULL"); match = false; break; @@ -270,12 +273,13 @@ static void s_match_wildcard_stream_subscriptions( } } - if (aws_byte_cursor_next_split(topic, '/', &topic_segment)) { + if (aws_byte_cursor_next_split(&publish_event->topic, '/', &topic_segment)) { match = false; } if (match) { AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== found subscription match"); + on_stream_operation_subscription_match(iter.element.value, publish_event); } else { AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== this is not the right subscription"); } @@ -284,27 +288,30 @@ static void s_match_wildcard_stream_subscriptions( void aws_mqtt_request_response_client_subscriptions_match( const struct aws_request_response_subscriptions *subscriptions, - const struct aws_byte_cursor *topic, + const struct aws_protocol_adapter_incoming_publish_event *publish_event, aws_mqtt_stream_operation_subscription_match_fn *on_stream_operation_subscription_match, - aws_mqtt_request_operation_subscription_match_fn *on_request_operation_subscription_match, - const struct aws_protocol_adapter_incoming_publish_event *publish_event) { + aws_mqtt_request_operation_subscription_match_fn *on_request_operation_subscription_match) { /* Streaming operation handling */ struct aws_hash_element *subscription_filter_element = NULL; if (aws_hash_table_find( - &subscriptions->streaming_operation_subscription_lists, topic, &subscription_filter_element) == - AWS_OP_SUCCESS && + &subscriptions->streaming_operation_subscription_lists, + &publish_event->topic, + &subscription_filter_element) == AWS_OP_SUCCESS && subscription_filter_element != NULL) { AWS_LOGF_DEBUG( AWS_LS_MQTT_REQUEST_RESPONSE, "id=%p: request-response client incoming publish on topic '" PRInSTR "' matches streaming topic", (void *)subscriptions->client, - AWS_BYTE_CURSOR_PRI(*topic)); + AWS_BYTE_CURSOR_PRI(publish_event->topic)); on_stream_operation_subscription_match(subscription_filter_element->value, publish_event); } - s_match_wildcard_stream_subscriptions(&subscriptions->streaming_operation_wildcards_subscription_lists, topic); + s_match_wildcard_stream_subscriptions( + &subscriptions->streaming_operation_wildcards_subscription_lists, + publish_event, + on_stream_operation_subscription_match); /* Request-Response handling */ struct aws_hash_element *response_path_element = NULL; From 8d85817964a32e46b57e8225b652641f1ecc43b3 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Mon, 20 Jan 2025 11:56:30 -0800 Subject: [PATCH 10/29] Change stream cb --- .../request_response_subscription_set.h | 11 +-- .../request_response_client.c | 15 ++-- .../request_response_subscription_set.c | 20 ++++-- tests/CMakeLists.txt | 2 + .../request_response_client_tests.c | 69 ++++++++++++++++++- 5 files changed, 100 insertions(+), 17 deletions(-) diff --git a/include/aws/mqtt/private/request-response/request_response_subscription_set.h b/include/aws/mqtt/private/request-response/request_response_subscription_set.h index 4beb72dc..bf9cf2a9 100644 --- a/include/aws/mqtt/private/request-response/request_response_subscription_set.h +++ b/include/aws/mqtt/private/request-response/request_response_subscription_set.h @@ -20,7 +20,7 @@ struct aws_request_response_subscriptions { /* Convenient access to request-response client instance. Lifetime of aws_request_response_subscriptions is bound to * client, so no ref-counting is required here. */ - struct aws_mqtt_request_response_client *client; + struct aws_mqtt_request_response_client *client; // TODO Remove /* * Map from cursor (topic filter) -> list of streaming operations using that filter @@ -68,8 +68,10 @@ struct aws_rr_response_path_entry { }; typedef void(aws_mqtt_stream_operation_subscription_match_fn)( - struct aws_rr_operation_list_topic_filter_entry *entry, - const struct aws_protocol_adapter_incoming_publish_event *publish_event); + const struct aws_linked_list *operations, + const struct aws_byte_cursor *topic_filter, // TODO Do we need this for anything other than tests? + const struct aws_protocol_adapter_incoming_publish_event *publish_event, + void *user_data); typedef void(aws_mqtt_request_operation_subscription_match_fn)( struct aws_mqtt_request_response_client *rr_client, @@ -103,7 +105,8 @@ AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_match( const struct aws_request_response_subscriptions *subscriptions, const struct aws_protocol_adapter_incoming_publish_event *publish_event, aws_mqtt_stream_operation_subscription_match_fn *on_stream_operation_subscription_match, - aws_mqtt_request_operation_subscription_match_fn *on_request_operation_subscription_match); + aws_mqtt_request_operation_subscription_match_fn *on_request_operation_subscription_match, + void *user_data); AWS_EXTERN_C_END diff --git a/source/request-response/request_response_client.c b/source/request-response/request_response_client.c index 26c60df9..371b71b7 100644 --- a/source/request-response/request_response_client.c +++ b/source/request-response/request_response_client.c @@ -778,12 +778,14 @@ static void s_aws_rr_client_protocol_adapter_subscription_event_callback( } static void s_apply_publish_to_streaming_operation_list( - struct aws_rr_operation_list_topic_filter_entry *entry, - const struct aws_protocol_adapter_incoming_publish_event *publish_event) { - AWS_FATAL_ASSERT(entry != NULL); + const struct aws_linked_list *operations, + const struct aws_byte_cursor *topic_filter, + const struct aws_protocol_adapter_incoming_publish_event *publish_event, + void *user_data) { + AWS_FATAL_ASSERT(operations != NULL); - struct aws_linked_list_node *node = aws_linked_list_begin(&entry->operations); - while (node != aws_linked_list_end(&entry->operations)) { + struct aws_linked_list_node *node = aws_linked_list_begin(operations); + while (node != aws_linked_list_end(operations)) { struct aws_mqtt_rr_client_operation *operation = AWS_CONTAINER_OF(node, struct aws_mqtt_rr_client_operation, node); node = aws_linked_list_next(node); @@ -964,7 +966,8 @@ static void s_aws_rr_client_protocol_adapter_incoming_publish_callback( &rr_client->subscriptions, publish_event, s_apply_publish_to_streaming_operation_list, - s_apply_publish_to_response_path_entry); + s_apply_publish_to_response_path_entry, + NULL); } static void s_aws_rr_client_protocol_adapter_terminate_callback(void *user_data) { diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index 91f7a2c6..6d97e21d 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -224,7 +224,8 @@ void aws_mqtt_request_response_client_subscriptions_remove_request_subscription( static void s_match_wildcard_stream_subscriptions( const struct aws_hash_table *subscriptions, const struct aws_protocol_adapter_incoming_publish_event *publish_event, - aws_mqtt_stream_operation_subscription_match_fn *on_stream_operation_subscription_match) { + aws_mqtt_stream_operation_subscription_match_fn *on_stream_operation_subscription_match, + void *user_data) { AWS_LOGF_INFO( AWS_LS_MQTT_REQUEST_RESPONSE, @@ -279,7 +280,8 @@ static void s_match_wildcard_stream_subscriptions( if (match) { AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== found subscription match"); - on_stream_operation_subscription_match(iter.element.value, publish_event); + on_stream_operation_subscription_match( + &entry->operations, &entry->topic_filter_cursor, publish_event, user_data); } else { AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== this is not the right subscription"); } @@ -290,7 +292,12 @@ void aws_mqtt_request_response_client_subscriptions_match( const struct aws_request_response_subscriptions *subscriptions, const struct aws_protocol_adapter_incoming_publish_event *publish_event, aws_mqtt_stream_operation_subscription_match_fn *on_stream_operation_subscription_match, - aws_mqtt_request_operation_subscription_match_fn *on_request_operation_subscription_match) { + aws_mqtt_request_operation_subscription_match_fn *on_request_operation_subscription_match, + void *user_data) { + + AWS_FATAL_PRECONDITION(publish_event); + AWS_FATAL_PRECONDITION(on_stream_operation_subscription_match); + AWS_FATAL_PRECONDITION(on_request_operation_subscription_match); /* Streaming operation handling */ struct aws_hash_element *subscription_filter_element = NULL; @@ -305,13 +312,16 @@ void aws_mqtt_request_response_client_subscriptions_match( (void *)subscriptions->client, AWS_BYTE_CURSOR_PRI(publish_event->topic)); - on_stream_operation_subscription_match(subscription_filter_element->value, publish_event); + struct aws_rr_operation_list_topic_filter_entry *entry = subscription_filter_element->value; + on_stream_operation_subscription_match( + &entry->operations, &entry->topic_filter_cursor, publish_event, user_data); } s_match_wildcard_stream_subscriptions( &subscriptions->streaming_operation_wildcards_subscription_lists, publish_event, - on_stream_operation_subscription_match); + on_stream_operation_subscription_match, + user_data); /* Request-Response handling */ struct aws_hash_element *response_path_element = NULL; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 67aeb1e7..2f8b2a85 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -577,6 +577,8 @@ add_test_case(rrc_request_response_failure_invalid_correlation_token_type) add_test_case(rrc_request_response_failure_non_matching_correlation_token) add_test_case(rrc_request_response_multi_operation_sequence) +add_test_case(rrs_match_subscription) + generate_test_driver(${PROJECT_NAME}-tests) set(TEST_PAHO_CLIENT_BINARY_NAME ${PROJECT_NAME}-paho-client) diff --git a/tests/request-response/request_response_client_tests.c b/tests/request-response/request_response_client_tests.c index 1e80aaba..bcdd5569 100644 --- a/tests/request-response/request_response_client_tests.c +++ b/tests/request-response/request_response_client_tests.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -2060,8 +2061,7 @@ static int s_rrc_streaming_operation_failure_exceeds_subscription_budget_fn( // make a third using topic filter 2 struct aws_byte_cursor record_key3 = aws_byte_cursor_from_c_str("key3"); - struct aws_mqtt_rr_client_operation *operation3 = - s_create_streaming_operation(&fixture, record_key3, topic2); + struct aws_mqtt_rr_client_operation *operation3 = s_create_streaming_operation(&fixture, record_key3, topic2); s_rrc_wait_for_n_streaming_subscription_events(&fixture, record_key3, 1); ASSERT_SUCCESS(s_rrc_verify_streaming_record_subscription_events( @@ -3240,3 +3240,68 @@ static int s_rrc_request_response_multi_operation_sequence_fn(struct aws_allocat } AWS_TEST_CASE(rrc_request_response_multi_operation_sequence, s_rrc_request_response_multi_operation_sequence_fn) + +static void s_rrs_fixture_on_stream_operation_subscription_match( + const struct aws_linked_list *operations, + const struct aws_byte_cursor *topic_filter, + const struct aws_protocol_adapter_incoming_publish_event *publish_event, + void *user_data) { + fprintf( + stderr, + "====== on stream called: topic %.*s; topic filter %.*s\n", + AWS_BYTE_CURSOR_PRI(publish_event->topic), + AWS_BYTE_CURSOR_PRI(*topic_filter)); +} + +static void s_rrs_fixture_on_request_operation_subscription_match( + struct aws_mqtt_request_response_client *rr_client, + struct aws_rr_response_path_entry *entry, + const struct aws_protocol_adapter_incoming_publish_event *publish_event) { + fprintf(stderr, "====== on req called\n"); +} + +static int s_rrs_match_subscription_fn(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + void *client = (void *)0x08; + + struct aws_request_response_subscriptions subscriptions; + + aws_mqtt_request_response_client_subscriptions_init(&subscriptions, client, allocator); + + struct aws_byte_cursor topic_filter1 = aws_byte_cursor_from_c_str("topic/123/+"); + struct aws_byte_cursor topic_filter2 = aws_byte_cursor_from_c_str("topic/+/abc"); + struct aws_byte_cursor topic1 = aws_byte_cursor_from_c_str("topic/123/abc"); + struct aws_byte_cursor payload1 = aws_byte_cursor_from_c_str("Payload1"); + + aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter1); + aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter2); + + struct aws_protocol_adapter_incoming_publish_event publish_event = { + .topic = topic1, + .payload = payload1, + }; + aws_mqtt_request_response_client_subscriptions_match( + &subscriptions, + &publish_event, + s_rrs_fixture_on_stream_operation_subscription_match, + s_rrs_fixture_on_request_operation_subscription_match, + NULL); + + struct aws_rr_client_fixture_publish_message_view expected_publishes[] = { + { + payload1, + topic1, + }, + { + payload1, + topic1, + }, + }; + + aws_mqtt_request_response_client_subscriptions_cleanup(&subscriptions); + + return AWS_OP_SUCCESS; +} + +AWS_TEST_CASE(rrs_match_subscription, s_rrs_match_subscription_fn) From 26b9d2e77dd2b38a51eb0cd2bafff7069acc1fcc Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Tue, 21 Jan 2025 10:12:27 -0800 Subject: [PATCH 11/29] Tests fixup --- .../request_response_client_tests.c | 141 ++++++++++++++++-- 1 file changed, 127 insertions(+), 14 deletions(-) diff --git a/tests/request-response/request_response_client_tests.c b/tests/request-response/request_response_client_tests.c index bcdd5569..1a1c1099 100644 --- a/tests/request-response/request_response_client_tests.c +++ b/tests/request-response/request_response_client_tests.c @@ -2026,7 +2026,7 @@ static int s_rrc_streaming_operation_failure_exceeds_subscription_budget_fn( }, }; ASSERT_SUCCESS(s_rrc_verify_streaming_record_subscription_events( - &fixture, record_key2, AWS_ARRAY_SIZE(expected_success_events), expected_success_events)); + &fixture, record_key2, AWS_ARRAY_SIZE(expected_success_events), expected_failure_events)); // two publishes on the mqtt client that get reflected into our subscription topic1 struct aws_byte_cursor payload1 = aws_byte_cursor_from_c_str("Payload1"); @@ -3241,16 +3241,115 @@ static int s_rrc_request_response_multi_operation_sequence_fn(struct aws_allocat AWS_TEST_CASE(rrc_request_response_multi_operation_sequence, s_rrc_request_response_multi_operation_sequence_fn) +struct aws_rr_client_fixture_matched_subscription { + struct aws_byte_buf payload; + struct aws_byte_buf topic; + struct aws_byte_buf topic_filter; +}; + +struct aws_rr_client_fixture_matched_subscription_view { + struct aws_byte_cursor payload; + struct aws_byte_cursor topic; + struct aws_byte_cursor topic_filter; +}; + +struct aws_rr_client_fixture_streaming_subscriptions_record { + struct aws_allocator *allocator; + // TODO hash map: topic_filter -> (list?){publish message} + struct aws_hash_table matches; + struct aws_array_list publishes; +}; + +struct aws_rr_client_fixture_streaming_subscriptions_record *s_aws_rr_client_fixture_streaming_subscriptions_record_new( + struct aws_allocator *allocator) { + struct aws_rr_client_fixture_streaming_subscriptions_record *record = + aws_mem_calloc(allocator, 1, sizeof(struct aws_rr_client_fixture_streaming_subscriptions_record)); + + record->allocator = allocator; + aws_array_list_init_dynamic( + &record->publishes, allocator, 10, sizeof(struct aws_rr_client_fixture_matched_subscription)); + + return record; +} + +void s_aws_rr_client_fixture_streaming_subscriptions_record_delete( + struct aws_rr_client_fixture_streaming_subscriptions_record *record) { + + size_t publish_count = aws_array_list_length(&record->publishes); + for (size_t i = 0; i < publish_count; ++i) { + struct aws_rr_client_fixture_matched_subscription matched_subscription; + aws_array_list_get_at(&record->publishes, &matched_subscription, i); + + aws_byte_buf_clean_up(&matched_subscription.payload); + aws_byte_buf_clean_up(&matched_subscription.topic); + aws_byte_buf_clean_up(&matched_subscription.topic_filter); + } + + aws_array_list_clean_up(&record->publishes); + + aws_mem_release(record->allocator, record); +} + +static int s_rrc_verify_streaming_subscriptions_publishes( + struct aws_rr_client_fixture_streaming_subscriptions_record *record, + size_t expected_publish_count, + struct aws_rr_client_fixture_matched_subscription_view *expected_matched_subscriptions) { + + size_t actual_publish_count = aws_array_list_length(&record->publishes); + ASSERT_INT_EQUALS(expected_publish_count, actual_publish_count); + + for (size_t i = 0; i < actual_publish_count; ++i) { + struct aws_rr_client_fixture_matched_subscription actual_matched_subscription; + aws_array_list_get_at(&record->publishes, &actual_matched_subscription, i); + + fprintf(stderr, "================================= %lu\n", actual_matched_subscription.topic_filter.len); + + struct aws_rr_client_fixture_matched_subscription_view *matched_subscription = + &expected_matched_subscriptions[i]; + + ASSERT_BIN_ARRAYS_EQUALS( + matched_subscription->payload.ptr, + matched_subscription->payload.len, + actual_matched_subscription.payload.buffer, + actual_matched_subscription.payload.len); + ASSERT_BIN_ARRAYS_EQUALS( + matched_subscription->topic.ptr, + matched_subscription->topic.len, + actual_matched_subscription.topic.buffer, + actual_matched_subscription.topic.len); + ASSERT_BIN_ARRAYS_EQUALS( + matched_subscription->topic_filter.ptr, + matched_subscription->topic_filter.len, + actual_matched_subscription.topic_filter.buffer, + actual_matched_subscription.topic_filter.len); + } + + return AWS_OP_SUCCESS; +} + static void s_rrs_fixture_on_stream_operation_subscription_match( const struct aws_linked_list *operations, const struct aws_byte_cursor *topic_filter, const struct aws_protocol_adapter_incoming_publish_event *publish_event, void *user_data) { + + (void)operations; + + struct aws_rr_client_fixture_streaming_subscriptions_record *fixture = user_data; + + struct aws_rr_client_fixture_matched_subscription matched_subscription; + aws_byte_buf_init_copy_from_cursor(&matched_subscription.payload, fixture->allocator, publish_event->payload); + aws_byte_buf_init_copy_from_cursor(&matched_subscription.topic, fixture->allocator, publish_event->topic); + aws_byte_buf_init_copy_from_cursor(&matched_subscription.topic_filter, fixture->allocator, *topic_filter); + + aws_array_list_push_back(&fixture->publishes, &matched_subscription); + fprintf( stderr, - "====== on stream called: topic %.*s; topic filter %.*s\n", - AWS_BYTE_CURSOR_PRI(publish_event->topic), - AWS_BYTE_CURSOR_PRI(*topic_filter)); + "====== on stream called: topic %.*s; payload %.*s; topic filter %.*s\n", + AWS_BYTE_BUF_PRI(matched_subscription.topic), + AWS_BYTE_BUF_PRI(matched_subscription.payload), + AWS_BYTE_BUF_PRI(matched_subscription.topic_filter)); } static void s_rrs_fixture_on_request_operation_subscription_match( @@ -3274,6 +3373,8 @@ static int s_rrs_match_subscription_fn(struct aws_allocator *allocator, void *ct struct aws_byte_cursor topic1 = aws_byte_cursor_from_c_str("topic/123/abc"); struct aws_byte_cursor payload1 = aws_byte_cursor_from_c_str("Payload1"); + aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter1); + // TODO This does nothing. Add test case for this. aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter1); aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter2); @@ -3281,24 +3382,36 @@ static int s_rrs_match_subscription_fn(struct aws_allocator *allocator, void *ct .topic = topic1, .payload = payload1, }; + + struct aws_rr_client_fixture_streaming_subscriptions_record *record = + s_aws_rr_client_fixture_streaming_subscriptions_record_new(allocator); + aws_mqtt_request_response_client_subscriptions_match( &subscriptions, &publish_event, s_rrs_fixture_on_stream_operation_subscription_match, s_rrs_fixture_on_request_operation_subscription_match, - NULL); + record); - struct aws_rr_client_fixture_publish_message_view expected_publishes[] = { - { - payload1, - topic1, - }, - { - payload1, - topic1, - }, + struct aws_rr_client_fixture_matched_subscription_view matched_subscriptions[] = { + {payload1, topic1, topic_filter2}, + {payload1, topic1, topic_filter1}, }; + fprintf( + stderr, + "=============== on stream called: topic filter %.*s\n", + AWS_BYTE_CURSOR_PRI(matched_subscriptions[0].topic_filter)); + fprintf( + stderr, + "=============== on stream called: topic filter %.*s\n", + AWS_BYTE_CURSOR_PRI(matched_subscriptions[1].topic_filter)); + + s_rrc_verify_streaming_subscriptions_publishes( + record, AWS_ARRAY_SIZE(matched_subscriptions), matched_subscriptions); + + s_aws_rr_client_fixture_streaming_subscriptions_record_delete(record); + aws_mqtt_request_response_client_subscriptions_cleanup(&subscriptions); return AWS_OP_SUCCESS; From ec659f1f837e2b2ae776f91d92ddd059394d99f8 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Tue, 21 Jan 2025 14:43:03 -0800 Subject: [PATCH 12/29] test fixup --- .../request_response_client_tests.c | 111 +++++++++--------- 1 file changed, 54 insertions(+), 57 deletions(-) diff --git a/tests/request-response/request_response_client_tests.c b/tests/request-response/request_response_client_tests.c index 1a1c1099..3334a7fe 100644 --- a/tests/request-response/request_response_client_tests.c +++ b/tests/request-response/request_response_client_tests.c @@ -3242,6 +3242,7 @@ static int s_rrc_request_response_multi_operation_sequence_fn(struct aws_allocat AWS_TEST_CASE(rrc_request_response_multi_operation_sequence, s_rrc_request_response_multi_operation_sequence_fn) struct aws_rr_client_fixture_matched_subscription { + struct aws_allocator *allocator; struct aws_byte_buf payload; struct aws_byte_buf topic; struct aws_byte_buf topic_filter; @@ -3255,38 +3256,41 @@ struct aws_rr_client_fixture_matched_subscription_view { struct aws_rr_client_fixture_streaming_subscriptions_record { struct aws_allocator *allocator; - // TODO hash map: topic_filter -> (list?){publish message} + // table: topic_filter -> aws_rr_client_fixture_matched_subscription struct aws_hash_table matches; - struct aws_array_list publishes; + size_t matches_count; }; +static void s_aws_rr_client_fixture_streaming_subscription_destroy(void *value) { + struct aws_rr_client_fixture_matched_subscription *matched_subscription = value; + aws_byte_buf_clean_up(&matched_subscription->payload); + aws_byte_buf_clean_up(&matched_subscription->topic); + aws_byte_buf_clean_up(&matched_subscription->topic_filter); + aws_mem_release(matched_subscription->allocator, matched_subscription); +} + struct aws_rr_client_fixture_streaming_subscriptions_record *s_aws_rr_client_fixture_streaming_subscriptions_record_new( struct aws_allocator *allocator) { struct aws_rr_client_fixture_streaming_subscriptions_record *record = aws_mem_calloc(allocator, 1, sizeof(struct aws_rr_client_fixture_streaming_subscriptions_record)); record->allocator = allocator; - aws_array_list_init_dynamic( - &record->publishes, allocator, 10, sizeof(struct aws_rr_client_fixture_matched_subscription)); + aws_hash_table_init( + &record->matches, + allocator, + 10, + aws_hash_byte_cursor_ptr, + aws_mqtt_byte_cursor_hash_equality, + NULL, + s_aws_rr_client_fixture_streaming_subscription_destroy); + record->matches_count = 0; return record; } void s_aws_rr_client_fixture_streaming_subscriptions_record_delete( struct aws_rr_client_fixture_streaming_subscriptions_record *record) { - - size_t publish_count = aws_array_list_length(&record->publishes); - for (size_t i = 0; i < publish_count; ++i) { - struct aws_rr_client_fixture_matched_subscription matched_subscription; - aws_array_list_get_at(&record->publishes, &matched_subscription, i); - - aws_byte_buf_clean_up(&matched_subscription.payload); - aws_byte_buf_clean_up(&matched_subscription.topic); - aws_byte_buf_clean_up(&matched_subscription.topic_filter); - } - - aws_array_list_clean_up(&record->publishes); - + aws_hash_table_clean_up(&record->matches); aws_mem_release(record->allocator, record); } @@ -3295,33 +3299,32 @@ static int s_rrc_verify_streaming_subscriptions_publishes( size_t expected_publish_count, struct aws_rr_client_fixture_matched_subscription_view *expected_matched_subscriptions) { - size_t actual_publish_count = aws_array_list_length(&record->publishes); - ASSERT_INT_EQUALS(expected_publish_count, actual_publish_count); + ASSERT_INT_EQUALS(expected_publish_count, record->matches_count); - for (size_t i = 0; i < actual_publish_count; ++i) { - struct aws_rr_client_fixture_matched_subscription actual_matched_subscription; - aws_array_list_get_at(&record->publishes, &actual_matched_subscription, i); + for (size_t i = 0; i < expected_publish_count; ++i) { + struct aws_rr_client_fixture_matched_subscription_view *expected_matched_subscription = + &expected_matched_subscriptions[i]; - fprintf(stderr, "================================= %lu\n", actual_matched_subscription.topic_filter.len); + struct aws_hash_element *element = NULL; + ASSERT_SUCCESS(aws_hash_table_find(&record->matches, &expected_matched_subscription->topic_filter, &element)); - struct aws_rr_client_fixture_matched_subscription_view *matched_subscription = - &expected_matched_subscriptions[i]; + struct aws_rr_client_fixture_matched_subscription *actual_matched_subscription = element->value; ASSERT_BIN_ARRAYS_EQUALS( - matched_subscription->payload.ptr, - matched_subscription->payload.len, - actual_matched_subscription.payload.buffer, - actual_matched_subscription.payload.len); + expected_matched_subscription->payload.ptr, + expected_matched_subscription->payload.len, + actual_matched_subscription->payload.buffer, + actual_matched_subscription->payload.len); ASSERT_BIN_ARRAYS_EQUALS( - matched_subscription->topic.ptr, - matched_subscription->topic.len, - actual_matched_subscription.topic.buffer, - actual_matched_subscription.topic.len); + expected_matched_subscription->topic.ptr, + expected_matched_subscription->topic.len, + actual_matched_subscription->topic.buffer, + actual_matched_subscription->topic.len); ASSERT_BIN_ARRAYS_EQUALS( - matched_subscription->topic_filter.ptr, - matched_subscription->topic_filter.len, - actual_matched_subscription.topic_filter.buffer, - actual_matched_subscription.topic_filter.len); + expected_matched_subscription->topic_filter.ptr, + expected_matched_subscription->topic_filter.len, + actual_matched_subscription->topic_filter.buffer, + actual_matched_subscription->topic_filter.len); } return AWS_OP_SUCCESS; @@ -3335,21 +3338,24 @@ static void s_rrs_fixture_on_stream_operation_subscription_match( (void)operations; - struct aws_rr_client_fixture_streaming_subscriptions_record *fixture = user_data; + struct aws_rr_client_fixture_streaming_subscriptions_record *record = user_data; - struct aws_rr_client_fixture_matched_subscription matched_subscription; - aws_byte_buf_init_copy_from_cursor(&matched_subscription.payload, fixture->allocator, publish_event->payload); - aws_byte_buf_init_copy_from_cursor(&matched_subscription.topic, fixture->allocator, publish_event->topic); - aws_byte_buf_init_copy_from_cursor(&matched_subscription.topic_filter, fixture->allocator, *topic_filter); + struct aws_rr_client_fixture_matched_subscription *matched_subscription = + aws_mem_calloc(record->allocator, 1, sizeof(struct aws_rr_client_fixture_matched_subscription)); + matched_subscription->allocator = record->allocator; + aws_byte_buf_init_copy_from_cursor(&matched_subscription->payload, record->allocator, publish_event->payload); + aws_byte_buf_init_copy_from_cursor(&matched_subscription->topic, record->allocator, publish_event->topic); + aws_byte_buf_init_copy_from_cursor(&matched_subscription->topic_filter, record->allocator, *topic_filter); - aws_array_list_push_back(&fixture->publishes, &matched_subscription); + aws_hash_table_put(&record->matches, topic_filter, matched_subscription, NULL); + ++record->matches_count; fprintf( stderr, "====== on stream called: topic %.*s; payload %.*s; topic filter %.*s\n", - AWS_BYTE_BUF_PRI(matched_subscription.topic), - AWS_BYTE_BUF_PRI(matched_subscription.payload), - AWS_BYTE_BUF_PRI(matched_subscription.topic_filter)); + AWS_BYTE_BUF_PRI(matched_subscription->topic), + AWS_BYTE_BUF_PRI(matched_subscription->payload), + AWS_BYTE_BUF_PRI(matched_subscription->topic_filter)); } static void s_rrs_fixture_on_request_operation_subscription_match( @@ -3398,17 +3404,8 @@ static int s_rrs_match_subscription_fn(struct aws_allocator *allocator, void *ct {payload1, topic1, topic_filter1}, }; - fprintf( - stderr, - "=============== on stream called: topic filter %.*s\n", - AWS_BYTE_CURSOR_PRI(matched_subscriptions[0].topic_filter)); - fprintf( - stderr, - "=============== on stream called: topic filter %.*s\n", - AWS_BYTE_CURSOR_PRI(matched_subscriptions[1].topic_filter)); - - s_rrc_verify_streaming_subscriptions_publishes( - record, AWS_ARRAY_SIZE(matched_subscriptions), matched_subscriptions); + ASSERT_SUCCESS(s_rrc_verify_streaming_subscriptions_publishes( + record, AWS_ARRAY_SIZE(matched_subscriptions), matched_subscriptions)); s_aws_rr_client_fixture_streaming_subscriptions_record_delete(record); From 49fde1f6187b9380ec62831b3d872fd4ff12db1e Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Tue, 21 Jan 2025 15:04:55 -0800 Subject: [PATCH 13/29] Support multi level wildcards --- .../request_response_subscription_set.c | 9 ++- tests/CMakeLists.txt | 3 +- .../request_response_client_tests.c | 67 +++++++++++++++++-- 3 files changed, 73 insertions(+), 6 deletions(-) diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index 6d97e21d..63d5c250 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -247,6 +247,7 @@ static void s_match_wildcard_stream_subscriptions( AWS_ZERO_STRUCT(topic_segment); bool match = true; + bool multi_level_wildcard = false; while (aws_byte_cursor_next_split(&entry->topic_filter_cursor, '/', &subscription_topic_filter_segment)) { AWS_LOGF_INFO( @@ -265,6 +266,12 @@ static void s_match_wildcard_stream_subscriptions( "======= topic segment is '" PRInSTR "'", AWS_BYTE_CURSOR_PRI(topic_segment)); + if (aws_byte_cursor_eq_c_str(&subscription_topic_filter_segment, "#")) { + multi_level_wildcard = true; + match = true; + break; + } + if (!aws_byte_cursor_eq_c_str(&subscription_topic_filter_segment, "+") && !aws_byte_cursor_eq_ignore_case(&topic_segment, &subscription_topic_filter_segment)) { AWS_LOGF_INFO( @@ -274,7 +281,7 @@ static void s_match_wildcard_stream_subscriptions( } } - if (aws_byte_cursor_next_split(&publish_event->topic, '/', &topic_segment)) { + if (!multi_level_wildcard && aws_byte_cursor_next_split(&publish_event->topic, '/', &topic_segment)) { match = false; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2f8b2a85..b6f36cc2 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -577,7 +577,8 @@ add_test_case(rrc_request_response_failure_invalid_correlation_token_type) add_test_case(rrc_request_response_failure_non_matching_correlation_token) add_test_case(rrc_request_response_multi_operation_sequence) -add_test_case(rrs_match_subscription) +add_test_case(rrs_match_subscription_with_single_level_wildcards) +add_test_case(rrs_match_subscription_with_multi_level_wildcards) generate_test_driver(${PROJECT_NAME}-tests) diff --git a/tests/request-response/request_response_client_tests.c b/tests/request-response/request_response_client_tests.c index 3334a7fe..054b4574 100644 --- a/tests/request-response/request_response_client_tests.c +++ b/tests/request-response/request_response_client_tests.c @@ -3365,7 +3365,7 @@ static void s_rrs_fixture_on_request_operation_subscription_match( fprintf(stderr, "====== on req called\n"); } -static int s_rrs_match_subscription_fn(struct aws_allocator *allocator, void *ctx) { +static int s_rrs_match_subscription_with_single_level_wildcards_fn(struct aws_allocator *allocator, void *ctx) { (void)ctx; void *client = (void *)0x08; @@ -3374,8 +3374,10 @@ static int s_rrs_match_subscription_fn(struct aws_allocator *allocator, void *ct aws_mqtt_request_response_client_subscriptions_init(&subscriptions, client, allocator); - struct aws_byte_cursor topic_filter1 = aws_byte_cursor_from_c_str("topic/123/+"); - struct aws_byte_cursor topic_filter2 = aws_byte_cursor_from_c_str("topic/+/abc"); + struct aws_byte_cursor topic_filter1 = aws_byte_cursor_from_c_str("topic/123/abc"); + struct aws_byte_cursor topic_filter2 = aws_byte_cursor_from_c_str("topic/123/+"); + struct aws_byte_cursor topic_filter3 = aws_byte_cursor_from_c_str("topic/+/abc"); + struct aws_byte_cursor topic1 = aws_byte_cursor_from_c_str("topic/123/abc"); struct aws_byte_cursor payload1 = aws_byte_cursor_from_c_str("Payload1"); @@ -3383,6 +3385,7 @@ static int s_rrs_match_subscription_fn(struct aws_allocator *allocator, void *ct // TODO This does nothing. Add test case for this. aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter1); aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter2); + aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter3); struct aws_protocol_adapter_incoming_publish_event publish_event = { .topic = topic1, @@ -3400,8 +3403,64 @@ static int s_rrs_match_subscription_fn(struct aws_allocator *allocator, void *ct record); struct aws_rr_client_fixture_matched_subscription_view matched_subscriptions[] = { + {payload1, topic1, topic_filter1}, {payload1, topic1, topic_filter2}, + {payload1, topic1, topic_filter3}, + }; + + ASSERT_SUCCESS(s_rrc_verify_streaming_subscriptions_publishes( + record, AWS_ARRAY_SIZE(matched_subscriptions), matched_subscriptions)); + + s_aws_rr_client_fixture_streaming_subscriptions_record_delete(record); + + aws_mqtt_request_response_client_subscriptions_cleanup(&subscriptions); + + return AWS_OP_SUCCESS; +} + +AWS_TEST_CASE( + rrs_match_subscription_with_single_level_wildcards, + s_rrs_match_subscription_with_single_level_wildcards_fn) + +static int s_rrs_match_subscription_with_multi_level_wildcards_fn(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + void *client = (void *)0x08; + + struct aws_request_response_subscriptions subscriptions; + + aws_mqtt_request_response_client_subscriptions_init(&subscriptions, client, allocator); + + struct aws_byte_cursor topic_filter1 = aws_byte_cursor_from_c_str("topic/123/abc"); + struct aws_byte_cursor topic_filter2 = aws_byte_cursor_from_c_str("topic/123/#"); + struct aws_byte_cursor topic_filter3 = aws_byte_cursor_from_c_str("topic/#"); + + struct aws_byte_cursor topic1 = aws_byte_cursor_from_c_str("topic/123/abc"); + struct aws_byte_cursor payload1 = aws_byte_cursor_from_c_str("Payload1"); + + aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter1); + aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter2); + aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter3); + + struct aws_protocol_adapter_incoming_publish_event publish_event = { + .topic = topic1, + .payload = payload1, + }; + + struct aws_rr_client_fixture_streaming_subscriptions_record *record = + s_aws_rr_client_fixture_streaming_subscriptions_record_new(allocator); + + aws_mqtt_request_response_client_subscriptions_match( + &subscriptions, + &publish_event, + s_rrs_fixture_on_stream_operation_subscription_match, + s_rrs_fixture_on_request_operation_subscription_match, + record); + + struct aws_rr_client_fixture_matched_subscription_view matched_subscriptions[] = { {payload1, topic1, topic_filter1}, + {payload1, topic1, topic_filter2}, + {payload1, topic1, topic_filter3}, }; ASSERT_SUCCESS(s_rrc_verify_streaming_subscriptions_publishes( @@ -3414,4 +3473,4 @@ static int s_rrs_match_subscription_fn(struct aws_allocator *allocator, void *ct return AWS_OP_SUCCESS; } -AWS_TEST_CASE(rrs_match_subscription, s_rrs_match_subscription_fn) +AWS_TEST_CASE(rrs_match_subscription_with_multi_level_wildcards, s_rrs_match_subscription_with_multi_level_wildcards_fn) From 9da0526fb44c99c84c33eca2d145575c4d38986f Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Wed, 29 Jan 2025 13:27:00 -0800 Subject: [PATCH 14/29] temp --- .../request_response_subscription_set.h | 5 ----- .../request_response_subscription_set.c | 11 ++--------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/include/aws/mqtt/private/request-response/request_response_subscription_set.h b/include/aws/mqtt/private/request-response/request_response_subscription_set.h index bf9cf2a9..50fe3856 100644 --- a/include/aws/mqtt/private/request-response/request_response_subscription_set.h +++ b/include/aws/mqtt/private/request-response/request_response_subscription_set.h @@ -18,10 +18,6 @@ struct aws_mqtt_request_response_client; struct aws_request_response_subscriptions { struct aws_allocator *allocator; - /* Convenient access to request-response client instance. Lifetime of aws_request_response_subscriptions is bound to - * client, so no ref-counting is required here. */ - struct aws_mqtt_request_response_client *client; // TODO Remove - /* * Map from cursor (topic filter) -> list of streaming operations using that filter */ @@ -82,7 +78,6 @@ AWS_EXTERN_C_BEGIN AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_init( struct aws_request_response_subscriptions *subscriptions, - struct aws_mqtt_request_response_client *client, struct aws_allocator *allocator); AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_cleanup( diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index 63d5c250..4a0279d8 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -5,7 +5,6 @@ #include -#include #include #include #include @@ -75,11 +74,11 @@ static void s_aws_rr_response_path_table_hash_element_destroy(void *value) { void aws_mqtt_request_response_client_subscriptions_init( struct aws_request_response_subscriptions *subscriptions, - struct aws_mqtt_request_response_client *client, struct aws_allocator *allocator) { + AWS_FATAL_ASSERT(subscriptions); + subscriptions->allocator = allocator; - subscriptions->client = client; aws_hash_table_init( &subscriptions->streaming_operation_subscription_lists, @@ -136,12 +135,6 @@ struct aws_rr_operation_list_topic_filter_entry *aws_mqtt_request_response_clien if (element == NULL) { entry = s_aws_rr_operation_list_topic_filter_entry_new(subscriptions->allocator, *topic_filter); aws_hash_table_put(subscription_lists, &entry->topic_filter_cursor, entry, NULL); - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "id=%p: request-response client adding wildcard topic filter '" PRInSTR - "' to streaming subscriptions table", - (void *)subscriptions->client, - AWS_BYTE_CURSOR_PRI(*topic_filter)); } else { entry = element->value; } From 4ade5cd365006faba109091dd4f49ee7cb539e9a Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Thu, 30 Jan 2025 14:59:06 -0800 Subject: [PATCH 15/29] Remove client from subscription module --- .../request_response_subscription_set.h | 4 ++-- .../request_response_client.c | 20 +++++++++++++------ .../request_response_subscription_set.c | 19 +++++++----------- .../request_response_client_tests.c | 12 ++++------- 4 files changed, 27 insertions(+), 28 deletions(-) diff --git a/include/aws/mqtt/private/request-response/request_response_subscription_set.h b/include/aws/mqtt/private/request-response/request_response_subscription_set.h index 50fe3856..3cd77c24 100644 --- a/include/aws/mqtt/private/request-response/request_response_subscription_set.h +++ b/include/aws/mqtt/private/request-response/request_response_subscription_set.h @@ -70,9 +70,9 @@ typedef void(aws_mqtt_stream_operation_subscription_match_fn)( void *user_data); typedef void(aws_mqtt_request_operation_subscription_match_fn)( - struct aws_mqtt_request_response_client *rr_client, struct aws_rr_response_path_entry *entry, - const struct aws_protocol_adapter_incoming_publish_event *publish_event); + const struct aws_protocol_adapter_incoming_publish_event *publish_event, + void *user_data); AWS_EXTERN_C_BEGIN diff --git a/source/request-response/request_response_client.c b/source/request-response/request_response_client.c index 371b71b7..2b3ebf6d 100644 --- a/source/request-response/request_response_client.c +++ b/source/request-response/request_response_client.c @@ -264,6 +264,10 @@ struct aws_mqtt_request_response_client { */ struct aws_priority_queue operations_by_timeout; + /* + * Structure to handle subscriptions: add/remove subscriptions, match incoming messages. + * TODO Add normal description. + */ struct aws_request_response_subscriptions subscriptions; /* @@ -782,6 +786,8 @@ static void s_apply_publish_to_streaming_operation_list( const struct aws_byte_cursor *topic_filter, const struct aws_protocol_adapter_incoming_publish_event *publish_event, void *user_data) { + (void)user_data; + AWS_FATAL_ASSERT(operations != NULL); struct aws_linked_list_node *node = aws_linked_list_begin(operations); @@ -804,8 +810,8 @@ static void s_apply_publish_to_streaming_operation_list( continue; } - void *user_data = operation->storage.streaming_storage.options.user_data; - (*incoming_publish_callback)(publish_event->payload, publish_event->topic, user_data); + void *operation_user_data = operation->storage.streaming_storage.options.user_data; + (*incoming_publish_callback)(publish_event->payload, publish_event->topic, operation_user_data); AWS_LOGF_DEBUG( AWS_LS_MQTT_REQUEST_RESPONSE, @@ -870,9 +876,11 @@ static void s_complete_operation_with_correlation_token( } static void s_apply_publish_to_response_path_entry( - struct aws_mqtt_request_response_client *rr_client, struct aws_rr_response_path_entry *entry, - const struct aws_protocol_adapter_incoming_publish_event *publish_event) { + const struct aws_protocol_adapter_incoming_publish_event *publish_event, + void *user_data) { + + struct aws_mqtt_request_response_client *rr_client = user_data; struct aws_json_value *json_payload = NULL; @@ -967,7 +975,7 @@ static void s_aws_rr_client_protocol_adapter_incoming_publish_callback( publish_event, s_apply_publish_to_streaming_operation_list, s_apply_publish_to_response_path_entry, - NULL); + rr_client); } static void s_aws_rr_client_protocol_adapter_terminate_callback(void *user_data) { @@ -1057,7 +1065,7 @@ static struct aws_mqtt_request_response_client *s_aws_mqtt_request_response_clie sizeof(struct aws_mqtt_rr_client_operation *), s_compare_rr_operation_timeouts); - aws_mqtt_request_response_client_subscriptions_init(&rr_client->subscriptions, rr_client, allocator); + aws_mqtt_request_response_client_subscriptions_init(&rr_client->subscriptions, allocator); aws_hash_table_init( &rr_client->operations_by_correlation_tokens, diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index 4a0279d8..6223d1fe 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -187,8 +187,7 @@ void aws_mqtt_request_response_client_subscriptions_remove_request_subscription( if (aws_hash_table_find(&subscriptions->request_response_paths, &path.topic, &element) || element == NULL) { AWS_LOGF_ERROR( AWS_LS_MQTT_REQUEST_RESPONSE, - "id=%p: internal state error removing reference to response path for topic " PRInSTR, - (void *)subscriptions->client, + "internal state error removing reference to response path for topic " PRInSTR, AWS_BYTE_CURSOR_PRI(path.topic)); continue; } @@ -199,15 +198,13 @@ void aws_mqtt_request_response_client_subscriptions_remove_request_subscription( if (entry->ref_count == 0) { AWS_LOGF_DEBUG( AWS_LS_MQTT_REQUEST_RESPONSE, - "id=%p: removing last reference to response path for topic " PRInSTR, - (void *)subscriptions->client, + "removing last reference to response path for topic " PRInSTR, AWS_BYTE_CURSOR_PRI(path.topic)); aws_hash_table_remove(&subscriptions->request_response_paths, &path.topic, NULL, NULL); } else { AWS_LOGF_DEBUG( AWS_LS_MQTT_REQUEST_RESPONSE, - "id=%p: removing reference to response path for topic " PRInSTR ", %zu references remain", - (void *)subscriptions->client, + "removing reference to response path for topic " PRInSTR ", %zu references remain", AWS_BYTE_CURSOR_PRI(path.topic), entry->ref_count); } @@ -222,7 +219,7 @@ static void s_match_wildcard_stream_subscriptions( AWS_LOGF_INFO( AWS_LS_MQTT_REQUEST_RESPONSE, - "= Looking subscription for topic '" PRInSTR "'", + "= Looking for subscription for topic '" PRInSTR "'", AWS_BYTE_CURSOR_PRI(publish_event->topic)); for (struct aws_hash_iter iter = aws_hash_iter_begin(subscriptions); !aws_hash_iter_done(&iter); @@ -308,8 +305,7 @@ void aws_mqtt_request_response_client_subscriptions_match( subscription_filter_element != NULL) { AWS_LOGF_DEBUG( AWS_LS_MQTT_REQUEST_RESPONSE, - "id=%p: request-response client incoming publish on topic '" PRInSTR "' matches streaming topic", - (void *)subscriptions->client, + "request-response client incoming publish on topic '" PRInSTR "' matches streaming topic", AWS_BYTE_CURSOR_PRI(publish_event->topic)); struct aws_rr_operation_list_topic_filter_entry *entry = subscription_filter_element->value; @@ -330,10 +326,9 @@ void aws_mqtt_request_response_client_subscriptions_match( response_path_element != NULL) { AWS_LOGF_DEBUG( AWS_LS_MQTT_REQUEST_RESPONSE, - "id=%p: request-response client incoming publish on topic '" PRInSTR "' matches response path", - (void *)subscriptions->client, + "request-response client incoming publish on topic '" PRInSTR "' matches response path", AWS_BYTE_CURSOR_PRI(publish_event->topic)); - on_request_operation_subscription_match(subscriptions->client, response_path_element->value, publish_event); + on_request_operation_subscription_match(response_path_element->value, publish_event, user_data); } } diff --git a/tests/request-response/request_response_client_tests.c b/tests/request-response/request_response_client_tests.c index 054b4574..82c750d5 100644 --- a/tests/request-response/request_response_client_tests.c +++ b/tests/request-response/request_response_client_tests.c @@ -3359,20 +3359,18 @@ static void s_rrs_fixture_on_stream_operation_subscription_match( } static void s_rrs_fixture_on_request_operation_subscription_match( - struct aws_mqtt_request_response_client *rr_client, struct aws_rr_response_path_entry *entry, - const struct aws_protocol_adapter_incoming_publish_event *publish_event) { + const struct aws_protocol_adapter_incoming_publish_event *publish_event, + void *user_data) { fprintf(stderr, "====== on req called\n"); } static int s_rrs_match_subscription_with_single_level_wildcards_fn(struct aws_allocator *allocator, void *ctx) { (void)ctx; - void *client = (void *)0x08; - struct aws_request_response_subscriptions subscriptions; - aws_mqtt_request_response_client_subscriptions_init(&subscriptions, client, allocator); + aws_mqtt_request_response_client_subscriptions_init(&subscriptions, allocator); struct aws_byte_cursor topic_filter1 = aws_byte_cursor_from_c_str("topic/123/abc"); struct aws_byte_cursor topic_filter2 = aws_byte_cursor_from_c_str("topic/123/+"); @@ -3425,11 +3423,9 @@ AWS_TEST_CASE( static int s_rrs_match_subscription_with_multi_level_wildcards_fn(struct aws_allocator *allocator, void *ctx) { (void)ctx; - void *client = (void *)0x08; - struct aws_request_response_subscriptions subscriptions; - aws_mqtt_request_response_client_subscriptions_init(&subscriptions, client, allocator); + aws_mqtt_request_response_client_subscriptions_init(&subscriptions, allocator); struct aws_byte_cursor topic_filter1 = aws_byte_cursor_from_c_str("topic/123/abc"); struct aws_byte_cursor topic_filter2 = aws_byte_cursor_from_c_str("topic/123/#"); From afb4028e4d62cce44b74e8fbbf69ff8751d78784 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Thu, 30 Jan 2025 15:33:11 -0800 Subject: [PATCH 16/29] Extract matching --- .../request_response_client.c | 1 + .../request_response_subscription_set.c | 71 ++++++++++++------- .../request_response_client_tests.c | 3 + 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/source/request-response/request_response_client.c b/source/request-response/request_response_client.c index 2b3ebf6d..a861c621 100644 --- a/source/request-response/request_response_client.c +++ b/source/request-response/request_response_client.c @@ -786,6 +786,7 @@ static void s_apply_publish_to_streaming_operation_list( const struct aws_byte_cursor *topic_filter, const struct aws_protocol_adapter_incoming_publish_event *publish_event, void *user_data) { + (void)topic_filter; (void)user_data; AWS_FATAL_ASSERT(operations != NULL); diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index 6223d1fe..8b533963 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -211,6 +211,26 @@ void aws_mqtt_request_response_client_subscriptions_remove_request_subscription( } } +static void s_match_stream_subscriptions( + const struct aws_hash_table *subscriptions, + const struct aws_protocol_adapter_incoming_publish_event *publish_event, + aws_mqtt_stream_operation_subscription_match_fn *on_stream_operation_subscription_match, + void *user_data) { + struct aws_hash_element *subscription_filter_element = NULL; + if (aws_hash_table_find(subscriptions, &publish_event->topic, &subscription_filter_element) == AWS_OP_SUCCESS && + subscription_filter_element != NULL) { + // TODO Deal with logs without client pointer. + AWS_LOGF_DEBUG( + AWS_LS_MQTT_REQUEST_RESPONSE, + "request-response client incoming publish on topic '" PRInSTR "' matches streaming topic", + AWS_BYTE_CURSOR_PRI(publish_event->topic)); + + struct aws_rr_operation_list_topic_filter_entry *entry = subscription_filter_element->value; + on_stream_operation_subscription_match( + &entry->operations, &entry->topic_filter_cursor, publish_event, user_data); + } +} + static void s_match_wildcard_stream_subscriptions( const struct aws_hash_table *subscriptions, const struct aws_protocol_adapter_incoming_publish_event *publish_event, @@ -285,6 +305,24 @@ static void s_match_wildcard_stream_subscriptions( } } +void s_match_request_response_subscriptions( + const struct aws_hash_table *request_response_paths, + const struct aws_protocol_adapter_incoming_publish_event *publish_event, + aws_mqtt_request_operation_subscription_match_fn *on_request_operation_subscription_match, + void *user_data) { + + struct aws_hash_element *response_path_element = NULL; + if (aws_hash_table_find(request_response_paths, &publish_event->topic, &response_path_element) == AWS_OP_SUCCESS && + response_path_element != NULL) { + AWS_LOGF_DEBUG( + AWS_LS_MQTT_REQUEST_RESPONSE, + "request-response client incoming publish on topic '" PRInSTR "' matches response path", + AWS_BYTE_CURSOR_PRI(publish_event->topic)); + + on_request_operation_subscription_match(response_path_element->value, publish_event, user_data); + } +} + void aws_mqtt_request_response_client_subscriptions_match( const struct aws_request_response_subscriptions *subscriptions, const struct aws_protocol_adapter_incoming_publish_event *publish_event, @@ -297,21 +335,11 @@ void aws_mqtt_request_response_client_subscriptions_match( AWS_FATAL_PRECONDITION(on_request_operation_subscription_match); /* Streaming operation handling */ - struct aws_hash_element *subscription_filter_element = NULL; - if (aws_hash_table_find( - &subscriptions->streaming_operation_subscription_lists, - &publish_event->topic, - &subscription_filter_element) == AWS_OP_SUCCESS && - subscription_filter_element != NULL) { - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "request-response client incoming publish on topic '" PRInSTR "' matches streaming topic", - AWS_BYTE_CURSOR_PRI(publish_event->topic)); - - struct aws_rr_operation_list_topic_filter_entry *entry = subscription_filter_element->value; - on_stream_operation_subscription_match( - &entry->operations, &entry->topic_filter_cursor, publish_event, user_data); - } + s_match_stream_subscriptions( + &subscriptions->streaming_operation_subscription_lists, + publish_event, + on_stream_operation_subscription_match, + user_data); s_match_wildcard_stream_subscriptions( &subscriptions->streaming_operation_wildcards_subscription_lists, @@ -320,15 +348,6 @@ void aws_mqtt_request_response_client_subscriptions_match( user_data); /* Request-Response handling */ - struct aws_hash_element *response_path_element = NULL; - if (aws_hash_table_find(&subscriptions->request_response_paths, &publish_event->topic, &response_path_element) == - AWS_OP_SUCCESS && - response_path_element != NULL) { - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "request-response client incoming publish on topic '" PRInSTR "' matches response path", - AWS_BYTE_CURSOR_PRI(publish_event->topic)); - - on_request_operation_subscription_match(response_path_element->value, publish_event, user_data); - } + s_match_request_response_subscriptions( + &subscriptions->request_response_paths, publish_event, on_request_operation_subscription_match, user_data); } diff --git a/tests/request-response/request_response_client_tests.c b/tests/request-response/request_response_client_tests.c index 82c750d5..6bc976e9 100644 --- a/tests/request-response/request_response_client_tests.c +++ b/tests/request-response/request_response_client_tests.c @@ -3362,6 +3362,9 @@ static void s_rrs_fixture_on_request_operation_subscription_match( struct aws_rr_response_path_entry *entry, const struct aws_protocol_adapter_incoming_publish_event *publish_event, void *user_data) { + (void)entry; + (void)publish_event; + (void)user_data; fprintf(stderr, "====== on req called\n"); } From 01fa22f2002bbedaef49c88362eedf2d0c007a45 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Thu, 30 Jan 2025 15:37:01 -0800 Subject: [PATCH 17/29] Fix log --- source/request-response/request_response_subscription_set.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index 8b533963..038bc5fb 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -284,8 +284,7 @@ static void s_match_wildcard_stream_subscriptions( if (!aws_byte_cursor_eq_c_str(&subscription_topic_filter_segment, "+") && !aws_byte_cursor_eq_ignore_case(&topic_segment, &subscription_topic_filter_segment)) { - AWS_LOGF_INFO( - AWS_LS_MQTT_REQUEST_RESPONSE, "======= topic segment differs", AWS_BYTE_CURSOR_PRI(topic_segment)); + AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "======= topic segment differs"); match = false; break; } From 42e85f60071cecdd4a199bd51dee659bf6eb6963 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Thu, 30 Jan 2025 15:50:13 -0800 Subject: [PATCH 18/29] Revert max_streaming_subscriptions in tests --- tests/request-response/request_response_client_tests.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/request-response/request_response_client_tests.c b/tests/request-response/request_response_client_tests.c index 6bc976e9..1fc829e1 100644 --- a/tests/request-response/request_response_client_tests.c +++ b/tests/request-response/request_response_client_tests.c @@ -1220,7 +1220,7 @@ static int s_init_fixture_streaming_operation_success( struct aws_mqtt_request_response_client_options rr_client_options = { .max_request_response_subscriptions = 2, - .max_streaming_subscriptions = 2, + .max_streaming_subscriptions = 1, .operation_timeout_seconds = 2, }; From e7f4a79c4f4e24e62a038eb66b81c00dde8e650c Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Fri, 31 Jan 2025 15:23:35 -0800 Subject: [PATCH 19/29] Add comments and test --- .../request_response_subscription_set.h | 37 +++++- .../request_response_client.c | 4 +- .../request_response_subscription_set.c | 125 +++++++++++------- tests/CMakeLists.txt | 2 + .../request_response_client_tests.c | 14 +- 5 files changed, 123 insertions(+), 59 deletions(-) diff --git a/include/aws/mqtt/private/request-response/request_response_subscription_set.h b/include/aws/mqtt/private/request-response/request_response_subscription_set.h index 3cd77c24..95208b61 100644 --- a/include/aws/mqtt/private/request-response/request_response_subscription_set.h +++ b/include/aws/mqtt/private/request-response/request_response_subscription_set.h @@ -9,12 +9,9 @@ #include #include #include -#include #include -struct aws_mqtt_request_response_client; - -/* Holds subscriptions for request-response client. */ +/* Handles subscriptions for request-response client. */ struct aws_request_response_subscriptions { struct aws_allocator *allocator; @@ -63,12 +60,18 @@ struct aws_rr_response_path_entry { struct aws_byte_buf correlation_token_json_path; }; +/* + * Callback type for matched stream subscriptions. + */ typedef void(aws_mqtt_stream_operation_subscription_match_fn)( const struct aws_linked_list *operations, const struct aws_byte_cursor *topic_filter, // TODO Do we need this for anything other than tests? const struct aws_protocol_adapter_incoming_publish_event *publish_event, void *user_data); +/* + * Callback type for matched request subscriptions. + */ typedef void(aws_mqtt_request_operation_subscription_match_fn)( struct aws_rr_response_path_entry *entry, const struct aws_protocol_adapter_incoming_publish_event *publish_event, @@ -76,26 +79,46 @@ typedef void(aws_mqtt_request_operation_subscription_match_fn)( AWS_EXTERN_C_BEGIN -AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_init( +/* + * Initialize internal state of a provided request-response subscriptions structure. + */ +AWS_MQTT_API int aws_mqtt_request_response_client_subscriptions_init( struct aws_request_response_subscriptions *subscriptions, struct aws_allocator *allocator); -AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_cleanup( +/* + * Clean up internals of a provided request-response subscriptions structure. + */ +AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_clean_up( struct aws_request_response_subscriptions *subscriptions); +/* + * Add a subscription for stream operations. + * If subscription with the same topic filter is already added, previously created + * aws_rr_operation_list_topic_filter_entry instance is returned. + */ AWS_MQTT_API struct aws_rr_operation_list_topic_filter_entry * aws_mqtt_request_response_client_subscriptions_add_stream_subscription( struct aws_request_response_subscriptions *subscriptions, const struct aws_byte_cursor *topic_filter); -AWS_MQTT_API int aws_mqtt_request_response_client_subscriptions_add_request_subscription( +/* + * Add subscriptions for request operations for topics specified in paths list. + */ +AWS_MQTT_API int aws_mqtt_request_response_client_subscriptions_add_request_subscriptions( struct aws_request_response_subscriptions *subscriptions, const struct aws_array_list *paths); +/* + * Remove subscriptions for request operations for topics specified in paths list. + */ AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_remove_request_subscription( struct aws_request_response_subscriptions *subscriptions, const struct aws_array_list *paths); +/* + * Call specified callbacks for all stream and request operations with subscriptions matching a provided publish event. + */ AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_match( const struct aws_request_response_subscriptions *subscriptions, const struct aws_protocol_adapter_incoming_publish_event *publish_event, diff --git a/source/request-response/request_response_client.c b/source/request-response/request_response_client.c index a861c621..8655044b 100644 --- a/source/request-response/request_response_client.c +++ b/source/request-response/request_response_client.c @@ -317,7 +317,7 @@ static void s_mqtt_request_response_client_final_destroy(struct aws_mqtt_request aws_priority_queue_clean_up(&client->operations_by_timeout); - aws_mqtt_request_response_client_subscriptions_cleanup(&client->subscriptions); + aws_mqtt_request_response_client_subscriptions_clean_up(&client->subscriptions); aws_hash_table_clean_up(&client->operations_by_correlation_tokens); aws_mem_release(client->allocator, client); @@ -1196,7 +1196,7 @@ static int s_add_request_operation_to_response_path_table( struct aws_mqtt_rr_client_operation *operation) { struct aws_array_list *paths = &operation->storage.request_storage.operation_response_paths; - return aws_mqtt_request_response_client_subscriptions_add_request_subscription(&client->subscriptions, paths); + return aws_mqtt_request_response_client_subscriptions_add_request_subscriptions(&client->subscriptions, paths); } static int s_add_request_operation_to_correlation_token_table( diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index 038bc5fb..4baba352 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -12,21 +12,6 @@ #define MQTT_RR_CLIENT_RESPONSE_TABLE_DEFAULT_SIZE 50 #define MQTT_RR_CLIENT_OPERATION_TABLE_DEFAULT_SIZE 50 -static struct aws_rr_operation_list_topic_filter_entry *s_aws_rr_operation_list_topic_filter_entry_new( - struct aws_allocator *allocator, - struct aws_byte_cursor topic_filter) { - struct aws_rr_operation_list_topic_filter_entry *entry = - aws_mem_calloc(allocator, 1, sizeof(struct aws_rr_operation_list_topic_filter_entry)); - - entry->allocator = allocator; - aws_byte_buf_init_copy_from_cursor(&entry->topic_filter, allocator, topic_filter); - entry->topic_filter_cursor = aws_byte_cursor_from_buf(&entry->topic_filter); - - aws_linked_list_init(&entry->operations); - - return entry; -} - static void s_aws_rr_operation_list_topic_filter_entry_destroy(struct aws_rr_operation_list_topic_filter_entry *entry) { if (entry == NULL) { return; @@ -72,51 +57,88 @@ static void s_aws_rr_response_path_table_hash_element_destroy(void *value) { s_aws_rr_response_path_entry_destroy(value); } -void aws_mqtt_request_response_client_subscriptions_init( +int aws_mqtt_request_response_client_subscriptions_init( struct aws_request_response_subscriptions *subscriptions, struct aws_allocator *allocator) { - AWS_FATAL_ASSERT(subscriptions); subscriptions->allocator = allocator; - aws_hash_table_init( - &subscriptions->streaming_operation_subscription_lists, - allocator, - MQTT_RR_CLIENT_OPERATION_TABLE_DEFAULT_SIZE, - aws_hash_byte_cursor_ptr, - aws_mqtt_byte_cursor_hash_equality, - NULL, - s_aws_rr_operation_list_topic_filter_entry_hash_element_destroy); - - aws_hash_table_init( - &subscriptions->streaming_operation_wildcards_subscription_lists, - allocator, - MQTT_RR_CLIENT_OPERATION_TABLE_DEFAULT_SIZE, - aws_hash_byte_cursor_ptr, - aws_mqtt_byte_cursor_hash_equality, - NULL, - s_aws_rr_operation_list_topic_filter_entry_hash_element_destroy); - - aws_hash_table_init( - &subscriptions->request_response_paths, - allocator, - MQTT_RR_CLIENT_RESPONSE_TABLE_DEFAULT_SIZE, - aws_hash_byte_cursor_ptr, - aws_mqtt_byte_cursor_hash_equality, - NULL, - s_aws_rr_response_path_table_hash_element_destroy); + if (aws_hash_table_init( + &subscriptions->streaming_operation_subscription_lists, + allocator, + MQTT_RR_CLIENT_OPERATION_TABLE_DEFAULT_SIZE, + aws_hash_byte_cursor_ptr, + aws_mqtt_byte_cursor_hash_equality, + NULL, + s_aws_rr_operation_list_topic_filter_entry_hash_element_destroy)) { + goto clean_up; + } + + if (aws_hash_table_init( + &subscriptions->streaming_operation_wildcards_subscription_lists, + allocator, + MQTT_RR_CLIENT_OPERATION_TABLE_DEFAULT_SIZE, + aws_hash_byte_cursor_ptr, + aws_mqtt_byte_cursor_hash_equality, + NULL, + s_aws_rr_operation_list_topic_filter_entry_hash_element_destroy)) { + goto clean_up; + } + + if (aws_hash_table_init( + &subscriptions->request_response_paths, + allocator, + MQTT_RR_CLIENT_RESPONSE_TABLE_DEFAULT_SIZE, + aws_hash_byte_cursor_ptr, + aws_mqtt_byte_cursor_hash_equality, + NULL, + s_aws_rr_response_path_table_hash_element_destroy)) { + goto clean_up; + } + + return AWS_OP_SUCCESS; + +clean_up: + aws_mqtt_request_response_client_subscriptions_clean_up(subscriptions); + return AWS_OP_ERR; } -void aws_mqtt_request_response_client_subscriptions_cleanup(struct aws_request_response_subscriptions *subscriptions) { - aws_hash_table_clean_up(&subscriptions->streaming_operation_subscription_lists); - aws_hash_table_clean_up(&subscriptions->streaming_operation_wildcards_subscription_lists); - aws_hash_table_clean_up(&subscriptions->request_response_paths); +void aws_mqtt_request_response_client_subscriptions_clean_up(struct aws_request_response_subscriptions *subscriptions) { + if (subscriptions == NULL) { + return; + } + + if (aws_hash_table_is_valid(&subscriptions->streaming_operation_subscription_lists)) { + aws_hash_table_clean_up(&subscriptions->streaming_operation_subscription_lists); + } + if (aws_hash_table_is_valid(&subscriptions->streaming_operation_wildcards_subscription_lists)) { + aws_hash_table_clean_up(&subscriptions->streaming_operation_wildcards_subscription_lists); + } + if (aws_hash_table_is_valid(&subscriptions->request_response_paths)) { + aws_hash_table_clean_up(&subscriptions->request_response_paths); + } +} + +static struct aws_rr_operation_list_topic_filter_entry *s_aws_rr_operation_list_topic_filter_entry_new( + struct aws_allocator *allocator, + struct aws_byte_cursor topic_filter) { + struct aws_rr_operation_list_topic_filter_entry *entry = + aws_mem_calloc(allocator, 1, sizeof(struct aws_rr_operation_list_topic_filter_entry)); + + entry->allocator = allocator; + aws_byte_buf_init_copy_from_cursor(&entry->topic_filter, allocator, topic_filter); + entry->topic_filter_cursor = aws_byte_cursor_from_buf(&entry->topic_filter); + + aws_linked_list_init(&entry->operations); + + return entry; } struct aws_rr_operation_list_topic_filter_entry *aws_mqtt_request_response_client_subscriptions_add_stream_subscription( struct aws_request_response_subscriptions *subscriptions, const struct aws_byte_cursor *topic_filter) { + AWS_FATAL_ASSERT(subscriptions); bool is_topic_with_wildcard = (memchr(topic_filter->ptr, '+', topic_filter->len) || memchr(topic_filter->ptr, '#', topic_filter->len)); @@ -144,9 +166,11 @@ struct aws_rr_operation_list_topic_filter_entry *aws_mqtt_request_response_clien return entry; } -int aws_mqtt_request_response_client_subscriptions_add_request_subscription( +int aws_mqtt_request_response_client_subscriptions_add_request_subscriptions( struct aws_request_response_subscriptions *subscriptions, const struct aws_array_list *paths) { + AWS_FATAL_ASSERT(subscriptions); + AWS_FATAL_ASSERT(paths); size_t path_count = aws_array_list_length(paths); for (size_t i = 0; i < path_count; ++i) { @@ -178,6 +202,9 @@ int aws_mqtt_request_response_client_subscriptions_add_request_subscription( void aws_mqtt_request_response_client_subscriptions_remove_request_subscription( struct aws_request_response_subscriptions *subscriptions, const struct aws_array_list *paths) { + AWS_FATAL_ASSERT(subscriptions); + AWS_FATAL_ASSERT(paths); + size_t path_count = aws_array_list_length(paths); for (size_t i = 0; i < path_count; ++i) { struct aws_mqtt_request_operation_response_path path; @@ -329,7 +356,9 @@ void aws_mqtt_request_response_client_subscriptions_match( aws_mqtt_request_operation_subscription_match_fn *on_request_operation_subscription_match, void *user_data) { + AWS_FATAL_PRECONDITION(subscriptions); AWS_FATAL_PRECONDITION(publish_event); + // TODO ? Allow NULLs? AWS_FATAL_PRECONDITION(on_stream_operation_subscription_match); AWS_FATAL_PRECONDITION(on_request_operation_subscription_match); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b6f36cc2..610f26a6 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -577,6 +577,8 @@ add_test_case(rrc_request_response_failure_invalid_correlation_token_type) add_test_case(rrc_request_response_failure_non_matching_correlation_token) add_test_case(rrc_request_response_multi_operation_sequence) +# "rrs" = request-response subscriptions +add_test_case(rrs_init_cleanup) add_test_case(rrs_match_subscription_with_single_level_wildcards) add_test_case(rrs_match_subscription_with_multi_level_wildcards) diff --git a/tests/request-response/request_response_client_tests.c b/tests/request-response/request_response_client_tests.c index 1fc829e1..079e4df0 100644 --- a/tests/request-response/request_response_client_tests.c +++ b/tests/request-response/request_response_client_tests.c @@ -3368,6 +3368,16 @@ static void s_rrs_fixture_on_request_operation_subscription_match( fprintf(stderr, "====== on req called\n"); } +static int s_rrs_init_cleanup_fn(struct aws_allocator *allocator, void *ctx) { + struct aws_request_response_subscriptions subscriptions; + + ASSERT_SUCCESS(aws_mqtt_request_response_client_subscriptions_init(&subscriptions, allocator)); + aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); + return AWS_OP_SUCCESS; +} + +AWS_TEST_CASE(rrs_init_cleanup, s_rrs_init_cleanup_fn) + static int s_rrs_match_subscription_with_single_level_wildcards_fn(struct aws_allocator *allocator, void *ctx) { (void)ctx; @@ -3414,7 +3424,7 @@ static int s_rrs_match_subscription_with_single_level_wildcards_fn(struct aws_al s_aws_rr_client_fixture_streaming_subscriptions_record_delete(record); - aws_mqtt_request_response_client_subscriptions_cleanup(&subscriptions); + aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); return AWS_OP_SUCCESS; } @@ -3467,7 +3477,7 @@ static int s_rrs_match_subscription_with_multi_level_wildcards_fn(struct aws_all s_aws_rr_client_fixture_streaming_subscriptions_record_delete(record); - aws_mqtt_request_response_client_subscriptions_cleanup(&subscriptions); + aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); return AWS_OP_SUCCESS; } From 98911da128c631115de0abdbf9149eb7bcf9a42b Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Fri, 31 Jan 2025 15:36:55 -0800 Subject: [PATCH 20/29] Fix tests naming --- tests/CMakeLists.txt | 5 +- .../request_response_client_tests.c | 61 ++++++++++++++++--- 2 files changed, 57 insertions(+), 9 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 610f26a6..1e6f0829 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -579,8 +579,9 @@ add_test_case(rrc_request_response_multi_operation_sequence) # "rrs" = request-response subscriptions add_test_case(rrs_init_cleanup) -add_test_case(rrs_match_subscription_with_single_level_wildcards) -add_test_case(rrs_match_subscription_with_multi_level_wildcards) +add_test_case(rrs_stream_subscriptions_match_single_level_wildcards) +add_test_case(rrs_stream_subscriptions_match_multi_level_wildcards) +add_test_case(rrs_stream_subscriptions_add_duplicate) generate_test_driver(${PROJECT_NAME}-tests) diff --git a/tests/request-response/request_response_client_tests.c b/tests/request-response/request_response_client_tests.c index 079e4df0..79a50fb6 100644 --- a/tests/request-response/request_response_client_tests.c +++ b/tests/request-response/request_response_client_tests.c @@ -3378,7 +3378,7 @@ static int s_rrs_init_cleanup_fn(struct aws_allocator *allocator, void *ctx) { AWS_TEST_CASE(rrs_init_cleanup, s_rrs_init_cleanup_fn) -static int s_rrs_match_subscription_with_single_level_wildcards_fn(struct aws_allocator *allocator, void *ctx) { +static int s_rrs_stream_subscriptions_match_single_level_wildcards_fn(struct aws_allocator *allocator, void *ctx) { (void)ctx; struct aws_request_response_subscriptions subscriptions; @@ -3392,8 +3392,6 @@ static int s_rrs_match_subscription_with_single_level_wildcards_fn(struct aws_al struct aws_byte_cursor topic1 = aws_byte_cursor_from_c_str("topic/123/abc"); struct aws_byte_cursor payload1 = aws_byte_cursor_from_c_str("Payload1"); - aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter1); - // TODO This does nothing. Add test case for this. aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter1); aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter2); aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter3); @@ -3430,10 +3428,10 @@ static int s_rrs_match_subscription_with_single_level_wildcards_fn(struct aws_al } AWS_TEST_CASE( - rrs_match_subscription_with_single_level_wildcards, - s_rrs_match_subscription_with_single_level_wildcards_fn) + rrs_stream_subscriptions_match_single_level_wildcards, + s_rrs_stream_subscriptions_match_single_level_wildcards_fn) -static int s_rrs_match_subscription_with_multi_level_wildcards_fn(struct aws_allocator *allocator, void *ctx) { +static int s_rrs_stream_subscriptions_match_multi_level_wildcards_fn(struct aws_allocator *allocator, void *ctx) { (void)ctx; struct aws_request_response_subscriptions subscriptions; @@ -3482,4 +3480,53 @@ static int s_rrs_match_subscription_with_multi_level_wildcards_fn(struct aws_all return AWS_OP_SUCCESS; } -AWS_TEST_CASE(rrs_match_subscription_with_multi_level_wildcards, s_rrs_match_subscription_with_multi_level_wildcards_fn) +AWS_TEST_CASE( + rrs_stream_subscriptions_match_multi_level_wildcards, + s_rrs_stream_subscriptions_match_multi_level_wildcards_fn) + +/* Adding multiple identical stream subscriptions should add only one. */ +static int s_rrs_stream_subscriptions_add_duplicate_fn(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_request_response_subscriptions subscriptions; + + aws_mqtt_request_response_client_subscriptions_init(&subscriptions, allocator); + + struct aws_byte_cursor topic_filter1 = aws_byte_cursor_from_c_str("topic/123/+"); + + struct aws_byte_cursor topic1 = aws_byte_cursor_from_c_str("topic/123/abc"); + struct aws_byte_cursor payload1 = aws_byte_cursor_from_c_str("Payload1"); + + aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter1); + aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter1); + + struct aws_protocol_adapter_incoming_publish_event publish_event = { + .topic = topic1, + .payload = payload1, + }; + + struct aws_rr_client_fixture_streaming_subscriptions_record *record = + s_aws_rr_client_fixture_streaming_subscriptions_record_new(allocator); + + aws_mqtt_request_response_client_subscriptions_match( + &subscriptions, + &publish_event, + s_rrs_fixture_on_stream_operation_subscription_match, + s_rrs_fixture_on_request_operation_subscription_match, + record); + + struct aws_rr_client_fixture_matched_subscription_view matched_subscriptions[] = { + {payload1, topic1, topic_filter1}, + }; + + ASSERT_SUCCESS(s_rrc_verify_streaming_subscriptions_publishes( + record, AWS_ARRAY_SIZE(matched_subscriptions), matched_subscriptions)); + + s_aws_rr_client_fixture_streaming_subscriptions_record_delete(record); + + aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); + + return AWS_OP_SUCCESS; +} + +AWS_TEST_CASE(rrs_stream_subscriptions_add_duplicate, s_rrs_stream_subscriptions_add_duplicate_fn) From 2390554077a981f5500fb7cd776723a34965dba0 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Mon, 3 Feb 2025 09:53:53 -0800 Subject: [PATCH 21/29] Simplify adding and removing req op --- .../request_response_subscription_set.h | 11 ++- .../request_response_client.c | 18 +++- .../request_response_subscription_set.c | 98 ++++++++----------- 3 files changed, 64 insertions(+), 63 deletions(-) diff --git a/include/aws/mqtt/private/request-response/request_response_subscription_set.h b/include/aws/mqtt/private/request-response/request_response_subscription_set.h index 95208b61..c9145f5d 100644 --- a/include/aws/mqtt/private/request-response/request_response_subscription_set.h +++ b/include/aws/mqtt/private/request-response/request_response_subscription_set.h @@ -103,18 +103,19 @@ AWS_MQTT_API struct aws_rr_operation_list_topic_filter_entry * const struct aws_byte_cursor *topic_filter); /* - * Add subscriptions for request operations for topics specified in paths list. + * Add a subscription for request operation. */ -AWS_MQTT_API int aws_mqtt_request_response_client_subscriptions_add_request_subscriptions( +AWS_MQTT_API int aws_mqtt_request_response_client_subscriptions_add_request_subscription( struct aws_request_response_subscriptions *subscriptions, - const struct aws_array_list *paths); + const struct aws_byte_cursor *topic_filter, + const struct aws_byte_cursor *correlation_token_json_path); /* - * Remove subscriptions for request operations for topics specified in paths list. + * Remove a subscription for a given request operation. */ AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_remove_request_subscription( struct aws_request_response_subscriptions *subscriptions, - const struct aws_array_list *paths); + const struct aws_byte_cursor *topic_filter); /* * Call specified callbacks for all stream and request operations with subscriptions matching a provided publish event. diff --git a/source/request-response/request_response_client.c b/source/request-response/request_response_client.c index 8655044b..5dbaf6cd 100644 --- a/source/request-response/request_response_client.c +++ b/source/request-response/request_response_client.c @@ -1196,7 +1196,16 @@ static int s_add_request_operation_to_response_path_table( struct aws_mqtt_rr_client_operation *operation) { struct aws_array_list *paths = &operation->storage.request_storage.operation_response_paths; - return aws_mqtt_request_response_client_subscriptions_add_request_subscriptions(&client->subscriptions, paths); + size_t path_count = aws_array_list_length(paths); + for (size_t i = 0; i < path_count; ++i) { + struct aws_mqtt_request_operation_response_path path; + aws_array_list_get_at(paths, &path, i); + if (aws_mqtt_request_response_client_subscriptions_add_request_subscription( + &client->subscriptions, &path.topic, &path.correlation_token_json_path)) { + return AWS_OP_ERR; + } + } + return AWS_OP_SUCCESS; } static int s_add_request_operation_to_correlation_token_table( @@ -1685,7 +1694,12 @@ static void s_remove_operation_from_client_tables(struct aws_mqtt_rr_client_oper struct aws_array_list *paths = &operation->storage.request_storage.operation_response_paths; - aws_mqtt_request_response_client_subscriptions_remove_request_subscription(&client->subscriptions, paths); + size_t path_count = aws_array_list_length(paths); + for (size_t i = 0; i < path_count; ++i) { + struct aws_mqtt_request_operation_response_path path; + aws_array_list_get_at(paths, &path, i); + aws_mqtt_request_response_client_subscriptions_remove_request_subscription(&client->subscriptions, &path.topic); + } } static void s_mqtt_rr_client_destroy_operation(struct aws_task *task, void *arg, enum aws_task_status status) { diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index 4baba352..c77ef181 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -7,7 +7,6 @@ #include #include -#include #define MQTT_RR_CLIENT_RESPONSE_TABLE_DEFAULT_SIZE 50 #define MQTT_RR_CLIENT_OPERATION_TABLE_DEFAULT_SIZE 50 @@ -166,34 +165,26 @@ struct aws_rr_operation_list_topic_filter_entry *aws_mqtt_request_response_clien return entry; } -int aws_mqtt_request_response_client_subscriptions_add_request_subscriptions( +int aws_mqtt_request_response_client_subscriptions_add_request_subscription( struct aws_request_response_subscriptions *subscriptions, - const struct aws_array_list *paths) { - AWS_FATAL_ASSERT(subscriptions); - AWS_FATAL_ASSERT(paths); - - size_t path_count = aws_array_list_length(paths); - for (size_t i = 0; i < path_count; ++i) { - struct aws_mqtt_request_operation_response_path path; - aws_array_list_get_at(paths, &path, i); - - struct aws_hash_element *element = NULL; - if (aws_hash_table_find(&subscriptions->request_response_paths, &path.topic, &element)) { - return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); - } + const struct aws_byte_cursor *topic_filter, + const struct aws_byte_cursor *correlation_token_json_path) { + struct aws_hash_element *element = NULL; + if (aws_hash_table_find(&subscriptions->request_response_paths, topic_filter, &element)) { + return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); + } - if (element != NULL) { - struct aws_rr_response_path_entry *entry = element->value; - ++entry->ref_count; - continue; - } + if (element != NULL) { + struct aws_rr_response_path_entry *entry = element->value; + ++entry->ref_count; + return AWS_OP_SUCCESS; + } - struct aws_rr_response_path_entry *entry = - s_aws_rr_response_path_entry_new(subscriptions->allocator, path.topic, path.correlation_token_json_path); - if (aws_hash_table_put(&subscriptions->request_response_paths, &entry->topic_cursor, entry, NULL)) { - s_aws_rr_response_path_entry_destroy(entry); - return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); - } + struct aws_rr_response_path_entry *entry = + s_aws_rr_response_path_entry_new(subscriptions->allocator, *topic_filter, *correlation_token_json_path); + if (aws_hash_table_put(&subscriptions->request_response_paths, &entry->topic_cursor, entry, NULL)) { + s_aws_rr_response_path_entry_destroy(entry); + return aws_raise_error(AWS_ERROR_MQTT_REQUEST_RESPONSE_INTERNAL_ERROR); } return AWS_OP_SUCCESS; @@ -201,40 +192,35 @@ int aws_mqtt_request_response_client_subscriptions_add_request_subscriptions( void aws_mqtt_request_response_client_subscriptions_remove_request_subscription( struct aws_request_response_subscriptions *subscriptions, - const struct aws_array_list *paths) { - AWS_FATAL_ASSERT(subscriptions); - AWS_FATAL_ASSERT(paths); + const struct aws_byte_cursor *topic_filter) { - size_t path_count = aws_array_list_length(paths); - for (size_t i = 0; i < path_count; ++i) { - struct aws_mqtt_request_operation_response_path path; - aws_array_list_get_at(paths, &path, i); + AWS_FATAL_ASSERT(subscriptions); + AWS_FATAL_ASSERT(topic_filter); - struct aws_hash_element *element = NULL; - if (aws_hash_table_find(&subscriptions->request_response_paths, &path.topic, &element) || element == NULL) { - AWS_LOGF_ERROR( - AWS_LS_MQTT_REQUEST_RESPONSE, - "internal state error removing reference to response path for topic " PRInSTR, - AWS_BYTE_CURSOR_PRI(path.topic)); - continue; - } + struct aws_hash_element *element = NULL; + if (aws_hash_table_find(&subscriptions->request_response_paths, topic_filter, &element) || element == NULL) { + AWS_LOGF_ERROR( + AWS_LS_MQTT_REQUEST_RESPONSE, + "internal state error removing reference to response path for topic " PRInSTR, + AWS_BYTE_CURSOR_PRI(*topic_filter)); + return; + } - struct aws_rr_response_path_entry *entry = element->value; - --entry->ref_count; + struct aws_rr_response_path_entry *entry = element->value; + --entry->ref_count; - if (entry->ref_count == 0) { - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "removing last reference to response path for topic " PRInSTR, - AWS_BYTE_CURSOR_PRI(path.topic)); - aws_hash_table_remove(&subscriptions->request_response_paths, &path.topic, NULL, NULL); - } else { - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "removing reference to response path for topic " PRInSTR ", %zu references remain", - AWS_BYTE_CURSOR_PRI(path.topic), - entry->ref_count); - } + if (entry->ref_count == 0) { + AWS_LOGF_DEBUG( + AWS_LS_MQTT_REQUEST_RESPONSE, + "removing last reference to response path for topic " PRInSTR, + AWS_BYTE_CURSOR_PRI(*topic_filter)); + aws_hash_table_remove(&subscriptions->request_response_paths, topic_filter, NULL, NULL); + } else { + AWS_LOGF_DEBUG( + AWS_LS_MQTT_REQUEST_RESPONSE, + "removing reference to response path for topic " PRInSTR ", %zu references remain", + AWS_BYTE_CURSOR_PRI(*topic_filter), + entry->ref_count); } } From 25fcb4af6c0ea2c753e8682e1f43b23490363052 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Mon, 3 Feb 2025 11:42:28 -0800 Subject: [PATCH 22/29] More tests --- tests/CMakeLists.txt | 4 + .../request_response_client_tests.c | 439 +++++++++++++++--- 2 files changed, 382 insertions(+), 61 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1e6f0829..61c7a1d8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -582,6 +582,10 @@ add_test_case(rrs_init_cleanup) add_test_case(rrs_stream_subscriptions_match_single_level_wildcards) add_test_case(rrs_stream_subscriptions_match_multi_level_wildcards) add_test_case(rrs_stream_subscriptions_add_duplicate) +add_test_case(rrs_request_subscriptions_add_single_subscription) +add_test_case(rrs_request_subscriptions_remove_subscription) +add_test_case(rrs_request_subscriptions_add_duplicate_then_remove) +add_test_case(rrs_request_subscriptions_remove_nonexistent_subscription) generate_test_driver(${PROJECT_NAME}-tests) diff --git a/tests/request-response/request_response_client_tests.c b/tests/request-response/request_response_client_tests.c index 79a50fb6..b6ba76bb 100644 --- a/tests/request-response/request_response_client_tests.c +++ b/tests/request-response/request_response_client_tests.c @@ -3241,74 +3241,144 @@ static int s_rrc_request_response_multi_operation_sequence_fn(struct aws_allocat AWS_TEST_CASE(rrc_request_response_multi_operation_sequence, s_rrc_request_response_multi_operation_sequence_fn) -struct aws_rr_client_fixture_matched_subscription { +struct aws_rr_client_fixture_matched_stream_subscription { struct aws_allocator *allocator; struct aws_byte_buf payload; struct aws_byte_buf topic; struct aws_byte_buf topic_filter; }; -struct aws_rr_client_fixture_matched_subscription_view { +struct aws_rr_client_fixture_matched_stream_subscription_view { struct aws_byte_cursor payload; struct aws_byte_cursor topic; struct aws_byte_cursor topic_filter; }; -struct aws_rr_client_fixture_streaming_subscriptions_record { +struct aws_rr_client_fixture_matched_request_subscription { struct aws_allocator *allocator; - // table: topic_filter -> aws_rr_client_fixture_matched_subscription - struct aws_hash_table matches; - size_t matches_count; + struct aws_byte_buf payload; + struct aws_byte_buf topic; + struct aws_byte_buf topic_filter; + struct aws_byte_buf token_path; }; -static void s_aws_rr_client_fixture_streaming_subscription_destroy(void *value) { - struct aws_rr_client_fixture_matched_subscription *matched_subscription = value; +struct aws_rr_client_fixture_matched_request_subscription_view { + struct aws_byte_cursor payload; + struct aws_byte_cursor topic; + struct aws_byte_cursor topic_filter; + struct aws_byte_cursor token_path; +}; + +struct aws_rr_client_fixture_subscriptions_matches_record { + struct aws_allocator *allocator; + // table: topic_filter -> aws_rr_client_fixture_matched_stream_subscription + struct aws_hash_table stream_matches; + // table: topic_filter -> aws_rr_client_fixture_matched_request_subscription + struct aws_hash_table request_matches; + size_t stream_matches_count; + size_t request_matches_count; +}; + +static void s_aws_rr_client_fixture_stream_subscription_destroy(void *value) { + struct aws_rr_client_fixture_matched_stream_subscription *matched_subscription = value; + aws_byte_buf_clean_up(&matched_subscription->payload); + aws_byte_buf_clean_up(&matched_subscription->topic); + aws_byte_buf_clean_up(&matched_subscription->topic_filter); + aws_mem_release(matched_subscription->allocator, matched_subscription); +} + +static void s_aws_rr_client_fixture_request_subscription_destroy(void *value) { + struct aws_rr_client_fixture_matched_request_subscription *matched_subscription = value; aws_byte_buf_clean_up(&matched_subscription->payload); aws_byte_buf_clean_up(&matched_subscription->topic); aws_byte_buf_clean_up(&matched_subscription->topic_filter); + aws_byte_buf_clean_up(&matched_subscription->token_path); aws_mem_release(matched_subscription->allocator, matched_subscription); } -struct aws_rr_client_fixture_streaming_subscriptions_record *s_aws_rr_client_fixture_streaming_subscriptions_record_new( +struct aws_rr_client_fixture_subscriptions_matches_record *s_aws_rr_client_fixture_subscriptions_matches_record_new( struct aws_allocator *allocator) { - struct aws_rr_client_fixture_streaming_subscriptions_record *record = - aws_mem_calloc(allocator, 1, sizeof(struct aws_rr_client_fixture_streaming_subscriptions_record)); + struct aws_rr_client_fixture_subscriptions_matches_record *record = + aws_mem_calloc(allocator, 1, sizeof(struct aws_rr_client_fixture_subscriptions_matches_record)); record->allocator = allocator; aws_hash_table_init( - &record->matches, + &record->stream_matches, allocator, 10, aws_hash_byte_cursor_ptr, aws_mqtt_byte_cursor_hash_equality, NULL, - s_aws_rr_client_fixture_streaming_subscription_destroy); - record->matches_count = 0; + s_aws_rr_client_fixture_stream_subscription_destroy); + record->stream_matches_count = 0; + + aws_hash_table_init( + &record->request_matches, + allocator, + 10, + aws_hash_byte_cursor_ptr, + aws_mqtt_byte_cursor_hash_equality, + NULL, + s_aws_rr_client_fixture_request_subscription_destroy); + record->request_matches_count = 0; return record; } -void s_aws_rr_client_fixture_streaming_subscriptions_record_delete( - struct aws_rr_client_fixture_streaming_subscriptions_record *record) { - aws_hash_table_clean_up(&record->matches); +void s_aws_rr_client_fixture_subscriptions_macthes_record_delete( + struct aws_rr_client_fixture_subscriptions_matches_record *record) { + aws_hash_table_clean_up(&record->stream_matches); + aws_hash_table_clean_up(&record->request_matches); aws_mem_release(record->allocator, record); } -static int s_rrc_verify_streaming_subscriptions_publishes( - struct aws_rr_client_fixture_streaming_subscriptions_record *record, - size_t expected_publish_count, - struct aws_rr_client_fixture_matched_subscription_view *expected_matched_subscriptions) { +static int s_rrc_verify_subscriptions_publishes( + struct aws_rr_client_fixture_subscriptions_matches_record *record, + size_t expected_stream_matches_count, + struct aws_rr_client_fixture_matched_stream_subscription_view *expected_stream_matched_subscriptions, + size_t expected_request_matches_count, + struct aws_rr_client_fixture_matched_request_subscription_view *expected_request_matched_subscriptions) { + + ASSERT_INT_EQUALS(expected_stream_matches_count, record->stream_matches_count); - ASSERT_INT_EQUALS(expected_publish_count, record->matches_count); + for (size_t i = 0; i < expected_stream_matches_count; ++i) { + struct aws_rr_client_fixture_matched_stream_subscription_view *expected_matched_subscription = + &expected_stream_matched_subscriptions[i]; - for (size_t i = 0; i < expected_publish_count; ++i) { - struct aws_rr_client_fixture_matched_subscription_view *expected_matched_subscription = - &expected_matched_subscriptions[i]; + struct aws_hash_element *element = NULL; + ASSERT_SUCCESS( + aws_hash_table_find(&record->stream_matches, &expected_matched_subscription->topic_filter, &element)); + + struct aws_rr_client_fixture_matched_stream_subscription *actual_matched_subscription = element->value; + + ASSERT_BIN_ARRAYS_EQUALS( + expected_matched_subscription->payload.ptr, + expected_matched_subscription->payload.len, + actual_matched_subscription->payload.buffer, + actual_matched_subscription->payload.len); + ASSERT_BIN_ARRAYS_EQUALS( + expected_matched_subscription->topic.ptr, + expected_matched_subscription->topic.len, + actual_matched_subscription->topic.buffer, + actual_matched_subscription->topic.len); + ASSERT_BIN_ARRAYS_EQUALS( + expected_matched_subscription->topic_filter.ptr, + expected_matched_subscription->topic_filter.len, + actual_matched_subscription->topic_filter.buffer, + actual_matched_subscription->topic_filter.len); + } + + ASSERT_INT_EQUALS(expected_request_matches_count, record->request_matches_count); + + for (size_t i = 0; i < expected_request_matches_count; ++i) { + struct aws_rr_client_fixture_matched_request_subscription_view *expected_matched_subscription = + &expected_request_matched_subscriptions[i]; struct aws_hash_element *element = NULL; - ASSERT_SUCCESS(aws_hash_table_find(&record->matches, &expected_matched_subscription->topic_filter, &element)); + ASSERT_SUCCESS( + aws_hash_table_find(&record->request_matches, &expected_matched_subscription->topic_filter, &element)); - struct aws_rr_client_fixture_matched_subscription *actual_matched_subscription = element->value; + struct aws_rr_client_fixture_matched_request_subscription *actual_matched_subscription = element->value; ASSERT_BIN_ARRAYS_EQUALS( expected_matched_subscription->payload.ptr, @@ -3325,6 +3395,11 @@ static int s_rrc_verify_streaming_subscriptions_publishes( expected_matched_subscription->topic_filter.len, actual_matched_subscription->topic_filter.buffer, actual_matched_subscription->topic_filter.len); + ASSERT_BIN_ARRAYS_EQUALS( + expected_matched_subscription->token_path.ptr, + expected_matched_subscription->token_path.len, + actual_matched_subscription->token_path.buffer, + actual_matched_subscription->token_path.len); } return AWS_OP_SUCCESS; @@ -3338,24 +3413,17 @@ static void s_rrs_fixture_on_stream_operation_subscription_match( (void)operations; - struct aws_rr_client_fixture_streaming_subscriptions_record *record = user_data; + struct aws_rr_client_fixture_subscriptions_matches_record *record = user_data; - struct aws_rr_client_fixture_matched_subscription *matched_subscription = - aws_mem_calloc(record->allocator, 1, sizeof(struct aws_rr_client_fixture_matched_subscription)); + struct aws_rr_client_fixture_matched_stream_subscription *matched_subscription = + aws_mem_calloc(record->allocator, 1, sizeof(struct aws_rr_client_fixture_matched_stream_subscription)); matched_subscription->allocator = record->allocator; aws_byte_buf_init_copy_from_cursor(&matched_subscription->payload, record->allocator, publish_event->payload); aws_byte_buf_init_copy_from_cursor(&matched_subscription->topic, record->allocator, publish_event->topic); aws_byte_buf_init_copy_from_cursor(&matched_subscription->topic_filter, record->allocator, *topic_filter); - aws_hash_table_put(&record->matches, topic_filter, matched_subscription, NULL); - ++record->matches_count; - - fprintf( - stderr, - "====== on stream called: topic %.*s; payload %.*s; topic filter %.*s\n", - AWS_BYTE_BUF_PRI(matched_subscription->topic), - AWS_BYTE_BUF_PRI(matched_subscription->payload), - AWS_BYTE_BUF_PRI(matched_subscription->topic_filter)); + aws_hash_table_put(&record->stream_matches, topic_filter, matched_subscription, NULL); + ++record->stream_matches_count; } static void s_rrs_fixture_on_request_operation_subscription_match( @@ -3364,13 +3432,25 @@ static void s_rrs_fixture_on_request_operation_subscription_match( void *user_data) { (void)entry; (void)publish_event; - (void)user_data; - fprintf(stderr, "====== on req called\n"); + + struct aws_rr_client_fixture_subscriptions_matches_record *record = user_data; + + struct aws_rr_client_fixture_matched_request_subscription *matched_subscription = + aws_mem_calloc(record->allocator, 1, sizeof(struct aws_rr_client_fixture_matched_request_subscription)); + matched_subscription->allocator = record->allocator; + aws_byte_buf_init_copy_from_cursor(&matched_subscription->payload, record->allocator, publish_event->payload); + aws_byte_buf_init_copy_from_cursor(&matched_subscription->topic, record->allocator, publish_event->topic); + aws_byte_buf_init_copy_from_cursor(&matched_subscription->topic_filter, record->allocator, publish_event->topic); + aws_byte_buf_init_copy(&matched_subscription->token_path, record->allocator, &entry->correlation_token_json_path); + + aws_hash_table_put(&record->request_matches, &publish_event->topic, matched_subscription, NULL); + ++record->request_matches_count; } static int s_rrs_init_cleanup_fn(struct aws_allocator *allocator, void *ctx) { - struct aws_request_response_subscriptions subscriptions; + (void)ctx; + struct aws_request_response_subscriptions subscriptions; ASSERT_SUCCESS(aws_mqtt_request_response_client_subscriptions_init(&subscriptions, allocator)); aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); return AWS_OP_SUCCESS; @@ -3401,8 +3481,8 @@ static int s_rrs_stream_subscriptions_match_single_level_wildcards_fn(struct aws .payload = payload1, }; - struct aws_rr_client_fixture_streaming_subscriptions_record *record = - s_aws_rr_client_fixture_streaming_subscriptions_record_new(allocator); + struct aws_rr_client_fixture_subscriptions_matches_record *record = + s_aws_rr_client_fixture_subscriptions_matches_record_new(allocator); aws_mqtt_request_response_client_subscriptions_match( &subscriptions, @@ -3411,16 +3491,20 @@ static int s_rrs_stream_subscriptions_match_single_level_wildcards_fn(struct aws s_rrs_fixture_on_request_operation_subscription_match, record); - struct aws_rr_client_fixture_matched_subscription_view matched_subscriptions[] = { + struct aws_rr_client_fixture_matched_stream_subscription_view matched_subscriptions[] = { {payload1, topic1, topic_filter1}, {payload1, topic1, topic_filter2}, {payload1, topic1, topic_filter3}, }; - ASSERT_SUCCESS(s_rrc_verify_streaming_subscriptions_publishes( - record, AWS_ARRAY_SIZE(matched_subscriptions), matched_subscriptions)); + ASSERT_SUCCESS(s_rrc_verify_subscriptions_publishes( + record, + AWS_ARRAY_SIZE(matched_subscriptions), + matched_subscriptions, + 0, /* expected_request_matches_count */ + NULL)); - s_aws_rr_client_fixture_streaming_subscriptions_record_delete(record); + s_aws_rr_client_fixture_subscriptions_macthes_record_delete(record); aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); @@ -3454,8 +3538,8 @@ static int s_rrs_stream_subscriptions_match_multi_level_wildcards_fn(struct aws_ .payload = payload1, }; - struct aws_rr_client_fixture_streaming_subscriptions_record *record = - s_aws_rr_client_fixture_streaming_subscriptions_record_new(allocator); + struct aws_rr_client_fixture_subscriptions_matches_record *record = + s_aws_rr_client_fixture_subscriptions_matches_record_new(allocator); aws_mqtt_request_response_client_subscriptions_match( &subscriptions, @@ -3464,16 +3548,20 @@ static int s_rrs_stream_subscriptions_match_multi_level_wildcards_fn(struct aws_ s_rrs_fixture_on_request_operation_subscription_match, record); - struct aws_rr_client_fixture_matched_subscription_view matched_subscriptions[] = { + struct aws_rr_client_fixture_matched_stream_subscription_view matched_subscriptions[] = { {payload1, topic1, topic_filter1}, {payload1, topic1, topic_filter2}, {payload1, topic1, topic_filter3}, }; - ASSERT_SUCCESS(s_rrc_verify_streaming_subscriptions_publishes( - record, AWS_ARRAY_SIZE(matched_subscriptions), matched_subscriptions)); + ASSERT_SUCCESS(s_rrc_verify_subscriptions_publishes( + record, + AWS_ARRAY_SIZE(matched_subscriptions), + matched_subscriptions, + 0, /* expected_request_matches_count */ + NULL)); - s_aws_rr_client_fixture_streaming_subscriptions_record_delete(record); + s_aws_rr_client_fixture_subscriptions_macthes_record_delete(record); aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); @@ -3484,7 +3572,7 @@ AWS_TEST_CASE( rrs_stream_subscriptions_match_multi_level_wildcards, s_rrs_stream_subscriptions_match_multi_level_wildcards_fn) -/* Adding multiple identical stream subscriptions should add only one. */ +/* Adding multiple identical stream subscriptions should add only one record. */ static int s_rrs_stream_subscriptions_add_duplicate_fn(struct aws_allocator *allocator, void *ctx) { (void)ctx; @@ -3505,8 +3593,8 @@ static int s_rrs_stream_subscriptions_add_duplicate_fn(struct aws_allocator *all .payload = payload1, }; - struct aws_rr_client_fixture_streaming_subscriptions_record *record = - s_aws_rr_client_fixture_streaming_subscriptions_record_new(allocator); + struct aws_rr_client_fixture_subscriptions_matches_record *record = + s_aws_rr_client_fixture_subscriptions_matches_record_new(allocator); aws_mqtt_request_response_client_subscriptions_match( &subscriptions, @@ -3515,14 +3603,18 @@ static int s_rrs_stream_subscriptions_add_duplicate_fn(struct aws_allocator *all s_rrs_fixture_on_request_operation_subscription_match, record); - struct aws_rr_client_fixture_matched_subscription_view matched_subscriptions[] = { + struct aws_rr_client_fixture_matched_stream_subscription_view matched_subscriptions[] = { {payload1, topic1, topic_filter1}, }; - ASSERT_SUCCESS(s_rrc_verify_streaming_subscriptions_publishes( - record, AWS_ARRAY_SIZE(matched_subscriptions), matched_subscriptions)); + ASSERT_SUCCESS(s_rrc_verify_subscriptions_publishes( + record, + AWS_ARRAY_SIZE(matched_subscriptions), + matched_subscriptions, + 0, /* expected_request_matches_count */ + NULL)); - s_aws_rr_client_fixture_streaming_subscriptions_record_delete(record); + s_aws_rr_client_fixture_subscriptions_macthes_record_delete(record); aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); @@ -3530,3 +3622,228 @@ static int s_rrs_stream_subscriptions_add_duplicate_fn(struct aws_allocator *all } AWS_TEST_CASE(rrs_stream_subscriptions_add_duplicate, s_rrs_stream_subscriptions_add_duplicate_fn) + +static int s_rrs_request_subscriptions_add_single_subscription_fn(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_request_response_subscriptions subscriptions; + + aws_mqtt_request_response_client_subscriptions_init(&subscriptions, allocator); + + struct aws_byte_cursor topic1 = aws_byte_cursor_from_c_str("topic/123/abc"); + struct aws_byte_cursor token_path1 = aws_byte_cursor_from_c_str("token1"); + struct aws_byte_cursor payload1 = aws_byte_cursor_from_c_str("Payload1"); + + aws_mqtt_request_response_client_subscriptions_add_request_subscription(&subscriptions, &topic1, &token_path1); + + struct aws_protocol_adapter_incoming_publish_event publish_event = { + .topic = topic1, + .payload = payload1, + }; + + struct aws_rr_client_fixture_subscriptions_matches_record *record = + s_aws_rr_client_fixture_subscriptions_matches_record_new(allocator); + + aws_mqtt_request_response_client_subscriptions_match( + &subscriptions, + &publish_event, + s_rrs_fixture_on_stream_operation_subscription_match, + s_rrs_fixture_on_request_operation_subscription_match, + record); + + struct aws_rr_client_fixture_matched_request_subscription_view matched_subscriptions[] = { + {payload1, topic1, topic1, token_path1}, + }; + + ASSERT_SUCCESS(s_rrc_verify_subscriptions_publishes( + record, + 0 /* expected_stream_matches_count */, + NULL, + AWS_ARRAY_SIZE(matched_subscriptions), + matched_subscriptions)); + + s_aws_rr_client_fixture_subscriptions_macthes_record_delete(record); + + aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); + + return AWS_OP_SUCCESS; +} + +AWS_TEST_CASE(rrs_request_subscriptions_add_single_subscription, s_rrs_request_subscriptions_add_single_subscription_fn) + +static int s_rrs_request_subscriptions_remove_subscription_fn(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_request_response_subscriptions subscriptions; + + aws_mqtt_request_response_client_subscriptions_init(&subscriptions, allocator); + + struct aws_byte_cursor topic1 = aws_byte_cursor_from_c_str("topic/123/abc"); + struct aws_byte_cursor token_path1 = aws_byte_cursor_from_c_str("token1"); + struct aws_byte_cursor payload1 = aws_byte_cursor_from_c_str("Payload1"); + + aws_mqtt_request_response_client_subscriptions_add_request_subscription(&subscriptions, &topic1, &token_path1); + + struct aws_protocol_adapter_incoming_publish_event publish_event = { + .topic = topic1, + .payload = payload1, + }; + + struct aws_rr_client_fixture_subscriptions_matches_record *record = + s_aws_rr_client_fixture_subscriptions_matches_record_new(allocator); + + aws_mqtt_request_response_client_subscriptions_match( + &subscriptions, + &publish_event, + s_rrs_fixture_on_stream_operation_subscription_match, + s_rrs_fixture_on_request_operation_subscription_match, + record); + + struct aws_rr_client_fixture_matched_request_subscription_view matched_subscriptions[] = { + {payload1, topic1, topic1, token_path1}, + }; + + ASSERT_SUCCESS(s_rrc_verify_subscriptions_publishes( + record, + 0 /* expected_stream_matches_count */, + NULL, + AWS_ARRAY_SIZE(matched_subscriptions), + matched_subscriptions)); + + s_aws_rr_client_fixture_subscriptions_macthes_record_delete(record); + + aws_mqtt_request_response_client_subscriptions_remove_request_subscription(&subscriptions, &topic1); + + struct aws_rr_client_fixture_subscriptions_matches_record *record2 = + s_aws_rr_client_fixture_subscriptions_matches_record_new(allocator); + + aws_mqtt_request_response_client_subscriptions_match( + &subscriptions, + &publish_event, + s_rrs_fixture_on_stream_operation_subscription_match, + s_rrs_fixture_on_request_operation_subscription_match, + record2); + + ASSERT_SUCCESS(s_rrc_verify_subscriptions_publishes( + record, 0 /* expected_stream_matches_count */, NULL, 0 /* expected_request_matches_count */, NULL)); + + s_aws_rr_client_fixture_subscriptions_macthes_record_delete(record2); + + aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); + + return AWS_OP_SUCCESS; +} + +AWS_TEST_CASE(rrs_request_subscriptions_remove_subscription, s_rrs_request_subscriptions_remove_subscription_fn) + +/* After adding multiple identical request subscriptions, the same number of removals is required to actually remove + * the subscription. */ +static int s_rrs_request_subscriptions_add_duplicate_then_remove_fn(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_request_response_subscriptions subscriptions; + + aws_mqtt_request_response_client_subscriptions_init(&subscriptions, allocator); + + struct aws_byte_cursor topic1 = aws_byte_cursor_from_c_str("topic/123/abc"); + struct aws_byte_cursor token_path1 = aws_byte_cursor_from_c_str("token1"); + struct aws_byte_cursor payload1 = aws_byte_cursor_from_c_str("Payload1"); + + aws_mqtt_request_response_client_subscriptions_add_request_subscription(&subscriptions, &topic1, &token_path1); + aws_mqtt_request_response_client_subscriptions_add_request_subscription(&subscriptions, &topic1, &token_path1); + + struct aws_protocol_adapter_incoming_publish_event publish_event = { + .topic = topic1, + .payload = payload1, + }; + + struct aws_rr_client_fixture_subscriptions_matches_record *record = + s_aws_rr_client_fixture_subscriptions_matches_record_new(allocator); + + aws_mqtt_request_response_client_subscriptions_match( + &subscriptions, + &publish_event, + s_rrs_fixture_on_stream_operation_subscription_match, + s_rrs_fixture_on_request_operation_subscription_match, + record); + + struct aws_rr_client_fixture_matched_request_subscription_view matched_subscriptions[] = { + {payload1, topic1, topic1, token_path1}, + }; + + ASSERT_SUCCESS(s_rrc_verify_subscriptions_publishes( + record, + 0 /* expected_stream_matches_count */, + NULL, + AWS_ARRAY_SIZE(matched_subscriptions), + matched_subscriptions)); + + s_aws_rr_client_fixture_subscriptions_macthes_record_delete(record); + + // First remove, decrement subscription's internal counter to 1. + aws_mqtt_request_response_client_subscriptions_remove_request_subscription(&subscriptions, &topic1); + + struct aws_rr_client_fixture_subscriptions_matches_record *record2 = + s_aws_rr_client_fixture_subscriptions_matches_record_new(allocator); + + aws_mqtt_request_response_client_subscriptions_match( + &subscriptions, + &publish_event, + s_rrs_fixture_on_stream_operation_subscription_match, + s_rrs_fixture_on_request_operation_subscription_match, + record2); + + ASSERT_SUCCESS(s_rrc_verify_subscriptions_publishes( + record2, + 0 /* expected_stream_matches_count */, + NULL, + AWS_ARRAY_SIZE(matched_subscriptions), + matched_subscriptions)); + + s_aws_rr_client_fixture_subscriptions_macthes_record_delete(record2); + + // Second remove, decrement subscription's internal counter to 0 and remove it. + aws_mqtt_request_response_client_subscriptions_remove_request_subscription(&subscriptions, &topic1); + + struct aws_rr_client_fixture_subscriptions_matches_record *record3 = + s_aws_rr_client_fixture_subscriptions_matches_record_new(allocator); + + aws_mqtt_request_response_client_subscriptions_match( + &subscriptions, + &publish_event, + s_rrs_fixture_on_stream_operation_subscription_match, + s_rrs_fixture_on_request_operation_subscription_match, + record3); + + ASSERT_SUCCESS(s_rrc_verify_subscriptions_publishes( + record3, 0 /* expected_stream_matches_count */, NULL, 0 /* expected_request_matches_count */, NULL)); + + s_aws_rr_client_fixture_subscriptions_macthes_record_delete(record3); + + aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); + + return AWS_OP_SUCCESS; +} + +AWS_TEST_CASE( + rrs_request_subscriptions_add_duplicate_then_remove, + s_rrs_request_subscriptions_add_duplicate_then_remove_fn) + +static int s_rrs_request_subscriptions_remove_nonexistent_subscription_fn(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_request_response_subscriptions subscriptions; + + aws_mqtt_request_response_client_subscriptions_init(&subscriptions, allocator); + + struct aws_byte_cursor topic1 = aws_byte_cursor_from_c_str("topic/123/abc"); + aws_mqtt_request_response_client_subscriptions_remove_request_subscription(&subscriptions, &topic1); + + aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); + + return AWS_OP_SUCCESS; +} + +AWS_TEST_CASE( + rrs_request_subscriptions_remove_nonexistent_subscription, + s_rrs_request_subscriptions_remove_nonexistent_subscription_fn) From c6495be8186bcb871a72beb328438ecb7a40d8ab Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Mon, 3 Feb 2025 11:48:10 -0800 Subject: [PATCH 23/29] Test for both subscriptions --- tests/CMakeLists.txt | 1 + .../request_response_client_tests.c | 58 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 61c7a1d8..1cb3d0e1 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -586,6 +586,7 @@ add_test_case(rrs_request_subscriptions_add_single_subscription) add_test_case(rrs_request_subscriptions_remove_subscription) add_test_case(rrs_request_subscriptions_add_duplicate_then_remove) add_test_case(rrs_request_subscriptions_remove_nonexistent_subscription) +add_test_case(rrs_stream_and_request_subscriptions_add_same_subscription) generate_test_driver(${PROJECT_NAME}-tests) diff --git a/tests/request-response/request_response_client_tests.c b/tests/request-response/request_response_client_tests.c index b6ba76bb..038bf61c 100644 --- a/tests/request-response/request_response_client_tests.c +++ b/tests/request-response/request_response_client_tests.c @@ -3847,3 +3847,61 @@ static int s_rrs_request_subscriptions_remove_nonexistent_subscription_fn(struct AWS_TEST_CASE( rrs_request_subscriptions_remove_nonexistent_subscription, s_rrs_request_subscriptions_remove_nonexistent_subscription_fn) + +static int s_rrs_stream_and_request_subscriptions_add_same_subscription_fn(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_request_response_subscriptions subscriptions; + + aws_mqtt_request_response_client_subscriptions_init(&subscriptions, allocator); + + struct aws_byte_cursor topic1 = aws_byte_cursor_from_c_str("topic/123/abc"); + struct aws_byte_cursor topic2 = aws_byte_cursor_from_c_str("topic/123/+"); + struct aws_byte_cursor token_path1 = aws_byte_cursor_from_c_str("token1"); + struct aws_byte_cursor payload1 = aws_byte_cursor_from_c_str("Payload1"); + + aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic1); + aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic2); + aws_mqtt_request_response_client_subscriptions_add_request_subscription(&subscriptions, &topic1, &token_path1); + + struct aws_protocol_adapter_incoming_publish_event publish_event = { + .topic = topic1, + .payload = payload1, + }; + + struct aws_rr_client_fixture_subscriptions_matches_record *record = + s_aws_rr_client_fixture_subscriptions_matches_record_new(allocator); + + aws_mqtt_request_response_client_subscriptions_match( + &subscriptions, + &publish_event, + s_rrs_fixture_on_stream_operation_subscription_match, + s_rrs_fixture_on_request_operation_subscription_match, + record); + + struct aws_rr_client_fixture_matched_stream_subscription_view matched_stream_subscriptions[] = { + {payload1, topic1, topic1}, + {payload1, topic1, topic2}, + }; + + struct aws_rr_client_fixture_matched_request_subscription_view matched_request_subscriptions[] = { + {payload1, topic1, topic1, token_path1}, + }; + + ASSERT_SUCCESS(s_rrc_verify_subscriptions_publishes( + record, + AWS_ARRAY_SIZE(matched_stream_subscriptions), + matched_stream_subscriptions, + AWS_ARRAY_SIZE(matched_request_subscriptions), + matched_request_subscriptions)); + + s_aws_rr_client_fixture_subscriptions_macthes_record_delete(record); + + aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); + + return AWS_OP_SUCCESS; +} + +AWS_TEST_CASE( + rrs_stream_and_request_subscriptions_add_same_subscription, + s_rrs_stream_and_request_subscriptions_add_same_subscription_fn) From 4c621ca8028f6c230a19b72fc487d0156d717359 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Mon, 3 Feb 2025 12:46:07 -0800 Subject: [PATCH 24/29] fixup --- tests/request-response/request_response_client_tests.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/request-response/request_response_client_tests.c b/tests/request-response/request_response_client_tests.c index 038bf61c..19f932c7 100644 --- a/tests/request-response/request_response_client_tests.c +++ b/tests/request-response/request_response_client_tests.c @@ -3725,7 +3725,7 @@ static int s_rrs_request_subscriptions_remove_subscription_fn(struct aws_allocat record2); ASSERT_SUCCESS(s_rrc_verify_subscriptions_publishes( - record, 0 /* expected_stream_matches_count */, NULL, 0 /* expected_request_matches_count */, NULL)); + record2, 0 /* expected_stream_matches_count */, NULL, 0 /* expected_request_matches_count */, NULL)); s_aws_rr_client_fixture_subscriptions_macthes_record_delete(record2); From 41d5067cfb594f9da69e4a797c9f5b1c069b7163 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Mon, 3 Feb 2025 13:18:53 -0800 Subject: [PATCH 25/29] Deal with logs --- .../request_response_subscription_set.h | 4 +- .../request_response_client.c | 30 +++++++++-- .../request_response_subscription_set.c | 54 ++----------------- 3 files changed, 31 insertions(+), 57 deletions(-) diff --git a/include/aws/mqtt/private/request-response/request_response_subscription_set.h b/include/aws/mqtt/private/request-response/request_response_subscription_set.h index c9145f5d..fb5d7960 100644 --- a/include/aws/mqtt/private/request-response/request_response_subscription_set.h +++ b/include/aws/mqtt/private/request-response/request_response_subscription_set.h @@ -65,7 +65,7 @@ struct aws_rr_response_path_entry { */ typedef void(aws_mqtt_stream_operation_subscription_match_fn)( const struct aws_linked_list *operations, - const struct aws_byte_cursor *topic_filter, // TODO Do we need this for anything other than tests? + const struct aws_byte_cursor *topic_filter, const struct aws_protocol_adapter_incoming_publish_event *publish_event, void *user_data); @@ -113,7 +113,7 @@ AWS_MQTT_API int aws_mqtt_request_response_client_subscriptions_add_request_subs /* * Remove a subscription for a given request operation. */ -AWS_MQTT_API void aws_mqtt_request_response_client_subscriptions_remove_request_subscription( +AWS_MQTT_API int aws_mqtt_request_response_client_subscriptions_remove_request_subscription( struct aws_request_response_subscriptions *subscriptions, const struct aws_byte_cursor *topic_filter); diff --git a/source/request-response/request_response_client.c b/source/request-response/request_response_client.c index 5dbaf6cd..3fb2d691 100644 --- a/source/request-response/request_response_client.c +++ b/source/request-response/request_response_client.c @@ -265,8 +265,7 @@ struct aws_mqtt_request_response_client { struct aws_priority_queue operations_by_timeout; /* - * Structure to handle subscriptions: add/remove subscriptions, match incoming messages. - * TODO Add normal description. + * Structure to handle stream and request subscriptions. */ struct aws_request_response_subscriptions subscriptions; @@ -786,11 +785,19 @@ static void s_apply_publish_to_streaming_operation_list( const struct aws_byte_cursor *topic_filter, const struct aws_protocol_adapter_incoming_publish_event *publish_event, void *user_data) { - (void)topic_filter; - (void)user_data; AWS_FATAL_ASSERT(operations != NULL); + struct aws_mqtt_request_response_client *rr_client = user_data; + + AWS_LOGF_DEBUG( + AWS_LS_MQTT_REQUEST_RESPONSE, + "id=%p: request-response client incoming publish on topic '" PRInSTR + "' matches streaming subscription on topic filter '" PRInSTR "'", + (void *)rr_client, + AWS_BYTE_CURSOR_PRI(publish_event->topic), + AWS_BYTE_CURSOR_PRI(*topic_filter)); + struct aws_linked_list_node *node = aws_linked_list_begin(operations); while (node != aws_linked_list_end(operations)) { struct aws_mqtt_rr_client_operation *operation = @@ -883,6 +890,12 @@ static void s_apply_publish_to_response_path_entry( struct aws_mqtt_request_response_client *rr_client = user_data; + AWS_LOGF_DEBUG( + AWS_LS_MQTT_REQUEST_RESPONSE, + "id=%p: request-response client incoming publish on topic '" PRInSTR "' matches response path", + (void *)rr_client, + AWS_BYTE_CURSOR_PRI(publish_event->topic)); + struct aws_json_value *json_payload = NULL; struct aws_byte_cursor correlation_token; @@ -1698,7 +1711,14 @@ static void s_remove_operation_from_client_tables(struct aws_mqtt_rr_client_oper for (size_t i = 0; i < path_count; ++i) { struct aws_mqtt_request_operation_response_path path; aws_array_list_get_at(paths, &path, i); - aws_mqtt_request_response_client_subscriptions_remove_request_subscription(&client->subscriptions, &path.topic); + if (aws_mqtt_request_response_client_subscriptions_remove_request_subscription( + &client->subscriptions, &path.topic) == AWS_OP_ERR) { + AWS_LOGF_ERROR( + AWS_LS_MQTT_REQUEST_RESPONSE, + "id=%p: internal state error removing reference to response path for topic " PRInSTR, + (void *)client, + AWS_BYTE_CURSOR_PRI(path.topic)); + } } } diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index c77ef181..c0f00c30 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -190,7 +190,7 @@ int aws_mqtt_request_response_client_subscriptions_add_request_subscription( return AWS_OP_SUCCESS; } -void aws_mqtt_request_response_client_subscriptions_remove_request_subscription( +int aws_mqtt_request_response_client_subscriptions_remove_request_subscription( struct aws_request_response_subscriptions *subscriptions, const struct aws_byte_cursor *topic_filter) { @@ -199,29 +199,17 @@ void aws_mqtt_request_response_client_subscriptions_remove_request_subscription( struct aws_hash_element *element = NULL; if (aws_hash_table_find(&subscriptions->request_response_paths, topic_filter, &element) || element == NULL) { - AWS_LOGF_ERROR( - AWS_LS_MQTT_REQUEST_RESPONSE, - "internal state error removing reference to response path for topic " PRInSTR, - AWS_BYTE_CURSOR_PRI(*topic_filter)); - return; + return AWS_OP_ERR; } struct aws_rr_response_path_entry *entry = element->value; --entry->ref_count; if (entry->ref_count == 0) { - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "removing last reference to response path for topic " PRInSTR, - AWS_BYTE_CURSOR_PRI(*topic_filter)); aws_hash_table_remove(&subscriptions->request_response_paths, topic_filter, NULL, NULL); - } else { - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "removing reference to response path for topic " PRInSTR ", %zu references remain", - AWS_BYTE_CURSOR_PRI(*topic_filter), - entry->ref_count); } + + return AWS_OP_SUCCESS; } static void s_match_stream_subscriptions( @@ -232,11 +220,6 @@ static void s_match_stream_subscriptions( struct aws_hash_element *subscription_filter_element = NULL; if (aws_hash_table_find(subscriptions, &publish_event->topic, &subscription_filter_element) == AWS_OP_SUCCESS && subscription_filter_element != NULL) { - // TODO Deal with logs without client pointer. - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "request-response client incoming publish on topic '" PRInSTR "' matches streaming topic", - AWS_BYTE_CURSOR_PRI(publish_event->topic)); struct aws_rr_operation_list_topic_filter_entry *entry = subscription_filter_element->value; on_stream_operation_subscription_match( @@ -250,18 +233,9 @@ static void s_match_wildcard_stream_subscriptions( aws_mqtt_stream_operation_subscription_match_fn *on_stream_operation_subscription_match, void *user_data) { - AWS_LOGF_INFO( - AWS_LS_MQTT_REQUEST_RESPONSE, - "= Looking for subscription for topic '" PRInSTR "'", - AWS_BYTE_CURSOR_PRI(publish_event->topic)); - for (struct aws_hash_iter iter = aws_hash_iter_begin(subscriptions); !aws_hash_iter_done(&iter); aws_hash_iter_next(&iter)) { struct aws_rr_operation_list_topic_filter_entry *entry = iter.element.value; - AWS_LOGF_INFO( - AWS_LS_MQTT_REQUEST_RESPONSE, - "= Checking subscription with topic filter " PRInSTR, - AWS_BYTE_CURSOR_PRI(entry->topic_filter_cursor)); struct aws_byte_cursor subscription_topic_filter_segment; AWS_ZERO_STRUCT(subscription_topic_filter_segment); @@ -273,22 +247,11 @@ static void s_match_wildcard_stream_subscriptions( bool multi_level_wildcard = false; while (aws_byte_cursor_next_split(&entry->topic_filter_cursor, '/', &subscription_topic_filter_segment)) { - AWS_LOGF_INFO( - AWS_LS_MQTT_REQUEST_RESPONSE, - "=== subscription topic filter segment is '" PRInSTR "'", - AWS_BYTE_CURSOR_PRI(subscription_topic_filter_segment)); - if (!aws_byte_cursor_next_split(&publish_event->topic, '/', &topic_segment)) { - AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== topic segment is NULL"); match = false; break; } - AWS_LOGF_INFO( - AWS_LS_MQTT_REQUEST_RESPONSE, - "======= topic segment is '" PRInSTR "'", - AWS_BYTE_CURSOR_PRI(topic_segment)); - if (aws_byte_cursor_eq_c_str(&subscription_topic_filter_segment, "#")) { multi_level_wildcard = true; match = true; @@ -297,7 +260,6 @@ static void s_match_wildcard_stream_subscriptions( if (!aws_byte_cursor_eq_c_str(&subscription_topic_filter_segment, "+") && !aws_byte_cursor_eq_ignore_case(&topic_segment, &subscription_topic_filter_segment)) { - AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "======= topic segment differs"); match = false; break; } @@ -308,11 +270,8 @@ static void s_match_wildcard_stream_subscriptions( } if (match) { - AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== found subscription match"); on_stream_operation_subscription_match( &entry->operations, &entry->topic_filter_cursor, publish_event, user_data); - } else { - AWS_LOGF_INFO(AWS_LS_MQTT_REQUEST_RESPONSE, "=== this is not the right subscription"); } } } @@ -326,10 +285,6 @@ void s_match_request_response_subscriptions( struct aws_hash_element *response_path_element = NULL; if (aws_hash_table_find(request_response_paths, &publish_event->topic, &response_path_element) == AWS_OP_SUCCESS && response_path_element != NULL) { - AWS_LOGF_DEBUG( - AWS_LS_MQTT_REQUEST_RESPONSE, - "request-response client incoming publish on topic '" PRInSTR "' matches response path", - AWS_BYTE_CURSOR_PRI(publish_event->topic)); on_request_operation_subscription_match(response_path_element->value, publish_event, user_data); } @@ -344,7 +299,6 @@ void aws_mqtt_request_response_client_subscriptions_match( AWS_FATAL_PRECONDITION(subscriptions); AWS_FATAL_PRECONDITION(publish_event); - // TODO ? Allow NULLs? AWS_FATAL_PRECONDITION(on_stream_operation_subscription_match); AWS_FATAL_PRECONDITION(on_request_operation_subscription_match); From 386e9919415e5ded122bd13a912e2d139b916ce1 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Mon, 3 Feb 2025 13:44:17 -0800 Subject: [PATCH 26/29] Fix comments and style --- .../request_response_subscription_set.h | 14 ++++++++++---- .../request_response_subscription_set.c | 17 +++++++++++------ 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/include/aws/mqtt/private/request-response/request_response_subscription_set.h b/include/aws/mqtt/private/request-response/request_response_subscription_set.h index fb5d7960..3f60d359 100644 --- a/include/aws/mqtt/private/request-response/request_response_subscription_set.h +++ b/include/aws/mqtt/private/request-response/request_response_subscription_set.h @@ -11,25 +11,31 @@ #include #include -/* Handles subscriptions for request-response client. */ +/* + * Handles subscriptions for request-response client. + * Lifetime of this struct is bound to request-response client. + */ struct aws_request_response_subscriptions { struct aws_allocator *allocator; /* * Map from cursor (topic filter) -> list of streaming operations using that filter + * + * We don't garbage collect this table over the course of normal client operation. We only clean it up + * when the client is shutting down. */ struct aws_hash_table streaming_operation_subscription_lists; /* * Map from cursor (topic filter with wildcards) -> list of streaming operations using that filter + * + * We don't garbage collect this table over the course of normal client operation. We only clean it up + * when the client is shutting down. */ struct aws_hash_table streaming_operation_wildcards_subscription_lists; /* * Map from cursor (topic) -> request response path (topic, correlation token json path) - * - * We don't garbage collect this table over the course of normal client operation. We only clean it up - * when the client is shutting down. */ struct aws_hash_table request_response_paths; }; diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index c0f00c30..ea0e7bd1 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -233,6 +233,11 @@ static void s_match_wildcard_stream_subscriptions( aws_mqtt_stream_operation_subscription_match_fn *on_stream_operation_subscription_match, void *user_data) { + /* + * Incoming event's topic is checked against each registered stream with wildcard. While this approach is far from + * optimal, it should be sufficient for request-response client where not many subscriptions with wildcards are + * used. + */ for (struct aws_hash_iter iter = aws_hash_iter_begin(subscriptions); !aws_hash_iter_done(&iter); aws_hash_iter_next(&iter)) { struct aws_rr_operation_list_topic_filter_entry *entry = iter.element.value; @@ -252,17 +257,17 @@ static void s_match_wildcard_stream_subscriptions( break; } - if (aws_byte_cursor_eq_c_str(&subscription_topic_filter_segment, "#")) { - multi_level_wildcard = true; - match = true; - break; - } - if (!aws_byte_cursor_eq_c_str(&subscription_topic_filter_segment, "+") && !aws_byte_cursor_eq_ignore_case(&topic_segment, &subscription_topic_filter_segment)) { match = false; break; } + + if (aws_byte_cursor_eq_c_str(&subscription_topic_filter_segment, "#")) { + multi_level_wildcard = true; + match = true; + break; + } } if (!multi_level_wildcard && aws_byte_cursor_next_split(&publish_event->topic, '/', &topic_segment)) { From af67f9c1ee4db5d858826757a492a647e129f572 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Mon, 3 Feb 2025 13:44:29 -0800 Subject: [PATCH 27/29] More tests --- tests/CMakeLists.txt | 2 + .../request_response_client_tests.c | 93 +++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1cb3d0e1..6dbe3a12 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -582,6 +582,8 @@ add_test_case(rrs_init_cleanup) add_test_case(rrs_stream_subscriptions_match_single_level_wildcards) add_test_case(rrs_stream_subscriptions_match_multi_level_wildcards) add_test_case(rrs_stream_subscriptions_add_duplicate) +add_test_case(rrs_stream_subscriptions_too_long_publish_topic) +add_test_case(rrs_stream_subscriptions_too_short_publish_topic) add_test_case(rrs_request_subscriptions_add_single_subscription) add_test_case(rrs_request_subscriptions_remove_subscription) add_test_case(rrs_request_subscriptions_add_duplicate_then_remove) diff --git a/tests/request-response/request_response_client_tests.c b/tests/request-response/request_response_client_tests.c index 19f932c7..5f11ccb9 100644 --- a/tests/request-response/request_response_client_tests.c +++ b/tests/request-response/request_response_client_tests.c @@ -3623,6 +3623,99 @@ static int s_rrs_stream_subscriptions_add_duplicate_fn(struct aws_allocator *all AWS_TEST_CASE(rrs_stream_subscriptions_add_duplicate, s_rrs_stream_subscriptions_add_duplicate_fn) +static int s_rrs_stream_subscriptions_too_long_publish_topic_fn(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_request_response_subscriptions subscriptions; + + aws_mqtt_request_response_client_subscriptions_init(&subscriptions, allocator); + + struct aws_byte_cursor topic_filter1 = aws_byte_cursor_from_c_str("topic/123/+"); + + struct aws_byte_cursor topic1 = aws_byte_cursor_from_c_str("topic/123/abc/def"); + struct aws_byte_cursor payload1 = aws_byte_cursor_from_c_str("Payload1"); + + aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter1); + aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter1); + + struct aws_protocol_adapter_incoming_publish_event publish_event = { + .topic = topic1, + .payload = payload1, + }; + + struct aws_rr_client_fixture_subscriptions_matches_record *record = + s_aws_rr_client_fixture_subscriptions_matches_record_new(allocator); + + aws_mqtt_request_response_client_subscriptions_match( + &subscriptions, + &publish_event, + s_rrs_fixture_on_stream_operation_subscription_match, + s_rrs_fixture_on_request_operation_subscription_match, + record); + + ASSERT_SUCCESS(s_rrc_verify_subscriptions_publishes( + record, + 0, /* expected_stream_matches_count */ + NULL, + 0, /* expected_request_matches_count */ + NULL)); + + s_aws_rr_client_fixture_subscriptions_macthes_record_delete(record); + + aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); + + return AWS_OP_SUCCESS; +} + +AWS_TEST_CASE(rrs_stream_subscriptions_too_long_publish_topic, s_rrs_stream_subscriptions_too_long_publish_topic_fn) + +static int s_rrs_stream_subscriptions_too_short_publish_topic_fn(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct aws_request_response_subscriptions subscriptions; + + aws_mqtt_request_response_client_subscriptions_init(&subscriptions, allocator); + + struct aws_byte_cursor topic_filter1 = aws_byte_cursor_from_c_str("topic/123/+"); + + struct aws_byte_cursor topic1 = aws_byte_cursor_from_c_str("topic/123"); + struct aws_byte_cursor payload1 = aws_byte_cursor_from_c_str("Payload1"); + + aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter1); + aws_mqtt_request_response_client_subscriptions_add_stream_subscription(&subscriptions, &topic_filter1); + + struct aws_protocol_adapter_incoming_publish_event publish_event = { + .topic = topic1, + .payload = payload1, + }; + + struct aws_rr_client_fixture_subscriptions_matches_record *record = + s_aws_rr_client_fixture_subscriptions_matches_record_new(allocator); + + aws_mqtt_request_response_client_subscriptions_match( + &subscriptions, + &publish_event, + s_rrs_fixture_on_stream_operation_subscription_match, + s_rrs_fixture_on_request_operation_subscription_match, + record); + + ASSERT_SUCCESS(s_rrc_verify_subscriptions_publishes( + record, + 0, /* expected_stream_matches_count */ + NULL, + 0, /* expected_request_matches_count */ + NULL)); + + + s_aws_rr_client_fixture_subscriptions_macthes_record_delete(record); + + aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); + + return AWS_OP_SUCCESS; +} + +AWS_TEST_CASE(rrs_stream_subscriptions_too_short_publish_topic, s_rrs_stream_subscriptions_too_short_publish_topic_fn) + static int s_rrs_request_subscriptions_add_single_subscription_fn(struct aws_allocator *allocator, void *ctx) { (void)ctx; From 1bf1b8bc28709da23c84e5d991ea74d9a0ea166f Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Mon, 3 Feb 2025 13:49:31 -0800 Subject: [PATCH 28/29] format --- tests/request-response/request_response_client_tests.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/request-response/request_response_client_tests.c b/tests/request-response/request_response_client_tests.c index 5f11ccb9..5cbb7405 100644 --- a/tests/request-response/request_response_client_tests.c +++ b/tests/request-response/request_response_client_tests.c @@ -3706,7 +3706,6 @@ static int s_rrs_stream_subscriptions_too_short_publish_topic_fn(struct aws_allo 0, /* expected_request_matches_count */ NULL)); - s_aws_rr_client_fixture_subscriptions_macthes_record_delete(record); aws_mqtt_request_response_client_subscriptions_clean_up(&subscriptions); From 061731fc16066d3899cc281aeaeb8c171a525714 Mon Sep 17 00:00:00 2001 From: Igor Abdrakhimov Date: Mon, 3 Feb 2025 14:30:21 -0800 Subject: [PATCH 29/29] fixup --- .../request_response_subscription_set.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/source/request-response/request_response_subscription_set.c b/source/request-response/request_response_subscription_set.c index ea0e7bd1..48403cfe 100644 --- a/source/request-response/request_response_subscription_set.c +++ b/source/request-response/request_response_subscription_set.c @@ -257,17 +257,17 @@ static void s_match_wildcard_stream_subscriptions( break; } - if (!aws_byte_cursor_eq_c_str(&subscription_topic_filter_segment, "+") && - !aws_byte_cursor_eq_ignore_case(&topic_segment, &subscription_topic_filter_segment)) { - match = false; - break; - } - if (aws_byte_cursor_eq_c_str(&subscription_topic_filter_segment, "#")) { multi_level_wildcard = true; match = true; break; } + + if (!aws_byte_cursor_eq_c_str(&subscription_topic_filter_segment, "+") && + !aws_byte_cursor_eq_ignore_case(&topic_segment, &subscription_topic_filter_segment)) { + match = false; + break; + } } if (!multi_level_wildcard && aws_byte_cursor_next_split(&publish_event->topic, '/', &topic_segment)) {