Merge branch 'spdvpk' of https://github.com/arangodb/arangodb into spdvpk

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();
+  OperationOptions opOptions;
-  bool const waitForSync = extractWaitForSync();
+  opOptions.waitForSync = extractWaitForSync();
 
-  if (ServerState::instance()->isCoordinator()) {
+  VPackBuilder builder;
-    return modifyDocumentCoordinator(collection, key, revision, policy,
+  {
-                                     waitForSync, isPatch, body);
+    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();
+  OperationResult result(TRI_ERROR_NO_ERROR);
-  // 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;
-  }
-
   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
+    result = trx.update(collection, search, body, opOptions);
-    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<TRI_json_t> 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);
   } else {
-    // replacing an existing document, using a lock
+    // replacing an existing document
-
+    result = trx.replace(collection, search, body, opOptions);
-    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<TRI_json_t> 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<TRI_json_t> json(
-        arangodb::basics::VelocyPackHelper::velocyPackToJson(body));
-    res = trx.update(trx.trxCollection(), key, 0, &mptr, json.get(), policy,
-                     revision, &rid, waitForSync);
   }
 
-  res = trx.finish(res);
+  res = trx.finish(result.code);
 
   // .............................................................................
   // outside write transaction
   // .............................................................................
 
-  if (res != TRI_ERROR_NO_ERROR) {
+  if (result.failed()) {
-    generateTransactionError(collectionName, res, (TRI_voc_key_t)key.c_str(),
+    generateTransactionError(result);
-                             rid);
-
     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;
 }

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
 ////////////////////////////////////////////////////////////////////////////////

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
   ///