diff --git a/arangod/RestHandler/RestDocumentHandler.cpp b/arangod/RestHandler/RestDocumentHandler.cpp index 58b475b9a1..9c372031c0 100644 --- a/arangod/RestHandler/RestDocumentHandler.cpp +++ b/arangod/RestHandler/RestDocumentHandler.cpp @@ -151,7 +151,7 @@ bool RestDocumentHandler::createDocument() { // ............................................................................. if (result.failed()) { - generateTransactionError(collection, result.code); + generateTransactionError(result); return false; } @@ -550,15 +550,19 @@ bool RestDocumentHandler::modifyDocument(bool isPatch) { } // extract or chose the update policy - TRI_doc_update_policy_e const policy = extractUpdatePolicy(); - bool const waitForSync = extractWaitForSync(); + OperationOptions opOptions; + opOptions.waitForSync = extractWaitForSync(); - if (ServerState::instance()->isCoordinator()) { - return modifyDocumentCoordinator(collection, key, revision, policy, - waitForSync, isPatch, body); + VPackBuilder builder; + { + VPackObjectBuilder guard(&builder); + builder.add(TRI_VOC_ATTRIBUTE_KEY, VPackValue(key)); + if (revision != 0) { + builder.add(TRI_VOC_ATTRIBUTE_REV, VPackValue(revision)); + } } - TRI_doc_mptr_copy_t mptr; + VPackSlice search = builder.slice(); // find and load collection given by name or identifier SingleCollectionWriteTransaction<1> trx(new StandaloneTransactionContext(), @@ -575,187 +579,52 @@ bool RestDocumentHandler::modifyDocument(bool isPatch) { return false; } - TRI_voc_cid_t const cid = trx.cid(); - // If we are a DBserver, we want to use the cluster-wide collection - // name for error reporting: - std::string collectionName = collection; - if (ServerState::instance()->isDBServer()) { - collectionName = trx.resolver()->getCollectionName(cid); - } - - TRI_voc_rid_t rid = 0; - TRI_document_collection_t* document = trx.documentCollection(); - TRI_ASSERT(document != nullptr); - auto shaper = document->getShaper(); // PROTECTED by trx here - - std::string const&& cidString = StringUtils::itoa(document->_info.planId()); - - if (trx.orderDitch(trx.trxCollection()) == nullptr) { - generateTransactionError(collectionName, TRI_ERROR_OUT_OF_MEMORY); - return false; - } - + OperationResult result(TRI_ERROR_NO_ERROR); if (isPatch) { // patching an existing document - bool nullMeansRemove; - bool mergeObjects; bool found; char const* valueStr = _request->value("keepNull", found); if (!found || StringUtils::boolean(valueStr)) { // default: null values are saved as Null - nullMeansRemove = false; + opOptions.keepNull = true; } else { // delete null attributes - nullMeansRemove = true; + opOptions.keepNull = false; } valueStr = _request->value("mergeObjects", found); if (!found || StringUtils::boolean(valueStr)) { // the default is true - mergeObjects = true; + opOptions.mergeObjects = true; } else { - mergeObjects = false; + opOptions.mergeObjects = false; } - // read the existing document - TRI_doc_mptr_copy_t oldDocument; - - // create a write lock that spans the initial read and the update - // otherwise the update is not atomic - trx.lockWrite(); - - // do not lock again - res = trx.document(trx.trxCollection(), &oldDocument, key); - - if (res != TRI_ERROR_NO_ERROR) { - trx.abort(); - generateTransactionError(collectionName, res, (TRI_voc_key_t)key.c_str(), - rid); - return false; - } - - if (oldDocument.getDataPtr() == nullptr) { // PROTECTED by trx here - trx.abort(); - generateTransactionError(collectionName, - TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND, - (TRI_voc_key_t)key.c_str(), rid); - return false; - } - - TRI_shaped_json_t shapedJson; - TRI_EXTRACT_SHAPED_JSON_MARKER( - shapedJson, oldDocument.getDataPtr()); // PROTECTED by trx here - TRI_json_t* old = TRI_JsonShapedJson(shaper, &shapedJson); - - if (old == nullptr) { - trx.abort(); - generateTransactionError(collectionName, TRI_ERROR_OUT_OF_MEMORY); - return false; - } - - std::unique_ptr json( - arangodb::basics::VelocyPackHelper::velocyPackToJson(body)); - if (ServerState::instance()->isDBServer()) { - // compare attributes in shardKeys - if (shardKeysChanged(_request->databaseName(), cidString, old, json.get(), - true)) { - TRI_FreeJson(shaper->memoryZone(), old); - - trx.abort(); - generateTransactionError( - collectionName, - TRI_ERROR_CLUSTER_MUST_NOT_CHANGE_SHARDING_ATTRIBUTES); - - return false; - } - } - - TRI_json_t* patchedJson = TRI_MergeJson( - TRI_UNKNOWN_MEM_ZONE, old, json.get(), nullMeansRemove, mergeObjects); - TRI_FreeJson(shaper->memoryZone(), old); - - if (patchedJson == nullptr) { - trx.abort(); - generateTransactionError(collectionName, TRI_ERROR_OUT_OF_MEMORY); - - return false; - } - - // do not acquire an extra lock - res = trx.update(trx.trxCollection(), key, 0, &mptr, patchedJson, policy, - revision, &rid, waitForSync); - TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, patchedJson); + result = trx.update(collection, search, body, opOptions); } else { - // replacing an existing document, using a lock - - if (ServerState::instance()->isDBServer()) { - // compare attributes in shardKeys - // read the existing document - TRI_doc_mptr_copy_t oldDocument; - - // do not lock again - trx.lockWrite(); - - res = trx.document(trx.trxCollection(), &oldDocument, key); - if (res != TRI_ERROR_NO_ERROR) { - trx.abort(); - generateTransactionError(collectionName, res, - (TRI_voc_key_t)key.c_str(), rid); - return false; - } - - if (oldDocument.getDataPtr() == nullptr) { // PROTECTED by trx here - trx.abort(); - generateTransactionError(collectionName, - TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND, - (TRI_voc_key_t)key.c_str(), rid); - return false; - } - - TRI_shaped_json_t shapedJson; - TRI_EXTRACT_SHAPED_JSON_MARKER( - shapedJson, oldDocument.getDataPtr()); // PROTECTED by trx here - TRI_json_t* old = TRI_JsonShapedJson(shaper, &shapedJson); - - std::unique_ptr json( - arangodb::basics::VelocyPackHelper::velocyPackToJson(body)); - if (shardKeysChanged(_request->databaseName(), cidString, old, json.get(), - false)) { - TRI_FreeJson(shaper->memoryZone(), old); - - trx.abort(); - generateTransactionError( - collectionName, - TRI_ERROR_CLUSTER_MUST_NOT_CHANGE_SHARDING_ATTRIBUTES); - - return false; - } - - if (old != nullptr) { - TRI_FreeJson(shaper->memoryZone(), old); - } - } - - std::unique_ptr json( - arangodb::basics::VelocyPackHelper::velocyPackToJson(body)); - res = trx.update(trx.trxCollection(), key, 0, &mptr, json.get(), policy, - revision, &rid, waitForSync); + // replacing an existing document + result = trx.replace(collection, search, body, opOptions); } - res = trx.finish(res); + res = trx.finish(result.code); // ............................................................................. // outside write transaction // ............................................................................. - if (res != TRI_ERROR_NO_ERROR) { - generateTransactionError(collectionName, res, (TRI_voc_key_t)key.c_str(), - rid); - + if (result.failed()) { + generateTransactionError(result); return false; } - generateSaved(trx, cid, mptr); + if (res != TRI_ERROR_NO_ERROR) { + // This should not occur. Updated worked but commit failed + generateTransactionError(collection, res, (TRI_voc_key_t)key.c_str(), 0); + return false; + } + + // TODO Fix Collection Type! + generateSaved(result, collection, TRI_COL_TYPE_DOCUMENT); return true; } diff --git a/arangod/RestHandler/RestVocbaseBaseHandler.cpp b/arangod/RestHandler/RestVocbaseBaseHandler.cpp index 679d6e2363..0e2f517dea 100644 --- a/arangod/RestHandler/RestVocbaseBaseHandler.cpp +++ b/arangod/RestHandler/RestVocbaseBaseHandler.cpp @@ -515,6 +515,7 @@ void RestVocbaseBaseHandler::generateDocument(VPackSlice const& document, //////////////////////////////////////////////////////////////////////////////// /// @brief generate an error message for a transaction error +/// DEPRECATED //////////////////////////////////////////////////////////////////////////////// void RestVocbaseBaseHandler::generateTransactionError( @@ -609,6 +610,94 @@ void RestVocbaseBaseHandler::generateTransactionError( } } +//////////////////////////////////////////////////////////////////////////////// +/// @brief generate an error message for a transaction error +//////////////////////////////////////////////////////////////////////////////// + +void RestVocbaseBaseHandler::generateTransactionError( + OperationResult const& result) { + switch (result.code) { + case TRI_ERROR_ARANGO_READ_ONLY: + generateError(HttpResponse::FORBIDDEN, result.code, + "collection is read-only"); + return; + + case TRI_ERROR_ARANGO_UNIQUE_CONSTRAINT_VIOLATED: + generateError(HttpResponse::CONFLICT, result.code, + "cannot create document, unique constraint violated"); + return; + + case TRI_ERROR_ARANGO_DOCUMENT_KEY_BAD: + generateError(HttpResponse::BAD, result.code, "invalid document key"); + return; + + case TRI_ERROR_ARANGO_OUT_OF_KEYS: + generateError(HttpResponse::SERVER_ERROR, result.code, "out of keys"); + return; + + case TRI_ERROR_ARANGO_DOCUMENT_KEY_UNEXPECTED: + generateError(HttpResponse::BAD, result.code, + "collection does not allow using user-defined keys"); + return; + + case TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND: + generateError(rest::HttpResponse::NOT_FOUND, + TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND); + return; + + case TRI_ERROR_ARANGO_DOCUMENT_TYPE_INVALID: + generateError(HttpResponse::BAD, result.code); + return; + + case TRI_ERROR_ARANGO_CONFLICT: + generatePreconditionFailed(result.slice()); + return; + + case TRI_ERROR_CLUSTER_SHARD_GONE: + generateError(HttpResponse::SERVER_ERROR, result.code, + "coordinator: no responsible shard found"); + return; + + case TRI_ERROR_CLUSTER_TIMEOUT: + generateError(HttpResponse::SERVER_ERROR, result.code); + return; + + case TRI_ERROR_CLUSTER_MUST_NOT_CHANGE_SHARDING_ATTRIBUTES: + case TRI_ERROR_CLUSTER_MUST_NOT_SPECIFY_KEY: { + generateError(HttpResponse::BAD, result.code); + return; + } + + case TRI_ERROR_CLUSTER_UNSUPPORTED: { + generateError(HttpResponse::NOT_IMPLEMENTED, result.code); + return; + } + + case TRI_ERROR_FORBIDDEN: { + generateError(HttpResponse::FORBIDDEN, result.code); + return; + } + + case TRI_ERROR_OUT_OF_MEMORY: + case TRI_ERROR_LOCK_TIMEOUT: + case TRI_ERROR_AID_NOT_FOUND: + case TRI_ERROR_DEBUG: + case TRI_ERROR_LEGEND_NOT_IN_WAL_FILE: + case TRI_ERROR_LOCKED: + case TRI_ERROR_DEADLOCK: { + generateError(HttpResponse::SERVER_ERROR, result.code); + return; + } + + default: + generateError( + HttpResponse::SERVER_ERROR, TRI_ERROR_INTERNAL, + "failed with error: " + std::string(TRI_errno_string(result.code))); + } +} + + + //////////////////////////////////////////////////////////////////////////////// /// @brief extracts the revision //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/RestHandler/RestVocbaseBaseHandler.h b/arangod/RestHandler/RestVocbaseBaseHandler.h index d030f2dba8..3f702df8c8 100644 --- a/arangod/RestHandler/RestVocbaseBaseHandler.h +++ b/arangod/RestHandler/RestVocbaseBaseHandler.h @@ -317,11 +317,18 @@ class RestVocbaseBaseHandler : public RestBaseHandler { ////////////////////////////////////////////////////////////////////////////// /// @brief generate an error message for a transaction error + /// DEPRECATED ////////////////////////////////////////////////////////////////////////////// void generateTransactionError(std::string const&, int, TRI_voc_key_t = 0, TRI_voc_rid_t = 0); + ////////////////////////////////////////////////////////////////////////////// + /// @brief generate an error message for a transaction error + ////////////////////////////////////////////////////////////////////////////// + + void generateTransactionError(OperationResult const&); + ////////////////////////////////////////////////////////////////////////////// /// @brief extracts the revision ///