Skip to content

Commit 3f4b6d0

Browse files
authoredMay 27, 2019
Merge pull request #15371 from elpaso/bugfix-21986-jsonarray-spatialite
Fix spatialite handling of JSON arrays
2 parents d5254ec + f399d3f commit 3f4b6d0

15 files changed

+727
-181
lines changed
 

‎python/core/auto_generated/fieldformatter/qgsvaluerelationfieldformatter.sip.in

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ Constructor for QgsValueRelationFieldFormatter.
5959

6060
static QStringList valueToStringList( const QVariant &value );
6161
%Docstring
62-
Utility to convert an array or a string representation of an array ``value`` to a string list
62+
Utility to convert a list or a string representation of an (hstore style: {1,2...}) list in ``value`` to a string list
6363

6464
.. versionadded:: 3.2
6565
%End

‎python/core/auto_generated/qgsjsonutils.sip.in

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -322,12 +322,14 @@ Exports all attributes from a QgsFeature as a JSON map type.
322322
%End
323323

324324

325-
static QVariantList parseArray( const QString &json, QVariant::Type type );
325+
static QVariantList parseArray( const QString &json, QVariant::Type type = QVariant::Invalid );
326326
%Docstring
327-
Parse a simple array (depth=1).
327+
Parse a simple array (depth=1)
328328

329329
:param json: the JSON to parse
330-
:param type: the type of the elements
330+
:param type: optional variant type of the elements, if specified (and not Invalid),
331+
the array items will be converted to the type, and discarded if
332+
the conversion is not possible.
331333

332334
.. versionadded:: 3.0
333335
%End

‎src/core/fieldformatter/qgsvaluerelationfieldformatter.cpp

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@
2222
#include "qgsapplication.h"
2323
#include "qgsexpressioncontextutils.h"
2424

25+
26+
#include <nlohmann/json.hpp>
27+
using json = nlohmann::json;
28+
2529
#include <QSettings>
2630

