MetadataUrl - Fix some "const" and PR review

qgis · Sep 16, 2021 · e8c3e7c · e8c3e7c
1 parent 2e9a151
commit e8c3e7c
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 65 deletions.
diff --git a/doc/api_break.dox b/doc/api_break.dox
@@ -33,6 +33,12 @@ QgsNewNameDialog       {#qgis_api_break_3_22_qgsnewnamedialog}
 away from the deprecated QRegExp class.  It has been replaced with QgsNewNameDialog::setRegularExpression which
 accepts a string argument.
 
+QgsVectorLayerServerProperties       {#qgis_api_break_3_22_qgsvectorlayerserverproperties}
+------------------------------
+
+- The content of QgsVectorLayerServerProperties has been moved in QgsMapLayerServerProperties. But the member
+in QgsVectorLayer::serverProperties() is still existing.
+
 QGIS 3.20        {#qgis_api_break_3_20}
 =========
 
@@ -48,7 +54,7 @@ QgsMapLayer       {#qgis_api_break_3_20_qgsmaplayer}
 
 - QgsMapLayer::setDataSource is no longer virtual. It has been replaced with QgsMapLayer::setDataSourcePrivate. This is only relevant for subclassing QgsMapLayer, for code that only uses the subclasses (QgsVectorLayer, QgsRasterLayer, ...) this has no effect.
 
-- QgsVectorLayerFeatureIterator::FetchJoinInfo::joinLayer was removed. This layer pointer was dangerous and not safe to access in any situation. 
+- QgsVectorLayerFeatureIterator::FetchJoinInfo::joinLayer was removed. This layer pointer was dangerous and not safe to access in any situation.
 
 QGIS 3.4        {#qgis_api_break_3_4}
 ========

diff --git a/src/core/qgsmaplayer.cpp b/src/core/qgsmaplayer.cpp
@@ -202,13 +202,14 @@ void QgsMapLayer::setMetadataUrl( const QString &metaUrl )
   QList<QgsMapLayerServerProperties::MetadataUrl> urls = serverProperties()->metadataUrls();
   if ( urls.isEmpty() )
   {
-    QgsMapLayerServerProperties::MetadataUrl newItem( metaUrl, QLatin1String(), QLatin1String() );
-    urls.insert( 0, newItem );
+    const QgsMapLayerServerProperties::MetadataUrl newItem = QgsMapLayerServerProperties::MetadataUrl( metaUrl, QLatin1String(), QLatin1String() );
+    urls.prepend( newItem );
   }
   else
   {
-    QgsMapLayerServerProperties::MetadataUrl old = urls.takeAt( 0 );
-    QgsMapLayerServerProperties::MetadataUrl newItem( metaUrl, old.type, old.format );
+    const QgsMapLayerServerProperties::MetadataUrl old = urls.takeFirst();
+    const QgsMapLayerServerProperties::MetadataUrl newItem( metaUrl, old.type, old.format );
+    urls.prepend( newItem );
   }
   serverProperties()->setMetadataUrls( urls );
 }
@@ -221,7 +222,7 @@ QString QgsMapLayer::metadataUrl() const
   }
   else
   {
-    return mServerProperties->metadataUrls().at( 0 ).url;
+    return mServerProperties->metadataUrls().first().url;
   }
 }
 
@@ -230,14 +231,14 @@ void QgsMapLayer::setMetadataUrlType( const QString &metaUrlType )
   QList<QgsMapLayerServerProperties::MetadataUrl> urls = mServerProperties->metadataUrls();
   if ( urls.isEmpty() )
   {
-    QgsMapLayerServerProperties::MetadataUrl newItem( QLatin1String(), metaUrlType, QLatin1String() );
-    urls.insert( 0, newItem );
+    const QgsMapLayerServerProperties::MetadataUrl newItem( QLatin1String(), metaUrlType, QLatin1String() );
+    urls.prepend( newItem );
   }
   else
   {
-    QgsMapLayerServerProperties::MetadataUrl old = urls.takeFirst();
-    QgsMapLayerServerProperties::MetadataUrl newItem( old.url, metaUrlType, old.format );
-    urls.insert( 0, newItem );
+    const QgsMapLayerServerProperties::MetadataUrl old = urls.takeFirst();
+    const QgsMapLayerServerProperties::MetadataUrl newItem( old.url, metaUrlType, old.format );
+    urls.prepend( newItem );
   }
   mServerProperties->setMetadataUrls( urls );
 }
@@ -250,7 +251,7 @@ QString QgsMapLayer::metadataUrlType() const
   }
   else
   {
-    return mServerProperties->metadataUrls().at( 0 ).type;
+    return mServerProperties->metadataUrls().first().type;
   }
 }
 
