Fix return servicePath in the entity response

[chetan-NEC]

Fix #2877

• Fix return servicePath in the entity response

[chetan-NEC]

@fgalan, Please review the PR and let me know if you have any suggestions.

[fgalan]

Thank you for the PR.

However, I think this is already covered with existing functionality. Let me elaborate:

• In the case of asynchronous context consumption, using notifications, NGSI payload patching can be used to include an attribute (with whichever name the user wants) with the service path information. For instance (from https://github.com/telefonicaid/fiware-orion/blob/master/test/functionalTest/cases/4085_custom_notifications_ngsi_payload/custom_notification_http_ngsi_special_macros.test):

        "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.

What do you think?

[chetan-NEC]

I agree with you. However, I am not aware of that because I am new to orion activity. I'll take the necessary precautions to avoid making the same mistakes in the future.

[fgalan]

Don't worry, it's not your fault :) It's ours, as we have should have detected this and close parent issue before you start working it...

Thus, do you agree in closing this PR?

[chetan-NEC]

Yes, I agree.

[fgalan]

Thank you for your understanding!

Issue #2877 has been also closed.

[fgalan]

Taking into account the feedback from @kzangeli at #2877 (comment), we are reopening this PR.

@chetan-NEC please do .test based on the case @kzangeli is proposing:

• Usage of multiple elements in fiware-servicepath (e.g. fiware-servicepath: /A, /B, /C)
• Usage of wildcards in fiware-servicepath (e.g. fiware-servicepath: /#)

Thanks!

[chetan-NEC]

As @kzangeli recommended, I am going to look into it.

[chetan-NEC]

@fgalan, please provide me some advice on how to use wildcards and multiple elements in the fiware-servicepath.

[fgalan]

@fgalan, please provide me some advice on how to use wildcards and multiple elements in the fiware-servicepath.

The functionally is described here: https://github.com/telefonicaid/fiware-orion/blob/master/doc/manuals/orion-api.md#service-path

Not sure if this answer your question, as it is very broad... In that case, please come back with more specific questions.

[kzangeli]

@fgalan, please provide me some advice on how to use wildcards and multiple elements in the fiware-servicepath.

If I remember correctly, an NGSIv2 entity has only one service path, and this issue is about returning it, right?
Why/where would you need to specify more than one? (that's for queries, not for reponses - if I'm right).

The service path will be (I assume) a field in the json response payload body, per entity, right next to entity id/type

[chetan-NEC]

@fgalan, please provide me some advice on how to use wildcards and multiple elements in the fiware-servicepath.

If I remember correctly, an NGSIv2 entity has only one service path, and this issue is about returning it, right? Why/where would you need to specify more than one? (that's for queries, not for reponses - if I'm right).

The service path will be (I assume) a field in the json response payload body, per entity, right next to entity id/type

Yes, you're correct. The service path should be returned in the entity response, according to this issue. and in this PR, I've added this functionality.

[fgalan]

Yes, you're correct. The service path should be returned in the entity response, according to this issue. and in this PR, I've added this functionality.

You should add more .test to cover the wildcards and list cases. For instance:

1. Create E in SP /Madrid/A
2. Create E in SP /Madrid/B
3. Create E in SP /Barcelona/C
4. Create E in SP /Barcelona/D
5. GET with fiware-service-path: /#
6. GET with fiware-service-path: /Madrid/#
7. GET with fiware-service-path: /Madrid/A,/Barcelona/C
8. GET with fiware-service-path: /Madrid/A,/Barcelona/#
9. GET with fiware-service-path: /Madrid/#,/Barcelona/#

[fgalan]

Btw, conflict in CHANGES_NEXT_RELEASE file (trivial to solve) needs to be addressed.

[chetan-NEC]

Yes, you're correct. The service path should be returned in the entity response, according to this issue. and in this PR, I've added this functionality.

You should add more .test to cover the wildcards and list cases. For instance:

1. Create E in SP /Madrid/A
2. Create E in SP /Madrid/B
3. Create E in SP /Barcelona/C
4. Create E in SP /Barcelona/D
5. GET with fiware-service-path: /#
6. GET with fiware-service-path: /Madrid/#
7. GET with fiware-service-path: /Madrid/A,/Barcelona/C
8. GET with fiware-service-path: /Madrid/A,/Barcelona/#
9. GET with fiware-service-path: /Madrid/#,/Barcelona/#

Thank you for the suggestion.

[chetan-NEC]

@fgalan, In this PR, I've added test cases as you suggested. Please review the PR and let me know if you have any suggestions.

[fgalan]

PR looks fine but the conflict in CHANGES_NEXT_RELEASE has to be solved. At the present moment, this is the only line it should appear in that file:

- Add: servicePath field to builtin attributes, so return ServicePath in the entity response (#2877)

In addition, I'd suggest to add some test testing the new servicePath built-in attribute in notifictions, using notification.attrs as [ "*", "servicePath" ].

[chetan-NEC]

@fgalan, I have updated this PR with an additional test case, as you suggested.

[chetan-NEC]

Hello, @fgalan I would like to request you that let's merge the current PR and open a new issue for this suggested functionality, as q and orderBy arguments are new functionalities for the servicePath built-in attribute. I feel this suggestion implementation would take little more effort.

[fgalan]

Builtin attributes should behave as regular attributes and, in this sense, they should properly support q and orderBy except in justified cases.

Although we could "skip" that support in this PR, that would leave an awful "technical debt" after it (that should be documented, which is also an extra work). Thus, I think it worths the effort to complete the implementation of q and orderBy (specially with the hints that I have provided, which should ease the work).

[chetan-NEC]

Thank you for your suggestion. I am looking into this thing, as you suggested.

[chetan-NEC]

@fgalan, please review the PR and let me know if you have any suggestions.

[fgalan]

LGTM. Thanks for the contribution!