Fix quoting and qgsattributedialog

for json features

src/core/qgsjsonutils.cpp

@@ -449,10 +449,14 @@ json QgsJsonUtils::jsonFromVariant( const QVariant &val )
 
 QVariant QgsJsonUtils::parseJson( const std::string &jsonString )
 {
+  // tracks whether entire json string is a primitive
+  bool isPrimitive = true;
+
   std::function<QVariant( json )> _parser { [ & ]( json jObj ) -> QVariant {
       QVariant result;
       if ( jObj.is_array() )
       {
+        isPrimitive = false;
         QVariantList results;
         for ( const auto &item : jObj )
         {
@@ -462,6 +466,7 @@ QVariant QgsJsonUtils::parseJson( const std::string &jsonString )
       }
       else if ( jObj.is_object() )
       {
+        isPrimitive = false;
         QVariantMap results;
         for ( const auto  &item : jObj.items() )
         {
@@ -492,7 +497,14 @@ QVariant QgsJsonUtils::parseJson( const std::string &jsonString )
         }
         else if ( jObj.is_string() )
         {
-          result = QString::fromStdString( jObj.get<std::string>() );
+          if ( isPrimitive && ( jObj.get<std::string>().length() == 0 || QString::fromStdString( jObj.get<std::string>() ).at( 0 ) != "\"" ) )
+          {
+            result = QString::fromStdString( jObj.get<std::string>() ).append( "\"" ).insert( 0, "\"" );
+          }
+          else
+          {
+            result = QString::fromStdString( jObj.get<std::string>() );
+          }
         }
         else if ( jObj.is_null() )
         {

src/gui/editorwidgets/qgstexteditwrapper.cpp

@@ -81,6 +81,13 @@ QVariant QgsTextEditWrapper::value() const
   }
   else if ( field().type() == QVariant::Map )
   {
+    // replace empty string (invalid) with quoted empty string
+    if ( v == "" )
+    {
+      QVariant qjson = QgsJsonUtils::parseJson( std::string( "\"\"" ) );
+      mInvalidJSON = false;
+      return qjson;
+    }
     if ( json::accept( v.toUtf8() ) )
     {
       QVariant qjson = QgsJsonUtils::parseJson( v.toStdString() );
@@ -90,10 +97,12 @@ QVariant QgsTextEditWrapper::value() const
     else
       // return null value if json is invalid
     {
-      if ( field().length() > 0 )
+      if ( v.length() > 0 )
       {
         mInvalidJSON = true;
-      } else {
+      }
+      else
+      {
         mInvalidJSON = false;
       }
       return QVariant();
@@ -189,7 +198,7 @@ void QgsTextEditWrapper::showIndeterminateState()
   if ( mLineEdit )
   {
     mLineEdit->blockSignals( true );
-    // for interdeminate state we need to clear the placeholder text - we want an empty line edit, not
+    // for indeterminate state we need to clear the placeholder text - we want an empty line edit, not
     // one showing the default value (e.g., "NULL")
     mLineEdit->setPlaceholderText( QString() );
   }
@@ -250,6 +259,11 @@ void QgsTextEditWrapper::setEnabled( bool enabled )
   }
 }
 
+bool QgsTextEditWrapper::isInvalidJSON()
+{
+  return mInvalidJSON;
+}
+
 void QgsTextEditWrapper::textChanged( const QString & )
 {
   if ( mLineEdit )

src/gui/editorwidgets/qgstexteditwrapper.h

@@ -59,6 +59,7 @@ class GUI_EXPORT QgsTextEditWrapper : public QgsEditorWidgetWrapper
   public:
     QVariant value() const override;
     void showIndeterminateState() override;
+    bool isInvalidJSON();
 
     /**
      * Add a hint text on the widget

src/gui/qgsattributedialog.cpp

@@ -21,6 +21,7 @@
 #include "qgshighlight.h"
 #include "qgsapplication.h"
 #include "qgssettings.h"
+#include "qgsmessagebar.h"
 
 QgsAttributeDialog::QgsAttributeDialog( QgsVectorLayer *vl, QgsFeature *thepFeature, bool featureOwner, QWidget *parent, bool showDialogButtons, const QgsAttributeEditorContext &context )
   : QDialog( parent )
@@ -65,8 +66,18 @@ void QgsAttributeDialog::setHighlight( QgsHighlight *h )
 
 void QgsAttributeDialog::accept()
 {
-  mAttributeForm->save();
-  QDialog::accept();
+  bool didSave = mAttributeForm->save();
+  if ( didSave )
+  {
+    QDialog::accept();
+  }
+  else
+  {
+    mMessageBar->pushMessage( QString(),
+                              tr( "Your JSON value is invalid and has not been saved" ),
+                              Qgis::MessageLevel::Critical,
+                              5 );
+  }
 }
 
 void QgsAttributeDialog::show()
@@ -91,6 +102,12 @@ void QgsAttributeDialog::init( QgsVectorLayer *layer, QgsFeature *feature, const
   setWindowTitle( tr( "%1 - Feature Attributes" ).arg( layer->name() ) );
   setLayout( new QGridLayout() );
   layout()->setMargin( 0 );
+  mMessageBar = new QgsMessageBar( this );
+  mMessageBar->setSizePolicy( QSizePolicy::MinimumExpanding, QSizePolicy::Fixed );
+  layout()->addWidget( mMessageBar );
+
+  setLayout( layout() );
+
   mTrackedVectorLayerTools.setVectorLayerTools( trackedContext.vectorLayerTools() );
   trackedContext.setVectorLayerTools( &mTrackedVectorLayerTools );
   if ( showDialogButtons )

src/gui/qgsattributedialog.h

@@ -113,6 +113,7 @@ class GUI_EXPORT QgsAttributeDialog : public QDialog
     QString mReturnvarname;
     QgsAttributeForm *mAttributeForm = nullptr;
     QgsFeature *mOwnedFeature = nullptr;
+    QgsMessageBar *mMessageBar = nullptr;
 
     QgsTrackedVectorLayerTools mTrackedVectorLayerTools;
 

src/gui/qgsattributeform.cpp

@@ -42,6 +42,7 @@
 #include "qgsapplication.h"
 #include "qgsexpressioncontextutils.h"
 #include "qgsfeaturerequest.h"
+#include "qgstexteditwrapper.h"
 
 #include <QDir>
 #include <QTextStream>
@@ -311,7 +312,6 @@ bool QgsAttributeForm::saveEdits()
   bool changedLayer = false;
 
   QgsFeature updatedFeature = QgsFeature( mFeature );
-
   if ( mFeature.isValid() || mMode == QgsAttributeEditorContext::AddFeatureMode )
   {
     bool doUpdate = false;
@@ -329,6 +329,12 @@ bool QgsAttributeForm::saveEdits()
       QgsEditorWidgetWrapper *eww = qobject_cast<QgsEditorWidgetWrapper *>( ww );
       if ( eww )
       {
+        // check for invalid JSON values
+        QgsTextEditWrapper *text_edit = qobject_cast<QgsTextEditWrapper *>( eww );
+        if ( text_edit && text_edit->isInvalidJSON() )
+        {
+          return false;
+        }
         QVariantList dstVars = QVariantList() << dst.at( eww->fieldIdx() );
         QVariantList srcVars = QVariantList() << eww->value();
         QList<int> fieldIndexes = QList<int>() << eww->fieldIdx();

src/providers/postgres/qgspostgresconn.cpp

@@ -1221,6 +1221,15 @@ QString QgsPostgresConn::quotedJsonValue( const QVariant &value )
 {
   if ( value.isNull() || !value.isValid() )
     return QStringLiteral( "null" );
+// where json is a string literal just construct it from that rather than dump
+  if ( value.type() == QVariant::String )
+  {
+    QString valueStr = value.toString();
+    if ( valueStr.at( 0 ) == "\"" && valueStr.at( valueStr.size() - 1 ) == "\"" )
+    {
+      return quotedString( value.toString() );
+    }
+  }
   const auto j = QgsJsonUtils::jsonFromVariant( value );
   return quotedString( QString::fromStdString( j.dump() ) );
 }

tests/src/gui/testqgstexteditwrapper.cpp

@@ -27,6 +27,7 @@
 #include <QTableWidget>
 #include "qgsgui.h"
 #include <nlohmann/json.hpp>
+#include "qgsjsonutils.h"
 
 class TestQgsTextEditWrapper : public QObject
 {
@@ -88,48 +89,84 @@ void TestQgsTextEditWrapper::testWithJsonInPostgres()
 
   // check text output from DB
   w_json.setFeature( vl_json->getFeature( 1 ) );
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "[1,2,3]" ) );
+  QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( w_json.value() ).dump() ), QStringLiteral( "[1,2,3]" ) );
   w_json.setFeature( vl_json->getFeature( 2 ) );
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "{\"a\":1,\"b\":2}" ) );
+  QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( w_json.value() ).dump() ), QStringLiteral( "{\"a\":1,\"b\":2}" ) );
 
   // check input into widget
   // test array
   widget->setText( QString( "[2]" ) );
   QVERIFY( w_json.value().isValid() );
   // w_json value is QListVariant
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "[2]" ) );
   QVERIFY( w_json.value().userType() == QMetaType::QVariantList );
+  QVERIFY( QgsJsonUtils::jsonFromVariant( w_json.value() ).is_array() );
+  QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( w_json.value() ).dump() ), QStringLiteral( "[2]" ) );
 
   //test object
   widget->setText( QString( "{\"foo\":\"bar\"}" ) );
   QVERIFY( w_json.value().isValid() );
   // w_json value is QMapVariant
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "{\"foo\":\"bar\"}" ) );
   QVERIFY( w_json.value().userType() == QMetaType::QVariantMap );
+  QVERIFY( QgsJsonUtils::jsonFromVariant( w_json.value() ).is_object() );
+  QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( w_json.value() ).dump() ), QStringLiteral( "{\"foo\":\"bar\"}" ) );
+
+  //test complex object
+  widget->setText( QString( "{\"foo\":\"bar\",\"baz\":[1,2,3]}" ) );
+  QVERIFY( w_json.value().isValid() );
+  QVERIFY( w_json.value().userType() == QMetaType::QVariantMap );
+  json complexJson =  QgsJsonUtils::jsonFromVariant( w_json.value() );
+  QVERIFY( complexJson.is_object() );
+  json jsonArr = complexJson.at( "baz" );
+  QCOMPARE( QString::fromStdString( jsonArr.dump() ), QStringLiteral( "[1,2,3]" ) );
 
   //test empty
   widget->setText( QString( "" ) );
   QVERIFY( !w_json.value().isValid() );
-  // w_json value is QMapVariant
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "" ) );
+
+  //test quoted empty
+  widget->setText( QString( "\"\"" ) );
+  QVERIFY( w_json.value().isValid() );
+  QVERIFY( QgsJsonUtils::jsonFromVariant( w_json.value() ).is_string() );
+  QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( w_json.value() ).front( ) ), QStringLiteral( "\" \"" ) );
 
   // test invalid JSON
   widget->setText( QString( "{\"body\";\"text\"}" ) );
   QVERIFY( !w_json.value().isValid() );
-  //invalid JSON will be parsed as text - it is up to the Postgres provider to reject it
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "" ) );
 
-  // test with bare integer (without container) which is valid JSON
+  // test with primitive integer (without container) which is valid JSON
   widget->setText( QString( "2" ) );
   QVERIFY( w_json.value().isValid() );
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "" ) );
   QCOMPARE( w_json.value(), QVariant( 2 ) );
   QVERIFY( w_json.value().userType() == QMetaType::Int );
+  QVERIFY( QgsJsonUtils::jsonFromVariant( w_json.value() ).is_number_integer() );
+  int n = QgsJsonUtils::jsonFromVariant( w_json.value() ).front();
+  QCOMPARE( QVariant( n ), QVariant( 2 ) );
+
+  // test with primitive boolean
+  widget->setText( QString( "true" ) );
+  QVERIFY( w_json.value().isValid() );
+  QCOMPARE( w_json.value(), QVariant( true ) );
+  QVERIFY( w_json.value().userType() == QMetaType::Bool );
+  QVERIFY( QgsJsonUtils::jsonFromVariant( w_json.value() ).is_boolean() );
+  bool x = QgsJsonUtils::jsonFromVariant( w_json.value() ).front();
+  QCOMPARE( QVariant( x ), QVariant( true ) );
+
+  // test with primitive null
+  widget->setText( QString( "null" ) );
+  QVERIFY( w_json.value().isNull() );
+  QVERIFY( QgsJsonUtils::jsonFromVariant( w_json.value() ).is_null() );
+  QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( w_json.value() ).dump() ), QStringLiteral( "null" ) );
 
   // test with bare string (not valid JSON)
   widget->setText( QString( "abc" ) );
-  QVERIFY( !w_json.value().isValid() ) ;
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "" ) );
+  QVERIFY( !w_json.value().isValid() );
+
+  // test with quoted string (valid JSON)
+  widget->setText( QString( "\"abc\"" ) );
+  QVERIFY( w_json.value().isValid() ) ;
+  QVERIFY( QgsJsonUtils::jsonFromVariant( w_json.value() ).is_string() );
+  // avoid dumping as strings are quoted, so would be double quoted
+  QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( w_json.value() ).front( ) ), QStringLiteral( "\"abc\"" ) );
 }
 
 void TestQgsTextEditWrapper::testWithJsonBInPostgres()
@@ -152,47 +189,85 @@ void TestQgsTextEditWrapper::testWithJsonBInPostgres()
 
   // check text output from DB
   w_json.setFeature( vl_json->getFeature( 1 ) );
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "[4,5,6]" ) );
+  QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( w_json.value() ).dump() ), QStringLiteral( "[4,5,6]" ) );
   w_json.setFeature( vl_json->getFeature( 2 ) );
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "{\"c\":4,\"d\":5}" ) );
+  QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( w_json.value() ).dump() ), QStringLiteral( "{\"c\":4,\"d\":5}" ) );
 
   // check input into widget
   // test array
   widget->setText( QString( "[2]" ) );
   QVERIFY( w_json.value().isValid() );
   // w_json value is QListVariant
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "[2]" ) );
   QVERIFY( w_json.value().userType() == QMetaType::QVariantList );
+  QVERIFY( QgsJsonUtils::jsonFromVariant( w_json.value() ).is_array() );
+  QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( w_json.value() ).dump() ), QStringLiteral( "[2]" ) );
 
   //test object
   widget->setText( QString( "{\"foo\":\"bar\"}" ) );
   QVERIFY( w_json.value().isValid() );
-  // w_json value is QMapVariant
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "{\"foo\":\"bar\"}" ) );
+  // w_json value is QMapVaJsonDocriant
+  QVERIFY( QgsJsonUtils::jsonFromVariant( w_json.value() ).is_object() );
+  QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( w_json.value() ).dump() ), QStringLiteral( "{\"foo\":\"bar\"}" ) );
+
+  //test complex object
+  widget->setText( QString( "{\"foo\":\"bar\",\"baz\":[1,2,3]}" ) );
+  QVERIFY( w_json.value().isValid() );
   QVERIFY( w_json.value().userType() == QMetaType::QVariantMap );
+  json complexJson =  QgsJsonUtils::jsonFromVariant( w_json.value() );
+  QVERIFY( complexJson.is_object() );
+  json jsonArr = complexJson.at( "baz" );
+  QCOMPARE( QString::fromStdString( jsonArr.dump() ), QStringLiteral( "[1,2,3]" ) );
+
 
   //test empty
   widget->setText( QString( "" ) );
   QVERIFY( !w_json.value().isValid() );
-  // w_json value is QMapVariant
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "" ) );
+
+  //test quoted empty
+  widget->setText( QString( "\"\"" ) );
+  QVERIFY( w_json.value().isValid() );
+  QVERIFY( QgsJsonUtils::jsonFromVariant( w_json.value() ).is_string() );
+  QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( w_json.value() ).front( ) ), QStringLiteral( "\" \"" ) );
 
   // test invalid JSON
   widget->setText( QString( "{\"body\";\"text\"}" ) );
   QVERIFY( !w_json.value().isValid() );
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "" ) );
 
-  // test with bare integer (without container) which is valid JSON
+  // test with primitive integer (without container) which is valid JSON
   widget->setText( QString( "2" ) );
   QVERIFY( w_json.value().isValid() );
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "" ) );
   QCOMPARE( w_json.value(), QVariant( 2 ) );
   QVERIFY( w_json.value().userType() == QMetaType::Int );
+  QVERIFY( QgsJsonUtils::jsonFromVariant( w_json.value() ).is_number_integer() );
+  int n = QgsJsonUtils::jsonFromVariant( w_json.value() ).front();
+  QCOMPARE( QVariant( n ), QVariant( 2 ) );
+
+  // test with primitive boolean
+  widget->setText( QString( "true" ) );
+  QVERIFY( w_json.value().isValid() );
+  QCOMPARE( w_json.value(), QVariant( true ) );
+  QVERIFY( w_json.value().userType() == QMetaType::Bool );
+  QVERIFY( QgsJsonUtils::jsonFromVariant( w_json.value() ).is_boolean() );
+  bool x = QgsJsonUtils::jsonFromVariant( w_json.value() ).front();
+  QCOMPARE( QVariant( x ), QVariant( true ) );
+
+  // test with primitive null
+  widget->setText( QString( "null" ) );
+  QVERIFY( w_json.value().isNull() );
+  QVERIFY( QgsJsonUtils::jsonFromVariant( w_json.value() ).is_null() );
+  QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( w_json.value() ).dump() ), QStringLiteral( "null" ) );
+
 
   // test with bare string (not valid JSON)
   widget->setText( QString( "abc" ) );
   QVERIFY( !w_json.value().isValid() );
-  QCOMPARE( QString::fromUtf8( QJsonDocument::fromVariant( w_json.value() ).toJson( QJsonDocument::Compact ).data() ), QStringLiteral( "" ) );
+
+  // test with quoted string (valid JSON)
+  widget->setText( QString( "\"abc\"" ) );
+  QVERIFY( w_json.value().isValid() ) ;
+  QVERIFY( QgsJsonUtils::jsonFromVariant( w_json.value() ).is_string() );
+  // avoid dumping as strings are quoted, so would be double quoted
+  QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( w_json.value() ).front( ) ), QStringLiteral( "\"abc\"" ) );
 }
 
 QGSTEST_MAIN( TestQgsTextEditWrapper )