fixup! util: extend oc_endpoint_addresses_t by custom on change callback

.github/workflows/ctt-test.yml

@@ -28,7 +28,7 @@ jobs:
 
       - name: Archive results
         if: success() || failure()
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: logs
           path: results/*.octt

.github/workflows/plgd-device-test-with-cfg.yml

@@ -129,7 +129,7 @@ jobs:
 
       - name: Upload coverage data
         if: ${{ inputs.coverage }}
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: plgd-device-coverage
           path: data/coverage/${{ steps.coverage.outputs.filename }}

.github/workflows/plgd-hub-test-with-cfg.yml

@@ -122,7 +122,7 @@ jobs:
 
       - name: Upload coverage data
         if: ${{ inputs.coverage }}
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: plgd-hub-coverage
           path: data/coverage/${{ steps.coverage.outputs.filename }}

.github/workflows/sonar-cloud-analysis.yml

@@ -113,7 +113,7 @@ jobs:
           build-wrapper-linux-x86-64 --out-dir ${{ env.BUILD_WRAPPER_OUT_DIR }} cmake --build build --verbose --target client-server-static --target all
 
       - name: Get coverage from all tests job
-        uses: actions/download-artifact@v3
+        uses: actions/download-artifact@v4
         with:
           path: tools/
 

.github/workflows/unit-test-with-cfg.yml

@@ -135,7 +135,7 @@ jobs:
 
       - name: Upload coverage data
         if: ${{ inputs.coverage }}
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: unit-test-coverage
           path: tools/${{ steps.coverage.outputs.filename }}

api/cloud/oc_cloud_context.c

@@ -60,8 +60,8 @@ need_to_reinitialize_cloud_storage(const oc_cloud_context_t *ctx)
   return cloud_is_deregistering(ctx);
 }
 
-static void
-cloud_on_server_change(void *data)
+void
+cloud_context_on_server_change(void *data)
 {
   oc_cloud_context_t *ctx = (oc_cloud_context_t *)data;
   if (ctx->cloud_manager) {
@@ -97,7 +97,7 @@ cloud_context_init(size_t device)
   ctx->cloud_ep_state = OC_SESSION_DISCONNECTED;
   ctx->cloud_ep = oc_new_endpoint();
   ctx->selected_identity_cred_id = -1;
-  oc_cloud_store_initialize(&ctx->store, cloud_on_server_change, ctx);
+  oc_cloud_store_initialize(&ctx->store, cloud_context_on_server_change, ctx);
   oc_cloud_store_load(&ctx->store);
   ctx->store.status &=
     ~(OC_CLOUD_LOGGED_IN | OC_CLOUD_TOKEN_EXPIRY | OC_CLOUD_REFRESHED_TOKEN |

api/cloud/oc_cloud_context_internal.h

@@ -171,6 +171,10 @@ void cloud_context_clear_access_token(oc_cloud_context_t *ctx) OC_NONNULL();
 bool cloud_context_has_refresh_token(const oc_cloud_context_t *ctx)
   OC_NONNULL();
 
+/** @brief Callback invoekd by ctx::store::ci_servers when the selected cloud
+ * server is changed  */
+void cloud_context_on_server_change(void *data) OC_NONNULL();
+
 /** @brief Initialize the registration context */
 void oc_cloud_registration_context_init(oc_cloud_registration_context_t *regctx,
                                         const oc_endpoint_addresses_t *servers)

api/cloud/oc_cloud_manager.c

@@ -256,28 +256,6 @@ cloud_start_process(oc_cloud_context_t *ctx)
   _oc_signal_event_loop();
 }
 
