Merge pull request #4612 from telefonicaid/hardening/4605-alterationt…

…ype_entityupdate-with-notifyonmetadatachange

Fix: metadata modifications are not considered as change (with regards to subscription alterationTypes) if notifyOnMetadataChange is false

CHANGES_NEXT_RELEASE

@@ -3,6 +3,7 @@
 - Fix: $max and $min operators were not supported with DateTime attributes (#4585)
 - Fix: wrong date values should not allowed in subscription's expires field (#4541)
 - Fix: do not raise DB alarm in case of wrong GeoJSON in client request
+- Fix: metadata modifications are not considered as change (with regards to subscription alterationTypes) if notifyOnMetadataChange is false (#4605)
 - Upgrade cjexl version from 0.3.0 to 0.4.0 (new transformations: now, getTime and toIsoString)
 - Upgrade Debian version from 12.4 to 12.6 in Dockerfile
 - Fix: invalid date in expires field of subscription (#2303)

src/lib/apiTypesV2/Subscription.cpp

@@ -42,7 +42,7 @@ ngsiv2::SubAltType parseAlterationType(const std::string& altType)
 {
   if (altType == "entityChange")
   {
-    return ngsiv2::SubAltType::EntityChange;
+    return ngsiv2::SubAltType::EntityChangeBothValueAndMetadata;
   }
   else if (altType == "entityUpdate")
   {
@@ -64,13 +64,26 @@ ngsiv2::SubAltType parseAlterationType(const std::string& altType)
 
 
 
+/* ****************************************************************************
+*
+* isChangeAltType -
+*/
+bool isChangeAltType(ngsiv2::SubAltType altType)
+{
+  return (altType == ngsiv2::SubAltType::EntityChangeBothValueAndMetadata) ||
+         (altType == ngsiv2::SubAltType::EntityChangeOnlyMetadata)         ||
+         (altType == ngsiv2::SubAltType::EntityChangeOnlyValue);
+}
+
+
+
 /* ****************************************************************************
 *
 * subAltType2string -
 */
 std::string subAltType2string(ngsiv2::SubAltType altType)
 {
-  if (altType == ngsiv2::SubAltType::EntityChange)
+  if (isChangeAltType(altType))
   {
     return "entityChange";
   }

src/lib/apiTypesV2/Subscription.h

@@ -57,7 +57,11 @@ typedef enum NotificationType
 */
 typedef enum SubAltType
 {
-  EntityChange,
+  // EntityChange has been specialized into three sub-types in order to solve #4605
+  // (EntityChangeBothValueAndMetadata is thre reference one used in parsing/rendering logic)
+  EntityChangeBothValueAndMetadata,
+  EntityChangeOnlyValue,
+  EntityChangeOnlyMetadata,
   EntityUpdate,
   EntityCreate,
   EntityDelete,
@@ -192,4 +196,12 @@ extern std::string subAltType2string(ngsiv2::SubAltType altType);
 
 
 
+/* ****************************************************************************
+*
+* isChangeAltType -
+*/
+extern bool isChangeAltType(ngsiv2::SubAltType altType);
+
+
+
 #endif  // SRC_LIB_APITYPESV2_SUBSCRIPTION_H_

src/lib/cache/subCache.cpp

@@ -403,7 +403,7 @@ static bool matchAltType(CachedSubscription* cSubP, ngsiv2::SubAltType targetAlt
   // If subAltTypeV size == 0 default alteration types are update with change and create
   if (cSubP->subAltTypeV.size() == 0)
   {
-    if ((targetAltType == ngsiv2::SubAltType::EntityChange) || (targetAltType == ngsiv2::SubAltType::EntityCreate))
+    if ((isChangeAltType(targetAltType)) || (targetAltType == ngsiv2::SubAltType::EntityCreate))
     {
       return true;
     }
@@ -417,9 +417,9 @@ static bool matchAltType(CachedSubscription* cSubP, ngsiv2::SubAltType targetAlt
     ngsiv2::SubAltType altType = cSubP->subAltTypeV[ix];
 
     // EntityUpdate is special, it is a "sub-type" of EntityChange
-    if (targetAltType == ngsiv2::SubAltType::EntityChange)
+    if (isChangeAltType(targetAltType))
     {
-      if ((altType == ngsiv2::SubAltType::EntityUpdate) || (altType == ngsiv2::SubAltType::EntityChange))
+      if ((altType == ngsiv2::SubAltType::EntityUpdate) || (isChangeAltType(altType)))
       {
         return true;
       }
@@ -452,6 +452,12 @@ static bool subMatch
   ngsiv2::SubAltType               targetAltType
 )
 {
+  // If notifyOnMetadataChange is false and only metadata has been changed, we "downgrade" to ngsiv2::EntityUpdate
+  if (!cSubP->notifyOnMetadataChange && (targetAltType == ngsiv2::EntityChangeOnlyMetadata))
+  {
+    targetAltType = ngsiv2::EntityUpdate;
+  }
+
   // Check alteration type
   if (!matchAltType(cSubP, targetAltType))
   {
@@ -490,7 +496,6 @@ static bool subMatch
     return false;
   }
 
-
   //
   // If one of the attribute names in the scope vector
   // of the subscription has the same name as the incoming attribute. there is a match.
@@ -507,10 +512,13 @@ static bool subMatch
       return false;
     }
   }
-  else if ((targetAltType == ngsiv2::EntityChange) || (targetAltType == ngsiv2::EntityCreate))
+  else if ((isChangeAltType(targetAltType)) || (targetAltType == ngsiv2::EntityCreate))
   {
-    if (!attributeMatch(cSubP, attrsWithModifiedValue) &&
-        !(cSubP->notifyOnMetadataChange && attributeMatch(cSubP, attrsWithModifiedMd)))
+    // No match if: 1) there is no change in the *value* of attributes listed in conditions.attrs and 2) there is no change
+    // in the *metadata* of the attributes listed in conditions.attrs (the 2) only if notifyOnMetadataChange is true)
+    bool b1 = attributeMatch(cSubP, attrsWithModifiedValue);
+    bool b2 = attributeMatch(cSubP, attrsWithModifiedMd);
+    if (!b1 && !(cSubP->notifyOnMetadataChange && b2))
     {
       LM_T(LmtSubCacheMatch, ("No match due to attributes"));
       return false;

src/lib/mongoBackend/MongoCommonUpdate.cpp

@@ -1493,7 +1493,7 @@ static bool matchAltType(orion::BSONObj sub, ngsiv2::SubAltType targetAltType)
   // change and create. Maybe this could be check at MongoDB query stage, but seems be more complex
   if (altTypeStrings.size() == 0)
   {
-    if ((targetAltType == ngsiv2::SubAltType::EntityChange) || (targetAltType == ngsiv2::SubAltType::EntityCreate))
+    if ((isChangeAltType(targetAltType)) || (targetAltType == ngsiv2::SubAltType::EntityCreate))
     {
       return true;
     }
@@ -1513,9 +1513,9 @@ static bool matchAltType(orion::BSONObj sub, ngsiv2::SubAltType targetAltType)
     else
     {
       // EntityUpdate is special, it is a "sub-type" of EntityChange
-      if (targetAltType == ngsiv2::SubAltType::EntityChange)
+      if (isChangeAltType(targetAltType))
       {
-        if ((altType == ngsiv2::SubAltType::EntityUpdate) || (altType == ngsiv2::SubAltType::EntityChange))
+        if ((altType == ngsiv2::SubAltType::EntityUpdate) || (isChangeAltType(altType)))
         {
           return true;
         }
@@ -1641,15 +1641,21 @@ static bool addTriggeredSubscriptions_noCache
 
     if (subs.count(subIdStr) == 0)
     {
+      // Early extraction of fiedl from DB document. The rest of fields are got later
+      bool  notifyOnMetadataChange = sub.hasField(CSUB_NOTIFYONMETADATACHANGE)? getBoolFieldF(sub, CSUB_NOTIFYONMETADATACHANGE) : true;
+
+      // If notifyOnMetadataChange is false and only metadata has been changed, we "downgrade" to ngsiv2::EntityUpdate
+      if (!notifyOnMetadataChange && (targetAltType == ngsiv2::EntityChangeOnlyMetadata))
+      {
+        targetAltType = ngsiv2::EntityUpdate;
+      }
+
       // Check alteration type
       if (!matchAltType(sub, targetAltType))
       {
         continue;
       }
 
-      // Early extraction of fiedl from DB document. The rest of fields are got later
-      bool  notifyOnMetadataChange = sub.hasField(CSUB_NOTIFYONMETADATACHANGE)? getBoolFieldF(sub, CSUB_NOTIFYONMETADATACHANGE) : true;
-
       // Depending of the alteration type, we use the list of attributes in the request or the list
       // with effective modifications. Note that EntityDelete doesn't check the list
       if (targetAltType == ngsiv2::EntityUpdate)
@@ -1659,11 +1665,14 @@ static bool addTriggeredSubscriptions_noCache
           continue;
         }
       }
-      else if ((targetAltType == ngsiv2::EntityChange) || (targetAltType == ngsiv2::EntityCreate))
+      else if ((isChangeAltType(targetAltType)) || (targetAltType == ngsiv2::EntityCreate))
       {
         // Skip if: 1) there is no change in the *value* of attributes listed in conditions.attrs and 2) there is no change
-        // in the *metadata* of the attributes listed in conditions.attrs (the 2) only if notifyOnMetadtaChange is true)
-        if (!condValueAttrMatch(sub, attrsWithModifiedValue) && !(notifyOnMetadataChange && condValueAttrMatch(sub, attrsWithModifiedMd)))
+        // in the *metadata* of the attributes listed in conditions.attrs (the 2) only if notifyOnMetadataChange is true)
+        bool b1 = condValueAttrMatch(sub, attrsWithModifiedValue);
+        bool b2 = condValueAttrMatch(sub, attrsWithModifiedMd);
+
+        if (!b1 && !(notifyOnMetadataChange && b2))
         {
           continue;
         }
@@ -2766,24 +2775,34 @@ static bool processContextAttributeVector
     {
       attrsWithModifiedValue.push_back(ca->name);
       attrsWithModifiedMd.push_back(ca->name);
+      targetAltType = ngsiv2::SubAltType::EntityChangeBothValueAndMetadata;
     }
     else if (changeType == CHANGE_ONLY_VALUE)
     {
       attrsWithModifiedValue.push_back(ca->name);
+      if ((targetAltType == ngsiv2::SubAltType::EntityChangeBothValueAndMetadata) || (targetAltType == ngsiv2::SubAltType::EntityChangeOnlyMetadata))
+      {
+        targetAltType = ngsiv2::SubAltType::EntityChangeBothValueAndMetadata;
+      }
+      else
+      {
+        targetAltType = ngsiv2::SubAltType::EntityChangeOnlyValue;
+      }
     }
     else if (changeType == CHANGE_ONLY_MD)
     {
       attrsWithModifiedMd.push_back(ca->name);
+      if ((targetAltType == ngsiv2::SubAltType::EntityChangeBothValueAndMetadata) || (targetAltType == ngsiv2::SubAltType::EntityChangeOnlyValue))
+      {
+        targetAltType = ngsiv2::SubAltType::EntityChangeBothValueAndMetadata;
+      }
+      else
+      {
+        targetAltType = ngsiv2::SubAltType::EntityChangeOnlyMetadata;
+      }
     }
 
     attributes.push_back(ca->name);
-
-    /* If actual update then targetAltType changes from EntityUpdate (the value used to initialize
-     * the variable) to EntityChange */
-    if (changeType != NO_CHANGE)
-    {
-      targetAltType = ngsiv2::SubAltType::EntityChange;
-    }
   }
 
   /* Add triggered subscriptions */
@@ -3948,7 +3967,7 @@ static unsigned int updateEntity
    * previous addTriggeredSubscriptions() invocations. Before that, we add
    * builtin attributes and metadata (both NGSIv1 and NGSIv2 as this is
    * for notifications and NGSIv2 builtins can be used in NGSIv1 notifications) */
-  addBuiltins(notifyCerP, subAltType2string(ngsiv2::SubAltType::EntityChange));
+  addBuiltins(notifyCerP, subAltType2string(ngsiv2::SubAltType::EntityChangeBothValueAndMetadata));
   unsigned int notifSent = processSubscriptions(subsToNotify,
                                                 notifyCerP,
                                                 tenant,