2731
bool orderByKeyLessThan( const QgsValueRelationFieldFormatter::ValueRelationItem &p1, const QgsValueRelationFieldFormatter::ValueRelationItem &p2 )
@@ -166,9 +170,43 @@ QStringList QgsValueRelationFieldFormatter::valueToStringList( const QVariant &v
166170
{
167171
QStringList checkList;
168172
if ( value.type() == QVariant::StringList )
173+
{
169174
checkList = value.toStringList();
175+
}
170176
else if ( value.type() == QVariant::String )
171-
checkList = value.toString().remove( QChar( '{' ) ).remove( QChar( '}' ) ).split( ',' );
177+
{
178+
// This must be an array representation
179+
auto newVal { value };
180+
if ( newVal.toString().trimmed().startsWith( '{' ) )
181+
{
182+
newVal = QVariant( newVal.toString().trimmed().mid( 1 ).mid( 0, newVal.toString().length() - 2 ).prepend( '[' ).append( ']' ) );
183+
}
184+
if ( newVal.toString().trimmed().startsWith( '[' ) )
185+
{
186+
try
187+
{
188+
for ( auto &element : json::parse( newVal.toString().toStdString() ) )
189+
{
190+
if ( element.is_number_integer() )
191+
{
192+
checkList << QString::number( element.get<int>() );
193+
}
194+
else if ( element.is_number_unsigned() )
195+
{
196+
checkList << QString::number( element.get<unsigned>() );
197+
}
198+
else if ( element.is_string() )
199+
{
200+
checkList << QString::fromStdString( element.get<std::string>() );
201+
}
202+
}
203+
}
204+
catch ( json::parse_error ex )
205+
{
206+
qDebug() << QString::fromStdString( ex.what() );
207+
}
208+
}
209+
}
172210
else if ( value.type() == QVariant::List )
173211
{
174212
QVariantList valuesList( value.toList( ) );

‎src/core/fieldformatter/qgsvaluerelationfieldformatter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ class CORE_EXPORT QgsValueRelationFieldFormatter : public QgsFieldFormatter
6666
QVariant createCache( QgsVectorLayer *layer, int fieldIndex, const QVariantMap &config ) const override;
6767

6868
/**
69-
* Utility to convert an array or a string representation of an array \a value to a string list
69+
* Utility to convert a list or a string representation of an (hstore style: {1,2...}) list in \a value to a string list
7070
* \since QGIS 3.2
7171
*/
7272
static QStringList valueToStringList( const QVariant &value );

‎src/core/qgsjsonutils.cpp

Lines changed: 59 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -315,28 +315,70 @@ QString QgsJsonUtils::exportAttributes( const QgsFeature &feature, QgsVectorLaye
315315

316316
QVariantList QgsJsonUtils::parseArray( const QString &json, QVariant::Type type )
317317
{
318-
QJsonParseError error;
319-
const QJsonDocument jsonDoc = QJsonDocument::fromJson( json.toUtf8(), &error );
318+
QString errorMessage;
320319
QVariantList result;
321-
if ( error.error != QJsonParseError::NoError )
320+
try
322321
{
323-
QgsLogger::warning( QStringLiteral( "Cannot parse json (%1): %2" ).arg( error.errorString(), json ) );
324-
return result;
325-
}
326-
if ( !jsonDoc.isArray() )
327-
{
328-
QgsLogger::warning( QStringLiteral( "Cannot parse json (%1) as array: %2" ).arg( error.errorString(), json ) );
329-
return result;
322+
const auto jObj { json::parse( json.toStdString() ) };
323+
if ( ! jObj.is_array() )
324+
{
325+
throw json::parse_error::create( 0, 0, QStringLiteral( "JSON value must be an array" ).toStdString() );
326+
}
327+
for ( const auto &item : jObj )
328+
{
329+
// Create a QVariant from the array item
330+
QVariant v;
331+
if ( item.is_number_integer() )
332+
{
333+
v = item.get<int>();
334+
}
335+
else if ( item.is_number_unsigned() )
336+
{
337+
v = item.get<unsigned>();
338+
}
339+
else if ( item.is_number_float() )
340+
{
341+
// Note: it's a double and not a float on purpose
342+
v = item.get<double>();
343+
}
344+
else if ( item.is_string() )
345+
{
346+
v = QString::fromStdString( item.get<std::string>() );
347+
}
348+
else if ( item.is_null() )
349+
{
350+
// Fallback to int
351+
v = QVariant( type == QVariant::Type::Invalid ? QVariant::Type::Int : type );
352+
}
353+
else
354+
{
355+
// do nothing and discard the item
356+
}
357+
358+
// If a destination type was specified (it's not invalid), try to convert
359+
if ( type != QVariant::Invalid )
360+
{
361+
if ( ! v.convert( static_cast<int>( type ) ) )
362+
{
363+
QgsLogger::warning( QStringLiteral( "Cannot convert json array element to specified type, ignoring: %1" ).arg( v.toString() ) );
364+
}
365+
else
366+
{
367+
result.push_back( v );
368+
}
369+
}
370+
else
371+
{
372+
result.push_back( v );
373+
}
374+
}
330375
}
331-
const auto constArray = jsonDoc.array();
332-
for ( const QJsonValue &cur : constArray )
376+
catch ( json::parse_error ex )
333377
{
334-
QVariant curVariant = cur.toVariant();
335-
if ( curVariant.convert( type ) )
336-
result.append( curVariant );
337-
else
338-
QgsLogger::warning( QStringLiteral( "Cannot convert json array element: %1" ).arg( cur.toString() ) );
378+
errorMessage = ex.what();
379+
QgsLogger::warning( QStringLiteral( "Cannot parse json (%1): %2" ).arg( ex.what(), json ) );
339380
}
381+
340382
return result;
341383
}
342384

‎src/core/qgsjsonutils.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -328,18 +328,20 @@ class CORE_EXPORT QgsJsonUtils
328328
const QVector<QVariant> &attributeWidgetCaches = QVector<QVariant>() ) SIP_SKIP;
329329

330330
/**
331-
* Parse a simple array (depth=1).
331+
* Parse a simple array (depth=1)
332332
* \param json the JSON to parse
333-
* \param type the type of the elements
333+
* \param type optional variant type of the elements, if specified (and not Invalid),
334+
* the array items will be converted to the type, and discarded if
335+
* the conversion is not possible.
334336
* \since QGIS 3.0
335337
*/
336-
static QVariantList parseArray( const QString &json, QVariant::Type type );
338+
static QVariantList parseArray( const QString &json, QVariant::Type type = QVariant::Invalid );
337339

338340

339341
/**
340342
* Converts a QVariant \a v to a json object
341343
* \note Not available in Python bindings
342-
* \since QGIS 3.10
344+
* \since QGIS 3.8
343345
*/
344346
static json jsonFromVariant( const QVariant &v ) SIP_SKIP;
345347

‎src/gui/editorwidgets/qgsvaluerelationwidgetwrapper.cpp

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "qgsvaluerelationfieldformatter.h"
2626
#include "qgsattributeform.h"
2727
#include "qgsattributes.h"
28+
#include "qgsjsonutils.h"
2829

2930
#include <QHeaderView>
3031
#include <QComboBox>
@@ -33,6 +34,10 @@
3334
#include <QStringListModel>
3435
#include <QCompleter>
3536

37+
#include <nlohmann/json.hpp>
38+
using json = nlohmann::json;
39+
40+
3641
QgsValueRelationWidgetWrapper::QgsValueRelationWidgetWrapper( QgsVectorLayer *layer, int fieldIdx, QWidget *editor, QWidget *parent )
3742
: QgsEditorWidgetWrapper( layer, fieldIdx, editor, parent )
3843
{
@@ -70,21 +75,26 @@ QVariant QgsValueRelationWidgetWrapper::value() const
7075
}
7176
}
7277

73-
if ( layer()->fields().at( fieldIdx() ).type() == QVariant::Map )
78+
QVariantList vl;
79+
//store as QVariantList because it's json
80+
for ( const QString &s : qgis::as_const( selection ) )
7481
{
75-
QVariantList vl;
76-
//store as QVariantList because it's json
77-
for ( const QString &s : qgis::as_const( selection ) )
82+
// Convert to proper type
83+
const QVariant::Type type { fkType() };
84+
switch ( type )
7885
{
79-
vl << s;
86+
case QVariant::Type::Int:
87+
vl.push_back( s.toInt() );
88+
break;
89+
case QVariant::Type::LongLong:
90+
vl.push_back( s.toLongLong() );
91+
break;
92+
default:
93+
vl.push_back( s );
94+
break;
8095
}
81-
v = vl;
82-
}
83-
else
84-
{
85-
//store as hstore string
86-
v = selection.join( ',' ).prepend( '{' ).append( '}' );
8796
}
97+
v = vl;
8898
}
8999

90100
if ( mLineEdit )
@@ -289,6 +299,21 @@ int QgsValueRelationWidgetWrapper::columnCount() const
289299
return std::max( 1, config( QStringLiteral( "NofColumns" ) ).toInt() );
290300
}
291301