@@ -259,14 +260,14 @@ void QgsMapLayer::setMetadataUrlFormat( const QString &metaUrlFormat )
   QList<QgsMapLayerServerProperties::MetadataUrl> urls = mServerProperties->metadataUrls();
   if ( urls.isEmpty() )
   {
-    QgsMapLayerServerProperties::MetadataUrl newItem( QLatin1String(), QLatin1String(), metaUrlFormat );
-    urls.insert( 0, newItem );
+    const QgsMapLayerServerProperties::MetadataUrl newItem( QLatin1String(), QLatin1String(), metaUrlFormat );
+    urls.prepend( newItem );
   }
   else
   {
-    QgsMapLayerServerProperties::MetadataUrl old = urls.takeAt( 0 );
-    QgsMapLayerServerProperties::MetadataUrl newItem( old.url, old.type, metaUrlFormat );
-    urls.insert( 0, newItem );
+    const QgsMapLayerServerProperties::MetadataUrl old = urls.takeFirst( );
+    const QgsMapLayerServerProperties::MetadataUrl newItem( old.url, old.type, metaUrlFormat );
+    urls.prepend( newItem );
   }
   mServerProperties->setMetadataUrls( urls );
 }
@@ -279,7 +280,7 @@ QString QgsMapLayer::metadataUrlFormat() const
   }
   else
   {
-    return mServerProperties->metadataUrls().at( 0 ).format;
+    return mServerProperties->metadataUrls().first().format;
   }
 }
 
