Merge pull request #15371 from elpaso/bugfix-21986-jsonarray-spatialite

Fix spatialite handling of JSON arrays
qgis · May 27, 2019 · 3f4b6d0 · 3f4b6d0
2 parents d5254ec + f399d3f
commit 3f4b6d0
Show file tree

Hide file tree

Showing 15 changed files with 727 additions and 181 deletions.
diff --git a/python/core/auto_generated/fieldformatter/qgsvaluerelationfieldformatter.sip.in b/python/core/auto_generated/fieldformatter/qgsvaluerelationfieldformatter.sip.in
@@ -59,7 +59,7 @@ Constructor for QgsValueRelationFieldFormatter.
 
     static QStringList valueToStringList( const QVariant &value );
 %Docstring
-Utility to convert an array or a string representation of an array ``value`` to a string list
+Utility to convert a list or a string representation of an (hstore style: {1,2...}) list in ``value`` to a string list
 
 .. versionadded:: 3.2
 %End

diff --git a/python/core/auto_generated/qgsjsonutils.sip.in b/python/core/auto_generated/qgsjsonutils.sip.in
@@ -322,12 +322,14 @@ Exports all attributes from a QgsFeature as a JSON map type.
 %End
 
 
-    static QVariantList parseArray( const QString &json, QVariant::Type type );
+    static QVariantList parseArray( const QString &json, QVariant::Type type = QVariant::Invalid );
 %Docstring
-Parse a simple array (depth=1).
+Parse a simple array (depth=1)
 
 :param json: the JSON to parse
-:param type: the type of the elements
+:param type: optional variant type of the elements, if specified (and not Invalid),
+             the array items will be converted to the type, and discarded if
+             the conversion is not possible.
 
 .. versionadded:: 3.0
 %End

diff --git a/src/core/fieldformatter/qgsvaluerelationfieldformatter.cpp b/src/core/fieldformatter/qgsvaluerelationfieldformatter.cpp
@@ -22,6 +22,10 @@
 #include "qgsapplication.h"
 #include "qgsexpressioncontextutils.h"
 
+
+#include <nlohmann/json.hpp>
+using json = nlohmann::json;
+
 #include <QSettings>
 
 bool orderByKeyLessThan( const QgsValueRelationFieldFormatter::ValueRelationItem &p1, const QgsValueRelationFieldFormatter::ValueRelationItem &p2 )
