csapi: Fix boolean query parameters incorrectly passed

17 files changed, 119 insertions, 76 deletions

diff --git a/lib/connection.cpp b/lib/connection.cpp
index 8adb18b5..ea74c1c4 100644
--- a/lib/connection.cpp
+++ b/lib/connection.cpp
@@ -30,6 +30,7 @@
 #include "csapi/leaving.h"
 #include "csapi/account-data.h"
 #include "csapi/joining.h"
+#include "csapi/to_device.h"
 #include "jobs/sendeventjob.h"
 #include "jobs/syncjob.h"
 #include "jobs/mediathumbnailjob.h"
@@ -570,6 +571,25 @@ ForgetRoomJob* Connection::forgetRoom(const QString& id)
     return forgetJob;
 }
 
+SendToDeviceJob* Connection::sendToDevices(const QString& eventType,
+        const UsersToDevicesToEvents& eventsMap) const
+{
+    QHash<QString, QHash<QString, QJsonObject>> json;
+    json.reserve(int(eventsMap.size()));
+    std::for_each(eventsMap.begin(), eventsMap.end(),
+        [&json] (const auto& userTodevicesToEvents) {
+            auto& jsonUser = json[userTodevicesToEvents.first];
+            const auto& devicesToEvents = userTodevicesToEvents.second;
+            std::for_each(devicesToEvents.begin(), devicesToEvents.end(),
+                [&jsonUser] (const auto& deviceToEvents) {
+                    jsonUser.insert(deviceToEvents.first,
+                                    deviceToEvents.second.toJson());
+                });
+        });
+    return callApi<SendToDeviceJob>(InBackground,
+                eventType, generateTxnId(), json);
+}
+
 QUrl Connection::homeserver() const
 {
     return d->data->baseUrl();
@@ -845,7 +865,7 @@ Connection::room_factory_t Connection::roomFactory =
 Connection::user_factory_t Connection::userFactory =
     [](Connection* c, const QString& id) { return new User(id, c); };
 
-QByteArray Connection::generateTxnId()
+QByteArray Connection::generateTxnId() const
 {
     return d->data->generateTxnId();
 }
diff --git a/lib/connection.h b/lib/connection.h
index 2c4c622d..5bf8e5ac 100644
--- a/lib/connection.h
+++ b/lib/connection.h
@@ -45,6 +45,8 @@ namespace QMatrixClient
     class UploadContentJob;
     class GetContentJob;
     class DownloadFileJob;
+    class SendToDeviceJob;
+    class Event;
 
     /** Enumeration with flags defining the network job running policy
      * So far only background/foreground flags are available.
@@ -77,6 +79,10 @@ namespace QMatrixClient
                 QT_VERSION >= QT_VERSION_CHECK(5, 5, 0),
                 QVariantHash, QVariantMap>;
 
+            using UsersToDevicesToEvents =
+                std::unordered_map<QString,
+                    std::unordered_map<QString, const Event&>>;
+
             enum RoomVisibility { PublishRoom, UnpublishRoom }; // FIXME: Should go inside CreateRoomJob
 
             explicit Connection(QObject* parent = nullptr);
@@ -234,7 +240,7 @@ namespace QMatrixClient
             /** Generates a new transaction id. Transaction id's are unique within
              * a single Connection object
              */
-            Q_INVOKABLE QByteArray generateTxnId();
+            Q_INVOKABLE QByteArray generateTxnId() const;
 
             template <typename T = Room>
             static void setRoomType()
@@ -347,6 +353,9 @@ namespace QMatrixClient
              */
             ForgetRoomJob* forgetRoom(const QString& id);
 
+            SendToDeviceJob* sendToDevices(const QString& eventType,
+                    const UsersToDevicesToEvents& eventsMap) const;
+
             // Old API that will be abolished any time soon. DO NOT USE.
 
             /** @deprecated Use callApi<PostMessageJob>() or Room::postMessage() instead */
diff --git a/lib/converters.h b/lib/converters.h
index ff06e232..711d0a7c 100644
--- a/lib/converters.h
+++ b/lib/converters.h
@@ -21,9 +21,11 @@
 #include <QtCore/QJsonObject>
 #include <QtCore/QJsonArray> // Includes <QtCore/QJsonValue>
 #include <QtCore/QDate>
+#include <QtCore/QUrlQuery>
 
 #include <unordered_map>
 #include <vector>
+#include <functional>
 #if 0 // Waiting for C++17
 #include <experimental/optional>
 
@@ -332,25 +334,39 @@ namespace QMatrixClient
     namespace _impl
     {
         // This one is for types that don't have isEmpty()
-        template <typename JsonT, typename = bool>
+        template <typename InserterT, typename JsonT, typename = bool>
         struct AddNode
         {
-            static void impl(QJsonObject& o, QString key, JsonT&& value)
+            static void impl(InserterT inserter, QString key, JsonT&& value)
             {
-                o.insert(std::move(key), std::forward<JsonT>(value));
+                inserter(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())>
+        template <typename InserterT, typename JsonT>
+        struct AddNode<InserterT, JsonT,
+                       decltype(std::declval<JsonT>().isEmpty())>
         {
-            static void impl(QJsonObject& o, QString key, JsonT&& value)
+            static void impl(InserterT inserter, QString key, JsonT&& value)
             {
                 if (!value.isEmpty())
-                    o.insert(std::move(key), std::forward<JsonT>(value));
+                    inserter(std::move(key), std::forward<JsonT>(value));
             }
         };
+
+        template <bool Force, typename InserterT, typename ValT>
+        inline void maybeAdd(InserterT inserter, QString key, ValT&& value)
+        {
+            auto&& json = toJson(std::forward<ValT>(value));
+            // QJsonValue doesn't have isEmpty and consumes all other types
+            // (QString, QJsonObject etc.).
+            AddNode<InserterT,
+                    std::conditional_t<Force, QJsonValue, decltype(json)>>
+                ::impl(inserter, std::move(key), std::move(json));
+
+        }
+
     }  // namespace _impl
 
     static constexpr bool IfNotEmpty = false;
@@ -358,10 +374,33 @@ namespace QMatrixClient
     template <bool Force = true, typename ValT>
     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));
+        using namespace std::placeholders;
+        _impl::maybeAdd<Force>(bind(&QJsonObject::insert, o, _1, _2),
+                               key, value);
+    }
+
+    template <bool Force = true, typename ValT>
+    inline void addToQuery(QUrlQuery& query, QString key, ValT&& value)
+    {
+        using namespace std::placeholders;
+        _impl::maybeAdd<Force>(
+            [&query] (QString k, auto&& jsonValue) {
+                query.addQueryItem(k, jsonValue.toString());
+        }, key, value);
+    }
+
+    template <bool Force = true>
+    inline void addToQuery(QUrlQuery& query, QString key, QString value)
+    {
+        if (Force || !value.isEmpty())
+            query.addQueryItem(key, value);
     }
