discovery: allow to ignore resource in batch responses

Use OC_IGNORE response code to ignore resource in batch responses
or notifications.

api/oc_discovery.c

@@ -223,7 +223,7 @@ encode_resource(CborEncoder *links, const oc_resource_t *resource,
     return false;
   }
 
-  oc_rep_start_object(links, link);
+  oc_rep_begin_object(links, link);
 
   // rel
   if (oc_core_get_resource_by_index(OCF_RES, resource->device) == resource) {
@@ -412,7 +412,7 @@ process_oic_1_1_resource(oc_resource_t *resource, oc_request_t *request,
     return false;
   }
 
-  oc_rep_start_object(links, res);
+  oc_rep_begin_object(links, res);
 
   // uri
   oc_rep_set_text_string_v1(res, href, oc_string(resource->uri),
@@ -502,7 +502,7 @@ process_oic_1_1_device_object(CborEncoder *device, oc_request_t *request,
   char uuid[OC_UUID_LEN];
   oc_uuid_to_str(oc_core_get_device_id(device_num), uuid, OC_ARRAY_SIZE(uuid));
 
-  oc_rep_start_object(device, links);
+  oc_rep_begin_object(device, links);
   oc_rep_set_text_string_v1(links, di, uuid,
                             oc_strnlen(uuid, OC_ARRAY_SIZE(uuid)));
 
@@ -700,6 +700,38 @@ discovery_process_batch_response_write_href(char *buffer, size_t buffer_size,
   buffer[to_write] = '\0';
 }
 
+static bool
+discovery_should_ignore_resource_response(
+  coap_status_t response_code, ptrdiff_t payload_start, size_t payload_size,
+  bool is_collection, bool is_notification, const oc_resource_t *resource)
+{
+  (void)resource;
+  if (response_code == CLEAR_TRANSACTION) {
+    OC_DBG("discovery%s: ignoring resource(%s) with CLEAR_TRANSACTION",
+           is_notification ? " notification" : "", oc_string(resource->uri));
+    return true;
+  }
+#ifdef OC_DISCOVERY_RESOURCE_OBSERVABLE
+  if (is_notification && response_code == CONTENT_2_05) {
+    const uint8_t *payload = oc_rep_get_encoder_buf() + payload_start;
+    if (payload_size == 0 ||
+        (!is_collection && payload_size == 2 &&
+         oc_rep_encoded_payload_is_empty_object(oc_rep_encoder_get_type(),
+                                                payload, payload_size))) {
+      OC_DBG("discovery notification: ignoring empty resource(%s)",
+             oc_string(resource->uri));
+      return true;
+    }
+  }
+#else  /* !OC_DISCOVERY_RESOURCE_OBSERVABLE */
+  (void)payload_start;
+  (void)payload_size;
+  (void)is_collection;
+  (void)is_notification;
+#endif /* OC_DISCOVERY_RESOURCE_OBSERVABLE */
+  return false;
+}
+
 typedef bool (*discovery_process_batch_response_filter_t)(const oc_resource_t *,
                                                           void *);
 
@@ -715,15 +747,13 @@ discovery_process_batch_response(
     return false;
   }
 
-#ifdef OC_DISCOVERY_RESOURCE_OBSERVABLE
   struct
   {
     CborEncoder global;
     CborEncoder links;
   } prev;
   memcpy(&prev.links, encoder, sizeof(CborEncoder));
   memcpy(&prev.global, oc_rep_get_encoder(), sizeof(CborEncoder));
-#endif /* OC_DISCOVERY_RESOURCE_OBSERVABLE */
 
   oc_request_t rest_request;
   memset(&rest_request, 0, sizeof(oc_request_t));
@@ -737,7 +767,7 @@ discovery_process_batch_response(
   rest_request.origin = endpoint;
   rest_request.method = OC_GET;
 
-  oc_rep_start_object(encoder, links_obj);
+  oc_rep_begin_object(encoder, links_obj);
 
   char href[OC_MAX_OCF_URI_SIZE] = { 0 };
   discovery_process_batch_response_write_href(href, OC_MAX_OCF_URI_SIZE,
@@ -779,24 +809,13 @@ discovery_process_batch_response(
     payload_size = total_payload_end - total_payload_start;
   }
 
-#ifdef OC_DISCOVERY_RESOURCE_OBSERVABLE
-  if (is_notification && response_buffer.code == CONTENT_2_05) {
-    const uint8_t *payload =
-      oc_rep_get_encoder_buf() + (ptrdiff_t)total_payload_start;
-    if (payload_size == 0 ||
-        (!is_collection && payload_size == 2 &&
-         oc_rep_encoded_payload_is_empty_object(oc_rep_encoder_get_type(),
-                                                payload, payload_size))) {
-      OC_DBG("ignoring empty resource(%s)", oc_string(resource->uri));
-      memcpy(encoder, &prev.links, sizeof(CborEncoder));
-      memcpy(oc_rep_get_encoder(), &prev.global, sizeof(CborEncoder));
-      return false;
-    }
+  if (discovery_should_ignore_resource_response(
+        response_buffer.code, (ptrdiff_t)total_payload_start, payload_size,
+        is_collection, is_notification, resource)) {
+    memcpy(encoder, &prev.links, sizeof(CborEncoder));
+    memcpy(oc_rep_get_encoder(), &prev.global, sizeof(CborEncoder));
+    return false;
   }
-#else  /* !OC_DISCOVERY_RESOURCE_OBSERVABLE */
-  (void)is_notification;
-  (void)is_collection;
-#endif /* OC_DISCOVERY_RESOURCE_OBSERVABLE */
 
   if (payload_size == 0) {
     oc_rep_start_root_object();

api/oc_server_api.c

@@ -139,6 +139,7 @@ oc_send_response_internal(oc_request_t *request, oc_status_t response_code,
 {
   int status_code = oc_status_code(response_code);
   if (status_code < 0) {
+    OC_ERR("could not send response: invalid response code");
     request->response->response_buffer->code = CLEAR_TRANSACTION;
     return false;
   }
@@ -190,7 +191,7 @@ oc_send_response_with_callback(oc_request_t *request, oc_status_t response_code,
   bool canTruncate = request->method != OC_GET || response_code != OC_STATUS_OK;
   if (!oc_send_response_internal(request, response_code, content_format,
                                  response_length(canTruncate), trigger_cb)) {
-    OC_ERR("could not send response: invalid response code");
+    return;
   }
 }
 

api/unittest/discovery/discovery.cpp

@@ -337,6 +337,14 @@ TestDiscoveryWithServer::onGetEmptyDynamicResource(oc_request_t *request,
   oc_send_response(request, OC_STATUS_OK);
 }
 
+void
+TestDiscoveryWithServer::onGetIgnoredDynamicResource(oc_request_t *request,
+                                                     oc_interface_mask_t,
+                                                     void *)
+{
+  oc_send_response(request, OC_IGNORE);
+}
+
 void
 TestDiscoveryWithServer::addDynamicResources()
 {
@@ -353,6 +361,9 @@ TestDiscoveryWithServer::addDynamicResources()
   oc::DynamicResourceHandler handlers3{};
   handlers3.onGet = onGetEmptyDynamicResource;
 
+  oc::DynamicResourceHandler handlers4{};
+  handlers4.onGet = onGetIgnoredDynamicResource;
+
   std::vector<oc::DynamicResourceToAdd> dynResources = {
     oc::makeDynamicResourceToAdd("Dynamic Resource 1",
                                  std::string(kDynamicURI1),
@@ -366,6 +377,10 @@ TestDiscoveryWithServer::addDynamicResources()
       "Dynamic Resource 3", std::string(kDynamicURI3),
       { "oic.d.observable", "oic.d.test" }, { OC_IF_BASELINE, OC_IF_R },
       handlers3, OC_DISCOVERABLE | OC_OBSERVABLE),
+    oc::makeDynamicResourceToAdd(
+      "Dynamic Resource 4", std::string(kDynamicURIIgnored),
+      { "oic.d.ignored", "oic.d.test" }, { OC_IF_BASELINE, OC_IF_R }, handlers4,
+      OC_DISCOVERABLE | OC_OBSERVABLE),
   };
   for (const auto &dr : dynResources) {
     oc_resource_t *res = oc::TestDevice::AddDynamicResource(dr, kDeviceID);
@@ -401,7 +416,6 @@ TestDiscoveryWithServer::addColletions()
   dynamicResources[std::string(kColDynamicURI1)] = { 404 };
   handlers1.onGet = onGetDynamicResource;
   handlers1.onGetData = &dynamicResources[std::string(kColDynamicURI1)];
-
   auto dr1 = oc::makeDynamicResourceToAdd(
     "Collection Resource 1", std::string(kColDynamicURI1),
     { std::string(powerSwitchRT), "oic.d.test" }, { OC_IF_BASELINE, OC_IF_R },
@@ -416,7 +430,6 @@ TestDiscoveryWithServer::addColletions()
   dynamicResources[std::string(kColDynamicURI2)] = { 1 };
   handlers2.onGet = onGetDynamicResource;
   handlers2.onGetData = &dynamicResources[std::string(kColDynamicURI2)];
-
   auto dr2 = oc::makeDynamicResourceToAdd(
     "Collection Resource 2", std::string(kColDynamicURI2),
     { std::string(powerSwitchRT), "oic.d.test" }, { OC_IF_BASELINE, OC_IF_R },
@@ -429,7 +442,6 @@ TestDiscoveryWithServer::addColletions()
 
   oc::DynamicResourceHandler handlers3{};
   handlers3.onGet = onGetEmptyDynamicResource;
-
   auto dr3 = oc::makeDynamicResourceToAdd(
     "Collection Resource 3", std::string(kColDynamicURI3),
     { std::string(powerSwitchRT), "oic.d.test" }, { OC_IF_BASELINE, OC_IF_R },
@@ -530,6 +542,10 @@ TestDiscoveryWithServer::getBatchResources(const oc_endpoint_t *endpoint)
   oc_resources_iterate(
     kDeviceID, true, true, true, true,
     [](oc_resource_t *resource, void *data) {
+      if (std::string(kDynamicURIIgnored) == oc_string(resource->uri)) {
+        // resource that returns OC_IGNORE shouldn't be in the batch payload
+        return true;
+      }
       if (auto *br = static_cast<batch_resources_t *>(data);
           oc_discovery_resource_is_in_batch_response(resource, br->endpoint,
                                                      true)) {

api/unittest/discovery/discovery.h

@@ -110,12 +110,12 @@ class TestDiscoveryWithServer : public ::testing::Test {
   void SetUp() override;
   void TearDown() override;
 
-  static void onGetDynamicResource(oc_request_t *request,
-                                   oc_interface_mask_t interface,
+  static void onGetDynamicResource(oc_request_t *request, oc_interface_mask_t,
                                    void *user_data);
   static void onGetEmptyDynamicResource(oc_request_t *request,
-                                        oc_interface_mask_t interface,
-                                        void *user_data);
+                                        oc_interface_mask_t, void *);
+  static void onGetIgnoredDynamicResource(oc_request_t *request,
+                                          oc_interface_mask_t, void *);
 
   static void addDynamicResources();
 #ifdef OC_COLLECTIONS
@@ -154,6 +154,7 @@ constexpr size_t kDeviceID{ 0 };
 constexpr std::string_view kDynamicURI1 = "/dyn/discoverable";
 constexpr std::string_view kDynamicURI2 = "/dyn/undiscoverable";
 constexpr std::string_view kDynamicURI3 = "/dyn/empty";
+constexpr std::string_view kDynamicURIIgnored = "/dyn/ignored";
 
 #ifdef OC_COLLECTIONS
 

api/unittest/discovery/discoveryobservetest.cpp

@@ -418,7 +418,7 @@ TEST_F(TestDiscoveryWithServer, ObserveBatchWithResourceAdded)
   verifyBatchPayload(obd.batch, &ep);
 }
 
-TEST_F(TestDiscoveryWithServer, ObserveBatchIgnoreEmptyChange)
+TEST_F(TestDiscoveryWithServer, ObserveBatchSkipEmptyResourceChange)
 {
   ASSERT_TRUE(oc_get_con_res_announced());
 
@@ -451,6 +451,40 @@ TEST_F(TestDiscoveryWithServer, ObserveBatchIgnoreEmptyChange)
   EXPECT_TRUE(obd.batch.empty());
 }
 
+TEST_F(TestDiscoveryWithServer, ObserveBatchSkipIgnoreResourceChange)
+{
+  ASSERT_TRUE(oc_get_con_res_announced());
+
+  auto epOpt = oc::TestDevice::GetEndpoint(kDeviceID);
+  ASSERT_TRUE(epOpt.has_value());
+  auto ep = std::move(*epOpt);
+
+  observeBatchData obd{};
+  ASSERT_TRUE(oc_do_observe(OCF_RES_URI, &ep, "if=" OC_IF_B_STR, onBatchObserve,
+                            HIGH_QOS, &obd));
+  oc::TestDevice::PoolEventsMsV1(1s);
+  EXPECT_EQ(OC_COAP_OPTION_OBSERVE_REGISTER, obd.observe);
+  ASSERT_FALSE(obd.batch.empty());
+  // all resources should be in the first payload
+  verifyBatchPayload(obd.batch, &ep);
+  obd.observe = 0;
+  obd.batch.clear();
+
+  auto *res = oc_ri_get_app_resource_by_uri(
+    kDynamicURIIgnored.data(), kDynamicURIIgnored.size(), kDeviceID);
+  ASSERT_NE(nullptr, res);
+  // notification with payload from resource that returns OC_IGNORE should be
+  // ignored
+  oc_notify_resource_changed(res);
+  int repeats = 0;
+  while (obd.observe == 0 && repeats < 30) {
+    oc::TestDevice::PoolEventsMsV1(10ms);
+    ++repeats;
+  }
+  EXPECT_EQ(0, obd.observe);
+  EXPECT_TRUE(obd.batch.empty());
+}
+
 TEST_F(TestDiscoveryWithServer, ObserveBatchWithEmptyPayload)
 {
   // TODO: remove resource from payload -> make it undiscoverable in runtime

api/unittest/resourcetest.cpp

@@ -52,6 +52,7 @@ static constexpr std::string_view kDevice2Name{ "Test Device 2" };
 
 constexpr std::string_view kDynamicURI1 = "/dyn/discoverable";
 constexpr std::string_view kDynamicURI2 = "/dyn/undiscoverable";
+constexpr std::string_view kDynamicIgnoredURI = "/dyn/ignored";
 constexpr std::string_view kCollectionURI = "/col";
 constexpr std::string_view kColDynamicURI1 = "/col/discoverable";
 #endif /* OC_SERVER && OC_DYNAMIC_ALLOCATION */
@@ -253,6 +254,8 @@ class TestResourceWithDevice : public testing::Test {
   static void addDynamicResources();
   static void onGetDynamicResource(oc_request_t *request, oc_interface_mask_t,
                                    void *user_data);
+  static void onGetIgnoredDynamicResource(oc_request_t *request,
+                                          oc_interface_mask_t, void *);
 #ifdef OC_COLLECTIONS
   static void addCollections();
 #endif /* OC_COLLECTIONS */
@@ -344,6 +347,13 @@ TestResourceWithDevice::onGetDynamicResource(oc_request_t *request,
   oc_send_response(request, OC_STATUS_OK);
 }
 
+void
+TestResourceWithDevice::onGetIgnoredDynamicResource(oc_request_t *request,
+                                                    oc_interface_mask_t, void *)
+{
+  oc_send_response(request, OC_IGNORE);
+}
+
 void
 TestResourceWithDevice::addDynamicResources()
 {
@@ -357,6 +367,9 @@ TestResourceWithDevice::addDynamicResources()
   handlers2.onGet = onGetDynamicResource;
   handlers2.onGetData = &m_dynamic_resources[std::string(kDynamicURI2)];
 
+  oc::DynamicResourceHandler handlers3{};
+  handlers3.onGet = onGetIgnoredDynamicResource;
+
   std::vector<oc::DynamicResourceToAdd> dynResources = {
     oc::makeDynamicResourceToAdd("Dynamic Resource 1",
                                  std::string(kDynamicURI1),
@@ -366,6 +379,10 @@ TestResourceWithDevice::addDynamicResources()
                                  std::string(kDynamicURI2),
                                  { "oic.d.undiscoverable", "oic.d.test" },
                                  { OC_IF_BASELINE, OC_IF_R }, handlers2, 0),
+    oc::makeDynamicResourceToAdd("Dynamic Resource 3",
+                                 std::string(kDynamicIgnoredURI),
+                                 { "oic.d.ignored", "oic.d.test" },
+                                 { OC_IF_BASELINE, OC_IF_R }, handlers3),
   };
   for (const auto &dr : dynResources) {
     oc_resource_t *res = oc::TestDevice::AddDynamicResource(dr, kDevice1ID);
@@ -568,6 +585,29 @@ TEST_F(TestResourceWithDevice, IterateCollectionResources)
   ASSERT_TRUE(collection4.empty());
 }
 
+// Resource that returns OC_IGNORE shouldn't return any payload
+TEST_F(TestResourceWithDevice, GetRequestIgnoredResource)
+{
+  auto epOpt = oc::TestDevice::GetEndpoint(kDevice1ID);
+  ASSERT_TRUE(epOpt.has_value());
+  auto ep = std::move(*epOpt);
+
+  bool invoked = false;
+  auto get_handler = [](oc_client_response_t *data) {
+    oc::TestDevice::Terminate();
+    *static_cast<bool *>(data->user_data) = true;
+    ASSERT_EQ(OC_REQUEST_TIMEOUT, data->code);
+    OC_DBG("GET payload: %s", oc::RepPool::GetJson(data->payload, true).data());
+  };
+
+  auto timeout = 1s;
+  EXPECT_TRUE(oc_do_get_with_timeout(kDynamicIgnoredURI.data(), &ep, nullptr,
+                                     timeout.count(), get_handler, LOW_QOS,
+                                     &invoked));
+  oc::TestDevice::PoolEventsMsV1(timeout, true);
+  ASSERT_TRUE(invoked);
+}
+
 #endif /* OC_SERVER && OC_DYNAMIC_ALLOCATION */
 
 #if !defined(OC_SECURITY) || defined(OC_HAS_FEATURE_RESOURCE_ACCESS_IN_RFOTM)
@@ -657,7 +697,7 @@ TEST_F(TestResourceWithDevice, BaselineInterfaceProperties)
   auto timeout = 1s;
   bool invoked = false;
   EXPECT_TRUE(oc_do_get_with_timeout("/oc/con", &ep, "if=" OC_IF_BASELINE_STR,
-                                     timeout.count(), get_handler, HIGH_QOS,
+                                     timeout.count(), get_handler, LOW_QOS,
                                      &invoked));
   oc::TestDevice::PoolEventsMsV1(timeout, true);