Subs with timeInterval: Giving 501 for for non-experimental. And for …

…experimental when geoQ is present
FIWARE · Sep 8, 2023 · 2a681cc · 2a681cc
1 parent 473fd01
commit 2a681cc
Show file tree

Hide file tree

Showing 20 changed files with 66 additions and 126 deletions.
diff --git a/src/lib/orionld/dbModel/dbModelToApiSubscription.cpp b/src/lib/orionld/dbModel/dbModelToApiSubscription.cpp
@@ -306,10 +306,6 @@ KjNode* dbModelToApiSubscription
     // There is no "isPattern" in NGSI-LD
     kjChildRemove(entityP, isPatternP);
 
-    LM_T(LmtSR, ("entityId:   %s", idP->value.s));
-    LM_T(LmtSR, ("isPattern:  %s", isPatternP->value.s));
-    LM_T(LmtSR, ("type:       %s", typeP->value.s));
-
     if (strcmp(isPatternP->value.s, "true") == 0)
     {
       if (strcmp(idP->value.s, ".*") == 0)
@@ -539,7 +535,6 @@ KjNode* dbModelToApiSubscription
   // Add "notification" to top level
   kjChildAdd(apiSubP, notificationP);
 
-
   if (dbReferenceP != NULL)
   {
     kjChildAdd(endpointP, dbReferenceP);

diff --git a/src/lib/orionld/kjTree/kjTreeLog.cpp b/src/lib/orionld/kjTree/kjTreeLog.cpp
@@ -50,7 +50,9 @@ void kjTreeLogFunction(KjNode* tree, const char* msg, const char* path, int line
 
   if (tree == NULL)
   {
-    lmOut((char*) "NULL Tree", 'T', fileNameOnly, lineNo, functionName, traceLevel, NULL);
+    char error[256];
+    snprintf(error, sizeof(error) - 1, "%s: NULL", msg);
+    lmOut(error, 'T', fileNameOnly, lineNo, functionName, traceLevel, NULL);
     return;
   }
 

diff --git a/src/lib/orionld/kjTree/kjTreeToSubscription.cpp b/src/lib/orionld/kjTree/kjTreeToSubscription.cpp
@@ -260,15 +260,8 @@ bool kjTreeToSubscription(ngsiv2::Subscription* subP, char** subIdPP, KjNode** e
     }
     else if (strcmp(kNodeP->name, "timeInterval") == 0)
     {
-      DUPLICATE_CHECK(timeIntervalP, "Subscription::timeInterval", kNodeP);
-      NUMBER_CHECK(timeIntervalP, "Subscription::timeInterval");
-      subP->timeInterval = (timeIntervalP->type == KjInt)? timeIntervalP->value.i : timeIntervalP->value.f;
-
-      if (subP->timeInterval <= 0)
-      {
-        orionldError(OrionldBadRequestData, "Invalid value for Subscription::timeInterval", "must be a Number > 0", 400);
-        return false;
-      }
+      orionldError(OrionldOperationNotSupported, "Not Implemented", "Periodic Notification Subscriptions are not implemented", 501);
+      return false;
     }
     else if ((kNodeP->name[0] == 'q') && (kNodeP->name[1] == 0))
     {

diff --git a/src/lib/orionld/mongoc/mongocEntitiesQuery.cpp b/src/lib/orionld/mongoc/mongocEntitiesQuery.cpp
@@ -255,7 +255,9 @@ static bool geoNearFilter(bson_t* mongoFilterP, OrionldGeoInfo*  geoInfoP)
   char geoPropertyPath[512];
   int  geoPropertyPathLen;
   if (geoPropertyDbPath(geoPropertyPath, sizeof(geoPropertyPath), geoInfoP->geoProperty, &geoPropertyPathLen) == false)
-    return false;
+    LM_RE(false, ("Failed to aeemble the geoProperty path"));
+
+  LM_T(LmtMongoc, ("geoPropertyPath: '%s'", geoPropertyPath));
 
   bson_append_document(mongoFilterP, geoPropertyPath, geoPropertyPathLen, &location);
 
@@ -310,6 +312,7 @@ static bool geoWithinFilter(bson_t* mongoFilterP, OrionldGeoInfo* geoInfoP)
   bson_init(&within);
   bson_init(&coordinates);
 
+  kjTreeLog(geoInfoP->coordinates, "coordinates", LmtMongoc);
   mongocKjTreeToBson(geoInfoP->coordinates, &coordinates);
 
   bson_append_array(&geometry,    "coordinates", 11, &coordinates);

diff --git a/src/lib/orionld/mongoc/mongocEntitiesQuery2.cpp b/src/lib/orionld/mongoc/mongocEntitiesQuery2.cpp
@@ -320,7 +320,7 @@ KjNode* mongocEntitiesQuery2
   if (geoInfoP != NULL)
   {
     if (geoFilter(&mongoFilter, geoInfoP) == false)
-      return NULL;
+      LM_RE(NULL, ("geoFilter flagged an error"));
     geojsonGeometry = geoInfoP->geoProperty;
   }
 

diff --git a/src/lib/orionld/notifications/httpNotify.cpp b/src/lib/orionld/notifications/httpNotify.cpp
@@ -44,7 +44,7 @@ int httpNotify
   const char*          subscriptionId,
   const char*          ip,
   unsigned short       port,
-  const	char* 	       path,
+  const char*          path,
   struct iovec*        ioVec,
   int                  ioVecLen,
   double               notificationTime

diff --git a/src/lib/orionld/notifications/httpNotify.h b/src/lib/orionld/notifications/httpNotify.h
@@ -43,7 +43,7 @@ extern int httpNotify
   const char*          subscriptionId,
   const char*          ip,
   unsigned short       port,
-  const	char* 	       path,
+  const char*          path,
   struct iovec*        ioVec,
   int                  ioVecLen,
   double               notificationTime

diff --git a/src/lib/orionld/payloadCheck/pCheckQuery.cpp b/src/lib/orionld/payloadCheck/pCheckQuery.cpp
@@ -269,7 +269,7 @@ TreeNode* pCheckQuery(KjNode* queryP)
   if ((treeNodeV[1].nodeP != NULL) && (pCheckEntities(treeNodeV[1].nodeP)          == false))                         return NULL;
   if ((treeNodeV[2].nodeP != NULL) && (pCheckAttrs(treeNodeV[2].nodeP)             == false))                         return NULL;
   if ((treeNodeV[3].nodeP != NULL) && ((treeNodeV[3].output = pcheckQ(treeNodeV[3].nodeP->value.s))  == NULL)) return NULL;
-  if ((treeNodeV[4].nodeP != NULL) && ((treeNodeV[4].output = pcheckGeoQ(treeNodeV[4].nodeP, false)) == NULL)) return NULL;
+  if ((treeNodeV[4].nodeP != NULL) && ((treeNodeV[4].output = pcheckGeoQ(&orionldState.kalloc, treeNodeV[4].nodeP, false)) == NULL)) return NULL;
 
   //
   // Now, for the query to not be too wide, we need at least one of:

diff --git a/src/lib/orionld/payloadCheck/pCheckSubscription.cpp b/src/lib/orionld/payloadCheck/pCheckSubscription.cpp
@@ -237,11 +237,17 @@ bool pCheckSubscription
     }
     else if (strcmp(subItemP->name, "geoQ") == 0)
     {
+      if (*timeInterval > 0)
+      {
+        orionldError(OrionldOperationNotSupported, "Not Implemented", "Geo-Query is not yet implemented for Periodic Notification Subscriptions", 501);
+        return false;
+      }
+
       PCHECK_OBJECT(subItemP, 0, NULL, SubscriptionGeoqPath, 400);
       PCHECK_DUPLICATE(geoqP, subItemP, 0, NULL, SubscriptionGeoqPath, 400);
 
       OrionldGeoInfo* geoInfoP;
-      if ((geoInfoP = pcheckGeoQ(geoqP, true)) == NULL)
+      if ((geoInfoP = pcheckGeoQ(&orionldState.kalloc, geoqP, true)) == NULL)
         return false;
 
       *geoCoordinatesPP = geoInfoP->coordinates;

diff --git a/src/lib/orionld/payloadCheck/pcheckGeoQ.cpp b/src/lib/orionld/payloadCheck/pcheckGeoQ.cpp
@@ -25,6 +25,7 @@
 #include <string.h>                                             // strcmp
 extern "C"
 {
+#include "kalloc/KAlloc.h"                                      // KAlloc
 #include "kjson/KjNode.h"                                       // KjNode
 #include "kjson/kjParse.h"                                      // kjParse
 }
@@ -52,13 +53,13 @@ extern "C"
 //
 // pcheckGeoQ -
 //
-OrionldGeoInfo* pcheckGeoQ(KjNode* geoqNodeP, bool isSubscription)
+OrionldGeoInfo* pcheckGeoQ(KAlloc* kallocP, KjNode* geoqNodeP, bool isSubscription)
 {
   KjNode*          geometryP    = NULL;
   KjNode*          coordinatesP = NULL;
   KjNode*          georelP      = NULL;
   KjNode*          geopropertyP = NULL;
-  OrionldGeoInfo*  geoInfoP     = (OrionldGeoInfo*) kaAlloc(&orionldState.kalloc, sizeof(OrionldGeoInfo));
+  OrionldGeoInfo*  geoInfoP     = (OrionldGeoInfo*) ((kallocP != NULL)? kaAlloc(kallocP, sizeof(OrionldGeoInfo)) : malloc(sizeof(OrionldGeoInfo)));
 
   if (geoInfoP == NULL)
   {

diff --git a/src/lib/orionld/payloadCheck/pcheckGeoQ.h b/src/lib/orionld/payloadCheck/pcheckGeoQ.h
@@ -27,6 +27,7 @@
 */
 extern "C"
 {
+#include "kalloc/KAlloc.h"                                       // KAlloc
 #include "kjson/KjNode.h"                                        // KjNode
 }
 
@@ -38,6 +39,6 @@ extern "C"
 //
 // pcheckGeoQ -
 //
-extern OrionldGeoInfo* pcheckGeoQ(KjNode* geoqNodeP, bool pCheckGeoGeometry);
+extern OrionldGeoInfo* pcheckGeoQ(KAlloc* kallocP, KjNode* geoqNodeP, bool pCheckGeoGeometry);
 
 #endif  // SRC_LIB_ORIONLD_PAYLOADCHECK_PCHECKGEOQ_H_
diff --git a/src/lib/orionld/payloadCheck/pcheckQuery.cpp b/src/lib/orionld/payloadCheck/pcheckQuery.cpp
@@ -200,7 +200,7 @@ bool pcheckQuery(KjNode* tree, KjNode** entitiesPP, KjNode** attrsPP, QNode** qT
     return false;
   if ((qP != NULL) && ((*qTreePP = pcheckQ(qP->value.s)) == NULL))
     return false;
-  if ((geoqP != NULL) && (pcheckGeoQ(geoqP, false) == NULL))
+  if ((geoqP != NULL) && (pcheckGeoQ(&orionldState.kalloc, geoqP, false) == NULL))
     return false;
 
   return true;

diff --git a/src/lib/orionld/payloadCheck/pcheckSubscription.cpp b/src/lib/orionld/payloadCheck/pcheckSubscription.cpp
@@ -161,7 +161,7 @@ bool pcheckSubscription
       DUPLICATE_CHECK(geoqP, "geoQ", nodeP);
       OBJECT_CHECK(nodeP, "geoQ");
       EMPTY_OBJECT_CHECK(nodeP, "geoQ");
-      OrionldGeoInfo* geoInfoP = pcheckGeoQ(nodeP, true);
+      OrionldGeoInfo* geoInfoP = pcheckGeoQ(&orionldState.kalloc, nodeP, true);
       if (geoInfoP == NULL)
         return false;
       *geoCoordinatesPP = geoInfoP->coordinates;

diff --git a/src/lib/orionld/pernot/PernotSubscription.h b/src/lib/orionld/pernot/PernotSubscription.h
@@ -36,9 +36,10 @@ extern "C"
 #include "common/RenderFormat.h"                               // RenderFormat
 #include "common/MimeType.h"                                   // MimeType
 
+#include "orionld/types/OrionldTenant.h"                       // OrionldTenant
 #include "orionld/types/Protocol.h"                            // Protocol
 #include "orionld/types/StringArray.h"                         // StringArray
-#include "orionld/types/OrionldTenant.h"                       // OrionldTenant
+#include "orionld/types/OrionldGeoInfo.h"                      // OrionldGeoInfo
 #include "orionld/q/QNode.h"                                   // QNode
 
 
@@ -84,6 +85,7 @@ typedef struct PernotSubscription
   KjNode*                     eSelector;
   KjNode*                     attrsSelector;
   QNode*                      qSelector;
+  OrionldGeoInfo*             geoSelector;
 
   // Counters
   uint32_t                    notificationAttemptsDb;   // Total number of notification attempts, in DB

diff --git a/src/lib/orionld/pernot/pernotSubCacheAdd.cpp b/src/lib/orionld/pernot/pernotSubCacheAdd.cpp
@@ -42,6 +42,7 @@ extern "C"
 #include "orionld/types/Protocol.h"                            // Protocol, protocolFromString
 #include "orionld/types/OrionldTenant.h"                       // OrionldTenant
 #include "orionld/q/QNode.h"                                   // QNode
+#include "orionld/payloadCheck/pcheckGeoQ.h"                   // pcheckGeoQ
 #include "orionld/kjTree/kjChildCount.h"                       // kjChildCount
 #include "orionld/context/OrionldContext.h"                    // OrionldContext
 #include "orionld/pernot/PernotSubscription.h"                 // PernotSubscription
@@ -74,7 +75,7 @@ static void receiverInfo(PernotSubscription* pSubP, KjNode* endpointP)
       KjNode* valueP = kjLookup(kvPairP, "value");
       int     sLen   = strlen(keyP->value.s) + strlen(valueP->value.s) + 4;
       char*   s      = (char*) malloc(sLen);
-        
+
       snprintf(s, sLen - 1, "%s:%s\r\n", keyP->value.s, valueP->value.s);
       pSubP->headers.array[ix] = s;
       ++ix;
@@ -173,8 +174,28 @@ PernotSubscription* pernotSubCacheAdd
   // Query parameters
   pSubP->eSelector      = kjLookup(pSubP->kjSubP, "entities");
   pSubP->attrsSelector  = kjLookup(notificationP, "attributes");
-  pSubP->qSelector      = qTree;  // I probably need to clone this ...
+  pSubP->qSelector      = qTree;
+  pSubP->geoSelector    = NULL;
 
+  KjNode* geoqP = kjLookup(apiSubP, "geoQ");
+  if (geoqP != NULL)
+  {
+    pSubP->geoSelector = pcheckGeoQ(NULL, geoqP, true);  // FIXME: Already done in pcheckSubscription()
+    if (pSubP->geoSelector != NULL)
+    {
+      if (pSubP->geoSelector->geoProperty == NULL)
+        pSubP->geoSelector->geoProperty = (char*) "location";
+
+      LM_T(LmtPernotQuery, ("geometry:    %d", pSubP->geoSelector->geometry));
+      LM_T(LmtPernotQuery, ("georel:      %d", pSubP->geoSelector->georel));
+      LM_T(LmtPernotQuery, ("minDistance: %d", pSubP->geoSelector->minDistance));
+      LM_T(LmtPernotQuery, ("maxDistance: %d", pSubP->geoSelector->maxDistance));
+      LM_T(LmtPernotQuery, ("geoProperty: '%s'", pSubP->geoSelector->geoProperty));
+      pSubP->geoSelector->coordinates = kjClone(NULL, pSubP->geoSelector->coordinates);
+      kjTreeLog(pSubP->geoSelector->coordinates, "coordinates", LmtPernotQuery);
+
+    }
+  }
 
   // notification::endpoint
   if (endpointP == NULL)

diff --git a/src/lib/orionld/pernot/pernotTreat.cpp b/src/lib/orionld/pernot/pernotTreat.cpp
@@ -119,8 +119,8 @@ static void* pernotTreat(void* vP)
   KjNode*          apiEntityArray;
 
   orionldState.uriParams.offset = 0;
-  orionldState.uriParams.limit  = 20;  // Or: set in subscription
-  orionldState.uriParams.count  = true;
+  orionldState.uriParams.limit  = 20;     // Or: set in subscription
+  orionldState.uriParams.count  = true;   // Need the count to be able to paginate
   orionldState.tenantP          = subP->tenantP;
 
   kjTreeLog(subP->eSelector,     "eSelector",     LmtPernotQuery);

diff --git a/src/lib/orionld/serviceRoutines/orionldGetSubscription.cpp b/src/lib/orionld/serviceRoutines/orionldGetSubscription.cpp
@@ -184,7 +184,7 @@ static bool orionldGetSubscriptionFromDb(void)
     return false;
   }
   kjTreeLog(dbSubP, "DB Sub", LmtSubCacheStats);
-  
+
   KjNode*      coordinatesNodeP = NULL;           // Not needed here, but dbModelToApiSubscription requires it
   KjNode*      contextNodeP     = NULL;           // Not needed here, but dbModelToApiSubscription requires it
   KjNode*      showChangesP     = NULL;           // Not needed here, but dbModelToApiSubscription requires it

diff --git a/test/functionalTest/cases/0000_ngsild/ngsild_new_subscription-create-error-handling.test b/test/functionalTest/cases/0000_ngsild/ngsild_new_subscription-create-error-handling.test
@@ -312,12 +312,6 @@ payload='{
   "watchedAttributes": [ "P2" ],
   "timeInterval": 12,
   "q": "P2>10",
-  "geoQ": {
-    "geometry": "Point",
-    "coordinates": "[1,2]",
-    "georel": "near;maxDistance==1000",
-    "geoproperty": "not supported"
-  },
   "isActive": false,
   "notification": {
     "attributes": [ "P1", "P2", "A3" ],

diff --git a/test/functionalTest/cases/0000_ngsild/ngsild_subscription_create.test b/test/functionalTest/cases/0000_ngsild/ngsild_subscription_create.test
@@ -47,7 +47,7 @@ orionldStart CB
 # 04. GET /ngsi-ld/v1/subscription/http://a.b.c/subs/sub01, accepting jsonld - see @context in payload
 # 05. Attempt to create a subscription with an ID that is not a URI - see failure
 # 06. Attempt to create a subscription without 'entities' nor 'watchedAttributes' - see failure
-# 07. Attempt to create a subscription with both 'watchedAttributes' and 'timeInterval' - see failure
+# 07. Attempt to create a subscription with 'timeInterval' - see 501
 # 08. Create a subscription without 'timeInterval' nor 'watchedAttributes' - OK
 # 09. Create a subscription 'sub09' without 'expires' and 'throttling' - OK
 # 10. GET sub09 with sysAttrs and make sure no 'expires' nor 'throttling' is returned
@@ -234,8 +234,8 @@ echo
 echo
 
 
-echo "07. Attempt to create a subscription with both 'watchedAttributes' and 'timeInterval' - see failure"
-echo "==================================================================================================="
+echo "07. Attempt to create a subscription with 'timeInterval' - see 501"
+echo "=================================================================="
 payload='{
   "id": "http://a.b.c/subs/sub03",
   "type": "Subscription",
@@ -258,12 +258,6 @@ payload='{
   "watchedAttributes": [ "P2" ],
   "timeInterval": 12,
   "q": "P2>10",
-  "geoQ": {
-    "geometry": "Point",
-    "coordinates": [1,2],
-    "georel": "near",
-    "geoproperty": "location"
-  },
   "csf": "not implemented",
   "isActive": false,
   "notification": {
@@ -653,17 +647,17 @@ Date: REGEX(.*)
 }
 
 
-07. Attempt to create a subscription with both 'watchedAttributes' and 'timeInterval' - see failure
-===================================================================================================
-HTTP/1.1 400 Bad Request
-Content-Length: 160
+07. Attempt to create a subscription with 'timeInterval' - see 501
+==================================================================
+HTTP/1.1 501 Not Implemented
+Content-Length: 161
 Content-Type: application/json
 Date: REGEX(.*)
 
 {
-    "detail": "Both 'timeInterval' and 'watchedAttributes' present",
-    "title": "Inconsistent subscription",
-    "type": "https://uri.etsi.org/ngsi-ld/errors/BadRequestData"
+    "detail": "Periodic Notification Subscriptions are not implemented",
+    "title": "Not Implemented",
+    "type": "https://uri.etsi.org/ngsi-ld/errors/OperationNotSupported"
 }