Fix for issue4149 in case of requestForwarding #4275

ArqamFarooqui110719 · 2023-02-10T15:42:58Z

Fix issue #4149

ArqamFarooqui110719 · 2023-02-14T13:16:35Z

Currently total 16 sets of existing test cases (with cache and without cache) are failing.
We have investigated and observed that some of the existing test cases are failing due to our code changes. We are fixing the same and will update the PR.

ArqamFarooqui110719 · 2023-02-20T16:37:29Z

Hi @fgalan ,
I have updated the code for 13 sets of failed test cases and modified the 3 existing FTs because those FTs are related with request forwarding.
Currently all the test cases are passing successfully. Kindly review the PR.

fgalan · 2023-05-10T15:26:38Z

test/functionalTest/cases/3068_ngsi_v2_based_forwarding/check_no_overquerying.test

@@ -107,7 +107,7 @@ Date: REGEX(.*)

 03. Dump accumulator and check that E/A was forwarded (not including B or C)
 ============================================================================
-POST http://localhost:REGEX(\d+)/v2/op/query
+POST http://localhost:REGEX(\d+)/v2/op/query?limit=20&offset=0&options=count


Existing tests should be modified by this PR.

If we change to rollback the change this way

Suggested change

POST http://localhost:REGEX(\d+)/v2/op/query?limit=20&offset=0&options=count

POST http://localhost:REGEX(\d+)/v2/op/query

does the test pass?

NTC (answered as general comment here)

ArqamFarooqui110719 · 2023-05-11T06:39:40Z

No, without modification test was failing.

Because we have modified the forwarding URL in case of request forwarding with pagination parameters if pagination parameters are not given in the API so default pagination parameters will be passed.

Due to which existing FT need to be modified.

fgalan · 2023-05-16T15:51:24Z

...onalTest/cases/4149_pagination_for_request_forwarding/pagination_for_request_forwarding.test

+# 25. Create entity C10/T with attribute A2, in CP2
+# 26. Register entities of type T, with ID .*, and attribute A1, for CP1
+# 27. Register entities of type T, with ID .*, and attribute A2, for CP2
+# 28. GET /v2/entities with limit=5 and check no limit forwarded to CP


Suggested change

# 28. GET /v2/entities with limit=5 and check no limit forwarded to CP

# 28. GET /v2/entities with limit=3 and check no limit forwarded to CP

As this is the actually step

Fixed in 1e47b07

fgalan · 2023-05-16T15:51:52Z

...onalTest/cases/4149_pagination_for_request_forwarding/pagination_for_request_forwarding.test

+
+
+
+echo "28. GET /v2/entities with limit<5 and check no limit forwarded to CP"


To be precise:

Suggested change

echo "28. GET /v2/entities with limit<5 and check no limit forwarded to CP"

echo "28. GET /v2/entities with limit=3 and check no limit forwarded to CP"

Fixed in 1e47b07

fgalan · 2023-05-16T15:52:54Z

...onalTest/cases/4149_pagination_for_request_forwarding/pagination_for_request_forwarding.test

+
+
+
+echo "29. GET /v2/entities with limit=5 and check no limit forwarded to CP"


This doesn't match with the text in the step description for step 29, which is:

GET /v2/entities with limit=10 and check remaining limit forwarded to CP

Please, review the overall file.

Fixed in 1e47b07

fgalan · 2023-05-16T15:55:31Z

...onalTest/cases/4149_pagination_for_request_forwarding/pagination_for_request_forwarding.test

+
+
+
+echo "32. GET /v2/entities with offset<5 and check no offset is forwarded to CP"


Please don't use < and > in the text, but precise statements with =. Otherwise, the steps could be more difficult to follow.

Fixed in 1e47b07

fgalan · 2023-05-16T16:00:01Z

...Test/cases/4149_pagination_for_request_forwarding/pagination_without_request_forwarding.test

+# 01. Create entity A1/T in CB
+# 02. Create entity A2/T in CB
+# 03. Create entity A3/T in CB
+# 04. Create entity A4/T in CB
+# 05. Create entity A5/T in CB
+# 06. GET /v2/entities with limit=5
+# 07. GET /v2/entities with offset=2
+# 08. GET /v2/entities with options=count
+# 09. GET /v2/entities with limit=5 and offset=2
+# 10. GET /v2/entities with limit=2 and options=count
+# 11. GET /v2/entities with offset=2 and options=count


