COMPASS-406 Index Error (#57)

* Add new bugsnag fixture data and tests gathered from the node driver

* Migrate invalid values check to new IXWARN_KEY_PATTERN

* Drop indexes test and support for it

Actually it was not possible to create a mongod with an invalid string going back at least to v2.6 which is our last mostly-supported version in Compass at this time.

pzrq@rocket:~$ mongo
MongoDB shell version: 2.6.12
connecting to: test
> db.funky.createIndex({c:'infinityStr'})
{
	"createdCollectionAutomatically" : true,
	"numIndexesBefore" : 1,
	"ok" : 0,
	"errmsg" : "bad index key pattern { c: \"infinityStr\" }: Unknown index plugin 'infinityStr'",
	"code" : 67
}

pzrq@rocket:~$ mongo
MongoDB shell version: 3.2.12
connecting to: test
> db.funky.createIndex({c:'infinityStr'})
{
	"ok" : 0,
	"errmsg" : "bad index key pattern { c: \"infinityStr\" }: Unknown index plugin 'infinityStr'",
	"code" : 67
}

Author: pzrq

lib/index-field.js

@@ -8,8 +8,7 @@ var IndexField = Model.extend({
   props: {
     field: 'string',
     value: {
-      type: 'any',
-      values: [1, -1, '2dsphere', '2d', 'geoHaystack', 'text', 'hashed']
+      type: 'any'
     }
   },
   derived: {

lib/warnings.js

@@ -6,11 +6,22 @@ var format = require('util').format;
 
 var WARNINGS = {
   'IXWARN_PREFIX': 1,
-  'IXWARN_UNUSED': 2
+  'IXWARN_UNUSED': 2,
+  'IXWARN_KEY_PATTERN': 3
 };
 
 // var debug = require('debug')('mongodb-index-model:warnings-mixin');
 
+/**
+ * List of valid values for an index.
+ *
+ * See https://docs.mongodb.com/manual/reference/method/db.collection.createIndex/#db.collection.createIndex
+ * See https://docs.mongodb.com/manual/release-notes/3.4-compatibility/#stricter-validation-of-index-specifications
+ *
+ * @type {string[]}
+ */
+var VALID_INDEX_TYPE_VALUES = [1, -1, '2dsphere', '2d', 'geoHaystack', 'text', 'hashed'];
+
 var WarningModel = Model.extend({
   idAttribute: 'code',
   props: {
@@ -67,6 +78,15 @@ var WarningsMixin = {
           details: 'This index has never been used and might therefore not be required.'
         }));
         break;
+      case WARNINGS.IXWARN_KEY_PATTERN:
+        idx.warnings.add(new WarningModel({
+          code: warningCode,
+          message: 'Index Key Pattern is not valid',
+          details: 'This index has a key pattern that may not be valid in MongoDB 3.4 and later.' +
+          'See https://docs.mongodb.com/manual/release-notes/3.4-compatibility/' +
+          '#stricter-validation-of-index-specifications for more information.'
+        }));
+        break;
       default:
         throw new Error('Index warning code %i unknown.');
     }
@@ -98,6 +118,14 @@ var WarningsMixin = {
       } else {
         idx.warnings.remove(WARNINGS.IXWARN_UNUSED);
       }
+
+      // Check for potentially invalid values, e.g. Boolean true, 0, NaN, or
+      // arbitrary strings that are not special like '2d' or 'geoHaystack'
+      _.values(idx.key).forEach(function(value) {
+        if (!_.includes(VALID_INDEX_TYPE_VALUES, value)) {
+          collection.addWarningToIndex(WARNINGS.IXWARN_KEY_PATTERN, idx, {});
+        }
+      });
     });
   }
 };

test/fixture-bugsnag.js

@@ -0,0 +1,27 @@
+module.exports = [
+  {
+    'v': 2,
+    'key': {'a': Infinity},
+    'name': 'a_Infinity',
+    'ns': 'indexes.funky'
+  },
+// Skip Decimal128 for now...
+// { v: 2,
+//   key:
+//    { a:
+//       Decimal128 {
+//         _bsontype: 'Decimal128',
+//         bytes: <Buffer 01 00 00 00 00 00 00 00 00 00 00 00 00 00 58 30> } },
+//   name: 'a_NumberDecimal("1E+12")',
+//   ns: 'indexes.funky' } { a:
+//    Decimal128 {
+//      _bsontype: 'Decimal128',
+//      bytes: <Buffer 01 00 00 00 00 00 00 00 00 00 00 00 00 00 58 30> } } Decimal128 {
+//   _bsontype: 'Decimal128',
+//   bytes: <Buffer 01 00 00 00 00 00 00 00 00 00 00 00 00 00 58 30> }
+  {'v': 2, 'key': {'a': -2}, 'name': 'a_-2', 'ns': 'indexes.funky'},
+  {'v': 2, 'key': {'a': 0.1}, 'name': 'a_0.1', 'ns': 'indexes.funky'},
+  {'v': 2, 'key': {'a': -0.5}, 'name': 'a_-0.5', 'ns': 'indexes.funky'},
+  {'v': 1, 'key': {'b': 0}, 'name': 'b_0', 'ns': 'indexes.funky'},
+  {'v': 1, 'key': {'b': true}, 'name': 'b_true', 'ns': 'indexes.funky'}
+];

