From bcd8490c1b8e740b10c484dd3016f6b7de7c6a75 Mon Sep 17 00:00:00 2001 From: Esteban Lombeyda Date: Thu, 22 May 2014 12:56:17 +0200 Subject: [PATCH] Supporting new signature for remove function --- arangod/V8Server/v8-vocbase.cpp | 82 ++++++----- .../modules/org/arangodb/arango-collection.js | 19 ++- js/common/tests/shell-document.js | 127 +++++++++++++++++- 3 files changed, 189 insertions(+), 39 deletions(-) diff --git a/arangod/V8Server/v8-vocbase.cpp b/arangod/V8Server/v8-vocbase.cpp index dd8d317295..bba510bf0c 100644 --- a/arangod/V8Server/v8-vocbase.cpp +++ b/arangod/V8Server/v8-vocbase.cpp @@ -430,31 +430,6 @@ static bool ExtractForceSync (v8::Arguments const& argv, return forceSync; } -//////////////////////////////////////////////////////////////////////////////// -/// @brief extract the update policy from the arguments -/// must specify the argument index starting from 1 -//////////////////////////////////////////////////////////////////////////////// - -static TRI_doc_update_policy_e ExtractUpdatePolicy (v8::Arguments const& argv, - const int index) { - assert(index > 0); - - // default value - TRI_doc_update_policy_e policy = TRI_DOC_UPDATE_ERROR; - - if (argv.Length() >= index) { - if (TRI_ObjectToBoolean(argv[index - 1])) { - // overwrite! - policy = TRI_DOC_UPDATE_LAST_WRITE; - } - else { - policy = TRI_DOC_UPDATE_CONFLICT; - } - } - - return policy; -} - TRI_doc_update_policy_e ExtractUpdatePolicy (bool overwrite) { TRI_doc_update_policy_e policy ; @@ -2878,6 +2853,16 @@ static v8::Handle RemoveVocbaseColCoordinator (TRI_vocbase_col_t cons } #endif +//////////////////////////////////////////////////////////////////////////////// +/// @brief internal struct which is used for reading the different option +/// parameters for the remove function +//////////////////////////////////////////////////////////////////////////////// + +struct RemoveOptions { + bool overwrite = true; + bool waitForSync = false; +}; + //////////////////////////////////////////////////////////////////////////////// /// @brief deletes a document //////////////////////////////////////////////////////////////////////////////// @@ -2885,14 +2870,36 @@ static v8::Handle RemoveVocbaseColCoordinator (TRI_vocbase_col_t cons static v8::Handle RemoveVocbaseCol (bool useCollection, v8::Arguments const& argv) { v8::HandleScope scope; + ReplaceOptions options; + TRI_doc_update_policy_e policy = TRI_DOC_UPDATE_ERROR; // check the arguments if (argv.Length() < 1 || argv.Length() > 3) { - TRI_V8_EXCEPTION_USAGE(scope, "remove(, , )"); + TRI_V8_EXCEPTION_USAGE(scope, "remove(, , ) or" + "remove(, , {overwrite: booleanValue, waitForSync: booleanValue})" + ); } - const TRI_doc_update_policy_e policy = ExtractUpdatePolicy(argv, 2); - const bool forceSync = ExtractForceSync(argv, 3); + if (argv.Length() > 1) { + if (argv[1]->IsObject()) { + v8::Handle optionsObject = argv[1].As(); + if (optionsObject->Has(v8::String::New("overwrite"))) { + options.overwrite = TRI_ObjectToBoolean(optionsObject->Get(v8::String::New("overwrite"))); + policy = ExtractUpdatePolicy(options.overwrite); + } + if (optionsObject->Has(v8::String::New("waitForSync"))) { + options.waitForSync = TRI_ObjectToBoolean(optionsObject->Get(v8::String::New("waitForSync"))); + } + } else {// old variant replace(, , , ) + if (argv.Length() > 1 ) { + options.overwrite = TRI_ObjectToBoolean(argv[1]); + policy = ExtractUpdatePolicy(options.overwrite); + } + if (argv.Length() > 2 ) { + options.waitForSync = TRI_ObjectToBoolean(argv[2]); + } + } + } TRI_voc_key_t key = 0; TRI_voc_rid_t rid; @@ -2938,7 +2945,7 @@ static v8::Handle RemoveVocbaseCol (bool useCollection, #ifdef TRI_ENABLE_CLUSTER if (ServerState::instance()->isCoordinator()) { - return scope.Close(RemoveVocbaseColCoordinator(col, policy, forceSync, argv)); + return scope.Close(RemoveVocbaseColCoordinator(col, policy, options.waitForSync, argv)); } #endif @@ -2950,7 +2957,7 @@ static v8::Handle RemoveVocbaseCol (bool useCollection, TRI_V8_EXCEPTION(scope, res); } - res = trx.deleteDocument(key, policy, forceSync, rid, &actualRevision); + res = trx.deleteDocument(key, policy, options.waitForSync, rid, &actualRevision); res = trx.finish(res); TRI_FreeString(TRI_CORE_MEM_ZONE, key); @@ -8732,7 +8739,9 @@ static v8::Handle JS_CreateEdgeCollectionVocbase (v8::Arguments const /// existed and was deleted. It returns @LIT{false}, if the document was already /// deleted. /// -/// @FUN{@FA{db}._remove(@FA{document}, true, @FA{waitForSync})} +/// @FUN{@FA{db}._remove(@FA{document}, true, @FA{waitForSync})} or +/// @FUN{@FA{db}._remove(@FA{document}, +/// {@FA{overwrite}: true or false, @FA{waitForSynca}: true or false})} /// /// The optional @FA{waitForSync} parameter can be used to force synchronisation /// of the document deletion operation to disk even in case that the @@ -8785,6 +8794,17 @@ static v8::Handle JS_CreateEdgeCollectionVocbase (v8::Arguments const /// !db._document(a1); /// ! ^ /// @endcode +/// Remove a document using new signature: +/// @code +/// arangod> db.example.save({ a: 1 } ); +/// { +// "_id" : "example/11265325374", +// "_rev" : "11265325374", +// "_key" : "11265325374" +// } +// arangod> db.example.remove("example/11265325374", {overwrite: true, waitForSync: false}) +// true +/// @endcode //////////////////////////////////////////////////////////////////////////////// static v8::Handle JS_RemoveVocbase (v8::Arguments const& argv) { diff --git a/js/client/modules/org/arangodb/arango-collection.js b/js/client/modules/org/arangodb/arango-collection.js index 52da5116dc..fbb74b8267 100644 --- a/js/client/modules/org/arangodb/arango-collection.js +++ b/js/client/modules/org/arangodb/arango-collection.js @@ -1109,13 +1109,24 @@ ArangoCollection.prototype.remove = function (id, overwrite, waitForSync) { id = id._id; } - var policy = ""; + var params = ""; - if (overwrite) { - policy = "?policy=last"; + if (typeof overwrite === "object") { + // we assume the caller uses new signature (id, data, options) + var options = overwrite; + if (options.hasOwnProperty("overwrite") && options.overwrite) { + params += "?policy=last"; + } + if (options.hasOwnProperty("waitForSync") ) { + waitForSync = options.waitForSync; + } + } else { + if (overwrite) { + params += "?policy=last"; + } } - var url = this._documenturl(id) + policy; + var url = this._documenturl(id) + params; url = this._appendSyncParameter(url, waitForSync); if (rev === null) { diff --git a/js/common/tests/shell-document.js b/js/common/tests/shell-document.js index eabf6bd7e3..e97c08f86d 100644 --- a/js/common/tests/shell-document.js +++ b/js/common/tests/shell-document.js @@ -1082,6 +1082,37 @@ function CollectionDocumentSuite () { var a4 = collection.remove(a1, true); + assertEqual(a4, false); + }, +//////////////////////////////////////////////////////////////////////////////// +/// @brief delete a document using new signature +//////////////////////////////////////////////////////////////////////////////// + + testDeleteWithNewSignatureDocument : function () { + var a1 = collection.save({ a : 1}); + + assertTypeOf("string", a1._id); + assertTypeOf("string", a1._rev); + + var a2 = collection.replace(a1, { a : 2 }); + + assertEqual(a1._id, a2._id); + assertNotEqual(a1._rev, a2._rev); + + try { + collection.remove(a1); + fail(); + } + catch (err) { + assertEqual(ERRORS.ERROR_ARANGO_CONFLICT.code, err.errorNum); + } + + var a3 = collection.remove(a1, {"overwrite": true}); + + assertEqual(a3, true); + + var a4 = collection.remove(a1, {"overwrite": true}); + assertEqual(a4, false); }, @@ -1094,9 +1125,24 @@ function CollectionDocumentSuite () { assertTypeOf("string", a1._id); assertTypeOf("string", a1._rev); - +// falsch var a2 = collection.update(a1, { a : 2 }, true, false); + assertEqual(a1._id, a2._id); + assertNotEqual(a1._rev, a2._rev); + }, +//////////////////////////////////////////////////////////////////////////////// +/// @brief update a document, waitForSync=false; using new signature +//////////////////////////////////////////////////////////////////////////////// + + testUpdateWithNewSignatureDocumentSyncFalse : function () { + var a1 = collection.save({ a : 1}); + + assertTypeOf("string", a1._id); + assertTypeOf("string", a1._rev); + + var a2 = collection.update(a1, { a : 2 }, {"overwrite": true, "waitForSync" : false}); + assertEqual(a1._id, a2._id); assertNotEqual(a1._rev, a2._rev); }, @@ -1110,9 +1156,24 @@ function CollectionDocumentSuite () { assertTypeOf("string", a1._id); assertTypeOf("string", a1._rev); - +// falsch, TODO var a2 = collection.update(a1, { a : 2 }, true, true); + assertEqual(a1._id, a2._id); + assertNotEqual(a1._rev, a2._rev); + }, +//////////////////////////////////////////////////////////////////////////////// +/// @brief update a document, waitForSync=true,new signature +//////////////////////////////////////////////////////////////////////////////// + + testUpdateWithNewSignatureDocumentSyncTrue : function () { + var a1 = collection.save({ a : 1}); + + assertTypeOf("string", a1._id); + assertTypeOf("string", a1._rev); + + var a2 = collection.update(a1, { a : 2 }, {"overwrite": true, "waitForSync" : true}); + assertEqual(a1._id, a2._id); assertNotEqual(a1._rev, a2._rev); }, @@ -1130,6 +1191,19 @@ function CollectionDocumentSuite () { var a2 = collection.remove(a1, true, false); assertEqual(a2, true); }, +//////////////////////////////////////////////////////////////////////////////// +/// @brief delete a document, waitForSync=false , using new signature +//////////////////////////////////////////////////////////////////////////////// + + testDeleteWithNewSignatureDocumentSyncFalse : function () { + var a1 = collection.save({ a : 1}); + + assertTypeOf("string", a1._id); + assertTypeOf("string", a1._rev); + + var a2 = collection.remove(a1, {"overwrite": true, "waitForSync": false}); + assertEqual(a2, true); + }, //////////////////////////////////////////////////////////////////////////////// /// @brief delete a document, waitForSync=true @@ -1144,6 +1218,19 @@ function CollectionDocumentSuite () { var a2 = collection.remove(a1, true, true); assertEqual(a2, true); }, +//////////////////////////////////////////////////////////////////////////////// +/// @brief delete a document, waitForSync=true , using new signature +//////////////////////////////////////////////////////////////////////////////// + + testDeleteWithNewSignatureDocumentSyncTrue : function () { + var a1 = collection.save({ a : 1}); + + assertTypeOf("string", a1._id); + assertTypeOf("string", a1._rev); + + var a2 = collection.remove(a1, {"overwrite": true, "waitForSync": true}); + assertEqual(a2, true); + }, //////////////////////////////////////////////////////////////////////////////// /// @brief delete a deleted document @@ -1784,11 +1871,43 @@ function DatabaseDocumentSuite () { assertEqual(ERRORS.ERROR_ARANGO_CONFLICT.code, err.errorNum); } - var a3 = db._remove(a1, true); + var a3 = db._remove(a1, {"overwrite" : true}); assertEqual(a3, true); - var a4 = db._remove(a1, true); + var a4 = db._remove(a1, {"overwrite" : true}); + + assertEqual(a4, false); + }, +//////////////////////////////////////////////////////////////////////////////// +/// @brief delete a document using new signature of the remove function +/// of the remove function +//////////////////////////////////////////////////////////////////////////////// + + testDeleteWithNewSignatureDocument : function () { + var a1 = collection.save({ a : 1}); + + assertTypeOf("string", a1._id); + assertTypeOf("string", a1._rev); + + var a2 = db._replace(a1, { a : 2 }); + + assertEqual(a1._id, a2._id); + assertNotEqual(a1._rev, a2._rev); + + try { + db._remove(a1); + fail(); + } + catch (err) { + assertEqual(ERRORS.ERROR_ARANGO_CONFLICT.code, err.errorNum); + } + + var a3 = db._remove(a1, {"overwrite" : true}); + + assertEqual(a3, true); + + var a4 = db._remove(a1, {"overwrite" : true}); assertEqual(a4, false); },