From 18d4153168e5787b1c1a49bb68207f7f5d6042af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Nowacki?= Date: Fri, 1 Aug 2014 12:42:53 +0000 Subject: Allow to directly query objects through object's id. The change inserts, if given, an object id to request path. It allows to construct query objects with only type name and id. Such query is equivalent to a "view" request, that means that instead of returning a array of results it gives full object only as a result. From user perspective this change simplify certain group of queries, for example this query: QJsonObject query = {{"objectType", "objects.foo"}, {"query", {{"id", "1233"}}}}; could be changed to: QJsonObject query = {{"objectType", "objects.foo"}, {"id", "1233"}}; The change cleanups a bit path computation, by not adding an object id as a fall-back operation. Because of that hint IncludeIdPath could be replaced by RequireIdPath, which is easier to interpret. It shows that an operation has no chances to success if an id is missing. Change-Id: If2bf09da0d7c4388493476a16b138640022c8581 Reviewed-by: Frederik Gladhorn --- src/enginio_client/enginioclient.cpp | 15 +++++ src/enginio_client/enginioclient_p.h | 71 +++++++++------------ tests/auto/enginioclient/tst_enginioclient.cpp | 86 +++++++++++++++++--------- 3 files changed, 101 insertions(+), 71 deletions(-) diff --git a/src/enginio_client/enginioclient.cpp b/src/enginio_client/enginioclient.cpp index 6a47696..3d1b888 100644 --- a/src/enginio_client/enginioclient.cpp +++ b/src/enginio_client/enginioclient.cpp @@ -262,6 +262,21 @@ QNetworkRequest EnginioClientConnectionPrivate::prepareRequest(const QUrl &url) return req; } +bool EnginioClientConnectionPrivate::appendIdToPathIfPossible(QString *path, const QString &id, QByteArray *errorMsg, EnginioClientConnectionPrivate::PathOptions flags, QByteArray errorMessageHint) +{ + Q_ASSERT(path && errorMsg); + if (id.isEmpty()) { + if (flags == RequireIdInPath) { + *errorMsg = constructErrorMessage(errorMessageHint); + return false; + } + return true; + } + path->append('/'); + path->append(id); + return true; +} + EnginioClientConnectionPrivate::EnginioClientConnectionPrivate() : _identity(), _serviceUrl(EnginioString::apiEnginIo), diff --git a/src/enginio_client/enginioclient_p.h b/src/enginio_client/enginioclient_p.h index 93ce864..286a30c 100644 --- a/src/enginio_client/enginioclient_p.h +++ b/src/enginio_client/enginioclient_p.h @@ -87,11 +87,11 @@ QT_BEGIN_NAMESPACE CHECK_AND_SET_URL_PATH_IMPL(Url, Object, Operation, EnginioClientConnectionPrivate::Default) #define CHECK_AND_SET_PATH_WITH_ID(Url, Object, Operation) \ - CHECK_AND_SET_URL_PATH_IMPL(Url, Object, Operation, EnginioClientConnectionPrivate::IncludeIdInPath) + CHECK_AND_SET_URL_PATH_IMPL(Url, Object, Operation, EnginioClientConnectionPrivate::RequireIdInPath) class ENGINIOCLIENT_EXPORT EnginioClientConnectionPrivate : public QObjectPrivate { - enum PathOptions { Default, IncludeIdInPath = 1}; + enum PathOptions { Default, RequireIdInPath = 1}; struct ENGINIOCLIENT_EXPORT GetPathReturnValue : public QPair { @@ -105,6 +105,8 @@ class ENGINIOCLIENT_EXPORT EnginioClientConnectionPrivate : public QObjectPrivat operator QString() const { return second; } }; + static bool appendIdToPathIfPossible(QString *path, const QString &id, QByteArray *errorMsg, PathOptions flags, QByteArray errorMessageHint = EnginioString::Requested_operation_requires_non_empty_id_value); + template static GetPathReturnValue getPath(const T &object, int operation, QString *path, QByteArray *errorMsg, PathOptions flags = Default) { @@ -114,6 +116,7 @@ class ENGINIOCLIENT_EXPORT EnginioClientConnectionPrivate : public QObjectPrivat QString &result = *path; result.reserve(96); result.append(QStringLiteral("/v1/")); + QString id = object[EnginioString::id].toString(); switch (operation) { case Enginio::ObjectOperation: { @@ -122,8 +125,9 @@ class ENGINIOCLIENT_EXPORT EnginioClientConnectionPrivate : public QObjectPrivat msg = constructErrorMessage(EnginioString::Requested_object_operation_requires_non_empty_objectType_value); return GetPathReturnValue(Failed); } - result.append(objectType.replace('.', '/')); + if (!appendIdToPathIfPossible(path, id, errorMsg, flags)) + return GetPathReturnValue(Failed); break; } case Enginio::AccessControlOperation: @@ -133,83 +137,65 @@ class ENGINIOCLIENT_EXPORT EnginioClientConnectionPrivate : public QObjectPrivat msg = constructErrorMessage(EnginioString::Requested_object_acl_operation_requires_non_empty_objectType_value); return GetPathReturnValue(Failed); } - result.append(objectType.replace('.', '/')); - QString id = object[EnginioString::id].toString(); - if (id.isEmpty()) { - msg = constructErrorMessage(EnginioString::Requested_object_acl_operation_requires_non_empty_id_value); + if (!appendIdToPathIfPossible(path, id, errorMsg, RequireIdInPath, EnginioString::Requested_object_acl_operation_requires_non_empty_id_value)) return GetPathReturnValue(Failed); - } - result.append('/'); - result.append(id); result.append('/'); result.append(EnginioString::access); return GetPathReturnValue(true, EnginioString::access); } case Enginio::FileOperation: { result.append(EnginioString::files); - // if we have a fileID, it becomes "view", otherwise it is up/download - QString fileId = object[EnginioString::id].toString(); - if (!fileId.isEmpty()) { - result.append('/'); - result.append(fileId); - } + if (!appendIdToPathIfPossible(path, id, errorMsg, flags)) + return GetPathReturnValue(Failed); break; } case Enginio::FileGetDownloadUrlOperation: { result.append(EnginioString::files); - QString fileId = object[EnginioString::id].toString(); - if (fileId.isEmpty()) { - msg = constructErrorMessage(EnginioString::Download_operation_requires_non_empty_fileId_value); + if (!appendIdToPathIfPossible(path, id, errorMsg, RequireIdInPath, EnginioString::Download_operation_requires_non_empty_fileId_value)) return GetPathReturnValue(Failed); - } - result.append(QLatin1Char('/') + fileId + QStringLiteral("/download_url")); + result.append(QStringLiteral("/download_url")); break; } case Enginio::FileChunkUploadOperation: { - const QString fileId = object[EnginioString::id].toString(); - Q_ASSERT(!fileId.isEmpty()); - result.append(EnginioString::files + QLatin1Char('/') + fileId + QStringLiteral("/chunk")); + Q_ASSERT(!id.isEmpty()); + result.append(EnginioString::files); + if (!appendIdToPathIfPossible(path, id, errorMsg, flags)) + return GetPathReturnValue(Failed); + result.append(QStringLiteral("/chunk")); break; } case Enginio::SearchOperation: result.append(EnginioString::search); + if (!appendIdToPathIfPossible(path, id, errorMsg, flags)) + return GetPathReturnValue(Failed); break; case Enginio::SessionOperation: result.append(EnginioString::session); + if (!appendIdToPathIfPossible(path, id, errorMsg, flags)) + return GetPathReturnValue(Failed); break; case Enginio::UserOperation: result.append(EnginioString::users); + if (!appendIdToPathIfPossible(path, id, errorMsg, flags)) + return GetPathReturnValue(Failed); break; case Enginio::UsergroupOperation: result.append(EnginioString::usergroups); + if (!appendIdToPathIfPossible(path, id, errorMsg, flags)) + return GetPathReturnValue(Failed); break; case Enginio::UsergroupMembersOperation: { - QString id = object[EnginioString::id].toString(); - if (id.isEmpty()) { - msg = constructErrorMessage(EnginioString::Requested_usergroup_member_operation_requires_non_empty_id_value); - return GetPathReturnValue(Failed); - } result.append(EnginioString::usergroups); - result.append('/'); - result.append(id); + if (!appendIdToPathIfPossible(path, id, errorMsg, RequireIdInPath, EnginioString::Requested_usergroup_member_operation_requires_non_empty_id_value)) + return GetPathReturnValue(Failed); result.append('/'); result.append(EnginioString::members); return GetPathReturnValue(true, EnginioString::member); } } - if (flags & IncludeIdInPath) { - QString id = object[EnginioString::id].toString(); - if (id.isEmpty()) { - msg = constructErrorMessage(EnginioString::Requested_operation_requires_non_empty_id_value); - return GetPathReturnValue(Failed); - } - result.append('/'); - result.append(id); - } - return GetPathReturnValue(true, QString()); } @@ -557,7 +543,6 @@ public: url.setQuery(urlQuery); QNetworkRequest req = prepareRequest(url); - return networkManager()->get(req); } @@ -565,7 +550,7 @@ public: QNetworkReply *downloadUrl(const ObjectAdaptor &object) { QUrl url(_serviceUrl); - CHECK_AND_SET_PATH(url, object, Enginio::FileGetDownloadUrlOperation); + CHECK_AND_SET_PATH_WITH_ID(url, object, Enginio::FileGetDownloadUrlOperation); if (object.contains(EnginioString::variant)) { QString variant = object[EnginioString::variant].toString(); QUrlQuery query; diff --git a/tests/auto/enginioclient/tst_enginioclient.cpp b/tests/auto/enginioclient/tst_enginioclient.cpp index aafebbe..d1f25cc 100644 --- a/tests/auto/enginioclient/tst_enginioclient.cpp +++ b/tests/auto/enginioclient/tst_enginioclient.cpp @@ -140,6 +140,49 @@ void tst_EnginioClient::initTestCase() prepareForSearch(); EnginioTests::prepareTestUsersAndUserGroups(_backendId); + + // create some todos objects + QJsonObject todos; + todos["name"] = QStringLiteral("todos"); + + QJsonObject title; + title["name"] = QStringLiteral("title"); + title["type"] = QStringLiteral("string"); + title["indexed"] = true; + QJsonObject completed; + completed["name"] = QStringLiteral("completed"); + completed["type"] = QStringLiteral("boolean"); + completed["indexed"] = false; + QJsonObject count; + count["name"] = QStringLiteral("count"); + count["type"] = QStringLiteral("number"); + count["indexed"] = false; + + QJsonArray properties; + properties.append(title); + properties.append(completed); + properties.append(count); + todos["properties"] = properties; + + QVERIFY(_backendManager.createObjectType(_backendName, EnginioTests::TESTAPP_ENV, todos)); + { + EnginioClient client; + QObject::connect(&client, SIGNAL(error(EnginioReply *)), this, SLOT(error(EnginioReply *))); + client.setBackendId(_backendId); + client.setServiceUrl(EnginioTests::TESTAPP_URL); + + QJsonObject object; + object["objectType"] = QString::fromUtf8("objects.todos"); + object["title"] = QString::fromUtf8("init todo 1"); + object["completed"] = false; + EnginioReply* reply1 = client.create(object); + object["title"] = QString::fromUtf8("init todo 2"); + EnginioReply* reply2 = client.create(object); + QTRY_VERIFY(reply1->isFinished()); + CHECK_NO_ERROR(reply1); + QTRY_VERIFY(reply2->isFinished()); + CHECK_NO_ERROR(reply2); + } } void tst_EnginioClient::cleanupTestCase() @@ -177,20 +220,31 @@ void tst_EnginioClient::query_todos() QSignalSpy spy(&client, SIGNAL(finished(EnginioReply *))); QSignalSpy spyError(&client, SIGNAL(error(EnginioReply*))); //![query-todo] - QJsonObject object; - object["objectType"] = QString::fromUtf8("objects.todos"); - const EnginioReply* reply = client.query(object); + QJsonObject query; + query["objectType"] = QString::fromUtf8("objects.todos"); + EnginioReply *reply = client.query(query); //![query-todo] QVERIFY(reply); QTRY_COMPARE(spy.count(), 1); QCOMPARE(spyError.count(), 0); - const EnginioReply *response = spy[0][0].value(); + EnginioReply *response = spy[0][0].value(); QCOMPARE(response, reply); CHECK_NO_ERROR(response); QVERIFY(!response->data().isEmpty()); QVERIFY(!response->data()["results"].isUndefined()); + + { // try to query the object with an id + QJsonArray results = reply->data()["results"].toArray(); + QVERIFY(results.count() > 1); // the test assumes that querying by id will return less items. + QString id = results[0].toObject()["id"].toString(); + query["id"] = id; + reply = client.query(query); + QTRY_VERIFY(reply->isFinished()); + CHECK_NO_ERROR(reply); + QCOMPARE(reply->data()["id"].toString(), id); + } } void tst_EnginioClient::query_todos_filter() @@ -882,30 +936,6 @@ void tst_EnginioClient::deleteReply() void tst_EnginioClient::create_todos() { - QJsonObject todos; - todos["name"] = QStringLiteral("todos"); - - QJsonObject title; - title["name"] = QStringLiteral("title"); - title["type"] = QStringLiteral("string"); - title["indexed"] = true; - QJsonObject completed; - completed["name"] = QStringLiteral("completed"); - completed["type"] = QStringLiteral("boolean"); - completed["indexed"] = false; - QJsonObject count; - count["name"] = QStringLiteral("count"); - count["type"] = QStringLiteral("number"); - count["indexed"] = false; - - QJsonArray properties; - properties.append(title); - properties.append(completed); - properties.append(count); - todos["properties"] = properties; - - QVERIFY(_backendManager.createObjectType(_backendName, EnginioTests::TESTAPP_ENV, todos)); - EnginioClient client; QObject::connect(&client, SIGNAL(error(EnginioReply *)), this, SLOT(error(EnginioReply *))); client.setBackendId(_backendId); -- cgit v1.2.3