test/index.test.js

@@ -1,14 +1,16 @@
 var assert = require('assert');
-var Index = require('../');
 var IndexCollection = require('../').Collection;
 var _ = require('lodash');
 
 var INDEX_FIXTURE = require('./fixture');
+var INDEX_BUGSNAG_FIXTURE = require('./fixture-bugsnag');
 
 describe('mongodb-index-model', function() {
   var indexes;
+  var bugsnagIndexes;
   before(function() {
     indexes = new IndexCollection(INDEX_FIXTURE, {parse: true});
+    bugsnagIndexes = new IndexCollection(INDEX_BUGSNAG_FIXTURE, {parse: true});
   });
 
   context('IndexModel', function() {
@@ -117,6 +119,33 @@ describe('mongodb-index-model', function() {
       assert.equal(indexes.get('seniors', 'name').fields.at(0).value, 1);
     });
 
+    context('when created via a v3.4.x mongod and mongoshell', function() {
+      it('should accept Infinity index field values', function() {
+        assert.equal(bugsnagIndexes.get('a_Infinity', 'name').fields.at(0).field, 'a');
+        assert.equal(bugsnagIndexes.get('a_Infinity', 'name').fields.at(0).value, Infinity);
+      });
+
+      it('should accept numbers less than -1 as index field values', function() {
+        assert.equal(bugsnagIndexes.get('a_-2', 'name').fields.at(0).field, 'a');
+        assert.equal(bugsnagIndexes.get('a_-2', 'name').fields.at(0).value, -2);
+      });
+
+      it('should accept floats as index field values', function() {
+        assert.equal(bugsnagIndexes.get('a_0.1', 'name').fields.at(0).field, 'a');
+        assert.equal(bugsnagIndexes.get('a_0.1', 'name').fields.at(0).value, 0.1);
+      });
+    });
+
+    context('when created via a v3.2.x mongod and mongoshell', function() {
+      // See https://jira.mongodb.org/browse/COMPASS-406
+      // See https://app.bugsnag.com/mongodb/mongodb-compass/errors/581ea54b07b7add195ad49ce?event_id=585a8ea91c8db711005c6cb2
+      // See https://jira.mongodb.org/browse/SERVER-11064
+      it('should accept boolean true as index field values', function() {
+        assert.equal(bugsnagIndexes.get('b_true', 'name').fields.at(0).field, 'b');
+        assert.equal(bugsnagIndexes.get('b_true', 'name').fields.at(0).value, true);
+      });
+    });
+
     it('should accept dotted field names', function() {
       assert.equal(indexes.get('seniors', 'name').fields.at(1).field, 'address.city');
       assert.equal(indexes.get('seniors', 'name').fields.at(1).value, 1);
@@ -130,19 +159,5 @@ describe('mongodb-index-model', function() {
       assert.equal(indexes.get('seniors', 'name').fields.at(0).geo, false);
       assert.equal(indexes.get('last_position_2dsphere', 'name').fields.at(0).geo, true);
     });
-
-    it('should not allow arbitary strings as values', function() {
-      assert.throws(function() {
-        /* eslint no-new: 0 */
-        new Index({
-          name: 'badIndex',
-          key: {
-            foo: 'someStrangeValue'
-          }
-        }, {
-          parse: true
-        });
-      });
-    });
   });
 });

test/warnings.test.js

@@ -1,18 +1,19 @@
 var assert = require('assert');
 var WarningsMixin = require('../lib/warnings').mixin;
 var IndexCollection = require('../').Collection;
-// var _ = require('lodash');
-//
 // var debug = require('debug')('mongodb-index-model:test:warnings');
 
 var INDEX_FIXTURE = require('./fixture');
+var INDEX_BUGSNAG_FIXTURE = require('./fixture-bugsnag');
 
 var IndexWithWarningsCollection = IndexCollection.extend(WarningsMixin);
 
 describe('Index Warnings', function() {
   var indexes;
+  var bugsnagIndexes;
   beforeEach(function() {
     indexes = new IndexWithWarningsCollection(INDEX_FIXTURE, {parse: true});
+    bugsnagIndexes = new IndexWithWarningsCollection(INDEX_BUGSNAG_FIXTURE, {parse: true});
   });
 
   context('IXWARN_PREFIX', function() {
@@ -125,4 +126,20 @@ describe('Index Warnings', function() {
       assert.ok(idx.warnings.length === 0);
     });
   });
+
+  context('IXWARN_KEY_PATTERN', function() {
+    it('warns boolean true is not valid in 3.4+', function() {
+      bugsnagIndexes.updateIndexWarnings();
+      var index = bugsnagIndexes.get('b_true', 'name');
+      assert.ok(index.warnings.length === 1);
+      assert.ok(index.warnings.at(0).message.match(/Index Key Pattern is not valid/));
+    });
+
+    it('warns 0 is not valid in 3.4+', function() {
+      bugsnagIndexes.updateIndexWarnings();
+      var index = bugsnagIndexes.get('b_0', 'name');
+      assert.ok(index.warnings.length === 1);
+      assert.ok(index.warnings.at(0).message.match(/Index Key Pattern is not valid/));
+    });
+  });
 });