From 30217e764d53ecb5dd1c568c84163c9b1c3e7df9 Mon Sep 17 00:00:00 2001 From: Maxim Olender Date: Thu, 14 Jan 2016 19:17:53 +0200 Subject: [PATCH 01/18] KAA-773 Add interfaces, stubs and documentation --- .../client-c/src/kaa/kaa_logging.c | 10 +++-- .../client-c/src/kaa/kaa_logging.h | 45 +++++++++++++++++-- .../client-multi/client-c/test/test_kaa_log.c | 37 ++++++++------- 3 files changed, 66 insertions(+), 26 deletions(-) diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.c b/client/client-multi/client-c/src/kaa/kaa_logging.c index 3901827ced..0238385367 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.c +++ b/client/client-multi/client-c/src/kaa/kaa_logging.c @@ -71,7 +71,7 @@ struct kaa_log_collector { -static const kaa_service_t logging_sync_services[1] = {KAA_SERVICE_LOGGING}; +static const kaa_service_t logging_sync_services[] = {KAA_SERVICE_LOGGING}; kaa_error_t kaa_logging_need_logging_resync(kaa_log_collector_t *self, bool *result) @@ -263,7 +263,7 @@ static void update_storage(kaa_log_collector_t *self) -kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_record_t *entry) +kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_record_t *entry, kaa_log_bucket_info_t *bucket) { KAA_RETURN_IF_NIL2(self, entry, KAA_ERR_BADPARAM); KAA_RETURN_IF_NIL(self->log_storage_context, KAA_ERR_NOT_INITIALIZED); @@ -304,7 +304,11 @@ kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_recor return KAA_ERR_NONE; } - +kaa_error_t kaa_logging_set_listeners(kaa_log_collector_t *self, const kaa_log_listeners_t *listeners) +{ + /* TODO */ + return KAA_ERR_UNSUPPORTED; +} kaa_error_t kaa_logging_request_get_size(kaa_log_collector_t *self, size_t *expected_size) { diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.h b/client/client-multi/client-c/src/kaa/kaa_logging.h index cc311c0d42..154f351cc9 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.h +++ b/client/client-multi/client-c/src/kaa/kaa_logging.h @@ -42,6 +42,32 @@ extern "C" { typedef struct kaa_log_collector kaa_log_collector_t; #endif +/** Log bucket information structure. */ +typedef struct +{ + uint16_t bucket_id; /**< ID of bucket present in storage. */ + size_t log_cnt; /**< Current logs count. */ +} kaa_log_bucket_info_t; + +/** + * @brief Event handler type. + * + * @param[in,out] ctx User-definied context. @sa kaa_logging_add_record + * @param[in] bucket Log bucket for which event was triggered. + */ +typedef void (*kaa_log_event_fn)(void *ctx, const kaa_log_bucket_info_t *bucket); + +/** Listeners aggreate */ +typedef struct +{ + kaa_log_event_fn on_success; /**< Handler called upon successfull log delivery. */ + kaa_log_event_fn on_failed; /**< Handler called upon failed delivery. */ + kaa_log_event_fn on_timeout; /**< Handler called upon timeouted delivery. */ + void *ctx; /**< User-defined context. */ +} kaa_log_listeners_t; + +/** Special macro that can be used to disable event handling. */ +#define KAA_LOG_EMPTY_LISTENERS ((kaa_log_listener_t){NULL, NULL, NULL, NULL}) /** * @brief Initializes data collection module with the storage interface, upload strategy, and other settings. @@ -54,16 +80,27 @@ extern "C" { */ kaa_error_t kaa_logging_init(kaa_log_collector_t *self, void *log_storage_context, void *log_upload_strategy_context); - /** * @brief Serializes and adds a log record to the log storage. * - * @param[in] self Pointer to a @link kaa_log_collector_t @endlink instance. - * @param[in] entry Pointer to log entry to be added to the storage. + * @param[in] self Pointer to a @link kaa_log_collector_t @endlink instance. + * @param[in] entry Pointer to log entry to be added to the storage. + * @param[out] bucket Pointer to log bucket info. May be NULL. + * + * @return Error code. + */ +kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_record_t *entry, kaa_log_bucket_info_t *bucket); + +/** + * @brief Sets listeners of log events. * + * @param[in] self Pointer to a @link kaa_log_collector_t @endlink instance. + * @param[in] listeners Pointer to listeners that will be used to handle + * various log delivery events. @sa KAA_LOG_EMPTY_LISTENERS + * can be used to unsubscribe from log events. * @return Error code. */ -kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_record_t *entry); +kaa_error_t kaa_logging_set_listeners(kaa_log_collector_t *self, const kaa_log_listeners_t *listeners); #ifdef __cplusplus } /* extern "C" */ diff --git a/client/client-multi/client-c/test/test_kaa_log.c b/client/client-multi/client-c/test/test_kaa_log.c index 218507d9bb..27e2e3f358 100644 --- a/client/client-multi/client-c/test/test_kaa_log.c +++ b/client/client-multi/client-c/test/test_kaa_log.c @@ -334,7 +334,7 @@ void test_create_request(void) error_code = kaa_logging_init(log_collector, create_mock_storage(), &strategy); ASSERT_EQUAL(error_code, KAA_ERR_NONE); - error_code = kaa_logging_add_record(log_collector, test_log_record); + error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); ASSERT_EQUAL(error_code, KAA_ERR_NONE); size_t expected_size = 0; @@ -473,7 +473,7 @@ void test_timeout(void) error_code = kaa_logging_init(log_collector, create_mock_storage(), &strategy); ASSERT_EQUAL(error_code, KAA_ERR_NONE); - error_code = kaa_logging_add_record(log_collector, test_log_record); + error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); ASSERT_EQUAL(error_code, KAA_ERR_NONE); size_t request_buffer_size = 256; @@ -487,7 +487,7 @@ void test_timeout(void) sleep(TEST_TIMEOUT + 1); - error_code = kaa_logging_add_record(log_collector, test_log_record); + error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); ASSERT_EQUAL(error_code, KAA_ERR_NONE); ASSERT_NOT_NULL(strategy.on_timeout_count); @@ -528,7 +528,7 @@ void test_decline_timeout(void) error_code = kaa_logging_init(log_collector, storage, &strategy); ASSERT_EQUAL(error_code, KAA_ERR_NONE); - error_code = kaa_logging_add_record(log_collector, test_log_record); + error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); ASSERT_EQUAL(error_code, KAA_ERR_NONE); size_t request_buffer_size = 256; @@ -570,7 +570,7 @@ void test_decline_timeout(void) ASSERT_EQUAL(error_code, KAA_ERR_NONE); ASSERT_NOT_NULL(storage->on_remove_by_id_count); - error_code = kaa_logging_add_record(log_collector, test_log_record); + error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); ASSERT_EQUAL(error_code, KAA_ERR_NONE); ASSERT_NULL(strategy.on_timeout_count); @@ -620,7 +620,7 @@ void test_max_parallel_uploads_with_log_sync(void) * Ensure the log delivery is forbidden at all. */ strategy.max_parallel_uploads = 0; - error_code = kaa_logging_add_record(log_collector, test_log_record); + error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); ASSERT_EQUAL(error_code, KAA_ERR_NONE); ASSERT_EQUAL(((mock_transport_channel_context_t *)transport_context.context)->on_sync_count, 0); @@ -629,7 +629,7 @@ void test_max_parallel_uploads_with_log_sync(void) * Ensure the first request is allowed. */ strategy.max_parallel_uploads = 1; - error_code = kaa_logging_add_record(log_collector, test_log_record); + error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); ASSERT_EQUAL(error_code, KAA_ERR_NONE); ASSERT_EQUAL(((mock_transport_channel_context_t *)transport_context.context)->on_sync_count, 1); @@ -645,7 +645,7 @@ void test_max_parallel_uploads_with_log_sync(void) error_code = kaa_logging_request_serialize(log_collector, writer); ASSERT_EQUAL(error_code, KAA_ERR_NONE); - error_code = kaa_logging_add_record(log_collector, test_log_record); + error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); ASSERT_EQUAL(error_code, KAA_ERR_NONE); /* @@ -701,7 +701,7 @@ void test_max_parallel_uploads_with_sync_all(void) * Ensure the log delivery is forbidden at all. */ strategy.max_parallel_uploads = 0; - error_code = kaa_logging_add_record(log_collector, test_log_record); + error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); ASSERT_EQUAL(error_code, KAA_ERR_NONE); size_t expected_size = 0; @@ -712,7 +712,7 @@ void test_max_parallel_uploads_with_sync_all(void) * Ensure the first request is allowed. */ strategy.max_parallel_uploads = 1; - error_code = kaa_logging_add_record(log_collector, test_log_record); + error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); ASSERT_EQUAL(error_code, KAA_ERR_NONE); /* @@ -729,7 +729,7 @@ void test_max_parallel_uploads_with_sync_all(void) error_code = kaa_logging_request_serialize(log_collector, writer); ASSERT_EQUAL(error_code, KAA_ERR_NONE); - error_code = kaa_logging_add_record(log_collector, test_log_record); + error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); ASSERT_EQUAL(error_code, KAA_ERR_NONE); /* @@ -788,15 +788,14 @@ int test_deinit(void) } - KAA_SUITE_MAIN(Log, test_init, test_deinit #ifndef KAA_DISABLE_FEATURE_LOGGING - , - KAA_TEST_CASE(create_request, test_create_request) - KAA_TEST_CASE(process_response, test_response) - KAA_TEST_CASE(process_timeout, test_timeout) - KAA_TEST_CASE(decline_timeout, test_decline_timeout) - KAA_TEST_CASE(max_parallel_uploads_with_log_sync, test_max_parallel_uploads_with_log_sync) - KAA_TEST_CASE(max_parallel_uploads_with_sync_all, test_max_parallel_uploads_with_sync_all) + , + KAA_TEST_CASE(create_request, test_create_request) + KAA_TEST_CASE(process_response, test_response) + KAA_TEST_CASE(process_timeout, test_timeout) + KAA_TEST_CASE(decline_timeout, test_decline_timeout) + KAA_TEST_CASE(max_parallel_uploads_with_log_sync, test_max_parallel_uploads_with_log_sync) + KAA_TEST_CASE(max_parallel_uploads_with_sync_all, test_max_parallel_uploads_with_sync_all) #endif ) From 6ae00c780b332fd6f826c9998e5c41cb7a1ae817 Mon Sep 17 00:00:00 2001 From: Maxim Olender Date: Thu, 14 Jan 2016 19:19:36 +0200 Subject: [PATCH 02/18] KAA-773 Extend test framework with useful macros Now it is possible to setup per-test fixture based on group in which it included. --- client/client-multi/client-c/test/kaa_test.h | 45 +++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/client/client-multi/client-c/test/kaa_test.h b/client/client-multi/client-c/test/kaa_test.h index 80fba57a50..85c0cea057 100644 --- a/client/client-multi/client-c/test/kaa_test.h +++ b/client/client-multi/client-c/test/kaa_test.h @@ -96,7 +96,6 @@ typedef int (*cleanup_fn)(void); #endif - #define KAA_SUITE_MAIN(SUITE_NAME, INIT_FN, CLEANUP_FN, ...) \ KAA_BEGIN_TEST_SUITE(SUITE_NAME, INIT_FN, CLEANUP_FN) \ __VA_ARGS__ \ @@ -104,4 +103,48 @@ typedef int (*cleanup_fn)(void); KAA_END_TEST_SUITE \ +/* Bunch of macroses that required execute setup() (initialization) + * and teardown() (cleanup) procedures per each test in so called "group". + * + * Group is a set of tests with common setup() and teardown() routines. + * You may have as many groups as you want inside test application. + * This contrasts with suite, which can be only one per each executable. + * + * NOTE: THIS IS INTERMIDIATE SOLUTION THAT WILL BE USED PRIOR TO INTEGRATION + * OF APPROPRIATE TEST FRAMEWORK WHICH SUPPORTS SUCH FEATURES. + */ + +/* Defines test case in the given group */ +#define KAA_TEST_CASE_EX(GROUP, NAME) \ + void GROUP##_##NAME##_test(void) + +/* Helper macro to control setup and teardown process per each test in group. + * Must be placed in the exact suite. + */ +#define KAA_RUN_TEST(GROUP, NAME) \ + do { \ + if (!init_ret_code) { \ + int rc = GROUP##_group_setup(); \ + ASSERT_EQUAL(0, rc); \ + GROUP##_##NAME##_test(); \ + rc = GROUP##_group_teardown(); \ + ASSERT_EQUAL(0, rc); \ + } \ + } while (0) + +/* Defines a setup process for given group. + * It runs before each test to make sure sytem is in predictable state + */ +#define KAA_GROUP_SETUP(GROUP) \ + int GROUP##_group_setup() + +/* Defines a teardown process for given group +/* Reverts any changes made by setup routine and makes sure no side effects + * will stay after test + */ +#define KAA_GROUP_TEARDOWN(GROUP) \ + int GROUP##_group_teardown() + + + #endif /* KAA_TEST_H_ */ From abaf02d9a700c9c8c1e5c776f4f141f23c7b187f Mon Sep 17 00:00:00 2001 From: Oleksandr Burdeinyi Date: Thu, 14 Jan 2016 20:25:51 +0200 Subject: [PATCH 03/18] KAA-773 renamed timeout info in bucket info; set log delivery listeners --- client/client-multi/client-c/src/kaa/kaa_logging.c | 14 +++++++++----- client/client-multi/client-c/src/kaa/kaa_logging.h | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.c b/client/client-multi/client-c/src/kaa/kaa_logging.c index 0238385367..e9bcefc505 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.c +++ b/client/client-multi/client-c/src/kaa/kaa_logging.c @@ -53,11 +53,14 @@ typedef enum { LOGGING_RESULT_FAILURE = 0x01 } logging_sync_result_t; -typedef struct { - uint16_t log_bucket_id; +typedef struct +{ + uint16_t log_bucket_id; /**< ID of bucket present in storage. */ kaa_time_t timeout; + size_t log_cnt; /**< Current logs count. */ } timeout_info_t; + struct kaa_log_collector { uint16_t log_bucket_id; void *log_storage_context; @@ -66,6 +69,7 @@ struct kaa_log_collector { kaa_channel_manager_t *channel_manager; kaa_logger_t *logger; kaa_list_t *timeouts; + kaa_log_listeners_t log_delivery_listeners; bool is_sync_ignored; }; @@ -263,7 +267,7 @@ static void update_storage(kaa_log_collector_t *self) -kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_record_t *entry, kaa_log_bucket_info_t *bucket) +kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_record_t *entry, uint16_t *bucket_id) { KAA_RETURN_IF_NIL2(self, entry, KAA_ERR_BADPARAM); KAA_RETURN_IF_NIL(self->log_storage_context, KAA_ERR_NOT_INITIALIZED); @@ -306,8 +310,8 @@ kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_recor kaa_error_t kaa_logging_set_listeners(kaa_log_collector_t *self, const kaa_log_listeners_t *listeners) { - /* TODO */ - return KAA_ERR_UNSUPPORTED; + self->log_delivery_listeners = *listeners; + return KAA_ERR_NONE; } kaa_error_t kaa_logging_request_get_size(kaa_log_collector_t *self, size_t *expected_size) diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.h b/client/client-multi/client-c/src/kaa/kaa_logging.h index 154f351cc9..facb7242bf 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.h +++ b/client/client-multi/client-c/src/kaa/kaa_logging.h @@ -89,7 +89,7 @@ kaa_error_t kaa_logging_init(kaa_log_collector_t *self, void *log_storage_contex * * @return Error code. */ -kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_record_t *entry, kaa_log_bucket_info_t *bucket); +kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_record_t *entry, uint16_t *bucket_id); /** * @brief Sets listeners of log events. From ce16b5dadf1edf646b21c7b24ffc54e87f1b8396 Mon Sep 17 00:00:00 2001 From: Oleksandr Burdeinyi Date: Thu, 14 Jan 2016 20:54:28 +0200 Subject: [PATCH 04/18] KAA-773 save record count in remember_request --- client/client-multi/client-c/src/kaa/kaa_logging.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.c b/client/client-multi/client-c/src/kaa/kaa_logging.c index e9bcefc505..dcdd982aee 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.c +++ b/client/client-multi/client-c/src/kaa/kaa_logging.c @@ -55,9 +55,9 @@ typedef enum { typedef struct { - uint16_t log_bucket_id; /**< ID of bucket present in storage. */ - kaa_time_t timeout; - size_t log_cnt; /**< Current logs count. */ + uint16_t log_bucket_id; /**< ID of bucket present in storage. */ + kaa_time_t timeout; /**< bucket timeout */ + uint16_t log_cnt; /**< Current logs count. */ } timeout_info_t; @@ -89,7 +89,7 @@ kaa_error_t kaa_logging_need_logging_resync(kaa_log_collector_t *self, bool *res return KAA_ERR_NONE; } -static kaa_error_t remember_request(kaa_log_collector_t *self, uint16_t bucket_id) +static kaa_error_t remember_request(kaa_log_collector_t *self, uint16_t bucket_id, uint16_t cnt) { KAA_RETURN_IF_NIL(self, KAA_ERR_BADPARAM); @@ -98,6 +98,7 @@ static kaa_error_t remember_request(kaa_log_collector_t *self, uint16_t bucket_i info->log_bucket_id = bucket_id; info->timeout = KAA_TIME() + (kaa_time_t)ext_log_upload_strategy_get_timeout(self->log_upload_strategy_context); + info->log_cnt = cnt; kaa_list_node_t *it = kaa_list_push_back(self->timeouts, info); if (!it) { @@ -422,7 +423,7 @@ kaa_error_t kaa_logging_request_serialize(kaa_log_collector_t *self, kaa_platfor *((uint16_t *) records_count_p) = KAA_HTONS(records_count); *writer = tmp_writer; - error = remember_request(self, self->log_bucket_id); + error = remember_request(self, self->log_bucket_id, records_count); if (error) { KAA_LOG_WARN(self->logger, error, "Failed to remember request time stamp"); } From 39ea108384f32e29c77e6dac4422e6661df8059d Mon Sep 17 00:00:00 2001 From: Oleksandr Burdeinyi Date: Thu, 14 Jan 2016 21:33:18 +0200 Subject: [PATCH 05/18] KKA-773 return expected bucket id --- client/client-multi/client-c/src/kaa/kaa_logging.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.c b/client/client-multi/client-c/src/kaa/kaa_logging.c index dcdd982aee..18962bf191 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.c +++ b/client/client-multi/client-c/src/kaa/kaa_logging.c @@ -306,6 +306,13 @@ kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_recor if (!is_timeout(self)) update_storage(self); + if (!self->log_bucket_id) { + self->log_bucket_id = self->status->log_bucket_id; + } + // TODO Check that we doesn't have policy that avoid log record from sending in next bucket + *bucket_id = self->log_bucket_id; + ++*bucket_id; + return KAA_ERR_NONE; } From 9048aab727786d6464a2a91aa07492d0e202ab89 Mon Sep 17 00:00:00 2001 From: Oleksandr Burdeinyi Date: Thu, 14 Jan 2016 22:46:33 +0200 Subject: [PATCH 06/18] KAA-773 Call listeners for success, timeout and fail --- .../client-multi/client-c/src/kaa/kaa_logging.c | 15 +++++++++++++++ .../client-multi/client-c/src/kaa/kaa_logging.h | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.c b/client/client-multi/client-c/src/kaa/kaa_logging.c index 18962bf191..14f9fb93a0 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.c +++ b/client/client-multi/client-c/src/kaa/kaa_logging.c @@ -151,6 +151,10 @@ static bool is_timeout(kaa_log_collector_t *self) while (it) { timeout_info_t *info = (timeout_info_t *)kaa_list_get_data(it); ext_log_storage_unmark_by_bucket_id(self->log_storage_context, info->log_bucket_id); + if (self->log_delivery_listeners.on_timeout){ + kaa_log_bucket_info_t log_bucket_info = {info->log_bucket_id, info->log_cnt}; + (*self->log_delivery_listeners.on_timeout)(self->log_delivery_listeners.ctx, &log_bucket_info); + } it = kaa_list_next(it); } @@ -228,6 +232,7 @@ kaa_error_t kaa_logging_init(kaa_log_collector_t *self, void *log_storage_contex self->log_storage_context = log_storage_context; self->log_upload_strategy_context = log_upload_strategy_context; + self->log_delivery_listeners = KAA_LOG_EMPTY_LISTENERS; KAA_LOG_DEBUG(self->logger, KAA_ERR_NONE, "Initialized log collector with log storage {%p}, log upload strategy {%p}" , log_storage_context, log_upload_strategy_context); @@ -473,8 +478,18 @@ kaa_error_t kaa_logging_handle_server_sync(kaa_log_collector_t *self if (delivery_result == LOGGING_RESULT_SUCCESS) { KAA_LOG_INFO(self->logger, KAA_ERR_NONE, "Log bucket uploaded successfully, id '%u'", bucket_id); + if (self->log_delivery_listeners.on_success){ + // TODO load log count from timeouts + kaa_log_bucket_info_t log_bucket_info = {bucket_id, 0}; + (*self->log_delivery_listeners.on_success)(self->log_delivery_listeners.ctx,&log_bucket_info); + } } else { KAA_LOG_WARN(self->logger, KAA_ERR_WRITE_FAILED, "Failed to upload log bucket, id '%u' (delivery error code '%u')", bucket_id, delivery_error_code); + if (self->log_delivery_listeners.on_failed){ + // TODO load log count from timeouts + kaa_log_bucket_info_t log_bucket_info = {bucket_id, 0}; + (*self->log_delivery_listeners.on_failed)(self->log_delivery_listeners.ctx, &log_bucket_info); + } } remove_request(self, bucket_id); diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.h b/client/client-multi/client-c/src/kaa/kaa_logging.h index facb7242bf..8674e19e4c 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.h +++ b/client/client-multi/client-c/src/kaa/kaa_logging.h @@ -67,7 +67,7 @@ typedef struct } kaa_log_listeners_t; /** Special macro that can be used to disable event handling. */ -#define KAA_LOG_EMPTY_LISTENERS ((kaa_log_listener_t){NULL, NULL, NULL, NULL}) +#define KAA_LOG_EMPTY_LISTENERS ((kaa_log_listeners_t){NULL, NULL, NULL, NULL}) /** * @brief Initializes data collection module with the storage interface, upload strategy, and other settings. From ffbc6eca2549f6f6dc168dac7269979dc5f7370e Mon Sep 17 00:00:00 2001 From: Maxim Olender Date: Thu, 14 Jan 2016 19:25:26 +0200 Subject: [PATCH 07/18] KAA-773 Add basic log delivery test --- .../client-multi/client-c/test/test_kaa_log.c | 386 +++++++++++++++++- 1 file changed, 382 insertions(+), 4 deletions(-) diff --git a/client/client-multi/client-c/test/test_kaa_log.c b/client/client-multi/client-c/test/test_kaa_log.c index 27e2e3f358..c956a21fe9 100644 --- a/client/client-multi/client-c/test/test_kaa_log.c +++ b/client/client-multi/client-c/test/test_kaa_log.c @@ -435,9 +435,9 @@ void test_response(void) error_code = kaa_logging_handle_server_sync(log_collector, reader, 0, response_buffer_size); ASSERT_EQUAL(error_code, KAA_ERR_NONE); - ASSERT_NOT_NULL(strategy.on_failure_count); - ASSERT_NOT_NULL(storage->on_remove_by_id_count); - ASSERT_NOT_NULL(storage->on_unmark_by_id_count); + ASSERT_TRUE(strategy.on_failure_count); + ASSERT_TRUE(storage->on_remove_by_id_count); + ASSERT_TRUE(storage->on_unmark_by_id_count); kaa_platform_message_reader_destroy(reader); kaa_log_collector_destroy(log_collector); @@ -752,6 +752,379 @@ void test_max_parallel_uploads_with_sync_all(void) #endif +/* ---------------------------------------------------------------------------*/ +/* Log delivery tests */ +/* ---------------------------------------------------------------------------*/ + +#define TEST_BUFFER_SIZE 1024 +#define TEST_EXT_OP 0 /* Simple stub */ + +static mock_strategy_context_t strategy; +static mock_storage_context_t *storage; +static kaa_log_collector_t *log_collector; +static size_t test_log_record_size = TEST_BUFFER_SIZE; +static char test_buffer[TEST_BUFFER_SIZE]; +/* Portion of the test buffer filled with valid data */ +static size_t test_filled_size; +static kaa_platform_message_reader_t *test_reader; + +/* Values to be checked inside mock event function */ +static void *expected_ctx; +static int check_bucket; +static uint16_t expected_bucked_id; + +/* Required to trace generic mock function calls */ +static int call_is_expected; +static int call_is_completed; + +/* Required to trace on fail mock function calls */ +static int failed_call_is_expected; +static int failed_call_completed; + +/* Required to trace on success mock function calls */ +static int success_call_is_expected; +static int success_call_completed; + +/* Required to trace on timeout mock function calls */ +static int timeout_call_is_expected; +static int timeout_call_completed; + +/* Mock event functions */ + +static void mock_log_event_generic_fn(void *ctx, const kaa_log_bucket_info_t *bucket) +{ + ASSERT_TRUE(call_is_expected); + ASSERT_NOT_NULL(bucket); /* Shouldn't be NULL no matter what */ + + if (check_bucket) { + ASSERT_EQUAL(expected_bucked_id, bucket->bucket_id); + } + + ASSERT_EQUAL(expected_ctx, ctx); +} + +static void mock_log_event_failed_fn(void *ctx, const kaa_log_bucket_info_t *bucket) +{ + ASSERT_TRUE(failed_call_is_expected); + /* Make sure that generic function will not fail */ + call_is_expected = 1; + mock_log_event_generic_fn(ctx, bucket); + + failed_call_completed = 1; +} + +static void mock_log_event_success_fn(void *ctx, const kaa_log_bucket_info_t *bucket) +{ + ASSERT_TRUE(success_call_is_expected); + /* Make sure that generic function will not fail */ + call_is_expected = 1; + mock_log_event_generic_fn(ctx, bucket); + + success_call_completed = 1; +} + +static void mock_log_event_timeout_fn(void *ctx, const kaa_log_bucket_info_t *bucket) +{ + ASSERT_TRUE(timeout_call_is_expected); + /* Make sure that generic function will not fail */ + call_is_expected = 1; + mock_log_event_generic_fn(ctx, bucket); + + timeout_call_completed = 1; +} + + + +/* ---------------------------------------------------------------------------*/ +/* Log delivery callback basic test group */ +/* ---------------------------------------------------------------------------*/ + + +KAA_GROUP_SETUP(log_callback_basic) +{ + kaa_error_t error_code; + + KAA_TRACE_IN(logger); + error_code = kaa_log_collector_create(&log_collector, + status, + channel_manager, + logger); + + ASSERT_EQUAL(error_code, KAA_ERR_NONE); + + memset(&strategy, 0, sizeof(strategy)); + + error_code = kaa_logging_init(log_collector, create_mock_storage(), &strategy); + ASSERT_EQUAL(error_code, KAA_ERR_NONE); + + expected_ctx = NULL; + expected_bucked_id = 0; + call_is_expected = 0; + + KAA_TRACE_IN(logger); + return 0; +} + +KAA_GROUP_TEARDOWN(log_callback_basic) +{ + KAA_TRACE_IN(logger); + kaa_log_collector_destroy(log_collector); + log_collector = NULL; + + KAA_TRACE_IN(logger); + return 0; +} + +KAA_TEST_CASE_EX(log_callback_basic, invalid_parameters) +{ + KAA_TRACE_IN(logger); + kaa_log_listeners_t listeners; + + /* NULL parameters case */ + + kaa_error_t rc = kaa_logging_set_listeners(log_collector, NULL); + ASSERT_EQUAL(KAA_ERR_BADPARAM, rc); + rc = kaa_logging_set_listeners(NULL, &listeners); + ASSERT_EQUAL(KAA_ERR_BADPARAM, rc); + + KAA_TRACE_OUT(logger); + return; +} + +/* This test is also testing the case when listeners isn't called + * if no logs added */ +KAA_TEST_CASE_EX(log_callback_basic, valid_parameters) +{ + KAA_TRACE_IN(logger); + + kaa_error_t rc; + + kaa_log_listeners_t listeners = { + mock_log_event_generic_fn, + mock_log_event_generic_fn, + mock_log_event_generic_fn, + NULL, + }; + + /* Any of listeners can be NULL */ + + rc = kaa_logging_set_listeners(log_collector, &listeners); + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + listeners.on_failed = NULL; + rc = kaa_logging_set_listeners(log_collector, &listeners); + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + listeners.on_success = NULL; + rc = kaa_logging_set_listeners(log_collector, &listeners); + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + listeners.on_timeout = NULL; + rc = kaa_logging_set_listeners(log_collector, &listeners); + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + /* Special macro should work too */ + rc = kaa_logging_set_listeners(log_collector, &KAA_LOG_EMPTY_LISTENERS); + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + KAA_TRACE_OUT(logger); + return; +} + +/* ---------------------------------------------------------------------------*/ +/* Log delivery callback extended test group */ +/* ---------------------------------------------------------------------------*/ + +KAA_GROUP_SETUP(log_callback_with_storage) +{ + kaa_error_t error_code; + + KAA_TRACE_IN(logger); + error_code = kaa_log_collector_create(&log_collector, + status, + channel_manager, + logger); + + ASSERT_EQUAL(error_code, KAA_ERR_NONE); + + memset(&strategy, 0, sizeof(mock_strategy_context_t)); + + error_code = kaa_logging_init(log_collector, create_mock_storage(), &strategy); + ASSERT_EQUAL(error_code, KAA_ERR_NONE); + + expected_ctx = NULL; + expected_bucked_id = 0; + call_is_expected = 0; + call_is_completed = 0; + failed_call_completed = 0; + failed_call_is_expected = 0; + success_call_completed = 0; + success_call_is_expected = 0; + check_bucket = 0; + + storage = create_mock_storage(); + error_code = kaa_logging_init(log_collector, storage, &strategy); + ASSERT_EQUAL(error_code, KAA_ERR_NONE); + + uint32_t response_count = 2; + + char *response = test_buffer; + *((uint32_t *)response) = KAA_HTONL(response_count); + response += sizeof(uint32_t); + + /* First response */ + *((uint16_t *)response) = KAA_HTONS(rand()); + response += sizeof(uint16_t); + *((uint8_t *)response) = 0x0; // SUCCESS + response += sizeof(uint8_t); + *((uint8_t *)response) = 0; + response += sizeof(uint8_t); + + /* Second response */ + *((uint16_t *)response) = KAA_HTONS(rand()); + response += sizeof(uint16_t); + *((uint8_t *)response) = 0x1; // FAILURE + response += sizeof(uint8_t); + *((uint8_t *)response) = rand() % 4; + response += sizeof(uint8_t); + + test_filled_size = response - test_buffer; + + error_code = kaa_platform_message_reader_create(&test_reader, + test_buffer, + test_filled_size); + + ASSERT_EQUAL(error_code, KAA_ERR_NONE); + ASSERT_NOT_NULL(test_reader); + + KAA_TRACE_OUT(logger); + return 0; +} + +KAA_GROUP_TEARDOWN(log_callback_with_storage) +{ + KAA_TRACE_IN(logger); + + kaa_platform_message_reader_destroy(test_reader); + test_reader = NULL; + /* Destroys mock storage as well */ + kaa_log_collector_destroy(log_collector); + log_collector = NULL; + + KAA_TRACE_OUT(logger); + return 0; +} + +KAA_TEST_CASE_EX(log_callback_with_storage, on_fail_called) +{ + KAA_TRACE_IN(logger); + + kaa_error_t rc; + + kaa_log_listeners_t listeners = { + NULL, + mock_log_event_failed_fn, + NULL, + NULL, + }; + + failed_call_is_expected = 1; + + rc = kaa_logging_set_listeners(log_collector, &listeners); + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + /* Test itself */ + rc = kaa_logging_handle_server_sync(log_collector, + test_reader, + TEST_EXT_OP, + test_filled_size + ); + + ASSERT_EQUAL(rc, KAA_ERR_NONE); + + ASSERT_FALSE(success_call_completed); + ASSERT_TRUE(failed_call_completed); + ASSERT_FALSE(timeout_call_completed); + + KAA_TRACE_OUT(logger); +} + +KAA_TEST_CASE_EX(log_callback_with_storage, on_success_called) +{ + KAA_TRACE_IN(logger); + + kaa_error_t rc; + + kaa_log_listeners_t listeners = { + mock_log_event_success_fn, + NULL, + NULL, + NULL, + }; + + success_call_is_expected = 1; + + rc = kaa_logging_set_listeners(log_collector, &listeners); + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + /* Test itself */ + rc = kaa_logging_handle_server_sync(log_collector, + test_reader, + TEST_EXT_OP, + test_filled_size + ); + + ASSERT_EQUAL(rc, KAA_ERR_NONE); + + ASSERT_TRUE(success_call_completed); + ASSERT_FALSE(failed_call_completed); + ASSERT_FALSE(timeout_call_completed); + + KAA_TRACE_OUT(logger); +} + + +KAA_TEST_CASE_EX(log_callback_with_storage, on_fail_and_success_called) +{ + KAA_TRACE_IN(logger); + + kaa_error_t rc; + + kaa_log_listeners_t listeners = { + mock_log_event_success_fn, + mock_log_event_failed_fn, + NULL, + NULL, + }; + + failed_call_is_expected = 1; + success_call_is_expected = 1; + + rc = kaa_logging_set_listeners(log_collector, &listeners); + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + /* Test itself */ + rc = kaa_logging_handle_server_sync(log_collector, + test_reader, + TEST_EXT_OP, + test_filled_size + ); + + ASSERT_EQUAL(rc, KAA_ERR_NONE); + + ASSERT_TRUE(success_call_completed); + ASSERT_TRUE(failed_call_completed); + ASSERT_FALSE(timeout_call_completed); + + KAA_TRACE_OUT(logger); +} + + +/* ---------------------------------------------------------------------------*/ +/* End of log delivery test groups */ +/* ---------------------------------------------------------------------------*/ + + int test_init(void) { @@ -759,7 +1132,6 @@ int test_init(void) if (error || !logger) return error; - kaa_context.logger = logger; #ifndef KAA_DISABLE_FEATURE_LOGGING @@ -797,5 +1169,11 @@ KAA_SUITE_MAIN(Log, test_init, test_deinit KAA_TEST_CASE(decline_timeout, test_decline_timeout) KAA_TEST_CASE(max_parallel_uploads_with_log_sync, test_max_parallel_uploads_with_log_sync) KAA_TEST_CASE(max_parallel_uploads_with_sync_all, test_max_parallel_uploads_with_sync_all) + KAA_RUN_TEST(log_callback_basic, invalid_parameters); + KAA_RUN_TEST(log_callback_basic, valid_parameters); + KAA_RUN_TEST(log_callback_with_storage, on_success_called); + KAA_RUN_TEST(log_callback_with_storage, on_fail_called); + KAA_RUN_TEST(log_callback_with_storage, on_fail_and_success_called); + #endif ) From 282653fc2c15f0f51dfd6ea68c1e8c87f198450c Mon Sep 17 00:00:00 2001 From: Maxim Olender Date: Mon, 18 Jan 2016 17:01:24 +0200 Subject: [PATCH 08/18] KAA-773 Clarify log record interfaces --- .../client-c/src/kaa/kaa_logging.c | 43 ++++++++++++------- .../client-c/src/kaa/kaa_logging.h | 34 ++++++++++++--- 2 files changed, 55 insertions(+), 22 deletions(-) diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.c b/client/client-multi/client-c/src/kaa/kaa_logging.c index 14f9fb93a0..1a1422a3b2 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.c +++ b/client/client-multi/client-c/src/kaa/kaa_logging.c @@ -62,7 +62,6 @@ typedef struct struct kaa_log_collector { - uint16_t log_bucket_id; void *log_storage_context; void *log_upload_strategy_context; kaa_status_t *status; @@ -70,7 +69,9 @@ struct kaa_log_collector { kaa_logger_t *logger; kaa_list_t *timeouts; kaa_log_listeners_t log_delivery_listeners; - bool is_sync_ignored; + bool is_sync_ignored; + uint32_t log_last_id; /**< Last log record ID */ + uint16_t log_bucket_id; }; @@ -153,7 +154,8 @@ static bool is_timeout(kaa_log_collector_t *self) ext_log_storage_unmark_by_bucket_id(self->log_storage_context, info->log_bucket_id); if (self->log_delivery_listeners.on_timeout){ kaa_log_bucket_info_t log_bucket_info = {info->log_bucket_id, info->log_cnt}; - (*self->log_delivery_listeners.on_timeout)(self->log_delivery_listeners.ctx, &log_bucket_info); + self->log_delivery_listeners.on_timeout(self->log_delivery_listeners.ctx, + &log_bucket_info); } it = kaa_list_next(it); } @@ -204,6 +206,7 @@ kaa_error_t kaa_log_collector_create(kaa_log_collector_t **log_collector_p KAA_RETURN_IF_NIL(collector, KAA_ERR_NOMEM); collector->log_bucket_id = 0; + collector->log_last_id = 0; collector->log_storage_context = NULL; collector->log_upload_strategy_context = NULL; collector->status = status; @@ -273,15 +276,17 @@ static void update_storage(kaa_log_collector_t *self) -kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_record_t *entry, uint16_t *bucket_id) +kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_record_t *entry, kaa_log_record_info_t *log_info) { KAA_RETURN_IF_NIL2(self, entry, KAA_ERR_BADPARAM); KAA_RETURN_IF_NIL(self->log_storage_context, KAA_ERR_NOT_INITIALIZED); kaa_log_record_t record = { NULL, entry->get_size(entry) }; if (!record.size) { - KAA_LOG_ERROR(self->logger, KAA_ERR_BADDATA, "Failed to add log record: serialized record size is null." - "Maybe log record schema is empty"); + KAA_LOG_ERROR(self->logger, + KAA_ERR_BADDATA, "Failed to add log record: " + "serialized record size is null. " + "Maybe log record schema is empty"); return KAA_ERR_BADDATA; } @@ -307,22 +312,26 @@ kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_recor ext_log_storage_deallocate_log_record_buffer(self->log_storage_context, &record); return error; } + KAA_LOG_TRACE(self->logger, KAA_ERR_NONE, "Added log record, size %zu", record.size); if (!is_timeout(self)) update_storage(self); if (!self->log_bucket_id) { - self->log_bucket_id = self->status->log_bucket_id; + self->log_bucket_id = self->status->log_bucket_id; + } + + if (log_info) { + log_info->bucket_id = self->log_bucket_id + 1; + log_info->log_id = self->log_last_id++; } - // TODO Check that we doesn't have policy that avoid log record from sending in next bucket - *bucket_id = self->log_bucket_id; - ++*bucket_id; return KAA_ERR_NONE; } kaa_error_t kaa_logging_set_listeners(kaa_log_collector_t *self, const kaa_log_listeners_t *listeners) { + KAA_RETURN_IF_NIL2(self, listeners, KAA_ERR_BADPARAM); self->log_delivery_listeners = *listeners; return KAA_ERR_NONE; } @@ -476,19 +485,21 @@ kaa_error_t kaa_logging_handle_server_sync(kaa_log_collector_t *self error_code = kaa_platform_message_read(reader, &delivery_error_code, sizeof(int8_t)); KAA_RETURN_IF_ERR(error_code); + kaa_log_bucket_info_t log_bucket_info = { bucket_id, 0 }; + if (delivery_result == LOGGING_RESULT_SUCCESS) { KAA_LOG_INFO(self->logger, KAA_ERR_NONE, "Log bucket uploaded successfully, id '%u'", bucket_id); - if (self->log_delivery_listeners.on_success){ + if (self->log_delivery_listeners.on_success) { // TODO load log count from timeouts - kaa_log_bucket_info_t log_bucket_info = {bucket_id, 0}; - (*self->log_delivery_listeners.on_success)(self->log_delivery_listeners.ctx,&log_bucket_info); + self->log_delivery_listeners.on_success(self->log_delivery_listeners.ctx, + &log_bucket_info); } } else { KAA_LOG_WARN(self->logger, KAA_ERR_WRITE_FAILED, "Failed to upload log bucket, id '%u' (delivery error code '%u')", bucket_id, delivery_error_code); - if (self->log_delivery_listeners.on_failed){ + if (self->log_delivery_listeners.on_failed) { // TODO load log count from timeouts - kaa_log_bucket_info_t log_bucket_info = {bucket_id, 0}; - (*self->log_delivery_listeners.on_failed)(self->log_delivery_listeners.ctx, &log_bucket_info); + self->log_delivery_listeners.on_failed(self->log_delivery_listeners.ctx, + &log_bucket_info); } } diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.h b/client/client-multi/client-c/src/kaa/kaa_logging.h index 8674e19e4c..5a3f0c1471 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.h +++ b/client/client-multi/client-c/src/kaa/kaa_logging.h @@ -42,16 +42,38 @@ extern "C" { typedef struct kaa_log_collector kaa_log_collector_t; #endif -/** Log bucket information structure. */ +/** + * @brief Log bucket information structure. + * One or more log records aggregated into the bucket. + */ typedef struct { uint16_t bucket_id; /**< ID of bucket present in storage. */ - size_t log_cnt; /**< Current logs count. */ + size_t log_count; /**< Logs left to upload across all buckets. */ } kaa_log_bucket_info_t; +/** + * @brief Log record info. + * + * Each log is contained in the bucket. Bucket is used to agreggate + * multiple logs into one entity that will be atomically sent to the server. + * Bucket can either be entirely successfully sent or be entirely failed. + * Corresponding events are generated. User may subscribe to them. + * @sa kaa_log_event_fn + * @sa kaa_log_listeners_t + * @sa kaa_logging_set_listeners + */ +typedef struct +{ + uint32_t log_id; /**< Id of a log record processed by kaa_logging_add_record() */ + uint16_t bucket_id; /**< Id of a bucket where a log record contained */ +} kaa_log_record_info_t; + /** * @brief Event handler type. * + * Bucket information can be used to retrieve a amount of logs that still + * * @param[in,out] ctx User-definied context. @sa kaa_logging_add_record * @param[in] bucket Log bucket for which event was triggered. */ @@ -61,9 +83,9 @@ typedef void (*kaa_log_event_fn)(void *ctx, const kaa_log_bucket_info_t *bucket) typedef struct { kaa_log_event_fn on_success; /**< Handler called upon successfull log delivery. */ - kaa_log_event_fn on_failed; /**< Handler called upon failed delivery. */ - kaa_log_event_fn on_timeout; /**< Handler called upon timeouted delivery. */ - void *ctx; /**< User-defined context. */ + kaa_log_event_fn on_failed; /**< Handler called upon failed delivery. */ + kaa_log_event_fn on_timeout; /**< Handler called upon timeouted delivery. */ + void *ctx; /**< User-defined context. */ } kaa_log_listeners_t; /** Special macro that can be used to disable event handling. */ @@ -89,7 +111,7 @@ kaa_error_t kaa_logging_init(kaa_log_collector_t *self, void *log_storage_contex * * @return Error code. */ -kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_record_t *entry, uint16_t *bucket_id); +kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_record_t *entry, kaa_log_record_info_t *log_info); /** * @brief Sets listeners of log events. From 2d06e260b387db6b5fab91e0b185a7103b36ba8b Mon Sep 17 00:00:00 2001 From: Maxim Olender Date: Mon, 18 Jan 2016 19:20:12 +0200 Subject: [PATCH 09/18] KAA-773 Implement log count summary for listeners --- .../client-c/src/kaa/kaa_logging.c | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.c b/client/client-multi/client-c/src/kaa/kaa_logging.c index 1a1422a3b2..d11d50fab4 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.c +++ b/client/client-multi/client-c/src/kaa/kaa_logging.c @@ -183,7 +183,13 @@ static bool is_upload_allowed(kaa_log_collector_t *self) return true; } - +/* Sums log counters across all buckets */ +static void sum_log_records(void *data, void *context) +{ + uint16_t count = ((timeout_info_t *)data)->log_cnt; + uint32_t *sum = context; + *sum += count; +} void kaa_log_collector_destroy(kaa_log_collector_t *self) { @@ -485,29 +491,35 @@ kaa_error_t kaa_logging_handle_server_sync(kaa_log_collector_t *self error_code = kaa_platform_message_read(reader, &delivery_error_code, sizeof(int8_t)); KAA_RETURN_IF_ERR(error_code); - kaa_log_bucket_info_t log_bucket_info = { bucket_id, 0 }; if (delivery_result == LOGGING_RESULT_SUCCESS) { KAA_LOG_INFO(self->logger, KAA_ERR_NONE, "Log bucket uploaded successfully, id '%u'", bucket_id); - if (self->log_delivery_listeners.on_success) { - // TODO load log count from timeouts - self->log_delivery_listeners.on_success(self->log_delivery_listeners.ctx, - &log_bucket_info); - } } else { KAA_LOG_WARN(self->logger, KAA_ERR_WRITE_FAILED, "Failed to upload log bucket, id '%u' (delivery error code '%u')", bucket_id, delivery_error_code); - if (self->log_delivery_listeners.on_failed) { - // TODO load log count from timeouts - self->log_delivery_listeners.on_failed(self->log_delivery_listeners.ctx, - &log_bucket_info); - } } remove_request(self, bucket_id); + /* Count logs left to upload */ + + uint32_t total_logs = 0; + kaa_list_for_each(kaa_list_begin(self->timeouts), + kaa_list_back(self->timeouts), + sum_log_records, &total_logs); + + kaa_log_bucket_info_t log_bucket_info = { bucket_id, total_logs }; + if (delivery_result == LOGGING_RESULT_SUCCESS) { + if (self->log_delivery_listeners.on_success) { + self->log_delivery_listeners.on_success(self->log_delivery_listeners.ctx, + &log_bucket_info); + } ext_log_storage_remove_by_bucket_id(self->log_storage_context, bucket_id); } else { + if (self->log_delivery_listeners.on_failed) { + self->log_delivery_listeners.on_failed(self->log_delivery_listeners.ctx, + &log_bucket_info); + } ext_log_storage_unmark_by_bucket_id(self->log_storage_context, bucket_id); ext_log_upload_strategy_on_failure(self->log_upload_strategy_context , (logging_delivery_error_code_t)delivery_error_code); From c078b96ae1e2b800b5c92c690a49c69e46833d16 Mon Sep 17 00:00:00 2001 From: Maxim Olender Date: Tue, 19 Jan 2016 17:41:10 +0200 Subject: [PATCH 10/18] KAA-773 Extend test facilities for CUnit case In order to support per-test setup()/teardown() procedures --- client/client-multi/client-c/test/kaa_test.h | 48 +++++++++++--------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/client/client-multi/client-c/test/kaa_test.h b/client/client-multi/client-c/test/kaa_test.h index 85c0cea057..23da32250e 100644 --- a/client/client-multi/client-c/test/kaa_test.h +++ b/client/client-multi/client-c/test/kaa_test.h @@ -43,6 +43,14 @@ #define KAA_RUN_TESTS \ CU_automated_run_tests(); \ +/* Helper macro to control setup and teardown process per each test in group. + * Must be placed in the exact suite. + */ +#define KAA_RUN_TEST(GROUP, NAME) \ + KAA_TEST_CASE(GROUP##_##NAME##_setup, GROUP##_group_setup); \ + KAA_TEST_CASE(GROUP##_##NAME##_test, GROUP##_##NAME##_test) \ + KAA_TEST_CASE(GROUP##_##NAME##_teardown, GROUP##_group_teardown) + #define KAA_END_TEST_SUITE \ unsigned int failed_tests = CU_get_number_of_failure_records(); \ CU_cleanup_registry(); \ @@ -85,6 +93,20 @@ typedef int (*cleanup_fn)(void); if (!init_ret_code) \ TEST_FN(); +/* Helper macro to control setup and teardown process per each test in group. + * Must be placed in the exact suite. + */ +#define KAA_RUN_TEST(GROUP, NAME) \ + do { \ + if (!init_ret_code) { \ + int rc = GROUP##_group_setup(); \ + ASSERT_EQUAL(0, rc); \ + GROUP##_##NAME##_test(); \ + rc = GROUP##_group_teardown(); \ + ASSERT_EQUAL(0, rc); \ + } \ + } while (0) + #define KAA_RUN_TESTS #define KAA_END_TEST_SUITE \ @@ -93,15 +115,8 @@ typedef int (*cleanup_fn)(void); } \ return (init_ret_code || cleanup_ret_code) ? -1 : 0; \ } - #endif -#define KAA_SUITE_MAIN(SUITE_NAME, INIT_FN, CLEANUP_FN, ...) \ - KAA_BEGIN_TEST_SUITE(SUITE_NAME, INIT_FN, CLEANUP_FN) \ - __VA_ARGS__ \ - KAA_RUN_TESTS \ - KAA_END_TEST_SUITE \ - /* Bunch of macroses that required execute setup() (initialization) * and teardown() (cleanup) procedures per each test in so called "group". @@ -118,20 +133,6 @@ typedef int (*cleanup_fn)(void); #define KAA_TEST_CASE_EX(GROUP, NAME) \ void GROUP##_##NAME##_test(void) -/* Helper macro to control setup and teardown process per each test in group. - * Must be placed in the exact suite. - */ -#define KAA_RUN_TEST(GROUP, NAME) \ - do { \ - if (!init_ret_code) { \ - int rc = GROUP##_group_setup(); \ - ASSERT_EQUAL(0, rc); \ - GROUP##_##NAME##_test(); \ - rc = GROUP##_group_teardown(); \ - ASSERT_EQUAL(0, rc); \ - } \ - } while (0) - /* Defines a setup process for given group. * It runs before each test to make sure sytem is in predictable state */ @@ -146,5 +147,10 @@ typedef int (*cleanup_fn)(void); int GROUP##_group_teardown() +#define KAA_SUITE_MAIN(SUITE_NAME, INIT_FN, CLEANUP_FN, ...) \ + KAA_BEGIN_TEST_SUITE(SUITE_NAME, INIT_FN, CLEANUP_FN) \ + __VA_ARGS__ \ + KAA_RUN_TESTS \ + KAA_END_TEST_SUITE \ #endif /* KAA_TEST_H_ */ From f2116cfaecb50257e06e4f4d6a2c3444bd595861 Mon Sep 17 00:00:00 2001 From: Maxim Olender Date: Tue, 19 Jan 2016 17:42:28 +0200 Subject: [PATCH 11/18] KAA-773 Small cleanup --- .../client-c/src/kaa/kaa_logging.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.c b/client/client-multi/client-c/src/kaa/kaa_logging.c index d11d50fab4..73fd05c5b8 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.c +++ b/client/client-multi/client-c/src/kaa/kaa_logging.c @@ -57,7 +57,7 @@ typedef struct { uint16_t log_bucket_id; /**< ID of bucket present in storage. */ kaa_time_t timeout; /**< bucket timeout */ - uint16_t log_cnt; /**< Current logs count. */ + uint16_t log_count; /**< Current logs count. */ } timeout_info_t; @@ -90,7 +90,7 @@ kaa_error_t kaa_logging_need_logging_resync(kaa_log_collector_t *self, bool *res return KAA_ERR_NONE; } -static kaa_error_t remember_request(kaa_log_collector_t *self, uint16_t bucket_id, uint16_t cnt) +static kaa_error_t remember_request(kaa_log_collector_t *self, uint16_t bucket_id, uint16_t count) { KAA_RETURN_IF_NIL(self, KAA_ERR_BADPARAM); @@ -99,7 +99,7 @@ static kaa_error_t remember_request(kaa_log_collector_t *self, uint16_t bucket_i info->log_bucket_id = bucket_id; info->timeout = KAA_TIME() + (kaa_time_t)ext_log_upload_strategy_get_timeout(self->log_upload_strategy_context); - info->log_cnt = cnt; + info->log_count = count; kaa_list_node_t *it = kaa_list_push_back(self->timeouts, info); if (!it) { @@ -152,8 +152,8 @@ static bool is_timeout(kaa_log_collector_t *self) while (it) { timeout_info_t *info = (timeout_info_t *)kaa_list_get_data(it); ext_log_storage_unmark_by_bucket_id(self->log_storage_context, info->log_bucket_id); - if (self->log_delivery_listeners.on_timeout){ - kaa_log_bucket_info_t log_bucket_info = {info->log_bucket_id, info->log_cnt}; + if (self->log_delivery_listeners.on_timeout) { + kaa_log_bucket_info_t log_bucket_info = { info->log_bucket_id, info->log_count }; self->log_delivery_listeners.on_timeout(self->log_delivery_listeners.ctx, &log_bucket_info); } @@ -186,7 +186,7 @@ static bool is_upload_allowed(kaa_log_collector_t *self) /* Sums log counters across all buckets */ static void sum_log_records(void *data, void *context) { - uint16_t count = ((timeout_info_t *)data)->log_cnt; + uint16_t count = ((timeout_info_t *)data)->log_count; uint32_t *sum = context; *sum += count; } @@ -535,10 +535,10 @@ kaa_error_t kaa_logging_handle_server_sync(kaa_log_collector_t *self extern void ext_log_upload_timeout(kaa_log_collector_t *self) { - if (!is_timeout(self)) - update_storage(self); - else if (ext_log_upload_strategy_is_timeout_strategy(self->log_upload_strategy_context)) + if (!is_timeout(self) + || ext_log_upload_strategy_is_timeout_strategy(self->log_upload_strategy_context)) { update_storage(self); + } } #endif From 8e8645ebcccfb99e5a0db62a0cf336b8eeda4446 Mon Sep 17 00:00:00 2001 From: Maxim Olender Date: Tue, 19 Jan 2016 17:45:06 +0200 Subject: [PATCH 12/18] KAA-773 Extend test cases with timeouts, context and ID checks Test structure improved as well --- .../client-multi/client-c/test/test_kaa_log.c | 271 +++++++++++++++--- 1 file changed, 239 insertions(+), 32 deletions(-) diff --git a/client/client-multi/client-c/test/test_kaa_log.c b/client/client-multi/client-c/test/test_kaa_log.c index c956a21fe9..18241e645e 100644 --- a/client/client-multi/client-c/test/test_kaa_log.c +++ b/client/client-multi/client-c/test/test_kaa_log.c @@ -490,7 +490,7 @@ void test_timeout(void) error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); ASSERT_EQUAL(error_code, KAA_ERR_NONE); - ASSERT_NOT_NULL(strategy.on_timeout_count); + ASSERT_TRUE(strategy.on_timeout_count); test_log_record->destroy(test_log_record); kaa_platform_message_writer_destroy(writer); @@ -756,17 +756,40 @@ void test_max_parallel_uploads_with_sync_all(void) /* Log delivery tests */ /* ---------------------------------------------------------------------------*/ +/* Server chunk, managed by a corresponding reader object. + * Perfectly packed. Packed attribute is intentionally avoided. */ +struct response_chunk +{ + uint8_t bucket_id[2]; /* 16 bits for bucket ID */ + uint8_t resp_code; /* 8 bits for response code. 0 == SUCCESS, 1 == FAILURE */ + uint8_t reserved; /* Should be 0 */ +}; + +struct response_packet +{ + uint8_t resp_cnt[4]; /* 32 bits for amount of responces in buffer */ + struct response_chunk resps[]; /* Responses itself */ +}; + +#define RESP_PACKETS 2 /* Amount of response packets */ +#define RESP_SUCCESS_IDX 0 /* Index of successfull response */ +#define RESP_FAILURE_IDX 1 /* Index of failed response */ #define TEST_BUFFER_SIZE 1024 #define TEST_EXT_OP 0 /* Simple stub */ +#define TEST_TIMEOUT 2 static mock_strategy_context_t strategy; static mock_storage_context_t *storage; static kaa_log_collector_t *log_collector; static size_t test_log_record_size = TEST_BUFFER_SIZE; -static char test_buffer[TEST_BUFFER_SIZE]; +/* Will contain response_packet. Thus required to be aligned. */ +_Alignas(4) static char test_reader_buffer[TEST_BUFFER_SIZE]; +_Alignas(4) static char test_writer_buffer[TEST_BUFFER_SIZE]; /* Portion of the test buffer filled with valid data */ static size_t test_filled_size; static kaa_platform_message_reader_t *test_reader; +static kaa_platform_message_writer_t *test_writer; +static kaa_user_log_record_t *test_log_record; /* Values to be checked inside mock event function */ static void *expected_ctx; @@ -775,7 +798,7 @@ static uint16_t expected_bucked_id; /* Required to trace generic mock function calls */ static int call_is_expected; -static int call_is_completed; +static int call_completed; /* Required to trace on fail mock function calls */ static int failed_call_is_expected; @@ -801,6 +824,8 @@ static void mock_log_event_generic_fn(void *ctx, const kaa_log_bucket_info_t *bu } ASSERT_EQUAL(expected_ctx, ctx); + + call_completed++; } static void mock_log_event_failed_fn(void *ctx, const kaa_log_bucket_info_t *bucket) @@ -810,7 +835,7 @@ static void mock_log_event_failed_fn(void *ctx, const kaa_log_bucket_info_t *buc call_is_expected = 1; mock_log_event_generic_fn(ctx, bucket); - failed_call_completed = 1; + failed_call_completed++; } static void mock_log_event_success_fn(void *ctx, const kaa_log_bucket_info_t *bucket) @@ -820,7 +845,7 @@ static void mock_log_event_success_fn(void *ctx, const kaa_log_bucket_info_t *bu call_is_expected = 1; mock_log_event_generic_fn(ctx, bucket); - success_call_completed = 1; + success_call_completed++; } static void mock_log_event_timeout_fn(void *ctx, const kaa_log_bucket_info_t *bucket) @@ -830,7 +855,7 @@ static void mock_log_event_timeout_fn(void *ctx, const kaa_log_bucket_info_t *bu call_is_expected = 1; mock_log_event_generic_fn(ctx, bucket); - timeout_call_completed = 1; + timeout_call_completed++; } @@ -927,6 +952,11 @@ KAA_TEST_CASE_EX(log_callback_basic, valid_parameters) rc = kaa_logging_set_listeners(log_collector, &KAA_LOG_EMPTY_LISTENERS); ASSERT_EQUAL(KAA_ERR_NONE, rc); + /* Post conditions check */ + + /* No callbacks should be called */ + ASSERT_FALSE(call_completed); + KAA_TRACE_OUT(logger); return; } @@ -948,6 +978,8 @@ KAA_GROUP_SETUP(log_callback_with_storage) ASSERT_EQUAL(error_code, KAA_ERR_NONE); memset(&strategy, 0, sizeof(mock_strategy_context_t)); + memset(test_reader_buffer, 0, sizeof(test_reader_buffer)); + memset(test_writer_buffer, 0, sizeof(test_writer_buffer)); error_code = kaa_logging_init(log_collector, create_mock_storage(), &strategy); ASSERT_EQUAL(error_code, KAA_ERR_NONE); @@ -955,7 +987,7 @@ KAA_GROUP_SETUP(log_callback_with_storage) expected_ctx = NULL; expected_bucked_id = 0; call_is_expected = 0; - call_is_completed = 0; + call_completed = 0; failed_call_completed = 0; failed_call_is_expected = 0; success_call_completed = 0; @@ -967,31 +999,26 @@ KAA_GROUP_SETUP(log_callback_with_storage) ASSERT_EQUAL(error_code, KAA_ERR_NONE); uint32_t response_count = 2; - - char *response = test_buffer; - *((uint32_t *)response) = KAA_HTONL(response_count); - response += sizeof(uint32_t); + struct response_packet *response = (struct response_packet *) test_reader_buffer; + *(uint32_t *) response->resp_cnt = KAA_HTONL(response_count); /* First response */ - *((uint16_t *)response) = KAA_HTONS(rand()); - response += sizeof(uint16_t); - *((uint8_t *)response) = 0x0; // SUCCESS - response += sizeof(uint8_t); - *((uint8_t *)response) = 0; - response += sizeof(uint8_t); + + /* Later on we'll override that */ + *(uint16_t *) response->resps[RESP_SUCCESS_IDX].bucket_id = 0; + response->resps[RESP_SUCCESS_IDX].resp_code = 0; // SUCCESS /* Second response */ - *((uint16_t *)response) = KAA_HTONS(rand()); - response += sizeof(uint16_t); - *((uint8_t *)response) = 0x1; // FAILURE - response += sizeof(uint8_t); - *((uint8_t *)response) = rand() % 4; - response += sizeof(uint8_t); - test_filled_size = response - test_buffer; + /* Later on we'll override that */ + *(uint16_t *) response->resps[RESP_FAILURE_IDX].bucket_id = 0; + response->resps[RESP_FAILURE_IDX].resp_code = 1; // FAILURE + + test_filled_size = sizeof(struct response_packet) + + sizeof(struct response_chunk) * 2; error_code = kaa_platform_message_reader_create(&test_reader, - test_buffer, + test_reader_buffer, test_filled_size); ASSERT_EQUAL(error_code, KAA_ERR_NONE); @@ -1020,19 +1047,29 @@ KAA_TEST_CASE_EX(log_callback_with_storage, on_fail_called) KAA_TRACE_IN(logger); kaa_error_t rc; + int dummy_ctx; kaa_log_listeners_t listeners = { NULL, mock_log_event_failed_fn, NULL, - NULL, + &dummy_ctx, }; + /* Notify mocks about test intentions */ failed_call_is_expected = 1; + expected_ctx = &dummy_ctx; + check_bucket = 1; + expected_bucked_id = 42; rc = kaa_logging_set_listeners(log_collector, &listeners); ASSERT_EQUAL(KAA_ERR_NONE, rc); + /* Response packet is passed internally via test reader */ + struct response_packet *response = (struct response_packet *) test_reader_buffer; + *(uint16_t *) response->resps[RESP_FAILURE_IDX].bucket_id + = KAA_HTONS(expected_bucked_id); + /* Test itself */ rc = kaa_logging_handle_server_sync(log_collector, test_reader, @@ -1042,6 +1079,8 @@ KAA_TEST_CASE_EX(log_callback_with_storage, on_fail_called) ASSERT_EQUAL(rc, KAA_ERR_NONE); + /* Post-conditions check */ + ASSERT_FALSE(success_call_completed); ASSERT_TRUE(failed_call_completed); ASSERT_FALSE(timeout_call_completed); @@ -1054,19 +1093,29 @@ KAA_TEST_CASE_EX(log_callback_with_storage, on_success_called) KAA_TRACE_IN(logger); kaa_error_t rc; + int dummy_ctx; kaa_log_listeners_t listeners = { mock_log_event_success_fn, NULL, NULL, - NULL, + &dummy_ctx, }; + /* Notify mocks about test intentions */ + check_bucket = 1; + expected_bucked_id = 42; success_call_is_expected = 1; + expected_ctx = &dummy_ctx; rc = kaa_logging_set_listeners(log_collector, &listeners); ASSERT_EQUAL(KAA_ERR_NONE, rc); + /* Response packet is passed internally via test reader */ + struct response_packet *response = (struct response_packet *) test_reader_buffer; + *(uint16_t *) response->resps[RESP_SUCCESS_IDX].bucket_id + = KAA_HTONS(expected_bucked_id); + /* Test itself */ rc = kaa_logging_handle_server_sync(log_collector, test_reader, @@ -1076,6 +1125,8 @@ KAA_TEST_CASE_EX(log_callback_with_storage, on_success_called) ASSERT_EQUAL(rc, KAA_ERR_NONE); + /* Post-conditions check */ + ASSERT_TRUE(success_call_completed); ASSERT_FALSE(failed_call_completed); ASSERT_FALSE(timeout_call_completed); @@ -1083,26 +1134,37 @@ KAA_TEST_CASE_EX(log_callback_with_storage, on_success_called) KAA_TRACE_OUT(logger); } - KAA_TEST_CASE_EX(log_callback_with_storage, on_fail_and_success_called) { KAA_TRACE_IN(logger); kaa_error_t rc; + int dummy_ctx; kaa_log_listeners_t listeners = { mock_log_event_success_fn, mock_log_event_failed_fn, NULL, - NULL, + &dummy_ctx, }; + /* Notify mocks about test intentions */ + check_bucket = 1; + expected_bucked_id = 42; failed_call_is_expected = 1; success_call_is_expected = 1; + expected_ctx = &dummy_ctx; rc = kaa_logging_set_listeners(log_collector, &listeners); ASSERT_EQUAL(KAA_ERR_NONE, rc); + /* Response packet is passed internally via test reader */ + struct response_packet *response = (struct response_packet *) test_reader_buffer; + *(uint16_t *) response->resps[RESP_SUCCESS_IDX].bucket_id + = KAA_HTONS(expected_bucked_id); + *(uint16_t *) response->resps[RESP_FAILURE_IDX].bucket_id + = KAA_HTONS(expected_bucked_id); + /* Test itself */ rc = kaa_logging_handle_server_sync(log_collector, test_reader, @@ -1112,6 +1174,8 @@ KAA_TEST_CASE_EX(log_callback_with_storage, on_fail_and_success_called) ASSERT_EQUAL(rc, KAA_ERR_NONE); + /* Post-conditions check */ + ASSERT_TRUE(success_call_completed); ASSERT_TRUE(failed_call_completed); ASSERT_FALSE(timeout_call_completed); @@ -1119,12 +1183,155 @@ KAA_TEST_CASE_EX(log_callback_with_storage, on_fail_and_success_called) KAA_TRACE_OUT(logger); } - /* ---------------------------------------------------------------------------*/ -/* End of log delivery test groups */ +/* Log delivery callback group with valid mock strategy. */ +/* The initialized mock strategy is essential for timeouts */ /* ---------------------------------------------------------------------------*/ +KAA_GROUP_SETUP(log_callback_with_storage_and_strategy) +{ + kaa_error_t error_code; + size_t test_log_record_size; + + KAA_TRACE_IN(logger); + error_code = kaa_log_collector_create(&log_collector, + status, + channel_manager, + logger); + + ASSERT_EQUAL(error_code, KAA_ERR_NONE); + + memset(&strategy, 0, sizeof(mock_strategy_context_t)); + strategy.timeout = TEST_TIMEOUT; + strategy.decision = NOOP; + strategy.batch_size = TEST_BUFFER_SIZE; + strategy.max_parallel_uploads = UINT32_MAX; + + error_code = kaa_logging_init(log_collector, create_mock_storage(), &strategy); + ASSERT_EQUAL(error_code, KAA_ERR_NONE); + expected_ctx = NULL; + expected_bucked_id = 0; + call_is_expected = 0; + call_completed = 0; + failed_call_completed = 0; + failed_call_is_expected = 0; + success_call_completed = 0; + success_call_is_expected = 0; + check_bucket = 0; + + storage = create_mock_storage(); + error_code = kaa_logging_init(log_collector, storage, &strategy); + ASSERT_EQUAL(error_code, KAA_ERR_NONE); + + uint32_t response_count = 2; + + struct response_packet *response = (struct response_packet *) test_reader_buffer; + *(uint32_t *) response->resp_cnt = KAA_HTONL(response_count); + + /* First response */ + + /* Later on we'll override that */ + *(uint16_t *) response->resps[RESP_SUCCESS_IDX].bucket_id = 0; + response->resps[RESP_SUCCESS_IDX].resp_code = 0; // SUCCESS + + /* Second response */ + + /* Later on we'll override that */ + *(uint16_t *) response->resps[RESP_FAILURE_IDX].bucket_id = 0; + response->resps[RESP_FAILURE_IDX].resp_code = 1; // FAILURE + + test_filled_size = sizeof(struct response_packet) + + sizeof(struct response_chunk) * 2; + + test_log_record = kaa_test_log_record_create(); + test_log_record->data = kaa_string_copy_create(TEST_LOG_BUFFER); + test_log_record_size = test_log_record->get_size(test_log_record); + + error_code = kaa_platform_message_reader_create(&test_reader, + test_reader_buffer, + test_filled_size); + + ASSERT_EQUAL(error_code, KAA_ERR_NONE); + ASSERT_NOT_NULL(test_reader); + + error_code = kaa_platform_message_writer_create(&test_writer, + test_writer_buffer, + test_log_record_size); + ASSERT_EQUAL(error_code, KAA_ERR_NONE); + + KAA_TRACE_OUT(logger); + return 0; +} + +KAA_GROUP_TEARDOWN(log_callback_with_storage_and_strategy) +{ + KAA_TRACE_IN(logger); + test_log_record->destroy(test_log_record); + + kaa_platform_message_writer_destroy(test_writer); + test_writer = NULL; + kaa_platform_message_reader_destroy(test_reader); + test_reader = NULL; + + /* Destroys mock storage as well */ + kaa_log_collector_destroy(log_collector); + log_collector = NULL; + + KAA_TRACE_OUT(logger); + return 0; +} + +KAA_TEST_CASE_EX(log_callback_with_storage_and_strategy, on_timeout_called) +{ + KAA_TRACE_IN(logger); + + kaa_error_t rc; + int dummy_ctx; + + kaa_log_listeners_t listeners = { + NULL, + NULL, + mock_log_event_timeout_fn, + &dummy_ctx, + }; + + kaa_log_record_info_t bucket; + + timeout_call_is_expected = 1; + expected_ctx = &dummy_ctx; + + rc = kaa_logging_set_listeners(log_collector, &listeners); + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + rc = kaa_logging_add_record(log_collector, test_log_record, &bucket); + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + /* Notify mocks about bucket */ + check_bucket = 1; + expected_bucked_id = bucket.bucket_id; + + /* Test itself */ + + rc = kaa_logging_request_serialize(log_collector, test_writer); + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + sleep(TEST_TIMEOUT + 1); + + rc = kaa_logging_add_record(log_collector, test_log_record, NULL); + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + /* Post-conditions check */ + + /* Timeout callback should be called once */ + ASSERT_EQUAL(1, timeout_call_completed); + + KAA_TRACE_OUT(logger); +} + +/* ---------------------------------------------------------------------------*/ +/* End of log delivery test groups */ +/* ---------------------------------------------------------------------------*/ int test_init(void) { @@ -1174,6 +1381,6 @@ KAA_SUITE_MAIN(Log, test_init, test_deinit KAA_RUN_TEST(log_callback_with_storage, on_success_called); KAA_RUN_TEST(log_callback_with_storage, on_fail_called); KAA_RUN_TEST(log_callback_with_storage, on_fail_and_success_called); - + KAA_RUN_TEST(log_callback_with_storage_and_strategy, on_timeout_called); #endif ) From 115461546e10e97f672cf8279a778d1cf7059443 Mon Sep 17 00:00:00 2001 From: Max Payne Date: Wed, 20 Jan 2016 12:18:07 +0200 Subject: [PATCH 13/18] KAA-773 Core review comments addressed --- .../client-c/src/kaa/kaa_logging.c | 54 +++++++------- .../client-c/src/kaa/kaa_logging.h | 35 +-------- .../kaa/platform/ext_log_delivery_listener.h | 71 +++++++++++++++++++ .../client-multi/client-c/test/test_kaa_log.c | 12 ++-- 4 files changed, 104 insertions(+), 68 deletions(-) create mode 100644 client/client-multi/client-c/src/kaa/platform/ext_log_delivery_listener.h diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.c b/client/client-multi/client-c/src/kaa/kaa_logging.c index 73fd05c5b8..3b0687e1d5 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.c +++ b/client/client-multi/client-c/src/kaa/kaa_logging.c @@ -34,51 +34,41 @@ #include "utilities/kaa_log.h" #include "avro_src/avro/io.h" - - #define KAA_LOGGING_RECEIVE_UPDATES_FLAG 0x01 #define KAA_MAX_PADDING_LENGTH (KAA_ALIGNMENT - 1) - - extern kaa_transport_channel_interface_t *kaa_channel_manager_get_transport_channel(kaa_channel_manager_t *self , kaa_service_t service_type); extern bool ext_log_upload_strategy_is_timeout_strategy(void *strategy); - - typedef enum { LOGGING_RESULT_SUCCESS = 0x00, LOGGING_RESULT_FAILURE = 0x01 } logging_sync_result_t; -typedef struct -{ +typedef struct { + kaa_time_t timeout; /**< Bucket timeout. */ uint16_t log_bucket_id; /**< ID of bucket present in storage. */ - kaa_time_t timeout; /**< bucket timeout */ uint16_t log_count; /**< Current logs count. */ } timeout_info_t; struct kaa_log_collector { - void *log_storage_context; - void *log_upload_strategy_context; - kaa_status_t *status; - kaa_channel_manager_t *channel_manager; - kaa_logger_t *logger; - kaa_list_t *timeouts; - kaa_log_listeners_t log_delivery_listeners; - bool is_sync_ignored; - uint32_t log_last_id; /**< Last log record ID */ - uint16_t log_bucket_id; + void *log_storage_context; + void *log_upload_strategy_context; + kaa_status_t *status; + kaa_channel_manager_t *channel_manager; + kaa_logger_t *logger; + kaa_list_t *timeouts; + kaa_log_delivery_listener_t log_delivery_listeners; + bool is_sync_ignored; + uint32_t log_last_id; /**< Last log record ID */ + uint16_t log_bucket_id; }; - - static const kaa_service_t logging_sync_services[] = {KAA_SERVICE_LOGGING}; - kaa_error_t kaa_logging_need_logging_resync(kaa_log_collector_t *self, bool *result) { KAA_RETURN_IF_NIL2(self, result, KAA_ERR_BADPARAM); @@ -153,7 +143,11 @@ static bool is_timeout(kaa_log_collector_t *self) timeout_info_t *info = (timeout_info_t *)kaa_list_get_data(it); ext_log_storage_unmark_by_bucket_id(self->log_storage_context, info->log_bucket_id); if (self->log_delivery_listeners.on_timeout) { - kaa_log_bucket_info_t log_bucket_info = { info->log_bucket_id, info->log_count }; + kaa_log_bucket_info_t log_bucket_info = { + .bucket_id = info->log_bucket_id, + .log_count = info->log_count, + }; + self->log_delivery_listeners.on_timeout(self->log_delivery_listeners.ctx, &log_bucket_info); } @@ -289,10 +283,9 @@ kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_recor kaa_log_record_t record = { NULL, entry->get_size(entry) }; if (!record.size) { - KAA_LOG_ERROR(self->logger, - KAA_ERR_BADDATA, "Failed to add log record: " - "serialized record size is null. " - "Maybe log record schema is empty"); + KAA_LOG_ERROR(self->logger, KAA_ERR_BADDATA, + "Failed to add log record: serialized record size is null. " + "Maybe log record schema is empty"); return KAA_ERR_BADDATA; } @@ -335,7 +328,7 @@ kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_recor return KAA_ERR_NONE; } -kaa_error_t kaa_logging_set_listeners(kaa_log_collector_t *self, const kaa_log_listeners_t *listeners) +kaa_error_t kaa_logging_set_listeners(kaa_log_collector_t *self, const kaa_log_delivery_listener_t *listeners) { KAA_RETURN_IF_NIL2(self, listeners, KAA_ERR_BADPARAM); self->log_delivery_listeners = *listeners; @@ -507,7 +500,10 @@ kaa_error_t kaa_logging_handle_server_sync(kaa_log_collector_t *self kaa_list_back(self->timeouts), sum_log_records, &total_logs); - kaa_log_bucket_info_t log_bucket_info = { bucket_id, total_logs }; + kaa_log_bucket_info_t log_bucket_info = { + .log_count = total_logs, + .bucket_id = bucket_id, + }; if (delivery_result == LOGGING_RESULT_SUCCESS) { if (self->log_delivery_listeners.on_success) { diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.h b/client/client-multi/client-c/src/kaa/kaa_logging.h index 5a3f0c1471..b9fc4ef637 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.h +++ b/client/client-multi/client-c/src/kaa/kaa_logging.h @@ -28,6 +28,7 @@ #include "gen/kaa_logging_definitions.h" #include "platform/ext_log_storage.h" #include "platform/ext_log_upload_strategy.h" +#include "platform/ext_log_delivery_listener.h" #ifdef __cplusplus extern "C" { @@ -42,16 +43,6 @@ extern "C" { typedef struct kaa_log_collector kaa_log_collector_t; #endif -/** - * @brief Log bucket information structure. - * One or more log records aggregated into the bucket. - */ -typedef struct -{ - uint16_t bucket_id; /**< ID of bucket present in storage. */ - size_t log_count; /**< Logs left to upload across all buckets. */ -} kaa_log_bucket_info_t; - /** * @brief Log record info. * @@ -69,28 +60,6 @@ typedef struct uint16_t bucket_id; /**< Id of a bucket where a log record contained */ } kaa_log_record_info_t; -/** - * @brief Event handler type. - * - * Bucket information can be used to retrieve a amount of logs that still - * - * @param[in,out] ctx User-definied context. @sa kaa_logging_add_record - * @param[in] bucket Log bucket for which event was triggered. - */ -typedef void (*kaa_log_event_fn)(void *ctx, const kaa_log_bucket_info_t *bucket); - -/** Listeners aggreate */ -typedef struct -{ - kaa_log_event_fn on_success; /**< Handler called upon successfull log delivery. */ - kaa_log_event_fn on_failed; /**< Handler called upon failed delivery. */ - kaa_log_event_fn on_timeout; /**< Handler called upon timeouted delivery. */ - void *ctx; /**< User-defined context. */ -} kaa_log_listeners_t; - -/** Special macro that can be used to disable event handling. */ -#define KAA_LOG_EMPTY_LISTENERS ((kaa_log_listeners_t){NULL, NULL, NULL, NULL}) - /** * @brief Initializes data collection module with the storage interface, upload strategy, and other settings. * @@ -122,7 +91,7 @@ kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_recor * can be used to unsubscribe from log events. * @return Error code. */ -kaa_error_t kaa_logging_set_listeners(kaa_log_collector_t *self, const kaa_log_listeners_t *listeners); +kaa_error_t kaa_logging_set_listeners(kaa_log_collector_t *self, const kaa_log_delivery_listener_t *listeners); #ifdef __cplusplus } /* extern "C" */ diff --git a/client/client-multi/client-c/src/kaa/platform/ext_log_delivery_listener.h b/client/client-multi/client-c/src/kaa/platform/ext_log_delivery_listener.h new file mode 100644 index 0000000000..2487aefe7a --- /dev/null +++ b/client/client-multi/client-c/src/kaa/platform/ext_log_delivery_listener.h @@ -0,0 +1,71 @@ +/* + * Copyright 2016 CyberVision, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file + * @brief External log delivery listener interfaces. + * + * Listener callbacks could be used to notify about log-releated events: + * success, fail or timeout. + * + */ + +#ifndef EXT_LOG_DELIVERY_LISTENER_ +#define EXT_LOG_DELIVERY_LISTENER_ + + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @brief Log bucket information structure. + * One or more log records are aggregated into the single bucket. + */ +typedef struct { + size_t log_count; /**< Logs left to upload across all buckets. */ + uint16_t bucket_id; /**< ID of bucket present in storage. */ +} kaa_log_bucket_info_t; + +/** + * @brief Event handler type. + * + * Bucket information can be used to retrieve a amount of logs that are + * pending to upload. + * + * @param[in,out] context User-definied context. @sa kaa_logging_add_record + * @param[in] bucket Log bucket for which event was triggered. + */ +typedef void (*kaa_log_event_fn)(void *context, const kaa_log_bucket_info_t *bucket); + +/** Listeners aggreate */ +typedef struct { + kaa_log_event_fn on_success; /**< Handler called upon successfull log delivery. */ + kaa_log_event_fn on_failed; /**< Handler called upon failed delivery. */ + kaa_log_event_fn on_timeout; /**< Handler called upon timeouted delivery. */ + void *ctx; /**< User-defined context. */ +} kaa_log_delivery_listener_t; + +/** Special macro that can be used to disable event handling. */ +#define KAA_LOG_EMPTY_LISTENERS ((kaa_log_delivery_listener_t){NULL, NULL, NULL, NULL}) + +#ifdef __cplusplus +} /* extern "C" */ +#endif + + + +#endif // EXT_LOG_DELIVERY_LISTENER_ diff --git a/client/client-multi/client-c/test/test_kaa_log.c b/client/client-multi/client-c/test/test_kaa_log.c index 18241e645e..ccd55adfc6 100644 --- a/client/client-multi/client-c/test/test_kaa_log.c +++ b/client/client-multi/client-c/test/test_kaa_log.c @@ -903,7 +903,7 @@ KAA_GROUP_TEARDOWN(log_callback_basic) KAA_TEST_CASE_EX(log_callback_basic, invalid_parameters) { KAA_TRACE_IN(logger); - kaa_log_listeners_t listeners; + kaa_log_delivery_listener_t listeners; /* NULL parameters case */ @@ -924,7 +924,7 @@ KAA_TEST_CASE_EX(log_callback_basic, valid_parameters) kaa_error_t rc; - kaa_log_listeners_t listeners = { + kaa_log_delivery_listener_t listeners = { mock_log_event_generic_fn, mock_log_event_generic_fn, mock_log_event_generic_fn, @@ -1049,7 +1049,7 @@ KAA_TEST_CASE_EX(log_callback_with_storage, on_fail_called) kaa_error_t rc; int dummy_ctx; - kaa_log_listeners_t listeners = { + kaa_log_delivery_listener_t listeners = { NULL, mock_log_event_failed_fn, NULL, @@ -1095,7 +1095,7 @@ KAA_TEST_CASE_EX(log_callback_with_storage, on_success_called) kaa_error_t rc; int dummy_ctx; - kaa_log_listeners_t listeners = { + kaa_log_delivery_listener_t listeners = { mock_log_event_success_fn, NULL, NULL, @@ -1141,7 +1141,7 @@ KAA_TEST_CASE_EX(log_callback_with_storage, on_fail_and_success_called) kaa_error_t rc; int dummy_ctx; - kaa_log_listeners_t listeners = { + kaa_log_delivery_listener_t listeners = { mock_log_event_success_fn, mock_log_event_failed_fn, NULL, @@ -1289,7 +1289,7 @@ KAA_TEST_CASE_EX(log_callback_with_storage_and_strategy, on_timeout_called) kaa_error_t rc; int dummy_ctx; - kaa_log_listeners_t listeners = { + kaa_log_delivery_listener_t listeners = { NULL, NULL, mock_log_event_timeout_fn, From 627def5d1a3ae0e36d0935da7bfdfe9bdc456c56 Mon Sep 17 00:00:00 2001 From: Max Payne Date: Wed, 20 Jan 2016 17:53:29 +0200 Subject: [PATCH 14/18] KAA-773 Move size constraints from strategy to logging modules Patch contains fixes for warnings from tests and API usage in various places as well. --- .../client-c/src/kaa/kaa_logging.c | 16 +++- .../client-c/src/kaa/kaa_logging.h | 12 ++- .../Econais/EC19D/econais_ec19d_kaa_client.c | 11 ++- .../common/ext_log_upload_strategies.c | 17 ---- .../common/ext_log_upload_strategies.h | 11 --- .../esp8266/esp8266_kaa_client.c | 12 ++- .../platform-impl/posix/posix_kaa_client.c | 11 ++- .../esp8266/esp8266_kaa_client.c | 12 ++- .../kaa/platform/ext_log_upload_strategy.h | 10 -- client/client-multi/client-c/test/kaa_test.h | 2 +- .../test_ext_log_storage_memory.c | 1 + .../test_ext_log_upload_strategies.c | 20 ---- .../client-c/test/test_kaa_common_schema.c | 1 + .../client-c/test/test_kaa_event.c | 2 +- .../client-multi/client-c/test/test_kaa_log.c | 96 ++++++++++++------- .../client-c/test/test_platform_protocol.c | 8 +- 16 files changed, 135 insertions(+), 107 deletions(-) diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.c b/client/client-multi/client-c/src/kaa/kaa_logging.c index 3b0687e1d5..4486cd8fe2 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.c +++ b/client/client-multi/client-c/src/kaa/kaa_logging.c @@ -53,8 +53,8 @@ typedef struct { uint16_t log_count; /**< Current logs count. */ } timeout_info_t; - struct kaa_log_collector { + kaa_log_bucket_constraints_t bucket_size; void *log_storage_context; void *log_upload_strategy_context; kaa_status_t *status; @@ -64,6 +64,8 @@ struct kaa_log_collector { kaa_log_delivery_listener_t log_delivery_listeners; bool is_sync_ignored; uint32_t log_last_id; /**< Last log record ID */ + uint32_t max_bucket_log_count; + uint32_t max_storage_size; uint16_t log_bucket_id; }; @@ -226,7 +228,7 @@ kaa_error_t kaa_log_collector_create(kaa_log_collector_t **log_collector_p -kaa_error_t kaa_logging_init(kaa_log_collector_t *self, void *log_storage_context, void *log_upload_strategy_context) +kaa_error_t kaa_logging_init(kaa_log_collector_t *self, void *log_storage_context, void *log_upload_strategy_context, const kaa_log_bucket_constraints_t *bucket_sizes) { KAA_RETURN_IF_NIL3(self, log_storage_context, log_upload_strategy_context, KAA_ERR_BADPARAM); @@ -236,6 +238,7 @@ kaa_error_t kaa_logging_init(kaa_log_collector_t *self, void *log_storage_contex self->log_storage_context = log_storage_context; self->log_upload_strategy_context = log_upload_strategy_context; self->log_delivery_listeners = KAA_LOG_EMPTY_LISTENERS; + self->bucket_size = *bucket_sizes; KAA_LOG_DEBUG(self->logger, KAA_ERR_NONE, "Initialized log collector with log storage {%p}, log upload strategy {%p}" , log_storage_context, log_upload_strategy_context); @@ -357,7 +360,7 @@ kaa_error_t kaa_logging_request_get_size(kaa_log_collector_t *self, size_t *expe *expected_size += sizeof(uint32_t); // request id + log records count size_t actual_size = records_count * sizeof(uint32_t) + records_count * KAA_MAX_PADDING_LENGTH + total_size; - size_t bucket_size = ext_log_upload_strategy_get_bucket_size(self->log_upload_strategy_context); + size_t bucket_size = self->bucket_size.max_bucket_size; *expected_size += ((actual_size > bucket_size) ? bucket_size : actual_size); } @@ -398,7 +401,10 @@ kaa_error_t kaa_logging_request_serialize(kaa_log_collector_t *self, kaa_platfor char *records_count_p = tmp_writer.current; // Pointer to the records count. Will be filled in later. tmp_writer.current += sizeof(uint16_t); - size_t bucket_size = ext_log_upload_strategy_get_bucket_size(self->log_upload_strategy_context); + /* Bucket size constraints */ + + size_t bucket_size = self->bucket_size.max_bucket_size; + size_t max_log_count = self->bucket_size.max_bucket_log_count; size_t actual_size = (tmp_writer.end - tmp_writer.current); bucket_size = (actual_size > bucket_size ? bucket_size : actual_size); @@ -407,7 +413,7 @@ kaa_error_t kaa_logging_request_serialize(kaa_log_collector_t *self, kaa_platfor uint16_t records_count = 0; - while (!error && bucket_size > sizeof(uint32_t)) { + while (!error && bucket_size > sizeof(uint32_t) && records_count < max_log_count) { size_t record_len = 0; error = ext_log_storage_write_next_record(self->log_storage_context , tmp_writer.current + sizeof(uint32_t) diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.h b/client/client-multi/client-c/src/kaa/kaa_logging.h index b9fc4ef637..687330a429 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.h +++ b/client/client-multi/client-c/src/kaa/kaa_logging.h @@ -54,22 +54,28 @@ extern "C" { * @sa kaa_log_listeners_t * @sa kaa_logging_set_listeners */ -typedef struct -{ +typedef struct { uint32_t log_id; /**< Id of a log record processed by kaa_logging_add_record() */ uint16_t bucket_id; /**< Id of a bucket where a log record contained */ } kaa_log_record_info_t; +/** Constraints applied to log buckets */ +typedef struct { + size_t max_bucket_size; /**< The maximum bucket size in bytes */ + size_t max_bucket_log_count; /**< The maximum log count within a single bucket */ +} kaa_log_bucket_constraints_t; + /** * @brief Initializes data collection module with the storage interface, upload strategy, and other settings. * * @param[in] self Pointer to a @link kaa_log_collector_t @endlink instance. * @param[in] log_storage_context Log storage context. * @param[in] log_upload_strategy_context Log upload strategy context. + * @param[in] bucket_sizes Bucket size constraints. * * @return Error code. */ -kaa_error_t kaa_logging_init(kaa_log_collector_t *self, void *log_storage_context, void *log_upload_strategy_context); +kaa_error_t kaa_logging_init(kaa_log_collector_t *self, void *log_storage_context, void *log_upload_strategy_context, const kaa_log_bucket_constraints_t *bucket_sizes); /** * @brief Serializes and adds a log record to the log storage. diff --git a/client/client-multi/client-c/src/kaa/platform-impl/Econais/EC19D/econais_ec19d_kaa_client.c b/client/client-multi/client-c/src/kaa/platform-impl/Econais/EC19D/econais_ec19d_kaa_client.c index f7390fb466..0136869678 100644 --- a/client/client-multi/client-c/src/kaa/platform-impl/Econais/EC19D/econais_ec19d_kaa_client.c +++ b/client/client-multi/client-c/src/kaa/platform-impl/Econais/EC19D/econais_ec19d_kaa_client.c @@ -91,7 +91,8 @@ void print_mem_stat(kaa_client_t *kaa_client); * Strategy-specific configuration parameters used by Kaa log collection feature. */ #define KAA_DEMO_MAX_UPLOAD_THRESHOLD 15 /* Size of collected serialized logs needed to initiate log upload */ -#define KAA_DEMO_MAX_LOG_BUCKET_SIZE 16 /* Max size of a log batch has been sent by SDK during one upload. */ +#define KAA_DEMO_MAX_LOG_BUCKET_SIZE 512 /* Max size of a log batch has been sent by SDK during one upload. */ +#define KAA_DEMO_MAX_LOGS_IN_BUCKET 16 /* Max count of logs in one bucket */ #define KAA_DEMO_MAX_CLEANUP_THRESHOLD 100 /* Max size of an inner log storage. If size is exceeded, elder logs will be removed. */ #define KAA_DEMO_LOG_GENERATION_FREQUENCY 3 /* seconds */ @@ -732,9 +733,15 @@ kaa_error_t kaa_log_collector_init(kaa_client_t *kaa_client) return error_code; } + kaa_log_bucket_constraints_t bucket_sizes = { + .max_bucket_size = KAA_DEMO_MAX_LOG_BUCKET_SIZE, + .max_bucket_log_count = KAA_DEMO_MAX_LOGS_IN_BUCKET, + }; + error_code = kaa_logging_init(kaa_client->kaa_context->log_collector , kaa_client->log_storage_context - , kaa_client->log_upload_strategy_context); + , kaa_client->log_upload_strategy_context + , &bucket_sizes); if (error_code) { KAA_LOG_ERROR(kaa_client->kaa_context->logger, diff --git a/client/client-multi/client-c/src/kaa/platform-impl/common/ext_log_upload_strategies.c b/client/client-multi/client-c/src/kaa/platform-impl/common/ext_log_upload_strategies.c index deb79e7c00..f3784a3f99 100644 --- a/client/client-multi/client-c/src/kaa/platform-impl/common/ext_log_upload_strategies.c +++ b/client/client-multi/client-c/src/kaa/platform-impl/common/ext_log_upload_strategies.c @@ -43,12 +43,6 @@ */ #define KAA_DEFAULT_UPLOAD_COUNT_THRESHOLD 64 -/** - * @brief The default value (in bytes) for the maximum size of the report pack that - * will be delivered in a single request to the Operaions server. - */ -#define KAA_DEFAULT_BATCH_SIZE 8 * 1024 - /** * @brief The default value for Max amount of log batches allowed to be uploaded parallel. */ @@ -91,7 +85,6 @@ kaa_error_t ext_log_upload_strategy_create(struct kaa_context_s *context, void * KAA_RETURN_IF_ERR( ext_log_upload_strategy_set_threshold_count(strategy, KAA_DEFAULT_UPLOAD_COUNT_THRESHOLD) ); KAA_RETURN_IF_ERR( ext_log_upload_strategy_set_upload_timeout(strategy, KAA_DEFAULT_UPLOAD_TIMEOUT) ); KAA_RETURN_IF_ERR( ext_log_upload_strategy_set_upload_retry_period(strategy, KAA_DEFAULT_RETRY_PERIOD) ); - KAA_RETURN_IF_ERR( ext_log_upload_strategy_set_batch_size(strategy, KAA_DEFAULT_BATCH_SIZE) ); KAA_RETURN_IF_ERR( ext_log_upload_strategy_set_max_parallel_uploads(strategy, KAA_DEFAULT_MAX_PARALLEL_UPLOADS) ); strategy->type = type; @@ -144,16 +137,6 @@ ext_log_upload_decision_t ext_log_upload_strategy_decide(void *context, const vo return decision; } - - -size_t ext_log_upload_strategy_get_bucket_size(void *context) -{ - KAA_RETURN_IF_NIL(context, 0); - return ((ext_log_upload_strategy_t *)context)->log_batch_size; -} - - - size_t ext_log_upload_strategy_get_timeout(void *context) { KAA_RETURN_IF_NIL(context, 0); diff --git a/client/client-multi/client-c/src/kaa/platform-impl/common/ext_log_upload_strategies.h b/client/client-multi/client-c/src/kaa/platform-impl/common/ext_log_upload_strategies.h index 5c5a3158b4..83962e201a 100644 --- a/client/client-multi/client-c/src/kaa/platform-impl/common/ext_log_upload_strategies.h +++ b/client/client-multi/client-c/src/kaa/platform-impl/common/ext_log_upload_strategies.h @@ -60,17 +60,6 @@ kaa_error_t ext_log_upload_strategy_set_threshold_count(void *strategy, size_t t -/** - * @brief Sets the new log batch size to the strategy. - * - * @param strategy The strategy instance. - * @param log_batch_size The new log batch size in bytes. - * @return Error code. - */ -kaa_error_t ext_log_upload_strategy_set_batch_size(void *strategy, size_t log_batch_size); - - - /** * @brief Sets the new upload timeout to the strategy. * diff --git a/client/client-multi/client-c/src/kaa/platform-impl/esp8266/esp8266_kaa_client.c b/client/client-multi/client-c/src/kaa/platform-impl/esp8266/esp8266_kaa_client.c index d3d65d8936..7e8bdea5e2 100644 --- a/client/client-multi/client-c/src/kaa/platform-impl/esp8266/esp8266_kaa_client.c +++ b/client/client-multi/client-c/src/kaa/platform-impl/esp8266/esp8266_kaa_client.c @@ -60,6 +60,10 @@ static kaa_service_t OPERATIONS_SERVICES[] = { KAA_SERVICE_PROFILE }; static const int OPERATIONS_SERVICES_COUNT = sizeof(OPERATIONS_SERVICES) / sizeof(kaa_service_t); +/* Logging constraints */ +#define MAX_LOG_COUNT SIZE_MAX +#define MAX_LOG_BUCKET_SIZE (8 * 1024) + struct kaa_client_t { kaa_context_t *context; bool operate; @@ -443,9 +447,15 @@ kaa_error_t kaa_log_collector_init(kaa_client_t *kaa_client) return error_code; } + kaa_log_bucket_constraints_t bucket_sizes = { + .max_bucket_size = MAX_LOG_BUCKET_SIZE, + .max_bucket_log_count = MAX_LOG_COUNT, + }; + error_code = kaa_logging_init(kaa_client->context->log_collector , kaa_client->log_storage_context - , kaa_client->log_upload_strategy_context); + , kaa_client->log_upload_strategy_context + , &bucket_sizes); if (error_code) { KAA_LOG_ERROR(kaa_client->context->logger, error_code,"Failed to init log collector"); return error_code; diff --git a/client/client-multi/client-c/src/kaa/platform-impl/posix/posix_kaa_client.c b/client/client-multi/client-c/src/kaa/platform-impl/posix/posix_kaa_client.c index dab68a51df..c95f5a0aaf 100644 --- a/client/client-multi/client-c/src/kaa/platform-impl/posix/posix_kaa_client.c +++ b/client/client-multi/client-c/src/kaa/platform-impl/posix/posix_kaa_client.c @@ -67,6 +67,9 @@ static kaa_service_t OPERATIONS_SERVICES[] = { KAA_SERVICE_PROFILE static const int OPERATIONS_SERVICES_COUNT = sizeof(OPERATIONS_SERVICES) / sizeof(kaa_service_t); +/* Logging constraints */ +#define MAX_LOG_COUNT SIZE_MAX +#define MAX_LOG_BUCKET_SIZE (8 * 1024) typedef enum { KAA_CLIENT_CHANNEL_STATE_NOT_CONNECTED = 0, @@ -449,9 +452,15 @@ kaa_error_t kaa_log_collector_init(kaa_client_t *kaa_client) return error_code; } + kaa_log_bucket_constraints_t bucket_sizes = { + .max_bucket_size = MAX_LOG_BUCKET_SIZE, + .max_bucket_log_count = MAX_LOG_COUNT, + }; + error_code = kaa_logging_init(kaa_client->kaa_context->log_collector , kaa_client->log_storage_context - , kaa_client->log_upload_strategy_context); + , kaa_client->log_upload_strategy_context + , &bucket_sizes); if (error_code) { KAA_LOG_ERROR(kaa_client->kaa_context->logger, error_code,"Failed to init log collector"); return error_code; diff --git a/client/client-multi/client-c/src/kaa/platform-impl/stm32/leafMapleMini/esp8266/esp8266_kaa_client.c b/client/client-multi/client-c/src/kaa/platform-impl/stm32/leafMapleMini/esp8266/esp8266_kaa_client.c index cdbc43688e..c4baef6d80 100644 --- a/client/client-multi/client-c/src/kaa/platform-impl/stm32/leafMapleMini/esp8266/esp8266_kaa_client.c +++ b/client/client-multi/client-c/src/kaa/platform-impl/stm32/leafMapleMini/esp8266/esp8266_kaa_client.c @@ -70,7 +70,9 @@ static kaa_service_t OPERATIONS_SERVICES[] = { KAA_SERVICE_PROFILE , KAA_SERVICE_LOGGING}; static const int OPERATIONS_SERVICES_COUNT = sizeof(OPERATIONS_SERVICES) / sizeof(kaa_service_t); - +/* Logging constraints */ +#define MAX_LOG_COUNT SIZE_MAX +#define MAX_LOG_BUCKET_SIZE (2 * 1024) typedef enum { KAA_CLIENT_ESP8266_STATE_UNINITED = 0, @@ -681,9 +683,15 @@ kaa_error_t kaa_log_collector_init(kaa_client_t *kaa_client) return error_code; } + kaa_log_bucket_constraints_t bucket_sizes = { + .max_bucket_size = MAX_LOG_BUCKET_SIZE, + .max_bucket_log_count = MAX_LOG_COUNT, + }; + error_code = kaa_logging_init(kaa_client->kaa_context->log_collector , kaa_client->log_storage_context - , kaa_client->log_upload_strategy_context); + , kaa_client->log_upload_strategy_context + , &bucket_sizes); if (error_code) { KAA_LOG_ERROR(kaa_client->kaa_context->logger, error_code,"Failed to logging init"); return error_code; diff --git a/client/client-multi/client-c/src/kaa/platform/ext_log_upload_strategy.h b/client/client-multi/client-c/src/kaa/platform/ext_log_upload_strategy.h index 5679bd6ddf..39c98ff621 100644 --- a/client/client-multi/client-c/src/kaa/platform/ext_log_upload_strategy.h +++ b/client/client-multi/client-c/src/kaa/platform/ext_log_upload_strategy.h @@ -71,16 +71,6 @@ kaa_error_t ext_log_upload_strategy_create(struct kaa_context_s *context, void * */ ext_log_upload_decision_t ext_log_upload_strategy_decide(void *context, const void *log_storage_context); -/** - * @brief Retrieves the maximum size of a report pack that will be delivered in a single request to the Operations server. - * - * @param[in] context Log upload strategy context. - * @return The size of a batch in bytes. - */ -size_t ext_log_upload_strategy_get_bucket_size(void *context); - - - /** * @brief The maximum time to wait a log delivery response. * diff --git a/client/client-multi/client-c/test/kaa_test.h b/client/client-multi/client-c/test/kaa_test.h index 23da32250e..9924fa96e7 100644 --- a/client/client-multi/client-c/test/kaa_test.h +++ b/client/client-multi/client-c/test/kaa_test.h @@ -140,7 +140,7 @@ typedef int (*cleanup_fn)(void); int GROUP##_group_setup() /* Defines a teardown process for given group -/* Reverts any changes made by setup routine and makes sure no side effects + * Reverts any changes made by setup routine and makes sure no side effects * will stay after test */ #define KAA_GROUP_TEARDOWN(GROUP) \ diff --git a/client/client-multi/client-c/test/platform-impl/test_ext_log_storage_memory.c b/client/client-multi/client-c/test/platform-impl/test_ext_log_storage_memory.c index c8095377ea..10bb5d1f49 100644 --- a/client/client-multi/client-c/test/platform-impl/test_ext_log_storage_memory.c +++ b/client/client-multi/client-c/test/platform-impl/test_ext_log_storage_memory.c @@ -17,6 +17,7 @@ #include #include #include +#include #include "../kaa_test.h" diff --git a/client/client-multi/client-c/test/platform-impl/test_ext_log_upload_strategies.c b/client/client-multi/client-c/test/platform-impl/test_ext_log_upload_strategies.c index 1bd81114f6..0c9a8e76c1 100644 --- a/client/client-multi/client-c/test/platform-impl/test_ext_log_upload_strategies.c +++ b/client/client-multi/client-c/test/platform-impl/test_ext_log_upload_strategies.c @@ -120,25 +120,6 @@ void test_set_upload_timeout(void) KAA_TRACE_OUT(logger); } -void test_set_batch_size(void) -{ - KAA_TRACE_IN(logger); - - kaa_error_t error_code = KAA_ERR_NONE; - - size_t DEFAULT_BATCH_SIZE = 8 * 1024; - - error_code = ext_log_upload_strategy_change_strategy(strategy, KAA_LOG_UPLOAD_VOLUME_STRATEGY); - ASSERT_EQUAL(error_code, KAA_ERR_NONE); - - error_code = ext_log_upload_strategy_set_batch_size(strategy, DEFAULT_BATCH_SIZE); - ASSERT_EQUAL(error_code, KAA_ERR_NONE); - - ASSERT_EQUAL(ext_log_upload_strategy_get_bucket_size(strategy), DEFAULT_BATCH_SIZE); - - KAA_TRACE_OUT(logger); -} - void test_upload_decision_by_volume(void) { KAA_TRACE_IN(logger); @@ -360,7 +341,6 @@ int test_deinit(void) KAA_SUITE_MAIN(MetaExtension, test_init, test_deinit, KAA_TEST_CASE(create_strategy, test_create_strategy) KAA_TEST_CASE(set_upload_timeout, test_set_upload_timeout) - KAA_TEST_CASE(set_batch_size, test_set_batch_size) KAA_TEST_CASE(upload_decision_by_volume, test_upload_decision_by_volume) KAA_TEST_CASE(upload_decision_by_count, test_upload_decision_by_count) KAA_TEST_CASE(upload_decision_by_timeout, test_upload_decision_by_timeout) diff --git a/client/client-multi/client-c/test/test_kaa_common_schema.c b/client/client-multi/client-c/test/test_kaa_common_schema.c index 7bc2f45c07..faf29c98bc 100644 --- a/client/client-multi/client-c/test/test_kaa_common_schema.c +++ b/client/client-multi/client-c/test/test_kaa_common_schema.c @@ -17,6 +17,7 @@ #include #include #include +#include #include "kaa_test.h" diff --git a/client/client-multi/client-c/test/test_kaa_event.c b/client/client-multi/client-c/test/test_kaa_event.c index f69024faa8..955e4c1dfe 100644 --- a/client/client-multi/client-c/test/test_kaa_event.c +++ b/client/client-multi/client-c/test/test_kaa_event.c @@ -521,7 +521,7 @@ void test_event_blocks(void) kaa_event_block_id trx_id = 0; error_code = kaa_event_create_transaction(event_manager, &trx_id); ASSERT_EQUAL(error_code, KAA_ERR_NONE); - ASSERT_NOT_NULL(trx_id); + ASSERT_TRUE(trx_id); const size_t event1_size = 6; char *event1 = (char *) KAA_MALLOC(event1_size + 1); diff --git a/client/client-multi/client-c/test/test_kaa_log.c b/client/client-multi/client-c/test/test_kaa_log.c index ccd55adfc6..51267d2301 100644 --- a/client/client-multi/client-c/test/test_kaa_log.c +++ b/client/client-multi/client-c/test/test_kaa_log.c @@ -82,7 +82,6 @@ static const int OPERATIONS_SERVICES_COUNT = sizeof(OPERATIONS_SERVICES) / sizeo typedef struct { size_t timeout; - size_t batch_size; size_t max_parallel_uploads; bool on_timeout_count; bool on_failure_count; @@ -171,11 +170,6 @@ ext_log_upload_decision_t ext_log_upload_strategy_decide(void *context, const vo return ((mock_strategy_context_t *)context)->decision; } -size_t ext_log_upload_strategy_get_bucket_size(void *context) -{ - return ((mock_strategy_context_t *)context)->batch_size; -} - size_t ext_log_upload_strategy_get_timeout(void *context) { return ((mock_strategy_context_t *)context)->timeout; @@ -328,10 +322,14 @@ void test_create_request(void) mock_strategy_context_t strategy; memset(&strategy, 0, sizeof(mock_strategy_context_t)); strategy.decision = NOOP; - strategy.batch_size = 2 * test_log_record_size; strategy.max_parallel_uploads = UINT32_MAX; - error_code = kaa_logging_init(log_collector, create_mock_storage(), &strategy); + kaa_log_bucket_constraints_t constraints = { + .max_bucket_size = 2 * test_log_record_size, + .max_bucket_log_count = UINT32_MAX, + }; + + error_code = kaa_logging_init(log_collector, create_mock_storage(), &strategy, &constraints); ASSERT_EQUAL(error_code, KAA_ERR_NONE); error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); @@ -400,7 +398,13 @@ void test_response(void) memset(&strategy, 0, sizeof(mock_strategy_context_t)); mock_storage_context_t *storage = create_mock_storage(); - error_code = kaa_logging_init(log_collector, storage, &strategy); + + kaa_log_bucket_constraints_t constraints = { + .max_bucket_size = 1024, + .max_bucket_log_count = UINT32_MAX, + }; + + error_code = kaa_logging_init(log_collector, storage, &strategy, &constraints); ASSERT_EQUAL(error_code, KAA_ERR_NONE); uint32_t response_count = 2; @@ -467,10 +471,14 @@ void test_timeout(void) memset(&strategy, 0, sizeof(mock_strategy_context_t)); strategy.timeout = TEST_TIMEOUT; strategy.decision = NOOP; - strategy.batch_size = 2 * test_log_record_size; strategy.max_parallel_uploads = UINT32_MAX; - error_code = kaa_logging_init(log_collector, create_mock_storage(), &strategy); + kaa_log_bucket_constraints_t constraints = { + .max_bucket_size = 2 * test_log_record_size, + .max_bucket_log_count = UINT32_MAX, + }; + + error_code = kaa_logging_init(log_collector, create_mock_storage(), &strategy, &constraints); ASSERT_EQUAL(error_code, KAA_ERR_NONE); error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); @@ -519,13 +527,17 @@ void test_decline_timeout(void) memset(&strategy, 0, sizeof(mock_strategy_context_t)); strategy.timeout = TEST_TIMEOUT; strategy.decision = NOOP; - strategy.batch_size = 2 * test_log_record_size; strategy.max_parallel_uploads = UINT32_MAX; mock_storage_context_t *storage = create_mock_storage(); ASSERT_NOT_NULL(storage); - error_code = kaa_logging_init(log_collector, storage, &strategy); + kaa_log_bucket_constraints_t constraints = { + .max_bucket_size = 2 * test_log_record_size, + .max_bucket_log_count = UINT32_MAX, + }; + + error_code = kaa_logging_init(log_collector, storage, &strategy, &constraints); ASSERT_EQUAL(error_code, KAA_ERR_NONE); error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); @@ -568,12 +580,12 @@ void test_decline_timeout(void) error_code = kaa_logging_handle_server_sync(log_collector, reader, 0, response_buffer_size); ASSERT_EQUAL(error_code, KAA_ERR_NONE); - ASSERT_NOT_NULL(storage->on_remove_by_id_count); + ASSERT_TRUE(storage->on_remove_by_id_count); error_code = kaa_logging_add_record(log_collector, test_log_record, NULL); ASSERT_EQUAL(error_code, KAA_ERR_NONE); - ASSERT_NULL(strategy.on_timeout_count); + ASSERT_FALSE(strategy.on_timeout_count); test_log_record->destroy(test_log_record); kaa_platform_message_writer_destroy(writer); @@ -606,14 +618,17 @@ void test_max_parallel_uploads_with_log_sync(void) mock_strategy_context_t strategy; memset(&strategy, 0, sizeof(mock_strategy_context_t)); strategy.timeout = UINT32_MAX; - strategy.batch_size = 2 * test_log_size; strategy.decision = UPLOAD; - mock_storage_context_t *storage = create_mock_storage(); ASSERT_NOT_NULL(storage); - error_code = kaa_logging_init(log_collector, storage, &strategy); + kaa_log_bucket_constraints_t constraints = { + .max_bucket_size = 2 * test_log_size, + .max_bucket_log_count = UINT32_MAX, + }; + + error_code = kaa_logging_init(log_collector, storage, &strategy, &constraints); ASSERT_EQUAL(error_code, KAA_ERR_NONE); /* @@ -688,13 +703,17 @@ void test_max_parallel_uploads_with_sync_all(void) mock_strategy_context_t strategy; memset(&strategy, 0, sizeof(mock_strategy_context_t)); strategy.timeout = UINT32_MAX; - strategy.batch_size = 2 * test_log_size; strategy.decision = UPLOAD; mock_storage_context_t *storage = create_mock_storage(); ASSERT_NOT_NULL(storage); - error_code = kaa_logging_init(log_collector, storage, &strategy); + kaa_log_bucket_constraints_t constraints = { + .max_bucket_size = 2 * test_log_size, + .max_bucket_log_count = UINT32_MAX, + }; + + error_code = kaa_logging_init(log_collector, storage, &strategy, &constraints); ASSERT_EQUAL(error_code, KAA_ERR_NONE); /* @@ -706,7 +725,7 @@ void test_max_parallel_uploads_with_sync_all(void) size_t expected_size = 0; error_code = kaa_logging_request_get_size(log_collector, &expected_size); - ASSERT_NULL(expected_size); + ASSERT_FALSE(expected_size); /* * Ensure the first request is allowed. @@ -719,7 +738,7 @@ void test_max_parallel_uploads_with_sync_all(void) * Do the first request to remember the delivery timeout of the log batch. */ error_code = kaa_logging_request_get_size(log_collector, &expected_size); - ASSERT_NOT_NULL(expected_size); + ASSERT_TRUE(expected_size); size_t request_buffer_size = 256; char request_buffer[request_buffer_size]; kaa_platform_message_writer_t *writer = NULL; @@ -736,7 +755,7 @@ void test_max_parallel_uploads_with_sync_all(void) * Ensure the second request is forbidden. */ error_code = kaa_logging_request_get_size(log_collector, &expected_size); - ASSERT_NULL(expected_size); + ASSERT_FALSE(expected_size); /* * Clean up. @@ -879,7 +898,13 @@ KAA_GROUP_SETUP(log_callback_basic) memset(&strategy, 0, sizeof(strategy)); - error_code = kaa_logging_init(log_collector, create_mock_storage(), &strategy); + kaa_log_bucket_constraints_t constraints = { + .max_bucket_size = 2 * test_log_record_size, + .max_bucket_log_count = UINT32_MAX, + }; + + error_code = kaa_logging_init(log_collector, create_mock_storage(), + &strategy, &constraints); ASSERT_EQUAL(error_code, KAA_ERR_NONE); expected_ctx = NULL; @@ -981,9 +1006,6 @@ KAA_GROUP_SETUP(log_callback_with_storage) memset(test_reader_buffer, 0, sizeof(test_reader_buffer)); memset(test_writer_buffer, 0, sizeof(test_writer_buffer)); - error_code = kaa_logging_init(log_collector, create_mock_storage(), &strategy); - ASSERT_EQUAL(error_code, KAA_ERR_NONE); - expected_ctx = NULL; expected_bucked_id = 0; call_is_expected = 0; @@ -995,9 +1017,16 @@ KAA_GROUP_SETUP(log_callback_with_storage) check_bucket = 0; storage = create_mock_storage(); - error_code = kaa_logging_init(log_collector, storage, &strategy); + kaa_log_bucket_constraints_t constraints = { + .max_bucket_size = 2 * test_log_record_size, + .max_bucket_log_count = UINT32_MAX, + }; + + error_code = kaa_logging_init(log_collector, storage, + &strategy, &constraints); ASSERT_EQUAL(error_code, KAA_ERR_NONE); + uint32_t response_count = 2; struct response_packet *response = (struct response_packet *) test_reader_buffer; *(uint32_t *) response->resp_cnt = KAA_HTONL(response_count); @@ -1204,11 +1233,12 @@ KAA_GROUP_SETUP(log_callback_with_storage_and_strategy) memset(&strategy, 0, sizeof(mock_strategy_context_t)); strategy.timeout = TEST_TIMEOUT; strategy.decision = NOOP; - strategy.batch_size = TEST_BUFFER_SIZE; strategy.max_parallel_uploads = UINT32_MAX; - error_code = kaa_logging_init(log_collector, create_mock_storage(), &strategy); - ASSERT_EQUAL(error_code, KAA_ERR_NONE); + kaa_log_bucket_constraints_t constraints = { + .max_bucket_size = TEST_BUFFER_SIZE, + .max_bucket_log_count = UINT32_MAX, + }; expected_ctx = NULL; expected_bucked_id = 0; @@ -1221,7 +1251,9 @@ KAA_GROUP_SETUP(log_callback_with_storage_and_strategy) check_bucket = 0; storage = create_mock_storage(); - error_code = kaa_logging_init(log_collector, storage, &strategy); + error_code = kaa_logging_init(log_collector, create_mock_storage(), + &strategy, &constraints); + ASSERT_EQUAL(error_code, KAA_ERR_NONE); uint32_t response_count = 2; diff --git a/client/client-multi/client-c/test/test_platform_protocol.c b/client/client-multi/client-c/test/test_platform_protocol.c index e6f749df87..19326e11a9 100644 --- a/client/client-multi/client-c/test/test_platform_protocol.c +++ b/client/client-multi/client-c/test/test_platform_protocol.c @@ -73,7 +73,13 @@ void test_empty_log_collector_extension_count(void) , KAA_LOG_UPLOAD_VOLUME_STRATEGY); ASSERT_EQUAL(error_code, KAA_ERR_NONE); - error_code = kaa_logging_init(kaa_context->log_collector, log_storage_context, log_upload_strategy_context); + + kaa_log_bucket_constraints_t constraints = { + .max_bucket_size = 1024, + .max_bucket_log_count = UINT32_MAX, + }; + + error_code = kaa_logging_init(kaa_context->log_collector, log_storage_context, log_upload_strategy_context, &constraints); ASSERT_EQUAL(error_code, KAA_ERR_NONE); error_code = kaa_platform_protocol_serialize_client_sync(kaa_context->platform_protocol, info, &buffer, &buffer_size); From da4cde9221fbaf048c267665f7f0daa817893828 Mon Sep 17 00:00:00 2001 From: Max Payne Date: Wed, 20 Jan 2016 18:53:23 +0200 Subject: [PATCH 15/18] KAA-773 Add log strategy and log storage setters Tests are included --- .../client-c/src/kaa/kaa_logging.c | 46 ++++- .../client-c/src/kaa/kaa_logging.h | 27 +++ .../client-multi/client-c/test/test_kaa_log.c | 171 ++++++++++++++++-- 3 files changed, 217 insertions(+), 27 deletions(-) diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.c b/client/client-multi/client-c/src/kaa/kaa_logging.c index 4486cd8fe2..59c1e62e75 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.c +++ b/client/client-multi/client-c/src/kaa/kaa_logging.c @@ -36,6 +36,8 @@ #define KAA_LOGGING_RECEIVE_UPDATES_FLAG 0x01 #define KAA_MAX_PADDING_LENGTH (KAA_ALIGNMENT - 1) +#define KAA_MAX_LOGS_IN_BUCKET 64 +#define KAA_MAX_BUCKET_SIZE 1024 extern kaa_transport_channel_interface_t *kaa_channel_manager_get_transport_channel(kaa_channel_manager_t *self , kaa_service_t service_type); @@ -207,14 +209,18 @@ kaa_error_t kaa_log_collector_create(kaa_log_collector_t **log_collector_p kaa_log_collector_t * collector = (kaa_log_collector_t *) KAA_MALLOC(sizeof(kaa_log_collector_t)); KAA_RETURN_IF_NIL(collector, KAA_ERR_NOMEM); - collector->log_bucket_id = 0; - collector->log_last_id = 0; - collector->log_storage_context = NULL; - collector->log_upload_strategy_context = NULL; - collector->status = status; - collector->channel_manager = channel_manager; - collector->logger = logger; - collector->is_sync_ignored = false; + collector->log_bucket_id = 0; + collector->log_last_id = 0; + collector->log_storage_context = NULL; + collector->log_upload_strategy_context = NULL; + collector->status = status; + collector->channel_manager = channel_manager; + collector->logger = logger; + collector->is_sync_ignored = false; + + /* Default values */ + collector->bucket_size.max_bucket_log_count = KAA_MAX_LOGS_IN_BUCKET; + collector->bucket_size.max_bucket_size = KAA_MAX_BUCKET_SIZE; collector->timeouts = kaa_list_create(); if (!collector->timeouts) { @@ -227,7 +233,6 @@ kaa_error_t kaa_log_collector_create(kaa_log_collector_t **log_collector_p } - kaa_error_t kaa_logging_init(kaa_log_collector_t *self, void *log_storage_context, void *log_upload_strategy_context, const kaa_log_bucket_constraints_t *bucket_sizes) { KAA_RETURN_IF_NIL3(self, log_storage_context, log_upload_strategy_context, KAA_ERR_BADPARAM); @@ -247,6 +252,29 @@ kaa_error_t kaa_logging_init(kaa_log_collector_t *self, void *log_storage_contex } +kaa_error_t kaa_logging_set_strategy(kaa_log_collector_t *self, void *log_upload_strategy_context) +{ + KAA_RETURN_IF_NIL2(self, log_upload_strategy_context, KAA_ERR_BADPARAM); + + if (self->log_upload_strategy_context) + ext_log_upload_strategy_destroy(self->log_upload_strategy_context); + + self->log_upload_strategy_context = log_upload_strategy_context; + + return KAA_ERR_NONE; +} + +kaa_error_t kaa_logging_set_storage(kaa_log_collector_t *self, void *log_storage_context) +{ + KAA_RETURN_IF_NIL2(self, log_storage_context, KAA_ERR_BADPARAM); + + if (self->log_storage_context) + ext_log_storage_destroy(self->log_storage_context); + + self->log_storage_context = log_storage_context; + + return KAA_ERR_NONE; +} static void do_sync(kaa_log_collector_t *self) { diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.h b/client/client-multi/client-c/src/kaa/kaa_logging.h index 687330a429..088d2d48be 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.h +++ b/client/client-multi/client-c/src/kaa/kaa_logging.h @@ -77,6 +77,33 @@ typedef struct { */ kaa_error_t kaa_logging_init(kaa_log_collector_t *self, void *log_storage_context, void *log_upload_strategy_context, const kaa_log_bucket_constraints_t *bucket_sizes); +/** + * @brief Sets custom strategy for given collector. + * + * If a strategy has been assigned to collector previously then it will be + * destroyed and new strategy will be assigned. + * + * @param[in] self Pointer to a @link kaa_log_collector_t @endlink instance. + * @param[in] log_storage_context Log storage context. + * + * @return Error code. + */ +kaa_error_t kaa_logging_set_strategy(kaa_log_collector_t *self, void *log_upload_strategy_context); + +/** + * @brief Sets custom storage for given collector. + * + * If a storage has been assigned to collector previously then it will be + * destroyed and new storage will be assigned. Be aware that all items from + * previous storage will be deleted. + * + * @param[in] self Pointer to a @link kaa_log_collector_t @endlink instance. + * @param[in] log_storage_context Log storage context. + * + * @return Error code. + */ +kaa_error_t kaa_logging_set_storage(kaa_log_collector_t *self, void *log_storage_context); + /** * @brief Serializes and adds a log record to the log storage. * diff --git a/client/client-multi/client-c/test/test_kaa_log.c b/client/client-multi/client-c/test/test_kaa_log.c index 51267d2301..1311919b0e 100644 --- a/client/client-multi/client-c/test/test_kaa_log.c +++ b/client/client-multi/client-c/test/test_kaa_log.c @@ -769,8 +769,6 @@ void test_max_parallel_uploads_with_sync_all(void) KAA_TRACE_OUT(logger); } -#endif - /* ---------------------------------------------------------------------------*/ /* Log delivery tests */ /* ---------------------------------------------------------------------------*/ @@ -797,8 +795,10 @@ struct response_packet #define TEST_EXT_OP 0 /* Simple stub */ #define TEST_TIMEOUT 2 -static mock_strategy_context_t strategy; -static mock_storage_context_t *storage; +static mock_strategy_context_t test_strategy1; +static mock_strategy_context_t test_strategy2; +static mock_storage_context_t *test_storage1; +static mock_storage_context_t *test_storage2; static kaa_log_collector_t *log_collector; static size_t test_log_record_size = TEST_BUFFER_SIZE; /* Will contain response_packet. Thus required to be aligned. */ @@ -877,6 +877,135 @@ static void mock_log_event_timeout_fn(void *ctx, const kaa_log_bucket_info_t *bu timeout_call_completed++; } +/* ---------------------------------------------------------------------------*/ +/* Log setters test group */ +/* ---------------------------------------------------------------------------*/ + +KAA_GROUP_SETUP(log_setters) +{ + kaa_error_t rc; + + KAA_TRACE_IN(logger); + rc = kaa_log_collector_create(&log_collector, + status, + channel_manager, + logger); + + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + /* Test objects for strategy test */ + + memset(&test_strategy1, 0, sizeof(test_strategy1)); + memset(&test_strategy2, 0, sizeof(test_strategy2)); + + kaa_log_bucket_constraints_t constraints = { + .max_bucket_size = 2 * test_log_record_size, + .max_bucket_log_count = UINT32_MAX, + }; + + /* Test objects for storage tests */ + + test_storage1 = create_mock_storage(); + test_storage2 = create_mock_storage(); + + rc = kaa_logging_init(log_collector, test_storage1, + &test_strategy1, &constraints); + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + expected_ctx = NULL; + expected_bucked_id = 0; + call_is_expected = 0; + + KAA_TRACE_OUT(logger); + return 0; + +} + +KAA_GROUP_TEARDOWN(log_setters) +{ + KAA_TRACE_IN(logger); + + /* If tests will pass, one of storages will be destroyed. + * Each test in this will decide which one should be deleted. */ + kaa_log_collector_destroy(log_collector); + log_collector = NULL; + + KAA_TRACE_OUT(logger); + return 0; + +} + + +KAA_TEST_CASE_EX(log_setters, set_strategy_invalid_parameters) +{ + kaa_error_t rc; + + KAA_TRACE_IN(logger); + + rc = kaa_logging_set_strategy(log_collector, NULL); + ASSERT_EQUAL(KAA_ERR_BADPARAM, rc); + + rc = kaa_logging_set_strategy(NULL, &test_strategy2); + ASSERT_EQUAL(KAA_ERR_BADPARAM, rc); + + ext_log_storage_destroy(test_storage2); + ext_log_upload_strategy_destroy(&test_strategy2); + + KAA_TRACE_OUT(logger); +} + + +KAA_TEST_CASE_EX(log_setters, set_storage_invalid_parameters) +{ + kaa_error_t rc; + KAA_TRACE_IN(logger); + + rc = kaa_logging_set_storage(log_collector, NULL); + ASSERT_EQUAL(KAA_ERR_BADPARAM, rc); + + rc = kaa_logging_set_storage(NULL, test_storage2); + ASSERT_EQUAL(KAA_ERR_BADPARAM, rc); + + ext_log_storage_destroy(test_storage2); + ext_log_upload_strategy_destroy(&test_strategy2); + + KAA_TRACE_OUT(logger); +} + +KAA_TEST_CASE_EX(log_setters, set_strategy_valid_parameters) +{ + kaa_error_t rc; + + KAA_TRACE_IN(logger); + + /* First strategy will be internally deleted */ + rc = kaa_logging_set_strategy(log_collector, &test_strategy2); + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + ext_log_storage_destroy(test_storage2); + + /* Second strategy will be deleted on test teardown */ + + KAA_TRACE_OUT(logger); +} + + +KAA_TEST_CASE_EX(log_setters, set_storage_valid_parameters) +{ + kaa_error_t rc; + + KAA_TRACE_IN(logger); + + /* First storage will be internally deleted */ + rc = kaa_logging_set_storage(log_collector, test_storage2); + ASSERT_EQUAL(KAA_ERR_NONE, rc); + + ext_log_upload_strategy_destroy(&test_strategy2); + + /* Second storage will be deleted on test teardown */ + + KAA_TRACE_OUT(logger); +} /* ---------------------------------------------------------------------------*/ @@ -896,7 +1025,7 @@ KAA_GROUP_SETUP(log_callback_basic) ASSERT_EQUAL(error_code, KAA_ERR_NONE); - memset(&strategy, 0, sizeof(strategy)); + memset(&test_strategy1, 0, sizeof(test_strategy1)); kaa_log_bucket_constraints_t constraints = { .max_bucket_size = 2 * test_log_record_size, @@ -904,14 +1033,14 @@ KAA_GROUP_SETUP(log_callback_basic) }; error_code = kaa_logging_init(log_collector, create_mock_storage(), - &strategy, &constraints); + &test_strategy1, &constraints); ASSERT_EQUAL(error_code, KAA_ERR_NONE); expected_ctx = NULL; expected_bucked_id = 0; call_is_expected = 0; - KAA_TRACE_IN(logger); + KAA_TRACE_OUT(logger); return 0; } @@ -921,7 +1050,7 @@ KAA_GROUP_TEARDOWN(log_callback_basic) kaa_log_collector_destroy(log_collector); log_collector = NULL; - KAA_TRACE_IN(logger); + KAA_TRACE_OUT(logger); return 0; } @@ -1002,7 +1131,7 @@ KAA_GROUP_SETUP(log_callback_with_storage) ASSERT_EQUAL(error_code, KAA_ERR_NONE); - memset(&strategy, 0, sizeof(mock_strategy_context_t)); + memset(&test_strategy1, 0, sizeof(mock_strategy_context_t)); memset(test_reader_buffer, 0, sizeof(test_reader_buffer)); memset(test_writer_buffer, 0, sizeof(test_writer_buffer)); @@ -1016,14 +1145,14 @@ KAA_GROUP_SETUP(log_callback_with_storage) success_call_is_expected = 0; check_bucket = 0; - storage = create_mock_storage(); + test_storage1 = create_mock_storage(); kaa_log_bucket_constraints_t constraints = { .max_bucket_size = 2 * test_log_record_size, .max_bucket_log_count = UINT32_MAX, }; - error_code = kaa_logging_init(log_collector, storage, - &strategy, &constraints); + error_code = kaa_logging_init(log_collector, test_storage1, + &test_strategy1, &constraints); ASSERT_EQUAL(error_code, KAA_ERR_NONE); @@ -1230,10 +1359,10 @@ KAA_GROUP_SETUP(log_callback_with_storage_and_strategy) ASSERT_EQUAL(error_code, KAA_ERR_NONE); - memset(&strategy, 0, sizeof(mock_strategy_context_t)); - strategy.timeout = TEST_TIMEOUT; - strategy.decision = NOOP; - strategy.max_parallel_uploads = UINT32_MAX; + memset(&test_strategy1, 0, sizeof(mock_strategy_context_t)); + test_strategy1.timeout = TEST_TIMEOUT; + test_strategy1.decision = NOOP; + test_strategy1.max_parallel_uploads = UINT32_MAX; kaa_log_bucket_constraints_t constraints = { .max_bucket_size = TEST_BUFFER_SIZE, @@ -1250,9 +1379,8 @@ KAA_GROUP_SETUP(log_callback_with_storage_and_strategy) success_call_is_expected = 0; check_bucket = 0; - storage = create_mock_storage(); error_code = kaa_logging_init(log_collector, create_mock_storage(), - &strategy, &constraints); + &test_strategy1, &constraints); ASSERT_EQUAL(error_code, KAA_ERR_NONE); @@ -1365,6 +1493,8 @@ KAA_TEST_CASE_EX(log_callback_with_storage_and_strategy, on_timeout_called) /* End of log delivery test groups */ /* ---------------------------------------------------------------------------*/ +#endif + int test_init(void) { kaa_error_t error = kaa_log_create(&logger, KAA_MAX_LOG_MESSAGE_LENGTH, KAA_MAX_LOG_LEVEL, NULL); @@ -1408,6 +1538,11 @@ KAA_SUITE_MAIN(Log, test_init, test_deinit KAA_TEST_CASE(decline_timeout, test_decline_timeout) KAA_TEST_CASE(max_parallel_uploads_with_log_sync, test_max_parallel_uploads_with_log_sync) KAA_TEST_CASE(max_parallel_uploads_with_sync_all, test_max_parallel_uploads_with_sync_all) + KAA_RUN_TEST(log_setters, set_strategy_invalid_parameters); + KAA_RUN_TEST(log_setters, set_strategy_valid_parameters); + KAA_RUN_TEST(log_setters, set_storage_invalid_parameters); + KAA_RUN_TEST(log_setters, set_storage_valid_parameters); + KAA_RUN_TEST(log_callback_basic, valid_parameters); KAA_RUN_TEST(log_callback_basic, invalid_parameters); KAA_RUN_TEST(log_callback_basic, valid_parameters); KAA_RUN_TEST(log_callback_with_storage, on_success_called); From 3b72afd0e9abf1200015098bd00caca7a56a78df Mon Sep 17 00:00:00 2001 From: Maxim Olender Date: Thu, 21 Jan 2016 17:27:40 +0200 Subject: [PATCH 16/18] KAA-773 Code review comments addressed --- .../client-c/src/kaa/kaa_logging.c | 68 +++++++++---------- .../client-c/src/kaa/kaa_status.c | 2 - .../client-c/src/kaa/kaa_status.h | 1 - .../Econais/EC19D/econais_ec19d_kaa_client.c | 4 +- .../esp8266/esp8266_kaa_client.c | 4 +- .../platform-impl/posix/posix_kaa_client.c | 4 +- .../esp8266/esp8266_kaa_client.c | 4 +- client/client-multi/client-c/test/kaa_test.h | 10 ++- .../client-multi/client-c/test/test_kaa_log.c | 11 --- 9 files changed, 49 insertions(+), 59 deletions(-) diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.c b/client/client-multi/client-c/src/kaa/kaa_logging.c index 59c1e62e75..ab74534e8f 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.c +++ b/client/client-multi/client-c/src/kaa/kaa_logging.c @@ -36,8 +36,6 @@ #define KAA_LOGGING_RECEIVE_UPDATES_FLAG 0x01 #define KAA_MAX_PADDING_LENGTH (KAA_ALIGNMENT - 1) -#define KAA_MAX_LOGS_IN_BUCKET 64 -#define KAA_MAX_BUCKET_SIZE 1024 extern kaa_transport_channel_interface_t *kaa_channel_manager_get_transport_channel(kaa_channel_manager_t *self , kaa_service_t service_type); @@ -66,11 +64,14 @@ struct kaa_log_collector { kaa_log_delivery_listener_t log_delivery_listeners; bool is_sync_ignored; uint32_t log_last_id; /**< Last log record ID */ - uint32_t max_bucket_log_count; - uint32_t max_storage_size; uint16_t log_bucket_id; }; +struct find_bucket_context { + uint16_t bucket_id; + size_t bucket_count; +}; + static const kaa_service_t logging_sync_services[] = {KAA_SERVICE_LOGGING}; kaa_error_t kaa_logging_need_logging_resync(kaa_log_collector_t *self, bool *result) @@ -104,21 +105,32 @@ static kaa_error_t remember_request(kaa_log_collector_t *self, uint16_t bucket_i return KAA_ERR_NONE; } - - static bool find_by_bucket_id(void *data, void *context) { KAA_RETURN_IF_NIL2(data, context, false); - return (((timeout_info_t *)data)->log_bucket_id == *((uint16_t *)context)); -} + struct find_bucket_context *find_context = context; + + if (((timeout_info_t *)data)->log_bucket_id == find_context->bucket_id) { + find_context->bucket_count++; + return true; + } + return false; +} -static kaa_error_t remove_request(kaa_log_collector_t *self, uint16_t bucket_id) +/* Returns amount of items processed if positive. + * Or error if negative */ +static ssize_t remove_request(kaa_log_collector_t *self, uint16_t bucket_id) { KAA_RETURN_IF_NIL(self, KAA_ERR_BADPARAM); - kaa_list_remove_first(self->timeouts, &find_by_bucket_id, &bucket_id, NULL); - return KAA_ERR_NONE; + struct find_bucket_context context = { + .bucket_id = bucket_id, + .bucket_count = 0 + }; + + kaa_list_remove_first(self->timeouts, &find_by_bucket_id, &context, NULL); + return context.bucket_count; } @@ -181,14 +193,6 @@ static bool is_upload_allowed(kaa_log_collector_t *self) return true; } -/* Sums log counters across all buckets */ -static void sum_log_records(void *data, void *context) -{ - uint16_t count = ((timeout_info_t *)data)->log_count; - uint32_t *sum = context; - *sum += count; -} - void kaa_log_collector_destroy(kaa_log_collector_t *self) { KAA_RETURN_IF_NIL(self, ); @@ -218,9 +222,9 @@ kaa_error_t kaa_log_collector_create(kaa_log_collector_t **log_collector_p collector->logger = logger; collector->is_sync_ignored = false; - /* Default values */ - collector->bucket_size.max_bucket_log_count = KAA_MAX_LOGS_IN_BUCKET; - collector->bucket_size.max_bucket_size = KAA_MAX_BUCKET_SIZE; + /* Must be overriden in _init() */ + collector->bucket_size.max_bucket_log_count = 0; + collector->bucket_size.max_bucket_size = 0; collector->timeouts = kaa_list_create(); if (!collector->timeouts) { @@ -347,10 +351,6 @@ kaa_error_t kaa_logging_add_record(kaa_log_collector_t *self, kaa_user_log_recor if (!is_timeout(self)) update_storage(self); - if (!self->log_bucket_id) { - self->log_bucket_id = self->status->log_bucket_id; - } - if (log_info) { log_info->bucket_id = self->log_bucket_id + 1; log_info->log_id = self->log_last_id++; @@ -420,8 +420,6 @@ kaa_error_t kaa_logging_request_serialize(kaa_log_collector_t *self, kaa_platfor return KAA_ERR_WRITE_FAILED; } - if (!self->log_bucket_id) - self->log_bucket_id = self->status->log_bucket_id; ++self->log_bucket_id; *((uint16_t *) tmp_writer.current) = KAA_HTONS(self->log_bucket_id); @@ -525,17 +523,17 @@ kaa_error_t kaa_logging_handle_server_sync(kaa_log_collector_t *self KAA_LOG_WARN(self->logger, KAA_ERR_WRITE_FAILED, "Failed to upload log bucket, id '%u' (delivery error code '%u')", bucket_id, delivery_error_code); } - remove_request(self, bucket_id); - /* Count logs left to upload */ + /* Count logs uploaded */ - uint32_t total_logs = 0; - kaa_list_for_each(kaa_list_begin(self->timeouts), - kaa_list_back(self->timeouts), - sum_log_records, &total_logs); + ssize_t count = remove_request(self, bucket_id); + + if (count < 0) { + return count; + } kaa_log_bucket_info_t log_bucket_info = { - .log_count = total_logs, + .log_count = count, .bucket_id = bucket_id, }; diff --git a/client/client-multi/client-c/src/kaa/kaa_status.c b/client/client-multi/client-c/src/kaa/kaa_status.c index aa916a28f8..7dfc0f368b 100644 --- a/client/client-multi/client-c/src/kaa/kaa_status.c +++ b/client/client-multi/client-c/src/kaa/kaa_status.c @@ -59,7 +59,6 @@ kaa_error_t kaa_status_create(kaa_status_t ** kaa_status_p) READ_BUFFER(read_buf, &kaa_status->event_seq_n, sizeof(kaa_status->event_seq_n)); READ_BUFFER(read_buf, &kaa_status->config_seq_n, sizeof(kaa_status->config_seq_n)); READ_BUFFER(read_buf, &kaa_status->notification_seq_n, sizeof(kaa_status->notification_seq_n)); - READ_BUFFER(read_buf, &kaa_status->log_bucket_id, sizeof(kaa_status->log_bucket_id)); READ_BUFFER(read_buf, kaa_status->endpoint_public_key_hash, SHA_1_DIGEST_LENGTH); READ_BUFFER(read_buf, kaa_status->profile_hash, SHA_1_DIGEST_LENGTH); @@ -153,7 +152,6 @@ kaa_error_t kaa_status_save(kaa_status_t *self) WRITE_BUFFER(&self->event_seq_n, buffer, sizeof(self->event_seq_n)); WRITE_BUFFER(&self->config_seq_n, buffer, sizeof(self->config_seq_n)); WRITE_BUFFER(&self->notification_seq_n, buffer, sizeof(self->notification_seq_n)); - WRITE_BUFFER(&self->log_bucket_id, buffer, sizeof(self->log_bucket_id)); WRITE_BUFFER(self->endpoint_public_key_hash, buffer, SHA_1_DIGEST_LENGTH); WRITE_BUFFER(self->profile_hash, buffer, SHA_1_DIGEST_LENGTH); WRITE_BUFFER(&endpoint_access_token_length, buffer, sizeof(endpoint_access_token_length)); diff --git a/client/client-multi/client-c/src/kaa/kaa_status.h b/client/client-multi/client-c/src/kaa/kaa_status.h index b9376f23cb..bd0af85f91 100644 --- a/client/client-multi/client-c/src/kaa/kaa_status.h +++ b/client/client-multi/client-c/src/kaa/kaa_status.h @@ -38,7 +38,6 @@ typedef struct uint32_t event_seq_n; uint32_t config_seq_n; uint32_t notification_seq_n; - uint16_t log_bucket_id; bool is_registered; bool is_attached; bool is_updated; diff --git a/client/client-multi/client-c/src/kaa/platform-impl/Econais/EC19D/econais_ec19d_kaa_client.c b/client/client-multi/client-c/src/kaa/platform-impl/Econais/EC19D/econais_ec19d_kaa_client.c index 0136869678..ebcb30d39d 100644 --- a/client/client-multi/client-c/src/kaa/platform-impl/Econais/EC19D/econais_ec19d_kaa_client.c +++ b/client/client-multi/client-c/src/kaa/platform-impl/Econais/EC19D/econais_ec19d_kaa_client.c @@ -91,12 +91,14 @@ void print_mem_stat(kaa_client_t *kaa_client); * Strategy-specific configuration parameters used by Kaa log collection feature. */ #define KAA_DEMO_MAX_UPLOAD_THRESHOLD 15 /* Size of collected serialized logs needed to initiate log upload */ -#define KAA_DEMO_MAX_LOG_BUCKET_SIZE 512 /* Max size of a log batch has been sent by SDK during one upload. */ +/* Max size of a log batch has been sent by SDK during one upload. */ +#define KAA_DEMO_MAX_LOG_BUCKET_SIZE (KAA_TCP_CHANNEL_OUT_BUFFER_SIZE >> 3) #define KAA_DEMO_MAX_LOGS_IN_BUCKET 16 /* Max count of logs in one bucket */ #define KAA_DEMO_MAX_CLEANUP_THRESHOLD 100 /* Max size of an inner log storage. If size is exceeded, elder logs will be removed. */ #define KAA_DEMO_LOG_GENERATION_FREQUENCY 3 /* seconds */ +_Static_assert(KAA_DEMO_MAX_LOG_BUCKET_SIZE, "Maximum bucket size cannot be 0!"); /* * Kaa status and public key storage file names. diff --git a/client/client-multi/client-c/src/kaa/platform-impl/esp8266/esp8266_kaa_client.c b/client/client-multi/client-c/src/kaa/platform-impl/esp8266/esp8266_kaa_client.c index 7e8bdea5e2..3dfbbec822 100644 --- a/client/client-multi/client-c/src/kaa/platform-impl/esp8266/esp8266_kaa_client.c +++ b/client/client-multi/client-c/src/kaa/platform-impl/esp8266/esp8266_kaa_client.c @@ -62,7 +62,9 @@ static const int OPERATIONS_SERVICES_COUNT = sizeof(OPERATIONS_SERVICES) / sizeo /* Logging constraints */ #define MAX_LOG_COUNT SIZE_MAX -#define MAX_LOG_BUCKET_SIZE (8 * 1024) +#define MAX_LOG_BUCKET_SIZE (KAA_TCP_CHANNEL_OUT_BUFFER_SIZE >> 3) + +_Static_assert(MAX_LOG_BUCKET_SIZE, "Maximum bucket size cannot be 0!"); struct kaa_client_t { kaa_context_t *context; diff --git a/client/client-multi/client-c/src/kaa/platform-impl/posix/posix_kaa_client.c b/client/client-multi/client-c/src/kaa/platform-impl/posix/posix_kaa_client.c index c95f5a0aaf..32ca0192f4 100644 --- a/client/client-multi/client-c/src/kaa/platform-impl/posix/posix_kaa_client.c +++ b/client/client-multi/client-c/src/kaa/platform-impl/posix/posix_kaa_client.c @@ -69,7 +69,9 @@ static const int OPERATIONS_SERVICES_COUNT = sizeof(OPERATIONS_SERVICES) / sizeo /* Logging constraints */ #define MAX_LOG_COUNT SIZE_MAX -#define MAX_LOG_BUCKET_SIZE (8 * 1024) +#define MAX_LOG_BUCKET_SIZE (KAA_TCP_CHANNEL_OUT_BUFFER_SIZE >> 3) + +_Static_assert(MAX_LOG_BUCKET_SIZE, "Maximum bucket size cannot be 0!"); typedef enum { KAA_CLIENT_CHANNEL_STATE_NOT_CONNECTED = 0, diff --git a/client/client-multi/client-c/src/kaa/platform-impl/stm32/leafMapleMini/esp8266/esp8266_kaa_client.c b/client/client-multi/client-c/src/kaa/platform-impl/stm32/leafMapleMini/esp8266/esp8266_kaa_client.c index c4baef6d80..ef0e602f15 100644 --- a/client/client-multi/client-c/src/kaa/platform-impl/stm32/leafMapleMini/esp8266/esp8266_kaa_client.c +++ b/client/client-multi/client-c/src/kaa/platform-impl/stm32/leafMapleMini/esp8266/esp8266_kaa_client.c @@ -72,7 +72,9 @@ static const int OPERATIONS_SERVICES_COUNT = sizeof(OPERATIONS_SERVICES) / sizeo /* Logging constraints */ #define MAX_LOG_COUNT SIZE_MAX -#define MAX_LOG_BUCKET_SIZE (2 * 1024) +#define MAX_LOG_BUCKET_SIZE (KAA_TCP_CHANNEL_OUT_BUFFER_SIZE >> 3) + +_Static_assert(MAX_LOG_BUCKET_SIZE, "Maximum bucket size cannot be 0!"); typedef enum { KAA_CLIENT_ESP8266_STATE_UNINITED = 0, diff --git a/client/client-multi/client-c/test/kaa_test.h b/client/client-multi/client-c/test/kaa_test.h index 9924fa96e7..e76be87742 100644 --- a/client/client-multi/client-c/test/kaa_test.h +++ b/client/client-multi/client-c/test/kaa_test.h @@ -99,11 +99,9 @@ typedef int (*cleanup_fn)(void); #define KAA_RUN_TEST(GROUP, NAME) \ do { \ if (!init_ret_code) { \ - int rc = GROUP##_group_setup(); \ - ASSERT_EQUAL(0, rc); \ + GROUP##_group_setup(); \ GROUP##_##NAME##_test(); \ - rc = GROUP##_group_teardown(); \ - ASSERT_EQUAL(0, rc); \ + GROUP##_group_teardown(); \ } \ } while (0) @@ -137,14 +135,14 @@ typedef int (*cleanup_fn)(void); * It runs before each test to make sure sytem is in predictable state */ #define KAA_GROUP_SETUP(GROUP) \ - int GROUP##_group_setup() + void GROUP##_group_setup() /* Defines a teardown process for given group * Reverts any changes made by setup routine and makes sure no side effects * will stay after test */ #define KAA_GROUP_TEARDOWN(GROUP) \ - int GROUP##_group_teardown() + void GROUP##_group_teardown() #define KAA_SUITE_MAIN(SUITE_NAME, INIT_FN, CLEANUP_FN, ...) \ diff --git a/client/client-multi/client-c/test/test_kaa_log.c b/client/client-multi/client-c/test/test_kaa_log.c index 1311919b0e..0e95126792 100644 --- a/client/client-multi/client-c/test/test_kaa_log.c +++ b/client/client-multi/client-c/test/test_kaa_log.c @@ -917,8 +917,6 @@ KAA_GROUP_SETUP(log_setters) call_is_expected = 0; KAA_TRACE_OUT(logger); - return 0; - } KAA_GROUP_TEARDOWN(log_setters) @@ -931,8 +929,6 @@ KAA_GROUP_TEARDOWN(log_setters) log_collector = NULL; KAA_TRACE_OUT(logger); - return 0; - } @@ -1041,7 +1037,6 @@ KAA_GROUP_SETUP(log_callback_basic) call_is_expected = 0; KAA_TRACE_OUT(logger); - return 0; } KAA_GROUP_TEARDOWN(log_callback_basic) @@ -1051,7 +1046,6 @@ KAA_GROUP_TEARDOWN(log_callback_basic) log_collector = NULL; KAA_TRACE_OUT(logger); - return 0; } KAA_TEST_CASE_EX(log_callback_basic, invalid_parameters) @@ -1112,7 +1106,6 @@ KAA_TEST_CASE_EX(log_callback_basic, valid_parameters) ASSERT_FALSE(call_completed); KAA_TRACE_OUT(logger); - return; } /* ---------------------------------------------------------------------------*/ @@ -1183,7 +1176,6 @@ KAA_GROUP_SETUP(log_callback_with_storage) ASSERT_NOT_NULL(test_reader); KAA_TRACE_OUT(logger); - return 0; } KAA_GROUP_TEARDOWN(log_callback_with_storage) @@ -1197,7 +1189,6 @@ KAA_GROUP_TEARDOWN(log_callback_with_storage) log_collector = NULL; KAA_TRACE_OUT(logger); - return 0; } KAA_TEST_CASE_EX(log_callback_with_storage, on_fail_called) @@ -1421,7 +1412,6 @@ KAA_GROUP_SETUP(log_callback_with_storage_and_strategy) ASSERT_EQUAL(error_code, KAA_ERR_NONE); KAA_TRACE_OUT(logger); - return 0; } KAA_GROUP_TEARDOWN(log_callback_with_storage_and_strategy) @@ -1439,7 +1429,6 @@ KAA_GROUP_TEARDOWN(log_callback_with_storage_and_strategy) log_collector = NULL; KAA_TRACE_OUT(logger); - return 0; } KAA_TEST_CASE_EX(log_callback_with_storage_and_strategy, on_timeout_called) From ad362f609290a8d548392c8319dba96f6488b2ab Mon Sep 17 00:00:00 2001 From: Maxim Olender Date: Thu, 21 Jan 2016 18:06:20 +0200 Subject: [PATCH 17/18] KAA-773 Fix log count retrieval --- .../client-c/src/kaa/kaa_logging.c | 39 ++++++------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.c b/client/client-multi/client-c/src/kaa/kaa_logging.c index ab74534e8f..e263911b91 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.c +++ b/client/client-multi/client-c/src/kaa/kaa_logging.c @@ -67,11 +67,6 @@ struct kaa_log_collector { uint16_t log_bucket_id; }; -struct find_bucket_context { - uint16_t bucket_id; - size_t bucket_count; -}; - static const kaa_service_t logging_sync_services[] = {KAA_SERVICE_LOGGING}; kaa_error_t kaa_logging_need_logging_resync(kaa_log_collector_t *self, bool *result) @@ -108,10 +103,11 @@ static kaa_error_t remember_request(kaa_log_collector_t *self, uint16_t bucket_i static bool find_by_bucket_id(void *data, void *context) { KAA_RETURN_IF_NIL2(data, context, false); - struct find_bucket_context *find_context = context; + timeout_info_t *timeout_to_find = context; + timeout_info_t *timeout_info = data; - if (((timeout_info_t *)data)->log_bucket_id == find_context->bucket_id) { - find_context->bucket_count++; + if (timeout_info->log_bucket_id == timeout_to_find->log_bucket_id) { + *timeout_to_find = *timeout_info; return true; } @@ -119,22 +115,18 @@ static bool find_by_bucket_id(void *data, void *context) } -/* Returns amount of items processed if positive. - * Or error if negative */ -static ssize_t remove_request(kaa_log_collector_t *self, uint16_t bucket_id) +/* Returns amount of logs in bucket */ +static size_t remove_request(kaa_log_collector_t *self, uint16_t bucket_id) { KAA_RETURN_IF_NIL(self, KAA_ERR_BADPARAM); - struct find_bucket_context context = { - .bucket_id = bucket_id, - .bucket_count = 0 + timeout_info_t target_timeout = { + .log_bucket_id = bucket_id, }; - kaa_list_remove_first(self->timeouts, &find_by_bucket_id, &context, NULL); - return context.bucket_count; + kaa_list_remove_first(self->timeouts, &find_by_bucket_id, &target_timeout, NULL); + return target_timeout.log_count; } - - static bool is_timeout(kaa_log_collector_t *self) { KAA_RETURN_IF_NIL2(self, self->timeouts, false); @@ -523,17 +515,10 @@ kaa_error_t kaa_logging_handle_server_sync(kaa_log_collector_t *self KAA_LOG_WARN(self->logger, KAA_ERR_WRITE_FAILED, "Failed to upload log bucket, id '%u' (delivery error code '%u')", bucket_id, delivery_error_code); } - - /* Count logs uploaded */ - - ssize_t count = remove_request(self, bucket_id); - - if (count < 0) { - return count; - } + size_t uploaded_count = remove_request(self, bucket_id); kaa_log_bucket_info_t log_bucket_info = { - .log_count = count, + .log_count = uploaded_count, .bucket_id = bucket_id, }; From 5bc2f0e18294937d0433855b8cf414e63b4133fc Mon Sep 17 00:00:00 2001 From: Maxim Olender Date: Mon, 25 Jan 2016 19:42:15 +0200 Subject: [PATCH 18/18] KAA-773 Improve timeouts list traversal --- .../client-c/src/kaa/kaa_logging.c | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/client/client-multi/client-c/src/kaa/kaa_logging.c b/client/client-multi/client-c/src/kaa/kaa_logging.c index e263911b91..e6479fed19 100644 --- a/client/client-multi/client-c/src/kaa/kaa_logging.c +++ b/client/client-multi/client-c/src/kaa/kaa_logging.c @@ -19,6 +19,7 @@ #include #include #include +#include #include "platform/stdio.h" #include "platform/sock.h" #include "platform/time.h" @@ -103,11 +104,10 @@ static kaa_error_t remember_request(kaa_log_collector_t *self, uint16_t bucket_i static bool find_by_bucket_id(void *data, void *context) { KAA_RETURN_IF_NIL2(data, context, false); - timeout_info_t *timeout_to_find = context; + uint16_t bucket_id = *(uint16_t *) context; timeout_info_t *timeout_info = data; - if (timeout_info->log_bucket_id == timeout_to_find->log_bucket_id) { - *timeout_to_find = *timeout_info; + if (timeout_info->log_bucket_id == bucket_id) { return true; } @@ -118,13 +118,23 @@ static bool find_by_bucket_id(void *data, void *context) /* Returns amount of logs in bucket */ static size_t remove_request(kaa_log_collector_t *self, uint16_t bucket_id) { - KAA_RETURN_IF_NIL(self, KAA_ERR_BADPARAM); - timeout_info_t target_timeout = { - .log_bucket_id = bucket_id, - }; + kaa_list_node_t *node; + timeout_info_t *info; + size_t logs_sent = 0; + + node = kaa_list_find_next(kaa_list_begin(self->timeouts), find_by_bucket_id, &bucket_id); + + if (node) { + info = kaa_list_get_data(node); + + if (info) { + logs_sent = info->log_count; + } + + kaa_list_remove_at(self->timeouts, node, NULL); + } - kaa_list_remove_first(self->timeouts, &find_by_bucket_id, &target_timeout, NULL); - return target_timeout.log_count; + return logs_sent; } static bool is_timeout(kaa_log_collector_t *self)