issue 465.5: when renaming a datasource use the object from within vocbase (#6413)

arangod/MMFiles/MMFilesWalRecoverState.cpp

@@ -713,16 +713,18 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker,
           }
         }
 
-        int res = vocbase->renameCollection(collection, name, true);
+        auto res = vocbase->renameCollection(collection->id(), name, true);
 
-        if (res != TRI_ERROR_NO_ERROR) {
+        if (!res.ok()) {
           LOG_TOPIC(WARN, arangodb::Logger::ENGINES)
               << "cannot rename collection " << collectionId << " in database "
               << databaseId << " to '" << name
-              << "': " << TRI_errno_string(res);
+              << "': " << res.errorMessage();
           ++state->errorCount;
+
           return state->canContinue();
         }
+
         break;
       }
 
@@ -833,12 +835,14 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker,
         // turn off sync temporarily if the database or collection are going to
         // be dropped later
         bool const forceSync = state->willViewBeDropped(databaseId, viewId);
-        
+
         VPackSlice nameSlice = payloadSlice.get("name");
+
         if (nameSlice.isString() && !nameSlice.isEqualString(view->name())) {
           std::string name = nameSlice.copyString();
           // check if other view exists with target name
           std::shared_ptr<arangodb::LogicalView> other = vocbase->lookupView(name);
+
           if (other != nullptr) {
             if (other->id() == view->id()) {
               LOG_TOPIC(TRACE, arangodb::Logger::ENGINES)
@@ -848,13 +852,16 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker,
             }
             vocbase->dropView(other->id(), true);
           }
-          int res = vocbase->renameView(view, name);
-          if (res != TRI_ERROR_NO_ERROR) {
+
+          auto res = vocbase->renameView(view->id(), name);
+
+          if (!res.ok()) {
             LOG_TOPIC(WARN, arangodb::Logger::ENGINES)
             << "cannot rename view " << viewId << " in database "
             << databaseId << " to '" << name
-            << "': " << TRI_errno_string(res);
+            << "': " << res.errorMessage();
             ++state->errorCount;
+
             return state->canContinue();
           }
         }

arangod/Replication/TailingSyncer.cpp

@@ -632,7 +632,7 @@ Result TailingSyncer::renameCollection(VPackSlice const& slice) {
         << "Renaming system collection " << col->name();
   }
 
-  return Result(vocbase->renameCollection(col, name, true));
+  return vocbase->renameCollection(col->id(), name, true);
 }
 
 /// @brief changes the properties of a collection,
@@ -734,17 +734,19 @@ Result TailingSyncer::changeView(VPackSlice const& slice) {
     return Result(TRI_ERROR_REPLICATION_INVALID_RESPONSE,
                   "view marker slice is no object");
   }
-  
+
   VPackSlice data = slice.get("data");
+
   if (!data.isObject()) {
     return Result(TRI_ERROR_REPLICATION_INVALID_RESPONSE,
                   "data slice is no object in view change marker");
   }
-  
+
   VPackSlice d = data.get("deleted");
   bool const isDeleted = (d.isBool() && d.getBool());
-  
+
   TRI_vocbase_t* vocbase = resolveVocbase(slice);