+
+    template <bool Force = true>
+    inline void addToQuery(QUrlQuery& query, QString key, bool value)
+    {
+        query.addQueryItem(key,
+            value ? QStringLiteral("true") : QStringLiteral("false"));
+    }
+
 }  // namespace QMatrixClient
diff --git a/lib/csapi/content-repo.cpp b/lib/csapi/content-repo.cpp
index a127dfad..81329345 100644
--- a/lib/csapi/content-repo.cpp
+++ b/lib/csapi/content-repo.cpp
@@ -22,8 +22,7 @@ class UploadContentJob::Private
 BaseJob::Query queryToUploadContent(const QString& filename)
 {
     BaseJob::Query _q;
-    if (!filename.isEmpty())
-        _q.addQueryItem("filename", filename);
+    addToQuery<IfNotEmpty>(_q, "filename", filename);
     return _q;
 }
 
@@ -66,7 +65,7 @@ class GetContentJob::Private
 BaseJob::Query queryToGetContent(bool allowRemote)
 {
     BaseJob::Query _q;
-    _q.addQueryItem("allow_remote", QString("%1").arg(allowRemote));
+    addToQuery<IfNotEmpty>(_q, "allow_remote", allowRemote);
     return _q;
 }
 
@@ -123,7 +122,7 @@ class GetContentOverrideNameJob::Private
 BaseJob::Query queryToGetContentOverrideName(bool allowRemote)
 {
     BaseJob::Query _q;
-    _q.addQueryItem("allow_remote", QString("%1").arg(allowRemote));
+    addToQuery<IfNotEmpty>(_q, "allow_remote", allowRemote);
     return _q;
 }
 
