diff --git a/src/core/qgsexpression.cpp b/src/core/qgsexpression.cpp index a18f9bc4ccf..b0a5094a55a 100644 --- a/src/core/qgsexpression.cpp +++ b/src/core/qgsexpression.cpp @@ -4978,6 +4978,8 @@ QString QgsExpression::group( const QString& name ) QString QgsExpression::formatPreviewString( const QVariant& value ) { + static const int MAX_PREVIEW = 60; + if ( value.canConvert() ) { //result is a geometry @@ -5017,15 +5019,31 @@ QString QgsExpression::formatPreviewString( const QVariant& value ) else if ( value.type() == QVariant::String ) { QString previewString = value.toString(); - if ( previewString.length() > 63 ) + if ( previewString.length() > MAX_PREVIEW + 3 ) { - return QString( tr( "'%1...'" ) ).arg( previewString.left( 60 ) ); + return QString( tr( "'%1...'" ) ).arg( previewString.left( MAX_PREVIEW ) ); } else { return previewString.prepend( '\'' ).append( '\'' ); } } + else if ( value.type() == QVariant::Map ) + { + QString mapStr; + const QVariantMap map = value.toMap(); + for ( QVariantMap::const_iterator it = map.constBegin(); it != map.constEnd(); ++it ) + { + if ( !mapStr.isEmpty() ) mapStr.append( ", " ); + mapStr.append( it.key() ).append( ": " ).append( formatPreviewString( it.value() ) ); + if ( mapStr.length() > MAX_PREVIEW + 3 ) + { + mapStr = QString( tr( "%1..." ) ).arg( mapStr.left( MAX_PREVIEW ) ); + break; + } + } + return tr( "<map: %1>" ).arg( mapStr ); + } else { return value.toString(); diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp index bc2bd826bfc..2f14a1b7497 100644 --- a/src/core/qgsvectorlayer.cpp +++ b/src/core/qgsvectorlayer.cpp @@ -2233,7 +2233,10 @@ bool QgsVectorLayer::deleteFeature( QgsFeatureId fid ) bool QgsVectorLayer::deleteFeatures( const QgsFeatureIds& fids ) { if ( !mEditBuffer ) + { + QgsDebugMsg( "Cannot delete features (mEditBuffer==NULL)" ); return false; + } bool res = mEditBuffer->deleteFeatures( fids ); diff --git a/src/core/qgsvectorlayereditbuffer.cpp b/src/core/qgsvectorlayereditbuffer.cpp index 048609b21a2..e339262eb36 100644 --- a/src/core/qgsvectorlayereditbuffer.cpp +++ b/src/core/qgsvectorlayereditbuffer.cpp @@ -20,7 +20,6 @@ #include "qgsvectordataprovider.h" #include "qgsvectorlayer.h" - //! populate two lists (ks, vs) from map - in reverse order template void mapToReversedLists( const QMap< Key, T >& map, QList& ks, QList& vs ) { @@ -150,17 +149,26 @@ bool QgsVectorLayerEditBuffer::addFeatures( QgsFeatureList& features ) bool QgsVectorLayerEditBuffer::deleteFeature( QgsFeatureId fid ) { if ( !( L->dataProvider()->capabilities() & QgsVectorDataProvider::DeleteFeatures ) ) + { + QgsDebugMsg( "Cannot delete features (missing DeleteFeature capability)" ); return false; + } if ( FID_IS_NEW( fid ) ) { if ( !mAddedFeatures.contains( fid ) ) + { + QgsDebugMsg( "Cannot delete features (in the list of added features)" ); return false; + } } else // existing feature { if ( mDeletedFeatureIds.contains( fid ) ) + { + QgsDebugMsg( "Cannot delete features (in the list of deleted features)" ); return false; + } } L->undoStack()->push( new QgsVectorLayerUndoCommandDeleteFeature( this, fid ) ); @@ -170,12 +178,16 @@ bool QgsVectorLayerEditBuffer::deleteFeature( QgsFeatureId fid ) bool QgsVectorLayerEditBuffer::deleteFeatures( const QgsFeatureIds& fids ) { if ( !( L->dataProvider()->capabilities() & QgsVectorDataProvider::DeleteFeatures ) ) + { + QgsDebugMsg( "Cannot delete features (missing DeleteFeatures capability)" ); return false; + } + bool ok = true; Q_FOREACH ( QgsFeatureId fid, fids ) - deleteFeature( fid ); + ok = deleteFeature( fid ) && ok; - return true; + return ok; } diff --git a/src/providers/postgres/CMakeLists.txt b/src/providers/postgres/CMakeLists.txt index 24f1ddc8972..eb180b238cc 100644 --- a/src/providers/postgres/CMakeLists.txt +++ b/src/providers/postgres/CMakeLists.txt @@ -56,8 +56,14 @@ INCLUDE_DIRECTORIES( ${CMAKE_CURRENT_BINARY_DIR}/../../ui ) +ADD_LIBRARY (postgresprovider_a STATIC ${PG_SRCS} ${PG_HDRS} ${PG_MOC_SRCS}) ADD_LIBRARY (postgresprovider MODULE ${PG_SRCS} ${PG_HDRS} ${PG_MOC_SRCS}) +TARGET_LINK_LIBRARIES (postgresprovider_a + ${POSTGRES_LIBRARY} + qgis_core + qgis_gui +) TARGET_LINK_LIBRARIES (postgresprovider ${POSTGRES_LIBRARY} qgis_core diff --git a/src/providers/postgres/qgspostgresconn.cpp b/src/providers/postgres/qgspostgresconn.cpp index 32ad429448e..d879f07778a 100644 --- a/src/providers/postgres/qgspostgresconn.cpp +++ b/src/providers/postgres/qgspostgresconn.cpp @@ -923,11 +923,42 @@ QString QgsPostgresConn::postgisVersion() return mPostgisVersionInfo; } -QString QgsPostgresConn::quotedIdentifier( QString ident ) +QString QgsPostgresConn::quotedIdentifier( const QString& ident ) { - ident.replace( '"', "\"\"" ); - ident = ident.prepend( '\"' ).append( '\"' ); - return ident; + QString result = ident; + result.replace( '"', "\"\"" ); + return result.prepend( '\"' ).append( '\"' ); +} + +static QString quotedString( const QString& v ) +{ + QString result = v; + result.replace( '\'', "''" ); + if ( result.contains( '\\' ) ) + return result.replace( '\\', "\\\\" ).prepend( "E'" ).append( '\'' ); + else + return result.prepend( '\'' ).append( '\'' ); +} + +static QString doubleQuotedMapValue( const QString& v ) +{ + QString result = v; + return "\"" + result.replace( '\\', "\\\\\\\\" ).replace( '\"', "\\\\\"" ).replace( '\'', "\\'" ) + "\""; +} + +static QString quotedMap( const QVariantMap& map ) +{ + QString ret; + for ( QVariantMap::const_iterator i = map.constBegin(); i != map.constEnd(); ++i ) + { + if ( !ret.isEmpty() ) + { + ret += ","; + } + ret.append( doubleQuotedMapValue( i.key() ) + "=>" + + doubleQuotedMapValue( i.value().toString() ) ); + } + return "E'" + ret + "'::hstore"; } QString QgsPostgresConn::quotedValue( const QVariant& value ) @@ -945,14 +976,12 @@ QString QgsPostgresConn::quotedValue( const QVariant& value ) case QVariant::Bool: return value.toBool() ? "TRUE" : "FALSE"; - default: + case QVariant::Map: + return quotedMap( value.toMap() ); + case QVariant::String: - QString v = value.toString(); - v.replace( '\'', "''" ); - if ( v.contains( '\\' ) ) - return v.replace( '\\', "\\\\" ).prepend( "E'" ).append( '\'' ); - else - return v.prepend( '\'' ).append( '\'' ); + default: + return quotedString( value.toString() ); } } diff --git a/src/providers/postgres/qgspostgresconn.h b/src/providers/postgres/qgspostgresconn.h index 6afe3ce3ad4..2594e87ffdc 100644 --- a/src/providers/postgres/qgspostgresconn.h +++ b/src/providers/postgres/qgspostgresconn.h @@ -263,7 +263,7 @@ class QgsPostgresConn : public QObject /** Double quote a PostgreSQL identifier for placement in a SQL string. */ - static QString quotedIdentifier( QString ident ); + static QString quotedIdentifier( const QString& ident ); /** Quote a value for placement in a SQL string. */ diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp index 8301c0aaf1b..ae9d2f0499b 100644 --- a/src/providers/postgres/qgspostgresprovider.cpp +++ b/src/providers/postgres/qgspostgresprovider.cpp @@ -218,6 +218,9 @@ QgsPostgresProvider::QgsPostgresProvider( QString const & uri ) << QgsVectorDataProvider::NativeType( tr( "Date" ), "date", QVariant::Date, -1, -1, -1, -1 ) << QgsVectorDataProvider::NativeType( tr( "Time" ), "time", QVariant::Time, -1, -1, -1, -1 ) << QgsVectorDataProvider::NativeType( tr( "Date & Time" ), "timestamp without time zone", QVariant::DateTime, -1, -1, -1, -1 ) + + // complex types + << QgsVectorDataProvider::NativeType( tr( "Map" ), "hstore", QVariant::Map, -1, -1, -1, -1 ) ; QString key; @@ -382,6 +385,9 @@ static bool operator<( const QVariant &a, const QVariant &b ) return al[i] < bl[i]; } + case QVariant::Map: + return a.toMap() < b.toMap(); + case QVariant::Date: return a.toDate() < b.toDate(); @@ -966,7 +972,6 @@ bool QgsPostgresProvider::loadFields() else if ( fieldTypeName == "text" || fieldTypeName == "bool" || fieldTypeName == "geometry" || - fieldTypeName == "hstore" || fieldTypeName == "inet" || fieldTypeName == "money" || fieldTypeName == "ltree" || @@ -1017,6 +1022,11 @@ bool QgsPostgresProvider::loadFields() fieldPrec = -1; } } + else if ( fieldTypeName == "hstore" ) + { + fieldType = QVariant::Map; + fieldSize = -1; + } else { QgsMessageLog::logMessage( tr( "Field %1 ignored, because of unsupported type %2" ).arg( fieldName, fieldTypeName ), tr( "PostGIS" ) ); @@ -2149,7 +2159,10 @@ bool QgsPostgresProvider::deleteFeatures( const QgsFeatureIds & id ) bool returnvalue = true; if ( mIsQuery ) + { + QgsDebugMsg( "Cannot delete features (is a query)" ); return false; + } QgsPostgresConn* conn = connectionRW(); if ( !conn ) @@ -3467,6 +3480,11 @@ bool QgsPostgresProvider::convertField( QgsField &field, const QMap 0 ) { @@ -3829,6 +3847,44 @@ QString QgsPostgresProvider::description() const return tr( "PostgreSQL/PostGIS provider\n%1\nPostGIS %2" ).arg( pgVersion, postgisVersion ); } // QgsPostgresProvider::description() + +static QVariant parseHstore( const QString& value ) +{ + QRegExp recordSep( "\\s*,\\s*" ); + QRegExp valueExtractor( "^(?:\"((?:\\.|.)*)\"|((?:\\.|.)*))\\s*=>\\s*(?:\"((?:\\.|.)*)\"|((?:\\.|.)*))$" ); + QVariantMap result; + Q_FOREACH ( QString record, value.split( recordSep ) ) + { + if ( valueExtractor.exactMatch( record ) ) + { + QString key = valueExtractor.cap( 1 ) + valueExtractor.cap( 2 ); + key.replace( "\\\"", "\"" ).replace( "\\\\", "\\" ); + QString value = valueExtractor.cap( 3 ) + valueExtractor.cap( 4 ); + value.replace( "\\\"", "\"" ).replace( "\\\\", "\\" ); + result.insert( key, value ); + } + else + { + QgsLogger::warning( "Error parsing hstore record: " + record ); + } + } + return result; +} + +QVariant QgsPostgresProvider::convertValue( QVariant::Type type, const QString& value ) +{ + if ( type == QVariant::Map ) + { + return parseHstore( value ); + } + QVariant v( value ); + + if ( !v.convert( type ) || value.isNull() ) + v = QVariant( type ); + + return v; +} + /** * Class factory to return a pointer to a newly created * QgsPostgresProvider object diff --git a/src/providers/postgres/qgspostgresprovider.h b/src/providers/postgres/qgspostgresprovider.h index ef696a3f51f..23e2e3b1c05 100644 --- a/src/providers/postgres/qgspostgresprovider.h +++ b/src/providers/postgres/qgspostgresprovider.h @@ -249,6 +249,14 @@ class QgsPostgresProvider : public QgsVectorDataProvider */ virtual QgsTransaction* transaction() const override; + /** + * Convert the postgres string representation into the given QVariant type. + * @param type the wanted type + * @param value the value to convert + * @return a QVariant of the given type or a null QVariant + */ + static QVariant convertValue( QVariant::Type type, const QString& value ); + signals: /** * This is emitted whenever the worker thread has fully calculated the diff --git a/tests/src/core/testqgsexpression.cpp b/tests/src/core/testqgsexpression.cpp index a9da5c69750..061a341049d 100644 --- a/tests/src/core/testqgsexpression.cpp +++ b/tests/src/core/testqgsexpression.cpp @@ -2331,6 +2331,19 @@ class TestQgsExpression: public QObject QCOMPARE( result.toString(), QString( "f2" ) ); } + void test_formatPreviewString() + { + QCOMPARE( QgsExpression::formatPreviewString( QVariant( "hello" ) ), QString( "'hello'" ) ); + QCOMPARE( QgsExpression::formatPreviewString( QVariant( QVariantMap() ) ), QString( "<map: >" ) ); + + QVariantMap map; + map["1"] = "One"; + map["2"] = "Two"; + QCOMPARE( QgsExpression::formatPreviewString( QVariant( map ) ), QString( "<map: 1: 'One', 2: 'Two'>" ) ); + map["3"] = "A very long string that is going to be truncated"; + QCOMPARE( QgsExpression::formatPreviewString( QVariant( map ) ), QString( "<map: 1: 'One', 2: 'Two', 3: 'A very long string that is going to ...>" ) ); + } + }; QTEST_MAIN( TestQgsExpression ) diff --git a/tests/src/providers/CMakeLists.txt b/tests/src/providers/CMakeLists.txt index 7e8b2e9ba0e..b8c032011c9 100644 --- a/tests/src/providers/CMakeLists.txt +++ b/tests/src/providers/CMakeLists.txt @@ -10,6 +10,7 @@ INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_SOURCE_DIR}/src/core/geometry ${CMAKE_SOURCE_DIR}/src/core/raster ${CMAKE_SOURCE_DIR}/src/providers/wms + ${CMAKE_SOURCE_DIR}/src/providers/postgres ) INCLUDE_DIRECTORIES(SYSTEM ${QT_INCLUDE_DIR} @@ -17,6 +18,7 @@ INCLUDE_DIRECTORIES(SYSTEM ${PROJ_INCLUDE_DIR} ${GEOS_INCLUDE_DIR} ${QCA_INCLUDE_DIR} + ${POSTGRES_INCLUDE_DIR} ) ############################################################# @@ -93,6 +95,12 @@ ADD_QGIS_TEST(wmsprovidertest testqgswmsprovider.cpp) TARGET_LINK_LIBRARIES(qgis_wmsprovidertest wmsprovider_a) +ADD_QGIS_TEST(postgresprovidertest testqgspostgresprovider.cpp) +TARGET_LINK_LIBRARIES(qgis_postgresprovidertest postgresprovider_a) + +ADD_QGIS_TEST(postgresconntest testqgspostgresconn.cpp) +TARGET_LINK_LIBRARIES(qgis_postgresconntest postgresprovider_a) + ############################################################# # WCS public servers test: # No need to test on all platforms diff --git a/tests/src/providers/testqgspostgresconn.cpp b/tests/src/providers/testqgspostgresconn.cpp new file mode 100644 index 00000000000..b7c9dfaaf39 --- /dev/null +++ b/tests/src/providers/testqgspostgresconn.cpp @@ -0,0 +1,30 @@ +#include +#include + +#include + +class TestQgsPostgresConn: public QObject +{ + Q_OBJECT + private slots: + void quotedValueHstore() + { + QVariantMap map; + map["1"] = "2"; + map["a"] = "b \"c' \\x"; + + const QString actual = QgsPostgresConn::quotedValue( map ); + QCOMPARE( actual, QString( "E'\"1\"=>\"2\",\"a\"=>\"b \\\\\"c\\' \\\\\\\\x\"'::hstore" ) ); + } + + void quotedValueString() + { + QCOMPARE( QgsPostgresConn::quotedValue( "b" ), QString( "'b'" ) ); + QCOMPARE( QgsPostgresConn::quotedValue( "b's" ), QString( "'b''s'" ) ); + QCOMPARE( QgsPostgresConn::quotedValue( "b \"c' \\x" ), QString( "E'b \"c'' \\\\x'" ) ); + } + +}; + +QTEST_MAIN( TestQgsPostgresConn ) +#include "testqgspostgresconn.moc" diff --git a/tests/src/providers/testqgspostgresprovider.cpp b/tests/src/providers/testqgspostgresprovider.cpp new file mode 100644 index 00000000000..3464279b28a --- /dev/null +++ b/tests/src/providers/testqgspostgresprovider.cpp @@ -0,0 +1,36 @@ +#include +#include + +#include + +class TestQgsPostgresProvider: public QObject +{ + Q_OBJECT + private slots: + void decodeHstore() + { + const QVariant decoded = QgsPostgresProvider::convertValue( QVariant::Map, "\"1\"=>\"2\", \"a\"=>\"b \\\"c'\"" ); + QCOMPARE( decoded.type(), QVariant::Map ); + + QVariantMap expected; + expected["1"] = "2"; + expected["a"] = "b \"c'"; + qDebug() << "actual: " << decoded; + QCOMPARE( decoded.toMap(), expected ); + } + + void decodeHstoreNoQuote() + { + const QVariant decoded = QgsPostgresProvider::convertValue( QVariant::Map, "1=>2, a=>b" ); + QCOMPARE( decoded.type(), QVariant::Map ); + + QVariantMap expected; + expected["1"] = "2"; + expected["a"] = "b"; + qDebug() << "actual: " << decoded; + QCOMPARE( decoded.toMap(), expected ); + } +}; + +QTEST_MAIN( TestQgsPostgresProvider ) +#include "testqgspostgresprovider.moc" diff --git a/tests/src/python/test_provider_postgres.py b/tests/src/python/test_provider_postgres.py index e5fa309f946..e371b7aaf7f 100644 --- a/tests/src/python/test_provider_postgres.py +++ b/tests/src/python/test_provider_postgres.py @@ -335,6 +335,37 @@ class TestPyQgsPostgresProvider(unittest.TestCase, ProviderTestCase): best2 = QgsEditorWidgetRegistry.instance().findBest(vl, "fld2") self.assertEqual(best2.type(), "TextEdit") + def testHstore(self): + vl = QgsVectorLayer('%s table="qgis_test"."dict" sql=' % (self.dbconn), "testhstore", "postgres") + self.assertTrue(vl.isValid()) + + fields = vl.dataProvider().fields() + self.assertEqual(fields.at(fields.indexFromName('value')).type(), QVariant.Map) + + f = next(vl.getFeatures(QgsFeatureRequest())) + + value_idx = vl.fieldNameIndex('value') + self.assertTrue(isinstance(f.attributes()[value_idx], dict)) + self.assertEqual(f.attributes()[value_idx], {'a': 'b', '1': '2'}) + + new_f = QgsFeature(vl.fields()) + new_f['pk'] = NULL + #new_f['value'] = {'x': 'a\'s "y" \\', 'z': 'end'} + new_f['value'] = {'simple': '1', 'doubleQuote': '"y"', 'quote': "'q'", 'backslash': '\\'} + r, fs = vl.dataProvider().addFeatures([new_f]) + self.assertTrue(r) + new_pk = fs[0]['pk'] + self.assertNotEqual(new_pk, NULL, fs[0].attributes()) + + try: + read_back = vl.getFeature(new_pk) + self.assertEqual(read_back['pk'], new_pk) + self.assertEqual(read_back['value'], new_f['value']) + finally: + self.assertTrue(vl.startEditing()) + self.assertTrue(vl.deleteFeatures([new_pk])) + self.assertTrue(vl.commitChanges()) + class TestPyQgsPostgresProviderCompoundKey(unittest.TestCase, ProviderTestCase): @@ -365,5 +396,6 @@ class TestPyQgsPostgresProviderCompoundKey(unittest.TestCase, ProviderTestCase): def partiallyCompiledFilters(self): return set([]) + if __name__ == '__main__': unittest.main() diff --git a/tests/testdata/provider/testdata_pg.sh b/tests/testdata/provider/testdata_pg.sh index da665a73e25..6661877ed7d 100755 --- a/tests/testdata/provider/testdata_pg.sh +++ b/tests/testdata/provider/testdata_pg.sh @@ -4,6 +4,7 @@ SCRIPTS=" tests/testdata/provider/testdata_pg.sql tests/testdata/provider/testdata_pg_reltests.sql tests/testdata/provider/testdata_pg_vectorjoin.sql + tests/testdata/provider/testdata_pg_hstore.sql " dropdb qgis_test 2> /dev/null || true diff --git a/tests/testdata/provider/testdata_pg_hstore.sql b/tests/testdata/provider/testdata_pg_hstore.sql new file mode 100644 index 00000000000..e8cd1832011 --- /dev/null +++ b/tests/testdata/provider/testdata_pg_hstore.sql @@ -0,0 +1,13 @@ +CREATE EXTENSION IF NOT EXISTS hstore; + +DROP TABLE IF EXISTS qgis_test.dict; + +CREATE TABLE qgis_test.dict +( + pk SERIAL NOT NULL PRIMARY KEY, + value hstore +); + +INSERT INTO qgis_test.dict(value) + VALUES + ('a=>b,1=>2');