lib/csapi/: Don't send parameters that were omitted

This is important because payloads that include omitted parameters may be malformed. Closes #208 (an example of such malformed payload).

4 files changed, 101 insertions, 25 deletions

diff --git a/lib/converters.h b/lib/converters.h
index 4cf51cbd..1d1686cd 100644
--- a/lib/converters.h
+++ b/lib/converters.h
@@ -24,6 +24,12 @@
 
 #include <unordered_map>
 #include <vector>
+#if 0 // Waiting for C++17
+#include <experimental/optional>
+
+template <typename T>
+using optional = std::experimental::optional<T>;
+#endif
 
 // Enable std::unordered_map<QString, T>
 namespace std
@@ -43,9 +49,9 @@ namespace QMatrixClient
     inline QJsonValue toJson(const QJsonValue& val) { return val; }
     inline QJsonObject toJson(const QJsonObject& o) { return o; }
     inline QJsonArray toJson(const QJsonArray& arr) { return arr; }
-#ifdef _MSC_VER // MSVC gets lost and doesn't know which overload to use
-    inline QJsonValue toJson(const QString& s) { return s; }
-#endif
+    // Special-case QStrings so that we could use isEmpty() on them
+    // (see _impl::AddNote<> below)
+    inline QString toJson(const QString& s) { return s; }
 
     template <typename T>
     inline QJsonArray toJson(const std::vector<T>& vals)
@@ -70,9 +76,9 @@ namespace QMatrixClient
         return QJsonArray::fromStringList(strings);
     }
 
-    inline QJsonValue toJson(const QByteArray& bytes)
+    inline QString toJson(const QByteArray& bytes)
     {
-        return QJsonValue(bytes.constData());
+        return bytes.constData();
     }
 
     template <typename T>
@@ -93,6 +99,17 @@ namespace QMatrixClient
         return json;
     }
 
+#if 0
+    template <typename T>
+    inline auto toJson(const optional<T>& optVal)
+    {
+        if (optVal)
+            return toJson(optVal.value());
+
+        return decltype(toJson(std::declval<T>()))();
+    }
+#endif
+
     template <typename T>
     struct FromJson
     {
@@ -232,4 +249,54 @@ namespace QMatrixClient
             return h;
         }
     };
+
+    // Conditional insertion into a QJsonObject
+
+    namespace _impl
+    {
+        // This one is for types that don't have isEmpty()
+        template <typename JsonT, typename = bool>
+        struct AddNode
+        {
+            static void impl(QJsonObject& o, QString key, JsonT&& value)
+            {
+                o.insert(std::move(key), std::forward<JsonT>(value));
+            }
+        };
+
+        // This one is for types that have isEmpty()
+        template <typename JsonT>
+        struct AddNode<JsonT, decltype(std::declval<JsonT>().isEmpty())>
+        {
+            static void impl(QJsonObject& o, QString key, JsonT&& value)
+            {
+                if (!value.isEmpty())
+                    o.insert(std::move(key), std::forward<JsonT>(value));
+            }
+        };
+    }
+
+    static constexpr bool IfNotEmpty = false;
+
+    template <bool Force = true, typename ValT = void>
+    inline void addToJson(QJsonObject& o, QString key, ValT&& value)
+    {
+        auto&& json = toJson(std::forward<ValT>(value));
+        // QJsonValue doesn't have isEmpty and consumes all other types
+        // (QString, QJsonObject etc.).
+        _impl::AddNode<std::conditional_t<Force, QJsonValue, decltype(json)>>
+                ::impl(o, std::move(key), std::move(json));
+    }
+
+    /** Construct an "omitted" value of a given type
+     * This is a workaround for the time being while we're not C++17 and
+     * cannot use `optional`.
+     */
+    template <typename T>
+    inline T omitted()
+    {
+        T val;
+        val.omitted = true;
+        return val;
+    }
 }  // namespace QMatrixClient
diff --git a/lib/csapi/gtad.yaml b/lib/csapi/gtad.yaml
index c7cbc73d..cb68ad1b 100644
--- a/lib/csapi/gtad.yaml
+++ b/lib/csapi/gtad.yaml
@@ -27,12 +27,10 @@ analyzer:
     number:
     - float: float
     - //: double
-    boolean: { type: bool, initializer: false }
+    boolean: { type: bool, omitted: false }
     string:
     - byte: &ByteStream
         type: QIODevice*
-        #initializer: '"{{defaultValue}}"'
-        #string?: true
         imports: <QtCore/QIODevice>
     - binary: *ByteStream
     - date:
@@ -114,6 +112,8 @@ analyzer:
       imports: '"variant_converters.h"'
     schema: # Properties of inline structure definitions
       avoidCopy?: true
+      omitted: omitted<{{dataType.name}}>()
+      imports: '"converters.h"'
 
   #operations:
 
@@ -121,10 +121,12 @@ mustache:
   definitions:
     _scopeRenderer: "{{scopeCamelCase}}Job::"
     _literalQuote: '"'
+    # Default value for omitted
+    omitted: '{}'
     maybeCrefType: "{{#avoidCopy?}}const {{/avoidCopy?}}{{dataType.name}}{{#avoidCopy?}}&{{/avoidCopy?}}{{#noCopy?}}&&{{/noCopy?}}"
     qualifiedMaybeCrefType:
       "{{#avoidCopy?}}const {{/avoidCopy?}}{{dataType.qualifiedName}}{{#avoidCopy?}}&{{/avoidCopy?}}{{#noCopy?}}&&{{/noCopy?}}"