@@ -166,9 +170,43 @@ QStringList QgsValueRelationFieldFormatter::valueToStringList( const QVariant &v
 {
   QStringList checkList;
   if ( value.type() == QVariant::StringList )
+  {
     checkList = value.toStringList();
+  }
   else if ( value.type() == QVariant::String )
-    checkList = value.toString().remove( QChar( '{' ) ).remove( QChar( '}' ) ).split( ',' );
+  {
+    // This must be an array representation
+    auto newVal { value };
+    if ( newVal.toString().trimmed().startsWith( '{' ) )
+    {
+      newVal = QVariant( newVal.toString().trimmed().mid( 1 ).mid( 0, newVal.toString().length() - 2 ).prepend( '[' ).append( ']' ) );
+    }
+    if ( newVal.toString().trimmed().startsWith( '[' ) )
+    {
+      try
+      {
+        for ( auto &element : json::parse( newVal.toString().toStdString() ) )
+        {
+          if ( element.is_number_integer() )
+          {
+            checkList << QString::number( element.get<int>() );
+          }
+          else if ( element.is_number_unsigned() )
+          {
+            checkList << QString::number( element.get<unsigned>() );
+          }
+          else if ( element.is_string() )
+          {
+            checkList << QString::fromStdString( element.get<std::string>() );
+          }
+        }
+      }
+      catch ( json::parse_error ex )
+      {
+        qDebug() << QString::fromStdString( ex.what() );
+      }
+    }
+  }
   else if ( value.type() == QVariant::List )
   {
     QVariantList valuesList( value.toList( ) );

diff --git a/src/core/fieldformatter/qgsvaluerelationfieldformatter.h b/src/core/fieldformatter/qgsvaluerelationfieldformatter.h
@@ -66,7 +66,7 @@ class CORE_EXPORT QgsValueRelationFieldFormatter : public QgsFieldFormatter
     QVariant createCache( QgsVectorLayer *layer, int fieldIndex, const QVariantMap &config ) const override;
 
     /**
-     * Utility to convert an array or a string representation of an array \a value to a string list
+     * Utility to convert a list or a string representation of an (hstore style: {1,2...}) list in \a value to a string list
      * \since QGIS 3.2
      */
     static QStringList valueToStringList( const QVariant &value );

diff --git a/src/core/qgsjsonutils.cpp b/src/core/qgsjsonutils.cpp
@@ -315,28 +315,70 @@ QString QgsJsonUtils::exportAttributes( const QgsFeature &feature, QgsVectorLaye
 
 QVariantList QgsJsonUtils::parseArray( const QString &json, QVariant::Type type )
 {
-  QJsonParseError error;
-  const QJsonDocument jsonDoc = QJsonDocument::fromJson( json.toUtf8(), &error );
+  QString errorMessage;
   QVariantList result;
-  if ( error.error != QJsonParseError::NoError )
+  try
   {
-    QgsLogger::warning( QStringLiteral( "Cannot parse json (%1): %2" ).arg( error.errorString(), json ) );
-    return result;
-  }
-  if ( !jsonDoc.isArray() )
-  {
-    QgsLogger::warning( QStringLiteral( "Cannot parse json (%1) as array: %2" ).arg( error.errorString(), json ) );
-    return result;
+    const auto jObj { json::parse( json.toStdString() ) };
+    if ( ! jObj.is_array() )
+    {
+      throw json::parse_error::create( 0, 0, QStringLiteral( "JSON value must be an array" ).toStdString() );
+    }
+    for ( const auto &item : jObj )
+    {
+      // Create a QVariant from the array item
+      QVariant v;
+      if ( item.is_number_integer() )
+      {
+        v = item.get<int>();
+      }
+      else if ( item.is_number_unsigned() )
+      {
+        v = item.get<unsigned>();
+      }
+      else if ( item.is_number_float() )
+      {
+        // Note: it's a double and not a float on purpose
+        v = item.get<double>();
+      }
+      else if ( item.is_string() )
+      {
+        v = QString::fromStdString( item.get<std::string>() );
+      }
+      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 )
+      {
+        if ( ! v.convert( static_cast<int>( type ) ) )
+        {
+          QgsLogger::warning( QStringLiteral( "Cannot convert json array element to specified type, ignoring: %1" ).arg( v.toString() ) );
+        }
+        else
+        {
+          result.push_back( v );
+        }
+      }
+      else
+      {
+        result.push_back( v );
+      }
+    }
   }
-  const auto constArray = jsonDoc.array();
-  for ( const QJsonValue &cur : constArray )
+  catch ( json::parse_error ex )
   {
-    QVariant curVariant = cur.toVariant();
-    if ( curVariant.convert( type ) )
-      result.append( curVariant );
-    else
-      QgsLogger::warning( QStringLiteral( "Cannot convert json array element: %1" ).arg( cur.toString() ) );
+    errorMessage = ex.what();
+    QgsLogger::warning( QStringLiteral( "Cannot parse json (%1): %2" ).arg( ex.what(), json ) );
   }
+
   return result;
 }
 

diff --git a/src/core/qgsjsonutils.h b/src/core/qgsjsonutils.h
@@ -328,18 +328,20 @@ class CORE_EXPORT QgsJsonUtils
         const QVector<QVariant> &attributeWidgetCaches = QVector<QVariant>() ) SIP_SKIP;
 
     /**
-     * Parse a simple array (depth=1).
+     * Parse a simple array (depth=1)
      * \param json the JSON to parse
-     * \param type the type of the elements
+     * \param type optional variant type of the elements, if specified (and not Invalid),
+     *        the array items will be converted to the type, and discarded if
+     *        the conversion is not possible.
      * \since QGIS 3.0
      */
-    static QVariantList parseArray( const QString &json, QVariant::Type type );
+    static QVariantList parseArray( const QString &json, QVariant::Type type = QVariant::Invalid );
 
 
     /**
      * Converts a QVariant \a v to a json object
      * \note Not available in Python bindings
-     * \since QGIS 3.10
+     * \since QGIS 3.8
      */
     static json jsonFromVariant( const QVariant &v ) SIP_SKIP;
 

diff --git a/src/gui/editorwidgets/qgsvaluerelationwidgetwrapper.cpp b/src/gui/editorwidgets/qgsvaluerelationwidgetwrapper.cpp
@@ -25,6 +25,7 @@
 #include "qgsvaluerelationfieldformatter.h"
 #include "qgsattributeform.h"
 #include "qgsattributes.h"
+#include "qgsjsonutils.h"
 
 #include <QHeaderView>
 #include <QComboBox>
@@ -33,6 +34,10 @@
 #include <QStringListModel>
 #include <QCompleter>
 
+#include <nlohmann/json.hpp>
+using json = nlohmann::json;
+
+
 QgsValueRelationWidgetWrapper::QgsValueRelationWidgetWrapper( QgsVectorLayer *layer, int fieldIdx, QWidget *editor, QWidget *parent )
   : QgsEditorWidgetWrapper( layer, fieldIdx, editor, parent )
 {
@@ -70,21 +75,26 @@ QVariant QgsValueRelationWidgetWrapper::value() const
       }
     }
 
-    if ( layer()->fields().at( fieldIdx() ).type() == QVariant::Map )
+    QVariantList vl;
+    //store as QVariantList because it's json
+    for ( const QString &s : qgis::as_const( selection ) )
     {
-      QVariantList vl;
-      //store as QVariantList because it's json
-      for ( const QString &s : qgis::as_const( selection ) )
+      // Convert to proper type
+      const QVariant::Type type { fkType() };
+      switch ( type )
       {
-        vl << s;
+        case QVariant::Type::Int:
+          vl.push_back( s.toInt() );
+          break;
+        case QVariant::Type::LongLong:
+          vl.push_back( s.toLongLong() );
+          break;
+        default:
+          vl.push_back( s );
+          break;
       }
-      v = vl;
-    }
-    else
-    {
-      //store as hstore string
-      v = selection.join( ',' ).prepend( '{' ).append( '}' );
     }
+    v = vl;
   }
 
   if ( mLineEdit )
