validate collection when leasing an existing transaction (#9990)

* validate collection when leasing an existing transaction

* use WITH statement

* allow implicit read collections again

* simplify collection name validation

* re-implement it

* remove dead code

* remove unused method

* apply review comments

* whoops, deleted too many methods

* fixit

arangod/Aql/OptimizerRules.cpp

@@ -825,7 +825,7 @@ Collection* addCollectionToQuery(Query* query, std::string const& cname, bool as
       auto cptr = query->trx()->vocbase().lookupCollection(cname);
 
       coll->setCollection(cptr.get());
-      query->trx()->addCollectionAtRuntime(cname);
+      query->trx()->addCollectionAtRuntime(cname, AccessMode::Type::READ);
     }
   }
 

arangod/RestHandler/RestCollectionHandler.cpp

@@ -611,7 +611,22 @@ void RestCollectionHandler::collectionRepresentation(
     bool showProperties, bool showFigures, bool showCount, bool detailedCount) {
   if (showProperties || showCount) {
     // Here we need a transaction
-    auto trx = createTransaction(coll.name(), AccessMode::Type::READ);
+    std::unique_ptr<transaction::Methods> trx;
+    try {
+      trx = createTransaction(coll.name(), AccessMode::Type::READ);
+    } catch (basics::Exception const& ex) {
+      if (ex.code() == TRI_ERROR_TRANSACTION_NOT_FOUND) {
+      // this will happen if the tid of a managed transaction is passed in,
+      // but the transaction hasn't yet started on the DB server. in
+      // this case, we create an ad-hoc transaction on the underlying
+      // collection
+        trx = std::make_unique<SingleCollectionTransaction>(transaction::StandaloneContext::Create(_vocbase), coll.name(), AccessMode::Type::READ);
+      } else {
+        throw;
+      }
+    }
+
+    TRI_ASSERT(trx != nullptr);
     Result res = trx->begin();
 
     if (res.fail()) {

arangod/RestHandler/RestEdgesHandler.cpp

@@ -27,10 +27,10 @@
 #include "Aql/Variable.h"
 #include "Cluster/ClusterMethods.h"
 #include "Cluster/ServerState.h"
+#include "Transaction/Methods.h"
 #include "Transaction/StandaloneContext.h"
 #include "Utils/CollectionNameResolver.h"
 #include "Utils/OperationCursor.h"
-#include "Utils/SingleCollectionTransaction.h"
 
 #include <velocypack/Iterator.h>
 #include <velocypack/velocypack-aliases.h>
@@ -64,7 +64,7 @@ RestStatus RestEdgesHandler::execute() {
 
 void RestEdgesHandler::readCursor(aql::AstNode* condition, aql::Variable const* var,
                                   std::string const& collectionName,
-                                  SingleCollectionTransaction& trx,
+                                  transaction::Methods& trx,
                                   std::function<void(LocalDocumentId const&)> const& cb) {
   transaction::Methods::IndexHandle indexId;
   bool foundIdx =
@@ -85,10 +85,10 @@ void RestEdgesHandler::readCursor(aql::AstNode* condition, aql::Variable const*
 }
 
 bool RestEdgesHandler::getEdgesForVertex(
-    std::string const& id, std::string const& collectionName,
+    std::string const& id, TRI_voc_cid_t cid, std::string const& collectionName,
-    TRI_edge_direction_e direction, SingleCollectionTransaction& trx,
+    TRI_edge_direction_e direction, transaction::Methods& trx,
     std::function<void(LocalDocumentId const&)> const& cb) {
-  trx.pinData(trx.cid());  // will throw when it fails
+  trx.pinData(cid);  // will throw when it fails
 
   // Create a conditionBuilder that manages the AstNodes for querying
   aql::EdgeConditionBuilderContainer condBuilder;
@@ -234,7 +234,8 @@ bool RestEdgesHandler::readEdges() {
   resultBuilder.add(VPackValue("edges"));  // only key
   resultBuilder.openArray();
 
-  auto collection = trx->documentCollection();
+  TRI_voc_cid_t cid = trx->addCollectionAtRuntime(collectionName, AccessMode::Type::READ);
+  auto collection = trx->documentCollection(cid);
   std::unordered_set<LocalDocumentId> foundTokens;
   auto cb = [&](LocalDocumentId const& token) {
     if (foundTokens.find(token) == foundTokens.end()) {
@@ -249,7 +250,7 @@ bool RestEdgesHandler::readEdges() {
   };
 
   // NOTE: collectionName is the shard-name in DBServer case
-  bool ok = getEdgesForVertex(startVertex, collectionName, direction, *(trx.get()), cb);
+  bool ok = getEdgesForVertex(startVertex, cid, collectionName, direction, *(trx.get()), cb);
   resultBuilder.close();
 
   res = trx->finish(res);
@@ -262,7 +263,7 @@ bool RestEdgesHandler::readEdges() {
     if (ServerState::instance()->isDBServer()) {
       // If we are a DBserver, we want to use the cluster-wide collection
       // name for error reporting:
-      collectionName = trx->resolver()->getCollectionNameCluster(trx->cid());
+      collectionName = trx->resolver()->getCollectionNameCluster(cid);
     }
     generateTransactionError(collectionName, res, "");
     return false;
@@ -373,7 +374,8 @@ bool RestEdgesHandler::readEdgesForMultipleVertices() {
   resultBuilder.add(VPackValue("edges"));  // only key
   resultBuilder.openArray();
 
-  auto collection = trx->documentCollection();
+  TRI_voc_cid_t cid = trx->addCollectionAtRuntime(collectionName, AccessMode::Type::READ);
+  auto collection = trx->documentCollection(cid);
   std::unordered_set<LocalDocumentId> foundTokens;
   auto cb = [&](LocalDocumentId const& token) {
     if (foundTokens.find(token) == foundTokens.end()) {
@@ -392,7 +394,7 @@ bool RestEdgesHandler::readEdgesForMultipleVertices() {
       std::string startVertex = it.copyString();
 
       // We ignore if this fails
-      getEdgesForVertex(startVertex, collectionName, direction, *(trx.get()), cb);
+      getEdgesForVertex(startVertex, cid, collectionName, direction, *(trx.get()), cb);
     }
   }
   resultBuilder.close();
@@ -402,7 +404,7 @@ bool RestEdgesHandler::readEdgesForMultipleVertices() {
     if (ServerState::instance()->isDBServer()) {
       // If we are a DBserver, we want to use the cluster-wide collection
       // name for error reporting:
-      collectionName = trx->resolver()->getCollectionNameCluster(trx->cid());
+      collectionName = trx->resolver()->getCollectionNameCluster(cid);
     }
     generateTransactionError(collectionName, res, "");
     return false;

arangod/RestHandler/RestEdgesHandler.h

@@ -31,7 +31,9 @@
 
 namespace arangodb {
 class LocalDocumentId;
-class SingleCollectionTransaction;
+namespace transaction {
+class Methods;
+}
 
 namespace aql {
 struct AstNode;
@@ -65,15 +67,16 @@ class RestEdgesHandler : public RestVocbaseBaseHandler {
   //////////////////////////////////////////////////////////////////////////////
 
   void readCursor(aql::AstNode* condition, aql::Variable const* var,
-                  std::string const& collectionName, SingleCollectionTransaction& trx,
+                  std::string const& collectionName, transaction::Methods& trx,
                   std::function<void(LocalDocumentId const&)> const& cb);
 
   //////////////////////////////////////////////////////////////////////////////
   /// @brief get all edges for a given vertex. Independent from the request
   //////////////////////////////////////////////////////////////////////////////
 
-  bool getEdgesForVertex(std::string const& id, std::string const& collectionName,
+  bool getEdgesForVertex(std::string const& id, TRI_voc_cid_t cid,
-                         TRI_edge_direction_e direction, SingleCollectionTransaction& trx,
+                         std::string const& collectionName,
+                         TRI_edge_direction_e direction, transaction::Methods& trx,
                          std::function<void(LocalDocumentId const&)> const& cb);
 
   //////////////////////////////////////////////////////////////////////////////

arangod/RestHandler/RestIndexHandler.cpp

@@ -26,6 +26,7 @@
 #include "Cluster/ClusterInfo.h"
 #include "Cluster/ServerState.h"
 #include "Rest/HttpRequest.h"
+#include "Transaction/Methods.h"
 #include "Transaction/StandaloneContext.h"
 #include "Utils/Events.h"
 #include "Utils/SingleCollectionTransaction.h"
@@ -180,7 +181,7 @@ RestStatus RestIndexHandler::getSelectivityEstimates() {
   }
 
   // transaction protects access onto selectivity estimates
-  std::unique_ptr<SingleCollectionTransaction> trx;
+  std::unique_ptr<transaction::Methods> trx;
 
   try {
     trx = createTransaction(cName, AccessMode::Type::READ);
@@ -204,7 +205,7 @@ RestStatus RestIndexHandler::getSelectivityEstimates() {
     return RestStatus::DONE;
   }
   
-  LogicalCollection* coll = trx->documentCollection();
+  LogicalCollection* coll = trx->documentCollection(cName);
   auto idxs = coll->getIndexes();
   
   VPackBuffer<uint8_t> buffer;

arangod/RestHandler/RestVocbaseBaseHandler.cpp

@@ -36,6 +36,7 @@
 #include "Transaction/Helpers.h"
 #include "Transaction/Manager.h"
 #include "Transaction/ManagerFeature.h"
+#include "Transaction/Methods.h"
 #include "Transaction/SmartContext.h"
 #include "Transaction/StandaloneContext.h"
 #include "Utils/SingleCollectionTransaction.h"
@@ -51,6 +52,15 @@ using namespace arangodb;
 using namespace arangodb::basics;
 using namespace arangodb::rest;
 
+namespace {
+class SimpleTransaction : public transaction::Methods {
+ public:
+  SimpleTransaction(std::shared_ptr<transaction::Context>&& transactionContext,
+                    transaction::Options&& options = transaction::Options())
+    : Methods(std::move(transactionContext), std::move(options)) {}
+};
+} // namespace
+
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief agency public path
 ////////////////////////////////////////////////////////////////////////////////
@@ -543,7 +553,7 @@ void RestVocbaseBaseHandler::extractStringParameter(std::string const& name,
   }
 }
 
-std::unique_ptr<SingleCollectionTransaction> RestVocbaseBaseHandler::createTransaction(
+std::unique_ptr<transaction::Methods> RestVocbaseBaseHandler::createTransaction(
     std::string const& collectionName, AccessMode::Type type) const {
   bool found = false;
   std::string value = _request->header(StaticStrings::TransactionId, found);
@@ -581,7 +591,7 @@ std::unique_ptr<SingleCollectionTransaction> RestVocbaseBaseHandler::createTrans
       LOG_TOPIC("e94ea", DEBUG, Logger::TRANSACTIONS) << "Transaction with id '" << tid << "' not found";
       THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_TRANSACTION_NOT_FOUND, std::string("transaction '") + std::to_string(tid) + "' not found");
     }
-    return std::make_unique<SingleCollectionTransaction>(ctx, collectionName, type);
+    return std::make_unique<SimpleTransaction>(std::move(ctx));
   } else {
     auto ctx = transaction::StandaloneContext::Create(_vocbase);
     return std::make_unique<SingleCollectionTransaction>(ctx, collectionName, type);

arangod/RestHandler/RestVocbaseBaseHandler.h

@@ -35,8 +35,10 @@
 struct TRI_vocbase_t;
 
 namespace arangodb {
+namespace transaction {
+class Methods;
+}
 
-class SingleCollectionTransaction;
 class VocbaseContext;
 
 /// @brief abstract base request handler
@@ -228,7 +230,7 @@ class RestVocbaseBaseHandler : public RestBaseHandler {
    * @return A freshly created transaction for the given collection with proper
    * locking or a leased transaction.
    */
-  std::unique_ptr<SingleCollectionTransaction> createTransaction(std::string const& cname,
+  std::unique_ptr<transaction::Methods> createTransaction(std::string const& cname,
                                                           AccessMode::Type mode) const;
   
   /// @brief create proper transaction context, including the proper IDs

arangod/StorageEngine/TransactionState.cpp

@@ -84,6 +84,24 @@ TransactionCollection* TransactionState::collection(TRI_voc_cid_t cid,
   return trxCollection;
 }
 
+/// @brief return the collection from a transaction
+TransactionCollection* TransactionState::collection(std::string const& name,
+                                                    AccessMode::Type accessType) const {
+  TRI_ASSERT(_status == transaction::Status::CREATED ||
+             _status == transaction::Status::RUNNING);
+
+  auto it = std::find_if(_collections.begin(), _collections.end(), [&name](TransactionCollection const* trxColl) {
+    return trxColl->collectionName() == name;
+  });
+
+  if (it == _collections.end() || !(*it)->canAccess(accessType)) {
+    // not found or not accessible in the requested mode
+    return nullptr;
+  }
+
+  return (*it);
+}
+
 TransactionState::Cookie* TransactionState::cookie(void const* key) noexcept {
   auto itr = _cookies.find(key);
 

arangod/StorageEngine/TransactionState.h

@@ -138,6 +138,10 @@ class TransactionState {
   TransactionCollection* collection(TRI_voc_cid_t cid,
                                     AccessMode::Type accessType) const;
   
+  /// @brief return the collection from a transaction
+  TransactionCollection* collection(std::string const& name,
+                                    AccessMode::Type accessType) const;
+
   /// @brief add a collection to a transaction
   Result addCollection(TRI_voc_cid_t cid, std::string const& cname,
                        AccessMode::Type accessType, int nestingLevel, bool force);

arangod/Transaction/Manager.cpp

@@ -199,10 +199,10 @@ uint64_t Manager::getActiveTransactionCount() {
 
 Manager::ManagedTrx::ManagedTrx(MetaType t, TransactionState* st)
     : type(t),
+      finalStatus(Status::UNDEFINED),
       usedTimeSecs(TRI_microtime()),
       state(st),
       user(::currentUser()),
-      finalStatus(Status::UNDEFINED),
       rwlock() {}
 
 bool Manager::ManagedTrx::expired() const {
@@ -714,6 +714,8 @@ Result Manager::updateTransaction(TRI_voc_tid_t tid, transaction::Status status,
                      "transaction was not running");
   }
 
+  bool const isCoordinator = state->isCoordinator();
+
   auto ctx = std::make_shared<ManagedContext>(tid, state.get(), AccessMode::Type::NONE);
   state.release();  // now owned by ctx
 
@@ -723,7 +725,7 @@ Result Manager::updateTransaction(TRI_voc_tid_t tid, transaction::Status status,
   TRI_ASSERT(trx.state()->nestingLevel() == 1);
   trx.state()->decreaseNesting();
   TRI_ASSERT(trx.state()->isTopLevelTransaction());
-  if (clearServers) {
+  if (clearServers && !isCoordinator) {
     trx.state()->clearKnownServers();
   }
   if (status == transaction::Status::COMMITTED) {

arangod/Transaction/Manager.h

@@ -75,13 +75,13 @@ class Manager final {
 
    public:
     MetaType type;            /// managed, AQL or tombstone
-    double usedTimeSecs;      /// last time used
-    TransactionState* state;  /// Transaction, may be nullptr
-    std::string user;         /// user owning the transaction
     /// @brief  final TRX state that is valid if this is a tombstone
     /// necessary to avoid getting error on a 'diamond' commit or accidantally
     /// repeated commit / abort messages
     transaction::Status finalStatus;
+    double usedTimeSecs;      /// last time used
+    TransactionState* state;  /// Transaction, may be nullptr
+    std::string user;         /// user owning the transaction
     /// cheap usage lock for *state
     mutable basics::ReadWriteSpinLock rwlock;
   };

arangod/Transaction/Methods.cpp

@@ -826,6 +826,15 @@ TransactionCollection* transaction::Methods::trxCollection(TRI_voc_cid_t cid,
   return _state->collection(cid, type);
 }
 
+/// @brief return the transaction collection for a document collection
+TransactionCollection* transaction::Methods::trxCollection(std::string const& name,
+                                                           AccessMode::Type type) const {
+  TRI_ASSERT(_state != nullptr);
+  TRI_ASSERT(_state->status() == transaction::Status::RUNNING ||
+             _state->status() == transaction::Status::CREATED);
+  return _state->collection(name, type);
+}
+
 /// @brief order a ditch for a collection
 void transaction::Methods::pinData(TRI_voc_cid_t cid) {
   TRI_ASSERT(_state != nullptr);
@@ -1104,16 +1113,24 @@ TRI_voc_cid_t transaction::Methods::addCollectionAtRuntime(TRI_voc_cid_t cid,
 
     res = applyDataSourceRegistrationCallbacks(*dataSource, *this);
 
+    if (res.ok()) {
+      res = _state->ensureCollections(_state->nestingLevel());
+    }
     if (res.fail()) {
       THROW_ARANGO_EXCEPTION(res);
     }
-
-    _state->ensureCollections(_state->nestingLevel());
     collection = trxCollection(cid);
 
     if (collection == nullptr) {
       throwCollectionNotFound(cname.c_str());
     }
+  } else {
+    AccessMode::Type collectionAccessType = collection->accessType();
+    if (AccessMode::isRead(collectionAccessType) && !AccessMode::isRead(type)) {
+      THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_TRANSACTION_UNREGISTERED_COLLECTION, 
+                                     std::string(TRI_errno_string(TRI_ERROR_TRANSACTION_UNREGISTERED_COLLECTION)) + ": " + cname + 
+                                     " [" + AccessMode::typeString(type) + "]");
+    }
   }
 
   TRI_ASSERT(collection != nullptr);
@@ -1121,7 +1138,8 @@ TRI_voc_cid_t transaction::Methods::addCollectionAtRuntime(TRI_voc_cid_t cid,
 }
 
 /// @brief add a collection to the transaction for read, at runtime
-TRI_voc_cid_t transaction::Methods::addCollectionAtRuntime(std::string const& collectionName) {
+TRI_voc_cid_t transaction::Methods::addCollectionAtRuntime(std::string const& collectionName,
+                                                           AccessMode::Type type) {
   if (collectionName == _collectionCache.name && !collectionName.empty()) {
     return _collectionCache.cid;
   }
@@ -1131,7 +1149,7 @@ TRI_voc_cid_t transaction::Methods::addCollectionAtRuntime(std::string const& co
   if (cid == 0) {
     throwCollectionNotFound(collectionName.c_str());
   }
-  addCollectionAtRuntime(cid, collectionName);
+  addCollectionAtRuntime(cid, collectionName, type);
   _collectionCache.cid = cid;
   _collectionCache.name = collectionName;
   return cid;
@@ -1164,8 +1182,9 @@ void transaction::Methods::invokeOnAllElements(std::string const& collectionName
     THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED);
   }
 
-  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName);
+  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName, AccessMode::Type::READ);
   TransactionCollection* trxCol = trxCollection(cid, AccessMode::Type::READ);
+  TRI_ASSERT(trxCol != nullptr);
   LogicalCollection* collection = documentCollection(trxCol);
   TRI_ASSERT(collection != nullptr);
   _transactionContextPtr->pinData(collection);
@@ -1218,7 +1237,7 @@ Result transaction::Methods::documentFastPath(std::string const& collectionName,
     return Result();
   }
 
-  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName);
+  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName, AccessMode::Type::READ);
   LogicalCollection* collection = documentCollection(trxCollection(cid));
 
   pinData(cid);  // will throw when it fails
@@ -1264,8 +1283,9 @@ Result transaction::Methods::documentFastPathLocal(std::string const& collection
   TRI_ASSERT(!ServerState::instance()->isCoordinator());
   TRI_ASSERT(_state->status() == transaction::Status::RUNNING);
 
-  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName);
+  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName, AccessMode::Type::READ);
   TransactionCollection* trxColl = trxCollection(cid);
+  TRI_ASSERT(trxColl != nullptr);
   LogicalCollection* collection = documentCollection(trxColl);
   TRI_ASSERT(collection != nullptr);
   _transactionContextPtr->pinData(collection);  // will throw when it fails
@@ -1491,7 +1511,7 @@ OperationResult transaction::Methods::documentCoordinator(std::string const& col
 OperationResult transaction::Methods::documentLocal(std::string const& collectionName,
                                                     VPackSlice const value,
                                                     OperationOptions& options) {
-  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName);
+  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName, AccessMode::Type::READ);
   LogicalCollection* collection = documentCollection(trxCollection(cid));
 
   if (!options.silent) {
@@ -1647,7 +1667,7 @@ static double chooseTimeout(size_t count, size_t totalBytes) {
 OperationResult transaction::Methods::insertLocal(std::string const& collectionName,
                                                   VPackSlice const value,
                                                   OperationOptions& options) {
-  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName);
+  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName, AccessMode::Type::WRITE);
   LogicalCollection* collection = documentCollection(trxCollection(cid));
 
   bool const needsLock = !isLocked(collection, AccessMode::Type::WRITE);
@@ -1995,7 +2015,7 @@ OperationResult transaction::Methods::modifyLocal(std::string const& collectionN
                                                   VPackSlice const newValue,
                                                   OperationOptions& options,
                                                   TRI_voc_document_operation_e operation) {
-  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName);
+  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName, AccessMode::Type::WRITE);
   LogicalCollection* collection = documentCollection(trxCollection(cid));
 
   bool const needsLock = !isLocked(collection, AccessMode::Type::WRITE);
@@ -2263,7 +2283,7 @@ OperationResult transaction::Methods::removeCoordinator(std::string const& colle
 OperationResult transaction::Methods::removeLocal(std::string const& collectionName,
                                                   VPackSlice const value,
                                                   OperationOptions& options) {
-  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName);
+  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName, AccessMode::Type::WRITE);
   LogicalCollection* collection = documentCollection(trxCollection(cid));
 
   bool const needsLock = !isLocked(collection, AccessMode::Type::WRITE);
@@ -2487,7 +2507,7 @@ OperationResult transaction::Methods::allCoordinator(std::string const& collecti
 OperationResult transaction::Methods::allLocal(std::string const& collectionName,
                                                uint64_t skip, uint64_t limit,
                                                OperationOptions& options) {
-  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName);
+  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName, AccessMode::Type::READ);
 
   pinData(cid);  // will throw when it fails
 
@@ -2550,7 +2570,7 @@ OperationResult transaction::Methods::truncateCoordinator(std::string const& col
 /// @brief remove all documents in a collection, local
 OperationResult transaction::Methods::truncateLocal(std::string const& collectionName,
                                                     OperationOptions& options) {
-  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName);
+  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName, AccessMode::Type::WRITE);
 
   LogicalCollection* collection = documentCollection(trxCollection(cid));
 
@@ -2604,7 +2624,6 @@ OperationResult transaction::Methods::truncateLocal(std::string const& collectio
   TRI_ASSERT(isLocked(collection, AccessMode::Type::WRITE));
 
   auto res = collection->truncate(*this, options);
-  ;
 
   if (res.fail()) {
     if (lockResult.is(TRI_ERROR_LOCKED)) {
@@ -2765,7 +2784,7 @@ OperationResult transaction::Methods::countCoordinatorHelper(
 /// @brief count the number of documents in a collection
 OperationResult transaction::Methods::countLocal(std::string const& collectionName,
                                                  transaction::CountType type) {
-  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName);
+  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName, AccessMode::Type::READ);
   LogicalCollection* collection = documentCollection(trxCollection(cid));
 
   Result lockResult = lockRecursive(cid, AccessMode::Type::READ);
@@ -3006,7 +3025,7 @@ std::unique_ptr<IndexIterator> transaction::Methods::indexScan(std::string const
     THROW_ARANGO_EXCEPTION(TRI_ERROR_CLUSTER_ONLY_ON_DBSERVER);
   }
 
-  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName);
+  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName, AccessMode::Type::READ);
   TransactionCollection* trxColl = trxCollection(cid);
   if (trxColl == nullptr) {
     THROW_ARANGO_EXCEPTION_MESSAGE(
@@ -3062,6 +3081,22 @@ arangodb::LogicalCollection* transaction::Methods::documentCollection(TRI_voc_ci
   return trxColl->collection().get();
 }
 
+/// @brief return the collection
+arangodb::LogicalCollection* transaction::Methods::documentCollection(std::string const& name) const {
+  TRI_ASSERT(_state != nullptr);
+  TRI_ASSERT(_state->status() == transaction::Status::RUNNING);
+
+  auto trxColl = trxCollection(name, AccessMode::Type::READ);
+  if (trxColl == nullptr) {
+    THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL,
+                                   "could not find collection");
+  }
+
+  TRI_ASSERT(trxColl != nullptr);
+  TRI_ASSERT(trxColl->collection() != nullptr);
+  return trxColl->collection().get();
+}
+
 /// @brief add a collection by id, with the name supplied
 Result transaction::Methods::addCollection(TRI_voc_cid_t cid, std::string const& cname,
                                            AccessMode::Type type) {
@@ -3183,7 +3218,7 @@ std::vector<std::shared_ptr<Index>> transaction::Methods::indexesForCollection(
   }
   // For a DBserver we use the local case.
 
-  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName);
+  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName, AccessMode::Type::READ);
   LogicalCollection* document = documentCollection(trxCollection(cid));
   std::vector<std::shared_ptr<Index>> indexes = document->getIndexes();
   if (!withHidden) {
@@ -3253,7 +3288,7 @@ transaction::Methods::IndexHandle transaction::Methods::getIndexByIdentifier(
     return IndexHandle(idx);
   }
 
-  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName);
+  TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName, AccessMode::Type::READ);
   LogicalCollection* document = documentCollection(trxCollection(cid));
 
   if (indexHandle.empty()) {

arangod/Transaction/Methods.h

@@ -248,10 +248,11 @@ class Methods {
   /// @brief add a collection to the transaction for read, at runtime
   ENTERPRISE_VIRT TRI_voc_cid_t
   addCollectionAtRuntime(TRI_voc_cid_t cid, std::string const& collectionName,
-                         AccessMode::Type type = AccessMode::Type::READ);
+                         AccessMode::Type type);
 
   /// @brief add a collection to the transaction for read, at runtime
-  virtual TRI_voc_cid_t addCollectionAtRuntime(std::string const& collectionName);
+  virtual TRI_voc_cid_t addCollectionAtRuntime(std::string const& collectionName,
+                                               AccessMode::Type type);
 
   /// @brief return the type of a collection
   bool isEdgeCollection(std::string const& collectionName) const;
@@ -379,7 +380,10 @@ class Methods {
   ENTERPRISE_VIRT bool isLocked(arangodb::LogicalCollection*, AccessMode::Type) const;
   
   /// @brief fetch the LogicalCollection by CID
-  arangodb::LogicalCollection* documentCollection(TRI_voc_cid_t) const;
+  arangodb::LogicalCollection* documentCollection(TRI_voc_cid_t cid) const;
+  
+  /// @brief fetch the LogicalCollection by name
+  arangodb::LogicalCollection* documentCollection(std::string const& name) const;
 
   /// @brief get the index by its identifier. Will either throw or
   ///        return a valid index. nullptr is impossible.
@@ -479,6 +483,9 @@ class Methods {
   ENTERPRISE_VIRT TransactionCollection* trxCollection(
       TRI_voc_cid_t cid, AccessMode::Type type = AccessMode::Type::READ) const;
   
+  TransactionCollection* trxCollection(
+      std::string const& name, AccessMode::Type type = AccessMode::Type::READ) const;
+
   OperationResult countCoordinator(std::string const& collectionName, CountType type);
 
   OperationResult countCoordinatorHelper(std::shared_ptr<LogicalCollection> const& collinfo,

arangod/Utils/SingleCollectionTransaction.cpp

@@ -22,6 +22,7 @@
 ////////////////////////////////////////////////////////////////////////////////
 
 #include "SingleCollectionTransaction.h"
+#include "Basics/StringUtils.h"
 #include "StorageEngine/TransactionCollection.h"
 #include "StorageEngine/TransactionState.h"
 #include "Transaction/Context.h"
@@ -59,7 +60,7 @@ SingleCollectionTransaction::SingleCollectionTransaction(
       _accessType(accessType) {
   // add the (sole) collection
   _cid = resolver()->getCollectionId(name);
-  Result res = addCollection(_cid, name.c_str(), _accessType);
+  Result res = addCollection(_cid, name, _accessType);
   if (res.fail()) {
     THROW_ARANGO_EXCEPTION(res);
   }
@@ -95,6 +96,19 @@ LogicalCollection* SingleCollectionTransaction::documentCollection() {
   return _documentCollection;
 }
   
+TRI_voc_cid_t SingleCollectionTransaction::addCollectionAtRuntime(std::string const& name,
+                                                                  AccessMode::Type type) {
+  // sanity check
+  TRI_ASSERT(!name.empty());
+  if ((name[0] < '0' || name[0] > '9') && 
+      name != resolveTrxCollection()->collectionName()) {
+    THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_TRANSACTION_UNREGISTERED_COLLECTION, 
+                                   std::string(TRI_errno_string(TRI_ERROR_TRANSACTION_UNREGISTERED_COLLECTION)) + ": " + name);
+  }
+
+  return _cid;
+}
+
 /// @brief get the underlying collection's name
 std::string SingleCollectionTransaction::name() {
   // will ensure we have the _trxCollection object set

arangod/Utils/SingleCollectionTransaction.h

@@ -25,6 +25,7 @@
 #define ARANGOD_UTILS_SINGLE_COLLECTION_TRANSACTION_H 1
 
 #include "Basics/Common.h"
+#include "StorageEngine/TransactionCollection.h"
 #include "Transaction/Methods.h"
 #include "VocBase/AccessMode.h"
 #include "VocBase/voc-types.h"
@@ -63,9 +64,7 @@ class SingleCollectionTransaction final : public transaction::Methods {
 #endif
   /// @brief add a collection to the transaction for read, at runtime
   /// note that this can only be ourselves
-  TRI_voc_cid_t addCollectionAtRuntime(std::string const&) override final {
+  TRI_voc_cid_t addCollectionAtRuntime(std::string const& name, AccessMode::Type type) override final;
-    return _cid;
-  }
 
   /// @brief get the underlying collection's name
   std::string name();

js/server/modules/@arangodb/foxx/manager.js

@@ -167,7 +167,7 @@ function selfHeal () {
   const serviceCollection = utils.getStorage();
   const bundleCollection = utils.getBundleStorage();
   const serviceDefinitions = db._query(aql`
-    FOR doc IN ${serviceCollection}
+    WITH ${bundleCollection} FOR doc IN ${serviceCollection}
     FILTER LEFT(doc.mount, 2) != "/_"
     LET bundleExists = DOCUMENT(${bundleCollection}, doc.checksum) != null
     RETURN [doc.mount, doc.checksum, doc._rev, bundleExists]

tests/IResearch/IResearchAnalyzerFeature-test.cpp

@@ -3268,7 +3268,7 @@ TEST_F(IResearchAnalyzerFeatureTest, test_upgrade_static_legacy) {
           LEGACY_ANALYZER_COLLECTION_NAME, arangodb::AccessMode::Type::WRITE);
       EXPECT_TRUE((true == trx.begin().ok()));
       EXPECT_TRUE(
-          (true == trx.insert(arangodb::tests::AnalyzerCollectionName,
+          (true == trx.insert(LEGACY_ANALYZER_COLLECTION_NAME,
                               VPackParser::fromJson("{\"name\": \"legacy\"}")->slice(), options)
                        .ok()));
       EXPECT_TRUE((trx.commit().ok()));

tests/js/client/shell/shell-graph-transaction.js

@@ -136,7 +136,7 @@ function JSTransactionGraphSuite () {
       db[vertex].insert({ _key: "test", value: "test" });
       
       db._executeTransaction({
-        collections: { write: vertex },
+        collections: { write: [vertex, edge] },
         action: function(params) {
           let graph = require("@arangodb/general-graph")._graph(params.graph); 
           let ERRORS = require("@arangodb").errors;

tests/js/client/shell/shell-transaction.js

@@ -1336,8 +1336,6 @@ function transactionCollectionsSuite () {
         }
 
         assertEqual(10, tc1.count());
-        assertEqual(10, tc2.count());
-
       } catch(err) {
         fail();
       } finally {