@@ -179,13 +178,10 @@ class GetContentThumbnailJob::Private
 BaseJob::Query queryToGetContentThumbnail(Omittable<int> width, Omittable<int> height, const QString& method, bool allowRemote)
 {
     BaseJob::Query _q;
-    if (width)
-        _q.addQueryItem("width", QString("%1").arg(width.value()));
-    if (height)
-        _q.addQueryItem("height", QString("%1").arg(height.value()));
-    if (!method.isEmpty())
-        _q.addQueryItem("method", method);
-    _q.addQueryItem("allow_remote", QString("%1").arg(allowRemote));
+    addToQuery<IfNotEmpty>(_q, "width", width);
+    addToQuery<IfNotEmpty>(_q, "height", height);
+    addToQuery<IfNotEmpty>(_q, "method", method);
+    addToQuery<IfNotEmpty>(_q, "allow_remote", allowRemote);
     return _q;
 }
 
@@ -235,9 +231,8 @@ class GetUrlPreviewJob::Private
 BaseJob::Query queryToGetUrlPreview(const QString& url, Omittable<qint64> ts)
 {
     BaseJob::Query _q;
-    _q.addQueryItem("url", url);
-    if (ts)
-        _q.addQueryItem("ts", QString("%1").arg(ts.value()));
+    addToQuery<>(_q, "url", url);
+    addToQuery<IfNotEmpty>(_q, "ts", ts);
     return _q;
 }
 
diff --git a/lib/csapi/event_context.cpp b/lib/csapi/event_context.cpp
index 97494062..37de58ab 100644
--- a/lib/csapi/event_context.cpp
+++ b/lib/csapi/event_context.cpp
@@ -26,8 +26,7 @@ class GetEventContextJob::Private
 BaseJob::Query queryToGetEventContext(Omittable<int> limit)
 {
     BaseJob::Query _q;
-    if (limit)
-        _q.addQueryItem("limit", QString("%1").arg(limit.value()));
+    addToQuery<IfNotEmpty>(_q, "limit", limit);
     return _q;
 }
 
diff --git a/lib/csapi/gtad.yaml b/lib/csapi/gtad.yaml
index 76418f3c..55932378 100644
--- a/lib/csapi/gtad.yaml
+++ b/lib/csapi/gtad.yaml
@@ -124,7 +124,6 @@ mustache:
     joinedParamDecl: '{{>maybeCrefType}} {{paramName}}{{^required?}} = {{>initializeDefaultValue}}{{/required?}}{{#@join}}, {{/@join}}'
     joinedParamDef: '{{>maybeCrefType}} {{paramName}}{{#@join}}, {{/@join}}'
     passQueryParams: '{{#queryParams}}{{paramName}}{{#@join}}, {{/@join}}{{/queryParams}}'
-    paramToString: '{{#isString}}{{nameCamelCase}}{{/isString}}{{^isString}}QString("%1").arg({{nameCamelCase}}{{^required?}}{{#useOmittable}}.value(){{/useOmittable}}{{/required?}}){{/isString}}'
   #  preamble: preamble.mustache
     copyrightName: Kitsune Ral
     copyrightEmail: <kitsune-ral@users.sf.net>
diff --git a/lib/csapi/keys.cpp b/lib/csapi/keys.cpp
index 2d6885dd..88d6690b 100644
--- a/lib/csapi/keys.cpp
+++ b/lib/csapi/keys.cpp
@@ -164,8 +164,8 @@ class GetKeysChangesJob::Private
 BaseJob::Query queryToGetKeysChanges(const QString& from, const QString& to)
 {
     BaseJob::Query _q;
-    _q.addQueryItem("from", from);
-    _q.addQueryItem("to", to);
+    addToQuery<>(_q, "from", from);
+    addToQuery<>(_q, "to", to);
     return _q;
 }
 
diff --git a/lib/csapi/list_public_rooms.cpp b/lib/csapi/list_public_rooms.cpp
index f104786b..5952fd6e 100644
--- a/lib/csapi/list_public_rooms.cpp
+++ b/lib/csapi/list_public_rooms.cpp
@@ -100,12 +100,9 @@ class GetPublicRoomsJob::Private
 BaseJob::Query queryToGetPublicRooms(Omittable<int> limit, const QString& since, const QString& server)
 {
     BaseJob::Query _q;
-    if (limit)
-        _q.addQueryItem("limit", QString("%1").arg(limit.value()));
-    if (!since.isEmpty())
-        _q.addQueryItem("since", since);
-    if (!server.isEmpty())
-        _q.addQueryItem("server", server);
+    addToQuery<IfNotEmpty>(_q, "limit", limit);
+    addToQuery<IfNotEmpty>(_q, "since", since);
+    addToQuery<IfNotEmpty>(_q, "server", server);
     return _q;
 }
 
