From c1fc8a115d72486a2e92b7111ff98754aa708b21 Mon Sep 17 00:00:00 2001 From: Nathan Osman Date: Thu, 13 Oct 2016 00:38:39 -0700 Subject: [PATCH] Remove all JSON marshalling code from QObjectHandler. --- doc/Doxyfile.in | 2 +- include/QHttpEngine/qobjecthandler.h | 95 ++++++++++----------- src/qobjecthandler.cpp | 123 ++++++++------------------- src/qobjecthandler_p.h | 11 +-- tests/TestQObjectHandler.cpp | 111 ++++++------------------ 5 files changed, 107 insertions(+), 235 deletions(-) diff --git a/doc/Doxyfile.in b/doc/Doxyfile.in index 54f218d..3b96ecd 100644 --- a/doc/Doxyfile.in +++ b/doc/Doxyfile.in @@ -1973,7 +1973,7 @@ INCLUDE_FILE_PATTERNS = # recursively expanded use the := operator instead of the = operator. # This tag requires that the tag ENABLE_PREPROCESSING is set to YES. -PREDEFINED = +PREDEFINED = DOXYGEN # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then this # tag can be used to specify a list of macro names that should be expanded. The diff --git a/include/QHttpEngine/qobjecthandler.h b/include/QHttpEngine/qobjecthandler.h index 84e79ce..73e892c 100644 --- a/include/QHttpEngine/qobjecthandler.h +++ b/include/QHttpEngine/qobjecthandler.h @@ -39,10 +39,7 @@ class QHTTPENGINE_EXPORT QObjectHandlerPrivate; * This handler enables incoming requests to be processed by slots in a * QObject-derived class or functor. Methods are registered by providing a * name and slot to invoke. The slot may take a pointer to the QHttpSocket for - * the request as an argument. For requests that include a body, the content - * is parsed as a JSON object and provided as a second parameter to the slot. - * The slot is expected to return a QVariantMap containing the response, which - * will be encoded as a JSON-document. + * the request as an argument. * * To use this class, simply create an instance and call the appropriate * registerMethod() overload. For example: @@ -52,7 +49,7 @@ class QHTTPENGINE_EXPORT QObjectHandlerPrivate; * { * Q_OBJECT * public slots: - * QVariantMap something(QHttpSocket *socket); + * void something(QHttpSocket *socket); * }; * * QObjectHandler handler; @@ -69,7 +66,7 @@ class QHTTPENGINE_EXPORT QObjectHandlerPrivate; * @code * QObjectHandler handler; * handler.registerMethod("something", [](QHttpSocket *socket) { - * return QVariantMap(); + * // do something * }); * @endcode */ @@ -89,62 +86,64 @@ public: * * This overload uses the traditional connection syntax with macros. */ - void registerMethod(const QString &name, QObject *receiver, const char *method, int acceptedStatusCodes = QHttpSocket::GET); + void registerMethod(const QString &name, QObject *receiver, const char *method); +#ifdef DOXYGEN /** * @brief Register a method * * This overload uses the new connection syntax with member pointers. */ - template - inline void registerMethod(const QString &name, - typename QtPrivate::FunctionPointer::Object *receiver, - Func1 slot, int acceptedStatusCodes = QHttpSocket::GET) { - - typedef QtPrivate::FunctionPointer SlotType; - - // Ensure the slot doesn't have too many parameters - Q_STATIC_ASSERT_X(int(SlotType::ArgumentCount) <= 2, - "The slot must have no more than two arguments."); - - // Ensure the parameters are of the correct type - Q_STATIC_ASSERT_X((QtPrivate::CheckCompatibleArguments, typename SlotType::Arguments>::value), - "The slot parameters do not match"); - - // Ensure the return value is correct - Q_STATIC_ASSERT_X((QtPrivate::AreArgumentsCompatible::value), - "Return type of the slot is not compatible with the return type of the signal."); - - // Invoke the implementation - registerMethodImpl(name, receiver, - new QtPrivate::QSlotObject(slot), - acceptedStatusCodes); - } + void registerMethod(const QString &name, QObject *receiver, PointerToMemberFunction method); /** * @brief Register a method * * This overload uses the new functor syntax (without context). */ - template - inline void registerMethod(const QString &name, Func1 slot, int acceptedStatusCodes = QHttpSocket::GET) { - registerMethod(name, Q_NULLPTR, slot, acceptedStatusCodes); - } + void registerMethod(const QString &name, Functor functor); /** * @brief Register a method * * This overload uses the new functor syntax (with context). */ + void registerMethod(const QString &name, QObject *receiver, Functor functor); +#else + template + inline void registerMethod(const QString &name, + typename QtPrivate::FunctionPointer::Object *receiver, + Func1 slot) { + + typedef QtPrivate::FunctionPointer SlotType; + + // Ensure the slot doesn't have too many arguments + Q_STATIC_ASSERT_X(int(SlotType::ArgumentCount) == 1, + "The slot must have exactly one argument."); + + // Ensure the argument is of the correct type + Q_STATIC_ASSERT_X((QtPrivate::AreArgumentsCompatible::Value>::value), + "The slot parameters do not match"); + + // Invoke the implementation + registerMethodImpl(name, receiver, new QtPrivate::QSlotObject(slot)); + } + + template + inline void registerMethod(const QString &name, Func1 slot) { + registerMethod(name, Q_NULLPTR, slot); + } + template inline typename QtPrivate::QEnableIf::IsPointerToMemberFunction && !QtPrivate::is_same::value, void>::Type - registerMethod(const QString &name, QObject *context, Func1 slot, int acceptedStatusCodes = QHttpSocket::GET) { + registerMethod(const QString &name, QObject *context, Func1 slot) { // There is an easier way to do this but then the header wouldn't // compile on non-C++11 compilers - return registerMethod_functor(name, context, slot, &Func1::operator(), acceptedStatusCodes); + return registerMethod_functor(name, context, slot, &Func1::operator()); } +#endif protected: @@ -156,29 +155,23 @@ protected: private: template - inline void registerMethod_functor(const QString &name, QObject *context, Func1 slot, Func1Operator, int acceptedStatusCodes) { + inline void registerMethod_functor(const QString &name, QObject *context, Func1 slot, Func1Operator) { typedef QtPrivate::FunctionPointer SlotType; - // Ensure the slot doesn't have too many parameters - Q_STATIC_ASSERT_X(int(SlotType::ArgumentCount) <= 2, - "The slot must have no more than two arguments."); + // Ensure the slot doesn't have too many arguments + Q_STATIC_ASSERT_X(int(SlotType::ArgumentCount) == 1, + "The slot must have exactly one argument."); - // Ensure the parameters are of the correct type - Q_STATIC_ASSERT_X((QtPrivate::CheckCompatibleArguments, typename SlotType::Arguments>::value), + // Ensure the argument is of the correct type + Q_STATIC_ASSERT_X((QtPrivate::AreArgumentsCompatible::Value>::value), "The slot parameters do not match"); - // Ensure the return value is correct - Q_STATIC_ASSERT_X((QtPrivate::AreArgumentsCompatible::value), - "Return type of the slot is not compatible with the return type of the signal."); - registerMethodImpl(name, context, - new QtPrivate::QFunctorSlotObject, SlotType::ArgumentCount>::Value, void>(slot), - acceptedStatusCodes); + new QtPrivate::QFunctorSlotObject(slot)); } - void registerMethodImpl(const QString &name, QObject *receiver, QtPrivate::QSlotObjectBase *slotObj, int acceptedStatusCodes); + void registerMethodImpl(const QString &name, QObject *receiver, QtPrivate::QSlotObjectBase *slotObj); QObjectHandlerPrivate *const d; friend class QObjectHandlerPrivate; diff --git a/src/qobjecthandler.cpp b/src/qobjecthandler.cpp index f0b38ff..bbe9d20 100644 --- a/src/qobjecthandler.cpp +++ b/src/qobjecthandler.cpp @@ -21,9 +21,6 @@ */ #include -#include -#include -#include #include #include @@ -36,75 +33,6 @@ QObjectHandlerPrivate::QObjectHandlerPrivate(QObjectHandler *handler) { } -void QObjectHandlerPrivate::invokeSlot(QHttpSocket *socket, const QString &path) -{ - Method m = map.value(path); - QVariantMap parameters; - - // If data was supplied, decode it as JSON - if (socket->bytesAvailable()) { - QJsonParseError error; - QJsonDocument document = QJsonDocument::fromJson(socket->readAll(), &error); - - // Ensure that the document is valid - if (error.error != QJsonParseError::NoError) { - socket->writeError(QHttpSocket::BadRequest); - return; - } - - parameters = document.object().toVariantMap(); - } - - QVariantMap retVal; - - // Invoke the slot - if (m.oldSlot) { - - // Obtain the slot index - int index = m.receiver->metaObject()->indexOfSlot(m.slot.method + 1); - if (index == -1) { - socket->writeError(QHttpSocket::InternalServerError); - return; - } - - QMetaMethod method = m.receiver->metaObject()->method(index); - - // Ensure the parameters are correct - QList params = method.parameterTypes(); - if (params.count() > 0 && params.at(0) != "QHttpSocket*" || - params.count() > 1 && params.at(1) != "QVariantMap" || - params.count() > 2 || - method.returnType() != QMetaType::QVariantMap) { - socket->writeError(QHttpSocket::InternalServerError); - return; - } - - // Invoke the method - if (!m.receiver->metaObject()->method(index).invoke( - m.receiver, - Q_RETURN_ARG(QVariantMap, retVal), - Q_ARG(QHttpSocket*, socket), - Q_ARG(QVariantMap, parameters))) { - socket->writeError(QHttpSocket::InternalServerError); - return; - } - } else { - void *args[3] = { - &retVal, - &socket, - ¶meters - }; - m.slot.slotObj->call(m.receiver, args); - } - - // Convert the return value to JSON and write it to the socket - QByteArray data = QJsonDocument(QJsonObject::fromVariantMap(retVal)).toJson(); - socket->setHeader("Content-Length", QByteArray::number(data.length())); - socket->setHeader("Content-Type", "application/json"); - socket->write(data); - socket->close(); -} - QObjectHandler::QObjectHandler(QObject *parent) : QHttpHandler(parent), d(new QObjectHandlerPrivate(this)) @@ -119,31 +47,48 @@ void QObjectHandler::process(QHttpSocket *socket, const QString &path) return; } - // Ensure the method is accepted QObjectHandlerPrivate::Method m = d->map.value(path); - if (!(m.acceptedMethods & socket->method())) { - // TODO: accept header - socket->writeError(QHttpSocket::MethodNotAllowed); - return; - } - // If the slot has finished receiving all of the data, jump directly to - // invokeSlot(), otherwise, wait until we have the rest of it - if (socket->bytesAvailable() >= socket->contentLength()) { - d->invokeSlot(socket, path); + // Invoke the slot + if (m.oldSlot) { + + // Obtain the slot index + int index = m.receiver->metaObject()->indexOfSlot(m.slot.method + 1); + if (index == -1) { + socket->writeError(QHttpSocket::InternalServerError); + return; + } + + QMetaMethod method = m.receiver->metaObject()->method(index); + + // Ensure the parameter is correct + QList params = method.parameterTypes(); + if (params.count() != 1 || params.at(0) != "QHttpSocket*") { + socket->writeError(QHttpSocket::InternalServerError); + return; + } + + // Invoke the method + if (!m.receiver->metaObject()->method(index).invoke( + m.receiver, Q_ARG(QHttpSocket*, socket))) { + socket->writeError(QHttpSocket::InternalServerError); + return; + } } else { - connect(socket, &QHttpSocket::readChannelFinished, [this, socket, path]() { - d->invokeSlot(socket, path); - }); + void *args[] = { + Q_NULLPTR, + &socket + }; + m.slot.slotObj->call(m.receiver, args); } } -void QObjectHandler::registerMethod(const QString &name, QObject *receiver, const char *method, int acceptedStatusCodes) +void QObjectHandler::registerMethod(const QString &name, QObject *receiver, const char *method) { - d->map.insert(name, QObjectHandlerPrivate::Method(receiver, method, acceptedStatusCodes)); + d->map.insert(name, QObjectHandlerPrivate::Method(receiver, method)); } -void QObjectHandler::registerMethodImpl(const QString &name, QObject *receiver, QtPrivate::QSlotObjectBase *slotObj, int acceptedStatusCodes) +void QObjectHandler::registerMethodImpl(const QString &name, QObject *receiver, QtPrivate::QSlotObjectBase *slotObj) { - d->map.insert(name, QObjectHandlerPrivate::Method(receiver, slotObj, acceptedStatusCodes)); + d->map.insert(name, QObjectHandlerPrivate::Method(receiver, slotObj)); } diff --git a/src/qobjecthandler_p.h b/src/qobjecthandler_p.h index db04bca..c314b61 100644 --- a/src/qobjecthandler_p.h +++ b/src/qobjecthandler_p.h @@ -37,18 +37,16 @@ public: explicit QObjectHandlerPrivate(QObjectHandler *handler); - void invokeSlot(QHttpSocket *socket, const QString &path); - // In order to invoke the slot, a "pointer" to it needs to be stored in a // map that lets us look up information by method name class Method { public: Method() {} - Method(QObject *receiver, const char *method, int acceptedMethods) - : receiver(receiver), oldSlot(true), slot(method), acceptedMethods(acceptedMethods) {} - Method(QObject *receiver, QtPrivate::QSlotObjectBase *slotObj, int acceptedMethods) - : receiver(receiver), oldSlot(false), slot(slotObj), acceptedMethods(acceptedMethods) {} + Method(QObject *receiver, const char *method) + : receiver(receiver), oldSlot(true), slot(method) {} + Method(QObject *receiver, QtPrivate::QSlotObjectBase *slotObj) + : receiver(receiver), oldSlot(false), slot(slotObj) {} QObject *receiver; bool oldSlot; @@ -59,7 +57,6 @@ public: const char *method; QtPrivate::QSlotObjectBase *slotObj; } slot; - int acceptedMethods; }; QMap map; diff --git a/tests/TestQObjectHandler.cpp b/tests/TestQObjectHandler.cpp index 3bb7716..5e44867 100644 --- a/tests/TestQObjectHandler.cpp +++ b/tests/TestQObjectHandler.cpp @@ -20,11 +20,8 @@ * IN THE SOFTWARE. */ -#include -#include #include #include -#include #include #include @@ -38,12 +35,11 @@ class DummyAPI : public QObject public Q_SLOTS: - int invalidReturnValue() { return 0; } - QVariantMap invalidArguments(int) { return QVariantMap(); } - QVariantMap noParameters() { return QVariantMap(); } - QVariantMap oneParameter(QHttpSocket *) { return QVariantMap(); } - QVariantMap twoParameters(QHttpSocket *, QVariantMap) { return QVariantMap(); } - QVariantMap echoPost(QHttpSocket *, QVariantMap d) { return d; } + void wrongArgumentCount() {} + void wrongArgumentType(int) {} + void valid(QHttpSocket *socket) { + socket->writeError(QHttpSocket::OK); + } }; class TestQObjectHandler : public QObject @@ -59,75 +55,31 @@ private Q_SLOTS: void TestQObjectHandler::testOldConnection_data() { - QTest::addColumn("registerPost"); - QTest::addColumn("requestPost"); QTest::addColumn("slot"); QTest::addColumn("statusCode"); - QTest::addColumn("data"); - QTest::newRow("invalid return") - << false - << false - << QByteArray(SLOT(invalidReturnValue())) - << static_cast(QHttpSocket::InternalServerError) - << QVariantMap(); + QTest::newRow("wrong argument count") + << QByteArray(SLOT(wrongArgumentCount())) + << static_cast(QHttpSocket::InternalServerError); - QTest::newRow("invalid arguments") - << false - << false - << QByteArray(SLOT(invalidArguments(int))) - << static_cast(QHttpSocket::InternalServerError) - << QVariantMap(); + QTest::newRow("wrong argument type") + << QByteArray(SLOT(wrongArgumentType(int))) + << static_cast(QHttpSocket::InternalServerError); - QTest::newRow("no parameters") - << false - << false - << QByteArray(SLOT(noParameters())) - << static_cast(QHttpSocket::OK) - << QVariantMap(); - - QTest::newRow("one parameter") - << false - << false - << QByteArray(SLOT(oneParameter(QHttpSocket*))) - << static_cast(QHttpSocket::OK) - << QVariantMap(); - - QTest::newRow("two parameters") - << false - << false - << QByteArray(SLOT(twoParameters(QHttpSocket*,QVariantMap))) - << static_cast(QHttpSocket::OK) - << QVariantMap(); - - QTest::newRow("invalid method") - << true - << false - << QByteArray(SLOT(echoPost(QHttpSocket*,QVariantMap))) - << static_cast(QHttpSocket::MethodNotAllowed) - << QVariantMap(); - - QTest::newRow("post data") - << true - << true - << QByteArray(SLOT(echoPost(QHttpSocket*,QVariantMap))) - << static_cast(QHttpSocket::OK) - << QVariantMap{{"a", "a"}, {"b", 1}}; + QTest::newRow("valid") + << QByteArray(SLOT(valid(QHttpSocket*))) + << static_cast(QHttpSocket::OK); } void TestQObjectHandler::testOldConnection() { - QFETCH(bool, registerPost); - QFETCH(bool, requestPost); QFETCH(QByteArray, slot); QFETCH(int, statusCode); - QFETCH(QVariantMap, data); QObjectHandler handler; DummyAPI api; - handler.registerMethod("test", &api, slot.constData(), - registerPost ? QHttpSocket::POST : QHttpSocket::GET); + handler.registerMethod("test", &api, slot.constData()); QSocketPair pair; QTRY_VERIFY(pair.isConnected()); @@ -135,26 +87,11 @@ void TestQObjectHandler::testOldConnection() QSimpleHttpClient client(pair.client()); QHttpSocket socket(pair.server(), &pair); - if (requestPost) { - QByteArray buff = QJsonDocument(QJsonObject::fromVariantMap(data)).toJson(); - client.sendHeaders("POST", "test", QHttpSocket::QHttpHeaderMap{ - {"Content-Length", QByteArray::number(buff.length())}, - }); - client.sendData(buff); - } else { - client.sendHeaders("GET", "test"); - } - + client.sendHeaders("GET", "test"); QTRY_VERIFY(socket.isHeadersParsed()); handler.route(&socket, socket.path()); QTRY_COMPARE(client.statusCode(), statusCode); - - if (requestPost) { - QVERIFY(client.headers().contains("Content-Length")); - QTRY_COMPARE(client.data().length(), client.headers().value("Content-Length").toInt()); - QCOMPARE(QJsonDocument::fromJson(client.data()).object(), QJsonObject::fromVariantMap(data)); - } } void TestQObjectHandler::testNewConnection() @@ -163,17 +100,17 @@ void TestQObjectHandler::testNewConnection() DummyAPI api; // Connect to object slot - handler.registerMethod("0", &api, &DummyAPI::noParameters); - handler.registerMethod("1", &api, &DummyAPI::oneParameter); - handler.registerMethod("2", &api, &DummyAPI::twoParameters); + handler.registerMethod("0", &api, &DummyAPI::valid); // Connect to functor - handler.registerMethod("3", []() { return QVariantMap(); }); - handler.registerMethod("4", &api, []() { return QVariantMap(); }); - handler.registerMethod("5", &api, [](QHttpSocket*) { return QVariantMap(); }); - handler.registerMethod("6", &api, [](QHttpSocket*, QVariantMap d) { return d; }); + handler.registerMethod("1", [](QHttpSocket *socket) { + socket->writeError(QHttpSocket::OK); + }); + handler.registerMethod("2", &api, [](QHttpSocket *socket) { + socket->writeError(QHttpSocket::OK); + }); - for (int i = 0; i < 7; ++i) { + for (int i = 0; i < 3; ++i) { QSocketPair pair; QTRY_VERIFY(pair.isConnected());