search

telefonicaid / fiware-orion

Context Broker and CEF building block for context data management, providing NGSI interfaces.
https://fiware-orion.rtfd.io/
GNU Affero General Public License v3.0
212 stars 265 forks source link

Basic query and update forwarding cases with registration '.*' pattern #3463

Open fgalan opened 5 years ago

fgalan commented 5 years ago

The basic cases are:

  1. regR = .*, query = .*, attrs = [ A, ... ]
  2. regR = .*, query = 'E', attrs = {null}
  3. regR = .*, query = 'E', attrs = [ A ... ]
  4. regR = .*, query = 'E', attrs = {null}
  5. regR = .*, update = 'E', attrs = [ A ... ]

This first one was covered in issue #3458. The present issue is about covering the remaining cases.

kzangeli commented 5 years ago

I have done some experimentation and found that query cases with entity id (cases 2-4) don't work. I haven't tested but I strongly doubt that the update case with entity id works. Doesn't really matter.

The implementation of forwarding is made with something else in mind. Instead of looking at the query/update, we look at the registrations. Modifying the existing code is far more difficult than rewriting the forwarding part.

My suggestion would be to leave NGSIv1 as is (for "backward compatibility") and completely rewrite the forwarding algorithm for NGSIv2.

kzangeli commented 5 years ago

See step 08 in the following functional test: legacy-query-over-entity-id-for-regs-with-idpattern.txt

kzangeli commented 5 years ago

To troubleshoot, use the branch (pushed to origin): bug/3463_forwarded_query_with_entity_in_for_reg_with_idpattern

In that branch, run the functest legacy-query-over-entity-id-for-regs-with-idpattern.test

Search for the second occurrence of Serving request GET /v2/entities/E1 in the logfile. That is the request that fails, it starts there.

From what I have seen, registrationsQuery (I modified it) seems to do its job, it finds the three matching regs. But this trace line: MongoGlobal.cpp[1901]:processEntity : Not Included ... is present for all three hits from registrationsQuery and the output from mongoQueryContext is an empty vector and no forwards are done by the service routine, when really, all three hits should be output in the vector and three forwards should be performed.

fgalan commented 5 years ago

I have had a look to the branch and test. And made it to work.

The "Not Found" problem in step 8 is solved just taking the ".*" pattern into account for matching. In particular, this modification in processEntity():

