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

Covered notifications in subscritpions #3693

Closed
fgalan opened this issue Sep 10, 2020 · 10 comments
Closed

Covered notifications in subscritpions #3693

fgalan opened this issue Sep 10, 2020 · 10 comments
Assignees
Labels
APIv2.1 To be included in a potential NGSI v2.1 API version backlog help wanted
Milestone

Comments

@fgalan
Copy link
Member

fgalan commented Sep 10, 2020

Currently, Orion notifications only include attributes that the entity actually has. For instance, if the subscription has notification.attrs: [A, B, C] but the entity only has A and B, C is not included in the notification.

That's fine with most of the notification receiver. However, some receivers may benefit from notifications with always the same set of attributes, no matter which attributes has actually the entity or not.

Thus, the issue is about including a new subscription parameter notification.covered (*) of boolean type.

  • If this attribute is false Orion will work as now.
  • If this attribute is true then Orion will complete the notification with any missing attribute in the notification. These attributes will have value null and type "None".

The default value in case of omitting the field is false to ensure backward compatibilty.

(*) covered in the sense the notification will cover completely the attributes in notification.attrs. However, suggestion for the name are welcome :)

@fgalan fgalan added backlog help wanted APIv2.1 To be included in a potential NGSI v2.1 API version labels Sep 10, 2020
@rajeswar-NEC
Copy link
Contributor

Hi @fgalan Sir,

I would like to work on this issue. Please assign this issue to me.

@rajeswar-NEC
Copy link
Contributor

Hi @fgalan Sir,

As per my investigation, in this issue I’ve to add a new parameter field “covered” in subscription payload within notification as mentioned below with bolean type.
As per this issue, if "notification.covered" field will be set to “true” then Orion will complete the notification with any missing attribute in the notification, These attributes will have value “null” and type "None".
If "notification.covered" field will be set to “false” Orion work as now. By default, this field will be set to “false”.
...
"notification": {
"attrs": [
"temperature"
],
“covered”: false/true
}
...

First we have to correctly parse the new parameter in JSON request/response for subscription management operations of the REST API. For this a new parameter should be added to the notification class.
The new parameter at JSON API level should be parsed correctly and stored correctly in the DB. A new parameter name should be defined for that in dbConstants in subscription creation and update operations.

@fgalan
Copy link
Member Author

fgalan commented Aug 26, 2021

@rajeswar-NEC thank you for your willingness to work in this issue! You have been assigned.

I have read your understanding of the issue above and I think is correct.

@Anjali-NEC
Copy link
Contributor

Hi @fgalan sir,

Due to medical reason @rajeswar-NEC is not able to work on this issue.

@Anjali-NEC
Copy link
Contributor

Hi @fgalan sir,

I would like to work on this issue, please assign this issue to me.

Thanks

@fgalan
Copy link
Member Author

fgalan commented Feb 9, 2022

Thank you for your willingness to work on this issue! You have been assigned :)

@Anjali-NEC
Copy link
Contributor

Hi @fgalan Sir,

I have divided this issue into three parts:

  1. Add new bool filed covered in subscription and parse the new filed in JSON request/response for subscription. Define the new filed in dbConstants. Store and get the value of new filed covered from Database. set the default value of covered : false

  2. Add check for if (covered == true) then, the notification includes all the attributes which are passed in notification.attrs.

  3. Add the value null and type "None" for attributes which are not existing in entity.

Currently I have done first part of this issue and raised PR #4068 for the same.

As per my understanding the code for getting attributes in notification are written in src/lib/ngsi/ContextAttributeVector.cpp in function get line no. 308. I have also tried to add logic in this function but I am getting only those attributes which are exist in entity. Please suggest me how we can include notification.attrs in notification response.

I have also tried to fixed this issue at mongoBackend and for this I have tried to add the logic in src/lib/mongoBackend/MongoCommonUpdate.cpp in function processOnChangeConditionForUpdateContext line no 1755 but I didn't get the expected response.

Could you please guide me which is the suitable place for fixing this issue.
Thanks

@fgalan
Copy link
Member Author

fgalan commented Feb 21, 2022

During the update logic at some point the code grabs matching subscriptions in either addTriggeredSubscriptions_withCache() or addTriggeredSubscriptions_noCache() depending the CB working mode. The grabbed subscriptions are stored in a map structure using TriggeredSubscription objects.

Check if TriggeredSubscription already has the information you need about the notification attributes (maybe the attrsL field? not sure right now...). Then, use that information at processSubcription() function.

@Anjali-NEC
Copy link
Contributor

During the update logic at some point the code grabs matching subscriptions in either addTriggeredSubscriptions_withCache() or addTriggeredSubscriptions_noCache() depending the CB working mode. The grabbed subscriptions are stored in a map structure using TriggeredSubscription objects.

Check if TriggeredSubscription already has the information you need about the notification attributes (maybe the attrsL field? not sure right now...). Then, use that information at processSubcription() function.

Hi @fgalan sir,

Fixed issue in my latest commit. Please review my PR and if it is ok then please merge it into master. Thanks

fgalan added a commit that referenced this issue Mar 21, 2022
Fix: Covered notifications in subscritpions #3693
@fgalan fgalan added this to the 3.7.0 milestone Mar 21, 2022
@fgalan
Copy link
Member Author

fgalan commented Mar 21, 2022

PR #4068 (to prelanding branch) and #4087 (finishing the former one after completing some pending items)

@fgalan fgalan closed this as completed Apr 13, 2022
mapedraza added a commit that referenced this issue Apr 18, 2022
…ications_prelanding

Fix: Covered notifications in subscritpions #3693 (prelanding)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APIv2.1 To be included in a potential NGSI v2.1 API version backlog help wanted
Projects
None yet
Development

No branches or pull requests

3 participants