@@ -213,8 +210,7 @@ class QueryPublicRoomsJob::Private
 BaseJob::Query queryToQueryPublicRooms(const QString& server)
 {
     BaseJob::Query _q;
-    if (!server.isEmpty())
-        _q.addQueryItem("server", server);
+    addToQuery<IfNotEmpty>(_q, "server", server);
     return _q;
 }
 
diff --git a/lib/csapi/message_pagination.cpp b/lib/csapi/message_pagination.cpp
index d23dd911..1a7d6ee9 100644
--- a/lib/csapi/message_pagination.cpp
+++ b/lib/csapi/message_pagination.cpp
@@ -23,14 +23,11 @@ class GetRoomEventsJob::Private
 BaseJob::Query queryToGetRoomEvents(const QString& from, const QString& to, const QString& dir, Omittable<int> limit, const QString& filter)
 {
     BaseJob::Query _q;
-    _q.addQueryItem("from", from);
-    if (!to.isEmpty())
-        _q.addQueryItem("to", to);
-    _q.addQueryItem("dir", dir);
-    if (limit)
-        _q.addQueryItem("limit", QString("%1").arg(limit.value()));
-    if (!filter.isEmpty())
-        _q.addQueryItem("filter", filter);
+    addToQuery<>(_q, "from", from);
+    addToQuery<IfNotEmpty>(_q, "to", to);
+    addToQuery<>(_q, "dir", dir);
+    addToQuery<IfNotEmpty>(_q, "limit", limit);
+    addToQuery<IfNotEmpty>(_q, "filter", filter);
     return _q;
 }
 
diff --git a/lib/csapi/notifications.cpp b/lib/csapi/notifications.cpp
index 9329857d..de5b41f2 100644
--- a/lib/csapi/notifications.cpp
+++ b/lib/csapi/notifications.cpp
@@ -50,12 +50,9 @@ class GetNotificationsJob::Private
 BaseJob::Query queryToGetNotifications(const QString& from, Omittable<int> limit, const QString& only)
 {
     BaseJob::Query _q;
-    if (!from.isEmpty())
-        _q.addQueryItem("from", from);
-    if (limit)
-        _q.addQueryItem("limit", QString("%1").arg(limit.value()));
-    if (!only.isEmpty())
-        _q.addQueryItem("only", only);
+    addToQuery<IfNotEmpty>(_q, "from", from);
+    addToQuery<IfNotEmpty>(_q, "limit", limit);
+    addToQuery<IfNotEmpty>(_q, "only", only);
     return _q;
 }
 
diff --git a/lib/csapi/peeking_events.cpp b/lib/csapi/peeking_events.cpp
index 884abe00..53f2d280 100644
--- a/lib/csapi/peeking_events.cpp
+++ b/lib/csapi/peeking_events.cpp
@@ -23,12 +23,9 @@ class PeekEventsJob::Private
 BaseJob::Query queryToPeekEvents(const QString& from, Omittable<int> timeout, const QString& roomId)
 {
     BaseJob::Query _q;
-    if (!from.isEmpty())
-        _q.addQueryItem("from", from);
-    if (timeout)
-        _q.addQueryItem("timeout", QString("%1").arg(timeout.value()));
-    if (!roomId.isEmpty())
-        _q.addQueryItem("room_id", roomId);
+    addToQuery<IfNotEmpty>(_q, "from", from);
+    addToQuery<IfNotEmpty>(_q, "timeout", timeout);
+    addToQuery<IfNotEmpty>(_q, "room_id", roomId);
     return _q;
 }
 
