Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #30137 from elpaso/bugfix-gh30131-postgres-json
Fix json hanlding of bools and complete json(b) PG support
  • Loading branch information
elpaso committed Jun 10, 2019
2 parents 5201151 + ef6dd4e commit 13099cc
Show file tree
Hide file tree
Showing 11 changed files with 215 additions and 48 deletions.
1 change: 1 addition & 0 deletions python/core/auto_generated/qgsjsonutils.sip.in
Expand Up @@ -337,6 +337,7 @@ Parse a simple array (depth=1)




};

/************************************************************************
Expand Down
105 changes: 96 additions & 9 deletions src/core/qgsjsonutils.cpp
Expand Up @@ -345,15 +345,15 @@ QVariantList QgsJsonUtils::parseArray( const QString &json, QVariant::Type type
{
v = QString::fromStdString( item.get<std::string>() );
}
else if ( item.is_boolean() )
{
v = item.get<bool>();
}
else if ( item.is_null() )
{
// Fallback to int
v = QVariant( type == QVariant::Type::Invalid ? QVariant::Type::Int : type );
}
else
{
// do nothing and discard the item
}

// If a destination type was specified (it's not invalid), try to convert
if ( type != QVariant::Invalid )
Expand Down Expand Up @@ -384,6 +384,11 @@ QVariantList QgsJsonUtils::parseArray( const QString &json, QVariant::Type type

json QgsJsonUtils::jsonFromVariant( const QVariant &val )
{
if ( val.isNull() || ! val.isValid() )
{
return nullptr;
}
json j;
if ( val.type() == QVariant::Type::Map )
{
const auto vMap { val.toMap() };
Expand All @@ -392,7 +397,7 @@ json QgsJsonUtils::jsonFromVariant( const QVariant &val )
{
jMap[ it.key().toStdString() ] = jsonFromVariant( it.value() );
}
return jMap;
j = jMap;
}
else if ( val.type() == QVariant::Type::List )
{
Expand All @@ -402,7 +407,7 @@ json QgsJsonUtils::jsonFromVariant( const QVariant &val )
{
jList.push_back( jsonFromVariant( v ) );
}
return jList;
j = jList;
}
else
{
Expand All @@ -412,14 +417,96 @@ json QgsJsonUtils::jsonFromVariant( const QVariant &val )
case QMetaType::UInt:
case QMetaType::LongLong:
case QMetaType::ULongLong:
return val.toLongLong();
j = val.toLongLong();
break;
case QMetaType::Double:
case QMetaType::Float:
return val.toDouble();
j = val.toDouble();
break;
case QMetaType::Bool:
j = val.toBool();
break;
default:
return val.toString().toStdString();
j = val.toString().toStdString();
break;
}
}
return j;
}

QVariant QgsJsonUtils::parseJson( const std::string &jsonString )
{
std::function<QVariant( json )> _parser { [ & ]( json jObj ) -> QVariant {
QVariant result;
QString errorMessage;
if ( jObj.is_array() )
{
QVariantList results;
for ( const auto &item : jObj )
{
results.push_back( _parser( item ) );
}
result = results;
}
else if ( jObj.is_object() )
{
QVariantMap results;
for ( const auto &item : jObj.items() )
{
const auto key { QString::fromStdString( item.key() ) };
const auto value { _parser( item.value() ) };
results[ key ] = value;
}
result = results;
}
else
{
if ( jObj.is_number_integer() )
{
result = jObj.get<int>();
}
else if ( jObj.is_number_unsigned() )
{
result = jObj.get<unsigned>();
}
else if ( jObj.is_boolean() )
{
result = jObj.get<bool>();
}
else if ( jObj.is_number_float() )
{
// Note: it's a double and not a float on purpose
result = jObj.get<double>();
}
else if ( jObj.is_string() )
{
result = QString::fromStdString( jObj.get<std::string>() );
}
else if ( jObj.is_null() )
{
// Do nothing (leave invalid)
}
}
return result;
}
};

try
{
const json j = json::parse( jsonString );
return _parser( j );
}
catch ( json::parse_error &ex )
{
QgsLogger::warning( QStringLiteral( "Cannot parse json (%1): %2" ).arg( QString::fromStdString( ex.what() ),
QString::fromStdString( jsonString ) ) );
}
return QVariant();
}

QVariant QgsJsonUtils::parseJson( const QString &jsonString )
{
return parseJson( jsonString.toStdString() );
}

json QgsJsonUtils::exportAttributesToJsonObject( const QgsFeature &feature, QgsVectorLayer *layer, const QVector<QVariant> &attributeWidgetCaches )
Expand Down
13 changes: 13 additions & 0 deletions src/core/qgsjsonutils.h
Expand Up @@ -345,6 +345,19 @@ class CORE_EXPORT QgsJsonUtils
*/
static json jsonFromVariant( const QVariant &v ) SIP_SKIP;

