Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Over-requesting to CPrs attributes that has been filtered out with attrs= #3745

Closed
fgalan opened this issue Dec 18, 2020 · 8 comments
Closed
Milestone

Comments

@fgalan
Copy link
Member

fgalan commented Dec 18, 2020

Situation: there are four entities (CAM1, CAM2, CAM3, CAM4) with a registered attribute makeSnapshot, setPTZ and setSnapshotsCrontab (depending on the entity one of them, two or the three).

We do a query that is filtering out the makeSnapshot, etc. registred attributes using attrs= query param

GET /v2/entities?type=Camera&attrs=TimeInstant%2CstreetAddress%2CstreamingURL

Regardless the filter, the makeSnapshot is queried to the CPr but it shouldn't. It can introduce an undesired response delay due to timeout if the CPr is not online, as shown in the trace below:

INFO@13:14:02  logTracing.cpp[146]: Starting forwarding for GET /v2/entities?type=Camera&attrs=TimeInstant%2CstreetAddress%2CstreamingURL
WARN@13:14:04  postQueryContext.cpp[304]: Other Error (context provider response to QueryContext is empty)
ERROR@13:14:09  postQueryContext.cpp[285]: Runtime Error (error 'Timeout was reached' forwarding 'Query' to providing application)
INFO@13:14:09  logTracing.cpp[212]: Request forwarded (regId: 5fd653693cf035ab6a56edcf): POST http://foo.net:8082/CAM1//op/query, request payload (91 bytes): {"entities":[{"id":"CAM1","type":"Camera"}],"attrs":["makeSnapshot"]}, response payload (0 bytes): , response code: Timeout was reached
ERROR@13:14:14  postQueryContext.cpp[285]: Runtime Error (error 'Timeout was reached' forwarding 'Query' to providing application)
INFO@13:14:14  logTracing.cpp[212]: Request forwarded (regId: 5fd653f23cf035ab6a56edd1): POST http://foo.net:8082/CAM2//op/query, request payload (90 bytes): {"entities":[{"id":"CAM2","type":"Camera"}],"attrs":["makeSnapshot"]}, response payload (0 bytes): , response code: Timeout was reached
ERROR@13:14:19  postQueryContext.cpp[285]: Runtime Error (error 'Timeout was reached' forwarding 'Query' to providing application)
INFO@13:14:19  logTracing.cpp[212]: Request forwarded (regId: 5fd650493cf035ab6a56edc7): POST http://foo.net:8082/CAM3//op/query, request payload (101 bytes): {"entities":[{"id":"CAM3","type":"Camera"}],"attrs":["makeSnapshot","setPTZ"]}, response payload (0 bytes): , response code: Timeout was reached
ERROR@13:14:24  postQueryContext.cpp[285]: Runtime Error (error 'Timeout was reached' forwarding 'Query' to providing application)
INFO@13:14:24  logTracing.cpp[212]: Request forwarded (regId: 5fcf8025b994162fe378e3bf): POST http://contextadapter.ddns.net:8082/CAM4//op/query, request payload (121 bytes): {"entities":[{"id":"CAM4","type":"Camera"}],"attrs":["makeSnapshot","setSnapshotsCrontab","setX"]}, response payload (0 bytes): , response code: Timeout was reached
INFO@13:14:24  logTracing.cpp[79]: Request received: GET /v2/entities?type=Camera&attrs=TimeInstant%2CstreetAddress%2CstreamingURL, response code: 200
@fgalan
Copy link
Member Author

fgalan commented Dec 18, 2020

It also happens if the GET is done in a specific entity (eg. CAM3). I mean:

GET /v2/entities/CAM3?type=Camera&attrs=TimeInstant%2CstreetAddress%2CstreamingURL

Results in:

INFO@13:54:51  logTracing.cpp[146]: Starting forwarding for GET /v2/entities/CAM3?type=Camera&attrs=TimeInstant%2CstreetAddress%2CstreamingURL
ERROR@13:54:56  postQueryContext.cpp[285]: Runtime Error (error 'Timeout was reached' forwarding 'Query' to providing application)
INFO@13:54:56  logTracing.cpp[212]: Request forwarded (regId: 5fd650493cf035ab6a56edc7): POST http://contextadapter.ddns.net:8082/CAM7-34DGSDMM//op/query, request payload (101 bytes): {"entities":[{"id":"CAM3","type":"Camera"}],"attrs":["makeSnapshot","setPTZ"]}, response payload (0 bytes): , response code: Timeout was reached
INFO@13:54:56  logTracing.cpp[79]: Request received: GET /v2/entities/CAM3?type=Camera&attrs=TimeInstant%2CstreetAddress%2CstreamingURL, response code: 200

@fgalan
Copy link
Member Author

fgalan commented Dec 18, 2020

(Maybe we already have this problem described in another issue, but I haven't found it so far)

@fgalan
Copy link
Member Author

fgalan commented Dec 18, 2020

Having a look to code in getEntity.cpp:

  // Get attrs and metadata filters from URL params
  // Note we cannot set the attrs filter on &parseDataP->qcr.res.attrsFilterList given that parameter is used for querying on DB
  // and some .test would break. We use a fresh variable (attributeFilter) for that
  StringList attributeFilter;
  setAttrsFilter(ciP->uriParam, ciP->uriParamOptions, &attributeFilter);
  ...

  // Call standard op postQueryContext
  postQueryContext(ciP, components, compV, parseDataP);

  // Render entity response
  Entity       entity;
  ...
  entity.fill(parseDataP->qcrs.res, &oe);

  if (oe.code == SccNone)
  {
    TIMED_RENDER(answer = entity.toJson(getRenderFormat(ciP->uriParamOptions),
                                        attributeFilter.stringV,
                                        false,
                                        parseDataP->qcr.res.metadataList.stringV));
  }
  ...

Note that the attrs= (stored in attributeFilter object) is not pushed down to the lower service routine function postQueryContext() where all the forwarding logic happens. It is only used to filter after that, in the entities.toJson() call.

It seems that in the past we considered pushing down it, but it seems has some problem:

  // Note we cannot set the attrs filter on &parseDataP->qcr.res.attrsFilterList given that parameter is used for querying on DB
  // and some .test would break. We use a fresh variable (attributeFilter) for that

@fgalan
Copy link
Member Author

fgalan commented Dec 18, 2020

Having a look to code in getEntity.cpp:

Situation is pretty much the same for getEntities.cpp

@fgalan
Copy link
Member Author

fgalan commented Dec 18, 2020

I have done this little hack:

diff --git a/src/lib/serviceRoutinesV2/getEntities.cpp b/src/lib/serviceRoutinesV2/getEntities.cpp
index cbc82502e..afcc483da 100644
--- a/src/lib/serviceRoutinesV2/getEntities.cpp
+++ b/src/lib/serviceRoutinesV2/getEntities.cpp
@@ -303,10 +303,11 @@ std::string getEntities
   }
 
   // Get attrs and metadata filters from URL params
-  // Note we cannot set the attrs filter on &parseDataP->qcr.res.attrsFilterList given that parameter is used for querying on DB
+  // Note we cannot set the attrs filter on &parseDataP->qcr.res.attrsList given that parameter is used for querying on DB
   // and some .test would break. We use a fresh variable (attributeFilter) for that
-  StringList attributeFilter;
-  setAttrsFilter(ciP->uriParam, ciP->uriParamOptions, &attributeFilter);
+  //StringList attributeFilter;
+  //setAttrsFilter(ciP->uriParam, ciP->uriParamOptions, &attributeFilter);
+  setAttrsFilter(ciP->uriParam, ciP->uriParamOptions, &parseDataP->qcr.res.attrsList);
   setMetadataFilter(ciP->uriParam, &parseDataP->qcr.res.metadataList);
 
   // 02. Call standard op postQueryContext