diff --git a/lib/csapi/pushrules.cpp b/lib/csapi/pushrules.cpp
index eca46247..807f1d0f 100644
--- a/lib/csapi/pushrules.cpp
+++ b/lib/csapi/pushrules.cpp
@@ -99,10 +99,8 @@ DeletePushRuleJob::DeletePushRuleJob(const QString& scope, const QString& kind,
 BaseJob::Query queryToSetPushRule(const QString& before, const QString& after)
 {
     BaseJob::Query _q;
-    if (!before.isEmpty())
-        _q.addQueryItem("before", before);
-    if (!after.isEmpty())
-        _q.addQueryItem("after", after);
+    addToQuery<IfNotEmpty>(_q, "before", before);
+    addToQuery<IfNotEmpty>(_q, "after", after);
     return _q;
 }
 
diff --git a/lib/csapi/registration.cpp b/lib/csapi/registration.cpp
index 1e239d2a..85a3e256 100644
--- a/lib/csapi/registration.cpp
+++ b/lib/csapi/registration.cpp
@@ -24,8 +24,7 @@ class RegisterJob::Private
 BaseJob::Query queryToRegister(const QString& kind)
 {
     BaseJob::Query _q;
-    if (!kind.isEmpty())
-        _q.addQueryItem("kind", kind);
+    addToQuery<IfNotEmpty>(_q, "kind", kind);
     return _q;
 }
 
@@ -130,7 +129,7 @@ class CheckUsernameAvailabilityJob::Private
 BaseJob::Query queryToCheckUsernameAvailability(const QString& username)
 {
     BaseJob::Query _q;
-    _q.addQueryItem("username", username);
+    addToQuery<>(_q, "username", username);
     return _q;
 }
 
diff --git a/lib/csapi/search.cpp b/lib/csapi/search.cpp
index d43f704b..d172dfbe 100644
--- a/lib/csapi/search.cpp
+++ b/lib/csapi/search.cpp
@@ -173,8 +173,7 @@ class SearchJob::Private
 BaseJob::Query queryToSearch(const QString& nextBatch)
 {
     BaseJob::Query _q;
-    if (!nextBatch.isEmpty())
-        _q.addQueryItem("next_batch", nextBatch);
+    addToQuery<IfNotEmpty>(_q, "next_batch", nextBatch);
     return _q;
 }
 
diff --git a/lib/csapi/{{base}}.cpp.mustache b/lib/csapi/{{base}}.cpp.mustache
index 8a7dd3b0..0955984d 100644
--- a/lib/csapi/{{base}}.cpp.mustache
+++ b/lib/csapi/{{base}}.cpp.mustache
@@ -61,10 +61,7 @@ class {{camelCaseOperationId}}Job::Private
 BaseJob::Query queryTo{{camelCaseOperationId}}({{#queryParams}}{{>joinedParamDef}}{{/queryParams}})
 {
     BaseJob::Query _q;{{#queryParams}}
-{{^required?}}{{#isString}}    if (!{{nameCamelCase}}.isEmpty())
-    {{/isString}}{{!
-}}{{#useOmittable}}    if ({{nameCamelCase}})
-    {{/useOmittable}}{{/required?}}    _q.addQueryItem("{{baseName}}", {{>paramToString}});{{/queryParams}}
+    addToQuery<{{^required?}}IfNotEmpty{{/required?}}>(_q, "{{baseName}}", {{paramName}});{{/queryParams}}
     return _q;
 }
 {{/queryParams?}}{{^bodyParams}}
diff --git a/lib/events/event.h b/lib/events/event.h
index 8449c2ec..ed212759 100644
--- a/lib/events/event.h
+++ b/lib/events/event.h
@@ -100,6 +100,8 @@ namespace QMatrixClient
 
             const QJsonObject contentJson() const;
 
+            virtual QJsonObject toJson() const { Q_ASSERT(false); }
+
         private:
             Type _type;
             QJsonObject _originalJson;
@@ -273,7 +275,7 @@ namespace QMatrixClient
                 , _content(std::forward<ContentParamTs>(contentParams)...)
             { }
 
-            QJsonObject toJson() const { return _content.toJson(); }
+            QJsonObject toJson() const override { return _content.toJson(); }
 
             const ContentT& content() const { return _content; }
             /** @deprecated Use prevContent instead */
diff --git a/lib/events/roommessageevent.h b/lib/events/roommessageevent.h
index dc734b6e..075d7188 100644
--- a/lib/events/roommessageevent.h
+++ b/lib/events/roommessageevent.h
@@ -63,7 +63,7 @@ namespace QMatrixClient
             bool hasFileContent() const;
             bool hasThumbnail() const;
 
-            QJsonObject toJson() const;
+            QJsonObject toJson() const override;
 
             static constexpr const char* typeId() { return "m.room.message"; }