Merge pull request telefonicaid#4048 from telefonicaid/remove/ngsiv1-…

…location-metadata

Remove obsolete location medadata in NGSIv1

CHANGES_NEXT_RELEASE

@@ -21,6 +21,7 @@
 - Add: CLI parameter -dbUri / env var ORION_MONGO_URI (#3794)
 - Fix: improve logs in MongoDB query logic
 - Fix: false positive in log deprecation logic when entity name (or other literal) includes the token "v1" (#4454)
+- Remove: deprecated NGSIv1 location metadata to specify entity geo-location
 - Upgrade Debian version from 11.6 to 12.4 in Dockerfile
 - Hardening: upgrade libmongoc dependency from 1.23.1 to 1.24.3
 - Reference MongoDB version changed from 4.4 to 6.0

doc/manuals.jp/admin/statistics.md

@@ -44,7 +44,6 @@ Orion Context broker は、`GET /statistics` と `GET /cache/statistics` を介
   "counters": {
     "deprecatedFeatures": {
       "geoFormat": 2,
-      "metadataLocation": 1,
       "ngsiv1Forwarding": 4,
       "ngsiv1NotifFormat": 4,
       "ngsiv1Requests": 4

doc/manuals/admin/logs.md

@@ -431,12 +431,6 @@ time=2024-01-11T16:23:24.701Z | lvl=WARN | corr=be709034-b09d-11ee-b5d1-080027cd
 time=2024-01-11T16:23:24.716Z | lvl=WARN | corr=be7ae5ac-b09d-11ee-98c8-080027cd35f1 | trans=1704990203-652-00000000015 | from=127.0.0.1 | srv=s1 | subsrv=/A | comp=Orion | op=Notifier.cpp[680]:buildSenderParams | msg=Deprecated usage of notification legacy format in notification (subId: 65a015fcda947708d30425eb)
 ```
 
-* Usages NGSIv1 usages of location metadata. Example:
-
-```
-time=2023-06-08T15:14:20.999Z | lvl=WARN | corr=24fd2acc-060f-11ee-94cc-000c29583ca5 | trans=1686237259-703-00000000003 | from=127.0.0.1 | srv=s1 | subsrv=/A | comp=Orion | op=location.cpp[329]:getGeoJson | msg=Deprecated usage of metadata location coords detected in attribute location at entity update, please use geo:json instead
-```
-
 * Usages of `geo:point`, `geo:line`, `geo:box` or `geo:line`.
 
 ```

doc/manuals/admin/statistics.md

@@ -56,7 +56,6 @@ The counter block provides information about counters for the times a particular
   "counters": {
     "deprecatedFeatures": {
       "geoFormat": 2,
-      "metadataLocation": 1,
       "ngsiv1Forwarding": 4,
       "ngsiv1NotifFormat": 4,
       "ngsiv1Requests": 4

doc/manuals/deprecated.md

@@ -58,7 +58,7 @@ A list of deprecated features and the version in which they were deprecated foll
 * `/ngsi10` and `/ngsi9` as URL path prefixes are deprecated in Orion 1.2.0. Please,
   use `/v1` and `/v1/registry` instead.
         * `/ngsi9` URL paths removed in Orion 3.8.0
-* `location` metadata to specify entity location is deprecated in Orion 1.1.0. The new way
+* `location` metadata to specify entity location is deprecated in Orion 1.1.0 (removed in Orion 3.11.0). The new way
   of specifying entity location is to use `geo:json` type for the attribute (see details in
   [the corresponding section of the Orion API specification](orion-api.md#geospatial-properties-of-entities).
 * Deprecated command line argument in Orion 0.26.1 (removed in Orion 1.0.0).
@@ -110,9 +110,9 @@ The following table provides information about the last Orion version supporting
 | `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                 |
-| `location` metadata to specify entity location                             | Not yet defined                           | Not yet defined                 |
 | `GET /v2` operation                                                        | Not yet defined                           | Not yet defined                 |
 | `geo:point`, `geo:line`, `geo:box` and `geo:polygon` attribute types       | Not yet defined                           | Not yet defined                 |
+| `location` metadata to specify entity location                             | 3.10.1                                    | June 12th, 2023                 |
 | NGSIv1 API (along with CLI: `-strictNgsiv1Ids` and `-ngsiv1Autocast`)      | 3.9.0 (*)                                 | June 2nd, 2023                  |
 | `/ngsi10` and `/ngsi9` URL prefixes                                        | 3.7.0 (*)                                 | May 26th, 2022                  |
 | Initial notification upon subscription creation or update                  | 3.1.0                                     | June 9th, 2021                  |

doc/manuals/orion-api.md

@@ -737,8 +737,6 @@ The list of builtin metadata is as follows:
   was included in the request that triggered the notification. Its value depends on the request operation
   type: `update` for updates, `append` for creation and `delete` for deletion. Its type is always `Text`.
 
-* `location`, which is currently [deprecated](deprecated.md), but still supported.
-
 Like regular metadata, they can be used in `mq` filters. However, they cannot be used in resource URLs.
 
 ## Attribute names restrictions

src/lib/common/globals.h

@@ -44,9 +44,6 @@
 
 #define EARTH_RADIUS_METERS         6371000
 
-#define LOCATION_WGS84              "WGS84"
-#define LOCATION_WGS84_LEGACY       "WSG84"    // We fixed the right string at 0.17.0, but the old one needs to be mantained
-
 
 
 /* ****************************************************************************

src/lib/common/statistics.cpp

@@ -153,7 +153,6 @@ int noOfSimulatedNotifications   = -1;
 int noOfDprNgsiv1Request         = -1;
 int noOfDprLegacyForwarding      = -1;
 int noOfDprLegacyNotif           = -1;
-int noOfDprLocationMetadata      = -1;
 int noOfDprGeoformat             = -1;
 
 

src/lib/common/statistics.h

@@ -280,7 +280,6 @@ extern int noOfSimulatedNotifications;
 extern int noOfDprNgsiv1Request;
 extern int noOfDprLegacyForwarding;
 extern int noOfDprLegacyNotif;
-extern int noOfDprLocationMetadata;
 extern int noOfDprGeoformat;
 
 /* ****************************************************************************

src/lib/mongoBackend/MongoCommonUpdate.cpp

@@ -2560,7 +2560,6 @@ static bool deleteContextAttributeItem
   std::string*              currentLocAttrName,
   bool*                     entityModified,
   orion::BSONDate*          dateExpiration,
-  ApiVersion                apiVersion,
   OrionError*               oe
 )
 {
@@ -2570,7 +2569,7 @@ static bool deleteContextAttributeItem
     *entityModified = true;
 
     /* Check aspects related with location */
-    if (!targetAttr->getLocation(&attrs, apiVersion).empty())
+    if (targetAttr->getLocation(&attrs))
     {
       std::string details = std::string("action: DELETE") +
                             " - entity: [" + entityDetail + "]" +
@@ -2742,7 +2741,6 @@ static bool processContextAttributeVector
                                       currentLocAttrName,
                                       &entityModified,
                                       dateExpiration,
-                                      apiVersion,
                                       oe))
       {
         return false;

src/lib/mongoBackend/location.cpp

@@ -304,6 +304,8 @@ static bool isSpecialGeoJsonType(const ContextAttribute* caP, orion::BSONObjBuil
 *
 * It returns true, except in the case of error (in which in addition errDetail gets filled)
 *
+* FIXME P6: try to avoid apiVersion
+*
 * FIXME P6: review the cases in which this function returns false. Maybe many cases (or all them)
 * can be moved to checkGeoJson() in the parsing layer, as preconditions.
 */
@@ -319,37 +321,6 @@ static bool getGeoJson
   std::vector<double>      coordLong;
   orion::BSONArrayBuilder  ba;
 
-  if ((apiVersion == V1) && (caP->type != GEO_POINT) && (caP->type != GEO_LINE) && (caP->type != GEO_BOX) &&
-      (caP->type != GEO_POLYGON) && (caP->type != GEO_JSON))
-  {
-    // This corresponds to the legacy way in NGSIv1 based in metadata
-    // The block is the same that for GEO_POINT but it is clearer if we keep it separated
-
-    __sync_fetch_and_add(&noOfDprLocationMetadata, 1);
-    if (logDeprecate)
-    {
-      LM_W(("Deprecated usage of metadata location %s detected in attribute %s at entity update, please use geo:json instead", caP->type.c_str(), caP->name.c_str()));
-    }
-
-    double  aLat;
-    double  aLong;
-
-    if (!string2coords(caP->stringValue, aLat, aLong))
-    {
-      *errDetail = "geo coordinates format error [see Orion user manual]: " + caP->stringValue;
-      return false;
-    }
-
-    geoJson->append("type", "Point");
-
-    orion::BSONArrayBuilder ba;
-    ba.append(aLong);
-    ba.append(aLat);
-    geoJson->append("coordinates", ba.arr());
-
-    return true;
-  }
-
   if ((caP->type == GEO_POINT) || (caP->type == GEO_LINE) || (caP->type == GEO_BOX) || (caP->type == GEO_POLYGON))
   {
     __sync_fetch_and_add(&noOfDprGeoformat, 1);
@@ -563,9 +534,7 @@ bool processLocationAtEntityCreation
   {
     const ContextAttribute* caP = caV[ix];
 
-    std::string location = caP->getLocation(NULL, apiVersion);
-
-    if (location.empty())
+    if (!caP->getLocation(NULL))
     {
       continue;
     }
@@ -577,13 +546,6 @@ bool processLocationAtEntityCreation
       return false;
     }
 
-    if ((location != LOCATION_WGS84) && (location != LOCATION_WGS84_LEGACY))
-    {
-      *errDetail = "only WGS84 are supported, found: " + location;
-      oe->fill(SccBadRequest, *errDetail, ERROR_BAD_REQUEST);
-      return false;
-    }
-
     if (!getGeoJson(caP, geoJson, errDetail, apiVersion))
     {
       oe->fill(SccBadRequest, "error parsing location attribute: " + *errDetail, ERROR_BAD_REQUEST);
@@ -614,22 +576,12 @@ bool processLocationAtUpdateAttribute
 )
 {
   std::string subErr;
-  std::string locationString = targetAttr->getLocation(attrsP, apiVersion);
-
-  /* Check that location (if any) is using the correct coordinates string (it only
-   * makes sense for NGSIv1, this is legacy code that will be eventually removed) */
-  if ((!locationString.empty()) && (locationString != LOCATION_WGS84) && (locationString != LOCATION_WGS84_LEGACY))
-  {
-    *errDetail = "only WGS84 is supported for location, found: [" + locationString + "]";
-    oe->fill(SccBadRequest, *errDetail, ERROR_BAD_REQUEST);
-    return false;
-  }
 
   //
   // Case 1:
   //   update *to* location. There are 3 sub-cases
   //
-  if (!locationString.empty())
+  if (targetAttr->getLocation(attrsP))
   {
     //
     // Case 1a:
@@ -740,19 +692,10 @@ bool processLocationAtAppendAttribute
 )
 {
   std::string subErr;
-  std::string locationString = targetAttr->getLocation(attrsP, apiVersion);
-
-  /* Check that location (if any) is using the correct coordinates string (it only
-     * makes sense for NGSIv1, this is legacy code that will be eventually removed) */
-  if ((!locationString.empty()) && (locationString != LOCATION_WGS84) && (locationString != LOCATION_WGS84_LEGACY))
-  {
-    *errDetail = "only WGS84 is supported for location, found: [" + locationString + "]";
-    oe->fill(SccBadRequest, *errDetail, ERROR_BAD_REQUEST);
-    return false;
-  }
+  bool        isALocation = targetAttr->getLocation(attrsP);
 
   /* Case 1: append of new location attribute */
-  if (actualAppend && (!locationString.empty()))
+  if (actualAppend && isALocation)
   {
     /* Case 1a: there is a previous location attribute -> error */
     if (!currentLocAttrName->empty())
@@ -777,7 +720,7 @@ bool processLocationAtAppendAttribute
     }
   }
   /* Case 2: append-as-update changing attribute type from no-location -> location */
-  else if (!actualAppend && (!locationString.empty()))
+  else if (!actualAppend && isALocation)
   {
     /* Case 2a: there is a previous (not empty and with different name) location attribute -> error */
     if ((!currentLocAttrName->empty()) && (*currentLocAttrName != targetAttr->name))
@@ -813,7 +756,7 @@ bool processLocationAtAppendAttribute
   }
   /* Check 3: in the case of append-as-update, type changes from location -> no-location for the current location
    * attribute, then remove location attribute */
-  else if (!actualAppend && (locationString.empty()) && (*currentLocAttrName == targetAttr->name))
+  else if (!actualAppend && !isALocation && (*currentLocAttrName == targetAttr->name))
   {
     *currentLocAttrName = "";
   }

src/lib/ngsi/ContextAttribute.cpp

@@ -706,81 +706,59 @@ ContextAttribute::ContextAttribute
 * - If overrideMetadata option is used (as in this case existing medatada are goint to be deleted and
 *   doesn't count for looking ignoreTYpe)
 */
-std::string ContextAttribute::getLocation(orion::BSONObj* attrsP, ApiVersion apiVersion) const
+bool ContextAttribute::getLocation(orion::BSONObj* attrsP) const
 {
-  if (apiVersion == V1)
+  // null value is allowed but inhibits the attribute to be used as location (e.g. in geo-queries)
+  if ((valueType != orion::ValueTypeNull) && ((type == GEO_POINT) || (type == GEO_LINE) || (type == GEO_BOX) || (type == GEO_POLYGON) || (type == GEO_JSON)))
   {
-    // Deprecated way, but still supported
-    for (unsigned int ix = 0; ix < metadataVector.size(); ++ix)
+    // First lookup in the metadata included in the request
+    if (metadataVector.lookupByName(NGSI_MD_IGNORE_TYPE))
     {
-      if (metadataVector[ix]->name == NGSI_MD_LOCATION)
-      {
-        return metadataVector[ix]->stringValue;
-      }
+      return !hasIgnoreType();
     }
 
-    // Current way of declaring location in NGSIv1, aligned with NGSIv2 (originally only only geo:point was supported
-    // but doing so have problems so we need to support all them at the end, 
-    // see https://github.com/telefonicaid/fiware-orion/issues/3442 for details)
-    if ((type == GEO_POINT) || (type == GEO_LINE) || (type == GEO_BOX) || (type == GEO_POLYGON) || (type == GEO_JSON))
+    // If ignoreType has not yet found, second lookup in the existing metadata attributes
+    // (if attrP is not NULL and metadata array exists)
+    if (attrsP != NULL)
     {
-      return LOCATION_WGS84;
-    }
-  }
-  else // v2
-  {
-    // null value is allowed but inhibits the attribute to be used as location (e.g. in geo-queries)
-    if ((valueType != orion::ValueTypeNull) && ((type == GEO_POINT) || (type == GEO_LINE) || (type == GEO_BOX) || (type == GEO_POLYGON) || (type == GEO_JSON)))
-    {
-      // First lookup in the metadata included in the request
-      if (metadataVector.lookupByName(NGSI_MD_IGNORE_TYPE))
-      {
-        return hasIgnoreType() ? "" : LOCATION_WGS84;
-      }
-
-      // If ignoreType has not yet found, second lookup in the existing metadata attributes
-      // (if attrP is not NULL and metadata array exists)
-      if (attrsP != NULL)
+      std::string effectiveName = dbEncode(name);
+      if (attrsP->hasField(effectiveName))
       {
-        std::string effectiveName = dbEncode(name);
-        if (attrsP->hasField(effectiveName))
+        orion::BSONObj attr = getObjectFieldF(*attrsP, effectiveName);
+        if (attr.hasField(ENT_ATTRS_MD))
         {
-          orion::BSONObj attr = getObjectFieldF(*attrsP, effectiveName);
-          if (attr.hasField(ENT_ATTRS_MD))
-          {
-            // FIXME P5: not sure if this way of lookup the metadata collection is the best one
-            // or can be simplified
-            orion::BSONObj         md = getFieldF(attr, ENT_ATTRS_MD).embeddedObject();
-            std::set<std::string>  mdsSet;
+          // FIXME P5: not sure if this way of lookup the metadata collection is the best one
+          // or can be simplified
+          orion::BSONObj         md = getFieldF(attr, ENT_ATTRS_MD).embeddedObject();
+          std::set<std::string>  mdsSet;
 
-            md.getFieldNames(&mdsSet);
+          md.getFieldNames(&mdsSet);
 
-            for (std::set<std::string>::iterator i = mdsSet.begin(); i != mdsSet.end(); ++i)
+          for (std::set<std::string>::iterator i = mdsSet.begin(); i != mdsSet.end(); ++i)
+          {
+            std::string  mdName = *i;
+            if (mdName == NGSI_MD_IGNORE_TYPE)
             {
-              std::string  mdName = *i;
-              if (mdName == NGSI_MD_IGNORE_TYPE)
+              orion::BSONObj mdItem = getObjectFieldF(md, mdName);
+              orion::BSONElement mdValue = getFieldF(mdItem, ENT_ATTRS_MD_VALUE);
+              if ((mdValue.type() == orion::Bool) && (mdValue.Bool() == true))
+              {
+                return false;
+              }
+              else  // false or not a boolean
               {
-                orion::BSONObj mdItem = getObjectFieldF(md, mdName);
-                orion::BSONElement mdValue = getFieldF(mdItem, ENT_ATTRS_MD_VALUE);
-                if ((mdValue.type() == orion::Bool) && (mdValue.Bool() == true))
-                {
-                  return "";
-                }
-                else  // false or not a boolean
-                {
-                  return LOCATION_WGS84;
-                }
+                return true;
               }
             }
           }
         }
       }
-
-      return LOCATION_WGS84;
     }
+
+    return true;
   }
 
-  return "";
+  return false;
 }
 
 

src/lib/ngsi/ContextAttribute.h

@@ -92,7 +92,7 @@ typedef struct ContextAttribute
   ContextAttribute(const std::string& _name, const std::string& _type, orion::CompoundValueNode* _compoundValueP);
 
   /* Check if attribute means a location  */
-  std::string  getLocation(orion::BSONObj* attrsP, ApiVersion apiVersion) const;
+  bool  getLocation(orion::BSONObj* attrsP) const;
 
   std::string  toJsonV1(bool                             asJsonObject,
                         RequestType                      request,