10000 issue 496.1: switch scope of responsibility between a TRI_vocbase_t a… · sita1999/arangodb@8f44afb · GitHub
[go: up one dir, main page]
Skip to content

Commit 8f44afb

Browse files
vasiliy-arangodbAndrey Abramov
vasiliy-arangodb
authored and
Andrey Abramov
committed
issue 496.1: switch scope of responsibility between a TRI_vocbase_t and a LogicalView in respect to view creation/deletion (arangodb#7101)
* issue 496.1: switch scope of responsi 10000 bility between a TRI_vocbase_t and a LogicalView in respect to view creation/deletion * backport: address test failures * backport: ensure arangosearch links get exported in the dump * backport: ensure view is created during restore on the coordinator * Updates for ArangoSearch DDL tests, IResearchView unregistration and known issues * Add fix for internal issue 483
1 parent 84de3f6 commit 8f44afb

37 files changed

+2235
-1243
lines changed

‎Documentation/Books/Manual/ReleaseNotes/KnownIssues34.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,4 @@ ArangoSearch
4343
* ArangoSearch ignores `_id` attribute even if `includeAllFields` is set to `true` (internal #445)
4444
* Using score functions (BM25/TFIDF) in ArangoDB expression is not supported (internal #316)
4545
* Using a loop variable in expressions within a corresponding SEARCH condition is not supported (internal #318)
46-
* ArangoSearch doesn't support joins with satellite collections (internal #440)
4746
* RocksDB recovery fails sometimes after renaming a view (internal #469)

‎arangod/Cluster/ClusterInfo.cpp

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -580,32 +580,28 @@ void ClusterInfo::loadPlan() {
580580
viewPairSlice.key.copyString();
581581

582582
try {
583-
auto preCommit = [this, viewId, databaseName](std::shared_ptr<LogicalView> const& view)->bool {
584-
auto& views = _newPlannedViews[databaseName];
585-
// register with name as well as with id:
586-
views.reserve(views.size() + 2);
587-
views[viewId] = view;
588-
views[view->name()] = view;
589-
views[view->guid()] = view;
590-
return true;
591-
};
592-
593-
const auto newView = LogicalView::create(
594-
*vocbase,
595-
viewPairSlice.value,
596-
false, // false == coming from Agency
597-
newPlanVersion,
598-
preCommit
583+
LogicalView::ptr view;
584+
auto res = LogicalView::instantiate(
585+
view, *vocbase, viewPairSlice.value, newPlanVersion
599586
);
600587

601-
if (!newView) {
588+
if (!res.ok() || !view) {
602589
LOG_TOPIC(ERR, Logger::AGENCY)
603590
<< "Failed to create view '" << viewId
604591
<< "'. The view will be ignored for now and the invalid information "
605592
"will be repaired. VelocyPack: "
606593
<< viewSlice.toJson();
594+
607595
continue;
608596
}
597+
598+
auto& views = _newPlannedViews[databaseName];
599+
600+
// register with guid/id/name
601+
views.reserve(views.size() + 3);
602+
views[viewId] = view;
603+
views[view->name()] = view;
604+
views[view->guid()] = view;
609605
} catch (std::exception const& ex) {
610606
// The Plan contains invalid view information.
611607
// This should not happen in healthy situations.
@@ -619,7 +615,6 @@ void ClusterInfo::loadPlan() {
619615
<< viewSlice.toJson();
620616

621617
TRI_ASSERT(false);
622-
continue;
623618
} catch (...) {
624619
// The Plan contains invalid view information.
625620
// This should not happen in healthy situations.
@@ -633,7 +628,6 @@ void ClusterInfo::loadPlan() {
633628
<< viewSlice.toJson();
634629

635630
TRI_ASSERT(false);
636-
continue;
637631
}
638632
}
639633

‎arangod/IResearch/IResearchFeature.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -345,11 +345,11 @@ void registerViewFactory() {
345345

346346
// DB server in custer or single-server
347347
if (arangodb::ServerState::instance()->isCoordinator()) {
348-
res = viewTypes->emplace(viewType, arangodb::iresearch::IResearchViewCoordinator::make);
348+
res = viewTypes->emplace(viewType, arangodb::iresearch::IResearchViewCoordinator::factory());
349349
} else if (arangodb::ServerState::instance()->isDBServer()) {
350-
res = viewTypes->emplace(viewType, arangodb::iresearch::IResearchViewDBServer::make);
350+
res = viewTypes->emplace(viewType, arangodb::iresearch::IResearchViewDBServer::factory());
351351
} else if (arangodb::ServerState::instance()->isSingleServer()) {
352-
res = viewTypes->emplace(viewType, arangodb::iresearch::IResearchView::make);
352+
res = viewTypes->emplace(viewType, arangodb::iresearch::IResearchView::factory());
353353
} else {
354354
THROW_ARANGO_EXCEPTION_MESSAGE(
355355
TRI_ERROR_FAILED,

‎arangod/IResearch/IResearchLink.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,13 @@ int IResearchLink::drop() {
147147
_dropCollectionInDestructor = false; // will do drop now
148148
_defaultGuid = _view->guid(); // remember view ID just in case (e.g. call to toVelocyPack(...) after unload())
149149

150-
TRI_voc_cid_t vid = _view->id();
150+
auto view = _view;
151151
_view = nullptr; // mark as unassociated
152152
_viewLock.unlock(); // release read-lock on the IResearch View
153153

154154
// FIXME TODO this workaround should be in ClusterInfo when moving 'Plan' to 'Current', i.e. IResearchViewDBServer::drop
155155
if (arangodb::ServerState::instance()->isDBServer()) {
156-
return _collection.vocbase().dropView(vid, true).errorNumber(); // cluster-view in ClusterInfo should already not have cid-view
156+
return view->drop().errorNumber(); // cluster-view in ClusterInfo should already not have cid-view
157157
}
158158

159159
return TRI_ERROR_NO_ERROR;
@@ -552,4 +552,4 @@ NS_END // arangodb
552552

553553
// -----------------------------------------------------------------------------
554554
// --SECTION-- END-OF-FILE
555-
// -----------------------------------------------------------------------------
555+
// -----------------------------------------------------------------------------

‎arangod/IResearch/IResearchLinkHelper.cpp

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,68 @@ namespace iresearch {
502502
return LINK_TYPE;
503503
}
504504

505+
/*static*/ arangodb::Result IResearchLinkHelper::validateLinks(
506+
TRI_vocbase_t& vocbase,
507+
arangodb::velocypack::Slice const& links
508+
) {
509+
if (!links.isObject()) {
510+
return arangodb::Result(
511+
TRI_ERROR_BAD_PARAMETER,
512+
std::string("while validating arangosearch link definition, error: definition is not an object")
513+
);
514+
}
515+
516+
size_t offset = 0;
517+
arangodb::CollectionNameResolver resolver(vocbase);
518+
519+
for (arangodb::velocypack::ObjectIterator itr(links);
520+
itr.valid();
521+
++itr, ++offset
522+
) {
523+
auto collectionName = itr.key();
524+
auto linkDefinition = itr.value();
525+
526+
if (!collectionName.isString()) {
527+
return arangodb::Result(
528+
TRI_ERROR_BAD_PARAMETER,
529+
std::string("while validating arangosearch link definition, error: collection at offset ") + std::to_string(offset) + " is not a string"
530+
);
531+
}
532+
533+
auto collection = resolver.getCollection(collectionName.copyString());
534+
535+
if (!collection) {
536+
return arangodb::Result(
537+
TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND,
538+
std::string("while validating arangosearch link definition, error: collection '") + collectionName.copyString() + "' not a string"
539+
);
540+
}
541+
542+
// check link auth as per https://github.com/arangodb/backlog/issues/459
543+
if (arangodb::ExecContext::CURRENT
544+
&& !arangodb::ExecContext::CURRENT->canUseCollection(vocbase.name(), collection->name(), arangodb::auth::Level::RO)) {
545+
return arangodb::Result(
546+
TRI_ERROR_FORBIDDEN,
547+
std::string("while validating arangosearch link definition, error: collection '") + collectionName.copyString() + "' not authorised for read access"
548+
);
549+
}
550+
551+
IResearchLinkMeta meta;
552+
std::string errorField;
553+
554+
if (!linkDefinition.isNull() && !meta.init(linkDefinition, errorField)) {
555+
return arangodb::Result(
556+
TRI_ERROR_BAD_PARAMETER,
557+
errorField.empty()
558+
? (std::string("while validating arangosearch link definition, error: invalid link definition for collection '") + collectionName.copyString() + "': " + linkDefinition.toString())
559+
: (std::string("while validating arangosearch link definition, error: invalid link definition for collection '") + collectionName.copyString() + "' error in attribute: " + errorField)
560+
);
561+
}
562+
}
563+
564+
return arangodb::Result();
565+
}
566+
505567
/*static*/ arangodb::Result IResearchLinkHelper::updateLinks(
506568
std::unordered_set<TRI_voc_cid_t>& modified,
507569
TRI_vocbase_t& vocbase,
@@ -577,4 +639,4 @@ namespace iresearch {
577639

578640
// -----------------------------------------------------------------------------
579641
// --SECTION-- END-OF-FILE
580-
// -----------------------------------------------------------------------------
642+
// -----------------------------------------------------------------------------

‎arangod/IResearch/IResearchLinkHelper.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,18 @@ struct IResearchLinkHelper {
6565
////////////////////////////////////////////////////////////////////////////////
6666
static std::string const& type() noexcept;
6767

68+
//////////////////////////////////////////////////////////////////////////////
69+
/// @brief validate the link specifications for:
70+
/// * valid link meta
71+
/// * collection existence
72+
/// * collection permissions
73+
/// * valid link meta
74+
//////////////////////////////////////////////////////////////////////////////
75+
static arangodb::Result validateLinks(
76+
TRI_vocbase_t& vocbase,
77+
arangodb::velocypack::Slice const& links
78+
);
79+
6880
//////////////////////////////////////////////////////////////////////////////
6981
/// @brief updates the collections in 'vocbase' to match the specified
7082
/// IResearchLink definitions

0 commit comments

Comments
 (0)
0