issue 355.3: allow IResearchLink creation, as opposed to IResearch View update, to direct which CIDs get included in an IResearchView snapshot

arangod/IResearch/IResearchLink.cpp

@@ -76,11 +76,16 @@ IResearchLink::IResearchLink(
   arangodb::LogicalCollection* collection
 ): _collection(collection),
    _defaultId(0), // 0 is never a valid id
+   _dropCollectionInDestructor(false),
    _id(iid),
    _view(nullptr) {
 }
 
 IResearchLink::~IResearchLink() {
+  if (_dropCollectionInDestructor) {
+    drop();
+  }
+
   unload(); // disassociate from view if it has not been done yet
 }
 
@@ -181,7 +186,8 @@ int IResearchLink::drop() {
     }
   }
 
-  // FIXME TODO remove link via update properties on view
+  _dropCollectionInDestructor = false; // will do drop now
+
   return _view->drop(_collection->id());
 }
 
@@ -260,6 +266,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
       _meta = std::move(meta);
       _view = std::move(view);
 
@@ -541,6 +548,7 @@ int IResearchLink::unload() {
     }
   }
 
+  _dropCollectionInDestructor = false; // valid link (since unload(..) called), should not be dropped
   _view = nullptr; // mark as unassociated
   _viewLock.unlock(); // release read-lock on the IResearch View
 

arangod/IResearch/IResearchLink.h

