Skip to content

Commit

Permalink
Correct postgres prepared statements for arrays
Browse files Browse the repository at this point in the history
Fixes #38784
  • Loading branch information
stev-0 authored and nyalldawson committed Dec 29, 2020
1 parent 5c225cc commit 4e125eb
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 1 deletion.
20 changes: 19 additions & 1 deletion src/providers/postgres/qgspostgresprovider.cpp
Expand Up @@ -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() )
{
Expand Down
115 changes: 115 additions & 0 deletions tests/src/gui/testqgslistwidget.cpp
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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 )
Expand Down

0 comments on commit 4e125eb

Please sign in to comment.