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
211 stars 265 forks source link

More granular subscriptions modes (create only, update only, delete only and combinations) #1494

Closed fgalan closed 2 years ago

fgalan commented 9 years ago

NGSIv1 uses ONCHANGE notifications, which triggers each time a new context element (entrity + attribute) is created and/or updated. Thus, it is not granular enough to difference between create and update and it is not able to trigger notifications upon context element deletion.

NGISIv2 should improve this and include the following modes:

In addition, notifications should include some way of specifying if the context element was created, updated or deleted.

The condition field in NGSIv2 subscriptions should be reviewed in order to see how it can be adapted to these behaviours.

(Mabye there are duplicate issues about ONCREATE or ONDELETION semantics...)

tobiasjacobs commented 9 years ago

Not sure if we talk about the same thing, but

Anyway, I think what you propose is to define different subscription modes in order to

fgalan commented 8 years ago

A way of specifying to which case (update, create, delete) a given attribute in the notification corresponds (see actionType metadata introduced in https://github.com/telefonicaid/fiware-orion/pull/2518). However, the way of setting to which evetns the subscription has to react has to be defined.

It is a just a matter on how to "code" that configuration in the subscripton payload.

jmcanterafonseca commented 8 years ago

We need to distinguish between actions referring to entities and actions referring to attributes.

I guess that if attrs is not defined or empty we will be referring to the whole entity and if not we will be referring to a particular attribute or attributes.

{
  "subject": {
    "actionType": "update"  /* ("delete", "append", "create", "change") */
  }
}

The difference between update and change would be that change will imply a change in the attribute value whereas update will imply an update of the element which could result in a attribute value change or not.

For backwards compatibility, no actionType will mean "change".

What do you think?

fgalan commented 8 years ago

Note from https://github.com/telefonicaid/fiware-orion/pull/2518#discussion_r80059510, to take into account in the context of this issue:

what should happen if the attribute was not formerly defined. I think in that case this metadata should not appear. But we need to specify it.

fgalan commented 8 years ago

Functional specification

Based on the @jmcanterafonseca idea, but with some changes:

[ "change", "append" ]

(triggering attributes = the ones in conditions.attrs)

The .apib file needs to be modified accordingly to describe the new fields.

fgalan commented 8 years ago

Implementation notes:

Have a loook to the skip field in ContextAttribute class and how it is used to mark an attribute as deleted in the entity memory image that is keep during the update process. It may be useful to detect which attributes were deleted so a notification has to be triggered in that case of actionType includes delete.

static void deleteAttrInNotifyCer
(
  ContextElementResponse* notifyCerP,
  ContextAttribute*       targetAttr
)
{  
  for (unsigned int ix = 0; ix < notifyCerP->contextElement.contextAttributeVector.size(); ix++)
  {
    ContextAttribute* caP = notifyCerP->contextElement.contextAttributeVector[ix];
    if (caP->name == targetAttr->name)
    {
      caP->skip = true;
    }
  }
}
fgalan commented 6 years ago

Out of the scope of NGSIv2, althouth it is a good idea that could (potentially) be included in a later version of the API (v2.1) in a backward compatible way.

CC: @jmcanterafonseca

fgalan commented 5 years ago

The following post in SOF should be edited upon issue completion: https://stackoverflow.com/questions/54220061/orion-subscriptions-do-not-detect-the-removal-of-entities

fjperalta commented 4 years ago

Currently, each notification includes the attributes id and type by default. It would be interesting to add a new internal attribute entitystatus with the values created, updated or deleted.

About https://github.com/telefonicaid/fiware-cygnus/issues/1415, Cygnus could save this value to a new field named entitystatus at the destination table in order to identify if a given entity has been created, updated or deleted.

fgalan commented 2 years ago

New functional specification:

[ "entityChange", "entityCreate" ]

In the future maybe we could consider attribute-based changeTypes (as the ones previously proposed) but, by the moment, the above ones cover the basic use cases.

fgalan commented 2 years ago

Currently, each notification includes the attributes id and type by default. It would be interesting to add a new internal attribute entitystatus with the values created, updated or deleted.

About telefonicaid/fiware-cygnus#1415, Cygnus could save this value to a new field named entitystatus at the destination table in order to identify if a given entity has been created, updated or deleted.

Agreed. We will use entityStatus (right camelCase ;) virtual attribute for this. It will behave as dateModified, etc.: by default hidden, but included if explicitly included in notification.attrs.

fgalan commented 2 years ago

With regards to entityDelete notifications, there are two options:

  1. Send an empty notification, i.e. just id and type (and eventually entityStatus) but no attribute
  2. Send a notification including the entity content just before being deleted

In principle we are going to take option 2

fgalan commented 2 years ago

changeType -> operations (better name)

fgalan commented 2 years ago

PR https://github.com/telefonicaid/fiware-orion/pull/4084

mapedraza commented 2 years ago

@kzangeli As discussed in the previous TSC, this is the Issue for implementing subscriptions notifications. It would be interesting to be aligned with further implementations in NGSI-LD. Take into account that this new parameter is going to be included under subject.condition. Since NGSI-LD renamed and moved to an upper level on the JSON object, the field included in subject.condition, you probably need to define it at the top level of the entity.

You can have a look to the tests implemented in PR #4084 to have a clear idea about how is expected to work.

kzangeli commented 2 years ago

Yes! We haven't discussed this in ETSI ISG CIM yet, but this here would be my personal initial proposal:

typedef enum OrionldAlterationType
{
  EntityCreated,
  EntityDeleted,
  EntityModified,  // Any of the Attribute alterations imply EntityModified
  AttributeAdded,
  AttributeDeleted,
  AttributeValueChanged,
  AttributeMetadataChanged,
  AttributeModifiedAtChanged  // No change at all, except, the modifiedAt timestamp is always updated
} OrionldAlterationType;
fgalan commented 2 years ago

alterationType sounds good. We are going to use that at the end instead of operations

fgalan commented 2 years ago

We are going to split the implementation in two PRs:

  1. PR https://github.com/telefonicaid/fiware-orion/pull/4084 with alterationType basic functionality
  2. A second PR, with entityStatus/triggeredBy/notifTriggeredBy/etc. built-in attribute implementation

Note however that the tests included in PR #4084 already include entityStatus in subscription creation, so they will be reused for step 2.

fgalan commented 2 years ago

A second PR, with entityStatus/triggeredBy/notifTriggeredBy/etc. built-in attribute implementation

Taking into account this rename maybe the best name for the new built-in attribute would be alterationType.

fgalan commented 2 years ago

PR https://github.com/telefonicaid/fiware-orion/pull/4091 completes this feature

djs0109 commented 2 years ago

Hello, this new feature is really helpful for my use case, where my IoT "sensor" sometimes sends measurements with the unchanged values, but I still want to trigger the notification in this case. I found that entityUpdate does not work as I expected.

My subscription:

"subject": {
            "entities": [
                {
                    "id": "urn:ngsi-ld:Sensor:001",
                    "type": "Sensor"
                }
            ],
            "condition": {
                "attrs": [
                    "temperature"
                ],
                "alterationTypes": [
                    "entityUpdate"
                ]
            }
        },
"notification": {
            "attrs": [
                "alterationType",
                "temperature"
            ],
            ...

The problem I found is that, the subscription can be triggered by a "PUT" request without actual change but not a "PATCH" request. However, IoTagent seems always send a "PATCH" request.

fgalan commented 2 years ago

The problem I found is that, the subscription can be triggered by a "PUT" request without actual change but not a "PATCH" request. However, IoTagent seems always send a "PATCH" request.

Could you provide examples of the PUT and PATCH requests (verb + URL + payload) do you refer so I can test, please?

djs0109 commented 2 years ago

Yes sure. You can test with the following 4 steps.

  1. Create entity
    
    POST

http://{{orion}}/v2/entities/

payload = { "id":"urn:ngsi-ld:Sensor:001", "type":"Sensor", "temperature":{"type":"Number", "value":50} }


2. Create subscription

POST

http://{{orion}}/v2/subscriptions/

payload= { "description": "QuantumLeap Subscription", "subject": { "entities": [ { "id": "urn:ngsi-ld:Sensor:001", "type": "Sensor" } ], "condition": { "attrs": ["temperature"], "alterationTypes": ["entityUpdate"] } }, "notification": { "attrs": ["alterationType", "temperature"], "onlyChangedAttrs": true, "attrsFormat": "normalized", "http": { "url": "http://quantumleap:8668/v2/notify" }, "metadata": [ "dateCreated", "dateModified", "TimeInstant", "timestamp" ] }, "throttling": 0 }


3. Update `temperature` using `PUT` (notify)

PUT

http://{{orion}}/v2/entities/urn:ngsi-ld:Sensor:001/attrs/

payload = { "temperature":{ "value": 50, "type": "Number" } }


4. Update `temperature` using `PATCH` (not notify)

PATCH

http://{{orion}}/v2/entities/urn:ngsi-ld:Sensor:001/attrs/

payload { "temperature":{ "value": 50, "type": "Number" } }

fgalan commented 2 years ago

I have modeled a new .test based in your case (see PR https://github.com/telefonicaid/fiware-orion/pull/4172) and it seems it is working... In my case I'm getting 2 notifications.

Note subscription is not exactly as yours, I have simplified removing some fields. I propose a kind of "mutual cross-check":

fgalan commented 2 years ago

Additional question, pls: which Orion version are you using? I mean, the result of GET /version

djs0109 commented 2 years ago

Additional question, pls: which Orion version are you using? I mean, the result of GET /version

It is a 3.7.0 image, which I pull from here. For clarity here is the whole response:

{
    "orion": {
        "version": "3.7.0",
        "uptime": "0 d, 2 h, 12 m, 33 s",
        "git_hash": "8b19705a8ec645ba1452cb97847a5615f0b2d3ca",
        "compile_time": "Thu May 26 11:45:49 UTC 2022",
        "compiled_by": "root",
        "compiled_in": "025d96e1419a",
        "release_date": "Thu May 26 11:45:49 UTC 2022",
        "machine": "x86_64",
        "doc": "https://fiware-orion.rtfd.io/en/3.7.0/",
        "libversions": {
            "boost": "1_74",
            "libcurl": "libcurl/7.74.0 OpenSSL/1.1.1n zlib/1.2.11 brotli/1.0.9 libidn2/2.3.0 libpsl/0.21.0 (+libidn2/2.3.0) libssh2/1.9.0 nghttp2/1.43.0 librtmp/2.3",
            "libmosquitto": "2.0.12",
            "libmicrohttpd": "0.9.70",
            "openssl": "1.1",
            "rapidjson": "1.1.0",
            "mongoc": "1.17.4",
            "bson": "1.17.4"
        }
    }
}
fgalan commented 2 years ago

I'll try to modify the subscription in my .test to get closer to yours

I have found that if I add "temperature" attribute to condition at subscription creation:

    "condition": {
      "attrs": ["temperature"],
      "alterationTypes": [ "entityUpdate" ]
    }

then both PUT and PATCH fail to generate notifications (which it is not exactly the same as you reported, as PUT is not working)

I'll have a look

fgalan commented 2 years ago

At the end I found a flaw in the implementation, fixing it in PR https://github.com/telefonicaid/fiware-orion/pull/4172 (now merged in master):

@djs0109 could you download image telefonicaiot/fiware-orion:latest and check if it works, please?

djs0109 commented 2 years ago

At the end I found a flaw in the implementation, fixing it in PR #4172 (now merged in master):

@djs0109 could you download image telefonicaiot/fiware-orion:latest and check if it works, please?

It is still not working with the example above. But when I set the "onlyChangedAttrs" to false, both PATCH and PUT can work properly.

fgalan commented 2 years ago

It is still not working with the example above. But when I set the "onlyChangedAttrs" to false, both PATCH and PUT can work properly.

I can confirm "onlyChangedAttrs" set to true makes notifications are not sent (in my case not sent in both PATCH and PUT cases). I'll have a look.

fgalan commented 2 years ago

Looking into this, I think is correct behaviour.

On the one hand, with regards to entityUpdate alteration type (https://fiware-orion.readthedocs.io/en/master/user/subscriptions_alttype.html):

entityUpdate: notification is sent whenever a entity covered by the subscription is updated (no matter if the entity actually changed or not)

So, according to this, the notification would be triggered in both PUT and PATCH cases in your use case.

However, at the same time, with regards to onlyChangedAttrs (https://fiware-orion.readthedocs.io/en/master/user/ngsiv2_implementation_notes.html#notify-only-attributes-that-change)

If set to true then notifications associated to the subscription include only attributes that changed in the triggering update request, in combination with the attrs or exceptAttrs field.

Thus, given the attribute temperature has not actually changed, it will not be included in the notification payload.

Thus, the notification should be triggered, but, at the same time it shouldn't include any attribute. In this situation (a notification but without actual content) Orion doesn't send anything (which I think makes sense: it is a waste of resources to send just an "envelope" without any data).

In your use case, you have only one attribute in the notification attribute list (apart from alterationType, which is a built-in)

        "attrs": ["alterationType", "temperature"],

thus I think onlyChangedAttrs: true doesn't have too much sense in this case (as you always want temperature to be included in your notifications, no matter if it changed or not). Or maybe I'm missing something... (in that case, please elaborate on why do you need onlyChangedAttrs: true).

djs0109 commented 2 years ago

@fgalan thank you for the explanation! I agree with you. In this regard the behavior definitely makes sense. In my use case for example, there is a room entity, which has temperature, humidity, co2 concentration, etc. as attributes. It should receive measurements from the sensors and I want to save these measurements to CrateDB in a reasonable manner, i.e. to save and only to save the actual signals.

Therefore, I first need the entityUpdate, so that a measurement will be saved although the measured value does not change (e.g. because It is damaged or has reached its limit).

And then I also need the onlyChangedAttrs (or maybe onlyUpdatedAttrs would be more reasonable). Otherwise dummy measurements will be saved to CrateDB, or I have to create multiple subscriptions for one room entity.

It would be very helpful, if this behavior is possible.

fgalan commented 2 years ago

Not sure of understanding your use case...

So, you have a given entity Room1 with several attributes temperature, humidity, co2 concentration, etc. Let's assume at a given moment the temperature is 24. Then, CB received update Room1 temperature to 24 (i.e. same value). What do you expect?

  1. Notification with temperature = 24 (and no other attribute)?
  2. Notification with temperature = 24 along with humidity, co2, etc. with the values they have at that moment at CB?
  3. No notification at all?
djs0109 commented 2 years ago

In this case, I will expect the CB to only notify temperature = 24. And if 5 minutes later, the temperature sensor sends another measurement with the same value. I will still expect the CB to only notify temperature = 24.

So, all and only the real measurements are saved into time series.

fgalan commented 2 years ago

In this case, I will expect the CB to only notify temperature = 24. And if 5 minutes later, the temperature sensor sends another measurement with the same value. I will still expect the CB to only notify temperature = 24.

Under my understanding, you can achieve this use case without using onlyChangedAttrs: true. If I'm wrong, could you elaborate why onlyChangedAttrs: true is needed (maybe with a particular example)?

djs0109 commented 2 years ago

Yes, I still need onlyChangedAttrs: true. Let's have a look. As you suggested, I create a CB subscription like this:

    "subject": {
        "entities": [
            {
                "id": "urn:ngsi-ld:Sensor:001",
                "type": "Sensor"
            }
        ],
        "condition": {
            "attrs": ["humidity", "temperature", "co2"],
            "alterationTypes": ["entityUpdate"]
        }
    },
    "notification": {
        "attrs": ["alterationType", "humidity", "temperature", "co2"],
        "onlyChangedAttrs": false,
    }

Let's assume that, at this moment, the room entity has the following status: humidity=40, temperature=24, co2=100. Now if the temperature sensor sends a new measurement, temperature=24, the CB will notify the time series DB with humidity=40, temperature=24, co2=100. However, humidity and co2 are actually not measured at this time point. Therefore, CB has notified dummy (or fake) data to the time series DB. My expected behavior is, that CB should only notify with temperature=24.

fgalan commented 2 years ago

What about using covered feature? I mean with:

    "notification": {
        "attrs": ["alterationType", "humidity", "temperature", "co2"],
        "covered": true,
        "onlyChangedAttrs": false,
    }

In the same situation you describe, CB will notify with humidity=null, temperature=24, co2=null, clearly stating which attributes has been included in the triggering update and which attribute haven't.

djs0109 commented 2 years ago

Unfortunately, it does not work for me.

I then also look into the CB docs, and found that the covered flag seems to be responsible for the attributes that not exist https://fiware-orion.readthedocs.io/en/master/user/ngsiv2_implementation_notes.html#covered-subscriptions. But maybe this is not the correct documentation for it.

fgalan commented 2 years ago

Not sure what do you mean by "attributes that not exist". Could you elaborate a bit more on why do you think this solution is not valid for your case?

djs0109 commented 2 years ago

That is the description in the docs. I have tested with "covered": true, however, CB still notify with humidity=40, temperature=24, co2=100 but not humidity=null, temperature=24, co2=null.

fgalan commented 2 years ago

You are right. I missed that covered notifications take attributes from the entity not from the update triggering notification.

What you are proposing is indeed a new feature. I have created a new fresh issue about it: https://github.com/telefonicaid/fiware-orion/issues/4202. Please, comment on that issue if the description I've provided there is not correct.