@@ -477,13 +478,13 @@ bool QgsMapLayer::readLayerXml( const QDomElement &layerElement, QgsReadWriteCon
   {
     // metadataUrl is still empty, maybe it's a QGIS Project < 3.22
     // keep for legacy
-    QDomElement metaUrlElem = layerElement.firstChildElement( QStringLiteral( "metadataUrl" ) );
+    const QDomElement metaUrlElem = layerElement.firstChildElement( QStringLiteral( "metadataUrl" ) );
     if ( !metaUrlElem.isNull() )
     {
-      QString url = metaUrlElem.text();
-      QString type = metaUrlElem.attribute( QStringLiteral( "type" ), QString() );
-      QString format = metaUrlElem.attribute( QStringLiteral( "format" ), QString() );
-      QgsMapLayerServerProperties::MetadataUrl newItem( url, type, format );
+      const QString url = metaUrlElem.text();
+      const QString type = metaUrlElem.attribute( QStringLiteral( "type" ), QString() );
+      const QString format = metaUrlElem.attribute( QStringLiteral( "format" ), QString() );
+      const QgsMapLayerServerProperties::MetadataUrl newItem( url, type, format );
       mServerProperties->setMetadataUrls( QList<QgsMapLayerServerProperties::MetadataUrl>() << newItem );
     }
   }

diff --git a/src/core/qgsmaplayerserverproperties.cpp b/src/core/qgsmaplayerserverproperties.cpp
@@ -130,7 +130,7 @@ const QList< QgsServerWmsDimensionProperties::WmsDimensionInfo > QgsServerWmsDim
 
 void QgsServerWmsDimensionProperties::readXml( const QDomNode &layer_node )
 {
-  QgsServerWmsDimensionProperties::reset();
+  reset();
 
   // Apply only for vector layers
   if ( layer()->type() != QgsMapLayerType::VectorLayer )
@@ -139,53 +139,54 @@ void QgsServerWmsDimensionProperties::readXml( const QDomNode &layer_node )
   const QgsFields fields = static_cast<const QgsVectorLayer *>( layer() )->fields();
   // QGIS Server WMS Dimensions
   const QDomNode wmsDimsNode = layer_node.namedItem( QStringLiteral( "wmsDimensions" ) );
-  if ( !wmsDimsNode.isNull() )
+  if ( wmsDimsNode.isNull() )
+  {
+    return;
+  }
+  const QDomElement wmsDimsElem = wmsDimsNode.toElement();
+  const QDomNodeList wmsDimsList = wmsDimsElem.elementsByTagName( QStringLiteral( "dimension" ) );
+  for ( int i = 0; i < wmsDimsList.size(); ++i )
   {
-    const QDomElement wmsDimsElem = wmsDimsNode.toElement();
-    QDomNodeList wmsDimsList = wmsDimsElem.elementsByTagName( QStringLiteral( "dimension" ) );
-    for ( int i = 0; i < wmsDimsList.size(); ++i )
+    const QDomElement dimElem = wmsDimsList.at( i ).toElement();
+    const QString dimName = dimElem.attribute( QStringLiteral( "name" ) );
+    const QString dimFieldName = dimElem.attribute( QStringLiteral( "fieldName" ) );
+    // check field name
+    const int dimFieldNameIndex = fields.indexOf( dimFieldName );
+    if ( dimFieldNameIndex == -1 )
     {
-      QDomElement dimElem = wmsDimsList.at( i ).toElement();
-      QString dimName = dimElem.attribute( QStringLiteral( "name" ) );
-      QString dimFieldName = dimElem.attribute( QStringLiteral( "fieldName" ) );
-      // check field name
-      int dimFieldNameIndex = fields.indexOf( dimFieldName );
-      if ( dimFieldNameIndex == -1 )
-      {
-        continue;
-      }
-      QVariant dimRefValue;
-      int dimDefaultDisplayType = dimElem.attribute( QStringLiteral( "defaultDisplayType" ) ).toInt();
-      if ( dimDefaultDisplayType == QgsServerWmsDimensionProperties::WmsDimensionInfo::AllValues )
+      continue;
+    }
+    QVariant dimRefValue;
+    const int dimDefaultDisplayType = dimElem.attribute( QStringLiteral( "defaultDisplayType" ) ).toInt();
+    if ( dimDefaultDisplayType == QgsServerWmsDimensionProperties::WmsDimensionInfo::AllValues )
+    {
+      const QString dimRefValueStr = dimElem.attribute( QStringLiteral( "referenceValue" ) );
+      if ( !dimRefValueStr.isEmpty() )
       {
-        QString dimRefValueStr = dimElem.attribute( QStringLiteral( "referenceValue" ) );
-        if ( !dimRefValueStr.isEmpty() )
+        const QgsField dimField = fields.at( dimFieldNameIndex );
+        dimRefValue = QVariant( dimRefValueStr );
+        if ( !dimField.convertCompatible( dimRefValue ) )
         {
-          QgsField dimField = fields.at( dimFieldNameIndex );
-          dimRefValue = QVariant( dimRefValueStr );
-          if ( !dimField.convertCompatible( dimRefValue ) )
-          {
-            continue;
-          }
+          continue;
         }
       }
-      QgsServerWmsDimensionProperties::WmsDimensionInfo dim( dimName, dimFieldName,
-          dimElem.attribute( QStringLiteral( "endFieldName" ) ),
-          dimElem.attribute( QStringLiteral( "units" ) ),
-          dimElem.attribute( QStringLiteral( "unitSymbol" ) ),
-          dimDefaultDisplayType, dimRefValue );
-      //XXX This add O(n^2) complexity !!!!
-      // addWmsDimension( dim );
-      // Better to trust the XML:
-      mWmsDimensions.append( dim );
     }
+    QgsServerWmsDimensionProperties::WmsDimensionInfo dim( dimName, dimFieldName,
+        dimElem.attribute( QStringLiteral( "endFieldName" ) ),
+        dimElem.attribute( QStringLiteral( "units" ) ),
+        dimElem.attribute( QStringLiteral( "unitSymbol" ) ),
+        dimDefaultDisplayType, dimRefValue );
+    //XXX This add O(n^2) complexity !!!!
+    // addWmsDimension( dim );
+    // Better to trust the XML:
+    mWmsDimensions.append( dim );
   }
 }
 
 void QgsServerWmsDimensionProperties::writeXml( QDomNode &layer_node, QDomDocument &document ) const
 {
   // save QGIS Server WMS Dimension definitions
-  if ( mWmsDimensions.size() > 0 )
+  if ( ! mWmsDimensions.isEmpty() )
   {
     QDomElement wmsDimsElem = document.createElement( QStringLiteral( "wmsDimensions" ) );
     for ( const QgsServerWmsDimensionProperties::WmsDimensionInfo &dim : mWmsDimensions )

diff --git a/src/gui/raster/qgsrasterlayerproperties.cpp b/src/gui/raster/qgsrasterlayerproperties.cpp
@@ -823,7 +823,7 @@ void QgsRasterLayerProperties::sync()
   const QList<QgsMapLayerServerProperties::MetadataUrl> &metaUrls = mRasterLayer->serverProperties()->metadataUrls();
   for ( const QgsMapLayerServerProperties::MetadataUrl &metaUrl : metaUrls )
   {
-    int row = mMetadataUrlModel->rowCount();
+    const int row = mMetadataUrlModel->rowCount();
     mMetadataUrlModel->setItem( row, 0, new QStandardItem( metaUrl.url ) );
     mMetadataUrlModel->setItem( row, 1, new QStandardItem( metaUrl.type ) );
     mMetadataUrlModel->setItem( row, 2, new QStandardItem( metaUrl.format ) );
@@ -1535,7 +1535,7 @@ QLinearGradient QgsRasterLayerProperties::highlightGradient()
 
 void QgsRasterLayerProperties::addMetadataUrl()
 {
-  int row = mMetadataUrlModel->rowCount();
+  const int row = mMetadataUrlModel->rowCount();
   mMetadataUrlModel->setItem( row, 0, new QStandardItem( QLatin1String() ) );
   mMetadataUrlModel->setItem( row, 1, new QStandardItem( QLatin1String() ) );
   mMetadataUrlModel->setItem( row, 2, new QStandardItem( QLatin1String() ) );

diff --git a/src/gui/vector/qgsvectorlayerproperties.cpp b/src/gui/vector/qgsvectorlayerproperties.cpp
@@ -362,7 +362,7 @@ QgsVectorLayerProperties::QgsVectorLayerProperties(
   const QList<QgsMapLayerServerProperties::MetadataUrl> &metaUrls = mLayer->serverProperties()->metadataUrls();
   for ( const QgsMapLayerServerProperties::MetadataUrl &metaUrl : metaUrls )
   {
-    int row = mMetadataUrlModel->rowCount();
+    const int row = mMetadataUrlModel->rowCount();
     mMetadataUrlModel->setItem( row, 0, new QStandardItem( metaUrl.url ) );
     mMetadataUrlModel->setItem( row, 1, new QStandardItem( metaUrl.type ) );
     mMetadataUrlModel->setItem( row, 2, new QStandardItem( metaUrl.format ) );
@@ -513,7 +513,7 @@ void QgsVectorLayerProperties::insertFieldOrExpression()
 
 void QgsVectorLayerProperties::addMetadataUrl()
 {
-  int row = mMetadataUrlModel->rowCount();
+  const int row = mMetadataUrlModel->rowCount();
   mMetadataUrlModel->setItem( row, 0, new QStandardItem( QLatin1String() ) );
   mMetadataUrlModel->setItem( row, 1, new QStandardItem( QLatin1String() ) );
   mMetadataUrlModel->setItem( row, 2, new QStandardItem( QLatin1String() ) );

diff --git a/tests/src/python/test_qgsvectorlayer.py b/tests/src/python/test_qgsvectorlayer.py
@@ -59,7 +59,6 @@
                        QgsMapLayerDependency,
                        QgsRenderContext,
                        QgsPalLayerSettings,
-                       # QgsVectorLayerServerProperties,
                        QgsVectorLayerSimpleLabeling,
                        QgsSingleCategoryDiagramRenderer,
                        QgsDiagramLayerSettings,
@@ -2920,8 +2919,6 @@ def testGetFeatureLimitWithEdits(self):
     def test_server_properties(self):
         """ Test server properties. """
         layer = QgsVectorLayer('Point?field=fldtxt:string', 'layer_1', 'memory')
-        # This should pass
-        # self.assertIsInstance(layer.serverProperties(), QgsVectorLayerServerProperties)
         self.assertIsInstance(layer.serverProperties(), QgsMapLayerServerProperties)
 
     def testClone(self):