You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This PR improves loop protection. Until this PR, this protection only works for self-custom notifications but now it works in any case (e.g. CEP loops).
It is not easy to test this PR without adding an extra tool (i.e. accumulator-py like) to redirect a notification back to the CB with a modification in the attribute name (pure self-notification doesn't work, as the attribute value doesn't change so a new notification is not triggered). Thus in this case no ftest can be added and we have tested manually the following way:
Hack the CB code so attributes with same value also trigger notifications:
diff --git a/src/lib/mongoBackend/MongoCommonUpdate.cpp b/src/lib/mongoBackend/MongoCommonUpdate.cpp
index a3fd978f0..47bb19e3a 100644
--- a/src/lib/mongoBackend/MongoCommonUpdate.cpp+++ b/src/lib/mongoBackend/MongoCommonUpdate.cpp@@ -306,6 +306,8 @@ static bool equalMetadata(const orion::BSONObj& md1, const orion::BSONObj& md2)
*/
static bool attrValueChanges(const orion::BSONObj& attr, ContextAttribute* caP, const bool& forcedUpdate, ApiVersion apiVersion)
{
+ return true;+
/* Not finding the attribute field at MongoDB is considered as an implicit "" */
if (!attr.hasField(ENT_ATTRS_VALUE))
{
Check that the following tets works (based in exiting cases/2937_self_notification_protection/self_notification_one_hop.test)
--NAME--
Self notification protection one-hop case with regular (non custom) subscription
--SHELL-INIT--
dbInit CB
brokerStart CB
--SHELL--
#
# 01. Create self-subscription for entity E attribute A
# 02. Create entity with A = boom
# 03. Wait for a while and check that only one notification was sent
# 04. Get entity with value boomx
# 05. Check the warning about loop detection in the log
echo "01. Create self-subscription for entity E attribute A"
echo "====================================================="
payload='{
"subject": {
"entities": [
{
"id" : "E"
}
],
"condition": {
"attrs": [ "A" ]
}
},
"notification": {
"http": {
"url": "http://localhost:'${CB_PORT}'/v2/op/notify"
}
}
}'
orionCurl --url /v2/subscriptions --payload "$payload"
echo
echo
echo "02. Create entity with A = boom"
echo "==============================="
payload='{
"id": "E",
"type": "Thing",
"A": {
"value": "boom",
"type": "Text"
}
}'
orionCurl --url /v2/entities --payload "$payload"
echo
echo
echo "03. Wait for a while and check that only one notification was sent"
echo "=================================================================="
sleep 5s
orionCurl --url /v2/subscriptions
echo
echo
echo "04. Get entity with value boom"
echo "=============================="
orionCurl --url /v2/entities/E
echo
echo
echo "05. Check the warning about loop detection in the log"
echo "====================================================="
egrep 'WARN' /tmp/contextBroker.log
echo
echo
--REGEXPECT--
01. Create self-subscription for entity E attribute A
=====================================================
HTTP/1.1 201 Created
Content-Length: 0
Location: /v2/subscriptions/REGEX([0-9a-f]{24})
Fiware-Correlator: REGEX([0-9a-f\-]{36})
Date: REGEX(.*)
02. Create entity with A = boom
===============================
HTTP/1.1 201 Created
Content-Length: 0
Location: /v2/entities/E?type=Thing
Fiware-Correlator: REGEX([0-9a-f\-]{36})
Date: REGEX(.*)
03. Wait for a while and check that only one notification was sent
==================================================================
HTTP/1.1 200 OK
Content-Length: 372
Content-Type: application/json
Fiware-Correlator: REGEX([0-9a-f\-]{36})
Date: REGEX(.*)
[
{
"id": "REGEX([0-9a-f]{24})",
"notification": {
"attrs": [],
"attrsFormat": "normalized",
"http": {
"url": "http://localhost:REGEX(\d+)/v2/op/notify"
},
"lastNotification": "REGEX(.*)",
"lastSuccess": "REGEX(.*)",
"lastSuccessCode": 200,
"onlyChangedAttrs": false,
"timesSent": 1
},
"status": "active",
"subject": {
"condition": {
"attrs": [
"A"
]
},
"entities": [
{
"id": "E"
}
]
}
}
]
04. Get entity with value boom
==============================
HTTP/1.1 200 OK
Content-Length: 74
Content-Type: application/json
Fiware-Correlator: REGEX([0-9a-f\-]{36})
Date: REGEX(.*)
{
"A": {
"metadata": {},
"type": "Text",
"value": "boom"
},
"id": "E",
"type": "Thing"
}
05. Check the warning about loop detection in the log
=====================================================
REGEX(.*) msg=Notification loop detected for entity id <E> type <Thing>, skipping subscription triggering
--TEARDOWN--
brokerStop CB
dbDrop CB
If we revert the change done in this PR and run the tests again, we can check it fails ("timesSent": 961 instead of "timesSent": 1 and step 05 doesn't show the expected line in the logs). This validates the fix done.
Fail in 3694_logs_improvements/log_notifications.test case shows that the fix is not so simple as I originally thought :)
Removing the (ngsiV2AttrsFormat == "custom") causes problems in POST /v2/op/update when the same entity is included more than one time in the batch, as CB processes each entity in the batch independently.
Thus, if for instance we have POST /v2/op/update - CorrA – {E1, E1}, when first E1 is processed CB stores in DB E1_last_correlator=CoorA. When processing second E1 the correlator in the request (CoorA) is the same than E1_last_correlator in DB (CoorA) thus wrongly detecting a loop. The log_notifications.test test is failing due to this.
A possible solution is not storing any last_correlator in DB while the batch is being processed. Instead of that, store in temporal memory structure, then flush that memory structure to DB once batch processing ends. However, this solution may be complex.
A possible solution is not storing any last_correlator in DB while the batch is being processed. Instead of that, store in temporal memory structure, then flush that memory structure to DB once batch processing ends. However, this solution may be complex.
This would be easier if issue #1153 is implemented. Do that issue before completing this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR improves loop protection. Until this PR, this protection only works for self-custom notifications but now it works in any case (e.g. CEP loops).
It is not easy to test this PR without adding an extra tool (i.e. accumulator-py like) to redirect a notification back to the CB with a modification in the attribute name (pure self-notification doesn't work, as the attribute value doesn't change so a new notification is not triggered). Thus in this case no ftest can be added and we have tested manually the following way:
"timesSent": 961instead of"timesSent": 1and step 05 doesn't show the expected line in the logs). This validates the fix done.