-    initializeDefaultValue: "{{#defaultValue}}{{>initializer}}{{/defaultValue}}{{^defaultValue}}{}{{/defaultValue}}"
+    initializeDefaultValue: "{{#defaultValue}}{{#initializer}}{{>initializer}}{{/initializer}}{{/defaultValue}}{{^defaultValue}}{{>omitted}}{{/defaultValue}}"
     joinedParamDecl: '{{>maybeCrefType}} {{paramName}}{{^required?}} = {{>initializeDefaultValue}}{{/required?}}{{#@join}}, {{/@join}}'
     joinedParamDef: '{{>maybeCrefType}} {{paramName}}{{#@join}}, {{/@join}}'
     passQueryParams: '{{#queryParams}}{{paramName}}{{#@join}}, {{/@join}}{{/queryParams}}'
diff --git a/lib/csapi/{{base}}.cpp.mustache b/lib/csapi/{{base}}.cpp.mustache
index 72ea56c9..c6504f42 100644
--- a/lib/csapi/{{base}}.cpp.mustache
+++ b/lib/csapi/{{base}}.cpp.mustache
@@ -10,18 +10,20 @@ using namespace QMatrixClient;
 {{#models.model}}{{#in?}}
 QJsonObject QMatrixClient::toJson(const {{qualifiedName}}& pod)
 {
-    QJsonObject o;
-{{#vars}}    o.insert("{{baseName}}", toJson(pod.{{nameCamelCase}}));
-{{/vars}}
-    return o;
+    QJsonObject _json;
+    if (pod.omitted)
+        return _json;
+{{#vars}}
+    addToJson<{{^required?}}IfNotEmpty{{/required?}}>(_json, "{{baseName}}", pod.{{nameCamelCase}});{{/vars}}
+    return _json;
 }
 {{/in?}}{{#out?}}
 {{qualifiedName}} FromJson<{{qualifiedName}}>::operator()(const QJsonValue& jv)
 {
-    const auto& o = jv.toObject();
+    const auto& _json = jv.toObject();
     {{qualifiedName}} result;
     {{#vars}}result.{{nameCamelCase}} =
-        fromJson<{{dataType.name}}>(o.value("{{baseName}}"));
+        fromJson<{{dataType.name}}>(_json.value("{{baseName}}"));
     {{/vars}}
     return result;
 }
@@ -34,20 +36,22 @@ namespace QMatrixClient
 {{#model}}{{#in?}}
     QJsonObject toJson(const {{qualifiedName}}& pod)
     {
-        QJsonObject o;
-{{#vars}}        o.insert("{{baseName}}", toJson(pod.{{nameCamelCase}}));
-{{/vars}}
-        return o;
+        QJsonObject _json;
+        if (pod.omitted)
+            return _json;
+{{#vars}}
+        addToJson<{{^required?}}IfNotEmpty{{/required?}}>(_json, "{{baseName}}", pod.{{nameCamelCase}});{{/vars}}
+        return _json;
     }
 {{/in?}}{{#out?}}
     template <> struct FromJson<{{qualifiedName}}>
     {
         {{qualifiedName}} operator()(const QJsonValue& jv)
         {
-            const auto& o = jv.toObject();
+            const auto& _json = jv.toObject();
             {{qualifiedName}} result;
 {{#vars}}            result.{{nameCamelCase}} =
-                fromJson<{{dataType.qualifiedName}}>(o.value("{{baseName}}"));
+                fromJson<{{dataType.qualifiedName}}>(_json.value("{{baseName}}"));
 {{/vars}}
             return result;
         }
@@ -90,8 +94,7 @@ QUrl {{camelCaseOperationId}}Job::makeRequestUrl(QUrl baseUrl{{#allParams?}}, {{
                         }}{{#consumesNonJson?}}{{nameCamelCase}}{{/consumesNonJson?
                         }}{{^consumesNonJson?}}toJson({{nameCamelCase}}){{/consumesNonJson?}}));{{/inlineBody
 }}{{^inlineBody}}    QJsonObject _data;{{#bodyParams}}
-{{^required?}}{{#string?}}    if (!{{paramName}}.isEmpty())
-    {{/string?}}{{/required?}}    _data.insert("{{baseName}}", toJson({{paramName}}));{{/bodyParams}}
+    addToJson<{{^required?}}IfNotEmpty{{/required?}}>(_data, "{{baseName}}", {{paramName}});{{/bodyParams}}
     setRequestData(_data);{{/inlineBody}}
 {{/bodyParams?}}{{#producesNonJson?}}    setExpectedContentTypes({ {{#produces}}"{{_}}"{{#@join}}, {{/@join}}{{/produces}} });
 {{/producesNonJson?}}}{{!<- mind the actual brace}}
diff --git a/lib/csapi/{{base}}.h.mustache b/lib/csapi/{{base}}.h.mustache
index 2eb2c656..6f08e871 100644
--- a/lib/csapi/{{base}}.h.mustache
+++ b/lib/csapi/{{base}}.h.mustache
@@ -14,7 +14,9 @@ namespace QMatrixClient
     struct {{name}}{{#parents?}} : {{#parents}}{{name}}{{#@join}}, {{/@join}}{{/parents}}{{/parents?}}
     {
 {{#vars}}        {{dataType.name}} {{nameCamelCase}};
-{{/vars}}    };
+{{/vars}}{{#in?}}
+        bool omitted;
+{{/in?}}    };
 {{#in?}}
     QJsonObject toJson(const {{name}}& pod);
 {{/in?}}{{#out?}}
@@ -33,7 +35,9 @@ namespace QMatrixClient
             struct {{name}}{{#parents?}} : {{#parents}}{{name}}{{#@join}}, {{/@join}}{{/parents}}{{/parents?}}
             {
 {{#vars}}                {{dataType.name}} {{nameCamelCase}};
-{{/vars}}            };
+{{/vars}}{{#in?}}
+                bool omitted;
+{{/in?}}            };
 {{/            model}}
             // Construction/destruction
 {{/        models}}