-static uint64_t
-refresh_token_expires_in_ms(int64_t expires_in_ms)
-{
-  if (expires_in_ms <= 0) {
-    return 0;
-  }
-
-  if (expires_in_ms > (int64_t)MILLISECONDS_PER_HOUR) {
-    // if time is more than 1h then set expires to (expires_in_ms - 10min).
-    return (uint64_t)(expires_in_ms - (int64_t)(10 * MILLISECONDS_PER_MINUTE));
-  }
-  if (expires_in_ms > (int64_t)(4 * MILLISECONDS_PER_MINUTE)) {
-    // if time is more than 240sec then set expires to (expires_in_ms - 2min).
-    return (uint64_t)(expires_in_ms - (int64_t)(2 * MILLISECONDS_PER_MINUTE));
-  }
-  if (expires_in_ms > (int64_t)(20 * MILLISECONDS_PER_SECOND)) {
-    // if time is more than 20sec then set expires to (expires_in_ms - 10sec).
-    return (uint64_t)(expires_in_ms - (int64_t)(10 * MILLISECONDS_PER_SECOND));
-  }
-  return (uint64_t)expires_in_ms;
-}
-
 static bool
 cloud_is_connection_error_or_timeout(oc_status_t code)
 {
@@ -388,7 +366,7 @@ cloud_manager_handle_redirect_response(oc_cloud_context_t *ctx,
       oc_endpoint_addresses_add(&ctx->store.ci_servers,
                                 oc_endpoint_address_make_view_with_uuid(
                                   oc_string_view2(redirecturi), sid)) == NULL) {
-    OC_ERR("failed to add server to the list");
+    OC_CLOUD_ERR("failed to add server to the list");
     return false;
   }
 
@@ -697,6 +675,26 @@ on_keepalive_response(oc_cloud_context_t *ctx, bool response_received,
   return true;
 }
 
+uint64_t
+cloud_manager_calculate_refresh_token_expiration(uint64_t expires_in_ms)
+{
+  assert(expires_in_ms > 0);
+
+  if (expires_in_ms > (int64_t)MILLISECONDS_PER_HOUR) {
+    // if time is more than 1h then set expires to (expires_in_ms - 10min).
+    return (uint64_t)(expires_in_ms - (int64_t)(10 * MILLISECONDS_PER_MINUTE));
+  }
+  if (expires_in_ms > (int64_t)(4 * MILLISECONDS_PER_MINUTE)) {
+    // if time is more than 240sec then set expires to (expires_in_ms - 2min).
+    return (uint64_t)(expires_in_ms - (int64_t)(2 * MILLISECONDS_PER_MINUTE));
+  }
+  if (expires_in_ms > (int64_t)(20 * MILLISECONDS_PER_SECOND)) {
+    // if time is more than 20sec then set expires to (expires_in_ms - 10sec).
+    return (uint64_t)(expires_in_ms - (int64_t)(10 * MILLISECONDS_PER_SECOND));
+  }
+  return (uint64_t)0;
+}
+
 static void
 cloud_manager_login_handler(oc_client_response_t *data)
 {
@@ -720,8 +718,8 @@ cloud_manager_login_handler(oc_client_response_t *data)
     if (ctx->store.expires_in > 0) {
       oc_reset_delayed_callback_ms(
         ctx, cloud_manager_refresh_token_async,
-        refresh_token_expires_in_ms(ctx->store.expires_in *
-                                    MILLISECONDS_PER_SECOND));
+        cloud_manager_calculate_refresh_token_expiration(
+          ctx->store.expires_in * MILLISECONDS_PER_SECOND));
     }
     oc_reset_delayed_callback(ctx, cloud_manager_callback_handler_async, 0);
     return;

api/cloud/oc_cloud_manager_internal.h

@@ -64,6 +64,20 @@ bool cloud_manager_handle_redirect_response(oc_cloud_context_t *ctx,
                                             const oc_rep_t *payload)
   OC_NONNULL();
 
+/**
+ * @brief Calculate the expiration time of the refresh token.
+ *
+ * The expiration time is used to determine when the refresh token should be
+ * refreshed. It should be less than the actual expiration time of the token, so
+ * it can be refreshed before it really expires.
+ *
+ *
+ * @param expires_in_ms expires_in value in milliseconds (must be >0)
+ * @return uint64_t expiration time in milliseconds
+ */
+uint64_t cloud_manager_calculate_refresh_token_expiration(
+  uint64_t expires_in_ms);
+
 /**
  * @brief Parse refresh token response retrieved from the server and store the
  * data to cloud context.

api/cloud/oc_cloud_rd.c

@@ -369,7 +369,7 @@ oc_cloud_publish_resources(size_t device)
 {
   oc_cloud_context_t *ctx = oc_cloud_get_context(device);
   if (ctx == NULL) {
-    OC_ERR("cannot publish resource: invalid device(%zu)", device);
+    OC_CLOUD_ERR("cannot publish resource: invalid device(%zu)", device);
     return -1;
   }
   publish_published_resources(ctx);

api/cloud/oc_cloud_resource.c

@@ -157,7 +157,7 @@ cloud_update_from_request(oc_cloud_context_t *ctx, const oc_request_t *request)
   if (sid != NULL) {
     oc_string_view_t sidv = oc_string_view2(sid);
     if (oc_str_to_uuid_v1(sidv.data, sidv.length, &data.sid) < 0) {
-      OC_ERR("failed parsing sid(%s)", sidv.data);
+      OC_CLOUD_ERR("failed parsing sid(%s)", sidv.data);
       return false;
     }
   }

api/cloud/oc_cloud_store.c

@@ -121,7 +121,7 @@ oc_cloud_store_dump(const oc_cloud_store_t *store)
   long ret = oc_storage_data_save(OC_CLOUD_STORE_NAME, store->device,
                                   store_encode_cloud, store);
   if (ret <= 0) {
-    OC_ERR("cannot dump cloud to storage: error(%ld)", ret);
+    OC_CLOUD_ERR("cannot dump cloud to storage: error(%ld)", ret);
     return false;
   }
   return true;
@@ -304,7 +304,7 @@ oc_cloud_store_decode(const oc_rep_t *rep, oc_cloud_store_t *store)
   // copy data to store
   if ((csd.ci_server != NULL || csd.ci_servers != NULL) &&
       !cloud_store_set_servers(store, csd.ci_server, csd.sid, csd.ci_servers)) {
-    OC_WRN("failed to set cloud servers from storage");
+    OC_CLOUD_WRN("failed to set cloud servers from storage");
   }
 
   if (csd.auth_provider != NULL) {
@@ -332,7 +332,7 @@ store_decode_cloud(const oc_rep_t *rep, size_t device, void *data)
   (void)device;
   oc_cloud_store_t *store = (oc_cloud_store_t *)data;
   if (!oc_cloud_store_decode(rep, store)) {
-    OC_ERR("cannot load cloud: cannot decode representation");
+    OC_CLOUD_ERR("cannot load cloud: cannot decode representation");
     return -1;
   }
   return 0;
@@ -343,11 +343,11 @@ oc_cloud_store_load(oc_cloud_store_t *store)
 {
   if (oc_storage_data_load(OC_CLOUD_STORE_NAME, store->device,
                            store_decode_cloud, store) <= 0) {
-    OC_DBG("failed to load cloud from storage");
+    OC_CLOUD_DBG("failed to load cloud from storage");
     oc_cloud_store_reinitialize(store);
     return false;
   }
-  OC_DBG("cloud loaded from storage");
+  OC_CLOUD_DBG("cloud loaded from storage");
   return true;
 }
 

api/cloud/rd_client.c

@@ -158,7 +158,7 @@ rd_prepare_write_buffer(oc_write_buffer_t *wb, char *buffer, size_t buffer_size,
   // enough room to write the "di={id}" part
   if (buffer_size <=
       /*di=*/3 + id.length) {
-    OC_ERR("buffer too small");
+    OC_CLOUD_ERR("buffer too small");
     return false;
   }
   wb->buffer = buffer;
