Fix using limit=0 and details=on to get only the count of elements #1492

[Anjali-NEC]

Fix issue #1492

[fgalan]

This PR should include .test files to cover the cases described in #1492 (comment)

In addition, some existing .test are failing. They has to be reviewed and fixed.

[Anjali-NEC]

This PR should include .test files to cover the cases described in #1492 (comment)

In addition, some existing .test are failing. They has to be reviewed and fixed.

Added new .test file and fixed existing .test are which are failing.

[fgalan]

Reviwing this PR I have realized that in line L161

        OrionError error(SccBadRequest, std::string("Bad pagination limit: /") + value + "/ [must be a decimal number]");

should be changed to a more precise message:

        OrionError error(SccBadRequest, std::string("Bad pagination limit: /") + value + "/ [must be a positive integer number]");

Could you do that change in the PR, please?

[fgalan]

"JSON Parse Error" doesn't seem to be a right message in this case, as actually the request in step 3 doesn't have any payload...

In fact, a OK response seems to be better.

[Anjali-NEC]

"JSON Parse Error" doesn't seem to be a right message in this case, as actually the request in step 3 doesn't have any payload...

In fact, a OK response seems to be better.

In step 3 everything is OK but it shows "JSON Parse Error". I didn't find reason for this error. Could you please suggest what modification should be done in this test case?

[fgalan]

I understand that the response in this case should be the same you get when do a /v1/queryContext operation without any matching result. You can test yourself using this curl:

curl -vvv localhost:1026/v1/queryContext -H 'content-type: application/json' -d @- <<EOF
{
    "entities": [
        {
            "type": "foo",
            "isPattern": "false",
            "id": "bar"
        }
    ]
}
EOF

You should get something like this:

< HTTP/1.1 200 OK
< Connection: Keep-Alive
< Content-Length: 70
< Content-Type: application/json
< Fiware-Correlator: aaee0e26-4621-11ec-9ae3-000c29583ca5
< Date: Mon, 15 Nov 2021 14:38:15 GMT
< 
* Connection #0 to host localhost left intact
{"errorCode":{"code":"404","reasonPhrase":"No context element found"}}

The expectation in the step should resemble that. The reason behind the "JSON Parser Error" in this case should be analyzed.

[Anjali-NEC]

Reviwing this PR I have realized that in line L161

        OrionError error(SccBadRequest, std::string("Bad pagination limit: /") + value + "/ [must be a decimal number]");

should be changed to a more precise message:

        OrionError error(SccBadRequest, std::string("Bad pagination limit: /") + value + "/ [must be a positive integer number]");

Could you do that change in the PR, please?

Fix in 8879a39

[fgalan]

Note that the PR is not passing code style check at the present point. It seems the problems is:

...
0002 errors of category 'Weird number of spaces at line-start'
=================================================================
...
src/lib/serviceRoutinesV2/getEntities.cpp:308:  Weird number of spaces at line-start.  Are you using a 2-space indent?  [whitespace/indent] [3]
src/lib/serviceRoutinesV2/getEntities.cpp:309:  Weird number of spaces at line-start.  Are you using a 2-space indent?  [whitespace/indent] [3]
...
Total errors found: 17

[Anjali-NEC]

Note that the PR is not passing code style check at the present point. It seems the problems is:

...
0002 errors of category 'Weird number of spaces at line-start'
=================================================================
...
src/lib/serviceRoutinesV2/getEntities.cpp:308:  Weird number of spaces at line-start.  Are you using a 2-space indent?  [whitespace/indent] [3]
src/lib/serviceRoutinesV2/getEntities.cpp:309:  Weird number of spaces at line-start.  Are you using a 2-space indent?  [whitespace/indent] [3]
...
Total errors found: 17

Fixed in 64cf793

[fgalan]

@Anjali-NEC taking into account the upcoming FIWARE TSC on Orion, could you provide an status update of this PR (or be prepared to attend to the TSC and report at the meeting), please? Thanks!

[Anjali-NEC]

Please, find a new round of comments provided. Basically, I think you should deal the check for limit 0 at mongoBackend layer and not add serviceRoutine layer. Maybe after fixing that also the V1 ParseError issue gets also solved.
I have done some code changes at mongoBackend layer to check for limit=0. it works in case of GET subscription and registration but in case of GET entities and types it doesn't show the expected response.
In case of GET v2/entities with limit=0 it show Fiware-Total-Count: 0 and in case of GET /v2/types the response is empty payload instead of []. I have tried to fix this but I haven't found suitable solution for this.
Can you please suggest what change need to be done for getting expected response.

@fgalan Gentle Reminder!!!

[fgalan]

I have done a new round of review. The PR looks better than last time. Congrats :)

However, there are new comments that need to be addressed. The main one is this: #3994 (comment), but the other are also important, related with indent, unnecessary whitelines, etc.

[fgalan]

In adddition, the coflict in CHANGES_NEXT_RELEASE should be solved, so functional tests get another pass.

[Anjali-NEC]

I have done a new round of review. The PR looks better than last time. Congrats :)

However, there are new comments that need to be addressed. The main one is this: #3994 (comment), but the other are also important, related with indent, unnecessary whitelines, etc.

The two test cases are still failing:

----------- Failing tests ------------------
  o  1492_limit_zero/limit_zero_entities.test
  o  1492_limit_zero/limit_zero_types.test

[fgalan]

I have done a new round of review. The PR looks better than last time. Congrats :)
However, there are new comments that need to be addressed. The main one is this: #3994 (comment), but the other are also important, related with indent, unnecessary whitelines, etc.

The two test cases are still failing:

----------- Failing tests ------------------
  o  1492_limit_zero/limit_zero_entities.test
  o  1492_limit_zero/limit_zero_types.test

I have had a look to the limit_zero_entities.test and limit_zero_types.test and they seems to be correct, so it seems there is some fail in your implementation. Could you review it and fix, please?

[Anjali-NEC]

I have had a look to the limit_zero_entities.test and limit_zero_types.test and they seems to be correct, so it seems there is some fail in your implementation. Could you review it and fix, please?

Hi @fgalan Sir,

Now all test cases are pass in my latest commit ff2b225. Please review it and if it is ok please merge it into master.
Thanks

[Anjali-NEC]

@fgalan Gentle Reminder!!

[Anjali-NEC]

Hi @fgalan Sir,
Please review my PR and if it is ok please merge it into master.
Thanks

[fgalan]

Work is not finished, but let's merge in pre-landing branch (issue1492-prelanding) to get easy further analysis and modifications.

[fgalan]

Work is not finished, but let's merge in pre-landing branch (issue1492-prelanding) to get easy further analysis and modifications.

Continues in PR #4104