fixed internal issue #2559 (#5634)

CHANGELOG

@@ -1,6 +1,10 @@
 v3.3.11 (2018-XX-XX)
 --------------------
 
+* fixed internal issue #2559: "unexpected document key" error when custom
+  shard keys are used and the "allowUserKeys" key generator option is set
+  to false
+
 * fixed AQL DOCUMENT lookup function for documents for sharded collections with
   more than a single shard and using a custom shard key (i.e. some shard
   key attribute other than `_key`).

js/common/tests/shell/shell-keygen.js

@@ -1,5 +1,5 @@
 /*jshint globalstrict:false, strict:false */
-/*global assertEqual, assertTrue, assertEqual, fail */
+/*global assertEqual, assertTrue, assertEqual, fail, ArangoClusterComm */
 
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief test the traditional key generators
@@ -33,7 +33,13 @@ var jsunity = require("jsunity");
 var arangodb = require("@arangodb");
 var db = arangodb.db;
 var ERRORS = arangodb.errors;
-
+let cluster;
+// quick hack to check if we are arangod or arangosh
+if (typeof ArangoClusterComm === "object") {
+  cluster = require("@arangodb/cluster");
+} else {
+  cluster = {};
+}
 
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief test suite: traditional key gen
@@ -53,6 +59,52 @@ function TraditionalSuite () {
       db._drop(cn);
     },
 
+////////////////////////////////////////////////////////////////////////////////
+/// @brief create with key
+////////////////////////////////////////////////////////////////////////////////
+
+    testCreateInvalidKeyNonDefaultSharding1 : function () {
+      var c = db._create(cn, { shardKeys: ["value"], keyOptions: { type: "traditional", allowUserKeys: false } });
+
+      try {
+        c.save({ _key: "1234" }); // no user keys allowed
+        fail();
+      }
+      catch (err) {
+        assertTrue(err.errorNum === ERRORS.ERROR_ARANGO_DOCUMENT_KEY_UNEXPECTED.code ||
+                   err.errorNum === ERRORS.ERROR_CLUSTER_MUST_NOT_SPECIFY_KEY.code);
+      }
+    },
+    
+    testCreateInvalidKeyNonDefaultSharding2 : function () {
+      if (!cluster || !cluster.isCluster || !cluster.isCluster()) {
+        return;
+      }
+
+      var c = db._create(cn, { shardKeys: ["value"], keyOptions: { type: "traditional", allowUserKeys: true } });
+
+      try {
+        c.save({ _key: "1234" }); // no user keys allowed
+        fail();
+      }
+      catch (err) {
+        assertTrue(err.errorNum === ERRORS.ERROR_ARANGO_DOCUMENT_KEY_UNEXPECTED.code ||
+                   err.errorNum === ERRORS.ERROR_CLUSTER_MUST_NOT_SPECIFY_KEY.code);
+      }
+    },
+    
+    testCreateKeyNonDefaultSharding : function () {
+      if (!cluster || !cluster.isCluster || !cluster.isCluster()) {
+        return;
+      }
+
+      var c = db._create(cn, { shardKeys: ["value"], keyOptions: { type: "traditional", allowUserKeys: true } });
+
+      let key = c.save({ value: "1" }); // no user keys allowed
+      let doc = c.document(key);
+      assertEqual("1", doc.value);
+    },
+
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief create with key
 ////////////////////////////////////////////////////////////////////////////////

js/server/modules/@arangodb/cluster.js

@@ -828,6 +828,11 @@ function executePlanForCollections(plannedCollections) {
                 let save = {id: collectionInfo.id, name: collectionInfo.name};
                 delete collectionInfo.id;     // must not
                 delete collectionInfo.name;
+                if (collectionInfo.keyOptions && 
+                    (collectionInfo.shardKeys.length !== 1 || collectionInfo.shardKeys[0] !== '_key')) {
+                  // custom sharding... we must allow the coordinator to set a _key
+                  collectionInfo.keyOptions.allowUserKeys = true;
+                }
                 if (collectionInfo.hasOwnProperty('globallyUniqueId')) {
                   console.warn('unexpected globallyUniqueId in %s',
                     JSON.stringify(collectionInfo));