Correct postgres prepared statements for arrays

Fixes #38784
qgis · Dec 29, 2020 · 4e125eb7f2725ba5c5ad4de81067d135602d1bcb · 4e125eb
1 parent 5c225cc
commit 4e125eb7f2725ba5c5ad4de81067d135602d1bcb
Unified Split

Showing with 134 additions and 1 deletion.

+19 −1 src/providers/postgres/qgspostgresprovider.cpp

+115 −0 tests/src/gui/testqgslistwidget.cpp
diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp
@@ -2531,7 +2531,25 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist, Flags flags )
         }
         else
         {
-          v = paramValue( value.toString(), defaultValues[ i ] );
+          // the conversion functions expects the list as a string, so convert it
+          if ( value.type() == QVariant::StringList )
+          {
+            QStringList list_vals = value.toStringList();
+            // all strings need to be double quoted to allow special postgres
+            // array characters such as {, or whitespace in the string
+            // but we need to escape all double quotes and backslashes
+            list_vals.replaceInStrings( "\\", "\\\\" );
+            list_vals.replaceInStrings( "\"", "\\\"" );
+            v = QStringLiteral( "{\"" ) + value.toStringList().join( QStringLiteral( "\",\"" ) ) + QStringLiteral( "\"}" );
+          }
+          else if ( value.type() == QVariant::List )
+          {
+            v = "{" + value.toStringList().join( "," ) + "}";
+          }
+          else
+          {
+            v = paramValue( value.toString(), defaultValues[ i ] );
+          }
 
           if ( v != value.toString() )
           {

diff --git a/tests/src/gui/testqgslistwidget.cpp b/tests/src/gui/testqgslistwidget.cpp
@@ -20,19 +20,38 @@
 #include <qgslistwidget.h>
 #include <editorwidgets/core/qgseditorwidgetwrapper.h>
 #include <qgsapplication.h>
+#include <qgsvectorlayer.h>
+#include <editorwidgets/qgslistwidgetwrapper.h>
 
 class TestQgsListWidget : public QObject
 {
     Q_OBJECT
+  private:
+    QString dbConn;
   private slots:
     void initTestCase() // will be called before the first testfunction is executed.
     {
       QgsApplication::init();
       QgsApplication::initQgis();
+      dbConn = getenv( "QGIS_PGTEST_DB" );
+      if ( dbConn.isEmpty() )
+      {
+        dbConn = "service=\"qgis_test\"";
+      }
     }
 
     void cleanupTestCase() // will be called after the last testfunction was executed.
     {
+      // delete new features in db from postgres test
+      QgsVectorLayer *vl_array_int = new QgsVectorLayer( QStringLiteral( "%1 sslmode=disable key=\"pk\" table=\"qgis_test\".\"array_tbl\" sql=" ).arg( dbConn ), QStringLiteral( "json" ), QStringLiteral( "postgres" ) );
+      vl_array_int->startEditing( );
+      QgsFeatureIds delete_ids = QSet<QgsFeatureId>() << Q_INT64_C( 997 ) << Q_INT64_C( 998 ) << Q_INT64_C( 999 );
+      vl_array_int->deleteFeatures( delete_ids );
+      vl_array_int->commitChanges( false );
+      QgsVectorLayer *vl_array_str = new QgsVectorLayer( QStringLiteral( "%1 sslmode=disable key=\"pk\" table=\"qgis_test\".\"string_array\" sql=" ).arg( dbConn ), QStringLiteral( "json" ), QStringLiteral( "postgres" ) );
+      vl_array_str->startEditing( );
+      vl_array_str->deleteFeatures( delete_ids );
+      vl_array_str->commitChanges( false );
       QgsApplication::exitQgis();
     }
 
@@ -120,6 +139,102 @@ class TestQgsListWidget : public QObject
       QCOMPARE( wrapper->value().toList(), expected );
       QVERIFY( widget->valid() );
     }
+
+    void testPostgres()
+    {
+      //create pg layers
+      QgsVectorLayer *vl_array_int = new QgsVectorLayer( QStringLiteral( "%1 sslmode=disable key=\"pk\" table=\"qgis_test\".\"array_tbl\" sql=" ).arg( dbConn ), QStringLiteral( "json" ), QStringLiteral( "postgres" ) );
+      QVERIFY( vl_array_int->isValid( ) );
+
+      QgsListWidgetWrapper w_array_int( vl_array_int, vl_array_int->fields().indexOf( QStringLiteral( "location" ) ), nullptr, nullptr );
+      QgsListWidget *widget = qobject_cast< QgsListWidget * >( w_array_int.widget( ) );
+
+      vl_array_int->startEditing( );
+      QVariantList newList;
+      newList.append( QStringLiteral( "100" ) );
+      widget->setList( QList<QVariant>() << 100 );
+      QVERIFY( w_array_int.value( ).isValid( ) );
+      QCOMPARE( widget->list( ), QList<QVariant>( ) << 100 );
+      // save value and check it is saved properly in postges
+      QgsFeature new_rec_997 {vl_array_int->fields(), 997};
+      new_rec_997.setAttribute( 0, QVariant( 997 ) );
+      vl_array_int->addFeature( new_rec_997, QgsFeatureSink::RollBackOnErrors );
+      vl_array_int->commitChanges( false );
+      bool success = vl_array_int->changeAttributeValue( 997, 1, w_array_int.value(), QVariant(), false );
+      QVERIFY( success );
+      success = vl_array_int->commitChanges( false );
+      QVERIFY( success );
+
+      w_array_int.setFeature( vl_array_int->getFeature( 997 ) );
+      QCOMPARE( widget->list( ), QList<QVariant>( ) << 100 );
+
+      // alter two values at a time which triggered old bug (#38784)
+      widget->setList( QList<QVariant>() << 4 << 5 << 6 );
+      QgsFeature new_rec_998 {vl_array_int->fields(), 998};
+      new_rec_998.setAttribute( 0, QVariant( 998 ) );
+      new_rec_998.setAttribute( 1, w_array_int.value() );
+      vl_array_int->addFeature( new_rec_998, QgsFeatureSink::RollBackOnErrors );
+
+      widget->setList( QList<QVariant>() << 10 << 11 << 12 );
+      QgsFeature new_rec_999 {vl_array_int->fields(), 999};
+      new_rec_999.setAttribute( 0, QVariant( 999 ) );
+      new_rec_999.setAttribute( 1, w_array_int.value() );
+      vl_array_int->addFeature( new_rec_999, QgsFeatureSink::RollBackOnErrors );
+      vl_array_int->commitChanges( false );
+
+      w_array_int.setFeature( vl_array_int->getFeature( 998 ) );
+      QCOMPARE( widget->list( ), QList<QVariant>( ) << 4 << 5 << 6 );
+
+      w_array_int.setFeature( vl_array_int->getFeature( 999 ) );
+      QCOMPARE( widget->list( ), QList<QVariant>() << 10 << 11 << 12 );
+
+      // do similar for array of strings
+      QgsVectorLayer *vl_array_str = new QgsVectorLayer( QStringLiteral( "%1 sslmode=disable key=\"pk\" table=\"qgis_test\".\"string_array\" sql=" ).arg( dbConn ), QStringLiteral( "json" ), QStringLiteral( "postgres" ) );
+      QVERIFY( vl_array_str->isValid() );
+
+      QgsListWidgetWrapper w_array_str( vl_array_str, vl_array_str->fields().indexOf( QStringLiteral( "value" ) ), nullptr, nullptr );
+      widget = qobject_cast< QgsListWidget * >( w_array_str.widget( ) );
+      vl_array_str->startEditing( );
+      QVariantList newListStr;
+
+      // test quotes
+      newListStr.append( QStringLiteral( "10\"0" ) );
+      widget->setList( newListStr );
+      QVERIFY( w_array_str.value().isValid() );
+      QCOMPARE( widget->list( ), QList<QVariant>( ) << QStringLiteral( "10\"0" ) );
+      // save value and check it is saved properly in postges
+      QgsFeature new_rec_997_str {vl_array_str->fields(), 997};
+      new_rec_997_str.setAttribute( 0, QVariant( 997 ) );
+      vl_array_str->addFeature( new_rec_997_str, QgsFeatureSink::RollBackOnErrors );
+      vl_array_str->commitChanges( false );
+      success = vl_array_str->changeAttributeValue( 997, 1, w_array_str.value(), QVariant(), false );
+      QVERIFY( success );
+      success = vl_array_str->commitChanges( false );
+      QVERIFY( success );
+
+      w_array_str.setFeature( vl_array_str->getFeature( 997 ) );
+      QCOMPARE( widget->list( ), QList<QVariant>( ) << QStringLiteral( "10\"0" ) );
+
+      // alter two values at a time which triggered old bug (#38784)
+      widget->setList( QList<QVariant>() << QStringLiteral( "four" ) << QStringLiteral( "five" ) << QStringLiteral( "six" ) );
+      QgsFeature new_rec_998_str {vl_array_str->fields(), 998};
+      new_rec_998_str.setAttribute( 0, QVariant( 998 ) );
+      new_rec_998_str.setAttribute( 1, w_array_str.value() );
+      vl_array_str->addFeature( new_rec_998_str, QgsFeatureSink::RollBackOnErrors );
+
+      widget->setList( QList<QVariant>() << QStringLiteral( "ten" ) << QStringLiteral( "eleven" ) << QStringLiteral( "twelve" ) );
+      QgsFeature new_rec_999_str {vl_array_str->fields(), 999};
+      new_rec_999_str.setAttribute( 0, QVariant( 999 ) );
+      new_rec_999_str.setAttribute( 1, w_array_str.value() );
+      vl_array_str->addFeature( new_rec_999_str, QgsFeatureSink::RollBackOnErrors );
+      vl_array_str->commitChanges( false );
+
+      w_array_str.setFeature( vl_array_str->getFeature( 998 ) );
+      QCOMPARE( widget->list( ), QList<QVariant>() << QStringLiteral( "four" ) << QStringLiteral( "five" ) << QStringLiteral( "six" ) );
+
+      w_array_str.setFeature( vl_array_str->getFeature( 999 ) );
+      QCOMPARE( widget->list( ), QList<QVariant>() << QStringLiteral( "ten" ) << QStringLiteral( "eleven" ) << QStringLiteral( "twelve" ) );
+    }
 };
 
 QGSTEST_MAIN( TestQgsListWidget )