search

telefonicaid / fiware-orion

Context Broker and CEF building block for context data management, providing NGSI interfaces.
https://github.com/telefonicaid/fiware-orion/blob/master/doc/manuals/orion-api.md
GNU Affero General Public License v3.0
210 stars 265 forks source link

Custom notification: sending 0-length request #3272

Closed fgalan closed 3 years ago

fgalan commented 6 years ago

The current customized notification implementation allow to define a custom payload (if "payload" field is provided with a non-empty value) or use the default one for NGSIv2 notifications (if "payload" field is ommited or it has "" value).

However, current implement lacks the posibility of not including payload at all, i.e. 0-length requests. You can use "payload": " " which produces a 1-byte empty payload, but it couldn't fit some server implementations.

This issue is about implementing that possiblity. The following syntax could be used:

"payload": null
kzangeli commented 6 years ago

Not sure who could want a notification without information. You can always remove the subscription ... Has a user asked for this functionality?

fgalan commented 6 years ago

The information could be in other parts of the request, like URL query parameters or headers. In fact, methods such as GET or DELETE (allowed in the current implementation of cutomized notification) don't typically used any payload.

The issue is motivated by a use case, although it has been workarrounded, so it doesn't have priority. @manucarrace and/or @mrutid could elaborate on the use case motivating this, if needed.

kzangeli commented 6 years ago

Nah, no need. I just found it a little surprising :)

Anjali-NEC commented 4 years ago

Hi @kzangeli @fgalan sir

I have done investigation on this issue. following are the two concern:

  1. As the type of payload is defined as string object so the NULL value can not assign to it. so we cannot replace "" (empty string) to NULL. In C++ std::string object should always initialized and always contains a string; its contents by default are an empty string ("").

  2. If I declare payload as pointer in apiTypesV2/HttpInfo.h file and modified the method for payload in such a way so that it can handle the pointer . But due to this 7 to 8 test cases are getting failed.

Please let me know if you have any other opinion.

fgalan commented 4 years ago

HttpInfo.h class has a payload field of std::string. However, this field is not expressive enough to be used to model the "no payload situation" needed by this issue.

I'd suggest to add a new field bool includePayload to that class for that. That field will be set to true at parsing stage (in the case of finding "payload": null in the request) and will be used at notification creating time to avoid including the payload.

Thanks for you help on this issue!

Anjali-NEC commented 4 years ago

Hi @fgalan sir,

I've fixed the issue. Please check PR #3683 and if it is ok then please merge the PR in master branch.

Anjali-NEC commented 3 years ago

Hi @fgalan Sir, Currently I have not found any solution to fix this issue PR #3683, So I am leaving this issue.

fgalan commented 3 years ago

I'll try to take your PR https://github.com/telefonicaid/fiware-orion/pull/3683 ("duplicating" it to a new PR so I can add new commits) and try to finish it, so your valuable work so far doesn't get wasted :)

fgalan commented 3 years ago

Implemented in PR #3850