@@ -332,7 +333,8 @@ std::string getEntities
     else
     {
       TIMED_RENDER(answer = entities.toJson(getRenderFormat(ciP->uriParamOptions),
-                                            attributeFilter.stringV,
+                                            parseDataP->qcr.res.attrsList.stringV,
+                                            //attributeFilter.stringV,
                                             false,
                                             parseDataP->qcr.res.metadataList.stringV));
       ciP->httpStatusCode = SccOk;
diff --git a/src/lib/serviceRoutinesV2/getEntity.cpp b/src/lib/serviceRoutinesV2/getEntity.cpp
index 044d6d5e7..3c70f7c87 100644
--- a/src/lib/serviceRoutinesV2/getEntity.cpp
+++ b/src/lib/serviceRoutinesV2/getEntity.cpp
@@ -87,10 +87,11 @@ std::string getEntity
   parseDataP->qcr.res.fill(entityId, type, "false", EntityTypeEmptyOrNotEmpty, "");
 
   // Get attrs and metadata filters from URL params
-  // Note we cannot set the attrs filter on &parseDataP->qcr.res.attrsFilterList given that parameter is used for querying on DB
+  // Note we cannot set the attrs filter on &parseDataP->qcr.res.attrsList given that parameter is used for querying on DB
   // and some .test would break. We use a fresh variable (attributeFilter) for that
-  StringList attributeFilter;
-  setAttrsFilter(ciP->uriParam, ciP->uriParamOptions, &attributeFilter);
+  //StringList attributeFilter;
+  //setAttrsFilter(ciP->uriParam, ciP->uriParamOptions, &attributeFilter);
+  setAttrsFilter(ciP->uriParam, ciP->uriParamOptions, &parseDataP->qcr.res.attrsList);
   setMetadataFilter(ciP->uriParam, &parseDataP->qcr.res.metadataList);
 
   // Call standard op postQueryContext
@@ -113,7 +114,8 @@ std::string getEntity
   if (oe.code == SccNone)
   {
     TIMED_RENDER(answer = entity.toJson(getRenderFormat(ciP->uriParamOptions),
-                                        attributeFilter.stringV,
+                                        parseDataP->qcr.res.attrsList.stringV,
+                                        //attributeFilter.stringV,
                                         false,
                                         parseDataP->qcr.res.metadataList.stringV));
   }

And now it works without forwarding requests:

GET /v2/entities?type=Camera&attrs=TimeInstant%2CstreetAddress%2CstreamingURL

INFO@14:51:28  logTracing.cpp[79]: Request received: GET /v2/entities?type=Camera&attrs=TimeInstant%2CstreetAddress%2CstreamingURL, response code: 200

GET /v2/entities/CAM3?type=Camera&attrs=TimeInstant%2CstreetAddress%2CstreamingURL

INFO@14:54:13  logTracing.cpp[79]: Request received: GET /v2/entities/CAM3?type=Camera&attrs=TimeInstant%2CstreetAddress%2CstreamingURL, response code: 200

Let's do a full pass in functional tests with the hacked code to see which ones break...

@fgalan
Copy link
Member Author

fgalan commented Dec 18, 2020

Let's do a full pass in functional tests with the hacked code to see which ones break...

Only one test is failing:

----------- Failing tests ------------------
  o  3068_ngsi_v2_based_forwarding/forwards_with_providers.test

@fgalan
Copy link
Member Author

fgalan commented Dec 18, 2020

That test has the following script:

# 01. Register entities of type T, with ID .*, and attribute A1, for CP1
# 02. Register entities of type T, with ID .*, and attribute A2, for CP2
# 03. Register entities of type T, with ID .*, and attribute A3, for CP3
# 04. Register entities of type U, with ID .*, and attribute A4, for CP4
# 05. Create E1/T in CP1
# 06. Create E2/T in CP2
# 07. Create E3/T in CP3
# 08. Create E4/U in CP4
# 09. Query entities of type T - see E1, E2 and E3
# 10. Query entities of type U - see E4
# 11. Query entities of type T and with attr A1 - see E1 with A1, and E2+E3 without attrs

The fail is in step 11 that instead of returning E1 with A1 and E2+E3 without attrs only returns E1 (A1). However, I think this is the right behavior... Maybe the test is wrong?

@fgalan
Copy link
Member Author

fgalan commented Jan 11, 2021

PR #3751

@fgalan fgalan added this to the 2.6.0 milestone Jan 11, 2021
@fgalan fgalan closed this as completed Jan 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant