fixed cached lookups of collections on the server, which may have caused spurious problems after collection rename operations

CHANGELOG

@@ -104,6 +104,17 @@ v2.3.0 (XXXX-XX-XX)
   storing JavaScript date objects in the database in a sensible manner.
 
 
+v2.2.x (????-??-??)
+-------------------
+
+* fixed issue #961: allow non-JSON values in undocument request bodies
+
+* fixed issue 1028: libicu is now statically linked
+
+* fixed cached lookups of collections on the server, which may have caused spurious
+  problems after collection rename operations
+
+
 v2.2.4 (2014-XX-XX)
 -------------------
 

UnitTests/Makefile.unittests

@@ -433,6 +433,7 @@ SHELL_COMMON = \
                @top_srcdir@/js/common/tests/shell-keygen.js \
                @top_srcdir@/js/common/tests/shell-keygen-noncluster.js \
                @top_srcdir@/js/common/tests/shell-index-ensure.js \
+               @top_srcdir@/js/common/tests/shell-rename-noncluster.js \
                @top_srcdir@/js/common/tests/shell-simple-query.js \
                @top_srcdir@/js/common/tests/shell-statement.js \
                @top_srcdir@/js/common/tests/shell-transactions.js \

arangod/V8Server/v8-collection-util.cpp

@@ -214,6 +214,7 @@ v8::Handle<v8::Object> WrapCollection (TRI_vocbase_col_t const* collection) {
 
     result->Set(v8g->_IdKey, V8CollectionId(collection->_cid), v8::ReadOnly);
     result->Set(v8g->_DbNameKey, v8::String::New(collection->_dbName));
+    result->Set(v8g->VersionKey, v8::Number::New((double) collection->_internalVersion), v8::DontEnum);
   }
 
   return scope.Close(result);

arangod/V8Server/v8-collection.h

@@ -39,22 +39,26 @@
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief free a coordinator collection
 ////////////////////////////////////////////////////////////////////////////////
+
 void FreeCoordinatorCollection (TRI_vocbase_col_t* collection);
 
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief releases a collection
 ////////////////////////////////////////////////////////////////////////////////
+
 void ReleaseCollection (TRI_vocbase_col_t const* collection);
 
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief convert a collection info into a TRI_vocbase_col_t
 ////////////////////////////////////////////////////////////////////////////////
+
 TRI_vocbase_col_t* CoordinatorCollection (TRI_vocbase_t* vocbase,
                                           triagens::arango::CollectionInfo const& ci);
 
 ///////////////////////////////////////////////////////////////////////////////
 /// @brief check if a name belongs to a collection
 ////////////////////////////////////////////////////////////////////////////////
+
 bool EqualCollection (triagens::arango::CollectionNameResolver const* resolver,
                       std::string const& collectionName,
                       TRI_vocbase_col_t const* collection);
