Merge pull request telefonicaid#4696 from telefonicaid/hardening/4681-removal-ngsiv1-follow-up

FIX use booleans for csub and registrations isPattern fields

Author: GregorioBlazquez

CHANGES_NEXT_RELEASE

@@ -6,4 +6,5 @@
 - Fix: some error messages in corner case situations (405, 406, 415 and some cases of 400) were not strictly following NGSIv2 error response syntax
 - Fix: admin log operations error responses were not following NGSIv2 error response syntax
 - Fix: memory leaks in parse logic in case of parse error (possibly)
+- Fix: csubs and registrations now use isPattern as bool instead of string (legacy from NGSIv1 model), although strings are still supported (#4681)
 - Hardening: remove a huge amount of NGSIv1 related dead-code (#4681)

doc/manuals/admin/database_model.md

@@ -211,15 +211,16 @@ Fields:
 -   **contextRegistration**: is an array whose elements contain the
     following information:
     -   **entities**: an array containing a list of
-        entities (mandatory). The JSON for each entity contains **id**,
-        **type**, **isPattern** and **isTypePattern**. Note that, due to legacy
-        reasons, **isPattern** may be `"true"` or `"false"` (text) while
-        **isTypePattern** may be `true` or `false` (boolean).
+        entities (mandatory). The JSON for each entity contains **id** (string),
+        **type** (string), **isPattern** (bool) and **isTypePattern** (bool) (*).
     -   **attrs**: an array containing a list of attributes (optional).
         The JSON for each attribute contains **name** and **type**.
     -   **providingApplication**: the URL of the providing application
         for this registration (mandatory)
 
+(*) Versions previous to Orion 4.3.0 use `isPattern` as strings (`"true"` or `"false"`). Orion suppports
+reading this as strings (legacy) or as bool (current) but always stores them as bool (`true` or `false`).
+
 Example document:
 
 ```
@@ -234,12 +235,12 @@ Example document:
                {
                    "id": "E1",
                    "type": "T1",
-                   "isPattern": "false"
+                   "isPattern": false
                },
                {
                    "id": "E2",
                    "type": "T2",
-                   "isPattern": "false"
+                   "isPattern": false
                }
            ],
            "attrs": [
@@ -287,9 +288,7 @@ Fields:
 -   **qos**: MQTT QoS value (only in MQTT notifications)
 -   **retain**: MQTT retain value (only in MQTT notifications)
 -   **entities**: an array of entities (mandatory). The JSON for each
-    entity contains **id**, **type**, **isPattern** and **isTypePattern**. Note that,
-    due to legacy reasons, **isPattern** may be `"true"` or `"false"` (text) while
-    **isTypePattern** may be `true` or `false` (boolean).
+    entity contains **id** (string), **type** (string), **isPattern** (bool)  **isTypePattern** (bool) (*).
 -   **attrs**: an array of attribute names (strings) (optional).
 -   **blacklist**: a boolean field that specifies if `attrs` has to be interpreted
     as a whitelist (if `blacklist` is equal to `false` or doesn't exist) or a
@@ -345,6 +344,9 @@ notifications. It is a number between 0 and 1800000. If defined to 0 or omitted,
     More information in [covered subscription section in Orion API specification](../orion-api.md#covered-subscriptions).
 -   **notifyOnMetadataChange**: if `true` metadata is considered part of the value of the attribute regarding subscription triggering. If `false` metadata is not considered part of the value of the attribute regarding subscription triggering. Default behaviour (if omitted) is the one for `true`.
 
+(*) Versions previous to Orion 4.3.0 use `isPattern` as strings (`"true"` or `"false"`). Orion suppports
+reading this as strings (legacy) or as bool (current) but always stores them as bool (`true` or `false`).
+
 Example document:
 
 ```
@@ -357,7 +359,7 @@ Example document:
                 {
                         "id" : ".*",
                         "type" : "Room",
-                        "isPattern" : "true",
+                        "isPattern" : true,
                         "isTypePattern": false
                 }
         ],

doc/manuals/deprecated.md

@@ -15,6 +15,8 @@ not maintained or evolved any longer. In particular:
 
 A list of deprecated features and the version in which they were deprecated follows:
 
+* String values for `isPattern` (`"true"` or `"false"`) in subscriptions and registrations database model in Orion 4.3.0.
+  Orion suppports reading this as strings (deprecated) or as bool but always stores them as bool (`true` or `false`).
 * CLI parameters (and associated env vars): `-dbhost`, `-rplSet`, `-dbTimeout`, `-dbuser`,
   `-dbAuthMech`, `-dbAuthDb`, `-dbSSL` and `-dbDisableRetryWrites` in Orion 3.12.0. Use `dbURI` instead,
   checking [this section](#mapping-to-mongouri-from-old-cli-parameters) if you need to know hot to build the MongoDB URI (removed in Orion 4.0.0).
@@ -136,6 +138,7 @@ The following table provides information about the last Orion version supporting
 
 | **Removed feature**                                                        | **Last Orion version supporting feature** | **That version release date**   |
 |----------------------------------------------------------------------------|-------------------------------------------|---------------------------------|
+| String values for `isPattern` in subscriptions and registrations database model | Not yet defined                           | Not yet defined                 |
 | `attributes` field in `POST /v2/entities` operation                        | Not yet defined                           | Not yet defined                 |
 | `APPEND`, `UPDATE`, etc. action types in `POST /v2/op/update`              | Not yet defined                           | Not yet defined                 |
 | `dateCreated` and `dateModified` in `options` URI parameter                | Not yet defined                           | Not yet defined                 |

src/lib/apiTypesV2/Registration.cpp

@@ -136,36 +136,43 @@ void Registration::setEntities(const orion::BSONObj& cr0)
     EntityId         entity;
     orion::BSONObj   ce = dbEntityV[ix].embeddedObject();
 
+    // id/idPattern
+    bool isIdPattern = false;
     if (ce.hasField(REG_ENTITY_ISPATTERN))
     {
-      std::string isPattern = getStringFieldF(ce, REG_ENTITY_ISPATTERN);
-
-      if (isPattern == "true")
+      if (getFieldF(ce, REG_ENTITY_ISPATTERN).type() != orion::Bool)
       {
-        entity.idPattern = getStringFieldF(ce, REG_ENTITY_ID);
+        // Old versions of the registrations model (previous to Orion 4.3.0) use isPattern as string
+        // Although from 4.3.0 on isPattern is always stored as boolean, we need this guard to avoid
+        // problems with old registrations. Maybe in the future we can clean our database and remove
+        // this guard
+        isIdPattern = (getStringFieldF(ce, REG_ENTITY_ISPATTERN) == "true");
       }
       else
       {
-        entity.id = getStringFieldF(ce, REG_ENTITY_ID);
+        isIdPattern = getBoolFieldF(ce, REG_ENTITY_ISPATTERN);
       }
     }
+
+    if (isIdPattern)
+    {
+      entity.idPattern = getStringFieldF(ce, REG_ENTITY_ID);
+    }
     else
     {
       entity.id = getStringFieldF(ce, REG_ENTITY_ID);
     }
 
+    // type/typePattern
+    bool isTypePattern = false;
     if (ce.hasField(REG_ENTITY_ISTYPEPATTERN))
     {
-      std::string isPattern = getStringFieldF(ce, REG_ENTITY_ISTYPEPATTERN);
+      isTypePattern = getBoolFieldF(ce, REG_ENTITY_ISTYPEPATTERN);
+    }
 
-      if (isPattern == "true")
-      {
-        entity.typePattern = getStringFieldF(ce, REG_ENTITY_TYPE);
-      }
-      else
-      {
-        entity.type = getStringFieldF(ce, REG_ENTITY_TYPE);
-      }
+    if (isTypePattern)
+    {
+      entity.typePattern = getStringFieldF(ce, REG_ENTITY_TYPE);
     }
     else
     {

src/lib/cache/subCache.cpp

@@ -79,14 +79,12 @@ EntityInfo::EntityInfo
 (
   const std::string&  _entityId,
   const std::string&  _entityType,
-  const std::string&  _isPattern,
+  bool                _isPattern,
   bool                _isTypePattern
 )
 :
-entityId(_entityId), entityType(_entityType), isTypePattern(_isTypePattern)
+entityId(_entityId), isPattern(_isPattern), entityType(_entityType), isTypePattern(_isTypePattern)
 {
-  isPattern    = (_isPattern == "true") || (_isPattern == "TRUE") || (_isPattern == "True");
-
   if (isPattern)
   {
     if (!regComp(&entityIdPattern, _entityId.c_str(), REG_EXTENDED))
@@ -951,7 +949,7 @@ void subCacheItemInsert
   for (unsigned int ix = 0; ix < entIdVector.size(); ++ix)
   {
     const EntityId*      eIdP = &entIdVector[ix];
-    std::string          isPattern      = (eIdP->id.empty())? "true" : "false";
+    bool                 isPattern      = (eIdP->id.empty());
     bool                 isTypePattern  = (eIdP->type.empty());
     std::string          id             = (eIdP->id.empty())? eIdP->idPattern   : eIdP->id;
     std::string          type           = (eIdP->type.empty())? eIdP->typePattern : eIdP->type;

src/lib/cache/subCache.h

@@ -75,7 +75,7 @@ struct EntityInfo
 
 
   EntityInfo() {}
-  EntityInfo(const std::string& _entityId, const std::string& _entityType, const std::string& _isPattern,
+  EntityInfo(const std::string& _entityId, const std::string& _entityType, bool _isPattern,
              bool _isTypePattern);
   ~EntityInfo() { release(); }
 

src/lib/common/JsonHelper.cpp

@@ -260,7 +260,7 @@ void JsonObjectHelper::addDate(const std::string& key, double timestamp)
 */
 void JsonObjectHelper::addBool(const std::string& key, bool b)
 {
-  addRaw(key, b? "true" : "false");
+  addRaw(key, FT(b));
 }
 
 
@@ -408,7 +408,7 @@ void JsonVectorHelper::addDate(double timestamp)
 */
 void JsonVectorHelper::addBool(bool b)
 {
-  addRaw(b? "true" : "false");
+  addRaw(FT(b));
 }
 
 

src/lib/common/globals.cpp

@@ -229,38 +229,6 @@ void orionInit
 
 
 
-/* ****************************************************************************
-*
-* isTrue - 
-*/
-bool isTrue(const std::string& s)
-{
-  if ((s == "true") || (s == "1"))
-  {
-    return true;
-  }
-
-  return false;
-}
-
-
-
-/* ****************************************************************************
-*
-* isFalse - 
-*/
-bool isFalse(const std::string& s)
-{
-  if ((s == "false") || (s == "0"))
-  {
-    return true;
-  }
-
-  return false;
-}
-
-
-
 /*****************************************************************************
 *
 * getTimer -

src/lib/common/globals.h

@@ -233,15 +233,6 @@ extern void orionInit
 
 
 
-/* ****************************************************************************
-*
-* isTrue - 
-*/
-extern bool isTrue(const std::string& s);
-extern bool isFalse(const std::string& s);
-
-
-
 /*****************************************************************************
 *
 * getTimer -

src/lib/expressions/ExprContext.cpp

@@ -124,12 +124,12 @@ void ExprContextObject::add(const std::string &key, bool _value)
 {
   if (basic)
   {
-    LM_T(LmtExpr, ("adding to basic expression context object (bool): %s=%s", key.c_str(), _value ? "true" : "false"));
-    repl.insert(std::pair<std::string, std::string>(key, _value? "true": "false"));
+    LM_T(LmtExpr, ("adding to basic expression context object (bool): %s=%s", key.c_str(), FT(_value)));
+    repl.insert(std::pair<std::string, std::string>(key, FT(_value)));
   }
   else
   {
-    LM_T(LmtExpr, ("adding to JEXL expression context object (bool): %s=%s", key.c_str(), _value ? "true" : "false"));
+    LM_T(LmtExpr, ("adding to JEXL expression context object (bool): %s=%s", key.c_str(), FT(_value)));
     jh.addBool(key, _value);
   }
 }