Feature/947 v2 entities #958
Conversation
| @@ -158,10 +158,12 @@ if (error EQUAL 0) | |||
| ADD_SUBDIRECTORY(src/lib/orionTypes) | |||
There was a problem hiding this comment.
CHANGES_NEXT_RELEASE entry is missing.
|
New functest (pagination for GET /v2/entities) in commit 7bb8611 |
| @@ -1 +1,2 @@ | |||
| Fix: Fixed a bug about pagination problem due to missing ErrorCode with details in the case of queries with forwarding to context providers. (Issue #945) | |||
| Add: New convop "GET /v2/entities" (Issue #947) | |||
There was a problem hiding this comment.
It was my understanding that in v2 we are no longer using the term convop, aren't we?
There was a problem hiding this comment.
Good point... I'd suggest to be neutral and just say:
Add: New operation "GET /v2/entities" (Issue #947)
There was a problem hiding this comment.
ok, I wasn't aware of that ...
| // /v2 conv-ops | ||
| // | ||
| #define ENT EntitiesRequest | ||
| #define ENT_COMPS_V2 2, { "v2", "entities" } |
There was a problem hiding this comment.
probably we can define a constant for 'entities' and use it as well in the entryPoints operation as well, WDYT?
There was a problem hiding this comment.
What do we win with a change like that?
[ Except that the code gets more difficult to read ... ]
| if (strstr(details.c_str(), "\"") != NULL) | ||
| { | ||
| int len = details.length() * 2; | ||
| char* s2 = (char*) calloc(1, len + 1); |
There was a problem hiding this comment.
I understand that we are using s2 here because s is already taken... However, I don't find any s...
There was a problem hiding this comment.
Can't say I remember exactly why I chose that name.
s2 is twice as long as the original string, that is probably the reason.
This is a copy from REALLY old stuff.
There was a problem hiding this comment.
's2' changed to 's' in 5c5e28d
| { | ||
| if (qcrsP->errorCode.code == SccContextElementNotFound) | ||
| { | ||
| errorCode.fill(SccOk); |
There was a problem hiding this comment.
I undertand that this "transformation" from SccContextElementNotFound to SccOk is related with the GET /v2/entities operation returns an empty arrayt [] in this case and not an error. I'd suggest to add a comment about it... it could be complex to understand in the future.
| else if (qcrsP->errorCode.code != SccOk) | ||
| { | ||
| // | ||
| // If any other error - use the error for the response |
There was a problem hiding this comment.
I would add also something like this:
"Taking into account that this way of working avoids that the errorCode generated by API v1 logic for count be included in the response to the user of the API v2 operation"
There was a problem hiding this comment.
I don't know where exactly this piece of comment fits in ...
The phrase is true, but ... where do I put it?
Perhaps in the "Non-existing else" ...
that would capture (qcrsP->errorCode.code == SccOk) ...
Is that what was meant for this comment ?
|
congrats @kzangeli good work! |
|
LGTM |
Post release fixes
Implements #947
This PR implements most if not all of "GET /v2/entities", including the addition of two new directories/libraries for types and service routines for version 2.
One almost complete functional test is present, one more is no its way (test for local pagination).