302+
QVariant::Type QgsValueRelationWidgetWrapper::fkType() const
303+
{
304+
QgsVectorLayer *layer = QgsProject::instance()->mapLayer<QgsVectorLayer *>( config().value( QStringLiteral( "Layer" ) ).toString() );
305+
if ( layer )
306+
{
307+
QgsFields fields = layer->fields();
308+
int idx { fields.lookupField( config().value( QStringLiteral( "Key" ) ).toString() ) };
309+
if ( idx >= 0 )
310+
{
311+
return fields.at( idx ).type();
312+
}
313+
}
314+
return QVariant::Type::Invalid;
315+
}
316+
292317
void QgsValueRelationWidgetWrapper::populate( )
293318
{
294319
// Initialize, note that signals are blocked, to avoid double signals on new features

‎src/gui/editorwidgets/qgsvaluerelationwidgetwrapper.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ class GUI_EXPORT QgsValueRelationWidgetWrapper : public QgsEditorWidgetWrapper
115115
*/
116116
int columnCount() const;
117117

118+
//! Returns the variant type of the fk
119+
QVariant::Type fkType() const;
120+
118121
//! Sets the values for the widgets, re-creates the cache when required
119122
void populate( );
120123

‎src/providers/spatialite/qgsspatialitefeatureiterator.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,10 @@ QVariant QgsSpatiaLiteFeatureIterator::getFeatureAttribute( sqlite3_stmt *stmt,
581581
{
582582
// assume arrays are stored as JSON
583583
QVariant result = QVariant( QgsJsonUtils::parseArray( txt, subType ) );
584-
result.convert( type );
584+
if ( ! result.convert( static_cast<int>( type ) ) )
585+
{
586+
QgsDebugMsgLevel( QStringLiteral( "Could not convert JSON value to requested QVariant type" ).arg( txt ), 3 );
587+
}
585588
return result;
586589
}
587590
return txt;

‎src/providers/spatialite/qgsspatialiteprovider.cpp

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ email : a.furieri@lqt.it
4444
#include <QDir>
4545
#include <QRegularExpression>
4646

47+
#include <nlohmann/json.hpp>
48+
using json = nlohmann::json;
49+
4750

4851
const QString SPATIALITE_KEY = QStringLiteral( "spatialite" );
4952
const QString SPATIALITE_DESCRIPTION = QStringLiteral( "SpatiaLite data provider" );
@@ -663,6 +666,10 @@ static TypeSubType getVariantType( const QString &type )
663666
type.length() - SPATIALITE_ARRAY_PREFIX.length() - SPATIALITE_ARRAY_SUFFIX.length() ) );
664667
return TypeSubType( subType.first == QVariant::String ? QVariant::StringList : QVariant::List, subType.first );
665668
}
669+
else if ( type == QLatin1String( "jsonarray" ) )
670+
{
671+
return TypeSubType( QVariant::List, QVariant::Invalid );
672+
}
666673
else
667674
// for sure any SQLite value can be represented as SQLITE_TEXT
668675
return TypeSubType( QVariant::String, QVariant::Invalid );
@@ -4385,6 +4392,7 @@ bool QgsSpatiaLiteProvider::changeAttributeValues( const QgsChangedAttributesMap
43854392
first = false;
43864393

43874394
QVariant::Type type = fld.type();
4395+
const auto typeName { fld.typeName() };
43884396

43894397
if ( val.isNull() || !val.isValid() )
43904398
{
@@ -4398,8 +4406,27 @@ bool QgsSpatiaLiteProvider::changeAttributeValues( const QgsChangedAttributesMap
43984406
}
43994407
else if ( type == QVariant::StringList || type == QVariant::List )
44004408
{
4401-
// binding an array value
4402-
sql += QStringLiteral( "%1=%2" ).arg( QgsSqliteUtils::quotedIdentifier( fld.name() ), QgsSqliteUtils::quotedString( QgsJsonUtils::encodeValue( val ) ) );
4409+
// binding an array value, parse JSON
4410+
QString jRepr;
4411+
try
4412+
{
4413+
const auto jObj { QgsJsonUtils::jsonFromVariant( val ) };
4414+
if ( ! jObj.is_array() )
4415+
{
4416+
throw json::parse_error::create( 0, 0, tr( "JSON value must be an array" ).toStdString() );
4417+
}
4418+
jRepr = QString::fromStdString( jObj.dump( ) );
4419+
sql += QStringLiteral( "%1=%2" ).arg( QgsSqliteUtils::quotedIdentifier( fld.name() ), QgsSqliteUtils::quotedString( jRepr ) );
4420+
}
4421+
catch ( json::exception ex )
4422+
{
4423+
const auto errM { tr( "Field type is JSON but the value cannot be converted to JSON array: %1" ).arg( ex.what() ) };
4424+
auto msgPtr { static_cast<char *>( sqlite3_malloc( errM.length() + 1 ) ) };
4425+
strcpy( static_cast<char *>( msgPtr ), errM.toStdString().c_str() );
4426+
errMsg = msgPtr;
4427+
handleError( jRepr, errMsg, true );
4428+
return false;
4429+
}
44034430
}
44044431
else
44054432
{

‎tests/src/core/testqgsjsonutils.cpp

Lines changed: 181 additions & 133 deletions
Large diffs are not rendered by default.

‎tests/src/gui/testqgsvaluerelationwidgetwrapper.cpp

Lines changed: 312 additions & 6 deletions
Large diffs are not rendered by default.

‎tests/src/python/test_provider_spatialite.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,12 @@ def setUpClass(cls):
163163
sql += "VALUES (1, '[\"toto\",\"tutu\"]', '[1,-2,724562]', '[1.0, -232567.22]', GeomFromText('POLYGON((0 0,1 0,1 1,0 1,0 0))', 4326))"
164164
cur.execute(sql)
165165

166+
# table with different array types, stored as JSON
167+
sql = "CREATE TABLE test_arrays_write (Id INTEGER NOT NULL PRIMARY KEY, array JSONARRAY NOT NULL, strings JSONSTRINGLIST NOT NULL, ints JSONINTEGERLIST NOT NULL, reals JSONREALLIST NOT NULL)"
168+
cur.execute(sql)
169+
sql = "SELECT AddGeometryColumn('test_arrays_write', 'Geometry', 4326, 'POLYGON', 'XY')"
170+
cur.execute(sql)
171+
166172
# 2 tables with relations
167173
sql = "PRAGMA foreign_keys = ON;"
168174
cur.execute(sql)
@@ -533,6 +539,45 @@ def test_arrays(self):
533539
self.assertEqual(read_back['ints'], new_f['ints'])
534540
self.assertEqual(read_back['reals'], new_f['reals'])
535541

542+
def test_arrays_write(self):
543+
"""Test writing of layers with arrays"""
544+
l = QgsVectorLayer("dbname=%s table=test_arrays_write (geometry)" % self.dbname, "test_arrays", "spatialite")
545+
self.assertTrue(l.isValid())
546+
547+
new_f = QgsFeature(l.fields())
548+
new_f['id'] = 2
549+
new_f['array'] = ['simple', '"doubleQuote"', "'quote'", 'back\\slash']
550+
new_f['strings'] = ['simple', '"doubleQuote"', "'quote'", 'back\\slash']
551+
new_f['ints'] = [1, 2, 3, 4]
552+
new_f['reals'] = [1e67, 1e-56]
553+
r, fs = l.dataProvider().addFeatures([new_f])
554+
self.assertTrue(r)
555+
556+
read_back = l.getFeature(new_f['id'])
557+
self.assertEqual(read_back['id'], new_f['id'])
558+
self.assertEqual(read_back['array'], new_f['array'])
559+
self.assertEqual(read_back['strings'], new_f['strings'])
560+
self.assertEqual(read_back['ints'], new_f['ints'])
561+
self.assertEqual(read_back['reals'], new_f['reals'])
562+
563+
new_f = QgsFeature(l.fields())
564+
new_f['id'] = 3
565+
new_f['array'] = [1, 1.2345, '"doubleQuote"', "'quote'", 'back\\slash']
566+
new_f['strings'] = ['simple', '"doubleQuote"', "'quote'", 'back\\slash']
567+
new_f['ints'] = [1, 2, 3, 4]
568+
new_f['reals'] = [1e67, 1e-56]
569+
r, fs = l.dataProvider().addFeatures([new_f])
570+
self.assertTrue(r)
571+
572+
read_back = l.getFeature(new_f['id'])
573+
self.assertEqual(read_back['id'], new_f['id'])
574+
self.assertEqual(read_back['array'], new_f['array'])
575+
self.assertEqual(read_back['strings'], new_f['strings'])
576+
self.assertEqual(read_back['ints'], new_f['ints'])
577+
self.assertEqual(read_back['reals'], new_f['reals'])
578+
579+
read_back = l.getFeature(new_f['id'])
580+
536581
def test_discover_relation(self):
537582
artist = QgsVectorLayer("dbname=%s table=test_relation_a (geometry)" % self.dbname, "test_relation_a", "spatialite")
538583
self.assertTrue(artist.isValid())

‎tests/src/python/test_qgsfieldformatters.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,12 @@ def _test(a, b):
130130
_test([1, 2, 3], ["1", "2", "3"])
131131
_test("{1,2,3}", ["1", "2", "3"])
132132
_test(['1', '2', '3'], ["1", "2", "3"])
133-
_test('not an array', ['not an array'])
133+
_test('not an array', [])
134+
_test('[1,2,3]', ["1", "2", "3"])
135+
_test('{1,2,3}', ["1", "2", "3"])
136+
_test('{"1","2","3"}', ["1", "2", "3"])
137+
_test('["1","2","3"]', ["1", "2", "3"])
138+
_test(r'["a string,comma","a string\"quote", "another string[]"]', ['a string,comma', 'a string"quote', 'another string[]'])
134139

135140
def test_expressionRequiresFormScope(self):
136141

Binary file not shown.

0 commit comments

Comments
 (0)
Please sign in to comment.