/**
* Converts JSON \a jsonString to a QVariant, in case of parsing error an invalid QVariant is returned.
* \note Not available in Python bindings
* \since QGIS 3.8
*/
static QVariant parseJson( const std::string &jsonString ) SIP_SKIP;

/**
* Converts JSON \a jsonString to a QVariant, in case of parsing error an invalid QVariant is returned.
* \note Not available in Python bindings
* \since QGIS 3.8
*/
static QVariant parseJson( const QString &jsonString ) SIP_SKIP;

};

Expand Down
12 changes: 11 additions & 1 deletion src/providers/postgres/qgspostgresconn.cpp
Expand Up @@ -27,12 +27,15 @@
#include "qgsvectordataprovider.h"
#include "qgswkbtypes.h"
#include "qgssettings.h"
#include "qgsjsonutils.h"

#include <QApplication>
#include <QThread>

#include <climits>

#include <nlohmann/json.hpp>

// for htonl
#ifdef Q_OS_WIN
#include <winsock.h>
Expand Down Expand Up @@ -992,7 +995,6 @@ static QString doubleQuotedMapValue( const QString &v )

static QString quotedMap( const QVariantMap &map )
{
//to store properly it should be decided if it's a hstore or a json/jsonb field here...
QString ret;
for ( QVariantMap::const_iterator i = map.constBegin(); i != map.constEnd(); ++i )
{
Expand Down Expand Up @@ -1057,6 +1059,14 @@ QString QgsPostgresConn::quotedValue( const QVariant &value )
}
}

QString QgsPostgresConn::quotedJsonValue( const QVariant &value )
{
if ( value.isNull() || !value.isValid() )
return QStringLiteral( "null" );
const auto j { QgsJsonUtils::jsonFromVariant( value ) };
return quotedString( QString::fromStdString( j.dump() ) );
}

