From 1520c7e2541f9e7e6c8db27f55b37111680955d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ferm=C3=ADn=20Gal=C3=A1n=20M=C3=A1rquez?= Date: Wed, 8 Nov 2023 09:54:31 +0100 Subject: [PATCH] FIX correctly identify changes in JSON attribute values in subscription triggering logic --- src/lib/mongoBackend/MongoCommonUpdate.cpp | 71 +++---- src/lib/parse/CompoundValueNode.cpp | 18 +- ...notify_upon_actual_change_in_geo_json.test | 2 +- ...tify_upon_actual_change_in_json_value.test | 187 ++++++++++++++++-- 4 files changed, 229 insertions(+), 49 deletions(-) diff --git a/src/lib/mongoBackend/MongoCommonUpdate.cpp b/src/lib/mongoBackend/MongoCommonUpdate.cpp index e00218931b..544c94d3c1 100644 --- a/src/lib/mongoBackend/MongoCommonUpdate.cpp +++ b/src/lib/mongoBackend/MongoCommonUpdate.cpp @@ -529,48 +529,49 @@ static ChangeType mergeAttrInfo /* Was it an actual update? */ ChangeType changeType = NO_CHANGE; + /* We consider there is a change in the value if one or more of the following are true: + * + * 1) the value of the attribute changed (see attrValueChanges or CompoundValueNode::equal() for details) + * 2) the type of the attribute changed (in this case, !attr.hasField(ENT_ATTRS_TYPE) is needed, as attribute + * type is optional according to NGSI and the attribute may not have that field in the BSON) + * + * In addition, we consider there is change in the metadata if: + * + * 3) the metadata changed (this is done checking if the size of the original and final metadata vectors is + * different and, if they are of the same size, checking if the vectors are not equal) + */ + bool valueChanged; + bool typeChanged; + bool mdChanged; if (caP->compoundValueP == NULL) { - /* In the case of simple value, we consider there is a change in the value if one or more of the following are true: - * - * 1) the value of the attribute changed (see attrValueChanges for details) - * 2) the type of the attribute changed (in this case, !attr.hasField(ENT_ATTRS_TYPE) is needed, as attribute - * type is optional according to NGSI and the attribute may not have that field in the BSON) - * - * In addition, we consider there is change in the metadata if: - * - * 3) the metadata changed (this is done checking if the size of the original and final metadata vectors is - * different and, if they are of the same size, checking if the vectors are not equal) - */ - bool valueChanged = attrValueChanges(attr, caP, forcedUpdate, apiVersion) || - ((!caP->type.empty()) && (!attr.hasField(ENT_ATTRS_TYPE) || getStringFieldF(attr, ENT_ATTRS_TYPE) != caP->type) ); - bool mdChanged = (mdNew.nFields() != mdSize || !equalMetadata(md, mdNew)); - - if (valueChanged && !mdChanged) - { - changeType = CHANGE_ONLY_VALUE; - } - else if (!valueChanged && mdChanged) - { - changeType = CHANGE_ONLY_MD; - } - else if (valueChanged && mdChanged) - { - changeType = CHANGE_VALUE_AND_MD; - } - else // !valueChanged && !mdChanged - { - changeType = NO_CHANGE; - } + valueChanged = attrValueChanges(attr, caP, forcedUpdate, apiVersion); } else { - // FIXME #643 P6: in the case of compound value, it's more difficult to know if an attribute - // has really changed its value (many levels have to be traversed). Until we can develop the - // matching logic, we consider CHANGE_VALUE_AND_MD always. - // + valueChanged = !caP->compoundValueP->equal(getFieldF(attr, ENT_ATTRS_VALUE)); + } + typeChanged = ((!caP->type.empty()) && (!attr.hasField(ENT_ATTRS_TYPE) || getStringFieldF(attr, ENT_ATTRS_TYPE) != caP->type)); + mdChanged = (mdNew.nFields() != mdSize || !equalMetadata(md, mdNew)); + + valueChanged = valueChanged || typeChanged; + + if (valueChanged && !mdChanged) + { + changeType = CHANGE_ONLY_VALUE; + } + else if (!valueChanged && mdChanged) + { + changeType = CHANGE_ONLY_MD; + } + else if (valueChanged && mdChanged) + { changeType = CHANGE_VALUE_AND_MD; } + else // !valueChanged && !mdChanged + { + changeType = NO_CHANGE; + } /* 5. Add modification date (actual change only if actual update) */ if (changeType) diff --git a/src/lib/parse/CompoundValueNode.cpp b/src/lib/parse/CompoundValueNode.cpp index 2d19d474bd..b66f011951 100644 --- a/src/lib/parse/CompoundValueNode.cpp +++ b/src/lib/parse/CompoundValueNode.cpp @@ -532,6 +532,7 @@ bool CompoundValueNode::equal(const orion::BSONElement& be) { // Note object cannot be declared inside switch block std::vector ba; + orion::BSONObj bo; switch (valueType) { @@ -575,7 +576,22 @@ bool CompoundValueNode::equal(const orion::BSONElement& be) { return false; } - // TBD + bo = be.embeddedObject(); + if ((int) childV.size() != bo.nFields()) + { + return false; + } + for (unsigned int ix = 0; ix < childV.size(); ix++) + { + if (!bo.hasField(childV[ix]->name)) + { + return false; + } + if (!(childV[ix]->equal(getFieldF(bo, childV[ix]->name)))) + { + return false; + } + } return true; case orion::ValueTypeNotGiven: diff --git a/test/functionalTest/cases/4211_notify_upon_actual_change_in_json_value/notify_upon_actual_change_in_geo_json.test b/test/functionalTest/cases/4211_notify_upon_actual_change_in_json_value/notify_upon_actual_change_in_geo_json.test index 9822816391..cd9a0c0506 100644 --- a/test/functionalTest/cases/4211_notify_upon_actual_change_in_json_value/notify_upon_actual_change_in_geo_json.test +++ b/test/functionalTest/cases/4211_notify_upon_actual_change_in_json_value/notify_upon_actual_change_in_geo_json.test @@ -110,7 +110,7 @@ echo echo -echo "05. Dump accumulator, see 3 notifications" +echo "05. Dump accumulator, see 2 notifications" echo "=========================================" accumulatorDump echo diff --git a/test/functionalTest/cases/4211_notify_upon_actual_change_in_json_value/notify_upon_actual_change_in_json_value.test b/test/functionalTest/cases/4211_notify_upon_actual_change_in_json_value/notify_upon_actual_change_in_json_value.test index 9d74d7bf56..b50fa070b2 100644 --- a/test/functionalTest/cases/4211_notify_upon_actual_change_in_json_value/notify_upon_actual_change_in_json_value.test +++ b/test/functionalTest/cases/4211_notify_upon_actual_change_in_json_value/notify_upon_actual_change_in_json_value.test @@ -34,10 +34,12 @@ accumulatorStart --pretty-print # 01. Create subscription covering E-A # 02. Create E-A entity with JSON object (notification) # 03. Update E-A entity with same JSON object (no notification) -# 04. Update E-A entity with JSON array (notification) -# 05. Update E-A entity with same JSON array (no notification) -# 06. Update E-A entity with same string (notification) -# 07. Dump accumulator, see 3 notifications +# 04. Update E-A entity with different JSON object (notification) +# 05. Update E-A entity with JSON array (notification) +# 06. Update E-A entity with same JSON array (no notification) +# 07. Update E-A entity with different JSON array (notification) +# 08. Update E-A entity with same string (notification) +# 09. Dump accumulator, see 5 notifications # echo "01. Create subscription covering E-A" @@ -130,7 +132,40 @@ echo echo -echo "04. Update E-A entity with JSON array (notification)" +echo "04. Update E-A entity with different JSON object (notification)" +echo "===============================================================" +payload='{ + "A": { + "value": { + "text": "foo", + "number": 10, + "bool": true, + "null": null, + "array": [ + "22", + { + "x" : [ "x1", "x2" ], + "y" : 3 + }, + [ "z1", "z2" ] + ], + "object": { + "x": { + "x1": "a", + "x2": "b" + }, + "y": [ "y1", "y_new" ] + } + }, + "type": "Json" + } +}' +orionCurl --url /v2/entities/E/attrs --payload "$payload" +echo +echo + + +echo "05. Update E-A entity with JSON array (notification)" echo "====================================================" payload='{ "A": { @@ -150,7 +185,7 @@ echo echo -echo "05. Update E-A entity with same JSON array (no notification)" +echo "06. Update E-A entity with same JSON array (no notification)" echo "============================================================" payload='{ "A": { @@ -170,7 +205,27 @@ echo echo -echo "06. Update E-A entity with same string (notification)" +echo "07. Update E-A entity with different JSON array (notification)" +echo "==============================================================" +payload='{ + "A": { + "value": [ + "", + { + "x": [ "x1", "x_new" ], + "y": "3" + }, + [ "z1", "z2" ] + ], + "type": "Array" + } +}' +orionCurl --url /v2/entities/E/attrs --payload "$payload" +echo +echo + + +echo "08. Update E-A entity with same string (notification)" echo "=====================================================" payload='{ "A": { @@ -183,7 +238,7 @@ echo echo -echo "07. Dump accumulator, see 3 notifications" +echo "09. Dump accumulator, see 5 notifications" echo "=========================================" accumulatorDump echo @@ -219,7 +274,15 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36}) -04. Update E-A entity with JSON array (notification) +04. Update E-A entity with different JSON object (notification) +=============================================================== +HTTP/1.1 204 No Content +Date: REGEX(.*) +Fiware-Correlator: REGEX([0-9a-f\-]{36}) + + + +05. Update E-A entity with JSON array (notification) ==================================================== HTTP/1.1 204 No Content Date: REGEX(.*) @@ -227,7 +290,7 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36}) -05. Update E-A entity with same JSON array (no notification) +06. Update E-A entity with same JSON array (no notification) ============================================================ HTTP/1.1 204 No Content Date: REGEX(.*) @@ -235,7 +298,15 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36}) -06. Update E-A entity with same string (notification) +07. Update E-A entity with different JSON array (notification) +============================================================== +HTTP/1.1 204 No Content +Date: REGEX(.*) +Fiware-Correlator: REGEX([0-9a-f\-]{36}) + + + +08. Update E-A entity with same string (notification) ===================================================== HTTP/1.1 204 No Content Date: REGEX(.*) @@ -243,7 +314,7 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36}) -07. Dump accumulator, see 3 notifications +09. Dump accumulator, see 5 notifications ========================================= POST http://127.0.0.1:REGEX(\d+)/notify Fiware-Servicepath: / @@ -301,6 +372,60 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36}); cbnotif=1 ======================================= POST http://127.0.0.1:REGEX(\d+)/notify Fiware-Servicepath: / +Content-Length: 274 +User-Agent: orion/REGEX(\d+\.\d+\.\d+.*) +Ngsiv2-Attrsformat: normalized +Host: 127.0.0.1:REGEX(\d+) +Accept: application/json +Content-Type: application/json; charset=utf-8 +Fiware-Correlator: REGEX([0-9a-f\-]{36}); cbnotif=1 + +{ + "data": [ + { + "A": { + "metadata": {}, + "type": "Json", + "value": { + "array": [ + "22", + { + "x": [ + "x1", + "x2" + ], + "y": 3 + }, + [ + "z1", + "z2" + ] + ], + "bool": true, + "null": null, + "number": 10, + "object": { + "x": { + "x1": "a", + "x2": "b" + }, + "y": [ + "y1", + "y_new" + ] + }, + "text": "foo" + } + }, + "id": "E", + "type": "T" + } + ], + "subscriptionId": "REGEX([0-9a-f]{24})" +} +======================================= +POST http://127.0.0.1:REGEX(\d+)/notify +Fiware-Servicepath: / Content-Length: 162 User-Agent: orion/REGEX(\d+\.\d+\.\d+.*) Ngsiv2-Attrsformat: normalized @@ -339,6 +464,44 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36}); cbnotif=1 ======================================= POST http://127.0.0.1:REGEX(\d+)/notify Fiware-Servicepath: / +Content-Length: 165 +User-Agent: orion/REGEX(\d+\.\d+\.\d+.*) +Ngsiv2-Attrsformat: normalized +Host: 127.0.0.1:REGEX(\d+) +Accept: application/json +Content-Type: application/json; charset=utf-8 +Fiware-Correlator: REGEX([0-9a-f\-]{36}); cbnotif=1 + +{ + "data": [ + { + "A": { + "metadata": {}, + "type": "Array", + "value": [ + "", + { + "x": [ + "x1", + "x_new" + ], + "y": "3" + }, + [ + "z1", + "z2" + ] + ] + }, + "id": "E", + "type": "T" + } + ], + "subscriptionId": "REGEX([0-9a-f]{24})" +} +======================================= +POST http://127.0.0.1:REGEX(\d+)/notify +Fiware-Servicepath: / Content-Length: 124 User-Agent: orion/REGEX(\d+\.\d+\.\d+.*) Ngsiv2-Attrsformat: normalized