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

Notifications sent on identical data when JSON attribute values are used #4211

Closed
WillFreitag opened this issue Sep 7, 2022 · 29 comments
Closed

Comments

@WillFreitag
Copy link

Bug description
Orion sends a notifier, even if no attribute is changed.

  • Orion version: Same behavior/tested with 2.4.2, 3.2.0 & 3.7.0
  • MongoDB version: 3.6
  • Env variables or CLI parameters: Orion is running with -noCache
  • Docker Image used: fiware/orion:2.4.2/3.2.0/3.7.0

How to reproduce it
Steps to reproduce the behavior (as an example)

  1. On a blank System, create a subscription like:
    {
        "description": "My Subscription",
        "subject": {
            "entities": [
                {
                    "idPattern": ".*",
                    "type": "Vehicle",
                }
            ]
            ,
            "condition":{
                "attrs":["location"]
            }
        },
        "notification": {
            "http": {
                "url": "http://quantumleap/v2/notify"
            },
            "attrs": [
                "location"
            ],
            "onlyChangedAttrs": true
        }
    }
  2. Send entities to Orion like:
    {
        "id": "entity_id:1",
        "type": "Vehicle",
        "name": {
            "type": "Text",
            "value": "Something"
        },
        "location": {
            "type": "geo:json",
            "value": { 
                "type": "Point",
                "coordinates": [10.0, 10.0]
            
            }
        }
    }
  3. The notification is sent each and every time, even if the location is not changed.
  4. This ends up in a database (Postgres behind QuantumLeap) having duplicate data (no changes in location).

Expected behavior
The notification is only fired on attribute change.

Additional information

@WillFreitag WillFreitag added the bug label Sep 7, 2022
@fgalan
Copy link
Member

fgalan commented Sep 7, 2022

Thank you for your issue report! Pretty clear and straightforward :)

We have recently solved a problem that sounds similar to the one you describe.

