Fix creating auxilary field fails if _ present in field name

qgis · Jul 16, 2019 · 1e4114a · 1e4114a
1 parent 943cbd0
commit 1e4114a
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 22 deletions.
diff --git a/src/core/qgsauxiliarystorage.cpp b/src/core/qgsauxiliarystorage.cpp
@@ -288,7 +288,7 @@ bool QgsAuxiliaryLayer::isHiddenProperty( int index ) const
   bool hidden = false;
   QgsPropertyDefinition def = propertyDefinitionFromIndex( index );
 
-  if ( def.origin().compare( "labeling" ) == 0 )
+  if ( def.origin().compare( QLatin1String( "labeling" ) ) == 0 )
   {
     for ( const QgsPalLayerSettings::Property &p : palHiddenProperties )
     {
@@ -367,13 +367,13 @@ QString QgsAuxiliaryLayer::nameFromProperty( const QgsPropertyDefinition &def, b
   QString fieldName = def.origin();
 
   if ( !def.name().isEmpty() )
-    fieldName =  QString( "%1_%2" ).arg( fieldName, def.name().toLower() );
+    fieldName =  QStringLiteral( "%1_%2" ).arg( fieldName, def.name().toLower() );
 
   if ( !def.comment().isEmpty() )
-    fieldName = QString( "%1_%2" ).arg( fieldName, def.comment() );
+    fieldName = QStringLiteral( "%1_%2" ).arg( fieldName, def.comment() );
 
   if ( joined )
-    fieldName = QString( "%1%2" ).arg( AS_JOINPREFIX, fieldName );
+    fieldName = QStringLiteral( "%1%2" ).arg( AS_JOINPREFIX, fieldName );
 
   return fieldName;
 }
@@ -392,17 +392,17 @@ QgsField QgsAuxiliaryLayer::createAuxiliaryField( const QgsPropertyDefinition &d
       case QgsPropertyDefinition::DataTypeString:
         type = QVariant::String;
         len = 50;
-        typeName = "String";
+        typeName = QStringLiteral( "String" );
         break;
       case QgsPropertyDefinition::DataTypeNumeric:
         type = QVariant::Double;
         len = 0;
         precision = 0;
-        typeName = "Real";
+        typeName = QStringLiteral( "Real" );
         break;
       case QgsPropertyDefinition::DataTypeBoolean:
         type = QVariant::Int; // sqlite does not have a bool type
-        typeName = "Integer";
+        typeName = QStringLiteral( "Integer" );
         break;
     }
 
@@ -427,44 +427,44 @@ QgsPropertyDefinition QgsAuxiliaryLayer::propertyDefinitionFromField( const QgsF
   const QString origin = parts[0];
   const QString propertyName = parts[1];
 
-  if ( origin.compare( "labeling", Qt::CaseInsensitive ) == 0 )
+  if ( origin.compare( QLatin1String( "labeling" ), Qt::CaseInsensitive ) == 0 )
   {
     const QgsPropertiesDefinition props = QgsPalLayerSettings::propertyDefinitions();
     for ( auto it = props.constBegin(); it != props.constEnd(); ++it )
     {
       if ( it.value().name().compare( propertyName, Qt::CaseInsensitive ) == 0 )
       {
         def = it.value();
-        if ( parts.size() == 3 )
-          def.setComment( parts[2] );
+        if ( parts.size() >= 3 )
+          def.setComment( parts.mid( 2 ).join( '_' ) );
         break;
       }
     }
   }
-  else if ( origin.compare( "symbol", Qt::CaseInsensitive ) == 0 )
+  else if ( origin.compare( QLatin1String( "symbol" ), Qt::CaseInsensitive ) == 0 )
   {
     const QgsPropertiesDefinition props = QgsSymbolLayer::propertyDefinitions();
     for ( auto it = props.constBegin(); it != props.constEnd(); ++it )
     {
       if ( it.value().name().compare( propertyName, Qt::CaseInsensitive ) == 0 )
       {
         def = it.value();
-        if ( parts.size() == 3 )
-          def.setComment( parts[2] );
+        if ( parts.size() >= 3 )
+          def.setComment( parts.mid( 2 ).join( '_' ) );
         break;
       }
     }
   }
-  else if ( origin.compare( "diagram", Qt::CaseInsensitive ) == 0 )
+  else if ( origin.compare( QLatin1String( "diagram" ), Qt::CaseInsensitive ) == 0 )
   {
     const QgsPropertiesDefinition props = QgsDiagramLayerSettings::propertyDefinitions();
     for ( auto it = props.constBegin(); it != props.constEnd(); ++it )
     {
       if ( it.value().name().compare( propertyName, Qt::CaseInsensitive ) == 0 )
       {
         def = it.value();
-        if ( parts.size() == 3 )
-          def.setComment( parts[2] );
+        if ( parts.size() >= 3 )
+          def.setComment( parts.mid( 2 ).join( '_' ) );
         break;
       }
     }
@@ -473,9 +473,24 @@ QgsPropertyDefinition QgsAuxiliaryLayer::propertyDefinitionFromField( const QgsF
   {
     def.setOrigin( origin );
     def.setName( propertyName );
+    switch ( f.type() )
+    {
+      case QVariant::Double:
+        def.setDataType( QgsPropertyDefinition::DataTypeNumeric );
+        break;
+
+      case QVariant::Bool:
+        def.setDataType( QgsPropertyDefinition::DataTypeBoolean );
+        break;
 
-    if ( parts.size() == 3 )
-      def.setComment( parts[2] );
+      case QVariant::String:
+      default:
+        def.setDataType( QgsPropertyDefinition::DataTypeString );
+        break;
+    }
+
+    if ( parts.size() >= 3 )
+      def.setComment( parts.mid( 2 ).join( '_' ) );
   }
 
   return def;
@@ -596,7 +611,7 @@ bool QgsAuxiliaryStorage::deleteTable( const QgsDataSourceUri &ogrUri )
 
     if ( database )
     {
-      QString sql = QString( "DROP TABLE %1" ).arg( uri.table() );
+      QString sql = QStringLiteral( "DROP TABLE %1" ).arg( uri.table() );
       rc = exec( sql, database.get() );
 
       sql = QStringLiteral( "VACUUM" );
@@ -733,7 +748,7 @@ spatialite_database_unique_ptr QgsAuxiliaryStorage::openDB( const QString &filen
 
   if ( rc )
   {
-    debugMsg( "sqlite3_open_v2", database.get() );
+    debugMsg( QStringLiteral( "sqlite3_open_v2" ), database.get() );
   }
 
   return database;
@@ -823,7 +838,7 @@ QgsDataSourceUri QgsAuxiliaryStorage::parseOgrUri( const QgsDataSourceUri &uri )
   if ( uriParts.count() < 2 )
     return newUri;
 
-  const QString databasePath = uriParts[0].replace( ' ', "" );
+  const QString databasePath = uriParts[0].replace( ' ', QString() );
 
   const QString table = uriParts[1];
   QStringList tableParts = table.split( ' ' );

diff --git a/tests/src/python/test_qgsauxiliarystorage.py b/tests/src/python/test_qgsauxiliarystorage.py
@@ -14,7 +14,7 @@
 
 import os
 
-from qgis.PyQt.QtCore import QTemporaryFile
+from qgis.PyQt.QtCore import QTemporaryFile, QVariant
 from qgis.core import (QgsAuxiliaryStorage,
                        QgsAuxiliaryLayer,
                        QgsVectorLayer,
@@ -403,6 +403,39 @@ def testCreateProperty(self):
         afIndex = vl.fields().indexOf(afName)
         self.assertEqual(index, afIndex)
 
+    def testCreateField(self):
+        s = QgsAuxiliaryStorage()
+        self.assertTrue(s.isValid())
+
+        # Create a new auxiliary layer with 'pk' as key
+        vl = createLayer()
+        pkf = vl.fields().field(vl.fields().indexOf('pk'))
+        al = s.createAuxiliaryLayer(pkf, vl)
+        self.assertTrue(al.isValid())
+        vl.setAuxiliaryLayer(al)
+
+        prop = QgsPropertyDefinition()
+        prop.setComment('test_field')
+        prop.setDataType(QgsPropertyDefinition.DataTypeNumeric)
+        prop.setOrigin('user')
+        prop.setName('custom')
+        self.assertTrue(al.addAuxiliaryField(prop))
+
+        prop = QgsPropertyDefinition()
+        prop.setComment('test_field_string')
+        prop.setDataType(QgsPropertyDefinition.DataTypeString)
+        prop.setOrigin('user')
+        prop.setName('custom')
+        self.assertTrue(al.addAuxiliaryField(prop))
+
+        self.assertEqual(len(al.auxiliaryFields()), 2)
+        self.assertEqual(al.auxiliaryFields()[0].name(), 'user_custom_test_field')
+        self.assertEqual(al.auxiliaryFields()[0].type(), QVariant.Double)
+        self.assertEqual(al.auxiliaryFields()[0].typeName(), 'Real')
+        self.assertEqual(al.auxiliaryFields()[1].name(), 'user_custom_test_field_string')
+        self.assertEqual(al.auxiliaryFields()[1].type(), QVariant.String)
+        self.assertEqual(al.auxiliaryFields()[1].typeName(), 'String')
+
     def testQgdCreation(self):
         # New project
         p = QgsProject()