@@ -62,6 +66,7 @@ bool EqualCollection (triagens::arango::CollectionNameResolver const* resolver,
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief wraps a TRI_vocbase_col_t
 ////////////////////////////////////////////////////////////////////////////////
+
 v8::Handle<v8::Object> WrapCollection (TRI_vocbase_col_t const* collection);
 
 void TRI_InitV8collection (v8::Handle<v8::Context> context,

arangod/V8Server/v8-vocbase.cpp

@@ -1414,7 +1414,7 @@ static v8::Handle<v8::Value> MapGetVocBase (v8::Local<v8::String> const name,
 
   if (*key == '_') {
     // special treatment for all properties starting with _
-    v8::Local<v8::String> const l = v8::String::New(key);
+    v8::Local<v8::String> const l = v8::String::New(key, keyLength);
 
     if (holder->HasRealNamedProperty(l)) {
       // some internal function inside db
@@ -1442,6 +1442,7 @@ static v8::Handle<v8::Value> MapGetVocBase (v8::Local<v8::String> const name,
       TRI_READ_LOCK_STATUS_VOCBASE_COL(collection);
       TRI_vocbase_col_status_e status = collection->_status;
       TRI_voc_cid_t cid = collection->_cid;
+      uint32_t internalVersion = collection->_internalVersion;
       TRI_READ_UNLOCK_STATUS_VOCBASE_COL(collection);
 
       // check if the collection is still alive
@@ -1450,13 +1451,17 @@ static v8::Handle<v8::Value> MapGetVocBase (v8::Local<v8::String> const name,
 
         if (value->Has(v8g->_IdKey)) {
           TRI_voc_cid_t cachedCid = static_cast<TRI_voc_cid_t>(TRI_ObjectToUInt64(value->Get(v8g->_IdKey), true));
+          uint32_t cachedVersion = (uint32_t) TRI_ObjectToInt64(value->Get(v8g->VersionKey));
 
-          if (cachedCid == cid) {
+          if (cachedCid == cid && cachedVersion == internalVersion) {
             // cache hit
             return scope.Close(value);
           }
 
-          // cid has changed (i.e. collection has been dropped and re-created)
+          // store the updated version number in the object for future comparisons
+          value->Set(v8g->VersionKey, v8::Number::New((double) internalVersion), v8::DontEnum);
+
+          // cid has changed (i.e. collection has been dropped and re-created) or version has changed
         }
       }
     }

arangod/VocBase/vocbase.cpp

@@ -476,18 +476,19 @@ static TRI_vocbase_col_t* AddCollection (TRI_vocbase_t* vocbase,
   // create the init object
   TRI_vocbase_col_t init;
 
-  init._vocbase     = vocbase;
+  init._vocbase         = vocbase;
-  init._cid         = cid;
+  init._cid             = cid;
-  init._planId      = 0;
+  init._planId          = 0;
-  init._type        = static_cast<TRI_col_type_t>(type);
+  init._type            = static_cast<TRI_col_type_t>(type);
+  init._internalVersion = 0;
 
-  init._status      = TRI_VOC_COL_STATUS_CORRUPTED;
+  init._status          = TRI_VOC_COL_STATUS_CORRUPTED;
-  init._collection  = nullptr;
+  init._collection      = nullptr;
 
   // default flags: everything is allowed
-  init._canDrop     = true;
+  init._canDrop         = true;
-  init._canRename   = true;
+  init._canRename       = true;
-  init._canUnload   = true;
+  init._canUnload       = true;
 
   // check for special system collection names
   if (TRI_IsSystemNameCollection(name)) {
@@ -809,6 +810,9 @@ static int RenameCollection (TRI_vocbase_t* vocbase,
 
   TRI_WRITE_UNLOCK_COLLECTIONS_VOCBASE(vocbase);
 
+  // to prevent caching
+  collection->_internalVersion++;
+
   TRI_WRITE_UNLOCK_STATUS_VOCBASE_COL(collection);
 
   if (! writeMarker) {
@@ -1159,6 +1163,7 @@ static int LoadCollectionVocBase (TRI_vocbase_t* vocbase,
       return TRI_set_errno(TRI_ERROR_ARANGO_CORRUPTED_COLLECTION);
     }
 
+    collection->_internalVersion = 0;
     collection->_collection = document;
     collection->_status = TRI_VOC_COL_STATUS_LOADED;
     TRI_CopyString(collection->_path, document->_directory, sizeof(collection->_path) - 1);

arangod/VocBase/vocbase.h

@@ -382,6 +382,9 @@ typedef struct TRI_vocbase_col_s {
 
   TRI_read_write_lock_t             _lock;       // lock protecting the status and name
 
+  uint32_t                          _internalVersion; // is incremented when a collection is renamed
+                                                 // this is used to prevent caching of collection objects
+                                                 // with "wrong" names in the "db" object
   TRI_vocbase_col_status_e          _status;     // status of the collection
   struct TRI_document_collection_t*  _collection; // NULL or pointer to loaded collection
   char _name[TRI_COL_NAME_LENGTH + 1];           // name of the collection

js/apps/system/aardvark/frontend/js/views/documentsView.js

@@ -731,7 +731,7 @@
       var result;
       if (this.type === 'document') {
         result = this.documentStore.deleteDocument(
-            this.collection.collectionID, this.docid
+          this.collection.collectionID, this.docid
         );
         if (result) {
           //on success
@@ -759,7 +759,7 @@
           $('#documentsTableID').dataTable().fnGetPosition(row)
         );
         $('#documentsTableID').dataTable().fnClearTable();
-          this.collection.getDocuments(this.getDocsCallback.bind(this));
+        this.collection.getDocuments(this.getDocsCallback.bind(this));
         $('#docDeleteModal').modal('hide');
       }
 

js/common/tests/shell-rename-noncluster.js

@@ -0,0 +1,104 @@
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test the random document selector 
+///
+/// @file
+///
+/// DISCLAIMER
+///
+/// Copyright 2010-2012 triagens GmbH, Cologne, Germany
+///
+/// Licensed under the Apache License, Version 2.0 (the "License");
+/// you may not use this file except in compliance with the License.
+/// You may obtain a copy of the License at
+///
+///     http://www.apache.org/licenses/LICENSE-2.0
+///
+/// Unless required by applicable law or agreed to in writing, software
+/// distributed under the License is distributed on an "AS IS" BASIS,
+/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+/// See the License for the specific language governing permissions and
+/// limitations under the License.
+///
+/// Copyright holder is triAGENS GmbH, Cologne, Germany
+///
+/// @author Jan Steemann
+/// @author Copyright 2012, triAGENS GmbH, Cologne, Germany
+////////////////////////////////////////////////////////////////////////////////
+
+var jsunity = require("jsunity");
+
+var arangodb = require("org/arangodb");
+var db = arangodb.db;
+
+// -----------------------------------------------------------------------------
+// --SECTION--                                                       RenameSuite                                                  
+// -----------------------------------------------------------------------------
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test suite
+////////////////////////////////////////////////////////////////////////////////
+
+function RenameSuite () {
+  var cn1 = "UnitTestsRename1";
+  var cn2 = "UnitTestsRename2";
+
+  return {
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief set up
+////////////////////////////////////////////////////////////////////////////////
+
+    setUp : function () {
+      db._drop(cn1);
+      db._drop(cn2);
+    },
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief tear down
+////////////////////////////////////////////////////////////////////////////////
+
+    tearDown : function () {
+      db._drop(cn2);
+      db._drop(cn1);
+    },
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief check AQL query after renaming collection
+////////////////////////////////////////////////////////////////////////////////
+
+    testAqlAfterRename : function () {
+      var c = db._create(cn1);
+      for (var i = 0; i < 100000; ++i) {
+        db.UnitTestsRename1.save({ value: i }); 
+      }
+      c.rename(cn2);
+      db._create(cn1); 
+      for (i = 0; i < 100; ++i) {
+        db.UnitTestsRename1.save({ value: i }); 
+      }
+
+      var result = db._query("FOR i IN " + cn1 + " LIMIT 99, 2 RETURN i").toArray();
+      assertEqual(1, result.length);
+      assertMatch(/^UnitTestsRename1\//, result[0]._id);
+    }
+
+  };
+}
+
+// -----------------------------------------------------------------------------
+// --SECTION--                                                              main
+// -----------------------------------------------------------------------------
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief executes the test suite
+////////////////////////////////////////////////////////////////////////////////
+
+jsunity.run(RenameSuite);
+
+return jsunity.done();
+
+// Local Variables:
+// mode: outline-minor
+// outline-regexp: "^\\(/// @brief\\|/// @addtogroup\\|// --SECTION--\\|/// @page\\|/// @}\\)"
+// End:
+

lib/V8/v8-globals.cpp

@@ -115,6 +115,7 @@ TRI_v8_global_s::TRI_v8_global_s (v8::Isolate* isolate)
     UrlKey(),
     UserKey(),
     ValueKey(),
+    VersionKey(),
     WaitForSyncKey(),
 
     _FromKey(),
@@ -198,6 +199,7 @@ TRI_v8_global_s::TRI_v8_global_s (v8::Isolate* isolate)
   UrlKey = v8::Persistent<v8::String>::New(isolate, TRI_V8_SYMBOL("url"));
   UserKey = v8::Persistent<v8::String>::New(isolate, TRI_V8_SYMBOL("user"));
   ValueKey = v8::Persistent<v8::String>::New(isolate, TRI_V8_SYMBOL("value"));
+  VersionKey = v8::Persistent<v8::String>::New(isolate, TRI_V8_SYMBOL("version"));
   WaitForSyncKey = v8::Persistent<v8::String>::New(isolate, TRI_V8_SYMBOL("waitForSync"));
 
   _FromKey = v8::Persistent<v8::String>::New(isolate, TRI_V8_SYMBOL("_from"));

lib/V8/v8-globals.h

@@ -700,6 +700,12 @@ typedef struct TRI_v8_global_s {
 
   v8::Persistent<v8::String> ValueKey;
 
+////////////////////////////////////////////////////////////////////////////////
+/// @brief "version" key
+////////////////////////////////////////////////////////////////////////////////
+
+  v8::Persistent<v8::String> VersionKey;
+
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief "waitForSync" key name
 ////////////////////////////////////////////////////////////////////////////////