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

option to return ServicePath in entity response #2877

Closed
cdupont opened this issue Apr 6, 2017 · 15 comments
Closed

option to return ServicePath in entity response #2877

cdupont opened this issue Apr 6, 2017 · 15 comments
Labels
Milestone

Comments

@cdupont
Copy link

cdupont commented Apr 6, 2017

Hi,
this is a feature request to add an option to return the servicePath in the entity response.
For example:

$ curl http://localhost:1026/v2/entities?options=servicePath

would return:

[
  {
    "id": "Room1",
    "type": "Room",
    "servicePath": "/MY/APP"
  }
]

Including the service path in the response would allow the user to perform actions on the returned entities.

@kzangeli
Copy link
Member

kzangeli commented Apr 6, 2017

This would definitely be a nice feature.
+1

@fgalan fgalan added the backlog label Apr 7, 2017
@fgalan
Copy link
Member

fgalan commented Apr 7, 2017

Thanks for the contribution. However, I'd suggest to implement it as a virtual attribute, i.e.

$ curl http://localhost:1026/v2/entities?attrs=servicePath

or (if you want the servicePath, along any other attribute, following the general rules for virtual attributes):

$ curl http://localhost:1026/v2/entities?attrs=*,servicePath

@cdupont
Copy link
Author

cdupont commented Apr 7, 2017

@fgalan yes, that would be perfect

@cdupont
Copy link
Author

cdupont commented Apr 11, 2017

Hi, I tried to implement it, but I have a small problem.
In the function Entity::render, the field servicePath is empty:
https://github.com/telefonicaid/fiware-orion/blob/master/src/lib/apiTypesV2/Entity.cpp#L73

When I read it, the service path is the empty string.

@cdupont cdupont closed this as completed Apr 11, 2017
@cdupont cdupont reopened this Apr 11, 2017
@fgalan
Copy link
Member

fgalan commented Apr 11, 2017

The servicePath field in the Entity class is not actually used. You can check it commenting it out in Entity.h and noticing that the code can be built without problems. Probably is a leftover of a time in which it was actually used for something.

Anyway, the servicePath virtual attribute should be implemented in a similar way other virtual attributes. Have a look to how DATE_CREATED (corresponding to dateCreated virtual attribute) is implemented, paying attention not only how this attribute is added to the response, but also that it may be used for sorting and filtering logic.

@cdupont
Copy link
Author

cdupont commented Apr 11, 2017

@fgalan for me it's not clear where I should get the service path from. In the database it's at the same level than id and type.
Should I reactivate this Entity.h field?

@fgalan
Copy link
Member

fgalan commented Apr 11, 2017

I don't think so. Probably the best approach is to modify the ContextElementResponse constructor.

ContextElementResponse::ContextElementResponse
(
  const mongo::BSONObj&  entityDoc,
  const AttributeList&   attrL,
  bool                   includeEmpty,
  bool                   includeCreDate,
  bool                   includeModDate,
  const std::string&     apiVersion
)

This constructor uses a BSON that comes from DB (in the entityDoc variable) where you could take the servicePath. As I mention before, look to how the DATE_CREATED stuff is implemented.

Probably you would need to add a bool includeServicePath. In fact, I don't like too much the current approach... it will work with a third includeAnotherVirtualAttr but probably it should be generalized with a vector of bools in the future (we can take that as technical debt).

@cdupont
Copy link
Author

cdupont commented Apr 13, 2017

@fgalan I don't see the booleans in the current constructor:
https://github.com/telefonicaid/fiware-orion/blob/master/src/lib/ngsi/ContextElementResponse.cpp#L132

Also, the servicePath is already decoded from the BSON as far as I can see...

@cdupont
Copy link
Author

cdupont commented Apr 13, 2017

I did a pull request here:
#2880
Still to do:

  • update to tests (I don't know how they work),
  • update to filters/sorting,
  • update to doc.

Unfortunately in the following weeks, I will have no time...
Anyway it was great to make a small contribution :)

@fgalan
Copy link
Member

fgalan commented Apr 18, 2017

@fgalan I don't see the booleans in the current constructor

You are right... my feedback was based in an old version of the code (corresponding to release/1.4.0 branch, https://github.com/telefonicaid/fiware-orion/blob/release/1.4.0/src/lib/ngsi/ContextElementResponse.cpp#L133) but the modern version in master simplified the mechanism, so no such booleans are used in the constructor signature. Sorry for the confussion...

Side-note (as this is not directly related with the service path stuff, but with hwo creation/modification date virtual attributes work): with the current mechanism, the mongoBackend sets the BSON virtual attributes with creation and modification dates at BSON building time. Note also that the processing done on ContextElementResponse to set creDate/modDate in the contained EntityId object is done to make these fields available to the matching logic used to check filters upon subscription notifications.

@fgalan
Copy link
Member

fgalan commented Apr 18, 2017

Also, the servicePath is already decoded from the BSON as far as I can see...

I understand you refer to:

contextElement.entityId.servicePath = id.hasField(ENT_SERVICE_PATH) ? getStringFieldF(id, ENT_SERVICE_PATH) : "";

That's correct. However, not that that servicePath field in the EntityId is used only for the notification case, in order to render the rigth Fiware-ServicePath header to be included in the notification.

@fgalan
Copy link
Member

fgalan commented May 18, 2023

At the end, and taken into account the functionality available in Orion 3.8.0, this issue is not actually needed, as it is covered by
existing functionality. Let me elaborate:

        "originalService": {
          "value": "${service}",
          "type": "Text"
        },
  • In the case of synchronous context consumption (e.g. GET /v2/entities) the user specifies the service path in the request (in particular in the fiware-servicepath header). Thus, including it in the response is redundant.

Thus, the issue is going to be closed.

If due to whaever reason we need to re-open it, note the following PR (authored by @chetan-NEC ) proposes a solution and can be useful in that case.

@fgalan fgalan closed this as completed May 18, 2023
@kzangeli
Copy link
Member

kzangeli commented May 18, 2023

Is it not true that you can give an array of service-paths in a GET /v2/entities request?
No way to know which of them were a match for which of the returned entities.
(or even "/#" that would match everything ...)

@fgalan
Copy link
Member

fgalan commented May 23, 2023

Is it not true that you can give an array of service-paths in a GET /v2/entities request?
No way to know which of them were a match for which of the returned entities.
(or even "/#" that would match everything ...)

Your are right (that's not a primary use case, so I didn't think on it in the first time ;)

@chetan-NEC we are going to reopen issue and PR. Sorry for the inconveniences...

@fgalan fgalan reopened this May 23, 2023
@fgalan fgalan added this to the 3.11.0 milestone Aug 9, 2023
@fgalan
Copy link
Member

fgalan commented Aug 9, 2023

PR #4329

@fgalan fgalan closed this as completed Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants