Fix Map-based JSON handling in QgsField's convertCompatible()

nirvn · github-actions[bot] · commit c287d2d1e7df · 2023-04-14T07:41:18.000Z
diff --git a/src/core/qgsfield.cpp b/src/core/qgsfield.cpp
@@ -572,16 +572,28 @@ bool QgsField::convertCompatible( QVariant &v, QString *errorMessage ) const
     }
   }
 
-  if ( d->type == QVariant::String && ( d->typeName.compare( QLatin1String( "json" ), Qt::CaseInsensitive ) == 0 || d->typeName == QLatin1String( "jsonb" ) ) )
+  if ( d->typeName.compare( QLatin1String( "json" ), Qt::CaseInsensitive ) == 0 || d->typeName.compare( QLatin1String( "jsonb" ), Qt::CaseInsensitive ) == 0 )
   {
-    const QJsonDocument doc = QJsonDocument::fromVariant( v );
-    if ( !doc.isNull() )
+    if ( d->type == QVariant::String )
     {
-      v = QString::fromUtf8( doc.toJson( QJsonDocument::Compact ).constData() );
-      return true;
+      const QJsonDocument doc = QJsonDocument::fromVariant( v );
+      if ( !doc.isNull() )
+      {
+        v = QString::fromUtf8( doc.toJson( QJsonDocument::Compact ).constData() );
+        return true;
+      }
+      v = QVariant( d->type );
+      return false;
+    }
+    else if ( d->type == QVariant::Map )
+    {
+      if ( v.type() == QVariant::StringList || v.type() == QVariant::List || v.type() == QVariant::Map )
+      {
+        return true;
+      }
+      v = QVariant( d->type );
+      return false;
     }
-    v = QVariant( d->type );
-    return false;
   }
 
   if ( ( d->type == QVariant::StringList || ( d->type == QVariant::List && d->subType == QVariant::String ) )
diff --git a/tests/src/core/testqgsfield.cpp b/tests/src/core/testqgsfield.cpp
@@ -784,19 +784,37 @@ void TestQgsField::convertCompatible()
   QVariant vZero = 0;
   QVERIFY( intField.convertCompatible( vZero ) );
 
-  // Test json field conversion
-  const QgsField jsonField( QStringLiteral( "json" ), QVariant::String, QStringLiteral( "json" ) );
-  QVariant jsonValue = QVariant::fromValue( QVariantList() << 1 << 5 << 8 );
-  QVERIFY( jsonField.convertCompatible( jsonValue ) );
-  QCOMPARE( jsonValue.type(), QVariant::String );
-  QCOMPARE( jsonValue, QString( "[1,5,8]" ) );
-  QVariantMap variantMap;
-  variantMap.insert( QStringLiteral( "a" ), 1 );
-  variantMap.insert( QStringLiteral( "c" ), 3 );
-  jsonValue = QVariant::fromValue( variantMap );
-  QVERIFY( jsonField.convertCompatible( jsonValue ) );
-  QCOMPARE( jsonValue.type(), QVariant::String );
-  QCOMPARE( jsonValue, QString( "{\"a\":1,\"c\":3}" ) );
+  // Test string-based json field conversion
+  {
+    const QgsField jsonField( QStringLiteral( "json" ), QVariant::String, QStringLiteral( "json" ) );
+    QVariant jsonValue = QVariant::fromValue( QVariantList() << 1 << 5 << 8 );
+    QVERIFY( jsonField.convertCompatible( jsonValue ) );
+    QCOMPARE( jsonValue.type(), QVariant::String );
+    QCOMPARE( jsonValue, QString( "[1,5,8]" ) );
+    QVariantMap variantMap;
+    variantMap.insert( QStringLiteral( "a" ), 1 );
+    variantMap.insert( QStringLiteral( "c" ), 3 );
+    jsonValue = QVariant::fromValue( variantMap );
+    QVERIFY( jsonField.convertCompatible( jsonValue ) );
+    QCOMPARE( jsonValue.type(), QVariant::String );
+    QCOMPARE( jsonValue, QString( "{\"a\":1,\"c\":3}" ) );
+  }
+
+  // Test map-based json field (i.e. OGR geopackage JSON fields) conversion
+  {
+    const QgsField jsonField( QStringLiteral( "json" ), QVariant::Map, QStringLiteral( "json" ) );
+    QVariant jsonValue = QVariant::fromValue( QVariantList() << 1 << 5 << 8 );
+    QVERIFY( jsonField.convertCompatible( jsonValue ) );
+    QCOMPARE( jsonValue.type(), QVariant::List );
+    QCOMPARE( jsonValue, QVariantList() << 1 << 5 << 8 );
+    QVariantMap variantMap;
+    variantMap.insert( QStringLiteral( "a" ), 1 );
+    variantMap.insert( QStringLiteral( "c" ), 3 );
+    jsonValue = QVariant::fromValue( variantMap );
+    QVERIFY( jsonField.convertCompatible( jsonValue ) );
+    QCOMPARE( jsonValue.type(), QVariant::Map );
+    QCOMPARE( jsonValue, variantMap );
+  }
 
   // geometry field conversion
   const QgsField geometryField( QStringLiteral( "geometry" ), QVariant::UserType, QStringLiteral( "geometry" ) );

Original file line number	Diff line number	Diff line change
`@@ -572,16 +572,28 @@ bool QgsField::convertCompatible( QVariant &v, QString *errorMessage ) const`
`572`	`572`	`}`
`573`	`573`	`}`
`574`	`574`
`575`		`- if ( d->type == QVariant::String && ( d->typeName.compare( QLatin1String( "json" ), Qt::CaseInsensitive ) == 0 \|\| d->typeName == QLatin1String( "jsonb" ) ) )`
	`575`	`+ if ( d->typeName.compare( QLatin1String( "json" ), Qt::CaseInsensitive ) == 0 \|\| d->typeName.compare( QLatin1String( "jsonb" ), Qt::CaseInsensitive ) == 0 )`
`576`	`576`	`{`
`577`		`- const QJsonDocument doc = QJsonDocument::fromVariant( v );`
`578`		`- if ( !doc.isNull() )`
	`577`	`+ if ( d->type == QVariant::String )`
`579`	`578`	`{`
`580`		`- v = QString::fromUtf8( doc.toJson( QJsonDocument::Compact ).constData() );`
`581`		`- return true;`
	`579`	`+ const QJsonDocument doc = QJsonDocument::fromVariant( v );`
	`580`	`+ if ( !doc.isNull() )`
	`581`	`+ {`
	`582`	`+ v = QString::fromUtf8( doc.toJson( QJsonDocument::Compact ).constData() );`
	`583`	`+ return true;`
	`584`	`+ }`
	`585`	`+ v = QVariant( d->type );`
	`586`	`+ return false;`
	`587`	`+ }`
	`588`	`+ else if ( d->type == QVariant::Map )`
	`589`	`+ {`
	`590`	`+ if ( v.type() == QVariant::StringList \|\| v.type() == QVariant::List \|\| v.type() == QVariant::Map )`
	`591`	`+ {`
	`592`	`+ return true;`
	`593`	`+ }`
	`594`	`+ v = QVariant( d->type );`
	`595`	`+ return false;`
`582`	`596`	`}`
`583`		`- v = QVariant( d->type );`
`584`		`- return false;`
`585`	`597`	`}`
`586`	`598`
`587`	`599`	`if ( ( d->type == QVariant::StringList \|\| ( d->type == QVariant::List && d->subType == QVariant::String ) )`