@@ -212,10 +212,10 @@ rd_delete_send_packet(const oc_endpoint_t *endpoint, oc_string_view_t query,
                       void *data)
 {
   rd_delete_packet_t *pkt = (rd_delete_packet_t *)data;
-  OC_DBG("Unpublishing links (query=%s)", query.data);
+  OC_CLOUD_DBG("Unpublishing links (query=%s)", query.data);
   if (!oc_do_delete(OC_RSRVD_RD_URI, endpoint, query.data, pkt->handler,
                     pkt->qos, pkt->user_data)) {
-    OC_ERR("failed to unpublish links (query=%s)", query.data);
+    OC_CLOUD_ERR("failed to unpublish links (query=%s)", query.data);
     return false;
   }
   return true;

api/cloud/unittest/cloud_manager_test.cpp

@@ -45,6 +45,7 @@ using namespace std::chrono_literals;
 // cannot use lower value than 1s because oc_cloud_schedule_action_t::timeout is
 // in seconds
 static constexpr auto kTimeout = 1s;
+static constexpr auto kTestServer = OC_STRING_VIEW("coap://224.0.1.187:5683");
 
 class TestCloudManager : public testing::Test {
 public:
@@ -66,26 +67,32 @@ class TestCloudManager : public testing::Test {
     memset(&m_context, 0, sizeof(m_context));
     m_context.cloud_ep = oc_new_endpoint();
     memset(m_context.cloud_ep, 0, sizeof(oc_endpoint_t));
-    oc_cloud_store_initialize(&m_context.store, nullptr, nullptr);
-    oc_string_view_t ep = OC_STRING_VIEW("coap://224.0.1.187:5683");
+    oc_cloud_store_initialize(&m_context.store, cloud_context_on_server_change,
+                              &m_context);
     oc_uuid_t sid;
     oc_gen_uuid(&sid);
-    ASSERT_NE(nullptr, oc_endpoint_addresses_add(
-                         &m_context.store.ci_servers,
-                         oc_endpoint_address_make_view_with_uuid(ep, sid)));
-    ASSERT_TRUE(
-      oc_endpoint_addresses_select_by_uri(&m_context.store.ci_servers, ep));
+    ASSERT_NE(nullptr,
+              oc_endpoint_addresses_add(
+                &m_context.store.ci_servers,
+                oc_endpoint_address_make_view_with_uuid(kTestServer, sid)));
+    ASSERT_TRUE(oc_endpoint_addresses_select_by_uri(&m_context.store.ci_servers,
+                                                    kTestServer));
     std::string uid = "501";
     oc_set_string(&m_context.store.uid, uid.c_str(), uid.length());
     std::string token = "access_token";
     oc_set_string(&m_context.store.access_token, token.c_str(), token.length());
     std::string rtoken = "refresh_token";
     oc_set_string(&m_context.store.refresh_token, rtoken.c_str(),
                   rtoken.length());
+
+    // TODO: rm
+    oc_log_set_level(OC_LOG_LEVEL_DEBUG);
   }
 
   void TearDown() override
   {
+    oc_log_set_level(OC_LOG_LEVEL_INFO);
+
     oc_cloud_manager_set_retry_timeouts(nullptr, 0);
 
     oc_free_endpoint(m_context.cloud_ep);
@@ -349,20 +356,24 @@ TEST_F(TestCloudManager, cloud_manager_select_next_server_on_retry)
   ASSERT_NE(nullptr, ep);
   ASSERT_FALSE(
     oc_endpoint_addresses_is_selected(&m_context.store.ci_servers, uri));
+  // default cloud server (127.0.0.1), kTestServer and uri
+  ASSERT_EQ(3, oc_endpoint_addresses_size(&m_context.store.ci_servers));
 
-  // When
+  ASSERT_TRUE(oc_endpoint_addresses_is_selected(&m_context.store.ci_servers,
+                                                kTestServer));
   m_context.store.status = OC_CLOUD_INITIALIZED;
   m_context.store.cps = OC_CPS_READYTOREGISTER;
-  cloud_manager_start(&m_context);
+  oc_cloud_manager_start(&m_context, nullptr, nullptr);
   // by default: first retry should happen timeout + jitter
   // ([timeout/2..timeout])  (see default_schedule_action)
-  oc::TestDevice::PoolEventsMsV1(
-    /*timeout*/ kTimeout + /*max possible jitter*/ kTimeout, true);
-  cloud_manager_stop(&m_context);
-
-  // Then
-  EXPECT_TRUE(
-    oc_endpoint_addresses_is_selected(&m_context.store.ci_servers, uri));
+  auto interval = (/* 3 servers to try */ 3) *
+                  (/*timeout*/ kTimeout + /*max possible jitter*/ kTimeout);
+  oc::TestDevice::PoolEventsMsV1(interval, true);
+
+  // the retries should loop all servers and loop back to the original
+  EXPECT_TRUE(oc_endpoint_addresses_is_selected(&m_context.store.ci_servers,
+                                                kTestServer));
+  oc_cloud_manager_stop(&m_context);
 }
 
 #endif /* !OC_SECURITY */
@@ -448,6 +459,36 @@ TestCloudManagerData::GetPayload(std::optional<std::string> access_token,
   return rep;
 }
 
+TEST_F(TestCloudManagerData, cloud_manager_calculate_refresh_token_expiration)
+{
+  // long internal (>1hour) -> refresh schedule 10mins before expiration
+  std::chrono::milliseconds expires_in = 2h;
+  auto expires_in_sec =
+    cloud_manager_calculate_refresh_token_expiration(expires_in.count());
+  EXPECT_LT(0, expires_in_sec);
+  EXPECT_GT(expires_in.count(), expires_in_sec);
+
+  // middle internal (>4mins) -> refresh schedule 2mins before expiration
+  expires_in = 5min;
+  expires_in_sec =
+    cloud_manager_calculate_refresh_token_expiration(expires_in.count());
+  EXPECT_LT(0, expires_in_sec);
+  EXPECT_GT(expires_in.count(), expires_in_sec);
+
+  // short internal (>20s) -> refresh schedule 10secs before expiration
+  expires_in = 1min;
+  expires_in_sec =
+    cloud_manager_calculate_refresh_token_expiration(expires_in.count());
+  EXPECT_LT(0, expires_in_sec);
+  EXPECT_GT(expires_in.count(), expires_in_sec);
+
+  // immediate expiration (<=20s) -> refresh immediately
+  expires_in = 10s;
+  expires_in_sec =
+    cloud_manager_calculate_refresh_token_expiration(expires_in.count());
+  EXPECT_EQ(0, expires_in_sec);
+}
+
 TEST_F(TestCloudManagerData, cloud_manager_parse_register_data_invalid)
 {
   // {