From 55729b78a90e2ab9dcb683450ce427b4998d39ce Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 22 Dec 2015 10:44:52 +0100 Subject: [PATCH 1/4] fixed test --- arangod/RestHandler/RestQueryHandler.cpp | 106 ++++++++++++----------- 1 file changed, 54 insertions(+), 52 deletions(-) diff --git a/arangod/RestHandler/RestQueryHandler.cpp b/arangod/RestHandler/RestQueryHandler.cpp index 9a1171e12a..8f9f41fca9 100644 --- a/arangod/RestHandler/RestQueryHandler.cpp +++ b/arangod/RestHandler/RestQueryHandler.cpp @@ -37,13 +37,13 @@ #include "Basics/string-buffer.h" #include "Basics/json-utilities.h" #include "Basics/VelocyPackHelper.h" +#include "Cluster/ClusterComm.h" +#include "Cluster/ClusterInfo.h" +#include "Cluster/ClusterMethods.h" +#include "Cluster/ServerState.h" #include "Rest/HttpRequest.h" #include "VocBase/document-collection.h" #include "VocBase/vocbase.h" -#include "Cluster/ServerState.h" -#include "Cluster/ClusterInfo.h" -#include "Cluster/ClusterComm.h" -#include "Cluster/ClusterMethods.h" using namespace std; using namespace triagens::basics; @@ -81,25 +81,37 @@ bool RestQueryHandler::isDirect () const { //////////////////////////////////////////////////////////////////////////////// HttpHandler::status_t RestQueryHandler::execute () { - // extract the sub-request type HttpRequest::HttpRequestType type = _request->requestType(); // execute one of the CRUD methods - switch (type) { - case HttpRequest::HTTP_REQUEST_DELETE: deleteQuery(); break; - case HttpRequest::HTTP_REQUEST_GET: readQuery(); break; - case HttpRequest::HTTP_REQUEST_PUT: replaceProperties(); break; - case HttpRequest::HTTP_REQUEST_POST: parseQuery(); break; + try { + switch (type) { + case HttpRequest::HTTP_REQUEST_DELETE: deleteQuery(); break; + case HttpRequest::HTTP_REQUEST_GET: readQuery(); break; + case HttpRequest::HTTP_REQUEST_PUT: replaceProperties(); break; + case HttpRequest::HTTP_REQUEST_POST: parseQuery(); break; - case HttpRequest::HTTP_REQUEST_HEAD: - case HttpRequest::HTTP_REQUEST_PATCH: - case HttpRequest::HTTP_REQUEST_ILLEGAL: - default: { - generateNotImplemented("ILLEGAL " + DOCUMENT_PATH); - break; + case HttpRequest::HTTP_REQUEST_HEAD: + case HttpRequest::HTTP_REQUEST_PATCH: + case HttpRequest::HTTP_REQUEST_ILLEGAL: + default: { + generateNotImplemented("ILLEGAL " + DOCUMENT_PATH); + break; + } } } + catch (Exception const& err) { + handleError(err); + } + catch (std::exception const& ex) { + triagens::basics::Exception err(TRI_ERROR_INTERNAL, ex.what(), __FILE__, __LINE__); + handleError(err); + } + catch (...) { + triagens::basics::Exception err(TRI_ERROR_INTERNAL, __FILE__, __LINE__); + handleError(err); + } // this handler is done return status_t(HANDLER_DONE); @@ -167,6 +179,7 @@ bool RestQueryHandler::readQueryProperties () { result.add("maxQueryStringLength", VPackValue(queryList->maxQueryStringLength())); result.close(); VPackSlice slice = result.slice(); + generateResult(slice); } catch (Exception const& err) { @@ -253,13 +266,14 @@ bool RestQueryHandler::readQueryProperties () { bool RestQueryHandler::readQuery (bool slow) { try { auto queryList = static_cast(_vocbase->_queries); - auto const&& queries = slow ? queryList->listSlow() : queryList->listCurrent(); + auto queries = slow ? queryList->listSlow() : queryList->listCurrent(); + VPackBuilder result; result.add(VPackValue(VPackValueType::Array)); - for (auto it : queries) { - const auto&& timeString = TRI_StringTimeStamp(it.started); - const auto& queryString = it.queryString; + for (auto const& it : queries) { + auto const& timeString = TRI_StringTimeStamp(it.started); + auto const& queryString = it.queryString; result.add(VPackValue(VPackValueType::Object)); result.add("id", VPackValue(StringUtils::itoa(it.id))); @@ -302,7 +316,7 @@ bool RestQueryHandler::readQuery () { return true; } - const auto& name = suffix[0]; + auto const& name = suffix[0]; if (name == "slow") { return readQuery(true); @@ -344,19 +358,14 @@ bool RestQueryHandler::deleteQuerySlow () { auto queryList = static_cast(_vocbase->_queries); queryList->clearSlow(); - try { - VPackBuilder result; - result.add(VPackValue(VPackValueType::Object)); - result.add("error", VPackValue(false)); - result.add("code", VPackValue(HttpResponse::OK)); - result.close(); - VPackSlice slice = result.slice(); - generateResult(slice); - } - catch (...) { - // Ignore the error - } - + VPackBuilder result; + result.add(VPackValue(VPackValueType::Object)); + result.add("error", VPackValue(false)); + result.add("code", VPackValue(HttpResponse::OK)); + result.close(); + VPackSlice slice = result.slice(); + generateResult(slice); + return true; } @@ -398,18 +407,13 @@ bool RestQueryHandler::deleteQuery (const string& name) { auto res = queryList->kill(id); if (res == TRI_ERROR_NO_ERROR) { - try { - VPackBuilder result; - result.add(VPackValue(VPackValueType::Object)); - result.add("error", VPackValue(false)); - result.add("code", VPackValue(HttpResponse::OK)); - result.close(); - VPackSlice slice = result.slice(); - generateResult(slice); - } - catch (...) { - // Ignore the error - } + VPackBuilder result; + result.add(VPackValue(VPackValueType::Object)); + result.add("error", VPackValue(false)); + result.add("code", VPackValue(HttpResponse::OK)); + result.close(); + VPackSlice slice = result.slice(); + generateResult(slice); } else { generateError(HttpResponse::BAD, res, "cannot kill query '" + name + "'"); @@ -432,14 +436,12 @@ bool RestQueryHandler::deleteQuery () { return true; } - const auto& name = suffix[0]; + auto const& name = suffix[0]; if (name == "slow") { return deleteQuerySlow(); } - else { - return deleteQuery(name); - } + return deleteQuery(name); } //////////////////////////////////////////////////////////////////////////////// @@ -544,8 +546,8 @@ bool RestQueryHandler::replaceProperties () { } attribute = body.get("slowQueryThreshold"); - if (attribute.isDouble()) { - slowQueryThreshold = attribute.getDouble(); + if (attribute.isNumber()) { + slowQueryThreshold = attribute.getNumber(); } attribute = body.get("maxQueryStringLength"); From a3898bd54e94a4f98bd244176cb6b8a8d82ba5b0 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 22 Dec 2015 11:27:05 +0100 Subject: [PATCH 2/4] fixed test --- arangod/Cluster/ClusterInfo.cpp | 26 ++++++++++++-------------- arangod/V8Server/v8-collection.cpp | 28 ++-------------------------- 2 files changed, 14 insertions(+), 40 deletions(-) diff --git a/arangod/Cluster/ClusterInfo.cpp b/arangod/Cluster/ClusterInfo.cpp index e81b2fe4f9..c6ab0afa31 100644 --- a/arangod/Cluster/ClusterInfo.cpp +++ b/arangod/Cluster/ClusterInfo.cpp @@ -1450,8 +1450,8 @@ int ClusterInfo::dropCollectionCoordinator (string const& databaseName, /// @brief set collection properties in coordinator //////////////////////////////////////////////////////////////////////////////// -int ClusterInfo::setCollectionPropertiesCoordinator (string const& databaseName, - string const& collectionID, +int ClusterInfo::setCollectionPropertiesCoordinator (std::string const& databaseName, + std::string const& collectionID, VocbaseCollectionInfo const* info) { AgencyComm ac; AgencyCommResult res; @@ -1485,25 +1485,23 @@ int ClusterInfo::setCollectionPropertiesCoordinator (string const& databaseName, return TRI_ERROR_OUT_OF_MEMORY; } - TRI_json_t* copy = TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, json); + std::unique_ptr copy(TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, json)); if (copy == nullptr) { return TRI_ERROR_OUT_OF_MEMORY; } - TRI_DeleteObjectJson(TRI_UNKNOWN_MEM_ZONE, copy, "doCompact"); - TRI_DeleteObjectJson(TRI_UNKNOWN_MEM_ZONE, copy, "journalSize"); - TRI_DeleteObjectJson(TRI_UNKNOWN_MEM_ZONE, copy, "waitForSync"); - TRI_DeleteObjectJson(TRI_UNKNOWN_MEM_ZONE, copy, "indexBuckets"); + TRI_DeleteObjectJson(TRI_UNKNOWN_MEM_ZONE, copy.get(), "doCompact"); + TRI_DeleteObjectJson(TRI_UNKNOWN_MEM_ZONE, copy.get(), "journalSize"); + TRI_DeleteObjectJson(TRI_UNKNOWN_MEM_ZONE, copy.get(), "waitForSync"); + TRI_DeleteObjectJson(TRI_UNKNOWN_MEM_ZONE, copy.get(), "indexBuckets"); - TRI_Insert3ObjectJson(TRI_UNKNOWN_MEM_ZONE, copy, "doCompact", TRI_CreateBooleanJson(TRI_UNKNOWN_MEM_ZONE, info->doCompact())); - TRI_Insert3ObjectJson(TRI_UNKNOWN_MEM_ZONE, copy, "journalSize", TRI_CreateNumberJson(TRI_UNKNOWN_MEM_ZONE, info->maximalSize())); - TRI_Insert3ObjectJson(TRI_UNKNOWN_MEM_ZONE, copy, "waitForSync", TRI_CreateBooleanJson(TRI_UNKNOWN_MEM_ZONE, info->waitForSync())); - TRI_Insert3ObjectJson(TRI_UNKNOWN_MEM_ZONE, copy, "indexBuckets", TRI_CreateNumberJson(TRI_UNKNOWN_MEM_ZONE, info->indexBuckets())); + TRI_Insert3ObjectJson(TRI_UNKNOWN_MEM_ZONE, copy.get(), "doCompact", TRI_CreateBooleanJson(TRI_UNKNOWN_MEM_ZONE, info->doCompact())); + TRI_Insert3ObjectJson(TRI_UNKNOWN_MEM_ZONE, copy.get(), "journalSize", TRI_CreateNumberJson(TRI_UNKNOWN_MEM_ZONE, info->maximalSize())); + TRI_Insert3ObjectJson(TRI_UNKNOWN_MEM_ZONE, copy.get(), "waitForSync", TRI_CreateBooleanJson(TRI_UNKNOWN_MEM_ZONE, info->waitForSync())); + TRI_Insert3ObjectJson(TRI_UNKNOWN_MEM_ZONE, copy.get(), "indexBuckets", TRI_CreateNumberJson(TRI_UNKNOWN_MEM_ZONE, info->indexBuckets())); res.clear(); - res = ac.setValue("Plan/Collections/" + databaseName + "/" + collectionID, copy, 0.0); - - TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, copy); + res = ac.setValue("Plan/Collections/" + databaseName + "/" + collectionID, copy.get(), 0.0); } if (res.successful()) { diff --git a/arangod/V8Server/v8-collection.cpp b/arangod/V8Server/v8-collection.cpp index 40c1adf644..f4cbf00ae1 100644 --- a/arangod/V8Server/v8-collection.cpp +++ b/arangod/V8Server/v8-collection.cpp @@ -58,7 +58,6 @@ #include #include "unicode/timezone.h" -#include using namespace std; using namespace triagens::basics; @@ -2436,12 +2435,13 @@ static void JS_PropertiesVocbaseCol (const v8::FunctionCallbackInfo& if (par->IsObject()) { VPackBuilder builder; { - VPackObjectBuilder b(&builder); int res = TRI_V8ToVPack(isolate, builder, args[0], false); + if (res != TRI_ERROR_NO_ERROR) { TRI_V8_THROW_EXCEPTION(res); } } + VPackSlice const slice = builder.slice(); if (slice.hasKey("journalSize")) { VPackSlice maxSizeSlice = slice.get("journalSize"); @@ -2461,29 +2461,6 @@ static void JS_PropertiesVocbaseCol (const v8::FunctionCallbackInfo& TRI_V8_THROW_EXCEPTION_PARAMETER("indexBucket must be a two-power between 1 and 1024"); } info.update(slice, false, collection->_vocbase); - - // TODO FIXME temporary ASSERTIONS - v8::Handle po = par->ToObject(); - - // extract doCompact flag - TRI_GET_GLOBAL_STRING(DoCompactKey); - if (po->Has(DoCompactKey)) { - TRI_ASSERT(info.doCompact() == TRI_ObjectToBoolean(po->Get(DoCompactKey))); - } - - // extract sync flag - TRI_GET_GLOBAL_STRING(WaitForSyncKey); - if (po->Has(WaitForSyncKey)) { - TRI_ASSERT(info.waitForSync() == TRI_ObjectToBoolean(po->Get(WaitForSyncKey))); - } - - // extract the journal size - TRI_GET_GLOBAL_STRING(JournalSizeKey); - if (po->Has(JournalSizeKey)) { - auto maximalSize = (TRI_voc_size_t) TRI_ObjectToUInt64(po->Get(JournalSizeKey), false); - TRI_ASSERT(info.maximalSize() == maximalSize); - } - } int res = ClusterInfo::instance()->setCollectionPropertiesCoordinator(databaseName, StringUtils::itoa(collection->_cid), &info); @@ -2493,7 +2470,6 @@ static void JS_PropertiesVocbaseCol (const v8::FunctionCallbackInfo& } } - // return the current parameter set v8::Handle result = v8::Object::New(isolate); From e2e483e06a629fc6d6dc433dcf75b6a04e79f1ee Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 22 Dec 2015 12:34:48 +0100 Subject: [PATCH 3/4] potential fix --- arangod/RestHandler/RestImportHandler.cpp | 3 +-- arangod/V8Server/v8-vocindex.cpp | 2 +- arangod/VocBase/collection.cpp | 13 ++++++++++++- arangod/VocBase/collection.h | 4 ++++ lib/Basics/VelocyPackHelper.cpp | 1 + 5 files changed, 19 insertions(+), 4 deletions(-) diff --git a/arangod/RestHandler/RestImportHandler.cpp b/arangod/RestHandler/RestImportHandler.cpp index 26ee141e92..5e77193e80 100644 --- a/arangod/RestHandler/RestImportHandler.cpp +++ b/arangod/RestHandler/RestImportHandler.cpp @@ -911,10 +911,9 @@ bool RestImportHandler::createFromJson (string const& type) { else { // the entire request body is one JSON document // TODO Workaround for cast char* to uint8_t* - std::string body(_request->body(), _request->bodySize()); std::shared_ptr parsedDocuments; try { - parsedDocuments = VPackParser::fromJson(body); + parsedDocuments = VPackParser::fromJson(reinterpret_cast(_request->body()), _request->bodySize()); } catch (VPackException const& e) { generateError(HttpResponse::BAD, diff --git a/arangod/V8Server/v8-vocindex.cpp b/arangod/V8Server/v8-vocindex.cpp index 83065ff66b..5d2349d333 100644 --- a/arangod/V8Server/v8-vocindex.cpp +++ b/arangod/V8Server/v8-vocindex.cpp @@ -1592,6 +1592,7 @@ static void CreateVocBase (const v8::FunctionCallbackInfo& args, if (res != TRI_ERROR_NO_ERROR) { TRI_V8_THROW_EXCEPTION(res); } + infoSlice = builder.slice(); } @@ -1805,7 +1806,6 @@ void TRI_InitV8indexArangoDB (v8::Isolate* isolate, TRI_AddMethodVocbase(isolate, rt, TRI_V8_ASCII_STRING("_createDocumentCollection"), JS_CreateDocumentCollectionVocbase); } - void TRI_InitV8indexCollection (v8::Isolate* isolate, v8::Handle rt) { diff --git a/arangod/VocBase/collection.cpp b/arangod/VocBase/collection.cpp index 694887a27f..95021ee2d2 100644 --- a/arangod/VocBase/collection.cpp +++ b/arangod/VocBase/collection.cpp @@ -45,6 +45,9 @@ #include "VocBase/server.h" #include "VocBase/vocbase.h" +#include +#include + using namespace std; using namespace triagens::arango; @@ -1257,13 +1260,21 @@ VocbaseCollectionInfo VocbaseCollectionInfo::fromFile (char const* path, std::string filePath(filename, strlen(filename)); std::shared_ptr content = triagens::basics::VelocyPackHelper::velocyPackFromFile(filePath); - VPackSlice const slice = content->slice(); + VPackSlice slice = content->slice(); if (! slice.isObject()) { LOG_ERROR("cannot open '%s', collection parameters are not readable", filename); TRI_FreeString(TRI_CORE_MEM_ZONE, filename); THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_ILLEGAL_PARAMETER_FILE); } TRI_FreeString(TRI_CORE_MEM_ZONE, filename); + + VPackBuilder bx; + bx.openObject(); + bx.add("isSystem", VPackValue(false)); + bx.close(); + VPackSlice isSystem = bx.slice(); + VPackBuilder b2 = VPackCollection::merge(slice, isSystem, false); + slice = b2.slice(); VocbaseCollectionInfo info(vocbase, collectionName, slice); // warn about wrong version of the collection diff --git a/arangod/VocBase/collection.h b/arangod/VocBase/collection.h index f0c09c6896..7fd6e538ed 100644 --- a/arangod/VocBase/collection.h +++ b/arangod/VocBase/collection.h @@ -301,6 +301,10 @@ namespace triagens { // Changes the name. Should only be called by TRI_RenameCollection // Use with caution! void rename (char const*); + + void setIsSystem (bool value) { + _isSystem = value; + } void setRevision (TRI_voc_rid_t, bool); diff --git a/lib/Basics/VelocyPackHelper.cpp b/lib/Basics/VelocyPackHelper.cpp index 0f1d043c86..7e0d8192d1 100644 --- a/lib/Basics/VelocyPackHelper.cpp +++ b/lib/Basics/VelocyPackHelper.cpp @@ -98,6 +98,7 @@ TRI_json_t* VelocyPackHelper::velocyPackToJson (VPackSlice const& slice) { std::shared_ptr VelocyPackHelper::velocyPackFromFile (std::string const& path) { size_t length; + // TODO: Fix memleak char* content = TRI_SlurpFile(TRI_UNKNOWN_MEM_ZONE, path.c_str(), &length); // The Parser might THROW return VPackParser::fromJson(reinterpret_cast(content), length); From 80c7645146cb4305d6ded42f289dbb02cab98a62 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 22 Dec 2015 13:10:43 +0100 Subject: [PATCH 4/4] one more try --- arangod/VocBase/collection.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/arangod/VocBase/collection.cpp b/arangod/VocBase/collection.cpp index 95021ee2d2..4067e00549 100644 --- a/arangod/VocBase/collection.cpp +++ b/arangod/VocBase/collection.cpp @@ -1267,14 +1267,24 @@ VocbaseCollectionInfo VocbaseCollectionInfo::fromFile (char const* path, THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_ILLEGAL_PARAMETER_FILE); } TRI_FreeString(TRI_CORE_MEM_ZONE, filename); + + // fiddle "isSystem" value, which is not contained in the JSON file + bool isSystemValue = false; + if (slice.hasKey("name")) { + auto name = slice.get("name").copyString(); + if (! name.empty()) { + isSystemValue = name[0] == '_'; + } + } VPackBuilder bx; bx.openObject(); - bx.add("isSystem", VPackValue(false)); + bx.add("isSystem", VPackValue(isSystemValue)); bx.close(); VPackSlice isSystem = bx.slice(); VPackBuilder b2 = VPackCollection::merge(slice, isSystem, false); slice = b2.slice(); + VocbaseCollectionInfo info(vocbase, collectionName, slice); // warn about wrong version of the collection