According to this script, this .test doesn't do anything related with CPr. Thus, it doesn't seem to be related with issue #4149. Or maybe I'm missing something?

Yes, these FTs are not related with CPr. These FTs are related with pagination in normal case.
We have included these FTs, to confirm that existing pagination functionality (Without Request Forwarding) is also working fine.

Yes, these FTs are not related with CPr. These FTs are related with pagination in normal case.
We have included these FTs, to test that existing pagination functionality (Without Request Forwarding) is also working fine.

Ok. It doesn't hurt :)

Let's keep the test. Although maybe it's better to use a better name. Suggestion: pagination_check_regular_functionality_does_not_break.test

Fixed in 1e47b07

fgalan · 2023-05-16T16:05:03Z

src/lib/rest/httpRequestSend.cpp

+   int                                        providerLimit,
+   int                                        providerOffset,
+   ApiVersion                                 apiVersion,
   CURL*                                      _curl,


As a common practise, we keep the existing argument order in functions and add new arguments at the end (i.e. after timeoutInMilliseconds).

Moreover, I think apiVersion can be removed.

I think, new parameters should be added before the timeoutInMilliseconds, as timeoutInMilliseconds is a default parameter.

So if we are keeping the newly added parameters after the timeoutInMilliseconds then it is taking incorrect values in httpRequestSend.cpp file, (I have added LM_E level log to print the values of new parametes), for e.g.

If we keep the parameters after the timeoutInMilliseconds:
Actual value of new parameters inside postQueryContext.cpp file, before httpRequestSend.cpp() function call:

time=2023-05-19T04:06:05.984Z | lvl=ERROR | corr=7a3c718e-f5fa-11ed-8540-e3fa43514f13 | trans=1684469149-080-00000000001 | from=127.0.0.1 | srv= | subsrv= | comp=Orion | op=postQueryContext.cpp[305]:queryForward | msg=Arqam-17may23 Error (providerLimit value in postQueryContext/queryForward at L304: '15') time=2023-05-19T04:06:05.984Z | lvl=ERROR | corr=7a3c718e-f5fa-11ed-8540-e3fa43514f13 | trans=1684469149-080-00000000001 | from=127.0.0.1 | srv= | subsrv= | comp=Orion | op=postQueryContext.cpp[306]:queryForward | msg=Arqam-17may23 Error (providerOffset value in postQueryContext/queryForward at L305: '0')

Incorrect value of new parameters being passed inside httpRequestSend.cpp file:

time=2023-05-19T04:06:05.984Z | lvl=ERROR | corr=7a3c718e-f5fa-11ed-8540-e3fa43514f13 | trans=1684469149-080-00000000001 | from=127.0.0.1 | srv= | subsrv= | comp=Orion | op=httpRequestSend.cpp[267]:httpRequestSend | msg=Arqam-17may23 Error (providerLimit value in httpRequestSend at L267: '0') time=2023-05-19T04:06:05.984Z | lvl=ERROR | corr=7a3c718e-f5fa-11ed-8540-e3fa43514f13 | trans=1684469149-080-00000000001 | from=127.0.0.1 | srv= | subsrv= | comp=Orion | op=httpRequestSend.cpp[268]:httpRequestSend | msg=Arqam-17may23 Error (providerOffset value in httpRequestSend at L268: '0')

If we keep the parameters before the parameter timeoutInMilliseconds:
In that case correct values are being received in httpRequestSend.cpp file (I have added LM_E level log to print the values):

Actual values of new parameters inside postQueryContext.cpp file, before httpRequestSend.cpp() function call:

time=2023-05-19T04:16:58.653Z | lvl=ERROR | corr=ff41bbfe-f5fb-11ed-8540-e3fa43514f13 | trans=1684469739-808-00000000001 | from=127.0.0.1 | srv=<none> | subsrv=<none> | comp=Orion | op=postQueryContext.cpp[305]:queryForward | msg=Arqam-17may23 Error (providerLimit value in postQueryContext/queryForward at L304: '15') time=2023-05-19T04:16:58.653Z | lvl=ERROR | corr=ff41bbfe-f5fb-11ed-8540-e3fa43514f13 | trans=1684469739-808-00000000001 | from=127.0.0.1 | srv=<none> | subsrv=<none> | comp=Orion | op=postQueryContext.cpp[306]:queryForward | msg=Arqam-17may23 Error (providerOffset value in postQueryContext/queryForward at L305: '0')