+
   if (vocbase == nullptr) {
     if (isDeleted) {
       // not a problem if a view that is going to be deleted anyway
@@ -753,36 +755,44 @@ Result TailingSyncer::changeView(VPackSlice const& slice) {
     }
     return Result(TRI_ERROR_ARANGO_DATABASE_NOT_FOUND);
   }
-  
+
   VPackSlice guidSlice = data.get(StaticStrings::DataSourceGuid);
+
   if (!guidSlice.isString() || guidSlice.getStringLength() == 0) {
     return Result(TRI_ERROR_REPLICATION_INVALID_RESPONSE,
                   "no guid specified for view");
   }
+
   auto view = vocbase->lookupView(guidSlice.copyString());
+
   if (view == nullptr) {
     if (isDeleted) {
       // not a problem if a collection that is going to be deleted anyway
       // does not exist on slave
       return Result();
     }
+
     return Result(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND);
   }
-  
-  
+
   VPackSlice nameSlice = data.get(StaticStrings::DataSourceName);
+
   if (nameSlice.isString() && !nameSlice.isEqualString(view->name())) {
-    int res = vocbase->renameView(view, nameSlice.copyString());
-    if (res != TRI_ERROR_NO_ERROR) {
+    auto res = vocbase->renameView(view->id(), nameSlice.copyString());
+
+    if (!res.ok()) {
       return res;
     }
   }
-  
+
   VPackSlice properties = data.get("properties");
+
   if (properties.isObject()) {
     bool doSync = DatabaseFeature::DATABASE->forceSyncProperties();
+
     return view->updateProperties(properties, false, doSync);
   }
+
   return {};
 }
 

arangod/RestHandler/RestViewHandler.cpp

@@ -303,12 +303,12 @@ void RestViewHandler::modifyView(bool partialUpdate) {
       }
 
       auto newNameStr = newName.copyString();
-      int res = _vocbase.renameView(view, newNameStr);
+      auto res = _vocbase.renameView(view->id(), newNameStr);
 
-      if (res == TRI_ERROR_NO_ERROR) {
+      if (res.ok()) {
         getView(newNameStr, false);
       } else {
-        generateError(GeneralResponse::responseCode(res), res);
+        generateError(res);
       }
 
       return;

arangod/V8Server/v8-views.cpp

@@ -650,14 +650,10 @@ static void JS_RenameViewVocbase(
     TRI_V8_THROW_EXCEPTION(TRI_ERROR_INTERNAL); // skip view
   }
 
-  // need to get the sharedPtr from the vocbase
-  arangodb::CollectionNameResolver resolver(view->vocbase());
-  auto viewPtr = resolver.getView(view->id());
+  auto res = view->vocbase().renameView(view->id(), name);
 
-  int res = view->vocbase().renameView(viewPtr, name);
-
-  if (res != TRI_ERROR_NO_ERROR) {
-    TRI_V8_THROW_EXCEPTION_MESSAGE(res, "cannot rename view");
+  if (!res.ok()) {
+    TRI_V8_THROW_EXCEPTION(res);
   }
 
   TRI_V8_RETURN_UNDEFINED();

arangod/VocBase/Methods/Collections.cpp

@@ -481,10 +481,10 @@ Result Collections::rename(LogicalCollection* coll, std::string const& newName,
   }
 
   std::string const oldName(coll->name());
-  int res = coll->vocbase().renameCollection(coll, newName, doOverride);
+  auto res = coll->vocbase().renameCollection(coll->id(), newName, doOverride);
 
-  if (res != TRI_ERROR_NO_ERROR) {
-    return Result(res, "cannot rename collection");
+  if (!res.ok()) {
+    return res;
   }
 
   // rename collection inside _graphs as well

arangod/VocBase/vocbase.cpp

@@ -1303,7 +1303,7 @@ arangodb::Result TRI_vocbase_t::dropCollection(
     bool allowDropSystem,
     double timeout
 ) {
-  auto* collection = lookupCollection(cid).get();
+  auto collection = lookupCollection(cid);
 
   if (!collection) {
     return TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND;
@@ -1328,7 +1328,7 @@ arangodb::Result TRI_vocbase_t::dropCollection(
     int res;
     {
       READ_LOCKER(readLocker, _inventoryLock);
-      res = dropCollectionWorker(collection, state, timeout);
+      res = dropCollectionWorker(collection.get(), state, timeout);
     }
 
     if (state == DROP_PERFORM) {
@@ -1356,10 +1356,16 @@ arangodb::Result TRI_vocbase_t::dropCollection(
 }
 
 /// @brief renames a view
-int TRI_vocbase_t::renameView(
-    std::shared_ptr<arangodb::LogicalView> const& view,
+arangodb::Result TRI_vocbase_t::renameView(
+    TRI_voc_cid_t cid,
     std::string const& newName
 ) {
+  auto const view = lookupView(cid);
+
+  if (!view) {
+    return TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND;
+  }
+
   // lock collection because we are going to copy its current name
   std::string oldName = view->name();
 
@@ -1422,11 +1428,17 @@ int TRI_vocbase_t::renameView(
 }
 
 /// @brief renames a collection
-int TRI_vocbase_t::renameCollection(
-    arangodb::LogicalCollection* collection,
+arangodb::Result TRI_vocbase_t::renameCollection(
+    TRI_voc_cid_t cid,
     std::string const& newName,
     bool doOverride
 ) {
+  auto collection = lookupCollection(cid);
+
+  if (!collection) {
+    return TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND;
+  }
+
   if (collection->system()) {
     return TRI_set_errno(TRI_ERROR_FORBIDDEN);
   }
@@ -1520,11 +1532,10 @@ int TRI_vocbase_t::renameCollection(
     return res.errorNumber(); // rename failed
   }
 
-  auto col = std::static_pointer_cast<arangodb::LogicalCollection>(itr1->second); // cast validated above
   auto* engine = EngineSelectorFeature::ENGINE;
 
   TRI_ASSERT(engine);
-  res = engine->renameCollection(*this, *col, oldName); // tell the engine
+  res = engine->renameCollection(*this, *collection, oldName); // tell the engine
 
   if (!res.ok()) {
     return res.errorNumber(); // rename failed
@@ -2232,4 +2243,4 @@ TRI_voc_rid_t TRI_StringToRid(char const* p, size_t len, bool& isOld,
 
 // -----------------------------------------------------------------------------
 // --SECTION--                                                       END-OF-FILE
-// -----------------------------------------------------------------------------
+// -----------------------------------------------------------------------------

arangod/VocBase/vocbase.h

@@ -325,15 +325,15 @@ struct TRI_vocbase_t {
   ) const;
 
   /// @brief renames a collection
-  int renameCollection(
-    arangodb::LogicalCollection* collection,
+  arangodb::Result renameCollection(
+    TRI_voc_cid_t cid,
     std::string const& newName,
     bool doOverride
   );
 
   /// @brief renames a view
-  int renameView(
-    std::shared_ptr<arangodb::LogicalView> const& view,
+  arangodb::Result renameView(
+    TRI_voc_cid_t cid,
     std::string const& newName
   );