@@ -289,6 +299,21 @@ int QgsValueRelationWidgetWrapper::columnCount() const
   return std::max( 1, config( QStringLiteral( "NofColumns" ) ).toInt() );
 }
 
+QVariant::Type QgsValueRelationWidgetWrapper::fkType() const
+{
+  QgsVectorLayer *layer = QgsProject::instance()->mapLayer<QgsVectorLayer *>( config().value( QStringLiteral( "Layer" ) ).toString() );
+  if ( layer )
+  {
+    QgsFields fields = layer->fields();
+    int idx { fields.lookupField( config().value( QStringLiteral( "Key" ) ).toString() )  };
+    if ( idx >= 0 )
+    {
+      return fields.at( idx ).type();
+    }
+  }
+  return QVariant::Type::Invalid;
+}
+
 void QgsValueRelationWidgetWrapper::populate( )
 {
   // Initialize, note that signals are blocked, to avoid double signals on new features

diff --git a/src/gui/editorwidgets/qgsvaluerelationwidgetwrapper.h b/src/gui/editorwidgets/qgsvaluerelationwidgetwrapper.h
@@ -115,6 +115,9 @@ class GUI_EXPORT QgsValueRelationWidgetWrapper : public QgsEditorWidgetWrapper
      */
     int columnCount() const;
 
+    //! Returns the variant type of the fk
+    QVariant::Type fkType() const;
+
     //! Sets the values for the widgets, re-creates the cache when required
     void populate( );
 

diff --git a/src/providers/spatialite/qgsspatialitefeatureiterator.cpp b/src/providers/spatialite/qgsspatialitefeatureiterator.cpp
@@ -581,7 +581,10 @@ QVariant QgsSpatiaLiteFeatureIterator::getFeatureAttribute( sqlite3_stmt *stmt,
     {
       // assume arrays are stored as JSON
       QVariant result = QVariant( QgsJsonUtils::parseArray( txt, subType ) );
-      result.convert( type );
+      if ( ! result.convert( static_cast<int>( type ) ) )
+      {
+        QgsDebugMsgLevel( QStringLiteral( "Could not convert JSON value to requested QVariant type" ).arg( txt ), 3 );
+      }
       return result;
     }
     return txt;

diff --git a/src/providers/spatialite/qgsspatialiteprovider.cpp b/src/providers/spatialite/qgsspatialiteprovider.cpp
@@ -44,6 +44,9 @@ email                : a.furieri@lqt.it
 #include <QDir>
 #include <QRegularExpression>
 
+#include <nlohmann/json.hpp>
+using json = nlohmann::json;
+
 
 const QString SPATIALITE_KEY = QStringLiteral( "spatialite" );
 const QString SPATIALITE_DESCRIPTION = QStringLiteral( "SpatiaLite data provider" );
@@ -663,6 +666,10 @@ static TypeSubType getVariantType( const QString &type )
                                           type.length() - SPATIALITE_ARRAY_PREFIX.length() - SPATIALITE_ARRAY_SUFFIX.length() ) );
     return TypeSubType( subType.first == QVariant::String ? QVariant::StringList : QVariant::List, subType.first );
   }