Correct values of new parameters being passed inside httpRequestSend.cpp file:

time=2023-05-19T04:16:58.653Z | lvl=ERROR | corr=ff41bbfe-f5fb-11ed-8540-e3fa43514f13 | trans=1684469739-808-00000000001 | from=127.0.0.1 | srv=<none> | subsrv=<none> | comp=Orion | op=httpRequestSend.cpp[267]:httpRequestSend | msg=Arqam-17may23 Error (providerLimit value in httpRequestSend at L267: '15') time=2023-05-19T04:16:58.653Z | lvl=ERROR | corr=ff41bbfe-f5fb-11ed-8540-e3fa43514f13 | trans=1684469739-808-00000000001 | from=127.0.0.1 | srv=<none> | subsrv=<none> | comp=Orion | op=httpRequestSend.cpp[268]:httpRequestSend | msg=Arqam-17may23 Error (providerOffset value in httpRequestSend at L268: '0')

Suggested signature:

extern int httpRequestSend ( CURL* curl, const std::string& idStringForLogs, ... long long* statusCodeP, const std::map<std::string, std::string>& extraHeaders, int providerLimit = -1, int providerOffset = -1, const std::string& acceptFormat = "", long timeoutInMilliseconds = -1 );

Note the suggestion to make providerLimit and priverOffset default parameter. The usage of -1 as default is aligned with previous comments and would make some parts of the code simpler (the ones in notification logic and in update forward logic, where CPrs pagination doesn't make sense).

Fixed in 1e47b07

fgalan · 2023-05-16T16:06:47Z

src/lib/ngsiNotify/doNotify.cpp

+  int                 providerLimit  = DEFAULT_PAGINATION_LIMIT_INT;
+  int                 providerOffset = DEFAULT_PAGINATION_OFFSET_INT;
+  ApiVersion          apiVersion     = V1;


Remove this block and use -1 for providerLimit and provideOffset in the call to httpRequestSend() below, meaning with <0 value that the parameter hasn't to be included.

Fixed in 1e47b07

fgalan · 2023-05-16T16:09:28Z

src/lib/rest/httpRequestSend.cpp

+
+  if (apiVersion == V2 && verb == "POST" && resource == "/v2/op/query")
+  {
+    std::string pLimit = std::to_string(providerLimit);
+    std::string pOffset = std::to_string(providerOffset);
+    url = protocol + url + ":" + portAsString + (resource.at(0) == '/'? "" : "/") + resource + "?limit=" + pLimit + "&offset=" + pOffset + "&options=count";
+  }
+  else
+  {
+    url = protocol + url + ":" + portAsString + (resource.at(0) == '/'? "" : "/") + resource;
+  }


Related with previous comments (apiVersion not needed and >=0 to mean you want to include some of the new URL query parameters), I'd suggest to do something like this:

Suggested change

if (apiVersion == V2 && verb == "POST" && resource == "/v2/op/query")

{

std::string pLimit = std::to_string(providerLimit);

std::string pOffset = std::to_string(providerOffset);

url = protocol + url + ":" + portAsString + (resource.at(0) == '/'? "" : "/") + resource + "?limit=" + pLimit + "&offset=" + pOffset + "&options=count";

}

else

{

url = protocol + url + ":" + portAsString + (resource.at(0) == '/'? "" : "/") + resource;

}

url = protocol + url + ":" + portAsString + (resource.at(0) == '/'? "" : "/") + resource;

if (providerLimit >= 0) && (providerOffset >=0)

{

std::string pLimit = std::to_string(providerLimit);

std::string pOffset = std::to_string(providerOffset);

url += "?limit=" + pLimit + "&offset=" + pOffset + "&options=count";

}

Fixed in 1e47b07

fgalan · 2023-05-16T16:16:16Z

src/lib/serviceRoutines/postUpdateContext.cpp

+  int             providerLimit  = DEFAULT_PAGINATION_LIMIT_INT;
+  int             providerOffset = DEFAULT_PAGINATION_OFFSET_INT;


Similar comment to the one done at src/lib/ngsiNotify/doNotify.cpp:

Remove this block and use -1 for providerLimit and provideOffset in the call to httpRequestSend() below, meaning with <0 value that the parameter hasn't to be included.

Fixed in 1e47b07

fgalan · 2023-05-16T16:18:04Z

src/lib/serviceRoutines/postQueryContext.cpp

-  if ((ciP->apiVersion == V2) && (ciP->uriParamOptions["count"]))
+  std::string georel  =  ciP->uriParam["georel"];
+  if ((ciP->apiVersion == V2))
  {
-    countP = &count;
+    if (!((ciP->inMimeType == JSON && strstr(ciP->payload, "near")) || strstr(georel.c_str(), "near")))
+    {
+      countP = &count;
+    }


Could you elaborate on the changes done in this part?

I don't get the part related with geolocation ("georel", "near", etc), as it seems geolocation hasn't nothing to do with issue #4149...

Without the above geolocation check, all the FTs related with geolocation attributes ("georel", "near") were failing for e.g. FT 1038_ngsiv2_geo_location_entities, 1177_geometry_and_coords and 4114_geojson_feature_support.
So we have added a georel related check here for those FTs.

I'm trying to "decode" the logical expression... it's hard:

(!((ciP->inMimeType == JSON && strstr(ciP->payload, "near")) || strstr(georel.c_str(), "near")))

ciP->inMimeType == JSON && strstr(ciP->payload, "near": the mime type in the request is JSON and the string near is in the payload of the request

strstr(georel.c_str(), "near"): the georeal (assuming georeal has been specified) is near

So, in sum

IF NOT (("the mime type in the request is JSON and the string near is in the payload of the request") OR "the georeal (assuming georeal has been specified) is near")

I don't get why this is needed to avoid some test to fail. What kind of fail did you get in those FT? Could you provide the diff in this tests (preveribly in a graphical way, e.g. meld?

I'd like to understand this in deep before applying the "guard" you propose...

If we are not applying the above geolocation check then below error is occurring on FT.

I have attach the screenshot of .out file for failed FT test/functionalTest/cases/4114_geojson_feature_support/geojson_feature_support.test

Functional Test case:

Output of FT:

Earlier we have used a simple check at postQueryContext.cpp (L#533) as below:

if (ciP->apiVersion == V2) { countP = &count; }

But due to some FTs were failing related with geolocation attribute, so we have added an extra check (at L#536) as below i.e. related with geolocation attributes so that our code changes does not affect the geolocation attribute related APIs:

std::string georel = ciP->uriParam["georel"]; if ((ciP->apiVersion == V2)) { if (!((ciP->inMimeType == JSON && strstr(ciP->payload, "near")) || strstr(georel.c_str(), "near"))) { countP = &count; } }

That's weird... I'd need to have a look to the code and debug myself.

Given this is the only pending comment at this point, I'd suggest to merge your PR in a pre-landing branch in the repository (issue-4149-prelanding) so I can debug in it. Then, we will continue if needed in that new PR.

That PR is #4341

I have tested to revert to the original code, this way:

diff --git a/src/lib/serviceRoutines/postQueryContext.cpp b/src/lib/serviceRoutines/postQueryContext.cpp index a2952d7b9..64201e799 100644 --- a/src/lib/serviceRoutines/postQueryContext.cpp +++ b/src/lib/serviceRoutines/postQueryContext.cpp @@ -531,13 +531,17 @@ std::string postQueryContext // In API version 2, this has changed completely. Here, the total count of local entities is returned // if the URI parameter 'count' is set to 'true', and it is returned in the HTTP header Fiware-Total-Count. // - std::string georel = ciP->uriParam["georel"]; + /*std::string georel = ciP->uriParam["georel"]; if ((ciP->apiVersion == V2)) { if (!((ciP->inMimeType == JSON && strstr(ciP->payload, "near")) || strstr(georel.c_str(), "near"))) { countP = &count; } + }*/ + if ((ciP->apiVersion == V2) && (ciP->uriParamOptions["count"])) + { + countP = &count; } else if ((ciP->apiVersion == V1) && (ciP->uriParam["details"] == "on")) {

and it's seem it's working:

$ ./testHarness.sh cases/4114_geojson_feature_support lun 05 jun 2023 10:00:11 CEST Run tests 0 to 0 0001/6: geojson_feature_errors.test .................................................................................. 04 seconds 0002/6: geojson_feature_errors_ignoretype.test ....................................................................... 04 seconds 0003/6: geojson_feature_support.test ................................................................................. 02 seconds 0004/6: geojson_feature_support_ignoretype.test ...................................................................... 03 seconds 0005/6: geojson_featurecollection_support.test ....................................................................... 02 seconds 0006/6: geojson_featurecollection_support_ignoretype.test ............................................................ 03 seconds Total test time: 18.30 seconds $ ./testHarness.sh cases/1038_ngsiv2_geo_location_entities/ lun 05 jun 2023 10:00:41 CEST Run tests 0 to 0 0001/16: entity_location_shape_changes.test ........................................................................... 05 seconds 0002/16: errors_general.test .......................................................................................... 04 seconds 0003/16: errors_geobox.test ........................................................................................... 04 seconds 0004/16: errors_geoline.test .......................................................................................... 05 seconds 0005/16: errors_geopoint.test ......................................................................................... 03 seconds 0006/16: errors_geopolygon.test ....................................................................................... 04 seconds 0007/16: geoquery_box_geobox.test ..................................................................................... 03 seconds 0008/16: geoquery_box_geobox_after_update.test ........................................................................ 03 seconds 0009/16: geoquery_box_geoline.test .................................................................................... 03 seconds 0010/16: geoquery_box_geoline_after_update.test ....................................................................... 03 seconds 0011/16: geoquery_box_geopolygon.test ................................................................................. 02 seconds 0012/16: geoquery_box_geopolygon_after_update.test .................................................................... 03 seconds 0013/16: geoquery_circle_geopoint.test ................................................................................ 05 seconds 0014/16: geoquery_circle_geopoint_after_update.test ................................................................... 04 seconds 0015/16: geoquery_polygon_geopoint.test ............................................................................... 06 seconds 0016/16: geoquery_polygon_geopoint_after_update.test .................................................................. 06 seconds Total test time: 62.86 seconds $ ./testHarness.sh cases/1177_geometry_and_coords/ lun 05 jun 2023 10:02:29 CEST Run tests 0 to 0 0001/2: geometry_and_coords.test ..................................................................................... 03 seconds 0002/2: geometry_and_coords_errors.test .............................................................................. 03 seconds Total test time: 6.20 seconds

fgalan · 2023-05-16T16:20:15Z

src/lib/serviceRoutines/postQueryContext.cpp

+  if ((ciP->apiVersion == V2) && (ciP->uriParamOptions["count"]))
+  {
+    char cV[32];
+
+    snprintf(cV, sizeof(cV), "%llu", *countP);
+
+    ciP->httpHeader.push_back(HTTP_FIWARE_TOTAL_COUNT);
+    ciP->httpHeaderValue.push_back(cV);
+  }


It seems this block has been moved, from old L526 to this point. Could you elaborate on which that movement is needed?

Yes, we have moved the block, because at old location i.e. at L#526, ciP->httpHeader vector were holding the count of only CB entities.
Now in new location i.e. at L#858, ciP->httpHeader vector is holding count of all the entities including CB+CP. Because Total count of all the entities (CP+CB) is available only after queryForward() function call i.e. at L#785.

Thanks for the clarification. I'd suggest to add an explanatory comment about it

NTC (with regards to this comment thread)

fgalan · 2023-05-16T16:21:13Z

src/lib/serviceRoutines/postQueryContext.cpp

+* And we can get the actualTotalCount value that includes total Entities of CP+CB.
+*
+*/
+void getProviderCount (std::string cpResponse, long long*  totalCount)


Style

Suggested change

void getProviderCount (std::string cpResponse, long long* totalCount)

static void getProviderCount(std::string cpResponse, long long* totalCount)

Btw, this function is somehow a "hack" to get the value of a given header but maybe a more general solution would be to process all the headers in the response (in a std:map, for instance) so we can easily access to the fiware-total-count header just doing something like reponseHeaders["fiware-total-count"].

In the same sense we have a function to get the response payload (I think is jsonPayloadClean()) we should have some responseHeaders() function for that. However, this can be deferred to a next PR, no need to include that in this (except if you think is easy and you want to do it :)

We are investigating on this comment. As per your suggestion we will raise a separate PR for this comment.

After the merge of PR #4321, you probably would need to change the order of some headres in the .test expectations.

Fixed in 1e47b07

NTC (beyond the fix in 1e47b07)

fgalan · 2023-05-16T16:27:05Z

Please include an entry in CHANGES_NEXT_RELEASE file briefly describing (a line is enough) the functionality implemented in this PR, citing the corresponding issues (#4149 and maybe others, such as #847).

fgalan · 2023-05-16T16:27:53Z

After the merge of PR #4321, you probably would need to change the order of some headres in the .test expectations.

ArqamFarooqui110719 · 2023-05-19T12:16:30Z

Please include an entry in CHANGES_NEXT_RELEASE file briefly describing (a line is enough) the functionality implemented in this PR, citing the corresponding issues (#4149 and maybe others, such as #847).

Is it good for CNR entry?

- Fix: Pagination functionality in case of request forwarding (#4149, #847)

fgalan · 2023-05-19T13:20:16Z

As alternative suggestion:

Fix: pagination support in request query forwarding to Context Providers (Request to implement pagination for request forwarding. #4149, Pagination trouble with CPr responses #847)

ArqamFarooqui110719 · 2023-05-24T14:01:18Z

[1e47b07](/telefonicaid/fiware-orion/pull/4275/commits/1e47b077ce86f15d09438331b764710751bf1910)

Fixed in 1e47b07

fgalan · 2023-05-25T07:12:15Z

CHANGES_NEXT_RELEASE

@@ -13,3 +13,4 @@
 - Fix: MQTT disconnection when MQTT publish fail (so re-connection is forced at next MQTT notification)
 - Fix: lineMaxSize and infoPayloadMaxSize in the log admin REST API (#3707)
 - Fix: check for service database name too long at cache sync logic (#2848)
+- Fix: pagination support in request query forwarding to Context Providers (#4149, #847)


@mapedraza reviewed time ago all the issues related with CPrs and forwarding. It would be great to have his feedback, just in case some other issue should be mentioned here.

Added a new issue ( #4337 ) containing the list of all issues related with CPrs. Adding this issue to the CNR should help as @fgalan mentioned above

Suggested change

- Fix: pagination support in request query forwarding to Context Providers (#4149, #847)

- Fix: pagination support in request query forwarding to Context Providers (#4149, #847, #4337)

Fixed in c7e89b0

fgalan · 2023-05-25T07:17:02Z

src/lib/serviceRoutines/postQueryContext.cpp

+*
+* getProviderCount is created for extrating the 'Fiware-Total-Count' header value so that we can add the same into total count,
+* And we can get the actualTotalCount value that includes total Entities of CP+CB.
+*


I'd suggest to describe the potential improvement in the logic we have discussed previously

Suggested change

*

*

* FIXME: this functions relies in text-based processing of the HTTP response stream (which includes headers).

* It would be smarted to parse all the response headers in the response (for instance in a std::map) in the part

* of the code dealing with response processing, so in this part we can just access to 'fiware-total-count' easility

* using something like reponseHeaders["fiware-total-count"]

Fixed in c7e89b0

fgalan · 2023-05-25T07:34:22Z

src/lib/serviceRoutines/postQueryContext.cpp

@@ -781,6 +854,16 @@ std::string postQueryContext
    }
  }

+  if ((ciP->apiVersion == V2) && (ciP->uriParamOptions["count"]))


Based in our previous discussion, I'd suggest to add an explanatory comment here:

Suggested change

if ((ciP->apiVersion == V2) && (ciP->uriParamOptions["count"]))

// Before implementing pagination for CPrs, this block of code was part of step 02.

// However, in that step we only have the count for CB entities. It is in the new location

// (after queryForward() invocation) when we have the total count CB+CPr

if ((ciP->apiVersion == V2) && (ciP->uriParamOptions["count"]))

Fixed in c7e89b0

fgalan · 2023-05-25T07:37:49Z

I have provide a new round of feedback after your recent changes. The PR is getting better :)

Most of my comment are minor, but this one is specially remarkable.

fgalan

LGTM to merge in prelanding branch

fgalan · 2023-05-25T16:33:22Z

Continuation PR #4341

ArqamFarooqui110719 added 2 commits February 10, 2023 15:40

Fix for issue4149 in case of requestForwarding

d38c523

updated code to fix the failed existing FTs of v1 APIs

cf74832

updated the code for failed FTs

acbad2a

ArqamFarooqui110719 mentioned this pull request Mar 21, 2023

Request to implement pagination for request forwarding. #4149

Closed

fgalan reviewed May 10, 2023

View reviewed changes

fgalan reviewed May 16, 2023

View reviewed changes

Merge branch 'telefonicaid:master' into issue/4149

5edbb8e

ArqamFarooqui110719 and others added 2 commits May 24, 2023 15:06

Merge branch 'telefonicaid:master' into issue/4149

9c85125

Modified as per comment

1e47b07

fgalan reviewed May 25, 2023

View reviewed changes

ArqamFarooqui110719 added 2 commits May 25, 2023 15:47

Merge branch 'telefonicaid:master' into issue/4149

9219a6c

Added comment as per suggestion

c7e89b0

fgalan mentioned this pull request May 25, 2023

Fix: timeout field in subscriptions for mqtt and mqttCustom cases #4285 #4313

Closed

fgalan changed the base branch from master to issue-4149-prelanding May 25, 2023 16:29

fgalan approved these changes May 25, 2023

View reviewed changes

fgalan merged commit d66d090 into telefonicaid:issue-4149-prelanding May 25, 2023
12 checks passed

fgalan mentioned this pull request May 25, 2023

Fix for issue4149 in case of requestForwarding (prelanding branch) #4341

Merged

3 tasks

Anjali-NEC mentioned this pull request May 31, 2023

Change in getProviderCount function from comment https://github.com/telefonicaid/fiware-orion/pull/4275#discussion_r1195415590 #4348

Open

fgalan mentioned this pull request Jun 5, 2023

Fix for issue4149 in case of requestForwarding (final fixes) #4352

Merged

	POST http://localhost:REGEX(\d+)/v2/op/query?limit=20&offset=0&options=count
	POST http://localhost:REGEX(\d+)/v2/op/query

	# 28. GET /v2/entities with limit=5 and check no limit forwarded to CP
	# 28. GET /v2/entities with limit=3 and check no limit forwarded to CP




		echo "28. GET /v2/entities with limit<5 and check no limit forwarded to CP"

	echo "28. GET /v2/entities with limit<5 and check no limit forwarded to CP"
	echo "28. GET /v2/entities with limit=3 and check no limit forwarded to CP"




		echo "29. GET /v2/entities with limit=5 and check no limit forwarded to CP"




		echo "32. GET /v2/entities with offset<5 and check no offset is forwarded to CP"

		int providerLimit = DEFAULT_PAGINATION_LIMIT_INT;
		int providerOffset = DEFAULT_PAGINATION_OFFSET_INT;

	void getProviderCount (std::string cpResponse, long long* totalCount)
	static void getProviderCount(std::string cpResponse, long long* totalCount)

	- Fix: pagination support in request query forwarding to Context Providers (#4149, #847)
	- Fix: pagination support in request query forwarding to Context Providers (#4149, #847, #4337)

-*
+*
+* FIXME: this functions relies in text-based processing of the HTTP response stream (which includes headers).
+* It would be smarted to parse all the response headers in the response (for instance in a std::map) in the part
+* of the code dealing with response processing, so in this part we can just access to 'fiware-total-count'  easility
+* using something like reponseHeaders["fiware-total-count"]

-  if ((ciP->apiVersion == V2) && (ciP->uriParamOptions["count"]))
+  // Before implementing pagination for CPrs, this block of code was part of step 02.
+  // However, in that step we only have the count for CB entities. It is in the new location
+  // (after queryForward() invocation) when we have the total count CB+CPr
+  if ((ciP->apiVersion == V2) && (ciP->uriParamOptions["count"]))

Fix for issue4149 in case of requestForwarding #4275

Fix for issue4149 in case of requestForwarding #4275

Conversation

ArqamFarooqui110719 commented Feb 10, 2023

ArqamFarooqui110719 commented Feb 14, 2023

ArqamFarooqui110719 commented Feb 20, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ArqamFarooqui110719 commented May 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fgalan commented May 16, 2023

fgalan commented May 16, 2023

ArqamFarooqui110719 commented May 19, 2023

fgalan commented May 19, 2023

ArqamFarooqui110719 commented May 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fgalan May 25, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fgalan commented May 25, 2023

fgalan left a comment

Choose a reason for hiding this comment

fgalan commented May 25, 2023

ArqamFarooqui110719 commented Feb 20, 2023 •

edited

fgalan May 25, 2023 •

edited