issue 381.1: ensure view lookup is done via collectionNameResover, ensure updateProperties returns current view properties, remove redundant functions (#5439)

arangod/IResearch/IResearchLink.cpp

@@ -131,10 +131,6 @@ bool IResearchLink::canBeDropped() const {
   return true; // valid for a link to be dropped from an iResearch view
 }
 
-LogicalCollection* IResearchLink::collection() const noexcept {
-  return _collection;
-}
-
 int IResearchLink::drop() {
   if (!_collection) {
     return TRI_ERROR_ARANGO_COLLECTION_NOT_LOADED; // '_collection' required
@@ -204,7 +200,7 @@ bool IResearchLink::init(arangodb::velocypack::Slice const& definition) {
     return false; // failed to parse metadata
   }
 
-  if (!collection()
+  if (!_collection
       || !definition.isObject()
       || !definition.get(StaticStrings::ViewIdField).isNumber<uint64_t>()) {
     LOG_TOPIC(WARN, arangodb::iresearch::TOPIC)
@@ -216,7 +212,7 @@ bool IResearchLink::init(arangodb::velocypack::Slice const& definition) {
 
   auto identifier = definition.get(StaticStrings::ViewIdField);
   auto viewId = identifier.getNumber<uint64_t>();
-      auto& vocbase = collection()->vocbase();
+  auto& vocbase = _collection->vocbase();
       auto logicalView = vocbase.lookupView(viewId);
 
       if (!logicalView
@@ -271,7 +267,7 @@ bool IResearchLink::init(arangodb::velocypack::Slice const& definition) {
         return false;
       }
 
-      _dropCollectionInDestructor = view->emplace(collection()->id()); // track if this is the instance that called emplace
+      _dropCollectionInDestructor = view->emplace(_collection->id()); // track if this is the instance that called emplace
       _meta = std::move(meta);
       _view = std::move(view);
       _wiew = std::move(wiew);
@@ -489,9 +485,7 @@ int IResearchLink::unload() {
 
   _defaultId = _wiew ? _wiew->id() : _view->id(); // remember view ID just in case (e.g. call to toVelocyPack(...) after unload())
 
-  auto* col = collection();
-
-  if (!col) {
+  if (!_collection) {
     LOG_TOPIC(WARN, arangodb::iresearch::TOPIC)
       << "failed finding collection while unloading IResearch link '" << _id << "'";
 
@@ -501,7 +495,7 @@ int IResearchLink::unload() {
   // this code is used by the MMFilesEngine
   // if the collection is in the process of being removed then drop it from the view
   // FIXME TODO remove once LogicalCollection::drop(...) will drop its indexes explicitly
-  if (col->deleted()) {
+  if (_collection->deleted()) {
     auto res = drop();
 
     if (TRI_ERROR_NO_ERROR != res) {

arangod/IResearch/IResearchLink.h

@@ -76,11 +76,6 @@ class IResearchLink {
 
   bool canBeDropped() const; // arangodb::Index override
 
-  ////////////////////////////////////////////////////////////////////////////////
-  /// @brief the collection of this link
-  ////////////////////////////////////////////////////////////////////////////////
-  LogicalCollection* collection() const noexcept;
-
   ////////////////////////////////////////////////////////////////////////////////
   /// @brief called when the iResearch Link is dropped
   ////////////////////////////////////////////////////////////////////////////////
@@ -208,4 +203,4 @@ class IResearchLink {
 NS_END // iresearch
 NS_END // arangodb
 
-#endif
+#endif

arangod/IResearch/IResearchLinkCoordinator.cpp

@@ -407,7 +407,8 @@ bool IResearchLinkCoordinator::init(VPackSlice definition) {
     return false; // failed to parse metadata
   }
 
-  if (!definition.isObject()
+  if (!_collection
+      || !definition.isObject()
       || !definition.get(StaticStrings::ViewIdField).isNumber<uint64_t>()) {
     LOG_TOPIC(WARN, arangodb::iresearch::TOPIC)
         << "error finding view for link '" << _id << "'";
@@ -418,7 +419,7 @@ bool IResearchLinkCoordinator::init(VPackSlice definition) {
 
   auto identifier = definition.get(StaticStrings::ViewIdField);
   auto viewId = identifier.getNumber<uint64_t>();
-    auto& vocbase = collection().vocbase();
+  auto& vocbase = _collection->vocbase();
     auto logicalView  = vocbase.lookupView(viewId);
 
     if (!logicalView

arangod/IResearch/IResearchLinkCoordinator.h

@@ -115,13 +115,6 @@ class IResearchLinkCoordinator {
     return _meta.memory();
   }
 
-  ////////////////////////////////////////////////////////////////////////////////
-  /// @brief the collection of this link
-  ////////////////////////////////////////////////////////////////////////////////
-  LogicalCollection const& collection() const noexcept {
-    return *_collection;
-  }
-
   ////////////////////////////////////////////////////////////////////////////////
   /// @brief fill and return a JSON description of a IResearchLinkCoordinator object
   ////////////////////////////////////////////////////////////////////////////////

arangod/IResearch/IResearchLinkHelper.cpp

@@ -231,6 +231,15 @@ namespace iresearch {
       collectionsToLock, // exclusiveCollections
       arangodb::transaction::Options() // use default lock timeout
     );
+    auto* trxResolver = trx.resolver();
+
+    if (!trxResolver) {
+      return arangodb::Result(
+        TRI_ERROR_ARANGO_ILLEGAL_STATE,
+        std::string("failed to find collection name resolver while updating IResearch view '") + view.name() + "'"
+      );
+    }
+
     auto res = trx.begin();
 
     if (!res.ok()) {
@@ -246,7 +255,7 @@ namespace iresearch {
         auto& state = *itr;
         auto& collectionName = collectionsToLock[state._collectionsToLockOffset];
 
-        state._collection = vocbase.lookupCollection(collectionName);
+        state._collection = trxResolver->getCollection(collectionName);
 
         if (!state._collection) {
           // remove modification state if removal of non-existant link on non-existant collection

arangod/IResearch/IResearchView.cpp

@@ -1790,11 +1790,8 @@ arangodb::Result IResearchView::updateProperties(
   IResearchViewMeta::Mask mask;
   WriteMutex mutex(_mutex); // '_meta' can be asynchronously read
   arangodb::Result res;
-
-  {
-    SCOPED_LOCK(mutex);
-
     arangodb::velocypack::Builder originalMetaJson; // required for reverting links on failure
+  SCOPED_LOCK_NAMED(mutex, mtx);
 
     if (!_meta.json(arangodb::velocypack::ObjectBuilder(&originalMetaJson))) {
       return arangodb::Result(
@@ -1826,7 +1823,7 @@ arangodb::Result IResearchView::updateProperties(
     }
 
     _meta = std::move(meta);
-  }
+  mutex.unlock(true); // downgrade to a read-lock
 
   if (!slice.hasKey(StaticStrings::LinksField)) {
     return res;
@@ -1842,13 +1839,19 @@ arangodb::Result IResearchView::updateProperties(
   auto links = slice.get(StaticStrings::LinksField);
 
   if (partialUpdate) {
+    mtx.unlock(); // release lock
+
     return IResearchLinkHelper::updateLinks(
       collections, vocbase(), *this, links
     );
   }
 
+  auto stale = _meta._collections;
+
+  mtx.unlock(); // release lock
+
   return IResearchLinkHelper::updateLinks(
-    collections, vocbase(), *this, links, _meta._collections
+    collections, vocbase(), *this, links, stale
   );
 }
 

arangod/V8Server/v8-views.cpp

@@ -27,6 +27,7 @@
 #include "Logger/Logger.h"
 #include "RestServer/DatabaseFeature.h"
 #include "Transaction/V8Context.h"
+#include "Utils/CollectionNameResolver.h"
 #include "V8/v8-conv.h"
 #include "V8/v8-globals.h"
 #include "V8/v8-utils.h"
@@ -449,7 +450,19 @@ static void JS_PropertiesViewVocbase(
     }
   }
 
+  // in the cluster the view object might contain outdated
+  // properties, which will break tests. We need an extra lookup
+  arangodb::CollectionNameResolver resolver(view->vocbase());
+  auto updatedView = resolver.getView(view->id());
+
+  if (!updatedView) {
+    TRI_V8_THROW_EXCEPTION(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND);
+  }
+
+  view = updatedView.get();
+
   VPackBuilder vpackProperties;
+
   vpackProperties.openObject();
   view->toVelocyPack(vpackProperties, true, false);
   vpackProperties.close();