+  else if ( type == QLatin1String( "jsonarray" ) )
+  {
+    return TypeSubType( QVariant::List, QVariant::Invalid );
+  }
   else
     // for sure any SQLite value can be represented as SQLITE_TEXT
     return TypeSubType( QVariant::String, QVariant::Invalid );
@@ -4385,6 +4392,7 @@ bool QgsSpatiaLiteProvider::changeAttributeValues( const QgsChangedAttributesMap
           first = false;
 
         QVariant::Type type = fld.type();
+        const auto typeName { fld.typeName() };
 
         if ( val.isNull() || !val.isValid() )
         {
@@ -4398,8 +4406,27 @@ bool QgsSpatiaLiteProvider::changeAttributeValues( const QgsChangedAttributesMap
         }
         else if ( type == QVariant::StringList || type == QVariant::List )
         {
-          // binding an array value
-          sql += QStringLiteral( "%1=%2" ).arg( QgsSqliteUtils::quotedIdentifier( fld.name() ), QgsSqliteUtils::quotedString( QgsJsonUtils::encodeValue( val ) ) );
+          // binding an array value, parse JSON
+          QString jRepr;
+          try
+          {
+            const auto jObj { QgsJsonUtils::jsonFromVariant( val ) };
+            if ( ! jObj.is_array() )
+            {
+              throw json::parse_error::create( 0, 0, tr( "JSON value must be an array" ).toStdString() );
+            }
+            jRepr = QString::fromStdString( jObj.dump( ) );
+            sql += QStringLiteral( "%1=%2" ).arg( QgsSqliteUtils::quotedIdentifier( fld.name() ),  QgsSqliteUtils::quotedString( jRepr ) );
+          }
+          catch ( json::exception ex )
+          {
+            const auto errM { tr( "Field type is JSON but the value cannot be converted to JSON array: %1" ).arg( ex.what() ) };
+            auto msgPtr { static_cast<char *>( sqlite3_malloc( errM.length() + 1 ) ) };
+            strcpy( static_cast<char *>( msgPtr ), errM.toStdString().c_str() );
+            errMsg = msgPtr;
+            handleError( jRepr, errMsg, true );
+            return false;
+          }
         }
         else
         {