Server WFS3 stable feature ids

qgis · Jan 23, 2021 · bd64ecd · bd64ecd
1 parent 0c581a8
commit bd64ecd
Show file tree

Hide file tree

Showing 18 changed files with 120 additions and 74 deletions.
diff --git a/src/server/services/wfs3/qgswfs3handlers.cpp b/src/server/services/wfs3/qgswfs3handlers.cpp
@@ -21,6 +21,7 @@
 #include "qgsserverrequest.h"
 #include "qgsserverresponse.h"
 #include "qgsserverapiutils.h"
+#include "qgsserverfeatureid.h"
 #include "qgsfeaturerequest.h"
 #include "qgsjsonutils.h"
 #include "qgsogrutils.h"
@@ -31,6 +32,7 @@
 #include "qgsserverinterface.h"
 #include "qgsexpressioncontext.h"
 #include "qgsexpressioncontextutils.h"
+#include "qgsvectorlayerutils.h"
 #include "qgslogger.h"
 
 #ifdef HAVE_SERVER_PYTHON_PLUGINS
@@ -1274,11 +1276,16 @@ void QgsWfs3CollectionsItemsHandler::handleRequest( const QgsServerApiContext &c
       QgsFeatureIterator features { mapLayer->getFeatures( featureRequest ) };
       QgsFeature feat;
       long i { 0 };
+      QMap<QgsFeatureId, QString> fidMap;
+
       while ( features.nextFeature( feat ) )
       {
         // Ignore records before offset
         if ( i >= offset )
+        {
+          fidMap.insert( feat.id(), QgsServerFeatureId::getServerFid( feat, mapLayer->dataProvider()->pkAttributeIndexes() ) );
           featureList << feat;
+        }
         i++;
       }
 
@@ -1307,6 +1314,12 @@ void QgsWfs3CollectionsItemsHandler::handleRequest( const QgsServerApiContext &c
 
       json data = exporter.exportFeaturesToJsonObject( featureList );
 
+      // Patch feature IDs with server feature IDs
+      for ( int i = 0; i < featureList.length(); i++ )
+      {
+        data[ "features" ][ i ]["id"] = fidMap.value( data[ "features" ][ i ]["id"] ).toStdString();
+      }
+
       // Add some metadata
       data["numberMatched"] = matchedFeaturesCount;
       data["numberReturned"] = featureList.count();
@@ -1413,7 +1426,8 @@ void QgsWfs3CollectionsItemsHandler::handleRequest( const QgsServerApiContext &c
         json postData = json::parse( context.request()->data() );
 
         // Process data: extract geometry (because we need to process attributes in a much more complex way)
-        const QgsFeatureList features = QgsOgrUtils::stringToFeatureList( context.request()->data(), mapLayer->fields(), QTextCodec::codecForName( "UTF-8" ) );
+        const QgsFields fields = QgsOgrUtils::stringToFields( context.request()->data(), QTextCodec::codecForName( "UTF-8" ) );
+        const QgsFeatureList features = QgsOgrUtils::stringToFeatureList( context.request()->data(), fields, QTextCodec::codecForName( "UTF-8" ) );
         if ( features.isEmpty() )
         {
           throw QgsServerApiBadRequestException( QStringLiteral( "Posted data does not contain any feature" ) );
@@ -1488,8 +1502,9 @@ void QgsWfs3CollectionsItemsHandler::handleRequest( const QgsServerApiContext &c
         }
         feat.setId( FID_NULL );
 
-        QgsFeatureList featuresToAdd;
-        featuresToAdd.append( feat );
+        QgsVectorLayerUtils::matchAttributesToFields( feat, mapLayer->fields( ) );
+
+        QgsFeatureList featuresToAdd( { feat } );
         if ( ! mapLayer->dataProvider()->addFeatures( featuresToAdd ) )
         {
           throw QgsServerApiInternalServerError( QStringLiteral( "Error adding feature to collection" ) );
@@ -1558,12 +1573,33 @@ void QgsWfs3CollectionsFeatureHandler::handleRequest( const QgsServerApiContext
   // Retrieve feature from storage
   const QString featureId { match.captured( QStringLiteral( "featureId" ) ) };
   QgsFeatureRequest featureRequest = filteredRequest( mapLayer, context );
-  featureRequest.setFilterFid( featureId.toLongLong() );
+  const QString fidExpression { QgsServerFeatureId::getExpressionFromServerFid( featureId, mapLayer->dataProvider() ) };
+  if ( ! fidExpression.isEmpty() )
+  {
+    QgsExpression *filterExpression { featureRequest.filterExpression() };
+    if ( ! filterExpression )
+    {
+      featureRequest.setFilterExpression( fidExpression );
+    }
+    else
+    {
+      featureRequest.setFilterExpression( QStringLiteral( "(%1) AND (%2)" ).arg( fidExpression, filterExpression->expression() ) );
+    }
+  }
+  else
+  {
+    bool ok;
+    featureRequest.setFilterFid( featureId.toLongLong( &ok ) );
+    if ( ! ok )
+    {
+      throw QgsServerApiInternalServerError( QStringLiteral( "Invalid feature ID [%1]" ).arg( featureId ) );
+    }
+  }
   QgsFeature feature;
   QgsFeatureIterator it { mapLayer->getFeatures( featureRequest ) };
-  if ( ! it.nextFeature( feature ) && feature.isValid() )
+  if ( ! it.nextFeature( feature ) || ! feature.isValid() )
   {
-    QgsServerApiInternalServerError( QStringLiteral( "Invalid feature [%1]" ).arg( featureId ) );
+    throw QgsServerApiInternalServerError( QStringLiteral( "Invalid feature [%1]" ).arg( featureId ) );
   }
 
   auto doGet = [ & ]( )
@@ -1583,6 +1619,8 @@ void QgsWfs3CollectionsFeatureHandler::handleRequest( const QgsServerApiContext
     exporter.setAttributes( featureRequest.subsetOfAttributes() );
     exporter.setAttributeDisplayName( true );
     json data = exporter.exportFeatureToJsonObject( feature );
+    // Patch feature ID
+    data["id"] = featureId.toStdString();
     data["links"] = links( context );
     json navigation = json::array();
     const QUrl url { context.request()->url() };
@@ -1651,7 +1689,8 @@ void QgsWfs3CollectionsFeatureHandler::handleRequest( const QgsServerApiContext
         // Parse
         json postData = json::parse( context.request()->data() );
         // Process data: extract geometry (because we need to process attributes in a much more complex way)
-        const QgsFeatureList features = QgsOgrUtils::stringToFeatureList( context.request()->data(), mapLayer->fields(), QTextCodec::codecForName( "UTF-8" ) );
+        const QgsFields fields( QgsOgrUtils::stringToFields( context.request()->data(), QTextCodec::codecForName( "UTF-8" ) ) );
+        const QgsFeatureList features = QgsOgrUtils::stringToFeatureList( context.request()->data(), fields, QTextCodec::codecForName( "UTF-8" ) );
         if ( features.isEmpty() )
         {
           throw QgsServerApiBadRequestException( QStringLiteral( "Posted data does not contain any feature" ) );

diff --git a/tests/src/python/test_qgsserver.py b/tests/src/python/test_qgsserver.py
@@ -41,7 +41,7 @@
 from io import StringIO
 from qgis.server import QgsServer, QgsServerRequest, QgsBufferServerRequest, QgsBufferServerResponse
 from qgis.core import QgsRenderChecker, QgsApplication, QgsFontUtils, QgsMultiRenderChecker
-from qgis.testing import unittest
+from qgis.testing import unittest, start_app
 from qgis.PyQt.QtCore import QSize
 from utilities import unitTestDataPath
 
@@ -50,6 +50,8 @@
 import base64
 
 
+start_app()
+
 # Strip path and content length because path may vary
 RE_STRIP_UNCHECKABLE = br'MAP=[^"]+|Content-Length: \d+'
 RE_ELEMENT = br'</*([^>\[\s]+)[ >]'
@@ -106,16 +108,6 @@ def assertXMLEqual(self, response, expected, msg='', raw=False):
                 self.assertEqual(expected_values, response_values, msg=msg + "\nXML attribute values differ at line {0}: {1} != {2}".format(line_no, expected_values, response_values))
             line_no += 1
 
-    @classmethod
-    def setUpClass(cls):
-
-        cls.app = QgsApplication([], False)
-        cls.app.initQgis()
-
-    @classmethod
-    def tearDownClass(cls):
-        cls.app.exitQgis()
-
     def setUp(self):
         """Create the server instance"""
         self.fontFamily = QgsFontUtils.standardTestFontFamily()
@@ -137,6 +129,7 @@ def setUp(self):
                 del os.environ[ev]
             except KeyError:
                 pass
+
         self.server = QgsServer()
 
         # Disable landing page API to test standard legacy XML responses in case of errors

diff --git a/tests/src/python/test_qgsserver_api.py b/tests/src/python/test_qgsserver_api.py
@@ -1144,6 +1144,23 @@ def test_wfs3_excluded_attributes(self):
             request, project, 'test_wfs3_collections_items_exclude_attribute_0.json')
         self.assertEqual(response.statusCode(), 200)
 
+    def test_wfs3_invalid_fids(self):
+        """Test exceptions for invalid fids"""
+
+        project = QgsProject()
+        project.read(unitTestDataPath('qgis_server') + '/test_project_api.qgs')
+        request = QgsBufferServerRequest(
+            'http://server.qgis.org/wfs3/collections/exclude_attribute/items/123456.geojson')
+        response = QgsBufferServerResponse()
+        self.server.handleRequest(request, response, project)
+        self.assertEqual(bytes(response.body()).decode('utf-8'), '[{"code":"Internal server error","description":"Invalid feature [123456]"}]')
+
+        request = QgsBufferServerRequest(
+            'http://server.qgis.org/wfs3/collections/exclude_attribute/items/xYz@#.geojson')
+        response = QgsBufferServerResponse()
+        self.server.handleRequest(request, response, project)
+        self.assertEqual(bytes(response.body()).decode('utf-8'), '[{"code":"Internal server error","description":"Invalid feature ID [xYz@]"}]')
+
     def test_wfs3_time_filters_ranges(self):
         """Test datetime filters"""
 
@@ -1887,6 +1904,8 @@ def testOgcApiHandler(self):
         self.assertTrue(
             h2.templatePath(ctx).endswith('/resources/server/api/ogc/templates/services/api2/handlerTwo.html'))
 
+        del(project)
+
     def testOgcApiHandlerContentType(self):
         """Test OGC API Handler content types"""
 
@@ -1936,6 +1955,8 @@ def testOgcApiHandlerContentType(self):
             'http://localhost:8000/project/7ecb/wfs3/collections/zg.grundnutzung.html')
         self.assertEqual(h3.contentTypeFromRequest(req), QgsServerOgcApi.HTML)
 
+        del(project)
+
     def testOgcApiHandlerException(self):
         """Test OGC API Handler exception"""
 
@@ -1966,6 +1987,8 @@ def testOgcApiHandlerException(self):
         self.assertEqual(
             str(ex.exception), "UTF-8 Exception 2 $ù~à^£")
 
+        del(project)
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/tests/src/python/test_qgsserver_wms_getfeatureinfo_postgres.py b/tests/src/python/test_qgsserver_wms_getfeatureinfo_postgres.py
@@ -23,20 +23,10 @@
 # executions
 os.environ['QT_HASH_SEED'] = '1'
 
-os.environ['QGIS_CUSTOM_CONFIG_PATH'] = tempfile.mkdtemp('', 'QGIS-PythonTestConfigPath')
-
 import re
-import urllib.request
 import urllib.parse
-import urllib.error
-
-import xml.etree.ElementTree as ET
-import json
-
-from qgis.testing import unittest, start_app
-from qgis.PyQt.QtCore import QSize
 
-import osgeo.gdal  # NOQA
+from qgis.testing import unittest
 
 from test_qgsserver_wms import TestQgsServerWMSTestBase
 from qgis.core import QgsProject, QgsVectorLayer, QgsFeatureRequest, QgsExpression, QgsProviderRegistry
@@ -51,13 +41,14 @@ def setUpClass(cls):
 
         super().setUpClass()
 
-        cls.dbconn = 'service=qgis_test'
         if 'QGIS_PGTEST_DB' in os.environ:
             cls.dbconn = os.environ['QGIS_PGTEST_DB']
+        else:
+            cls.dbconn = 'service=qgis_test dbname=qgis_test sslmode=disable '
 
         # Test layer
         md = QgsProviderRegistry.instance().providerMetadata('postgres')
-        uri = cls.dbconn + 'dbname=qgis_test sslmode=disable '
+        uri = cls.dbconn + ' dbname=qgis_test sslmode=disable '
         conn = md.createConnection(uri, {})
         conn.executeSql('DROP TABLE IF EXISTS "qgis_test"."someDataLong" CASCADE')
         conn.executeSql('SELECT * INTO "qgis_test"."someDataLong" FROM "qgis_test"."someData"')

diff --git a/...tdata/qgis_server/api/test_wfs3_collections_items_as-areas-short-name_3857.json b/...tdata/qgis_server/api/test_wfs3_collections_items_as-areas-short-name_3857.json
@@ -172,7 +172,7 @@ Content-Type: application/geo+json
         ],
         "type": "MultiPolygon"
       },
-      "id": 1,
+      "id": "1",
       "properties": {
         "bearbeiter": "scholle-b",
         "bemerkung": "",
@@ -247,7 +247,7 @@ Content-Type: application/geo+json
         ],
         "type": "MultiPolygon"
       },
-      "id": 2,
+      "id": "2",
       "properties": {
         "bearbeiter": "",
         "bemerkung": "",
@@ -314,7 +314,7 @@ Content-Type: application/geo+json
         ],
         "type": "MultiPolygon"
       },
-      "id": 3,
+      "id": "3",
       "properties": {
         "bearbeiter": "scholle-b",
         "bemerkung": null,
@@ -393,7 +393,7 @@ Content-Type: application/geo+json
         ],
         "type": "MultiPolygon"
       },
-      "id": 4,
+      "id": "4",
       "properties": {
         "bearbeiter": "scholle-b",
         "bemerkung": "",
@@ -456,7 +456,7 @@ Content-Type: application/geo+json
         ],
         "type": "MultiPolygon"
       },
-      "id": 5,
+      "id": "5",
       "properties": {
         "bearbeiter": "scholle-b",
         "bemerkung": "",
@@ -531,7 +531,7 @@ Content-Type: application/geo+json
         ],
         "type": "MultiPolygon"
       },
-      "id": 6,
+      "id": "6",
       "properties": {
         "bearbeiter": "scholle-b",
         "bemerkung": "",
@@ -626,7 +626,7 @@ Content-Type: application/geo+json
         ],
         "type": "MultiPolygon"
       },
-      "id": 7,
+      "id": "7",
       "properties": {
         "bearbeiter": "scholle-b",
         "bemerkung": "",
@@ -705,7 +705,7 @@ Content-Type: application/geo+json
         ],
         "type": "MultiPolygon"
       },
-      "id": 8,
+      "id": "8",
       "properties": {
         "bearbeiter": "scholle-b",
         "bemerkung": null,
@@ -768,7 +768,7 @@ Content-Type: application/geo+json
         ],
         "type": "MultiPolygon"
       },
-      "id": 9,
+      "id": "9",
       "properties": {
         "bearbeiter": "scholle-b",
         "bemerkung": null,
@@ -943,7 +943,7 @@ Content-Type: application/geo+json
         ],
         "type": "MultiPolygon"
       },
-      "id": 10,
+      "id": "10",
       "properties": {
         "bearbeiter": "scholle-b",
         "bemerkung": null,