@@ -205,14 +205,12 @@ class IResearchLink {
   // FIXME TODO remove once View::updateProperties(...) will be fixed to write
   // the update delta into the WAL marker instead of the full persisted state
   // FIXME TODO remove #include "IResearchView.h"
-//  friend arangodb::Result IResearchView::updatePropertiesImpl(
+  // friend arangodb::Result IResearchView::updateProperties(arangodb::velocypack::Slice const&, bool);
-//    arangodb::velocypack::Slice const&, bool, bool
-//  );
   friend class IResearchView;
 
-
   LogicalCollection* _collection; // the linked collection
   TRI_voc_cid_t _defaultId; // the identifier of the desired view (iff _view == nullptr)
+  bool _dropCollectionInDestructor; // collection should be dropped from view in the destructor (for the case where init(..) is called folowed by distructor)
   TRI_idx_iid_t const _id; // the index identifier
   IResearchLinkMeta _meta; // how this collection should be indexed
   mutable irs::async_utils::read_write_mutex _mutex; // for use with _view to allow asynchronous disassociation
@@ -232,4 +230,4 @@ int EnhanceJsonIResearchLink(
 NS_END // iresearch
 NS_END // arangodb
 
-#endif
+#endif

arangod/IResearch/IResearchView.cpp

@@ -48,7 +48,6 @@
 #include "Aql/SortCondition.h"
 #include "Basics/Result.h"
 #include "Basics/files.h"
-#include "Basics/WriteLocker.h"
 #include "Logger/Logger.h"
 #include "Logger/LogMacros.h"
 #include "StorageEngine/EngineSelectorFeature.h"
@@ -306,6 +305,27 @@ std::shared_ptr<arangodb::iresearch::IResearchLink> findFirstMatchingLink(
   return nullptr;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+/// @brief compute the data path to user for iresearch persisted-store
+///        get base path from DatabaseServerFeature (similar to MMFilesEngine)
+///        the path is hardcoded to reside under:
+///        <DatabasePath>/<IResearchView::type()>-<view id>
+///        similar to the data path calculation for collections
+////////////////////////////////////////////////////////////////////////////////
+irs::utf8_path getPersistedPath(
+    arangodb::DatabasePathFeature const& dbPathFeature, TRI_voc_cid_t id
+) {
+  irs::utf8_path dataPath(dbPathFeature.directory());
+  static const std::string subPath("databases");
+
+  dataPath /= subPath;
+  dataPath /= arangodb::iresearch::IResearchView::type().name();
+  dataPath += "-";
+  dataPath += std::to_string(id);
+
+  return dataPath;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief inserts ArangoDB document into an IResearch data store
 ////////////////////////////////////////////////////////////////////////////////
@@ -340,6 +360,100 @@ inline void insertDocument(
   doc.insert(irs::action::store, primaryKey);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+/// @brief persist view definition to the storage engine
+///        if in-recovery then register a post-recovery lambda for persistence
+/// @return success
+////////////////////////////////////////////////////////////////////////////////
+arangodb::Result persistProperties(
+    arangodb::LogicalView const& view,
+    arangodb::iresearch::IResearchView::AsyncSelf::ptr asyncSelf
+) {
+  auto* engine = arangodb::EngineSelectorFeature::ENGINE;
+
+  if (!engine) {
+    return arangodb::Result(
+      TRI_ERROR_INTERNAL,
+      std::string("failure to get storage engine while persisting definition for LogicalView '") + view.name() + "'"
+    );
+  }
+
+  if (!engine->inRecovery()) {
+    // change view throws exception on error
+    try {
+      engine->changeView(view.vocbase(), view.id(), &view, true);
+    } catch (std::exception const& e) {
+      return arangodb::Result(
+        TRI_ERROR_INTERNAL,
+        std::string("caught exception during persistance of properties for IResearch View '") + view.name() + "': " + e.what()
+      );
+    } catch (...) {
+      return arangodb::Result(
+        TRI_ERROR_INTERNAL,
+        std::string("caught exception during persistance of properties for IResearch View '") + view.name() + "'"
+      );
+    }
+
+    return arangodb::Result();
+  }
+
+  auto* feature =
+    arangodb::iresearch::getFeature<arangodb::DatabaseFeature>("Database");
+
+  if (!feature) {
+    return arangodb::Result(
+      TRI_ERROR_INTERNAL,
+      std::string("failure to get 'Database' feature while persisting definition for LogicalView '") + view.name() + "'"
+    );
+  }
+
+  return feature->registerPostRecoveryCallback(
+    [&view, asyncSelf]()->arangodb::Result {
+      auto* engine = arangodb::EngineSelectorFeature::ENGINE;
+
+      if (!engine) {
+        return arangodb::Result(
+          TRI_ERROR_INTERNAL,
+          std::string("failure to get storage engine while persisting definition for LogicalView")
+        );
+      }
+
+      if (!asyncSelf) {
+        return arangodb::Result(
+          TRI_ERROR_INTERNAL,
+          std::string("invalid view instance passed while persisting definition for LogicalView")
+        );
+      }
+
+      SCOPED_LOCK(asyncSelf->mutex());
+
+      if (!asyncSelf->get()) {
+        LOG_TOPIC(INFO, arangodb::iresearch::IResearchFeature::IRESEARCH)
+          << "no view instance available while persisting definition for LogicalView";
+
+        return arangodb::Result(); // nothing to persist, view allready deallocated
+      }
+
+      // change view throws exception on error
+      try {
+        engine->changeView(view.vocbase(), view.id(), &view, true);
+      } catch (std::exception const& e) {
+        return arangodb::Result(
+          TRI_ERROR_INTERNAL,
+          std::string("caught exception during persistance of properties for IResearch View '") + view.name() + "': " + e.what()
+        );
+      } catch (...) {
+        return arangodb::Result(
+          TRI_ERROR_INTERNAL,
+          std::string("caught exception during persistance of properties for IResearch View '") + view.name() + "'"
+        );
+      }
+
+      return arangodb::Result();
+    }
+  );
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief syncs an IResearch DataStore if required
 /// @return a sync was executed
@@ -767,16 +881,16 @@ IResearchView::PersistedStore::PersistedStore(irs::utf8_path&& path)
 IResearchView::IResearchView(
     TRI_vocbase_t* vocbase,
     arangodb::velocypack::Slice const& info,
-    irs::utf8_path&& persistedPath,
+    arangodb::DatabasePathFeature const& dbPathFeature,
     bool isNew
-) : DBServerLogicalView(vocbase, info, isNew),
+): DBServerLogicalView(vocbase, info, isNew),
     FlushTransaction(toString(*this)),
    _asyncMetaRevision(1),
    _asyncSelf(irs::memory::make_unique<AsyncSelf>(this)),
    _asyncTerminate(false),
    _memoryNode(&_memoryNodes[0]), // set current memory node (arbitrarily 0)
    _toFlush(&_memoryNodes[1]), // set flush-pending memory node (not same as _memoryNode)
-   _storePersisted(std::move(persistedPath)),
+   _storePersisted(getPersistedPath(dbPathFeature, id())),
    _threadPool(0, 0), // 0 == create pool with no threads, i.e. not running anything
    _inRecovery(false) {
   // set up in-recovery insertion hooks
@@ -791,7 +905,7 @@ IResearchView::IResearchView(
       auto* viewPtr = view->get();
 
       if (!viewPtr) {
-        LOG_TOPIC(WARN, arangodb::iresearch::IResearchFeature::IRESEARCH)
+        LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
           << "Invalid call to post-recovery callback of iResearch view";
 
         return arangodb::Result(); // view no longer in recovery state
@@ -800,25 +914,25 @@ IResearchView::IResearchView(
       viewPtr->verifyKnownCollections();
 
       if (viewPtr->_storePersisted) {
-        LOG_TOPIC(DEBUG, iresearch::IResearchFeature::IRESEARCH)
+        LOG_TOPIC(DEBUG, IResearchFeature::IRESEARCH)
           << "starting persisted-sync sync for iResearch view '" << viewPtr->id() << "'";
 
         try {
           viewPtr->_storePersisted.sync();
         } catch (std::exception const& e) {
-          LOG_TOPIC(ERR, iresearch::IResearchFeature::IRESEARCH)
+          LOG_TOPIC(ERR, IResearchFeature::IRESEARCH)
             << "caught exception while committing persisted store for iResearch view '" << viewPtr->id()
             << "': " << e.what();
 
           return arangodb::Result(TRI_ERROR_INTERNAL, e.what());
         } catch (...) {
-          LOG_TOPIC(ERR, iresearch::IResearchFeature::IRESEARCH)
+          LOG_TOPIC(ERR, IResearchFeature::IRESEARCH)
             << "caught exception while committing persisted store for iResearch view '" << viewPtr->id() << "'";
 
           return arangodb::Result(TRI_ERROR_INTERNAL);
         }
 
-        LOG_TOPIC(DEBUG, iresearch::IResearchFeature::IRESEARCH)
+        LOG_TOPIC(DEBUG, IResearchFeature::IRESEARCH)
           << "finished persisted-sync sync for iResearch view '" << viewPtr->id() << "'";
       }
 
@@ -856,7 +970,7 @@ IResearchView::IResearchView(
       auto res = viewPtr->finish(state.id(), false);
 
       if (TRI_ERROR_NO_ERROR != res) {
-        LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+        LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
           << "failed to finish abort while processing write-transaction callback for IResearch view '" << viewPtr->name() << "'";
       }
 
@@ -866,10 +980,10 @@ IResearchView::IResearchView(
       auto res = viewPtr->finish(state.id(), true);
 
       if (TRI_ERROR_NO_ERROR != res) {
-        LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+        LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
           << "failed to finish commit while processing write-transaction callback for IResearch view '" << viewPtr->name() << "'";
       } else if (state.waitForSync() && !viewPtr->sync()) {
-        LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+        LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
           << "failed to sync while processing write-transaction callback for IResearch view '" << viewPtr->name() << "'";
       }
 
@@ -1119,17 +1233,17 @@ void IResearchView::drop() {
     // remove persisted data store directory if present
     if (_storePersisted._path.exists_directory(exists)
         && (!exists || _storePersisted._path.remove())) {
-      DBServerLogicalView::drop();
       deleted(true);
+
       return; // success
     }
   } catch (std::exception const& e) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while removing iResearch view '" << id() << "': " << e.what();
     IR_LOG_EXCEPTION();
     throw;
   } catch (...) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while removing iResearch view '" << id() << "'";
     IR_LOG_EXCEPTION();
     throw;
@@ -1140,8 +1254,25 @@ void IResearchView::drop() {
 
 int IResearchView::drop(TRI_voc_cid_t cid) {
   std::shared_ptr<irs::filter> shared_filter(iresearch::FilterFactory::filter(cid));
-  ReadMutex mutex(_mutex); // '_storeByTid' can be asynchronously updated
+  WriteMutex mutex(_mutex); // '_meta' and '_storeByTid' can be asynchronously updated
   SCOPED_LOCK(mutex);
+  auto cid_itr = _meta._collections.find(cid);
+
+  if (cid_itr != _meta._collections.end()) {
+    auto result = persistProperties(*this, _asyncSelf);
+
+    if (!result.ok()) {
+      LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
+        << "failed to persist logical view while dropping collection ' " << cid
+        << "' from IResearch View '" << name() << "': " << result.errorMessage();
+
+      return result.errorNumber();
+    }
+
+    _meta._collections.erase(cid_itr);
+  }
+
+  mutex.unlock(true); // downgrade to a read-lock
 
   // ...........................................................................
   // if an exception occurs below than a drop retry would most likely happen
@@ -1160,12 +1291,12 @@ int IResearchView::drop(TRI_voc_cid_t cid) {
 
     return TRI_ERROR_NO_ERROR;
   } catch (std::exception const& e) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while removing from iResearch view '" << id()
       << "', collection '" << cid << "': " << e.what();
     IR_LOG_EXCEPTION();
   } catch (...) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while removing from iResearch view '" << id()
       << "', collection '" << cid << "'";
     IR_LOG_EXCEPTION();
@@ -1174,6 +1305,45 @@ int IResearchView::drop(TRI_voc_cid_t cid) {
   return TRI_ERROR_INTERNAL;
 }
 
+bool IResearchView::emplace(TRI_voc_cid_t cid) {
+  WriteMutex mutex(_mutex); // '_meta' can be asynchronously updated
+  SCOPED_LOCK(mutex);
+  arangodb::Result result;
+
+  if (!_meta._collections.emplace(cid).second) {
+    return false;
+  }
+
+  try {
+    result = persistProperties(*this, _asyncSelf);
+
+    if (result.ok()) {
+      return true;
+    }
+  } catch (std::exception const& e) {
+    _meta._collections.erase(cid); // undo meta modification
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
+      << "caught exception during persisting of logical view while emplacing collection ' " << cid
+      << "' into IResearch View '" << name() << "': " << e.what();
+    IR_LOG_EXCEPTION();
+    throw;
+  } catch (...) {
+    _meta._collections.erase(cid); // undo meta modification
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
+      << "caught exception during persisting of logical view while emplacing collection ' " << cid
+      << "' into IResearch View '" << name() << "'";
+    IR_LOG_EXCEPTION();
+    throw;
+  }
+
+  _meta._collections.erase(cid); // undo meta modification
+  LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
+    << "failed to persist logical view while emplacing collection ' " << cid
+    << "' into IResearch View '" << name() << "': " << result.errorMessage();
+
+  return false;
+}
+
 int IResearchView::finish(TRI_voc_tid_t tid, bool commit) {
   std::vector<std::shared_ptr<irs::filter>> removals;
   DataStore trxStore;
@@ -1230,12 +1400,12 @@ int IResearchView::finish(TRI_voc_tid_t tid, bool commit) {
 
     return TRI_ERROR_NO_ERROR;
   } catch (std::exception const& e) {
-    LOG_TOPIC(ERR, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(ERR, IResearchFeature::IRESEARCH)
       << "caught exception while committing transaction for iResearch view '" << id()
       << "', tid '" << tid << "': " << e.what();
     IR_LOG_EXCEPTION();
   } catch (...) {
-    LOG_TOPIC(ERR, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(ERR, IResearchFeature::IRESEARCH)
       << "caught exception while committing transaction for iResearch view '" << id()
       << "', tid '" << tid << "'";
     IR_LOG_EXCEPTION();
@@ -1283,11 +1453,11 @@ arangodb::Result IResearchView::commit() {
 
     return TRI_ERROR_NO_ERROR;
   } catch (std::exception const& e) {
-    LOG_TOPIC(ERR, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(ERR, IResearchFeature::IRESEARCH)
       << "caught exception while committing memory store for iResearch view '" << id() << "': " << e.what();
     IR_LOG_EXCEPTION();
   } catch (...) {
-    LOG_TOPIC(ERR, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(ERR, IResearchFeature::IRESEARCH)
       << "caught exception while committing memory store for iResearch view '" << id();
     IR_LOG_EXCEPTION();
   }
@@ -1363,7 +1533,7 @@ void IResearchView::getPropertiesVPack(
           linkBuilder.openObject();
 
           if (!ptr->json(linkBuilder, false)) {
-            LOG_TOPIC(WARN, arangodb::iresearch::IResearchFeature::IRESEARCH)
+            LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
               << "failed to generate json for IResearch link '" << ptr->id()
               << "' while generating json for IResearch view '" << id() << "'";
             continue; // skip invalid link definitions
@@ -1377,12 +1547,12 @@ void IResearchView::getPropertiesVPack(
 
     trx.commit();
   } catch (std::exception const& e) {
-    LOG_TOPIC(WARN, arangodb::iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while generating json for IResearch view '" << id() << "': " << e.what();
     IR_LOG_EXCEPTION();
     return; // do not add 'links' section
   } catch (...) {
-    LOG_TOPIC(WARN, arangodb::iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while generating json for IResearch view '" << id() << "'";
     IR_LOG_EXCEPTION();
     return; // do not add 'links' section
@@ -1440,16 +1610,16 @@ int IResearchView::insert(
       return TRI_ERROR_NO_ERROR;
     }
 
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "failed inserting into iResearch view '" << id()
       << "', collection '" << cid << "', revision '" << documentId.id() << "'";
   } catch (std::exception const& e) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while inserting into iResearch view '" << id()
       << "', collection '" << cid << "', revision '" << documentId.id() << "': " << e.what();
     IR_LOG_EXCEPTION();
   } catch (...) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while inserting into iResearch view '" << id()
       << "', collection '" << cid << "', revision '" << documentId.id() << "'";
     IR_LOG_EXCEPTION();
@@ -1526,18 +1696,18 @@ int IResearchView::insert(
 
   try {
     if (!store->_writer->insert(insert)) {
-      LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+      LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
         << "failed inserting batch into iResearch view '" << id() << "', collection '" << cid;
       return TRI_ERROR_INTERNAL;
     }
 
     store->_writer->commit(); // no need to consolidate if batch size is set correctly
   } catch (std::exception const& e) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while inserting batch into iResearch view '" << id() << "', collection '" << cid << e.what();
     IR_LOG_EXCEPTION();
   } catch (...) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while inserting batch into iResearch view '" << id() << "', collection '" << cid;
     IR_LOG_EXCEPTION();
   }
@@ -1573,18 +1743,8 @@ arangodb::Result IResearchView::link(
   builder.close();
 
   std::unordered_set<TRI_voc_cid_t> collections;
-  auto result = updateLinks(collections, *vocbase, *this, builder.slice());
 
-  if (result.ok()) {
+  return updateLinks(collections, *vocbase, *this, builder.slice());
-    WriteMutex mutex(_mutex); // '_meta' can be asynchronously read
-    SCOPED_LOCK(mutex);
-
-    collections.insert(_meta._collections.begin(), _meta._collections.end());
-    validateLinks(collections, *vocbase, *this); // remove invalid cids (no such collection or no such link)
-    _meta._collections = std::move(collections);
-  }
-
-  return result;
 }
 
 /*static*/ std::shared_ptr<LogicalView> IResearchView::make(
@@ -1592,59 +1752,31 @@ arangodb::Result IResearchView::link(
     arangodb::velocypack::Slice const& info,
     bool isNew
 ) {
-  auto const id = readViewId(info);
-
-  if (0 == id) {
-    // invalid ID
-    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
-      << "got invalid view identifier while constructing IResearch view";
-    return nullptr;
-  }
-
   auto* feature =
     arangodb::iresearch::getFeature<arangodb::DatabasePathFeature>("DatabasePath");
 
   if (!feature) {
     LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
-      << "failure to find feature 'DatabasePath' while constructing IResearch view '"
+      << "failure to find feature 'DatabasePath' while constructing IResearch View in database '" << vocbase.id() << "'";
-      << id << "' in database '" << vocbase.id() << "'";
 
     return nullptr;
   }
 
-  // get base path from DatabaseServerFeature (similar to MMFilesEngine)
+  PTR_NAMED(IResearchView, view, &vocbase, info, *feature, isNew);
-  // the path is hardcoded to reside under:
+  auto& impl = reinterpret_cast<IResearchView&>(*view);
-  // <DatabasePath>/<IResearchView::type()>-<view id>
+  auto& json = info.isObject() ? info : emptyObjectSlice(); // if no 'info' then assume defaults
-  // similar to the data path calculation for collections
+  auto props = json.get("properties");
-  irs::utf8_path dataPath(feature->directory());
+  auto& properties = props.isObject() ? props : emptyObjectSlice(); // if no 'info' then assume defaults
-  static std::string subPath("databases");
-
-  dataPath /= subPath;
-  dataPath /= arangodb::iresearch::IResearchView::type().name();
-  dataPath += "-";
-  dataPath += std::to_string(id);
-
-  auto view = std::shared_ptr<IResearchView>(
-    new IResearchView(&vocbase, info, std::move(dataPath), isNew)
-  );
-
-  auto props = info.get("properties");
-
-  if (props.isNone()) {
-    // if no 'properties' then assume defaults
-    props = emptyObjectSlice();
-  }
-
   std::string error;
 
-  if (!view->_meta.init(props, error)) {
+  if (!impl._meta.init(properties, error)) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "failed to initialize iResearch view from definition, error: " << error;
 
     return nullptr;
   }
 
-  return view;
+  return std::move(view);
 }
 
 size_t IResearchView::memory() const {
@@ -1676,15 +1808,13 @@ size_t IResearchView::memory() const {
 }
 
 void IResearchView::open() {
-  DBServerLogicalView::open();
-
   auto* engine = arangodb::EngineSelectorFeature::ENGINE;
 
   if (engine) {
     _inRecovery = engine->inRecovery();
   } else {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
-      << "failure to get storage engine while starting feature 'IResearchAnalyzer'";
+      << "failure to get storage engine while opening IResearch View: " << name();
     // assume not inRecovery()
   }
 
@@ -1731,18 +1861,18 @@ void IResearchView::open() {
       }
     }
   } catch (std::exception const& e) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while opening iResearch view '" << id() << "': " << e.what();
     IR_LOG_EXCEPTION();
     throw;
   } catch (...) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while opening iResearch view '" << id() << "'";
     IR_LOG_EXCEPTION();
     throw;
   }
 
-  LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+  LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
     << "failed to open IResearch view '" << name() << "' at: " << _storePersisted._path.utf8();
 
   throw std::runtime_error(
@@ -1800,12 +1930,12 @@ int IResearchView::remove(
 
     return TRI_ERROR_NO_ERROR;
   } catch (std::exception const& e) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while removing from iResearch view '" << id()
       << "', collection '" << cid << "', revision '" << documentId.id() << "': " << e.what();
     IR_LOG_EXCEPTION();
   } catch (...) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while removing from iResearch view '" << id()
       << "', collection '" << cid << "', revision '" << documentId.id() << "'";
     IR_LOG_EXCEPTION();
@@ -1834,7 +1964,7 @@ PrimaryKeyIndexReader* IResearchView::snapshot(
   }
 
   if (state.waitForSync() && !sync()) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "failed to sync while creating snapshot for IResearch view '" << name() << "', previous snapshot will be used instead";
   }
 
@@ -1857,14 +1987,14 @@ PrimaryKeyIndexReader* IResearchView::snapshot(
       reader.add(_storePersisted._reader);
     }
   } catch (std::exception const& e) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while collecting readers for snapshot of IResearch view '" << id()
       << "': " << e.what();
     IR_LOG_EXCEPTION();
 
     return nullptr;
   } catch (...) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception while collecting readers for snapshot of IResearch view '" << id() << "'";
     IR_LOG_EXCEPTION();
 
@@ -1887,17 +2017,17 @@ bool IResearchView::sync(size_t maxMsec /*= 0*/) {
   try {
     SCOPED_LOCK(mutex);
 
-    LOG_TOPIC(DEBUG, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(DEBUG, IResearchFeature::IRESEARCH)
       << "starting active memory-store sync for iResearch view '" << id() << "'";
     _memoryNode->_store.sync();
-    LOG_TOPIC(DEBUG, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(DEBUG, IResearchFeature::IRESEARCH)
       << "finished memory-store sync for iResearch view '" << id() << "'";
 
     if (maxMsec && TRI_microtime() >= thresholdSec) {
       return true; // skip if timout exceeded
     }
 
-    LOG_TOPIC(DEBUG, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(DEBUG, IResearchFeature::IRESEARCH)
       << "starting pending memory-store sync for iResearch view '" << id() << "'";
     _toFlush->_store._segmentCount.store(0); // reset to zero to get count of new segments that appear during commit
     _toFlush->_store._writer->commit();
@@ -1908,7 +2038,7 @@ bool IResearchView::sync(size_t maxMsec /*= 0*/) {
       _toFlush->_store._segmentCount += _toFlush->_store._reader.size(); // add commited segments
     }
 
-    LOG_TOPIC(DEBUG, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(DEBUG, IResearchFeature::IRESEARCH)
       << "finished pending memory-store sync for iResearch view '" << id() << "'";
 
     if (maxMsec && TRI_microtime() >= thresholdSec) {
@@ -1917,7 +2047,7 @@ bool IResearchView::sync(size_t maxMsec /*= 0*/) {
 
     // must sync persisted store as well to ensure removals are applied
     if (_storePersisted) {
-      LOG_TOPIC(DEBUG, iresearch::IResearchFeature::IRESEARCH)
+      LOG_TOPIC(DEBUG, IResearchFeature::IRESEARCH)
         << "starting persisted-sync sync for iResearch view '" << id() << "'";
       _storePersisted._segmentCount.store(0); // reset to zero to get count of new segments that appear during commit
       _storePersisted._writer->commit();
@@ -1928,17 +2058,17 @@ bool IResearchView::sync(size_t maxMsec /*= 0*/) {
         _storePersisted._segmentCount += _storePersisted._reader.size(); // add commited segments
       }
 
-      LOG_TOPIC(DEBUG, iresearch::IResearchFeature::IRESEARCH)
+      LOG_TOPIC(DEBUG, IResearchFeature::IRESEARCH)
         << "finished persisted-sync sync for iResearch view '" << id() << "'";
     }
 
     return true;
   } catch (std::exception const& e) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception during sync of iResearch view '" << id() << "': " << e.what();
     IR_LOG_EXCEPTION();
   } catch (...) {
-    LOG_TOPIC(WARN, iresearch::IResearchFeature::IRESEARCH)
+    LOG_TOPIC(WARN, IResearchFeature::IRESEARCH)
       << "caught exception during sync of iResearch view '" << id() << "'";
     IR_LOG_EXCEPTION();
   }
@@ -1954,39 +2084,10 @@ bool IResearchView::sync(size_t maxMsec /*= 0*/) {
   return type;
 }
 
-void IResearchView::toVelocyPack(
-    velocypack::Builder& result,
-    bool includeProperties,
-    bool includeSystem
-) const {
-  // We write into an open object
-  TRI_ASSERT(result.isOpenObject());
-
-  DBServerLogicalView::toVelocyPack(result, includeProperties, includeSystem);
-
-  // Object is still open
-  TRI_ASSERT(result.isOpenObject());
-
-  if (includeProperties) {
-    // implementation Information
-    result.add("properties", VPackValue(VPackValueType::Object));
-    // note: includeSystem and forPersistence are not 100% synonymous,
-    // however, for our purposes this is an okay mapping; we only set
-    // includeSystem if we are persisting the properties
-    getPropertiesVPack(result, includeSystem);
-    result.close();
-  }
-
-  TRI_ASSERT(result.isOpenObject()); // We leave the object open
-}
-
 arangodb::Result IResearchView::updateProperties(
-    velocypack::Slice const& slice,
+    arangodb::velocypack::Slice const& slice,
-    bool partialUpdate,
+    bool partialUpdate
-    bool doSync
 ) {
-  WRITE_LOCKER(writeLocker, _infoLock);
-
   auto* vocbase = this->vocbase();
 
   if (!vocbase) {
@@ -2103,52 +2204,37 @@ arangodb::Result IResearchView::updateProperties(
     _meta = std::move(meta);
   }
 
-  std::unordered_set<TRI_voc_cid_t> collections;
+  if (!slice.hasKey(LINKS_FIELD)) {
+    return res;
+  }
 
+  // ...........................................................................
   // update links if requested (on a best-effort basis)
   // indexing of collections is done in different threads so no locks can be held and rollback is not possible
   // as a result it's also possible for links to be simultaneously modified via a different callflow (e.g. from collections)
-  if (slice.hasKey(LINKS_FIELD)) {
-    if (partialUpdate) {
-      res = updateLinks(collections, *vocbase, *this, slice.get(LINKS_FIELD));
-    } else {
-      arangodb::velocypack::Builder builder;
-
-      builder.openObject();
-
-      if (!appendLinkRemoval(builder, _meta)
-          || !mergeSlice(builder, slice.get(LINKS_FIELD))) {
-        return arangodb::Result(
-          TRI_ERROR_INTERNAL,
-          std::string("failed to construct link update directive while updating iResearch view '") + std::to_string(id()) + "'"
-        );
-      }
-
-      builder.close();
-      res = updateLinks(collections, *vocbase, *this, builder.slice());
-    }
-  }
-
   // ...........................................................................
-  // if an exception occurs below then it would only affect collection linking
+
-  // consistency and an update retry would most likely happen
+  std::unordered_set<TRI_voc_cid_t> collections;
-  // always re-validate '_collections' because may have had externally triggered
+
-  // collection/link drops
+  if (partialUpdate) {
-  // ...........................................................................
+    return updateLinks(collections, *vocbase, *this, slice.get(LINKS_FIELD));
-  {
-    SCOPED_LOCK(mutex); // '_meta' can be asynchronously read
-    collections.insert(_meta._collections.begin(), _meta._collections.end());
-    validateLinks(collections, *vocbase, *this); // remove invalid cids (no such collection or no such link)
-    _meta._collections = std::move(collections);
   }
 
-  // FIXME TODO to ensure valid recovery remove the original datapath only if the entire, but under lock to prevent double rename
+  arangodb::velocypack::Builder builder;
 
-  if (res.ok()) {
+  builder.openObject();
-    res = DBServerLogicalView::updateProperties(slice, partialUpdate, doSync);
+
+  if (!appendLinkRemoval(builder, _meta)
+      || !mergeSlice(builder, slice.get(LINKS_FIELD))) {
+    return arangodb::Result(
+      TRI_ERROR_INTERNAL,
+      std::string("failed to construct link update directive while updating IResearch View '") + name() + "'"
+    );
   }
 
-  return res;
+  builder.close();
+
+  return updateLinks(collections, *vocbase, *this, builder.slice());
 }
 
 void IResearchView::registerFlushCallback() {
@@ -2234,7 +2320,7 @@ void IResearchView::verifyKnownCollections() {
     State state;
 
     if (!appendKnownCollections(cids, *snapshot(state, true))) {
-      LOG_TOPIC(ERR, iresearch::IResearchFeature::IRESEARCH)
+      LOG_TOPIC(ERR, IResearchFeature::IRESEARCH)
         << "failed to collect collection IDs for IResearch view '" << id() << "'";
 
       return;
@@ -2246,7 +2332,7 @@ void IResearchView::verifyKnownCollections() {
 
     if (!collection) {
       // collection no longer exists, drop it and move on
-      LOG_TOPIC(TRACE, arangodb::iresearch::IResearchFeature::IRESEARCH)
+      LOG_TOPIC(TRACE, IResearchFeature::IRESEARCH)
         << "collection '" << cid
         << "' no longer exists! removing from IResearch view '"
         << id() << "'";
@@ -2255,7 +2341,7 @@ void IResearchView::verifyKnownCollections() {
       // see if the link still exists, otherwise drop and move on
       auto link = findFirstMatchingLink(*collection, *this);
       if (!link) {
-        LOG_TOPIC(TRACE, arangodb::iresearch::IResearchFeature::IRESEARCH)
+        LOG_TOPIC(TRACE, IResearchFeature::IRESEARCH)
           << "collection '" << cid
           << "' no longer linked! removing from IResearch view '"
           << id() << "'";
@@ -2270,4 +2356,4 @@ NS_END // arangodb
 
 // -----------------------------------------------------------------------------
 // --SECTION--                                                       END-OF-FILE
-// -----------------------------------------------------------------------------
+// -----------------------------------------------------------------------------

arangod/IResearch/IResearchView.h

@@ -26,8 +26,6 @@
 
 #include "Containers.h"
 #include "IResearchViewMeta.h"
-#include "Basics/ReadWriteLock.h"
-#include "Basics/WriteLocker.h"
 #include "VocBase/LogicalDataSource.h"
 #include "VocBase/LocalDocumentId.h"
 #include "VocBase/LogicalView.h"
@@ -41,6 +39,7 @@
 
 NS_BEGIN(arangodb)
 
+class DatabasePathFeature; // forward declaration
 class TransactionState; // forward declaration
 class ViewIterator; // forward declaration
 
@@ -112,6 +111,7 @@ class PrimaryKeyIndexReader: public irs::index_reader {
 class IResearchView final: public arangodb::DBServerLogicalView,
                            public arangodb::FlushTransaction {
  public:
+
   ///////////////////////////////////////////////////////////////////////////////
   /// @brief AsyncValue holding the view itself, modifiable by IResearchView
   ///////////////////////////////////////////////////////////////////////////////
@@ -147,10 +147,19 @@ class IResearchView final: public arangodb::DBServerLogicalView,
   ////////////////////////////////////////////////////////////////////////////////
   /// @brief remove all documents matching collection 'cid' from this IResearch
   ///        View and the underlying IResearch stores
-  ///        also remove 'cid' from the runtime list of tracked collection IDs
+  ///        also remove 'cid' from the persisted list of tracked collection IDs
   ////////////////////////////////////////////////////////////////////////////////
   int drop(TRI_voc_cid_t cid);
 
+  ////////////////////////////////////////////////////////////////////////////////
+  /// @brief acquire locks on the specified 'cid' during read-transactions
+  ///        allowing retrieval of documents contained in the aforementioned
+  ///        collection
+  ///        also track 'cid' via the persisted list of tracked collection IDs
+  /// @return the 'cid' was newly added to the IResearch View
+  ////////////////////////////////////////////////////////////////////////////////
+  bool emplace(TRI_voc_cid_t cid);
+
   ////////////////////////////////////////////////////////////////////////////////
   /// @brief insert a document into this IResearch View and the underlying
   ///        IResearch stores
@@ -246,20 +255,7 @@ class IResearchView final: public arangodb::DBServerLogicalView,
   ////////////////////////////////////////////////////////////////////////////////
   static arangodb::LogicalDataSource::Type const& type() noexcept;
 
-  void toVelocyPack(
+  using LogicalView::updateProperties;
-    velocypack::Builder& result,
-    bool includeProperties,
-    bool includeSystem
-  ) const override;
-
-  ///////////////////////////////////////////////////////////////////////////////
-  /// @brief called when a view's properties are updated (i.e. delta-modified)
-  ///////////////////////////////////////////////////////////////////////////////
-  arangodb::Result updateProperties(
-    arangodb::velocypack::Slice const& slice,
-    bool partialUpdate,
-    bool doSync
-  ) override;
 
   ///////////////////////////////////////////////////////////////////////////////
   /// @brief visit all collection IDs that were added to the view
@@ -267,7 +263,28 @@ class IResearchView final: public arangodb::DBServerLogicalView,
   ///////////////////////////////////////////////////////////////////////////////
   bool visitCollections(CollectionVisitor const& visitor) const override;
 
+ protected:
+
+  //////////////////////////////////////////////////////////////////////////////
+  /// @brief fill and return a JSON description of a IResearchView object
+  ///        only fields describing the view itself, not 'link' descriptions
+  //////////////////////////////////////////////////////////////////////////////
+  void getPropertiesVPack(
+    arangodb::velocypack::Builder& builder,
+    bool forPersistence
+  ) const override;
+
+  //////////////////////////////////////////////////////////////////////////////
+  /// @brief called when a view's properties are updated (i.e. delta-modified)
+  //////////////////////////////////////////////////////////////////////////////
+  arangodb::Result updateProperties(
+    arangodb::velocypack::Slice const& slice,
+    bool partialUpdate
+  ) override;
+
  private:
+  DECLARE_SPTR(LogicalView);
+
   struct DataStore {
     irs::directory::ptr _directory;
     irs::directory_reader _reader;
@@ -317,19 +334,10 @@ class IResearchView final: public arangodb::DBServerLogicalView,
   IResearchView(
     TRI_vocbase_t* vocbase,
     arangodb::velocypack::Slice const& info,
-    irs::utf8_path&& persistedPath,
+    arangodb::DatabasePathFeature const& dbPathFeature,
     bool isNew
   );
 
-  ///////////////////////////////////////////////////////////////////////////////
-  /// @brief fill and return a JSON description of a IResearchView object
-  ///        only fields describing the view itself, not 'link' descriptions
-  ////////////////////////////////////////////////////////////////////////////////
-  void getPropertiesVPack(
-    arangodb::velocypack::Builder& builder,
-    bool forPersistence
-  ) const;
-
   //////////////////////////////////////////////////////////////////////////////
   /// @brief Called in post-recovery to remove any dangling documents old links
   //////////////////////////////////////////////////////////////////////////////
@@ -365,11 +373,8 @@ class IResearchView final: public arangodb::DBServerLogicalView,
   std::function<void(arangodb::TransactionState& state)> _trxReadCallback; // for snapshot(...)
   std::function<void(arangodb::TransactionState& state)> _trxWriteCallback; // for insert(...)/remove(...)
   std::atomic<bool> _inRecovery;
-
-  // FIXME came from "LogicalView", check whether it needs to be there
-  mutable basics::ReadWriteLock _infoLock;  // lock protecting the properties
 };
 
 NS_END // iresearch
 NS_END // arangodb
-#endif
+#endif

arangod/IResearch/IResearchViewMeta.h

@@ -107,7 +107,7 @@ struct IResearchViewMeta {
     explicit Mask(bool mask = false) noexcept;
   };
 
-  std::unordered_set<TRI_voc_cid_t> _collections; // collection links added to this view via view property modification (may contain no-longer valid cids)
+  std::unordered_set<TRI_voc_cid_t> _collections; // collection links added to this view via IResearchLink creation (may contain no-longer valid cids)
   CommitMeta _commit;
   std::locale _locale; // locale used for ordering processed attribute names
   size_t _threadsMaxIdle; // maximum idle number of threads for single-run tasks

arangod/MMFiles/MMFilesEngine.cpp

@@ -2063,7 +2063,7 @@ TRI_vocbase_t* MMFilesEngine::openExistingDatabase(TRI_voc_tick_t id,
 
       auto const viewPath = readPath(it);
 
-      if (!viewPath.empty()) {
+      if (viewPath.empty()) {
         THROW_ARANGO_EXCEPTION_MESSAGE(
           TRI_ERROR_BAD_PARAMETER,
           "view path cannot be empty"

arangod/RestHandler/RestViewHandler.cpp

@@ -124,7 +124,7 @@ void RestViewHandler::createView() {
     if (view != nullptr) {
       VPackBuilder props;
       props.openObject();
-      view->toVelocyPack(props, false, false);
+      view->toVelocyPack(props, true, false);
       props.close();
       generateResult(rest::ResponseCode::CREATED, props.slice());
     } else {
@@ -314,9 +314,9 @@ void RestViewHandler::getViewProperties(std::string const& name) {
   if (view.get() != nullptr) {
     VPackBuilder props;
     props.openObject();
-    view->toVelocyPack(props, false, false);
+    view->toVelocyPack(props, true, false);
     props.close();
-    generateResult(rest::ResponseCode::OK, props.slice());
+    generateResult(rest::ResponseCode::OK, props.slice().get("properties"));
   } else {
     generateError(rest::ResponseCode::NOT_FOUND,
                   TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND);

arangod/VocBase/LogicalView.cpp

@@ -60,9 +60,7 @@ TRI_voc_cid_t ReadPlanId(VPackSlice info, TRI_voc_cid_t vid) {
   return vid;
 }
 
-}  // namespace
+/*static*/ TRI_voc_cid_t ReadViewId(VPackSlice info) {
-
-/*static*/ TRI_voc_cid_t LogicalView::readViewId(VPackSlice info) {
   if (!info.isObject()) {
     // ERROR CASE
     return 0;
@@ -83,10 +81,42 @@ TRI_voc_cid_t ReadPlanId(VPackSlice info, TRI_voc_cid_t vid) {
   return id;
 }
 
+} // namespace
+
 // -----------------------------------------------------------------------------
 // --SECTION--                                                       LogicalView
 // -----------------------------------------------------------------------------
 
+// @brief Constructor used in coordinator case.
+// The Slice contains the part of the plan that
+// is relevant for this view
+LogicalView::LogicalView(TRI_vocbase_t* vocbase, VPackSlice const& info)
+    : LogicalDataSource(
+        category(),
+        LogicalDataSource::Type::emplace(
+          arangodb::basics::VelocyPackHelper::getStringRef(info, "type", "")
+        ),
+        vocbase,
+        ReadViewId(info),
+        ReadPlanId(info, 0),
+        arangodb::basics::VelocyPackHelper::getStringValue(info, "name", ""),
+        Helper::readBooleanValue(info, "deleted", false)
+      ) {
+  if (!TRI_vocbase_t::IsAllowedName(info)) {
+    THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_ILLEGAL_NAME);
+  }
+
+  if (!id()) {
+    THROW_ARANGO_EXCEPTION_MESSAGE(
+      TRI_ERROR_BAD_PARAMETER,
+      "got invalid view identifier while constructing LogicalView"
+    );
+  }
+
+  // update server's tick value
+  TRI_UpdateTickServer(static_cast<TRI_voc_tick_t>(id()));
+}
+
 /*static*/ std::shared_ptr<LogicalView> LogicalView::create(
     TRI_vocbase_t& vocbase,
     velocypack::Slice definition,
@@ -122,29 +152,6 @@ TRI_voc_cid_t ReadPlanId(VPackSlice info, TRI_voc_cid_t vid) {
   return category;
 }
 
-// @brief Constructor used in coordinator case.
-// The Slice contains the part of the plan that
-// is relevant for this view
-LogicalView::LogicalView(TRI_vocbase_t* vocbase, VPackSlice const& info)
-    : LogicalDataSource(
-        category(),
-        LogicalDataSource::Type::emplace(
-          arangodb::basics::VelocyPackHelper::getStringRef(info, "type", "")
-        ),
-        vocbase,
-        LogicalView::readViewId(info),
-        ReadPlanId(info, 0),
-        arangodb::basics::VelocyPackHelper::getStringValue(info, "name", ""),
-        Helper::readBooleanValue(info, "deleted", false)
-      ) {
-  if (!TRI_vocbase_t::IsAllowedName(info)) {
-    THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_ILLEGAL_NAME);
-  }
-
-  // update server's tick value
-  TRI_UpdateTickServer(static_cast<TRI_voc_tick_t>(id()));
-}
-
 // -----------------------------------------------------------------------------
 // --SECTION--                                               DBServerLogicalView
 // -----------------------------------------------------------------------------
@@ -165,6 +172,13 @@ DBServerLogicalView::~DBServerLogicalView() {
   }
 }
 
+void DBServerLogicalView::drop() {
+  TRI_ASSERT(!ServerState::instance()->isCoordinator());
+  StorageEngine* engine = EngineSelectorFeature::ENGINE;
+  TRI_ASSERT(engine);
+  engine->dropView(vocbase(), this);
+}
+
 void DBServerLogicalView::open() {
   // Coordinators are not allowed to have local views!
   TRI_ASSERT(!ServerState::instance()->isCoordinator());
@@ -179,13 +193,6 @@ void DBServerLogicalView::open() {
   _isNew = false;
 }
 
-void DBServerLogicalView::drop() {
-  TRI_ASSERT(!ServerState::instance()->isCoordinator());
-  StorageEngine* engine = EngineSelectorFeature::ENGINE;
-  TRI_ASSERT(engine);
-  engine->dropView(vocbase(), this);
-}
-
 Result DBServerLogicalView::rename(std::string&& newName, bool doSync) {
   auto oldName = name();
 
@@ -213,7 +220,7 @@ Result DBServerLogicalView::rename(std::string&& newName, bool doSync) {
 
 void DBServerLogicalView::toVelocyPack(
     velocypack::Builder &result,
-    bool /*includeProperties*/,
+    bool includeProperties,
     bool includeSystem
 ) const {
   // We write into an open object
@@ -233,20 +240,42 @@ void DBServerLogicalView::toVelocyPack(
 
     // storage engine related properties
     StorageEngine* engine = EngineSelectorFeature::ENGINE;
-    TRI_ASSERT(engine );
+    TRI_ASSERT(engine);
     engine->getViewProperties(vocbase(), this, result);
   }
+
+  if (includeProperties) {
+    // implementation Information
+    result.add("properties", VPackValue(VPackValueType::Object));
+    // note: includeSystem and forPersistence are not 100% synonymous,
+    // however, for our purposes this is an okay mapping; we only set
+    // includeSystem if we are persisting the properties
+    getPropertiesVPack(result, includeSystem);
+    result.close();
+  }
+
+  TRI_ASSERT(result.isOpenObject()); // We leave the object open
 }
 
 arangodb::Result DBServerLogicalView::updateProperties(
-    VPackSlice const& /*slice*/,
+    VPackSlice const& slice,
-    bool /*partialUpdate*/,
+    bool partialUpdate,
     bool doSync
 ) {
+  auto res = updateProperties(slice, partialUpdate);
+
+  if (!res.ok()) {
+    return res;
+  }
+
   // after this call the properties are stored
   StorageEngine* engine = EngineSelectorFeature::ENGINE;
   TRI_ASSERT(engine);
 
+  if (engine->inRecovery()) {
+    return arangodb::Result(); // do not modify engine while in recovery
+  }
+
   try {
     engine->changeView(vocbase(), id(), this, doSync);
   } catch (arangodb::basics::Exception const& e) {
@@ -260,4 +289,4 @@ arangodb::Result DBServerLogicalView::updateProperties(
 
 // -----------------------------------------------------------------------------
 // --SECTION--                                                       END-OF-FILE
-// -----------------------------------------------------------------------------
+// -----------------------------------------------------------------------------

arangod/VocBase/LogicalView.h

@@ -60,12 +60,6 @@ class LogicalView : public LogicalDataSource {
   //////////////////////////////////////////////////////////////////////////////
   static Category const& category() noexcept;
 
-  //////////////////////////////////////////////////////////////////////////////
-  /// @brief invoke visitor on all collections that a view will return
-  /// @return visitation was successful
-  //////////////////////////////////////////////////////////////////////////////
-  virtual bool visitCollections(CollectionVisitor const& visitor) const = 0;
-
   //////////////////////////////////////////////////////////////////////////////
   /// @brief opens an existing view when the server is restarted
   //////////////////////////////////////////////////////////////////////////////
@@ -102,14 +96,20 @@ class LogicalView : public LogicalDataSource {
     bool doSync
   ) = 0;
 
- protected:
+  //////////////////////////////////////////////////////////////////////////////
-  static TRI_voc_cid_t readViewId(velocypack::Slice slice);
+  /// @brief invoke visitor on all collections that a view will return
+  /// @return visitation was successful
+  //////////////////////////////////////////////////////////////////////////////
+  virtual bool visitCollections(CollectionVisitor const& visitor) const = 0;
 
+ protected:
   LogicalView(TRI_vocbase_t* vocbase, velocypack::Slice const& definition);
 
  private:
   // FIXME seems to be ugly
   friend struct ::TRI_vocbase_t;
+
+  // ensure LogicalDataSource members (e.g. _deleted/_name) are not modified asynchronously
   mutable basics::ReadWriteLock _lock;
 }; // LogicalView
 
@@ -135,10 +135,10 @@ class DBServerLogicalView : public LogicalView {
  public:
   ~DBServerLogicalView() override;
 
-  void open() override;
-
   void drop() override;
 
+  void open() override;
+
   Result rename(
     std::string&& newName,
     bool doSync
@@ -148,13 +148,13 @@ class DBServerLogicalView : public LogicalView {
     velocypack::Builder& result,
     bool includeProperties,
     bool includeSystem
-  ) const override;
+  ) const override final;
 
   arangodb::Result updateProperties(
     velocypack::Slice const& properties,
     bool partialUpdate,
     bool doSync
-  ) override;
+  ) override final;
 
  protected:
   DBServerLogicalView(
@@ -163,10 +163,26 @@ class DBServerLogicalView : public LogicalView {
     bool isNew
   );
 
+  //////////////////////////////////////////////////////////////////////////////
+  /// @brief fill and return a jSON description of a View object implementation
+  //////////////////////////////////////////////////////////////////////////////
+  virtual void getPropertiesVPack(
+    velocypack::Builder& builder,
+    bool forPersistence
+  ) const = 0;
+
+  ///////////////////////////////////////////////////////////////////////////////
+  /// @brief called when a view's properties are updated (i.e. delta-modified)
+  ///////////////////////////////////////////////////////////////////////////////
+  virtual arangodb::Result updateProperties(
+    velocypack::Slice const& slice,
+    bool partialUpdate
+  ) = 0;
+
  private:
   bool _isNew;
 }; // LogicalView
 
 }  // namespace arangodb
 
-#endif
+#endif

arangod/VocBase/vocbase.cpp

@@ -1357,7 +1357,7 @@ int TRI_vocbase_t::renameView(
   auto itr1 = _dataSourceByName.find(oldName);
 
   if (itr1 == _dataSourceByName.end()
-      || arangodb::LogicalView::category() == itr1->second->category()) {
+      || arangodb::LogicalView::category() != itr1->second->category()) {
     return TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND;
   }
 
@@ -1614,9 +1614,29 @@ std::shared_ptr<arangodb::LogicalView> TRI_vocbase_t::createViewWorker(
     THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_DUPLICATE_NAME);
   }
 
+  // Coordinators are not allowed to have local views!
+  TRI_ASSERT(!ServerState::instance()->isCoordinator());
+
+  StorageEngine* engine = EngineSelectorFeature::ENGINE;
+  TRI_ASSERT(engine);
+  arangodb::velocypack::Builder builder;
+  auto res = engine->getViews(this, builder);
+  TRI_ASSERT(TRI_ERROR_NO_ERROR == res);
+  auto slice  = builder.slice();
+  TRI_ASSERT(slice.isArray());
+  auto viewId = std::to_string(view->id());
+
+  // We have not yet persisted this view
+  for (auto entry: arangodb::velocypack::ArrayIterator(slice)) {
+    TRI_ASSERT(arangodb::basics::VelocyPackHelper::getStringRef(entry, "id", arangodb::velocypack::StringRef()).compare(viewId));
+  }
+
   registerView(basics::ConditionalLocking::DoNotLock, view);
 
   try {
+    // Let's try to persist it.
+    engine->createView(this, view->id(), view.get());
+
     // And lets open it.
     view->open();
 
@@ -1782,6 +1802,9 @@ int TRI_vocbase_t::dropView(std::shared_ptr<arangodb::LogicalView> view) {
   view->drop();
   unregisterView(view);
 
+  StorageEngine* engine = EngineSelectorFeature::ENGINE;
+  engine->dropView(this, view.get());
+
   locker.unlock();
   writeLocker.unlock();
 
@@ -2184,4 +2207,4 @@ TRI_voc_rid_t TRI_StringToRid(char const* p, size_t len, bool& isOld,
 
 // -----------------------------------------------------------------------------
 // --SECTION--                                                       END-OF-FILE
-// -----------------------------------------------------------------------------
+// -----------------------------------------------------------------------------

tests/IResearch/IResearchLink-test.cpp

@@ -168,6 +168,7 @@ TEST_CASE("IResearchLinkTest", "[iresearch][iresearch-link]") {
 SECTION("test_defaults") {
   // no view specified
   {
+    s.engine.views.clear();
     TRI_vocbase_t vocbase(TRI_vocbase_type_e::TRI_VOCBASE_TYPE_NORMAL, 1, "testVocbase");
     auto collectionJson = arangodb::velocypack::Parser::fromJson("{ \"name\": \"testCollection\" }");
     auto* logicalCollection = vocbase.createCollection(collectionJson->slice());
@@ -179,6 +180,7 @@ SECTION("test_defaults") {
 
   // no view can be found
   {
+    s.engine.views.clear();
     TRI_vocbase_t vocbase(TRI_vocbase_type_e::TRI_VOCBASE_TYPE_NORMAL, 1, "testVocbase");
     auto collectionJson = arangodb::velocypack::Parser::fromJson("{ \"name\": \"testCollection\" }");
     auto* logicalCollection = vocbase.createCollection(collectionJson->slice());
@@ -190,6 +192,7 @@ SECTION("test_defaults") {
 
   // valid link creation
   {
+    s.engine.views.clear();
     TRI_vocbase_t vocbase(TRI_vocbase_type_e::TRI_VOCBASE_TYPE_NORMAL, 1, "testVocbase");
     auto linkJson = arangodb::velocypack::Parser::fromJson("{ \"type\": \"arangosearch\", \"view\": 42 }");
     auto collectionJson = arangodb::velocypack::Parser::fromJson("{ \"name\": \"testCollection\" }");
@@ -242,6 +245,7 @@ SECTION("test_defaults") {
 
   // ensure jSON is still valid after unload()
   {
+    s.engine.views.clear();
     TRI_vocbase_t vocbase(TRI_vocbase_type_e::TRI_VOCBASE_TYPE_NORMAL, 1, "testVocbase");
     auto linkJson = arangodb::velocypack::Parser::fromJson("{ \"type\": \"arangosearch\", \"view\": 42 }");
     auto collectionJson = arangodb::velocypack::Parser::fromJson("{ \"name\": \"testCollection\" }");
@@ -310,6 +314,269 @@ SECTION("test_defaults") {
   }
 }
 
+SECTION("test_init") {
+  // collection registered with view (collection initially not in view)
+  {
+    auto collectionJson = arangodb::velocypack::Parser::fromJson("{ \"name\": \"testCollection\", \"id\": 100 }");
+    auto linkJson = arangodb::velocypack::Parser::fromJson("{ \"type\": \"arangosearch\", \"view\": 42 }");
+    auto viewJson = arangodb::velocypack::Parser::fromJson("{ \"name\": \"testView\", \"id\": 42, \"type\": \"arangosearch\" }");
+    TRI_vocbase_t vocbase(TRI_vocbase_type_e::TRI_VOCBASE_TYPE_NORMAL, 1, "testVocbase");
+    auto* logicalCollection = vocbase.createCollection(collectionJson->slice());
+    REQUIRE((nullptr != logicalCollection));
+    auto logicalView = vocbase.createView(viewJson->slice(), 0);
+    REQUIRE((false == !logicalView));
+
+    // no collections in view before
+    {
+      std::unordered_set<TRI_voc_cid_t> expected;
+      std::set<TRI_voc_cid_t> actual;
+
+      CHECK((logicalView->visitCollections([&actual](TRI_voc_cid_t cid)->bool { actual.emplace(cid); return true; })));
+
+      for (auto& cid: expected) {
+        CHECK((1 == actual.erase(cid)));
+      }
+
+      CHECK((actual.empty()));
+    }
+
+    auto link = arangodb::iresearch::IResearchMMFilesLink::make(1, logicalCollection, linkJson->slice());
+    CHECK((false == !link));
+
+    // collection in view after
+    {
+      std::unordered_set<TRI_voc_cid_t> expected = { 100 };
+      std::set<TRI_voc_cid_t> actual;
+
+      CHECK((logicalView->visitCollections([&actual](TRI_voc_cid_t cid)->bool { actual.emplace(cid); return true; })));
+
+      for (auto& cid: expected) {
+        CHECK((1 == actual.erase(cid)));
+      }
+
+      CHECK((actual.empty()));
+    }
+
+    link.reset();
+
+    // collection not in view on destruct
+    {
+      std::unordered_set<TRI_voc_cid_t> expected;
+      std::set<TRI_voc_cid_t> actual;
+
+      CHECK((logicalView->visitCollections([&actual](TRI_voc_cid_t cid)->bool { actual.emplace(cid); return true; })));
+
+      for (auto& cid: expected) {
+        CHECK((1 == actual.erase(cid)));
+      }
+
+      CHECK((actual.empty()));
+    }
+  }
+
+  // collection registered with view (collection initially is in view)
+  {
+    auto collectionJson = arangodb::velocypack::Parser::fromJson("{ \"name\": \"testCollection\", \"id\": 101 }");
+    auto linkJson = arangodb::velocypack::Parser::fromJson("{ \"type\": \"arangosearch\", \"view\": 43 }");
+    auto viewJson = arangodb::velocypack::Parser::fromJson("{ \"name\": \"testView\", \"id\": 43, \"type\": \"arangosearch\", \"properties\": { \"collections\": [ 101 ] } }");
+    TRI_vocbase_t vocbase(TRI_vocbase_type_e::TRI_VOCBASE_TYPE_NORMAL, 1, "testVocbase");
+    auto* logicalCollection = vocbase.createCollection(collectionJson->slice());
+    REQUIRE((nullptr != logicalCollection));
+    auto logicalView = vocbase.createView(viewJson->slice(), 0);
+    REQUIRE((false == !logicalView));
+
+    // collection in view before
+    {
+      std::unordered_set<TRI_voc_cid_t> expected = { 101 };
+      std::set<TRI_voc_cid_t> actual;
+
+      CHECK((logicalView->visitCollections([&actual](TRI_voc_cid_t cid)->bool { actual.emplace(cid); return true; })));
+
+      for (auto& cid: expected) {
+        CHECK((1 == actual.erase(cid)));
+      }
+
+      CHECK((actual.empty()));
+    }
+
+    auto link = arangodb::iresearch::IResearchMMFilesLink::make(1, logicalCollection, linkJson->slice());
+    CHECK((false == !link));
+
+    // collection in view after
+    {
+      std::unordered_set<TRI_voc_cid_t> expected = { 101 };
+      std::set<TRI_voc_cid_t> actual;
+
+      CHECK((logicalView->visitCollections([&actual](TRI_voc_cid_t cid)->bool { actual.emplace(cid); return true; })));
+
+      for (auto& cid: expected) {
+        CHECK((1 == actual.erase(cid)));
+      }
+
+      CHECK((actual.empty()));
+    }
+
+    link.reset();
+
+    // collection still in view on destruct
+    {
+      std::unordered_set<TRI_voc_cid_t> expected = { 101 };
+      std::set<TRI_voc_cid_t> actual;
+
+      CHECK((logicalView->visitCollections([&actual](TRI_voc_cid_t cid)->bool { actual.emplace(cid); return true; })));
+
+      for (auto& cid: expected) {
+        CHECK((1 == actual.erase(cid)));
+      }
+
+      CHECK((actual.empty()));
+    }
+  }
+}
+
+SECTION("test_drop") {
+  // collection drop (removes collection from view) subsequent destroy does not touch view
+  {
+    auto collectionJson = arangodb::velocypack::Parser::fromJson("{ \"name\": \"testCollection\", \"id\": 100 }");
+    auto linkJson = arangodb::velocypack::Parser::fromJson("{ \"type\": \"arangosearch\", \"view\": 42 }");
+    auto viewJson = arangodb::velocypack::Parser::fromJson("{ \"name\": \"testView\", \"id\": 42, \"type\": \"arangosearch\" }");
+    TRI_vocbase_t vocbase(TRI_vocbase_type_e::TRI_VOCBASE_TYPE_NORMAL, 1, "testVocbase");
+    auto* logicalCollection = vocbase.createCollection(collectionJson->slice());
+    REQUIRE((nullptr != logicalCollection));
+    auto logicalView = vocbase.createView(viewJson->slice(), 0);
+    REQUIRE((false == !logicalView));
+
+    auto link0 = arangodb::iresearch::IResearchMMFilesLink::make(1, logicalCollection, linkJson->slice());
+    CHECK((false == !link0));
+
+    // collection in view before
+    {
+      std::unordered_set<TRI_voc_cid_t> expected = { 100 };
+      std::set<TRI_voc_cid_t> actual;
+
+      CHECK((logicalView->visitCollections([&actual](TRI_voc_cid_t cid)->bool { actual.emplace(cid); return true; })));
+
+      for (auto& cid: expected) {
+        CHECK((1 == actual.erase(cid)));
+      }
+
+      CHECK((actual.empty()));
+    }
+
+    CHECK((TRI_ERROR_NO_ERROR == link0->drop()));
+
+    // collection not in view after
+    {
+      std::unordered_set<TRI_voc_cid_t> expected;
+      std::set<TRI_voc_cid_t> actual;
+
+      CHECK((logicalView->visitCollections([&actual](TRI_voc_cid_t cid)->bool { actual.emplace(cid); return true; })));
+
+      for (auto& cid: expected) {
+        CHECK((1 == actual.erase(cid)));
+      }
+
+      CHECK((actual.empty()));
+    }
+
+    auto link1 = arangodb::iresearch::IResearchMMFilesLink::make(1, logicalCollection, linkJson->slice());
+    CHECK((false == !link1));
+
+    // collection in view before (new link)
+    {
+      std::unordered_set<TRI_voc_cid_t> expected = { 100 };
+      std::set<TRI_voc_cid_t> actual;
+
+      CHECK((logicalView->visitCollections([&actual](TRI_voc_cid_t cid)->bool { actual.emplace(cid); return true; })));
+
+      for (auto& cid: expected) {
+        CHECK((1 == actual.erase(cid)));
+      }
+
+      CHECK((actual.empty()));
+    }
+
+    link0.reset();
+
+    // collection in view after (new link) subsequent destroy does not touch view
+    {
+      std::unordered_set<TRI_voc_cid_t> expected = { 100 };
+      std::set<TRI_voc_cid_t> actual;
+
+      CHECK((logicalView->visitCollections([&actual](TRI_voc_cid_t cid)->bool { actual.emplace(cid); return true; })));
+
+      for (auto& cid: expected) {
+        CHECK((1 == actual.erase(cid)));
+      }
+
+      CHECK((actual.empty()));
+    }
+  }
+}
+
+SECTION("test_unload") {
+  // index unload does not touch the view (subsequent destroy)
+  {
+    auto collectionJson = arangodb::velocypack::Parser::fromJson("{ \"name\": \"testCollection\", \"id\": 100 }");
+    auto linkJson = arangodb::velocypack::Parser::fromJson("{ \"type\": \"arangosearch\", \"view\": 42 }");
+    auto viewJson = arangodb::velocypack::Parser::fromJson("{ \"name\": \"testView\", \"id\": 42, \"type\": \"arangosearch\" }");
+    TRI_vocbase_t vocbase(TRI_vocbase_type_e::TRI_VOCBASE_TYPE_NORMAL, 1, "testVocbase");
+    auto* logicalCollection = vocbase.createCollection(collectionJson->slice());
+    REQUIRE((nullptr != logicalCollection));
+    auto logicalView = vocbase.createView(viewJson->slice(), 0);
+    REQUIRE((false == !logicalView));
+
+    auto link = arangodb::iresearch::IResearchMMFilesLink::make(1, logicalCollection, linkJson->slice());
+    CHECK((false == !link));
+
+    // collection in view before
+    {
+      std::unordered_set<TRI_voc_cid_t> expected = { 100 };
+      std::set<TRI_voc_cid_t> actual;
+
+      CHECK((logicalView->visitCollections([&actual](TRI_voc_cid_t cid)->bool { actual.emplace(cid); return true; })));
+
+      for (auto& cid: expected) {
+        CHECK((1 == actual.erase(cid)));
+      }
+
+      CHECK((actual.empty()));
+    }
+
+    link->unload();
+
+    // collection in view after unload
+    {
+      std::unordered_set<TRI_voc_cid_t> expected = { 100 };
+      std::set<TRI_voc_cid_t> actual;
+
+      CHECK((logicalView->visitCollections([&actual](TRI_voc_cid_t cid)->bool { actual.emplace(cid); return true; })));
+
+      for (auto& cid: expected) {
+        CHECK((1 == actual.erase(cid)));
+      }
+
+      CHECK((actual.empty()));
+    }
+
+    link.reset();
+
+    // collection in view after destruct, subsequent destroy does not touch view
+    {
+      std::unordered_set<TRI_voc_cid_t> expected = { 100 };
+      std::set<TRI_voc_cid_t> actual;
+
+      CHECK((logicalView->visitCollections([&actual](TRI_voc_cid_t cid)->bool { actual.emplace(cid); return true; })));
+
+      for (auto& cid: expected) {
+        CHECK((1 == actual.erase(cid)));
+      }
+
+      CHECK((actual.empty()));
+    }
+  }
+}
+
 SECTION("test_write") {
   static std::vector<std::string> const EMPTY;
   auto doc0 = arangodb::velocypack::Parser::fromJson("{ \"abc\": \"def\" }");
@@ -388,4 +655,4 @@ SECTION("test_write") {
 
 // -----------------------------------------------------------------------------
 // --SECTION--                                                       END-OF-FILE
-// -----------------------------------------------------------------------------
+// -----------------------------------------------------------------------------

tests/IResearch/StorageEngineMock.cpp

@@ -544,8 +544,6 @@ arangodb::PhysicalCollection* PhysicalCollectionMock::clone(arangodb::LogicalCol
 }
 
 int PhysicalCollectionMock::close() {
-  before();
-
   for (auto& index: _indexes) {
     index->unload();
   }
@@ -943,6 +941,7 @@ arangodb::Result PhysicalCollectionMock::updateProperties(arangodb::velocypack::
   return arangodb::Result(TRI_ERROR_NO_ERROR); // assume mock collection updated OK
 }
 
+std::function<void()> StorageEngineMock::before = []()->void {};
 bool StorageEngineMock::inRecoveryResult = false;
 
 StorageEngineMock::StorageEngineMock()
@@ -956,10 +955,12 @@ arangodb::WalAccess const* StorageEngineMock::walAccess() const {
 }
 
 void StorageEngineMock::addAqlFunctions() {
+  before();
   // NOOP
 }
 
 void StorageEngineMock::addOptimizerRules() {
+  before();
   // NOOP
 }
 
@@ -975,8 +976,17 @@ void StorageEngineMock::changeCollection(TRI_vocbase_t* vocbase, TRI_voc_cid_t i
   // NOOP, assume physical collection changed OK
 }
 
-void StorageEngineMock::changeView(TRI_vocbase_t* vocbase, TRI_voc_cid_t id, arangodb::LogicalView const*, bool doSync) {
+void StorageEngineMock::changeView(TRI_vocbase_t* vocbase, TRI_voc_cid_t id, arangodb::LogicalView const* view, bool doSync) {
-  // does nothing
+  before();
+  TRI_ASSERT(vocbase);
+  TRI_ASSERT(view);
+  TRI_ASSERT(views.find(std::make_pair(vocbase->id(), view->id())) != views.end());
+  arangodb::velocypack::Builder builder;
+
+  builder.openObject();
+  view->toVelocyPack(builder, true, true);
+  builder.close();
+  views[std::make_pair(vocbase->id(), view->id())] = std::move(builder);
 }
 
 std::string StorageEngineMock::collectionPath(TRI_vocbase_t const* vocbase, TRI_voc_cid_t id) const {
@@ -1003,6 +1013,7 @@ arangodb::Result StorageEngineMock::createLoggerState(TRI_vocbase_t*, VPackBuild
 }
 
 arangodb::PhysicalCollection* StorageEngineMock::createPhysicalCollection(arangodb::LogicalCollection* collection, VPackSlice const& info) {
+  before();
   return new PhysicalCollectionMock(collection, info);
 }
 
@@ -1016,6 +1027,7 @@ arangodb::TransactionCollection* StorageEngineMock::createTransactionCollection(
 }
 
 arangodb::transaction::ContextData* StorageEngineMock::createTransactionContextData() {
+  before();
   return new ContextDataMock();
 }
 
@@ -1029,18 +1041,22 @@ arangodb::TransactionState* StorageEngineMock::createTransactionState(TRI_vocbas
 }
 
 void StorageEngineMock::createView(TRI_vocbase_t* vocbase, TRI_voc_cid_t id, arangodb::LogicalView const*) {
+  before();
   // NOOP, assume physical view created OK
 }
 
 void StorageEngineMock::getViewProperties(TRI_vocbase_t* vocbase, arangodb::LogicalView const* view, VPackBuilder& builder) {
+  before();
  // NOOP
 }
 
 TRI_voc_tick_t StorageEngineMock::currentTick() const {
+  before();
   return TRI_CurrentTickServer();
 }
 
 std::string StorageEngineMock::databasePath(TRI_vocbase_t const* vocbase) const {
+  before();
   return ""; // no valid path filesystem persisted, return empty string
 }
 
@@ -1049,7 +1065,8 @@ void StorageEngineMock::destroyCollection(TRI_vocbase_t* vocbase, arangodb::Logi
 }
 
 void StorageEngineMock::destroyView(TRI_vocbase_t* vocbase, arangodb::LogicalView* view) noexcept {
-  // NOOP
+  before();
+  // NOOP, assume physical view destroyed OK
 }
 
 arangodb::Result StorageEngineMock::dropCollection(TRI_vocbase_t* vocbase, arangodb::LogicalCollection* collection) {
@@ -1061,12 +1078,13 @@ arangodb::Result StorageEngineMock::dropDatabase(TRI_vocbase_t*) {
   return arangodb::Result();
 }
 
-arangodb::Result StorageEngineMock::renameView(TRI_vocbase_t* vocbase, std::shared_ptr<arangodb::LogicalView>,
+arangodb::Result StorageEngineMock::dropView(TRI_vocbase_t* vocbase, arangodb::LogicalView* view) {
-                                               std::string const& newName) {
+  before();
-  return arangodb::Result(TRI_ERROR_NO_ERROR); // assume mock view renames OK
+  TRI_ASSERT(vocbase);
-}
+  TRI_ASSERT(view);
+  TRI_ASSERT(views.find(std::make_pair(vocbase->id(), view->id())) != views.end());
+  views.erase(std::make_pair(vocbase->id(), view->id()));
 
-arangodb::Result StorageEngineMock::dropView(TRI_vocbase_t*, arangodb::LogicalView*) {
   return arangodb::Result(TRI_ERROR_NO_ERROR); // assume mock view dropped OK
 }
 
@@ -1094,6 +1112,7 @@ int StorageEngineMock::getCollectionsAndIndexes(TRI_vocbase_t* vocbase, arangodb
 }
 
 void StorageEngineMock::getDatabases(arangodb::velocypack::Builder& result) {
+  before();
   arangodb::velocypack::Builder system;
 
   system.openObject();
@@ -1107,19 +1126,29 @@ void StorageEngineMock::getDatabases(arangodb::velocypack::Builder& result) {
 }
 
 arangodb::velocypack::Builder StorageEngineMock::getReplicationApplierConfiguration(TRI_vocbase_t* vocbase, int& result) {
+  before();
   result = TRI_ERROR_FILE_NOT_FOUND; // assume no ReplicationApplierConfiguration for vocbase
 
   return arangodb::velocypack::Builder();
 }
 
 arangodb::velocypack::Builder StorageEngineMock::getReplicationApplierConfiguration(int& result) {
+  before();
   result = TRI_ERROR_FILE_NOT_FOUND;
+
   return arangodb::velocypack::Builder();
 }
 
 int StorageEngineMock::getViews(TRI_vocbase_t* vocbase, arangodb::velocypack::Builder& result) {
-  TRI_ASSERT(false);
+  result.openArray();
-  return TRI_ERROR_INTERNAL;
+
+  for (auto& entry: views) {
+    result.add(entry.second.slice());
+  }
+
+  result.close();
+
+  return TRI_ERROR_NO_ERROR;
 }
 
 arangodb::Result StorageEngineMock::handleSyncKeys(arangodb::DatabaseInitialSyncer&, arangodb::LogicalCollection*, std::string const&) {
@@ -1137,6 +1166,8 @@ arangodb::Result StorageEngineMock::lastLogger(TRI_vocbase_t*, std::shared_ptr<a
 }
 
 TRI_vocbase_t* StorageEngineMock::openDatabase(arangodb::velocypack::Slice const& args, bool isUpgrade, int& status) {
+  before();
+
   if (!args.isObject() || !args.hasKey("name") || !args.get("name").isString()) {
     status = TRI_ERROR_ARANGO_DATABASE_NAME_INVALID;
 
@@ -1155,10 +1186,22 @@ TRI_vocbase_t* StorageEngineMock::openDatabase(arangodb::velocypack::Slice const
 }
 
 arangodb::Result StorageEngineMock::persistCollection(TRI_vocbase_t* vocbase, arangodb::LogicalCollection const* collection) {
+  before();
   return arangodb::Result(TRI_ERROR_NO_ERROR); // assume mock collection persisted OK
 }
 
-arangodb::Result StorageEngineMock::persistView(TRI_vocbase_t* vocbase, arangodb::LogicalView const*) {
+arangodb::Result StorageEngineMock::persistView(TRI_vocbase_t* vocbase, arangodb::LogicalView const* view) {
+  before();
+  TRI_ASSERT(vocbase);
+  TRI_ASSERT(view);
+  TRI_ASSERT(views.find(std::make_pair(vocbase->id(), view->id())) == views.end()); // called after createView()
+  arangodb::velocypack::Builder builder;
+
+  builder.openObject();
+  view->toVelocyPack(builder, true, true);
+  builder.close();
+  views[std::make_pair(vocbase->id(), view->id())] = std::move(builder);
+
   return arangodb::Result(TRI_ERROR_NO_ERROR); // assume mock view persisted OK
 }
 
@@ -1167,10 +1210,12 @@ void StorageEngineMock::prepareDropDatabase(TRI_vocbase_t* vocbase, bool useWrit
 }
 
 TRI_voc_tick_t StorageEngineMock::releasedTick() const {
+  before();
   return _releasedTick;
 }
 
 void StorageEngineMock::releaseTick(TRI_voc_tick_t tick) {
+  before();
   _releasedTick = tick;
 }
 
@@ -1189,6 +1234,21 @@ arangodb::Result StorageEngineMock::renameCollection(TRI_vocbase_t* vocbase, ara
   return arangodb::Result(TRI_ERROR_INTERNAL);
 }
 
+arangodb::Result StorageEngineMock::renameView(TRI_vocbase_t* vocbase, std::shared_ptr<arangodb::LogicalView> view, std::string const& newName) {
+  before();
+  TRI_ASSERT(vocbase);
+  TRI_ASSERT(view);
+  TRI_ASSERT(views.find(std::make_pair(vocbase->id(), view->id())) != views.end());
+  arangodb::velocypack::Builder builder;
+
+  builder.openObject();
+  view->toVelocyPack(builder, true, true);
+  builder.close();
+  views[std::make_pair(vocbase->id(), view->id())] = std::move(builder);
+
+  return arangodb::Result(TRI_ERROR_NO_ERROR); // assume mock view renames OK
+}
+
 int StorageEngineMock::saveReplicationApplierConfiguration(TRI_vocbase_t*, arangodb::velocypack::Slice, bool) {
   TRI_ASSERT(false);
   return TRI_ERROR_NO_ERROR;
@@ -1200,10 +1260,12 @@ int StorageEngineMock::saveReplicationApplierConfiguration(arangodb::velocypack:
 }
 
 int StorageEngineMock::shutdownDatabase(TRI_vocbase_t* vocbase) {
+  before();
   return TRI_ERROR_NO_ERROR; // assume shutdown successful
 }
 
 void StorageEngineMock::signalCleanup(TRI_vocbase_t* vocbase) {
+  before();
   // NOOP, assume cleanup thread signaled OK
 }
 
@@ -1213,6 +1275,7 @@ bool StorageEngineMock::supportsDfdb() const {
 }
 
 void StorageEngineMock::unloadCollection(TRI_vocbase_t* vocbase, arangodb::LogicalCollection* collection) {
+  before();
   // NOOP assume collection unloaded OK
 }
 
@@ -1357,4 +1420,4 @@ bool TransactionStateMock::hasFailedOperations() const {
 
 // -----------------------------------------------------------------------------
 // --SECTION--                                                       END-OF-FILE
-// -----------------------------------------------------------------------------
+// -----------------------------------------------------------------------------

tests/IResearch/StorageEngineMock.h

@@ -134,7 +134,9 @@ class TransactionStateMock: public arangodb::TransactionState {
 
 class StorageEngineMock: public arangodb::StorageEngine {
  public:
+  static std::function<void()> before;
   static bool inRecoveryResult;
+  std::map<std::pair<TRI_voc_tick_t, TRI_voc_cid_t>, arangodb::velocypack::Builder> views;
   std::vector<std::unique_ptr<TRI_vocbase_t>> vocbases; // must allocate on heap because TRI_vocbase_t does not have a 'noexcept' move constructor
 
   StorageEngineMock();
@@ -207,4 +209,4 @@ class StorageEngineMock: public arangodb::StorageEngine {
   TRI_voc_tick_t _releasedTick;
 };
 
-#endif
+#endif

tests/VocBase/LogicalDataSource-test.cpp

@@ -64,6 +64,27 @@ SECTION("test_category") {
 
     CHECK((arangodb::LogicalCollection::category() == instance.category()));
   }
+
+  // LogicalView
+  {
+    class LogicalViewImpl: public arangodb::LogicalView {
+     public:
+      LogicalViewImpl(TRI_vocbase_t* vocbase, arangodb::velocypack::Slice const& definition)
+        : LogicalView(vocbase, definition) {
+      }
+      virtual void drop() override {}
+      virtual void open() override {}
+      virtual arangodb::Result rename(std::string&& newName, bool doSync) override { return arangodb::Result(); }
+      virtual void toVelocyPack(arangodb::velocypack::Builder& result, bool includeProperties, bool includeSystem) const override {}
+      virtual arangodb::Result updateProperties(arangodb::velocypack::Slice const& properties, bool partialUpdate, bool doSync) override { return arangodb::Result(); }
+      virtual bool visitCollections(CollectionVisitor const& visitor) const override { return true; }
+    };
+
+    auto json = arangodb::velocypack::Parser::fromJson("{ \"name\": \"testView\" }");
+    LogicalViewImpl instance(nullptr, json->slice());
+
+    CHECK((arangodb::LogicalView::category() == instance.category()));
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -74,4 +95,4 @@ SECTION("test_category") {
 
 // -----------------------------------------------------------------------------
 // --SECTION--                                                       END-OF-FILE
-// -----------------------------------------------------------------------------
+// -----------------------------------------------------------------------------