- Fix: conditions.alterationTypes not working properly when conditions.attributes is used in entityUpdate case (#1494, reopened)

However, that fix has not been yet released (it is planned for Orion 3.8.0, which is the next version)

Could you test with telefonicaiot/fiware-orion:latest docker image and tell us if it works, please?

@WillFreitag
Copy link
Author

Great! I will test (tomorrow) and give feedback!

Thanks!

@WillFreitag
Copy link
Author

All the same :-(
Using telefonicaiot/fiware-orion:latest (Orion-Version "3.7.0-next") has the same result.
I had the idea, that location or doubles in general might be a problem (you know best, comparing doubles is not as easy as "val1 == val2" when it comes to check the condition for the notifier), so I changed the payload sending a simple string - did not help.
So the issue is still there.

@WillFreitag
Copy link
Author

For completeness, this is, what I'm currently trying:
Notification:

{
    "description": "MG",
    "subject": {
        "entities": [
            {
                "idPattern": ".*",
                "type": "Vehicle",
            }
        ]
        ,
        "condition":{
            "attrs":["test_attribute"]
        }
    },
    "notification": {
        "http": {
            "url": "http://quantumleap:8668/v2/notify"
        },
        "attrs": [
            "test_attribute"
        ],
        "onlyChangedAttrs": true
    }

And the payload, sent via "PUT" after a first "POST":

{
    "test_attribute": {
        "type": "Text",
        "value": "test"
    }
}

I will try some more things (e.g. using POST with upsert, switch from normailzed to keyValues) in the next days...

@kzangeli
Copy link
Member

kzangeli commented Sep 8, 2022

Comparing floating point values is a difficult task in a program.
CPU's are kind of made for integers ...

I ran into the same problem with Orion-LD and my idea of solving it (haven't implementing it yet) is to use a range for floating point comparisons and not an exact comparison.

Instead of comparing with EXACT numbers:

   (P1.x == 0.123), 

I'd do an AND:

  (P1.x >= 0.1229999) AND (P1.x <= 0.123001).

Then the question arrives, WHO exactly defines the precision?
Well, in the case of queries, it'd be a URI param (?precision=4 (decimals)), and for subscriptions, we'd add a field expressing the same.

@fgalan
Copy link
Member

fgalan commented Sep 8, 2022

All the same :-( Using telefonicaiot/fiware-orion:latest (Orion-Version "3.7.0-next") has the same result. I had the idea, that location or doubles in general might be a problem (you know best, comparing doubles is not as easy as "val1 == val2" when it comes to check the condition for the notifier), so I changed the payload sending a simple string - did not help. So the issue is still there.

Just to be sure... could you do a GET /version in Orion service port and post the result, pls?

@WillFreitag
Copy link
Author

@fgalan The output is:

{
"orion" : {
  "version" : "3.7.0-next",
  "uptime" : "0 d, 2 h, 18 m, 47 s",
  "git_hash" : "90d592b1293645f43cb12af49fe128c3e7aedb7a",
  "compile_time" : "Thu Sep 1 12:17:37 UTC 2022",
  "compiled_by" : "root",
  "compiled_in" : "d30c3c23186e",
  "release_date" : "Thu Sep 1 12:17:37 UTC 2022",
  "machine" : "x86_64",
  "doc" : "https://fiware-orion.rtfd.io/",
  "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"
  }
}
}

@kzangeli You're right and the "solution" (in C++) is EPSILON. But as I mentioned before...the problem here is not related to that thingy...

@WillFreitag
Copy link
Author

What I did before (I'm using a Node-RED flow), is sending the above payload to Orion Context Broker using the verb PUT (id and type in URL and not in payload) and on a 404 (entity not found - cannot update), I added id and type to the payload and sent it via POST. Next time, the same id can be updated via PUT.
Now, I switched over to POST always with options=upsert and the subscription works as expected (only fired, if data has changed) as long as the payload contains only "simple" types (strings, numbers...integer or double)!
As soon as I send location data, the notification is sent always...weird!
To be more precise:
works (notification only sent when value changes):

{
    "test_attribute": {
        "type": "Number",
        "value": 10.1
    }
}

does not work (always notifying - even on identical values):

{
    "test_attribute": {
        "type": "geo:json",
        "value": { 
            "type": "Point",
            "coordinates": [10.2, 10.0]
        
        }
    }
}

@fgalan
Copy link
Member

fgalan commented Sep 9, 2022

Maybe the problem is related which the usage of JSON values in attributes. By the above report, it seems it is not related with numbers exclusively.

Could you test the following case and report the result, please?

Case 1:

{
    "test_attribute": {
        "type": "noMatter",
        "value": { 
            "x": "a",
            "y": "b"
        }
    }
}

Case 2:

{
    "test_attribute": {
        "type": "noMatter",
        "value":  [ "x", "y" ]
    }
}

@WillFreitag
Copy link
Author

Here we go:
OKAY means: Subscription works as expected, only sent on changed data.
FAIL means: Subscription is firing always, even on identical data.


OKAY:
let simple = {
    "test_attribute": {
        "type": "Number",
        "value": 10.1
    },
};

FAIL:
let geo = {
    "test_attribute": {
        "type": "geo:json",
        "value": { 
            "type": "Point",
            "coordinates": [10.2, 10.0]
        
        }
    },
};

FAIL:
let object_ = {
    "test_attribute": {
        "type": "noMatter",
        "value": { 
            "x": "a",
            "y": "b"
        }
    },
};

FAIL:
let array_ = {
    "test_attribute": {
        "type": "noMatter",
        "value":  [ "x", "y" ]
    },
};

Conclusion:
You're totally right. This as nothing to do with "geo:json" or doubles or so, but with "complex" types (structures).

@kzangeli
Copy link
Member

kzangeli commented Sep 9, 2022

Not so sure Orion supports "check of value change for compound value". It's ... a bit complex.
Fermin - easy to check! (last time I was there, it wasn't implemented for compounds)

I implemented it myself for Orion-LD - by rendering the old vs rendering the new value, then just a simple strcmp() ...
That works just fine as long as the order of the fields doesn't change ... A bit useless to tell you the truth ...

To do it well, you'd need to order all fields of the compound values (old and new - recursively), and after that you can compare
rendered strings.

As I said, a bit complex

That said, float comparisons, if not done with a range (GE AND LE) will give problems as well, as I stated earlier.
Don't dismiss that.

@WillFreitag
Copy link
Author

Great "idea"!
About that ordering-thing: If you parse both JSONs into an object and flush those objects into JSON afterwards...should'nt both JSONs have the same order?

@kzangeli
Copy link
Member

kzangeli commented Sep 9, 2022

If the parser you use supports ordering of fields, then yes, of course.
Orion uses rapidjson - I'm not a fan and far from an expert ... :)

You still might have a problem with Arrays.
Can't really order the values inside an array, as the broker doesn't know what the user wants, how it semantically understands the order of the values in an array.

Anyway, best effort, right?

I mean:

[  "a", "b" ]

is not necessarily the same as:

[ "b", "a" ]

@WillFreitag
Copy link
Author

If Orion uses std::array there is "no problem" comparing two arrays for equality...

std::array<string, 2> as1 {"a", "b"};
std::array<string, 2> as2 {"b", "a"};

cout << "Arrays are equal? " << as1 == as2;

@kzangeli
Copy link
Member

kzangeli commented Sep 10, 2022

Had it been that simple ...
It might be an object containing an array, containing an object, containing an array ...
Any depth.

Either you compare the old tree with the new tree (recursively looking up members from tree 1 in tree 2, and then mark every field as "compared", afterwards make sure all fields were compared in both trees), or you sort and render and strcmp.
This is far from trivial, there's no easy fix.
That's why it wasn't implemented "back in the day",
Guess we had bigger fish to fry (I believe this may date back to 2016 or so).

It is possible to implement, though, I did it myself. It's just a bit of work ...

Array will always be a problem though.
For some implementations [ 1,2 ] === [ 2, 1 ], for other implementations that might be an important difference (e.g. "I have one sister and two brothers" vs "I have two sisters and one brother").
The broker is agnostic to this type of semantics.

We'd need an option somewhere for the comparison, sometyhing expression "pay attention to array order"

@fgalan
Copy link
Member

fgalan commented Sep 12, 2022

The solution to this issue would be to implement "deep compare" in attribute value JSON structures. Orion uses an internal way of representing JSON (CompoundValue) so some recursive function should be implemented to do so. The feedback in the previous comments can be useful to that end.

I'm changing the title to describe more precisely the issue.

(I think this is not the first time we find this and even there is an old issue about this. However, I haven't been able to find it among the open issues).

@fgalan fgalan changed the title Notifications sent on identical data Notifications sent on identical data when JSON attribute values are used Sep 12, 2022
@kzangeli
Copy link
Member

The solution to this issue would be to implement "deep compare"

Yes, correct.
That's much nicer than "sort/render/strcmp"

@WillFreitag
Copy link
Author

Just to have an answer to my client...Do you think, this will be implemented in 3.8.0? ...and further: is there a release date planned yet?
And again (already stated this in another issue): I'd really love to contribute but I don't find the time at all :-(

@fgalan
Copy link
Member

fgalan commented Sep 13, 2022

Just to have an answer to my client...Do you think, this will be implemented in 3.8.0? ...and further: is there a release date planned yet?

It would depend if (and when) some brave developer volunteers to implement it :) By the moment it is [not] ours immediate priorities. You can always use a side not-JSON attribute that changes at the same time than the JSON one (typically, a "TimeInstant" attribute of type DateTime with the timestamp of the change).

And again (already stated this in another issue): I'd really love to contribute but I don't find the time at all :-(

Maybe you can ask budget to your client so you can allocate resources to contribute with it. I understand your client could be an interested party in getting this functionality done ;)

@WillFreitag
Copy link
Author

Okay.
My current workaround is, that no objects/arrays are used at all (e.g. Geo:JSON is now split into two doubles lat/lng). Naturally, we are leaving Smart Data Model with this but it's a workaround only - okay with our customer.

About the contribution: That project is about evaluating the FIWARE stack -the customer wants to know, if it fits his needs - so no money for development here :-( ...

And as a reminder for the fix: Keep in mind, that the condition acts differently, depending on how the Orion Context Broker is used. PUT/POST or POST with upsert. (See my comment above)

@fgalan
Copy link
Member

fgalan commented Sep 14, 2022

Okay. My current workaround is, that no objects/arrays are used at all (e.g. Geo:JSON is now split into two doubles lat/lng). Naturally, we are leaving Smart Data Model with this but it's a workaround only - okay with our customer.

Note that if you split latitude and longitude into two different attributes instead of using GeoJSON, you will not be able to benefic from the geo-query features in Orion.

About the contribution: That project is about evaluating the FIWARE stack -the customer wants to know, if it fits his needs - so no money for development here :-( ...

Pitty :(

And as a reminder for the fix: Keep in mind, that the condition acts differently, depending on how the Orion Context Broker is used. PUT/POST or POST with upsert. (See my comment above)

Your comment will be taken into account when this issue gets addressed.

Thanks for all your valuable feedback!

@JeffersonLupinacci
Copy link

JeffersonLupinacci commented Jul 7, 2023

Hello everyone, any news about this problem with GEO:JSON? We are having a lot of problems with the new version of orion:3.7.0 and the coordinates.

Would it perhaps be possible to use checks from the turf library to validate that the geo:json's are different?

https://github.com/Turfjs/turf/tree/master/packages/turf-boolean-equal

https://libgeos.org/ - Symmetric Difference

Br,
Jefferson.

@fgalan
Copy link
Member

fgalan commented Jul 7, 2023

Hello everyone, any news about this problem with GEO:JSON? We are having a lot of problems with the new version of orion:3.7.0 and the coordinates.

Based on the previous analysis (see all the discussion above) it seems is not a problem with geo:json in particular, but with any JSON used as attribute value.

As I told before:

It would depend if (and when) some brave developer volunteers to implement it :)

Maybe you would like to implement the fix (given that is seems you have a strong use case needing it)? I'd be more than happy to review your pull request and provide feedback and guidance on it :)

Finally, you say "a lot or problems with the new version of orion:3.7.0 and the coordinates". Two comments:

  1. At the present moment, 3.7.0 is not "new version" :). I'd suggest to use the latests version available at the present moment (Orion 3.10.1)
  2. Could you detail the exact problems do you have with the coordinates, please? I'd like to know if all them are related with this issue or if there is something else.

Thanks for your feedback!

@JeffersonLupinacci
Copy link

Whenever I run the request to the informed endpoint, the entity is received by the subscription, regardless of having changes or not.

curl 'http://orion:1026/v2/op/update' -H 'Content-Type: application/json' -H 'Accept: application/json' -H 'fiware-service: orion' -H 'fiware-servicepath: /test' -d '{
    "actionType": "UPDATE",
    "entities": [
        {
            "id": "orion.0001",
            "type": "orion",
            "position": {
                "value": {
                    "type": "Point",
                    "coordinates": [
                        -77.02503667,
                        -12.131508679
                    ]
                },
                "type": "geo:json"
            },
            "typology": {
                "type": "string",
                "value": "String"
            },
            
        }
    ]
}'

In order not to receive notifications constantly, we are using coordinates in the geo:point format and this way we don't have the problem of receiving them all the time.

{
       "position": {
            "type": "geo:point",
            "value": "38.742375,-9.128965",
            "metadata": {}
        },
}

Sometimes we have much more complex coordinates like MultiPoint, MultiPolygon or MultiLineString

As our integrations with external services are designed to receive data via NIFI, we have not implemented a logic to validate field by field before sending it to the broker and in many cases the data does not change.

@fgalan
Copy link
Member

fgalan commented Nov 3, 2023

Related issue #4434 (similar problem in metadata)

@fgalan
Copy link
Member

fgalan commented Nov 8, 2023

PR #4444

@fgalan
Copy link
Member

fgalan commented Nov 8, 2023

(I think this is not the first time we find this and even there is an old issue about this. However, I haven't been able to find it among the open issues).

It was #643 :)

@fgalan
Copy link
Member

fgalan commented Nov 8, 2023

A fix for this problem has been provided (in master branch by the time being). The version of Orion available in telefonicaiot/fiware-orion:latest at dockerhub includes this fix.

@WillFreitag @JeffersonLupinacci it would be great if you could test with that version an provide feedback (either if the issue is solver or not), please. Thanks!

@fgalan
Copy link
Member

fgalan commented Jan 25, 2024

Orion 3.11.0 is about to be released. Thus, we are closing this issue.

Of course it could be reopened if @WillFreitag and/or @JeffersonLupinacci feedback comes in the future.

@fgalan fgalan closed this as completed Jan 25, 2024
@fgalan fgalan unpinned this issue Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants