diff --git a/arangod/Aql/GraphNode.cpp b/arangod/Aql/GraphNode.cpp index f971de06aa..64a8d1d208 100644 --- a/arangod/Aql/GraphNode.cpp +++ b/arangod/Aql/GraphNode.cpp @@ -149,7 +149,9 @@ GraphNode::GraphNode(ExecutionPlan* plan, size_t id, TRI_vocbase_t* vocbase, } seenCollections.emplace(eColName, dir); - if (resolver->getCollectionTypeCluster(eColName) != TRI_COL_TYPE_EDGE) { + auto collection = resolver->getCollection(eColName); + + if (!collection || collection->type() != TRI_COL_TYPE_EDGE) { std::string msg("collection type invalid for collection '" + std::string(eColName) + ": expecting collection type 'edge'"); diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 2dae89e92f..95a02b6391 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -4656,7 +4656,7 @@ static bool applyFulltextOptimization(EnumerateListNode* elnode, coll = colls->add(name, AccessMode::Type::READ); if (!ServerState::instance()->isCoordinator()) { TRI_ASSERT(coll != nullptr); - coll->setCollection(vocbase->lookupCollection(name)); + coll->setCollection(vocbase->lookupCollection(name).get()); // FIXME: does this need to happen in the coordinator? plan->getAst()->query()->trx()->addCollectionAtRuntime(name); } diff --git a/arangod/IResearch/IResearchAnalyzerFeature.cpp b/arangod/IResearch/IResearchAnalyzerFeature.cpp index 20125cc332..4287efaacd 100644 --- a/arangod/IResearch/IResearchAnalyzerFeature.cpp +++ b/arangod/IResearch/IResearchAnalyzerFeature.cpp @@ -943,7 +943,7 @@ void IResearchAnalyzerFeature::start() { LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH) << "failure to get system database while starting feature 'IResearchAnalyzer'"; // assume configuration collection exists } else { - auto* collection = vocbase->lookupCollection(ANALYZER_COLLECTION_NAME); + auto collection = vocbase->lookupCollection(ANALYZER_COLLECTION_NAME); if (!collection) { auto* engine = arangodb::EngineSelectorFeature::ENGINE; diff --git a/arangod/IResearch/IResearchRocksDBRecoveryHelper.cpp b/arangod/IResearch/IResearchRocksDBRecoveryHelper.cpp index 7e254cd7bc..1a33b3b150 100644 --- a/arangod/IResearch/IResearchRocksDBRecoveryHelper.cpp +++ b/arangod/IResearch/IResearchRocksDBRecoveryHelper.cpp @@ -58,7 +58,7 @@ std::pair lookupDatabaseAndCollect return std::make_pair(nullptr, nullptr); } - return std::make_pair(vocbase, vocbase->lookupCollection(pair.second)); + return std::make_pair(vocbase, vocbase->lookupCollection(pair.second).get()); } std::vector> lookupLinks( @@ -82,7 +82,7 @@ arangodb::iresearch::IResearchLink* lookupLink( TRI_voc_cid_t cid, TRI_idx_iid_t iid ) { - auto* col = vocbase.lookupCollection(cid); + auto col = vocbase.lookupCollection(cid); if (!col) { // invalid cid @@ -166,7 +166,7 @@ void ensureLink( return; } - auto* col = vocbase->lookupCollection(cid); + auto col = vocbase->lookupCollection(cid); if (!col) { // if the underlying collection gone, we can go on @@ -246,7 +246,7 @@ void dropCollectionFromView( auto* vocbase = db.useDatabase(dbId); if (vocbase) { - auto* logicalCollection = vocbase->lookupCollection(collectionId); + auto logicalCollection = vocbase->lookupCollection(collectionId); if (!logicalCollection) { LOG_TOPIC(TRACE, arangodb::iresearch::IResearchFeature::IRESEARCH) diff --git a/arangod/IResearch/IResearchView.cpp b/arangod/IResearch/IResearchView.cpp index b3601e5bf0..7fec2dfbdf 100644 --- a/arangod/IResearch/IResearchView.cpp +++ b/arangod/IResearch/IResearchView.cpp @@ -468,7 +468,7 @@ arangodb::Result updateLinks( } struct State { - arangodb::LogicalCollection* _collection = nullptr; + std::shared_ptr _collection; size_t _collectionsToLockOffset; // std::numeric_limits::max() == removal only arangodb::iresearch::IResearchLink const* _link = nullptr; size_t _linkDefinitionsOffset; @@ -554,12 +554,12 @@ arangodb::Result updateLinks( // ); } - auto* resolver = trx.resolver(); + auto* vocbase = trx.vocbase(); - if (!resolver) { + if (!vocbase) { return arangodb::Result( TRI_ERROR_INTERNAL, - std::string("failed to get resolver from transaction while updating while iResearch view '") + std::to_string(view.id()) + "'" + std::string("failed to get vocbase from transaction while updating while IResearch view '") + std::to_string(view.id()) + "'" ); } @@ -572,7 +572,7 @@ arangodb::Result updateLinks( auto& state = *itr; auto& collectionName = collectionsToLock[state._collectionsToLockOffset]; - state._collection = const_cast(resolver->getCollectionStruct(collectionName)); + state._collection = vocbase->lookupCollection(collectionName); if (!state._collection) { // remove modification state if removal of non-existant link on non-existant collection @@ -708,7 +708,7 @@ void validateLinks( arangodb::iresearch::IResearchView const& view ) { for (auto itr = collections.begin(), end = collections.end(); itr != end;) { - auto* collection = vocbase.lookupCollection(*itr); + auto collection = vocbase.lookupCollection(*itr); if (!collection || !findFirstMatchingLink(*collection, view)) { itr = collections.erase(itr); @@ -2012,7 +2012,7 @@ arangodb::Result IResearchView::updateProperties( } for (auto& cid: meta._collections) { - auto* collection = vocbase->lookupCollection(cid); + auto collection = vocbase->lookupCollection(cid); if (collection) { _meta._collections.emplace(cid); diff --git a/arangod/MMFiles/MMFilesRestReplicationHandler.cpp b/arangod/MMFiles/MMFilesRestReplicationHandler.cpp index e6940e3216..4c936fdd36 100644 --- a/arangod/MMFiles/MMFilesRestReplicationHandler.cpp +++ b/arangod/MMFiles/MMFilesRestReplicationHandler.cpp @@ -372,7 +372,7 @@ void MMFilesRestReplicationHandler::handleCommandLoggerFollow() { std::string const& value6 = _request->value("collection", found); if (found) { - arangodb::LogicalCollection* c = _vocbase->lookupCollection(value6); + auto c = _vocbase->lookupCollection(value6); if (c == nullptr) { generateError(rest::ResponseCode::NOT_FOUND, @@ -661,7 +661,7 @@ void MMFilesRestReplicationHandler::handleCommandCreateKeys() { tickEnd = static_cast(StringUtils::uint64(value)); } - arangodb::LogicalCollection* c = _vocbase->lookupCollection(collection); + auto c = _vocbase->lookupCollection(collection); if (c == nullptr) { generateError(rest::ResponseCode::NOT_FOUND, @@ -990,7 +990,8 @@ void MMFilesRestReplicationHandler::handleCommandDump() { withTicks = StringUtils::boolean(value7); } - LogicalCollection* c = _vocbase->lookupCollection(collection); + auto c = _vocbase->lookupCollection(collection); + if (c == nullptr) { generateError(rest::ResponseCode::NOT_FOUND, TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND); diff --git a/arangod/MMFiles/MMFilesTransactionCollection.cpp b/arangod/MMFiles/MMFilesTransactionCollection.cpp index 873055cebd..cb4ea22128 100644 --- a/arangod/MMFiles/MMFilesTransactionCollection.cpp +++ b/arangod/MMFiles/MMFilesTransactionCollection.cpp @@ -213,7 +213,7 @@ int MMFilesTransactionCollection::use(int nestingLevel) { _collection = _transaction->vocbase()->useCollection(_cid, status); } else { // use without usage-lock (lock already set externally) - _collection = _transaction->vocbase()->lookupCollection(_cid); + _collection = _transaction->vocbase()->lookupCollection(_cid).get(); if (_collection == nullptr) { return TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND; diff --git a/arangod/MMFiles/MMFilesV8Functions.cpp b/arangod/MMFiles/MMFilesV8Functions.cpp index 278559e274..09a485f515 100644 --- a/arangod/MMFiles/MMFilesV8Functions.cpp +++ b/arangod/MMFiles/MMFilesV8Functions.cpp @@ -472,8 +472,7 @@ static void JS_WaitCollectorWal( } std::string const name = TRI_ObjectToString(args[0]); - - arangodb::LogicalCollection* col = vocbase->lookupCollection(name); + auto col = vocbase->lookupCollection(name); if (col == nullptr) { TRI_V8_THROW_EXCEPTION(TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND); diff --git a/arangod/MMFiles/MMFilesWalRecoverState.cpp b/arangod/MMFiles/MMFilesWalRecoverState.cpp index cfe9a36e28..aa75d8e1af 100644 --- a/arangod/MMFiles/MMFilesWalRecoverState.cpp +++ b/arangod/MMFiles/MMFilesWalRecoverState.cpp @@ -676,7 +676,7 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker, state->releaseCollection(collectionId); if (collection == nullptr) { - collection = vocbase->lookupCollection(collectionId); + collection = vocbase->lookupCollection(collectionId).get(); } if (collection == nullptr) { @@ -697,7 +697,7 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker, std::string name = nameSlice.copyString(); // check if other collection exist with target name - arangodb::LogicalCollection* other = vocbase->lookupCollection(name); + auto other = vocbase->lookupCollection(name); if (other != nullptr) { if (other->cid() == collection->cid()) { @@ -708,7 +708,7 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker, } else { TRI_voc_cid_t otherCid = other->cid(); state->releaseCollection(otherCid); - vocbase->dropCollection(other, true, -1.0); + vocbase->dropCollection(other.get(), true, -1.0); } } @@ -963,8 +963,7 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker, return true; } - arangodb::LogicalCollection* col = - vocbase->lookupCollection(collectionId); + auto col = vocbase->lookupCollection(collectionId); if (col == nullptr) { // if the underlying collection gone, we can go on @@ -1053,7 +1052,7 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker, state->releaseCollection(collectionId); if (collection == nullptr) { - collection = vocbase->lookupCollection(collectionId); + collection = vocbase->lookupCollection(collectionId).get(); } if (collection != nullptr) { @@ -1071,7 +1070,7 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker, if (nameSlice.isString()) { name = nameSlice.copyString(); - collection = vocbase->lookupCollection(name); + collection = vocbase->lookupCollection(name).get(); if (collection != nullptr) { TRI_voc_cid_t otherCid = collection->cid(); @@ -1345,8 +1344,7 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker, return true; } - arangodb::LogicalCollection* col = - vocbase->lookupCollection(collectionId); + auto col = vocbase->lookupCollection(collectionId); if (col == nullptr) { // if the underlying collection gone, we can go on @@ -1396,7 +1394,7 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker, state->releaseCollection(collectionId); if (collection == nullptr) { - collection = vocbase->lookupCollection(collectionId); + collection = vocbase->lookupCollection(collectionId).get(); } if (collection != nullptr) { diff --git a/arangod/Pregel/Conductor.cpp b/arangod/Pregel/Conductor.cpp index 692fe8132f..7681f83a53 100644 --- a/arangod/Pregel/Conductor.cpp +++ b/arangod/Pregel/Conductor.cpp @@ -478,7 +478,8 @@ static void resolveInfo( std::vector& allShards) { ServerState* ss = ServerState::instance(); if (!ss->isRunningInCluster()) { // single server mode - LogicalCollection* lc = vocbase->lookupCollection(collectionID); + auto lc = vocbase->lookupCollection(collectionID); + if (lc == nullptr || lc->deleted()) { THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND, collectionID); diff --git a/arangod/Replication/DatabaseInitialSyncer.cpp b/arangod/Replication/DatabaseInitialSyncer.cpp index 88a69d55b0..f54131bc4a 100644 --- a/arangod/Replication/DatabaseInitialSyncer.cpp +++ b/arangod/Replication/DatabaseInitialSyncer.cpp @@ -802,7 +802,7 @@ Result DatabaseInitialSyncer::handleCollection(VPackSlice const& parameters, if (col == nullptr) { // not found... - col = vocbase()->lookupCollection(masterName); + col = vocbase()->lookupCollection(masterName).get(); if (col != nullptr && (col->name() != masterName || diff --git a/arangod/Replication/Syncer.cpp b/arangod/Replication/Syncer.cpp index c95af78494..274cc54e1b 100644 --- a/arangod/Replication/Syncer.cpp +++ b/arangod/Replication/Syncer.cpp @@ -311,15 +311,14 @@ std::string Syncer::getCName(VPackSlice const& slice) const { LogicalCollection* Syncer::getCollectionByIdOrName(TRI_vocbase_t* vocbase, TRI_voc_cid_t cid, std::string const& name) { - - arangodb::LogicalCollection* idCol = vocbase->lookupCollection(cid); + auto* idCol = vocbase->lookupCollection(cid).get(); arangodb::LogicalCollection* nameCol = nullptr; if (!name.empty()) { // try looking up the collection by name then - nameCol = vocbase->lookupCollection(name); + nameCol = vocbase->lookupCollection(name).get(); } - + if (idCol != nullptr && nameCol != nullptr) { if (idCol->cid() == nameCol->cid()) { // found collection by id and name, and both are identical! @@ -331,16 +330,16 @@ LogicalCollection* Syncer::getCollectionByIdOrName(TRI_vocbase_t* vocbase, // system collection. always return collection by name when in doubt return nameCol; } - + // no system collection. still prefer local collection return nameCol; } - + if (nameCol != nullptr) { TRI_ASSERT(idCol == nullptr); return nameCol; } - + // may be nullptr return idCol; } @@ -385,13 +384,14 @@ arangodb::LogicalCollection* Syncer::resolveCollection(TRI_vocbase_t* vocbase, TRI_voc_cid_t cid = getCid(slice); if (!simulate32Client() || cid == 0) { VPackSlice uuid; + if ((uuid = slice.get("cuid")).isString()) { - return vocbase->lookupCollectionByUuid(uuid.copyString()); + return vocbase->lookupCollectionByUuid(uuid.copyString()).get(); } else if ((uuid = slice.get("globallyUniqueId")).isString()) { - return vocbase->lookupCollectionByUuid(uuid.copyString()); + return vocbase->lookupCollectionByUuid(uuid.copyString()).get(); } } - + if (cid == 0) { LOG_TOPIC(ERR, Logger::REPLICATION) << TRI_errno_string(TRI_ERROR_REPLICATION_INVALID_RESPONSE); @@ -559,23 +559,22 @@ Result Syncer::createCollection(TRI_vocbase_t* vocbase, // collection already exists. TODO: compare attributes return Result(); } - + bool forceRemoveCid = false; if (col != nullptr && simulate32Client()) { forceRemoveCid = true; LOG_TOPIC(TRACE, Logger::REPLICATION) << "would have got a wrong collection!"; // go on now and truncate or drop/re-create the collection } - + // conflicting collections need to be dropped from 3.3 onwards - col = vocbase->lookupCollection(name); + col = vocbase->lookupCollection(name).get(); + if (col != nullptr) { if (col->isSystem()) { TRI_ASSERT(!simulate32Client() || col->globallyUniqueId() == col->name()); - SingleCollectionTransaction trx(transaction::StandaloneContext::Create(vocbase), col->cid(), AccessMode::Type::WRITE); - Result res = trx.begin(); if (!res.ok()) { return res; @@ -590,7 +589,7 @@ Result Syncer::createCollection(TRI_vocbase_t* vocbase, vocbase->dropCollection(col, false, -1.0); } } - + VPackSlice uuid = slice.get("globallyUniqueId"); // merge in "isSystem" attribute, doesn't matter if name does not start with '_' VPackBuilder s; diff --git a/arangod/Replication/TailingSyncer.cpp b/arangod/Replication/TailingSyncer.cpp index aabf05a08c..d7b6a0452f 100644 --- a/arangod/Replication/TailingSyncer.cpp +++ b/arangod/Replication/TailingSyncer.cpp @@ -559,7 +559,9 @@ Result TailingSyncer::renameCollection(VPackSlice const& slice) { return Result(TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND, "unknown cuid"); } } else if (collection.hasKey("oldName")) { - col = vocbase->lookupCollection(collection.get("oldName").copyString()); + col = + vocbase->lookupCollection(collection.get("oldName").copyString()).get(); + if (col == nullptr) { return Result(TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND, "unknown old collection name"); } diff --git a/arangod/RestHandler/RestEdgesHandler.cpp b/arangod/RestHandler/RestEdgesHandler.cpp index e288a373f6..133f6d3892 100644 --- a/arangod/RestHandler/RestEdgesHandler.cpp +++ b/arangod/RestHandler/RestEdgesHandler.cpp @@ -150,7 +150,9 @@ bool RestEdgesHandler::parseDirection(TRI_edge_direction_e& direction) { bool RestEdgesHandler::validateCollection(std::string const& name) { CollectionNameResolver resolver(_vocbase); - TRI_col_type_e colType = resolver.getCollectionTypeCluster(name); + auto collection = resolver.getCollection(name); + auto colType = collection ? collection->type() : TRI_COL_TYPE_UNKNOWN; + if (colType == TRI_COL_TYPE_UNKNOWN) { generateError(rest::ResponseCode::NOT_FOUND, TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND); diff --git a/arangod/RestHandler/RestIndexHandler.cpp b/arangod/RestHandler/RestIndexHandler.cpp index d0ec1bbe6a..302a577b29 100644 --- a/arangod/RestHandler/RestIndexHandler.cpp +++ b/arangod/RestHandler/RestIndexHandler.cpp @@ -67,9 +67,10 @@ LogicalCollection* RestIndexHandler::collection( } catch (...) { } } else { - return _vocbase->lookupCollection(cName); + return _vocbase->lookupCollection(cName).get(); } } + return nullptr; } diff --git a/arangod/RestHandler/RestReplicationHandler.cpp b/arangod/RestHandler/RestReplicationHandler.cpp index 17cb49038d..30efc26a00 100644 --- a/arangod/RestHandler/RestReplicationHandler.cpp +++ b/arangod/RestHandler/RestReplicationHandler.cpp @@ -887,7 +887,7 @@ Result RestReplicationHandler::processRestoreCollection( } grantTemporaryRights(); - LogicalCollection* col = _vocbase->lookupCollection(name); + auto* col = _vocbase->lookupCollection(name).get(); // drop an existing collection if it exists if (col != nullptr) { @@ -2422,13 +2422,14 @@ int RestReplicationHandler::createCollection(VPackSlice slice, TRI_col_type_e const type = static_cast( arangodb::basics::VelocyPackHelper::getNumericValue( slice, "type", int(TRI_COL_TYPE_DOCUMENT))); - arangodb::LogicalCollection* col = nullptr; + if (!uuid.empty()) { - col = _vocbase->lookupCollectionByUuid(uuid); + col = _vocbase->lookupCollectionByUuid(uuid).get(); } + if (col != nullptr) { - col = _vocbase->lookupCollection(name); + col = _vocbase->lookupCollection(name).get(); } if (col != nullptr && col->type() == type) { diff --git a/arangod/RestHandler/RestSimpleHandler.cpp b/arangod/RestHandler/RestSimpleHandler.cpp index e9a7b294de..4af2f9f000 100644 --- a/arangod/RestHandler/RestSimpleHandler.cpp +++ b/arangod/RestHandler/RestSimpleHandler.cpp @@ -301,7 +301,7 @@ void RestSimpleHandler::lookupByKeys(VPackSlice const& slice) { collectionName = value.copyString(); if (!collectionName.empty()) { - auto const* col = _vocbase->lookupCollection(collectionName); + auto col = _vocbase->lookupCollection(collectionName); if (col != nullptr && collectionName != col->name()) { // user has probably passed in a numeric collection id. diff --git a/arangod/RestHandler/RestSimpleQueryHandler.cpp b/arangod/RestHandler/RestSimpleQueryHandler.cpp index c29c44c60b..736ca81267 100644 --- a/arangod/RestHandler/RestSimpleQueryHandler.cpp +++ b/arangod/RestHandler/RestSimpleQueryHandler.cpp @@ -102,8 +102,8 @@ void RestSimpleQueryHandler::allDocuments() { "expecting string for "); return; } - - auto const* col = _vocbase->lookupCollection(collectionName); + + auto col = _vocbase->lookupCollection(collectionName); if (col != nullptr && collectionName != col->name()) { // user has probably passed in a numeric collection id. @@ -191,14 +191,14 @@ void RestSimpleQueryHandler::allDocumentKeys() { } else { collectionName = _request->value("collection"); } - + if (collectionName.empty()) { generateError(rest::ResponseCode::BAD, TRI_ERROR_TYPE_ERROR, "expecting string for "); return; } - - auto const* col = _vocbase->lookupCollection(collectionName); + + auto col = _vocbase->lookupCollection(collectionName); if (col != nullptr && collectionName != col->name()) { // user has probably passed in a numeric collection id. @@ -286,13 +286,13 @@ void RestSimpleQueryHandler::byExample() { generateError(ResponseCode::BAD, TRI_ERROR_BAD_PARAMETER); return; } - + // velocypack will throw an exception for negative numbers size_t skip = basics::VelocyPackHelper::getNumericValue(body, "skip", 0); size_t limit = basics::VelocyPackHelper::getNumericValue(body, "limit", 0); size_t batchSize = basics::VelocyPackHelper::getNumericValue(body, "batchSize", 0); VPackSlice example = body.get("example"); - + std::string cname; if (body.hasKey("collection")) { VPackSlice const value = body.get("collection"); @@ -302,30 +302,32 @@ void RestSimpleQueryHandler::byExample() { } else { cname = _request->value("collection"); } - + if (cname.empty()) { generateError(rest::ResponseCode::BAD, TRI_ERROR_TYPE_ERROR, "expecting string for "); return; } - - auto const* col = _vocbase->lookupCollection(cname); + + auto col = _vocbase->lookupCollection(cname); + if (col != nullptr && cname != col->name()) { // user has probably passed in a numeric collection id. // translate it into a "real" collection name cname = col->name(); } - + VPackBuilder data; data.openObject(); buildExampleQuery(data, cname, example, skip, limit); - + if (batchSize > 0) { data.add("batchSize", VPackValue(batchSize)); } + data.add("count", VPackSlice::trueSlice()); data.close(); - + // now run the actual query and handle the result processQuery(data.slice()); } diff --git a/arangod/RestHandler/RestWalAccessHandler.cpp b/arangod/RestHandler/RestWalAccessHandler.cpp index d8183b08ea..d8dd2d1a4f 100644 --- a/arangod/RestHandler/RestWalAccessHandler.cpp +++ b/arangod/RestHandler/RestWalAccessHandler.cpp @@ -85,12 +85,14 @@ bool RestWalAccessHandler::parseFilter(WalAccess::Filter& filter) { // extract collection std::string const& value2 = _request->value("collection", found); if (found) { - LogicalCollection* c = _vocbase->lookupCollection(value2); + auto c = _vocbase->lookupCollection(value2); + if (c == nullptr) { generateError(rest::ResponseCode::NOT_FOUND, TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND); return false; } + filter.collection = c->cid(); } } diff --git a/arangod/RocksDBEngine/RocksDBRecoveryManager.cpp b/arangod/RocksDBEngine/RocksDBRecoveryManager.cpp index 599a4335cb..c324bc8337 100644 --- a/arangod/RocksDBEngine/RocksDBRecoveryManager.cpp +++ b/arangod/RocksDBEngine/RocksDBRecoveryManager.cpp @@ -218,19 +218,20 @@ class WBReader final : public rocksdb::WriteBatch::Handler { if (std::get<0>(triple) == 0 && std::get<1>(triple) == 0) { return nullptr; } - + DatabaseFeature* df = DatabaseFeature::DATABASE; TRI_vocbase_t* vb = df->useDatabase(std::get<0>(triple)); if (vb == nullptr) { return nullptr; } TRI_DEFER(vb->release()); - - LogicalCollection* coll = vb->lookupCollection(std::get<1>(triple)); + + auto coll = vb->lookupCollection(std::get<1>(triple)); + if (coll == nullptr) { return nullptr; } - + std::shared_ptr index = coll->lookupIndex(std::get<2>(triple)); if (index == nullptr) { return nullptr; diff --git a/arangod/RocksDBEngine/RocksDBReplicationContext.cpp b/arangod/RocksDBEngine/RocksDBReplicationContext.cpp index 36aa8e8faa..5005318876 100644 --- a/arangod/RocksDBEngine/RocksDBReplicationContext.cpp +++ b/arangod/RocksDBEngine/RocksDBReplicationContext.cpp @@ -129,7 +129,8 @@ int RocksDBReplicationContext::bindCollection( ((_collection->name() != collectionIdentifier) && std::to_string(_collection->cid()) != collectionIdentifier && _collection->globallyUniqueId() != collectionIdentifier)) { - _collection = _trx->vocbase()->lookupCollection(collectionIdentifier); + _collection = _trx->vocbase()->lookupCollection(collectionIdentifier).get(); + if (_collection == nullptr) { return TRI_ERROR_BAD_PARAMETER; } diff --git a/arangod/RocksDBEngine/RocksDBReplicationTailing.cpp b/arangod/RocksDBEngine/RocksDBReplicationTailing.cpp index 5c45e5aa2e..ca67bb86fe 100644 --- a/arangod/RocksDBEngine/RocksDBReplicationTailing.cpp +++ b/arangod/RocksDBEngine/RocksDBReplicationTailing.cpp @@ -547,7 +547,7 @@ class WALParser : public rocksdb::WriteBatch::Handler { } return false; } - + LogicalCollection* loadCollection(TRI_voc_cid_t cid) { TRI_ASSERT(cid != 0); if (_vocbase != nullptr) { @@ -555,12 +555,15 @@ class WALParser : public rocksdb::WriteBatch::Handler { if (it != _collectionCache.end()) { return it->second.collection(); } - LogicalCollection* collection = _vocbase->lookupCollection(cid); + + auto* collection = _vocbase->lookupCollection(cid).get(); + if (collection != nullptr) { _collectionCache.emplace(cid, CollectionGuard(_vocbase, collection)); return collection; } } + return nullptr; } diff --git a/arangod/RocksDBEngine/RocksDBRestReplicationHandler.cpp b/arangod/RocksDBEngine/RocksDBRestReplicationHandler.cpp index da65684614..60280c9b60 100644 --- a/arangod/RocksDBEngine/RocksDBRestReplicationHandler.cpp +++ b/arangod/RocksDBEngine/RocksDBRestReplicationHandler.cpp @@ -258,7 +258,7 @@ void RocksDBRestReplicationHandler::handleCommandLoggerFollow() { TRI_voc_cid_t cid = 0; std::string const& value6 = _request->value("collection", found); if (found) { - arangodb::LogicalCollection* c = _vocbase->lookupCollection(value6); + auto c = _vocbase->lookupCollection(value6); if (c == nullptr) { generateError(rest::ResponseCode::NOT_FOUND, diff --git a/arangod/RocksDBEngine/RocksDBTransactionCollection.cpp b/arangod/RocksDBEngine/RocksDBTransactionCollection.cpp index 5eb7149ab0..9eea742eac 100644 --- a/arangod/RocksDBEngine/RocksDBTransactionCollection.cpp +++ b/arangod/RocksDBEngine/RocksDBTransactionCollection.cpp @@ -194,7 +194,7 @@ int RocksDBTransactionCollection::use(int nestingLevel) { } } else { // use without usage-lock (lock already set externally) - _collection = _transaction->vocbase()->lookupCollection(_cid); + _collection = _transaction->vocbase()->lookupCollection(_cid).get(); if (_collection == nullptr) { return TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND; diff --git a/arangod/Transaction/Methods.cpp b/arangod/Transaction/Methods.cpp index a8c06ac1b5..863712c003 100644 --- a/arangod/Transaction/Methods.cpp +++ b/arangod/Transaction/Methods.cpp @@ -972,10 +972,9 @@ bool transaction::Methods::isDocumentCollection( /// @brief return the type of a collection TRI_col_type_e transaction::Methods::getCollectionType( std::string const& collectionName) const { - if (_state->isCoordinator()) { - return resolver()->getCollectionTypeCluster(collectionName); - } - return resolver()->getCollectionType(collectionName); + auto collection = resolver()->getCollection(collectionName); + + return collection ? collection->type() : TRI_COL_TYPE_UNKNOWN; } /// @brief return the name of a collection @@ -2884,46 +2883,30 @@ Result transaction::Methods::addCollection(TRI_voc_cid_t cid, char const* name, Result res; bool visited = false; auto visitor = _state->isEmbeddedTransaction() - ? std::function( - [this, name, type, &res, cid, &visited](TRI_voc_cid_t ccid)->bool { - res = addCollectionEmbedded(ccid, name, type); + ? std::function( + [this, name, type, &res, cid, &visited](LogicalCollection& col)->bool { + res = addCollectionEmbedded(col.id(), name, type); if (!res.ok()) { return false; // break on error } - auto dataSource = resolver()->getDataSource(ccid); - - if (!dataSource) { - res = TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND; - - return false; // break on error - } - - res = applyStateRegistrationCallbacks(*dataSource, *_state); - visited |= cid == ccid; + res = applyStateRegistrationCallbacks(col, *_state); + visited |= cid == col.id(); return res.ok(); // add the remaining collections (or break on error) } ) - : std::function( - [this, name, type, &res, cid, &visited](TRI_voc_cid_t ccid)->bool { - res = addCollectionToplevel(ccid, name, type); + : std::function( + [this, name, type, &res, cid, &visited](LogicalCollection& col)->bool { + res = addCollectionToplevel(col.id(), name, type); if (!res.ok()) { return false; // break on error } - auto dataSource = resolver()->getDataSource(ccid); - - if (!dataSource) { - res = TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND; - - return false; // break on error - } - - res = applyStateRegistrationCallbacks(*dataSource, *_state); - visited |= cid == ccid; + res = applyStateRegistrationCallbacks(col, *_state); + visited |= cid == col.id(); return res.ok(); // add the remaining collections (or break on error) } @@ -2931,6 +2914,14 @@ Result transaction::Methods::addCollection(TRI_voc_cid_t cid, char const* name, ; if (!resolver()->visitCollections(visitor, cid) || !res.ok()) { + // trigger exception as per the original behaviour (tests depend on this) + if (res.ok() && !visited) { + res = _state->isEmbeddedTransaction() + ? addCollectionEmbedded(cid, name, type) + : addCollectionToplevel(cid, name, type) + ; + } + return res.ok() ? Result(TRI_ERROR_INTERNAL) : res; // return first error } @@ -2941,11 +2932,10 @@ Result transaction::Methods::addCollection(TRI_voc_cid_t cid, char const* name, auto dataSource = resolver()->getDataSource(cid); - if (!dataSource) { - return TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND; - } - - return applyStateRegistrationCallbacks(*dataSource, *_state); + return dataSource + ? applyStateRegistrationCallbacks(*dataSource, *_state) + : Result(TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND) + ; } /// @brief add a collection by id, with the name supplied @@ -3221,4 +3211,4 @@ Result transaction::Methods::resolveId(char const* handle, size_t length, // ----------------------------------------------------------------------------- // --SECTION-- END-OF-FILE -// ----------------------------------------------------------------------------- +// ----------------------------------------------------------------------------- \ No newline at end of file diff --git a/arangod/Utils/CollectionNameResolver.cpp b/arangod/Utils/CollectionNameResolver.cpp index fec40f3655..b911b7d5b9 100644 --- a/arangod/Utils/CollectionNameResolver.cpp +++ b/arangod/Utils/CollectionNameResolver.cpp @@ -32,7 +32,33 @@ #include "VocBase/LogicalView.h" #include "VocBase/vocbase.h" -using namespace arangodb; +namespace arangodb { + +std::shared_ptr CollectionNameResolver::getCollection( + TRI_voc_cid_t id +) const noexcept { + #ifdef ARANGODB_ENABLE_MAINTAINER_MODE + return std::dynamic_pointer_cast(getDataSource(id)); + #else + auto dataSource = getDataSource(id); + + return dataSource.category() == LogicalCollection::category() + ? std::static_pointer_cast(dataSource) : nullptr; + #endif +} + +std::shared_ptr CollectionNameResolver::getCollection( + std::string const& nameOrId +) const noexcept { + #ifdef ARANGODB_ENABLE_MAINTAINER_MODE + return std::dynamic_pointer_cast(getDataSource(nameOrId)); + #else + auto dataSource = getDataSource(nameOrId); + + return dataSource.category() == LogicalCollection::category() + ? std::static_pointer_cast(dataSource) : nullptr; + #endif +} ////////////////////////////////////////////////////////////////////////////// /// @brief look up a collection id for a collection name (local case), @@ -50,7 +76,7 @@ TRI_voc_cid_t CollectionNameResolver::getCollectionIdLocal( arangodb::LogicalCollection const* collection = getCollectionStruct(name); if (collection != nullptr) { - return collection->cid(); + return collection->id(); } auto view = _vocbase->lookupView(name); @@ -77,8 +103,10 @@ TRI_voc_cid_t CollectionNameResolver::getCollectionIdCluster( if (name[0] >= '0' && name[0] <= '9') { // name is a numeric id TRI_voc_cid_t cid = NumberUtils::atoi_zero(name.data(), name.data() + name.size()); + auto collection = getCollection(cid); // Now validate the cid - TRI_col_type_e type = getCollectionTypeCluster(getCollectionNameCluster(cid)); + auto type = collection ? collection->type() : TRI_COL_TYPE_UNKNOWN; + if (type == TRI_COL_TYPE_UNKNOWN) { return 0; } @@ -91,7 +119,7 @@ TRI_voc_cid_t CollectionNameResolver::getCollectionIdCluster( auto cinfo = ci->getCollection(_vocbase->name(), name); if (cinfo) { - return cinfo->cid(); + return cinfo->id(); } auto vinfo = ci->getView(_vocbase->name(), name); @@ -121,25 +149,6 @@ TRI_voc_cid_t CollectionNameResolver::getCollectionId( return getCollectionIdCluster(name); } -////////////////////////////////////////////////////////////////////////////// -/// @brief look up a collection type for a collection name (local case) -////////////////////////////////////////////////////////////////////////////// - -TRI_col_type_e CollectionNameResolver::getCollectionType( - std::string const& name) const { - if (name[0] >= '0' && name[0] <= '9') { - // name is a numeric id - return getCollectionType(getCollectionName(NumberUtils::atoi_zero(name.data(), name.data() + name.size()))); - } - - arangodb::LogicalCollection const* collection = getCollectionStruct(name); - - if (collection != nullptr) { - return collection->type(); - } - return TRI_COL_TYPE_UNKNOWN; -} - ////////////////////////////////////////////////////////////////////////////// /// @brief look up a collection struct for a collection name ////////////////////////////////////////////////////////////////////////////// @@ -152,7 +161,7 @@ arangodb::LogicalCollection const* CollectionNameResolver::getCollectionStruct( return (*it).second; } - arangodb::LogicalCollection const* collection = _vocbase->lookupCollection(name); + auto* collection = _vocbase->lookupCollection(name).get(); if (collection != nullptr) { _resolvedNames.emplace(name, collection); @@ -161,35 +170,6 @@ arangodb::LogicalCollection const* CollectionNameResolver::getCollectionStruct( return collection; } -////////////////////////////////////////////////////////////////////////////// -/// @brief look up a cluster collection type for a cluster collection name on -/// the -/// coordinator and for a shard name on the db server -////////////////////////////////////////////////////////////////////////////// - -TRI_col_type_e CollectionNameResolver::getCollectionTypeCluster( - std::string const& name) const { - // This fires in Single server case as well - if (!ServerState::isCoordinator(_serverRole)) { - return getCollectionType(name); - } - if (name[0] >= '0' && name[0] <= '9') { - // name is a numeric id - return getCollectionTypeCluster( - getCollectionName(NumberUtils::atoi_zero(name.data(), name.data() + name.size()))); - } - - try { - // We have to look up the collection info: - ClusterInfo* ci = ClusterInfo::instance(); - auto cinfo = ci->getCollection(_vocbase->name(), name); - TRI_ASSERT(cinfo != nullptr); - return cinfo->type(); - } catch(...) { - } - return TRI_COL_TYPE_UNKNOWN; -} - ////////////////////////////////////////////////////////////////////////////// /// @brief look up a collection name for a collection id, this implements /// some magic in the cluster case: a DBserver in a cluster will automatically @@ -282,7 +262,7 @@ std::string CollectionNameResolver::localNameLookup(TRI_voc_cid_t cid) const { auto it = _vocbase->_dataSourceById.find(cid); if (it != _vocbase->_dataSourceById.end() - && std::dynamic_pointer_cast(it->second)) { + && LogicalCollection::category() == it->second->category()) { if (it->second->planId() == it->second->id()) { // DBserver local case name = (*it).second->name(); @@ -292,7 +272,11 @@ std::string CollectionNameResolver::localNameLookup(TRI_voc_cid_t cid) const { std::shared_ptr ci; try { ci = ClusterInfo::instance()->getCollection( - std::dynamic_pointer_cast(it->second)->dbName(), + #ifdef ARANGODB_ENABLE_MAINTAINER_MODE + dynamic_cast(it->second.get())->dbName(), + #else + static_cast(it->second.get())->dbName(), + #endif name ); } @@ -319,36 +303,97 @@ std::string CollectionNameResolver::localNameLookup(TRI_voc_cid_t cid) const { std::shared_ptr CollectionNameResolver::getDataSource( TRI_voc_cid_t id ) const noexcept { + auto itr = _dataSourceById.find(id); + + if (itr != _dataSourceById.end()) { + return itr->second; + } + + std::shared_ptr ptr; + // db server / standalone - if (!ServerState::isRunningInCluster(_serverRole) - || ServerState::isDBServer(_serverRole)) { - return _vocbase ? _vocbase->lookupDataSource(id) : nullptr; + if (!ServerState::isCoordinator(_serverRole)) { + ptr = _vocbase ? _vocbase->lookupDataSource(id) : nullptr; + } else { + ptr = getDataSource(std::to_string(id)); } - // cluster coordinator - auto* ci = ClusterInfo::instance(); - - if (!ci) { - return nullptr; + if (ptr) { + _dataSourceById.emplace(id, ptr); } - try { - auto name = std::to_string(id); - auto cinfo = ci->getCollection(_vocbase->name(), name); + return ptr; +} - if (cinfo) { - return std::static_pointer_cast(cinfo); +std::shared_ptr CollectionNameResolver::getDataSource( + std::string const& nameOrId +) const noexcept { + auto itr = _dataSourceByName.find(nameOrId); + + if (itr != _dataSourceByName.end()) { + return itr->second; + } + + std::shared_ptr ptr; + + // db server / standalone + if (!ServerState::isCoordinator(_serverRole)) { + ptr = _vocbase ? _vocbase->lookupDataSource(nameOrId) : nullptr; + } else { + // cluster coordinator + auto* ci = ClusterInfo::instance(); + + if (!ci) { + return nullptr; } - return std::static_pointer_cast( - ci->getView(_vocbase->name(), name) - ); - } catch (...) { - LOG_TOPIC(ERR, arangodb::Logger::FIXME) - << "caught exception while resolving cluster data-source id: " << id; + try { + ptr = std::static_pointer_cast( + ci->getCollection(_vocbase->name(), nameOrId) + ); + + if (!ptr) { + ptr = std::static_pointer_cast( + ci->getView(_vocbase->name(), nameOrId) + ); + } + } catch (...) { + LOG_TOPIC(ERR, arangodb::Logger::FIXME) + << "caught exception while resolving cluster data-source: " << nameOrId; + } } - return nullptr; + if (ptr) { + _dataSourceByName.emplace(nameOrId, ptr); + } + + return ptr; +} + +std::shared_ptr CollectionNameResolver::getView( + TRI_voc_cid_t id +) const noexcept { + #ifdef ARANGODB_ENABLE_MAINTAINER_MODE + return std::dynamic_pointer_cast(getDataSource(id)); + #else + auto dataSource = getDataSource(id); + + return dataSource.category() == LogicalView::category() + ? std::static_pointer_cast(dataSource) : nullptr; + #endif +} + +std::shared_ptr CollectionNameResolver::getView( + std::string const& nameOrId +) const noexcept { + #ifdef ARANGODB_ENABLE_MAINTAINER_MODE + return std::dynamic_pointer_cast(getDataSource(nameOrId)); + #else + auto dataSource = getDataSource(nameOrId); + + return dataSource.category() == LogicalView::category() + ? std::static_pointer_cast(dataSource) : nullptr; + #endif } std::string CollectionNameResolver::getViewNameCluster( @@ -364,29 +409,41 @@ std::string CollectionNameResolver::getViewNameCluster( } bool CollectionNameResolver::visitCollections( - std::function const& visitor, TRI_voc_cid_t cid + std::function const& visitor, + TRI_voc_cid_t id ) const { - if (!_vocbase) { + auto dataSource = getDataSource(id); + + if (!dataSource) { return false; // no way to determine what to visit } - auto* collection = _vocbase->lookupCollection(cid); + if (LogicalCollection::category() == dataSource->category()) { + #ifdef ARANGODB_ENABLE_MAINTAINER_MODE + auto collection = std::dynamic_pointer_cast(dataSource); + TRI_ASSERT(collection); + #else + auto collection = std::static_pointer_cast(dataSource); + #endif - if (collection) { // TODO resolve smart edge collection CIDs here - return visitor(cid); + return visitor(*collection); } - auto view = _vocbase->lookupView(cid); + if (LogicalView::category() == dataSource->category()) { + #ifdef ARANGODB_ENABLE_MAINTAINER_MODE + auto view = std::dynamic_pointer_cast(dataSource); + #else + auto view = std::static_pointer_cast(dataSource); + #endif - if (view) { // each CID in a view might need further resolution - return view->visitCollections([this, &visitor, cid](TRI_voc_cid_t ccid)->bool { - return ccid == cid ? visitor(ccid) : visitCollections(visitor, ccid); + return view->visitCollections([this, &visitor, id](TRI_voc_cid_t cid)->bool { + return cid == id ? false : visitCollections(visitor, cid); // avoid infinite recursion }); } - // no way to determine what to visit - // emulate the original behaviour, assume 'cid' is for a regular collection and visit it as is - return visitor(cid); + return false; // no way to determine what to visit } + +} // namespace arangodb diff --git a/arangod/Utils/CollectionNameResolver.h b/arangod/Utils/CollectionNameResolver.h index 6768c8e247..159686b51c 100644 --- a/arangod/Utils/CollectionNameResolver.h +++ b/arangod/Utils/CollectionNameResolver.h @@ -30,9 +30,15 @@ enum TRI_col_type_e : uint32_t; namespace arangodb { + class LogicalCollection; class LogicalDataSource; +class LogicalView; // forward declaration +//////////////////////////////////////////////////////////////////////////////// +/// @brief data-source id/name resolver and cache (single-server and cluster) +/// @note not thread-safe +//////////////////////////////////////////////////////////////////////////////// class CollectionNameResolver { public: ////////////////////////////////////////////////////////////////////////////// @@ -52,6 +58,22 @@ class CollectionNameResolver { ~CollectionNameResolver() = default; public: + + ////////////////////////////////////////////////////////////////////////////// + /// @brief look up a collection struct for a collection id + ////////////////////////////////////////////////////////////////////////////// + std::shared_ptr getCollection( + TRI_voc_cid_t id + ) const noexcept; + + ////////////////////////////////////////////////////////////////////////////// + /// @brief look up a collection struct for a + /// collection name, stringified id (or uuid for dbserver / standalone) + ////////////////////////////////////////////////////////////////////////////// + std::shared_ptr getCollection( + std::string const& nameOrId + ) const noexcept; + ////////////////////////////////////////////////////////////////////////////// /// @brief look up a collection id for a collection name (local case), /// use this if you know you are on a single server or on a DBserver @@ -78,26 +100,12 @@ class CollectionNameResolver { TRI_voc_cid_t getCollectionId(std::string const& name) const; - ////////////////////////////////////////////////////////////////////////////// - /// @brief look up a collection type for a collection name (local case) - ////////////////////////////////////////////////////////////////////////////// - - TRI_col_type_e getCollectionType(std::string const& name) const; - ////////////////////////////////////////////////////////////////////////////// /// @brief look up a collection struct for a collection name ////////////////////////////////////////////////////////////////////////////// arangodb::LogicalCollection const* getCollectionStruct(std::string const& name) const; - ////////////////////////////////////////////////////////////////////////////// - /// @brief look up a cluster collection type for a cluster collection name on - /// the - /// coordinator and for a shard name on the db server - ////////////////////////////////////////////////////////////////////////////// - - TRI_col_type_e getCollectionTypeCluster(std::string const& name) const; - ////////////////////////////////////////////////////////////////////////////// /// @brief look up a collection name for a collection id, this implements /// some magic in the cluster case: a DBserver in a cluster will automatically @@ -122,12 +130,33 @@ class CollectionNameResolver { std::string getCollectionName(std::string const& nameOrId) const; ////////////////////////////////////////////////////////////////////////////// - /// @brief look up a collection struct for a collection name + /// @brief look up a data-source struct for a data-source id ////////////////////////////////////////////////////////////////////////////// std::shared_ptr getDataSource( TRI_voc_cid_t id ) const noexcept; + ////////////////////////////////////////////////////////////////////////////// + /// @brief look up a data-source struct for a + /// data-source name, stringified id (or uuid for dbserver/standalone) + ////////////////////////////////////////////////////////////////////////////// + std::shared_ptr getDataSource( + std::string const& nameOrId + ) const noexcept; + + ////////////////////////////////////////////////////////////////////////////// + /// @brief look up a view struct for a view id + ////////////////////////////////////////////////////////////////////////////// + std::shared_ptr getView(TRI_voc_cid_t id) const noexcept; + + ////////////////////////////////////////////////////////////////////////////// + /// @brief look up a view struct for a + /// view name, stringified id (or uuid for dbserver / standalone) + ////////////////////////////////////////////////////////////////////////////// + std::shared_ptr getView( + std::string const& nameOrId + ) const noexcept; + ////////////////////////////////////////////////////////////////////////////// /// @brief look up a cluster-wide view name for a cluster-wide view id ////////////////////////////////////////////////////////////////////////////// @@ -135,15 +164,18 @@ class CollectionNameResolver { std::string getViewNameCluster(TRI_voc_cid_t cid) const; ////////////////////////////////////////////////////////////////////////////// - /// @brief invoke visitor on all collections that map to the specified 'cid' + /// @brief invoke visitor on all collections that map to the specified 'id' /// @return visitation was successful ////////////////////////////////////////////////////////////////////////////// bool visitCollections( - std::function const& visitor, TRI_voc_cid_t cid + std::function const& visitor, + TRI_voc_cid_t id ) const; private: + mutable std::unordered_map> _dataSourceById; // cached data-source by id + mutable std::unordered_map> _dataSourceByName; // cached data-source by name std::string localNameLookup(TRI_voc_cid_t cid) const; @@ -154,11 +186,11 @@ class CollectionNameResolver { ////////////////////////////////////////////////////////////////////////////// TRI_vocbase_t* _vocbase; - + ////////////////////////////////////////////////////////////////////////////// /// @brief role of server in cluster ////////////////////////////////////////////////////////////////////////////// - + ServerState::RoleEnum _serverRole; ////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/V8Server/v8-collection.cpp b/arangod/V8Server/v8-collection.cpp index 21bdcc61d0..b167c75bed 100644 --- a/arangod/V8Server/v8-collection.cpp +++ b/arangod/V8Server/v8-collection.cpp @@ -1953,7 +1953,8 @@ static void JS_PregelStart(v8::FunctionCallbackInfo const& args) { TRI_V8_THROW_EXCEPTION_MESSAGE(TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND, name); } } else if (ss->getRole() == ServerState::ROLE_SINGLE) { - LogicalCollection *coll = vocbase->lookupCollection(name); + auto coll = vocbase->lookupCollection(name); + if (coll == nullptr || coll->status() == TRI_VOC_COL_STATUS_DELETED || coll->deleted()) { TRI_V8_THROW_EXCEPTION_MESSAGE(TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND, name); @@ -1992,7 +1993,8 @@ static void JS_PregelStart(v8::FunctionCallbackInfo const& args) { TRI_V8_THROW_EXCEPTION_MESSAGE(TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND, name); } } else if (ss->getRole() == ServerState::ROLE_SINGLE) { - LogicalCollection *coll = vocbase->lookupCollection(name); + auto coll = vocbase->lookupCollection(name); + if (coll == nullptr || coll->deleted()) { TRI_V8_THROW_EXCEPTION_MESSAGE(TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND, name); } @@ -2126,10 +2128,11 @@ static arangodb::LogicalCollection* GetCollectionFromArgument( // number if (val->IsNumber() || val->IsNumberObject()) { uint64_t cid = TRI_ObjectToUInt64(val, true); - return vocbase->lookupCollection(cid); + + return vocbase->lookupCollection(cid).get(); } - return vocbase->lookupCollection(TRI_ObjectToString(val)); + return vocbase->lookupCollection(TRI_ObjectToString(val)).get(); } //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/V8Server/v8-vocbase.cpp b/arangod/V8Server/v8-vocbase.cpp index 88370dff5a..39e8fcb1da 100644 --- a/arangod/V8Server/v8-vocbase.cpp +++ b/arangod/V8Server/v8-vocbase.cpp @@ -1436,7 +1436,7 @@ static void MapGetVocBase(v8::Local const name, auto colCopy = ci->clone(); collection = colCopy.release(); // will be delete on garbage collection } else { - collection = vocbase->lookupCollection(std::string(key)); + collection = vocbase->lookupCollection(std::string(key)).get(); } } catch (...) { // do not propagate exception from here diff --git a/arangod/VocBase/Methods/Collections.cpp b/arangod/VocBase/Methods/Collections.cpp index 9f719a23cb..815f991411 100644 --- a/arangod/VocBase/Methods/Collections.cpp +++ b/arangod/VocBase/Methods/Collections.cpp @@ -110,7 +110,8 @@ Result methods::Collections::lookup(TRI_vocbase_t* vocbase, return Result(TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND); } - LogicalCollection* coll = vocbase->lookupCollection(name); + auto coll = vocbase->lookupCollection(name); + if (coll != nullptr) { // check authentication after ensuring the collection exists if (exec != nullptr && @@ -118,7 +119,7 @@ Result methods::Collections::lookup(TRI_vocbase_t* vocbase, return Result(TRI_ERROR_FORBIDDEN, "No access to collection '" + name + "'"); } try { - func(coll); + func(coll.get()); } catch (basics::Exception const& ex) { return Result(ex.code(), ex.what()); } catch (std::exception const& ex) { diff --git a/arangod/VocBase/vocbase.cpp b/arangod/VocBase/vocbase.cpp index a1669a2ae1..50e8c79db6 100644 --- a/arangod/VocBase/vocbase.cpp +++ b/arangod/VocBase/vocbase.cpp @@ -240,7 +240,7 @@ void TRI_vocbase_t::registerCollection( std::shared_ptr const& collection ) { std::string name = collection->name(); - TRI_voc_cid_t cid = collection->cid(); + TRI_voc_cid_t cid = collection->id(); { RECURSIVE_WRITE_LOCKER_NAMED(writeLocker, _dataSourceLock, _dataSourceLockWriteOwner, doLock); @@ -264,7 +264,7 @@ void TRI_vocbase_t::registerCollection( if (!it2.second) { std::string msg; - msg.append(std::string("duplicate collection identifier ") + std::to_string(collection->cid()) + " for name '" + name + "'"); + msg.append(std::string("duplicate collection identifier ") + std::to_string(collection->id()) + " for name '" + name + "'"); LOG_TOPIC(ERR, arangodb::Logger::FIXME) << msg; THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_ARANGO_DUPLICATE_IDENTIFIER, msg); @@ -507,7 +507,7 @@ std::shared_ptr TRI_vocbase_t::createCollectionWork int TRI_vocbase_t::loadCollection(arangodb::LogicalCollection* collection, TRI_vocbase_col_status_e& status, bool setStatus) { - TRI_ASSERT(collection->cid() != 0); + TRI_ASSERT(collection->id() != 0); // read lock // check if the collection is already loaded @@ -758,7 +758,7 @@ int TRI_vocbase_t::dropCollectionWorker(arangodb::LogicalCollection* collection, if (!collection->deleted()) { collection->deleted(true); try { - engine->changeCollection(this, collection->cid(), collection, doSync); + engine->changeCollection(this, collection->id(), collection, doSync); } catch (arangodb::basics::Exception const& ex) { collection->deleted(false); events::DropCollection(colName, ex.code()); @@ -796,7 +796,7 @@ int TRI_vocbase_t::dropCollectionWorker(arangodb::LogicalCollection* collection, ->forceSyncProperties(); VPackBuilder builder; - engine->getCollectionInfo(this, collection->cid(), builder, false, 0); + engine->getCollectionInfo(this, collection->id(), builder, false, 0); arangodb::Result res = collection->updateProperties( builder.slice().get("parameters"), doSync); @@ -959,7 +959,7 @@ void TRI_vocbase_t::inventory( // In cluster case cids are not created by ticks but by cluster uniqIds if (!ServerState::instance()->isRunningInCluster() && - collection->cid() > maxTick) { + collection->id() > maxTick) { // collection is too new continue; } @@ -974,7 +974,7 @@ void TRI_vocbase_t::inventory( continue; } - if (collection->cid() <= maxTick) { + if (collection->id() <= maxTick) { result.openObject(); result.add(VPackValue("indexes")); @@ -1013,36 +1013,57 @@ std::string TRI_vocbase_t::viewName(TRI_voc_cid_t id) const { return view ? view->name() : StaticStrings::Empty; } -/// @brief looks up a collection by name or stringified cid or uuid -arangodb::LogicalCollection* TRI_vocbase_t::lookupCollection( - std::string const& nameOrId -) const noexcept { - return std::dynamic_pointer_cast( - lookupDataSource(nameOrId) - ).get(); -} - /// @brief looks up a collection by identifier -arangodb::LogicalCollection* TRI_vocbase_t::lookupCollection( +std::shared_ptr TRI_vocbase_t::lookupCollection( TRI_voc_cid_t id ) const noexcept { - return std::dynamic_pointer_cast( - lookupDataSource(id) - ).get(); + #ifdef ARANGODB_ENABLE_MAINTAINER_MODE + return std::dynamic_pointer_cast( + lookupDataSource(id) + ); + #else + auto dataSource = lookupDataSource(id); + + return dataSource.category() == LogicalCollection::category() + ? std::static_pointer_cast(dataSource) : nullptr; + #endif +} + +/// @brief looks up a collection by name or stringified cid or uuid +std::shared_ptr TRI_vocbase_t::lookupCollection( + std::string const& nameOrId +) const noexcept { + #ifdef ARANGODB_ENABLE_MAINTAINER_MODE + return std::dynamic_pointer_cast( + lookupDataSource(nameOrId) + ); + #else + auto dataSource = lookupDataSource(nameOrId); + + return dataSource.category() == LogicalCollection::category() + ? std::static_pointer_cast(dataSource) : nullptr; + #endif } /// @brief looks up a collection by uuid -arangodb::LogicalCollection* TRI_vocbase_t::lookupCollectionByUuid( +std::shared_ptr TRI_vocbase_t::lookupCollectionByUuid( std::string const& uuid ) const noexcept { // otherwise we'll look up the collection by name RECURSIVE_READ_LOCKER(_dataSourceLock, _dataSourceLockWriteOwner); auto itr = _dataSourceByUuid.find(uuid); - return itr == _dataSourceByUuid.end() - ? nullptr - : std::dynamic_pointer_cast(itr->second).get() - ; + #ifdef ARANGODB_ENABLE_MAINTAINER_MODE + return itr == _dataSourceByUuid.end() + ? nullptr + : std::dynamic_pointer_cast(itr->second) + ; + #else + return itr == _dataSourceByUuid.end() + || dataSource.category() != LogicalCollection::category() + ? nullptr + : std::static_pointer_cast(dataSource) + #endif } /// @brief looks up a data-source by identifier @@ -1468,7 +1489,7 @@ arangodb::LogicalCollection* TRI_vocbase_t::useCollection( TRI_voc_cid_t cid, TRI_vocbase_col_status_e& status) { auto collection = lookupCollection(cid); - return useCollectionInternal(collection, status); + return useCollectionInternal(collection.get(), status); } /// @brief locks a collection for usage by name @@ -1496,7 +1517,7 @@ arangodb::LogicalCollection* TRI_vocbase_t::useCollectionByUuid( std::string const& uuid, TRI_vocbase_col_status_e& status) { auto collection = lookupCollectionByUuid(uuid); - return useCollectionInternal(collection, status); + return useCollectionInternal(collection.get(), status); } arangodb::LogicalCollection* TRI_vocbase_t::useCollectionInternal( diff --git a/arangod/VocBase/vocbase.h b/arangod/VocBase/vocbase.h index b792ca9e83..a4b5a2bfec 100644 --- a/arangod/VocBase/vocbase.h +++ b/arangod/VocBase/vocbase.h @@ -279,17 +279,17 @@ struct TRI_vocbase_t { std::function const& nameFilter); /// @brief looks up a collection by identifier - arangodb::LogicalCollection* lookupCollection( + std::shared_ptr lookupCollection( TRI_voc_cid_t id ) const noexcept; /// @brief looks up a collection by name or stringified cid or uuid - arangodb::LogicalCollection* lookupCollection( + std::shared_ptr lookupCollection( std::string const& nameOrId ) const noexcept; /// @brief looks up a collection by uuid - arangodb::LogicalCollection* lookupCollectionByUuid( + std::shared_ptr lookupCollectionByUuid( std::string const& uuid ) const noexcept; @@ -447,4 +447,4 @@ void TRI_SanitizeObject(arangodb::velocypack::Slice const slice, void TRI_SanitizeObjectWithEdges(arangodb::velocypack::Slice const slice, arangodb::velocypack::Builder& builder); -#endif +#endif \ No newline at end of file diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 68c9eb07f4..12bf1ae026 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -48,6 +48,7 @@ if (USE_IRESEARCH) IResearch/ExpressionContextMock.cpp IResearch/VelocyPackHelper-test.cpp IResearch/ExecutionBlockMock-test.cpp + VocBase/LogicalDataSource-test.cpp ) endif () @@ -113,7 +114,6 @@ add_executable( RocksDBEngine/IndexEstimatorTest.cpp RocksDBEngine/TypeConversionTest.cpp SimpleHttpClient/CommunicatorTest.cpp - VocBase/LogicalDataSource-test.cpp IResearch/StorageEngineMock.cpp ${IRESEARCH_TESTS_SOURCES} main.cpp diff --git a/tests/IResearch/IResearchAnalyzerFeature-test.cpp b/tests/IResearch/IResearchAnalyzerFeature-test.cpp index ef9f1bf1d8..4edc67ccee 100644 --- a/tests/IResearch/IResearchAnalyzerFeature-test.cpp +++ b/tests/IResearch/IResearchAnalyzerFeature-test.cpp @@ -532,10 +532,10 @@ SECTION("test_persistence") { // ensure there is an empty configuration collection { - auto* collection = vocbase->lookupCollection("_iresearch_analyzers"); + auto collection = vocbase->lookupCollection("_iresearch_analyzers"); if (collection) { - vocbase->dropCollection(collection, true, -1); + vocbase->dropCollection(collection.get(), true, -1); } collection = vocbase->lookupCollection("_iresearch_analyzers"); @@ -678,7 +678,8 @@ SECTION("test_persistence") { { arangodb::OperationOptions options; arangodb::ManagedDocumentResult result; - auto* collection = vocbase->lookupCollection("_iresearch_analyzers"); + TRI_voc_tick_t resultMarkerTick; + auto collection = vocbase->lookupCollection("_iresearch_analyzers"); collection->truncate(nullptr, options); } @@ -833,10 +834,10 @@ SECTION("test_start") { { // ensure no configuration collection { - auto* collection = vocbase->lookupCollection("_iresearch_analyzers"); + auto collection = vocbase->lookupCollection("_iresearch_analyzers"); if (collection) { - vocbase->dropCollection(collection, true, -1); + vocbase->dropCollection(collection.get(), true, -1); } collection = vocbase->lookupCollection("_iresearch_analyzers"); @@ -868,10 +869,10 @@ SECTION("test_start") { { // ensure no configuration collection { - auto* collection = vocbase->lookupCollection("_iresearch_analyzers"); + auto collection = vocbase->lookupCollection("_iresearch_analyzers"); if (collection) { - vocbase->dropCollection(collection, true, -1); + vocbase->dropCollection(collection.get(), true, -1); } collection = vocbase->lookupCollection("_iresearch_analyzers"); @@ -906,10 +907,10 @@ SECTION("test_start") { { // ensure there is an empty configuration collection { - auto* collection = vocbase->lookupCollection("_iresearch_analyzers"); + auto collection = vocbase->lookupCollection("_iresearch_analyzers"); if (collection) { - vocbase->dropCollection(collection, true, -1); + vocbase->dropCollection(collection.get(), true, -1); } collection = vocbase->lookupCollection("_iresearch_analyzers"); @@ -947,10 +948,10 @@ SECTION("test_start") { { // ensure there is an empty configuration collection { - auto* collection = vocbase->lookupCollection("_iresearch_analyzers"); + auto collection = vocbase->lookupCollection("_iresearch_analyzers"); if (collection) { - vocbase->dropCollection(collection, true, -1); + vocbase->dropCollection(collection.get(), true, -1); } collection = vocbase->lookupCollection("_iresearch_analyzers"); @@ -990,10 +991,10 @@ SECTION("test_start") { { // ensure no configuration collection { - auto* collection = vocbase->lookupCollection("_iresearch_analyzers"); + auto collection = vocbase->lookupCollection("_iresearch_analyzers"); if (collection) { - vocbase->dropCollection(collection, true, -1); + vocbase->dropCollection(collection.get(), true, -1); } collection = vocbase->lookupCollection("_iresearch_analyzers"); @@ -1022,10 +1023,10 @@ SECTION("test_start") { { // ensure no configuration collection { - auto* collection = vocbase->lookupCollection("_iresearch_analyzers"); + auto collection = vocbase->lookupCollection("_iresearch_analyzers"); if (collection) { - vocbase->dropCollection(collection, true, -1); + vocbase->dropCollection(collection.get(), true, -1); } collection = vocbase->lookupCollection("_iresearch_analyzers"); @@ -1056,10 +1057,10 @@ SECTION("test_start") { // ensure there is an empty configuration collection { - auto* collection = vocbase->lookupCollection("_iresearch_analyzers"); + auto collection = vocbase->lookupCollection("_iresearch_analyzers"); if (collection) { - vocbase->dropCollection(collection, true, -1); + vocbase->dropCollection(collection.get(), true, -1); } collection = vocbase->lookupCollection("_iresearch_analyzers"); @@ -1094,10 +1095,10 @@ SECTION("test_start") { { // ensure there is an empty configuration collection { - auto* collection = vocbase->lookupCollection("_iresearch_analyzers"); + auto collection = vocbase->lookupCollection("_iresearch_analyzers"); if (collection) { - vocbase->dropCollection(collection, true, -1); + vocbase->dropCollection(collection.get(), true, -1); } collection = vocbase->lookupCollection("_iresearch_analyzers"); @@ -1155,10 +1156,10 @@ SECTION("test_tokens") { // ensure there is no configuration collection { - auto* collection = vocbase->lookupCollection("_iresearch_analyzers"); + auto collection = vocbase->lookupCollection("_iresearch_analyzers"); if (collection) { - vocbase->dropCollection(collection, true, -1); + vocbase->dropCollection(collection.get(), true, -1); } collection = vocbase->lookupCollection("_iresearch_analyzers");