--- a/src/lib/mongoBackend/MongoGlobal.cpp
+++ b/src/lib/mongoBackend/MongoGlobal.cpp
@@ -1890,7 +1890,16 @@ static void processEntity(ContextRegistrationResponse* crr, const EntityIdVector
   en.isPattern = entity.hasField(REG_ENTITY_ISPATTERN)? getStringFieldF(entity, REG_ENTITY_ISPATTERN) : "false";

   LM_T(LmtForward, ("Processing entity '%s' / '%s' / '%s'", en.id.c_str(), en.type.c_str(), en.isPattern.c_str()));
-  if (includedEntity(en, enV))
+
+  if ((en.isPattern == "true") && (en.id == ".*"))
+  {
+    // The .* pattern always matches
+    LM_T(LmtForward, ("Included (1) !!!"));
+    EntityId* enP = new EntityId(en.id, en.type, en.isPattern);
+
+    crr->contextRegistration.entityIdVector.push_back(enP);
+  }
+  else if (includedEntity(en, enV))
   {
     LM_T(LmtForward, ("Included!!!"));
     EntityId* enP = new EntityId(en.id, en.type, en.isPattern);

So, now the service routing logic is able to get the rigth CPr information provided by the mongoBackend. In this situation a new problem arises: the responses has more entities (E1, E2 and E3) than the ones requested by the cliente (E1). In the .test it appears a as "Too many results" given the NGSIv1-to-NGSIv2 conversion logic between service routinges.

However, the solution is also simple: just filtering not required entities out in the populate() stage at the end of the service routing logic. This is the change (quick & dirty, but easy to do cleanly):

--- a/src/lib/orionTypes/QueryContextResponseVector.cpp
+++ b/src/lib/orionTypes/QueryContextResponseVector.cpp
@@ -32,6 +32,7 @@
 #include "common/globals.h"
 #include "common/tag.h"
 #include "orionTypes/QueryContextResponseVector.h"
+#include "ngsi10/QueryContextRequest.h"
 #include "ngsi/Request.h"

@@ -204,12 +205,11 @@ std::string QueryContextResponseVector::toJsonV1(bool asJsonObject, bool details
 }

 /* ****************************************************************************
 *
 * QueryContextResponseVector::populate -
 */
-void QueryContextResponseVector::populate(QueryContextResponse* responseP)
+void QueryContextResponseVector::populate(QueryContextRequest* qcrP, QueryContextResponse* responseP)
 {
   //
   // We have a vector of QueryContextResponse.
@@ -280,6 +280,27 @@ void QueryContextResponseVector::populate(QueryContextResponse* responseP)
       }
       else  // Not found so we will have to create a new ContextElementResponse
       {
+        // FIXME: only works for queries id so: 1) not pattern, 2) the '.*' pattern
+        // FIXME: type should be also taken into account
+        // FIXME: should be an external function
+        // -------------------------
+        bool toFilter = true;
+        for (unsigned int iix = 0; iix < qcrP->entityIdVector.size(); ++iix)
+        {
+          EntityId* enFromQuery = qcrP->entityIdVector[iix];
+          if ( ((enFromQuery->id == ".*") && (enFromQuery->isPattern == "true")) ||
+               (enFromQuery->id == cerP->entity.id) )
+          {
+            toFilter = false;
+            break;
+          }
+        }
+        if (toFilter)
+        {
+          continue;
+        }
+        // ---------------------------
+
         ContextElementResponse* newCerP = new ContextElementResponse(cerP);

         newCerP->statusCode.fill(SccOk);
--- a/src/lib/orionTypes/QueryContextResponseVector.h
+++ b/src/lib/orionTypes/QueryContextResponseVector.h
@@ -29,7 +29,7 @@
 #include <vector>

 #include "ngsi10/QueryContextResponse.h"
-
+#include "ngsi10/QueryContextRequest.h"

 /* ****************************************************************************
@@ -44,7 +44,8 @@ typedef struct QueryContextResponseVector
   void                   push_back(QueryContextResponse* item);
   void                   release(void);
   std::string            toJsonV1(bool asJsonObject, bool details, const std::string& detailsString);
-  void                   populate(QueryContextResponse* responseP);
+  //void                   populate(QueryContextResponse* responseP);
+  void                   populate(QueryContextRequest* qcrP, QueryContextResponse* responseP);

   QueryContextResponse*  operator[](unsigned int ix) const;
--- a/src/lib/serviceRoutines/postQueryContext.cpp
+++ b/src/lib/serviceRoutines/postQueryContext.cpp
@@ -575,7 +575,6 @@ std::string postQueryContext

   TIMED_RENDER(answer = responseV.toJsonV1(asJsonObject, details, qcrsP->errorCode.details));

-
   //
   // Time to cleanup.
   // But before doing that ...
@@ -588,7 +587,8 @@ std::string postQueryContext
   // we don't leak any memory.
   //
   qcrsP->release();
-  responseV.populate(qcrsP);
+  //responseV.populate(qcrsP);
+  responseV.populate(qcrP, qcrsP);

   qcrP->release();
   requestV.release();

And that's all.

Thus, the current logic flow (which actually is not so hard after having a look to the corresponding flows in the development manual [1][2] and a brief gdb session looking the program flow and how the data sctructures evolve across it ;) is well prepared to cope wih the new cases just adapting the entity matching logic in mongoBackend and the filtering logic in the calling service routine. No need of a large rewrite of the code.

fgalan commented 3 years ago

Related question at SOF: https://stackoverflow.com/questions/67520066/orion-context-provider-query-multiple-entities/67530891

fgalan commented 3 years ago

PR https://github.com/telefonicaid/fiware-orion/pull/3852 solves some of the cases included in this issue. In particular, it solves cases 2-4 (actually 4 is a duplicated of two).

This can be seen in cases/3463_regid_global_pattern_query_forwarding/regid_global_pattern_query_forwarding.test steps

# 07. Query E1, attrs = {null} at CB, see E1 A1/A2 <--- case 2/4
# 08. Query E1, attrs = [ A2 ] at CB, see E1 A2  <--- case 3

In addition, the test included in https://github.com/telefonicaid/fiware-orion/issues/3463#issuecomment-476227327 has been as cases/3463_regid_global_pattern_query_forwarding/regid_global_pattern_query_forwarding2.test with the following adjustments:

--- /tmp/legacy-query-over-entity-id-for-regs-with-idpattern.txt    2021-05-19 08:40:51.000000000 +0200
+++ cases/3463_regid_global_pattern_query_forwarding/regid_global_pattern_query_forwarding2.test    2021-05-19 08:26:29.209078309 +0200
@@ -21,14 +21,14 @@
 # VALGRIND_READY - to mark the test ready for valgrindTestSuite.sh

 --NAME--
-Forwarded Query with Entity ID and registrations with idPattern
+Reg ID global pattern query forwarding cases (extra)

 --SHELL-INIT--
 dbInit CB
 dbInit CP1
 dbInit CP2
 dbInit CP3
-brokerStart CB 38,186-187,235 IPV4
+brokerStart CB 0 IPV4
 brokerStart CP1
 brokerStart CP2
 brokerStart CP3
@@ -203,6 +203,7 @@
 ================================================
 HTTP/1.1 201 Created
 Content-Length: 0
+Location: /v2/entities/E1?type=T
 Fiware-Correlator: REGEX([0-9a-f\-]{36})
 Date: REGEX(.*)

@@ -212,6 +213,7 @@
 ================================================
 HTTP/1.1 201 Created
 Content-Length: 0
+Location: /v2/entities/E2?type=T
 Fiware-Correlator: REGEX([0-9a-f\-]{36})
 Date: REGEX(.*)

@@ -221,6 +223,7 @@
 ================================================
 HTTP/1.1 201 Created
 Content-Length: 0
+Location: /v2/entities/E3?type=T
 Fiware-Correlator: REGEX([0-9a-f\-]{36})
 Date: REGEX(.*)

@@ -237,29 +240,29 @@
 [
     {
         "A1": {
-            "metadata": {}, 
-            "type": "Number", 
+            "metadata": {},
+            "type": "Number",
             "value": "1"
-        }, 
-        "id": "E1", 
+        },
+        "id": "E1",
         "type": "T"
-    }, 
+    },
     {
         "A2": {
-            "metadata": {}, 
-            "type": "Number", 
+            "metadata": {},
+            "type": "Number",
             "value": "2"
-        }, 
-        "id": "E2", 
+        },
+        "id": "E2",
         "type": "T"
-    }, 
+    },
     {
         "A3": {
-            "metadata": {}, 
-            "type": "Number", 
+            "metadata": {},
+            "type": "Number",
             "value": "3"
-        }, 
-        "id": "E3", 
+        },
+        "id": "E3",
         "type": "T"
     }
 ]
@@ -268,25 +271,28 @@
 08. Query entities of ID E1 - see E1
 ====================================
 HTTP/1.1 200 OK
-Content-Length: 
+Content-Length: 71
 Content-Type: application/json
 Fiware-Correlator: REGEX([0-9a-f\-]{36})
 Date: REGEX(.*)

 {
     "A1": {
-        "metadata": {}, 
-        "type": "Number", 
+        "metadata": {},
+        "type": "Number",
         "value": "1"
-    }, 
-    "id": "E1", 
+    },
+    "id": "E1",
     "type": "T"
 }

 --TEARDOWN--
 brokerStop CB
-#dbDrop CB
+brokerStop CP1
+brokerStop CP2
+brokerStop CP3
+dbDrop CB
 dbDrop CP1
 dbDrop CP2
 dbDrop CP3

At the end, the solution is not the one described above but it also works.

fgalan commented 3 years ago

The issue will remain opened, as case 5 has not been yet covered:

  1. regR = .*, update = 'E', attrs = [ A ... ]
ArqamFarooqui110719 commented 1 year ago

Hi @fgalan sir, is this issue still valid?

fgalan commented 1 year ago

I'd suggest look for issues not related with registration or forwarding functionality. That functionality could be deprecated in the future (it is somehow problematic) so it would be wiser to devote effort to other more productive issues.