telefonicaid / fiware-orion

Context Broker and CEF building block for context data management, providing NGSI interfaces.
https://fiware-orion.rtfd.io/
GNU Affero General Public License v3.0
212 stars 264 forks source link

Avoid triggering notification if metadata changes but not the attribute value itself #3727

Closed fgalan closed 1 year ago

fgalan commented 3 years ago

Currently, the metadata of an attribute is considered part of its value. Thus, a subscription using a given attribute as a triggering condition will trigger notifications in cases in which the attribute value doesn't change but metadata does. This is precluding some use cases.

Thus, in order to increase the flexibility of the subscription, this case should be allowed. The simpler approach seems to use a global general setting in the subscription condition (i.e. "metadataAreNotPartOfValue": false :) so notifications are sent only if the value of the attribute itself changes.

Although maybe is a too "coarse-grained" solution and other approaches are possible...

Anjali-NEC commented 2 years ago

Hi @fgalan Sir,

Please confirm this issue need to be fixed? if yes then I will try to analyze and start working on it.

kzangeli commented 2 years ago

"notifyOnMetadataChange" might be a better name. And, seems like a good idea, if you ask me :)

fgalan commented 2 years ago

Please confirm this issue need to be fixed? if yes then I will try to analyze and start working on it.

Yes, as far as I understand, this issue is still valid. Thanks for your willingness to work on it!

With regards to the name of the field, notifyOnMetadataChange sounds better than my original proposal, although by the moment the actual name it's not important (we can change it easily during the PR process with a search-and-replace on the code if we find a better one).

Anjali-NEC commented 2 years ago

@fgalan @kzangeli,

As per my understanding we need to add an optional bool field notifyOnMetadataChange in the subscription payload. when "notifyOnMetadataChange": false no notifications are sent if the value of metadata changes, notifications are sent only if the value of the attribute itself changes. similarly in case of "notifyOnMetadataChange": true notifications are sent in both the cases (i.e changes in value of attribute or changes in metadata)

Please confirm my understanding and assign this issue to me. Thanks

kzangeli commented 2 years ago

Sounds good to me.

What should be the default value for "notifyOnMetadataChange" ? Preferably what's done right now, which I guess is to not notify (@fgalan knows) ?

fgalan commented 2 years ago

@Anjali-NEC analysis sounds also good to me.

Regarding the default for notifyOnMetadataChange I agree it should be the one that match with current behaviour to avoid backward compatibility problems.

Anjali-NEC commented 2 years ago

@fgalan sir,

I have added bool field notifyOnMetadataChange in subscription payload, Now I want to use this field where the metadata updated. As per my analysis the code for metadata update is written in src/lib/mongoBackend/MongoCommonUpdate.cpp in mergeAttrInfo function L676 to L679 https://github.com/telefonicaid/fiware-orion/blob/master/src/lib/mongoBackend/MongoCommonUpdate.cpp#L676 I want to add logic in this function like this :

      if (notifyOnMetadataChange == false)
      {
         actualUpdate = (attrValueChanges(attr, caP, forcedUpdate, apiVersion) ||
                        ((!caP->type.empty()) &&
                         (!attr.hasField(ENT_ATTRS_TYPE) || getStringFieldF(attr, ENT_ATTRS_TYPE) != caP->type)));
      }
      else
      {
         actualUpdate = (attrValueChanges(attr, caP, forcedUpdate, apiVersion) ||
                        ((!caP->type.empty()) &&
                         (!attr.hasField(ENT_ATTRS_TYPE) || getStringFieldF(attr, ENT_ATTRS_TYPE) != caP->type)) ||
                           mdNew.nFields() != mdSize || !equalMetadata(md, mdNew));
      }

But I have no idea how to get the value of notifyOnMetadataChange from database and use in if/else condition in this function. Please confirm my approach and suggest the right place for applying this condition. Thanks

fgalan commented 2 years ago

In my opinion, in that part of the code you could detect if the entity has changed its attributes or only has changed metadata. Maybe you could transform actualUpdate in an enum with three values: None, onlyMetadata, value.

Next, you should manage to pass back that information and use it to filter out subscriptions that doesn't match the criteria. You can do it in one of these two points:

This section in the development manual may be useful to get an idea of the overall update entity program flow.

I hope this comment helps you in the implementation of this feature.

Anjali-NEC commented 2 years ago

Hi @fgalan sir, Currently I am working on another issues So, I am putting this issue on hold. I will work on this issue in future and will try to analyzing this issue.

chandradeep11 commented 1 year ago

@fgalan We apology for putting the issue on hold for long period. @Anjali-NEC was working on other high priority issue. Currently she is on holiday due to her personal reasons. Now we have planned to resume working on this ticket in end of February.

chetan-NEC commented 1 year ago

Hi @fgalan, I'm working on it, and soon I will upload a diff.

fgalan commented 1 year ago

Hi @fgalan, I'm working on it, and soon I will upload a diff.

Great! Thanks for your contribution! :)

chetan-NEC commented 1 year ago

Hi @fgalan, It's a strange thought, in my opinion. When I only change an entity's metadata, the attribute value is set to "null," and the value type is set to "None." as I've seen in master branch code. Do you think this is correct? Could you please provide your suggestions?

Below is a detailed explanation.

Step 1: Entity Creation

curl localhost:1026/v2/entities -s -S --header 'Content-Type: application/json' -d @- <<EOF
{
  "id": "Room1",
  "type": "Room",
  "temperature": {
    "value": 26.5,
    "type": "Float",
    "metadata": {
      "accuracy": {
        "value": 0.8,
        "type": "Float"
      }
    }
  },
  "pressure": {
    "value": 720,
    "type": "Integer",
    "metadata": {
      "accuracy": {
        "value": 0.9,
        "type": "Float"
      }
    }
  }
}
EOF

Step 2: Update Entity

curl localhost:1026/v2/entities/Room1/attrs -s -S -H 'Content-Type: application/json' -X PATCH -d @- <<EOF
{
  "temperature": {
    "metadata": {
            "accuracy": {
                "type": "Float",
                "value": 9.77
            }
       }
    }
}
EOF

Step 3: The updated Room1 entity output is shown below curl localhost:1026/v2/entities/Room1 -s -S -H 'Accept: application/json' | python -mjson.tool

{
    "id": "Room1",
    "type": "Room",
    "temperature": {
        "type": "None",
        "value": null,
        "metadata": {
            "accuracy": {
                "type": "Float",
                "value": 9.77
            }
        }
    },
    "pressure": {
        "type": "Integer",
        "value": 720,
        "metadata": {
            "accuracy": {
                "type": "Float",
                "value": 0.9
            }
        }
    }
}
fgalan commented 1 year ago

With regards to step 2:

curl localhost:1026/v2/entities/Room1/attrs -s -S -H 'Content-Type: application/json' -X PATCH -d @- <<EOF
{
  "temperature": {
    "metadata": {
            "accuracy": {
                "type": "Float",
                "value": 9.77
            }
       }
    }
}
EOF

Take into account what it said in this section of the documentation about Partial Representation (i.e. when some of the elements in the request is missing) https://github.com/telefonicaid/fiware-orion/blob/master/doc/manuals/orion-api.md#partial-representations:

Attribute/metadata value may be omitted in requests, meaning that the attribute/metadata has null value.

That means an implicit "value": null in your request.

In addition, it is said:

Attribute/metadata type may be omitted in requests. When omitted in attribute/metadata creation or in update operations, a default is used for the type depending on the value:

  • ...
  • If value is null, then None is used.

That means an implicit "type": "None" in your request.

Thus, step 2 request is equivalent to:

curl localhost:1026/v2/entities/Room1/attrs -s -S -H 'Content-Type: application/json' -X PATCH -d @- <<EOF
{
  "temperature": {
    "value": null,
    "type": "None",
    "metadata": {
            "accuracy": {
                "type": "Float",
                "value": 9.77
            }
       }
    }
}
EOF

Which is consistent with result in step 3.

Note that at the present moment, NGSIv2 API doesn't include operations to manage metadata (i.e. adding a metadata without touching the attribute value). There is an issue about it (https://github.com/telefonicaid/fiware-orion/issues/2567) that is open to contributions.

fgalan commented 1 year ago

PR ~#4274~ ~#4300~ #4310