From b11206cbc8cbbf337cdf8006d03b2827efd8ee09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=BCrz?= Date: Sat, 5 Sep 2015 15:11:08 +0200 Subject: [PATCH] fix tests and http reply --- server/httpreply.cpp | 1 + server/rest/devicesresource.cpp | 4 -- server/rest/restresource.cpp | 6 +- server/rest/restserver.cpp | 2 - server/rest/rulesresource.cpp | 8 ++- server/webserver.cpp | 62 ++++++++++++------- server/webserver.h | 3 +- .../testrestdeviceclasses.cpp | 6 +- tests/auto/restdevices/testrestdevices.cpp | 2 +- tests/auto/restplugins/testrestplugins.cpp | 10 ++- 10 files changed, 58 insertions(+), 46 deletions(-) diff --git a/server/httpreply.cpp b/server/httpreply.cpp index 1d95e749..c83dcebb 100644 --- a/server/httpreply.cpp +++ b/server/httpreply.cpp @@ -170,6 +170,7 @@ HttpReply::HttpReply(const HttpReply::HttpStatusCode &statusCode, const HttpRepl void HttpReply::setHttpStatusCode(const HttpReply::HttpStatusCode &statusCode) { m_statusCode = statusCode; + m_reasonPhrase = getHttpReasonPhrase(m_statusCode); packReply(); } diff --git a/server/rest/devicesresource.cpp b/server/rest/devicesresource.cpp index 8fb7c4d9..b1c26c9c 100644 --- a/server/rest/devicesresource.cpp +++ b/server/rest/devicesresource.cpp @@ -101,7 +101,6 @@ HttpReply *DevicesResource::proccessGetRequest(const HttpRequest &request, const if (urlTokens.count() == 5 && urlTokens.at(4) == "states") return getDeviceStateValues(m_device); - // /api/v1/devices/{deviceId}/states/{stateTypeId} if (urlTokens.count() >= 6 && urlTokens.at(4) == "states") { StateTypeId stateTypeId = StateTypeId(urlTokens.at(5)); @@ -152,7 +151,6 @@ HttpReply *DevicesResource::proccessPutRequest(const HttpRequest &request, const HttpReply *DevicesResource::proccessPostRequest(const HttpRequest &request, const QStringList &urlTokens) { - // POST /api/v1/devices if (urlTokens.count() == 3) return addConfiguredDevice(request.payload()); @@ -199,7 +197,6 @@ HttpReply *DevicesResource::proccessOptionsRequest(const HttpRequest &request, c { Q_UNUSED(request) Q_UNUSED(urlTokens) - qCDebug(dcRest) << "process options request\n" << request; return RestResource::createCorsSuccessReply(); } @@ -263,7 +260,6 @@ HttpReply *DevicesResource::removeDevice(Device *device) const if (result == DeviceManager::DeviceErrorNoError) { HttpReply *reply = createSuccessReply(); - reply->setCloseConnection(true); return reply; } return createErrorReply(HttpReply::Forbidden); diff --git a/server/rest/restresource.cpp b/server/rest/restresource.cpp index 0e7c2143..18150c18 100644 --- a/server/rest/restresource.cpp +++ b/server/rest/restresource.cpp @@ -82,6 +82,7 @@ RestResource::~RestResource() HttpReply *RestResource::createSuccessReply() { HttpReply *reply = new HttpReply(HttpReply::Ok, HttpReply::TypeSync); + reply->setPayload("200 Ok"); reply->setHeader(HttpReply::ContentTypeHeader, "application/json; charset=\"utf-8\";"); return reply; } @@ -90,12 +91,12 @@ HttpReply *RestResource::createCorsSuccessReply() { HttpReply *reply = RestResource::createSuccessReply(); reply->setHeader(HttpReply::ContentTypeHeader, "text/plain"); - reply->setRawHeader("Accept","text/plain"); + reply->setRawHeader("Accept","application/json"); reply->setRawHeader("Allow", "PUT, POST, GET, DELETE, OPTIONS"); reply->setRawHeader("Access-Control-Allow-Methods", "PUT, POST, GET, DELETE, OPTIONS"); + reply->setHeader(HttpReply::ContentLenghtHeader, QByteArray::number(0)); reply->setRawHeader("Access-Control-Allow-Headers", "Origin, Content-Type, Accept"); reply->setRawHeader("Access-Control-Max-Age", "1728000"); - //reply->setCloseConnection(true); return reply; } @@ -111,6 +112,7 @@ HttpReply *RestResource::createErrorReply(const HttpReply::HttpStatusCode &statu HttpReply *RestResource::createAsyncReply() { HttpReply *reply = new HttpReply(HttpReply::Ok, HttpReply::TypeAsync); + reply->setPayload(QByteArray::number(reply->httpStatusCode()) + " " + reply->httpReasonPhrase()); return reply; } diff --git a/server/rest/restserver.cpp b/server/rest/restserver.cpp index 490fce59..b4be601e 100644 --- a/server/rest/restserver.cpp +++ b/server/rest/restserver.cpp @@ -113,10 +113,8 @@ void RestServer::processHttpRequest(const QUuid &clientId, const HttpRequest &re // check CORS call for main resource if (request.method() == HttpRequest::Options && urlTokens.count() == 3) { - qCDebug(dcRest) << "process options request\n" << request; HttpReply *reply = RestResource::createCorsSuccessReply(); reply->setClientId(clientId); - qCDebug(dcRest) << reply->data(); m_webserver->sendHttpReply(reply); reply->deleteLater(); return; diff --git a/server/rest/rulesresource.cpp b/server/rest/rulesresource.cpp index aecca238..c88e2574 100644 --- a/server/rest/rulesresource.cpp +++ b/server/rest/rulesresource.cpp @@ -182,9 +182,11 @@ HttpReply *RulesResource::removeRule(const RuleId &ruleId) const RuleEngine::RuleError status = GuhCore::instance()->removeRule(ruleId); - if (status == RuleEngine::RuleErrorNoError) - return createSuccessReply(); - + if (status == RuleEngine::RuleErrorNoError) { + HttpReply *reply = createSuccessReply(); + reply->setHeader(HttpReply::ContentLenghtHeader, "0"); + return reply; + } return createErrorReply(HttpReply::InternalServerError); } diff --git a/server/webserver.cpp b/server/webserver.cpp index a8ab288d..70299f41 100644 --- a/server/webserver.cpp +++ b/server/webserver.cpp @@ -1,3 +1,4 @@ + /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * Copyright (C) 2015 Simon Stuerz * @@ -141,11 +142,12 @@ void WebServer::sendHttpReply(HttpReply *reply) // send raw data reply->packReply(); + qCDebug(dcWebServer) << "respond" << reply->httpStatusCode() << reply->httpReasonPhrase(); socket->write(reply->data()); - // close the connection if wanted - if (reply->closeConnection()) - socket->close(); + // // close the connection if wanted + // if (reply->closeConnection()) + // socket->close(); } @@ -255,7 +257,6 @@ void WebServer::incomingConnection(qintptr socketDescriptor) emit clientConnected(clientId); } - void WebServer::readClient() { if (!m_enabled) @@ -268,7 +269,6 @@ void WebServer::readClient() if (clientId.isNull()) { qCWarning(dcWebServer) << "Client not recognized"; socket->close(); - socket->deleteLater(); return; } @@ -350,6 +350,9 @@ void WebServer::readClient() if (file.fileName().endsWith(".html")) { reply->setHeader(HttpReply::ContentTypeHeader, "text/html; charset=\"utf-8\";"); } + if (file.fileName().endsWith(".css")) { + reply->setHeader(HttpReply::ContentTypeHeader, "text/css; charset=\"utf-8\";"); + } reply->setPayload(file.readAll()); reply->setClientId(clientId); sendHttpReply(reply); @@ -369,15 +372,30 @@ void WebServer::readClient() void WebServer::onDisconnected() { QSslSocket* socket = static_cast(sender()); - qCDebug(dcConnection) << "Webserver client disonnected."; + + // remove connection from server client + foreach (WebServerClient *client, m_webServerClients) { + if (client->address() == socket->peerAddress()) { + client->removeConnection(socket); + qCDebug(dcWebServer) << "connection count" << client->connections().count(); + if (client->connections().isEmpty()) { + m_webServerClients.removeAll(client); + qCDebug(dcWebServer) << "delete client" << client->address().toString(); + client->deleteLater(); + } + break; + } + } + + qCDebug(dcConnection) << QString("Webserver client disonnected %1:%2").arg(socket->peerAddress().toString()).arg(socket->peerPort()); // clean up QUuid clientId = m_clientList.key(socket); m_clientList.remove(clientId); m_incompleteRequests.remove(socket); - socket->deleteLater(); - emit clientDisconnected(clientId); + + socket->deleteLater(); } void WebServer::onEncrypted() @@ -405,6 +423,7 @@ bool WebServer::startServer() m_enabled = false; return false; } + if (m_useSsl) { qCDebug(dcConnection) << "Started webserver on" << QString("https://%1:%2").arg(serverAddress().toString()).arg(m_port); } else { @@ -444,17 +463,24 @@ void WebServerClient::addConnection(QSslSocket *socket) { QTimer *timer = new QTimer(this); timer->setSingleShot(true); - timer->setInterval(6000); + timer->setInterval(9500); connect(timer, &QTimer::timeout, this, &WebServerClient::onTimout); m_runningConnections.insert(timer, socket); m_connections.append(socket); - connect(socket, SIGNAL(disconnected()), this, SLOT(onDisconnected())); - timer->start(); } +void WebServerClient::removeConnection(QSslSocket *socket) +{ + QTimer *timer = m_runningConnections.key(socket); + m_runningConnections.remove(timer); + m_connections.removeAll(socket); + + timer->deleteLater(); +} + void WebServerClient::resetTimout(QSslSocket *socket) { QTimer *timer = 0; @@ -466,22 +492,10 @@ void WebServerClient::resetTimout(QSslSocket *socket) void WebServerClient::onTimout() { QTimer *timer = static_cast(sender()); - QSslSocket *socket = m_runningConnections.take(timer); + QSslSocket *socket = m_runningConnections.value(timer); qCDebug(dcWebServer) << QString("Client connection timout %1:%2 -> closing connection").arg(socket->peerAddress().toString()).arg(socket->peerPort()); socket->close(); - - timer->deleteLater(); } -void WebServerClient::onDisconnected() -{ - QSslSocket *socket = static_cast(sender()); - if (!m_runningConnections.values().contains(socket)) - return; - - QTimer *timer = m_runningConnections.key(socket); - m_runningConnections.remove(timer); - timer->deleteLater(); -} } diff --git a/server/webserver.h b/server/webserver.h index 560549ad..c84f3cad 100644 --- a/server/webserver.h +++ b/server/webserver.h @@ -50,6 +50,7 @@ public: QList connections(); void addConnection(QSslSocket *socket); + void removeConnection(QSslSocket *socket); void resetTimout(QSslSocket *socket); @@ -60,7 +61,7 @@ private: private slots: void onTimout(); - void onDisconnected(); + }; diff --git a/tests/auto/restdeviceclasses/testrestdeviceclasses.cpp b/tests/auto/restdeviceclasses/testrestdeviceclasses.cpp index 5eede816..b2eee2f3 100644 --- a/tests/auto/restdeviceclasses/testrestdeviceclasses.cpp +++ b/tests/auto/restdeviceclasses/testrestdeviceclasses.cpp @@ -297,19 +297,19 @@ void TestRestDeviceClasses::discoverDevices() qDebug() << data; statusCode = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); - reply->deleteLater(); QCOMPARE(statusCode, expectedStatusCode); - jsonDoc = QJsonDocument::fromJson(data, &error); QCOMPARE(error.error, QJsonParseError::NoError); + reply->deleteLater(); + QVariantMap response = jsonDoc.toVariant().toMap(); DeviceId deviceId = DeviceId(response.value("id").toString()); QVERIFY2(!deviceId.isNull(), "got invalid device id"); - // REMOVE added device request = QNetworkRequest(QUrl(QString("http://localhost:3333/api/v1/devices/%1").arg(deviceId.toString()))); + qDebug() << request.url().toString(); clientSpy.clear(); reply = nam->deleteResource(request); clientSpy.wait(); diff --git a/tests/auto/restdevices/testrestdevices.cpp b/tests/auto/restdevices/testrestdevices.cpp index 633b9d28..80cac772 100644 --- a/tests/auto/restdevices/testrestdevices.cpp +++ b/tests/auto/restdevices/testrestdevices.cpp @@ -419,7 +419,7 @@ void TestRestDevices::editDevices() QNetworkRequest request(QUrl(QString("http://localhost:3333/api/v1/devices"))); - QNetworkReply *reply = nam->post(request, QJsonDocument::fromVariant(params).toJson()); + QNetworkReply *reply = nam->post(request, QJsonDocument::fromVariant(params).toJson(QJsonDocument::Compact)); clientSpy.wait(); QCOMPARE(clientSpy.count(), 1); diff --git a/tests/auto/restplugins/testrestplugins.cpp b/tests/auto/restplugins/testrestplugins.cpp index d8bcbb33..d4bdedd9 100644 --- a/tests/auto/restplugins/testrestplugins.cpp +++ b/tests/auto/restplugins/testrestplugins.cpp @@ -160,7 +160,7 @@ void TestRestDeviceClasses::setPluginConfiguration() request.setUrl(QUrl(QString("http://localhost:3333/api/v1/plugins/%1/configuration").arg(pluginId.toString()))); QNetworkReply *reply = nam->get(request); clientSpy.wait(); - QVERIFY2(clientSpy.count() == 1, "expected exactly 1 response from webserver"); + QCOMPARE(clientSpy.count(), 1); int statusCode = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); reply->deleteLater(); @@ -184,7 +184,7 @@ void TestRestDeviceClasses::setPluginConfiguration() request.setUrl(QUrl(QString("http://localhost:3333/api/v1/plugins/%1/configuration").arg(pluginId.toString()))); reply = nam->put(request, QJsonDocument::fromVariant(newConfigurations).toJson(QJsonDocument::Compact)); clientSpy.wait(); - QVERIFY2(clientSpy.count() == 1, "expected exactly 1 response from webserver"); + QCOMPARE(clientSpy.count(), 1); statusCode = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); QCOMPARE(statusCode, expectedStatusCode); reply->deleteLater(); @@ -197,7 +197,7 @@ void TestRestDeviceClasses::setPluginConfiguration() request.setUrl(QUrl(QString("http://localhost:3333/api/v1/plugins/%1/configuration").arg(pluginId.toString()))); reply = nam->get(request); clientSpy.wait(); - QVERIFY2(clientSpy.count() == 1, "expected exactly 1 response from webserver"); + QCOMPARE(clientSpy.count(), 1); statusCode = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); QCOMPARE(statusCode, expectedStatusCode); data = reply->readAll(); @@ -211,14 +211,12 @@ void TestRestDeviceClasses::setPluginConfiguration() // verify new configurations verifyParams(newConfigurations, checkConfigurations); - restartServer(); - // check new configurations after restart clientSpy.clear(); request.setUrl(QUrl(QString("http://localhost:3333/api/v1/plugins/%1/configuration").arg(pluginId.toString()))); reply = nam->get(request); clientSpy.wait(); - QVERIFY2(clientSpy.count() == 1, "expected exactly 1 response from webserver"); + QCOMPARE(clientSpy.count(), 1); statusCode = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); QCOMPARE(statusCode, 200); data = reply->readAll();