[TMP] Checkvalgrind bug/4643 4644 4645 subs issues

CHANGES_NEXT_RELEASE

@@ -1,5 +1,9 @@
 - Fix: rendering of large float numbers (according IEEE754 standard) (#4642)
+- Fix: notification was not triggered in subs using entityUpdate alterationType when some attribute not included in condition.attrs has an actual value change (#4647)
+- Fix: previousValue metadata was not taken into account in attributes not in the update that triggers the notification (#4643)
 - Fix: improve attribute and metadata invalid format dates for DateTime types in logs (#4616)
+- Fix: attrsFormat keyValues and values were not working in custom notifications using ngsi payloads (#4644)
+- Fix: JEXL expressions were not working in attrsFormat simplifiedKeyValues, keyValues or values (#4645)
 - Hardening: upgrade microhttpd dependency from 0.9.76 to 1.0.1
 - Hardening: upgrade libmosquitto dependency from 2.0.15 to 2.0.20
 - Hardening: upgrade libmongoc dependency from 1.24.3 to 1.29.0

doc/manuals/orion-api.md

@@ -3429,6 +3429,10 @@ update takes place. The elements in the `alterationTypes` array are interpreted
 Default `alterationTypes` (i.e. the one for subscription not explicitly specifying it)
 is `["entityCreate", "entityChange"]`.
 
+In the case of using `entityChange` and `entityUpdate` at the same time, `entityUpdate` takes precedence
+(in other words, using `"alterationTypes": [ "entityUpdate", "entityChange" ]` is equivalente to
+use "alterationTypes": [ "entityUpdate" ]`).
+
 The particular alteration type can be got in notifications using the
 [`alterationType` builtin attribute](#builtin-attributes).
 

src/lib/apiTypesV2/Entity.cpp

@@ -318,13 +318,14 @@ std::string Entity::toJson
   switch (renderFormat)
   {
   case NGSI_V2_VALUES:
-    out = toJsonValues(orderedAttrs);
+    out = toJsonValues(orderedAttrs, exprContextObjectP);
     break;
   case NGSI_V2_UNIQUE_VALUES:
+    // unique is not allowed in attrsFormat, so no need of passing exprContextObjectP here
     out = toJsonUniqueValues(orderedAttrs);
     break;
   case NGSI_V2_KEYVALUES:
-    out = toJsonKeyvalues(orderedAttrs);
+    out = toJsonKeyvalues(orderedAttrs, exprContextObjectP);
     break;
   default:  // NGSI_V2_NORMALIZED
     out = toJsonNormalized(orderedAttrs, metadataFilter, renderNgsiField, exprContextObjectP);
@@ -357,14 +358,14 @@ std::string Entity::toJson(RenderFormat renderFormat, bool renderNgsiField)
 *
 * Entity::toJsonValues -
 */
-std::string Entity::toJsonValues(const std::vector<ContextAttribute*>& orderedAttrs)
+std::string Entity::toJsonValues(const std::vector<ContextAttribute*>& orderedAttrs, ExprContextObject* exprContextObjectP)
 {
   JsonVectorHelper jh;
 
   for (unsigned int ix = 0; ix < orderedAttrs.size(); ix++)
   {
     ContextAttribute* caP = orderedAttrs[ix];
-    jh.addRaw(caP->toJsonValue());
+    jh.addRaw(caP->toJsonValue(exprContextObjectP));
   }
 
   return jh.str();
@@ -409,7 +410,7 @@ std::string Entity::toJsonUniqueValues(const std::vector<ContextAttribute*>& ord
 *
 * Entity::toJsonKeyvalues -
 */
-std::string Entity::toJsonKeyvalues(const std::vector<ContextAttribute*>& orderedAttrs)
+std::string Entity::toJsonKeyvalues(const std::vector<ContextAttribute*>& orderedAttrs, ExprContextObject* exprContextObjectP)
 {
   JsonObjectHelper jh;
 
@@ -424,7 +425,7 @@ std::string Entity::toJsonKeyvalues(const std::vector<ContextAttribute*>& ordere
   for (unsigned int ix = 0; ix < orderedAttrs.size(); ix++)
   {
     ContextAttribute* caP = orderedAttrs[ix];
-    jh.addRaw(caP->name, caP->toJsonValue());
+    jh.addRaw(caP->name, caP->toJsonValue(exprContextObjectP));
   }
 
   return jh.str();

src/lib/apiTypesV2/Entity.h

@@ -140,9 +140,9 @@ class Entity
 
   void addAllAttrsExceptFiltered(std::vector<ContextAttribute*>*  orderedAttrs);
 
-  std::string toJsonValues(const std::vector<ContextAttribute*>& orderedAttrs);
+  std::string toJsonValues(const std::vector<ContextAttribute*>& orderedAttrs, ExprContextObject* exprContextObjectP = NULL);
   std::string toJsonUniqueValues(const std::vector<ContextAttribute*>& orderedAttrs);
-  std::string toJsonKeyvalues(const std::vector<ContextAttribute*>& orderedAttrs);
+  std::string toJsonKeyvalues(const std::vector<ContextAttribute*>& orderedAttrs, ExprContextObject* exprContextObjectP = NULL);
   std::string toJsonNormalized(const std::vector<ContextAttribute*>&  orderedAttrs,
                                const std::vector<std::string>&        metadataFilter,
                                bool                                   renderNgsiField   = false,

src/lib/cache/subCache.cpp

@@ -393,6 +393,28 @@ static bool servicePathMatch(CachedSubscription* cSubP, char* servicePath)
 
 
 
+/* ****************************************************************************
+*
+* subHasAltType
+*
+*/
+static bool subHasAltType(CachedSubscription* cSubP, ngsiv2::SubAltType targetAltType)
+{
+  if (cSubP->subAltTypeV.size() != 0)
+  {
+    for (unsigned int ix = 0; ix < cSubP->subAltTypeV.size(); ix++)
+    {
+      if (cSubP->subAltTypeV[ix] == targetAltType)
+      {
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
+
+
 /* ****************************************************************************
 *
 * matchAltType -
@@ -504,7 +526,9 @@ static bool subMatch
   //
   // 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)
+  // The second part of the || (i.e. isChangeAltType and sub alt types include entityUpdate) is due to issue #4647
+  // (not sure if a smarter and cleaner solution is possible...)
+  if ((targetAltType == ngsiv2::EntityUpdate) || ((isChangeAltType(targetAltType)) && (subHasAltType(cSubP, ngsiv2::EntityUpdate))))
   {
     if (!attributeMatch(cSubP, attributes))
     {

src/lib/mongoBackend/MongoCommonUpdate.cpp

@@ -1476,6 +1476,33 @@ static void fill_idPtypeP
 
 
 
+/* ****************************************************************************
+*
+* subHasAltType
+*
+*/
+static bool subHasAltType(orion::BSONObj sub, ngsiv2::SubAltType targetAltType)
+{
+  std::vector<std::string> altTypeStrings;
+  if (sub.hasField(CSUB_ALTTYPES))
+  {
+    setStringVectorF(sub, CSUB_ALTTYPES, &altTypeStrings);
+  }
+  if (altTypeStrings.size() != 0)
+  {
+    for (unsigned int ix = 0; ix < altTypeStrings.size(); ix++)
+    {
+      if (parseAlterationType(altTypeStrings[ix]) == targetAltType)
+      {
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
+
+
 /* ****************************************************************************
 *
 * matchAltType
@@ -1658,7 +1685,9 @@ static bool addTriggeredSubscriptions_noCache
 
       // 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)
+      // The second part of the || (i.e. isChangeAltType and sub alt types include entityUpdate) is due to issue #4647
+      // (not sure if a smarter and cleaner solution is possible...)
+      if ((targetAltType == ngsiv2::EntityUpdate) || ((isChangeAltType(targetAltType)) && (subHasAltType(sub, ngsiv2::EntityUpdate))))
       {
         if (!condValueAttrMatch(sub, attributes))
         {

src/lib/mongoBackend/MongoGlobal.cpp

@@ -1213,48 +1213,94 @@ static void addIfNotPresentPreviousValueMetadata(ContextAttribute* caP)
     return;
   }
 
-  // Created the metadata
   Metadata* mdP = NULL;
-  ContextAttribute* previousValueP = caP->previousValue;
 
-  if (previousValueP->compoundValueP == NULL)
+  if (caP->previousValue == NULL)
   {
-    switch (previousValueP->valueType)
+    // in this case previousValue is filled with caP value
+    if (caP->compoundValueP == NULL)
     {
-    case orion::ValueTypeString:
-      mdP = new Metadata(NGSI_MD_PREVIOUSVALUE, previousValueP->type, previousValueP->stringValue);
-      break;
+      switch (caP->valueType)
+      {
+      case orion::ValueTypeString:
+        mdP = new Metadata(NGSI_MD_PREVIOUSVALUE, caP->type, caP->stringValue);
+        break;
 
-    case orion::ValueTypeBoolean:
-      mdP = new Metadata(NGSI_MD_PREVIOUSVALUE, previousValueP->type, previousValueP->boolValue);
-      break;
+      case orion::ValueTypeBoolean:
+        mdP = new Metadata(NGSI_MD_PREVIOUSVALUE, caP->type, caP->boolValue);
+        break;
 
-    case orion::ValueTypeNumber:
-      mdP = new Metadata(NGSI_MD_PREVIOUSVALUE, previousValueP->type, previousValueP->numberValue);
-      break;
+      case orion::ValueTypeNumber:
+        mdP = new Metadata(NGSI_MD_PREVIOUSVALUE, caP->type, caP->numberValue);
+        break;
 
-    case orion::ValueTypeNull:
-      mdP = new Metadata(NGSI_MD_PREVIOUSVALUE, previousValueP->type, "");
-      mdP->valueType = orion::ValueTypeNull;
-      break;
+      case orion::ValueTypeNull:
+        mdP = new Metadata(NGSI_MD_PREVIOUSVALUE, caP->type, "");
+        mdP->valueType = orion::ValueTypeNull;
+        break;
 
-    case orion::ValueTypeNotGiven:
-      LM_E(("Runtime Error (value not given for metadata)"));
-      return;
+      case orion::ValueTypeNotGiven:
+        LM_E(("Runtime Error (value not given for metadata)"));
+        return;
 
-    default:
-      LM_E(("Runtime Error (unknown value type: %d)", previousValueP->valueType));
-      return;
+      default:
+        LM_E(("Runtime Error (unknown value type: %d)", caP->valueType));
+        return;
+      }
+    }
+    else
+    {
+      mdP            = new Metadata(NGSI_MD_PREVIOUSVALUE, caP->type, "");
+      mdP->valueType = caP->valueType;
+
+      // Different with the case when previousValue is not NULL, we cannot
+      // steal the value here, as the caP->compoundValueP has its own memory
+      // independent memory lifecyel
+      mdP->compoundValueP = caP->compoundValueP->clone();
     }
   }
   else
   {
-    mdP            = new Metadata(NGSI_MD_PREVIOUSVALUE, previousValueP->type, "");
-    mdP->valueType = previousValueP->valueType;
+    ContextAttribute* previousValueP = caP->previousValue;
+    if (previousValueP->compoundValueP == NULL)
+    {
+      switch (previousValueP->valueType)
+      {
+      case orion::ValueTypeString:
+        mdP = new Metadata(NGSI_MD_PREVIOUSVALUE, previousValueP->type, previousValueP->stringValue);
+        break;
+
+      case orion::ValueTypeBoolean:
+        mdP = new Metadata(NGSI_MD_PREVIOUSVALUE, previousValueP->type, previousValueP->boolValue);
+        break;
+
+      case orion::ValueTypeNumber:
+        mdP = new Metadata(NGSI_MD_PREVIOUSVALUE, previousValueP->type, previousValueP->numberValue);
+        break;
+
+      case orion::ValueTypeNull:
+        mdP = new Metadata(NGSI_MD_PREVIOUSVALUE, previousValueP->type, "");
+        mdP->valueType = orion::ValueTypeNull;
+        break;
+
+      case orion::ValueTypeNotGiven:
+        LM_E(("Runtime Error (value not given for metadata)"));
+        return;
+
+      default:
+        LM_E(("Runtime Error (unknown value type: %d)", previousValueP->valueType));
+        return;
+      }
+    }
+    else
+    {
+      mdP            = new Metadata(NGSI_MD_PREVIOUSVALUE, previousValueP->type, "");
+      mdP->valueType = previousValueP->valueType;
 
-    // Steal the compound
-    mdP->compoundValueP = previousValueP->compoundValueP;
-    previousValueP->compoundValueP = NULL;
+      // Steal the compound
+      mdP->compoundValueP = previousValueP->compoundValueP;
+      previousValueP->compoundValueP = NULL;
+    }
   }
 
   // Add it to the vector (shadowed)
@@ -1333,8 +1379,8 @@ void addBuiltins(ContextElementResponse* cerP, const std::string& alterationType
       addIfNotPresentMetadata(caP, NGSI_MD_ACTIONTYPE, DEFAULT_ATTR_STRING_TYPE, caP->actionType);
     }
 
-    // previousValue
-    if (caP->previousValue != NULL)
+    // previousValue (except in entityCreate case)
+    if (alterationType != "entityCreate")
     {
       addIfNotPresentPreviousValueMetadata(caP);
     }

src/lib/ngsiNotify/Notifier.cpp

@@ -315,7 +315,8 @@ static bool setNgsiPayload
   ncr.subscriptionId  = subscriptionId;
   ncr.contextElementResponseVector.push_back(&cer);
 
-  if ((renderFormat == NGSI_V2_SIMPLIFIEDNORMALIZED) || (renderFormat == NGSI_V2_SIMPLIFIEDKEYVALUES))
+  if ((renderFormat == NGSI_V2_SIMPLIFIEDNORMALIZED) || (renderFormat == NGSI_V2_SIMPLIFIEDKEYVALUES) ||
+      (renderFormat == NGSI_V2_KEYVALUES) || (renderFormat == NGSI_V2_VALUES) )
   {
     *payloadP = ncr.toJson(renderFormat, attrsFilter, blacklist, metadataFilter, exprContextObjectP);
   }