PGresult *QgsPostgresConn::PQexec( const QString &query, bool logError, bool retry ) const
{
QMutexLocker locker( &mLock );
Expand Down
6 changes: 6 additions & 0 deletions src/providers/postgres/qgspostgresconn.h
Expand Up @@ -283,6 +283,12 @@ class QgsPostgresConn : public QObject
*/
static QString quotedValue( const QVariant &value );

/**
* Quote a json(b) value for placement in a SQL string.
* \note a null value will be represented as a NULL and not as a json null.
*/
static QString quotedJsonValue( const QVariant &value );

/**
* Gets the list of supported layers
* \param layers list to store layers in
Expand Down
26 changes: 20 additions & 6 deletions src/providers/postgres/qgspostgresprovider.cpp
Expand Up @@ -41,6 +41,7 @@
#include "qgslogger.h"
#include "qgsfeedback.h"
#include "qgssettings.h"
#include "qgsjsonutils.h"

#ifdef HAVE_GUI
#include "qgspgsourceselect.h"
Expand Down Expand Up @@ -2185,10 +2186,17 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist, Flags flags )
.arg( delim,
quotedValue( v.toString() ) );
}
else if ( fieldTypeName == QLatin1String( "jsonb" ) )
{
values += delim + quotedJsonValue( v ) + QLatin1String( "::jsonb" );
}
else if ( fieldTypeName == QLatin1String( "json" ) )
{
values += delim + quotedJsonValue( v ) + QLatin1String( "::json" );
}
//TODO: convert arrays and hstore to native types
else
{
//this should be for json/jsonb in future
values += delim + quotedValue( v );
}
}
Expand Down Expand Up @@ -2734,6 +2742,16 @@ bool QgsPostgresProvider::changeAttributeValues( const QgsChangedAttributesMap &
sql += QStringLiteral( "st_geographyfromewkt(%1)" )
.arg( quotedValue( siter->toString() ) );
}
else if ( fld.typeName() == QLatin1String( "jsonb" ) )
{
sql += QStringLiteral( "%1::jsonb" )
.arg( quotedJsonValue( siter.value() ) );
}
else if ( fld.typeName() == QLatin1String( "json" ) )
{
sql += QStringLiteral( "%1::json" )
.arg( quotedJsonValue( siter.value() ) );
}
else
{
sql += quotedValue( *siter );
Expand Down Expand Up @@ -4316,11 +4334,7 @@ QVariant QgsPostgresProvider::parseHstore( const QString &txt )

QVariant QgsPostgresProvider::parseJson( const QString &txt )
{
QVariant result;
QJsonDocument jsonResponse = QJsonDocument::fromJson( txt.toUtf8() );
//it's null if no json format
result = jsonResponse.toVariant();
return result;
return QgsJsonUtils::parseJson( txt );
}

QVariant QgsPostgresProvider::parseOtherArray( const QString &txt, QVariant::Type subType, const QString &typeName )
Expand Down
1 change: 1 addition & 0 deletions src/providers/postgres/qgspostgresprovider.h
Expand Up @@ -445,6 +445,7 @@ class QgsPostgresProvider : public QgsVectorDataProvider

static QString quotedIdentifier( const QString &ident ) { return QgsPostgresConn::quotedIdentifier( ident ); }
static QString quotedValue( const QVariant &value ) { return QgsPostgresConn::quotedValue( value ); }
static QString quotedJsonValue( const QVariant &value ) { return QgsPostgresConn::quotedJsonValue( value ); }

friend class QgsPostgresFeatureSource;

Expand Down
29 changes: 29 additions & 0 deletions tests/src/core/testqgsjsonutils.cpp
Expand Up @@ -38,6 +38,7 @@ class TestQgsJsonUtils : public QObject
private slots:
void testStringList();
void testJsonArray();
void testParseJson();
void testIntList();
void testDoubleList();
void testExportAttributesJson_data();
Expand Down Expand Up @@ -91,6 +92,8 @@ void TestQgsJsonUtils::testJsonArray()
// Empty
QCOMPARE( QgsJsonUtils::parseArray( R"([])", QVariant::Int ), QVariantList() );
QCOMPARE( QgsJsonUtils::parseArray( "", QVariant::Int ), QVariantList() );
// Booleans
QCOMPARE( QgsJsonUtils::parseArray( "[true,false]", QVariant::Bool ), QVariantList() << true << false );
// Nulls
for ( const QVariant &value : QgsJsonUtils::parseArray( R"([null, null])" ) )
{
Expand All @@ -106,6 +109,32 @@ void TestQgsJsonUtils::testJsonArray()
}
}

void TestQgsJsonUtils::testParseJson()
{
QStringList tests {{
"null",
"false",
"true",
"123",
"123.45",
R"j("a string")j",
"[1,2,3.4,null]",
R"j({"_bool":true,"_double":1234.45,"_int":123,"_list":[1,2,3.4,null],"_null":null,"_object":{"int":123}})j",
}};

for ( const auto &testJson : tests )
{
const auto parsed { QgsJsonUtils::parseJson( testJson ) };
QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( parsed ).dump() ), testJson );
}

// Test empty string: null
QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( QgsJsonUtils::parseJson( QStringLiteral( "" ) ) ).dump() ), QString( "null" ) );
// invalid json -> null
QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( QgsJsonUtils::parseJson( QStringLiteral( "invalid json" ) ) ).dump() ), QString( "null" ) );

}

void TestQgsJsonUtils::testIntList()
{
QVariantList list;
Expand Down

0 comments on commit 13099cc

Please sign in to comment.