Added error for inconsitent mappings, mapper tests

src/courier/errors.js

@@ -43,11 +43,20 @@ define(function (require) {
   function VersionConflict(resp) {
     this.name = 'VersionConflict';
     this.resp = resp;
-    this.message = 'Failed to store document changes do to a version conflict.';
+    this.message = 'Failed to store document changes due to a version conflict.';
   }
   VersionConflict.prototype = new Error();
   VersionConflict.prototype.constructor = VersionConflict;
   errors.VersionConflict = VersionConflict;
 
+  // there was a conflict storing a doc
+  function MappingConflict(field) {
+    this.name = 'MappingConflict';
+    this.message = 'Field ' + field + ' is defined as at least two different types in indices matching the pattern';
+  }
+  MappingConflict.prototype = new Error();
+  MappingConflict.prototype.constructor = MappingConflict;
+  errors.MappingConflict = MappingConflict;
+
   return errors;
 });

src/courier/mapper.js

@@ -123,8 +123,6 @@ define(function (require) {
      * @param {Function} callback A function to be executed with the results.
      */
     this.getFieldsFromMapping = function (dataSource, callback) {
-
-
       var params = {
         // TODO: Change index to be newest resolved index. Change _state.index to id().
         index: dataSource._state.index,
@@ -133,6 +131,7 @@ define(function (require) {
 
       // TODO: Add week/month check
       client.indices.getFieldMapping(params, function (err, response, status) {
+
         // TODO: Add error message
 
         var fields = {};
@@ -141,6 +140,8 @@ define(function (require) {
           _.each(index.mappings, function (type) {
             _.each(type, function (field, name) {
               if (_.isUndefined(field.mapping) || name[0] === '_') return;
+              if (!_.isUndefined(fields[name]) && fields[name] !== field.mapping[_.keys(field.mapping)[0]])
+                return courier._error(new Error.MappingConflict());
               fields[name] = field.mapping[_.keys(field.mapping)[0]];
             });
           });
@@ -182,7 +183,7 @@ define(function (require) {
      * @param {dataSource} dataSource
      * @param {Function} callback A function to be executed with the results.
      */
-    var clearCache = function (dataSource, callback) {
+    this.clearCache = function (dataSource, callback) {
       if (!_.isUndefined(mappings[dataSource._state.index])) {
         delete mappings[dataSource._state.index];
       }
@@ -195,12 +196,12 @@ define(function (require) {
 
 
     /**
-     * Sets a number of fields to be ignored in the mapping
+     * Sets a number of fields to be ignored in the mapping. Not sure this is a good idea?
      * @param {dataSource} dataSource
      * @param {Array} fields An array of fields to be ignored
      * @param {Function} callback A function to be executed with the results.
      */
-    var ignoreFields = function (dataSource, fields, callback) {
+    this.ignoreFields = function (dataSource, fields, callback) {
       if (!_.isArray(fields)) fields = [fields];
       var ignore = _.object(_.map(fields, function (field) {
         return [field, {type: 'ignore'}];

src/kibana/controllers/kibana.js

@@ -27,10 +27,10 @@ define(function (require) {
           $scope.mappedFields = {};
 
           var source = $rootScope.dataSource.extend()
-            .index('logstash-2014.02.14')
+            .index('logstash-*')
             .type($scope.type)
             .source({
-              include: 'country'
+              include: 'geo'
             })
             .on('results', function (resp) {
               $scope.count ++;

test/unit/specs/mapper.js

@@ -14,33 +14,121 @@ define(function (require) {
     client: client
   });
 
-  describe('Mapper Module', function () {
+  describe('Mapper', function () {
     var server, source, mapper;
 
     beforeEach(function() {
       source = courier.createSource('search')
-        .index('logs*')
+        .index('valid')
         .size(5);
       mapper = new Mapper(courier);
 
+      // Stub out a mini mapping response.
+      sinon.stub(client.indices, 'getFieldMapping',function (params, callback) {
+        if(params.index === 'valid') {
+          setTimeout(callback(undefined,{
+            "test": {
+              "mappings": {
+                "testType": {
+                  "foo.bar": {
+                    "full_name": "foo.bar",
+                      "mapping": {
+                        "bar": {
+                          "type": "string"
+                        }}}}}}}
+          ),0);
+        } else {
+          setTimeout(callback('Error: Not Found',undefined));
+        }
+      });
+
+      sinon.stub(client, 'getSource', function (params, callback) {
+        if(params.id === 'valid') {
+          setTimeout(callback(undefined,{"foo.bar": {"type": "string"}}),0);
+        } else {
+          setTimeout(callback('Error: Not Found',undefined),0);
+        }
+      });
+
+      sinon.stub(client, 'delete', function (params, callback) {callback(undefined,true);});
     });
 
     afterEach(function () {
+      client.indices.getFieldMapping.restore();
+      client.getSource.restore();
+      client.delete.restore();
     });
 
     it('provides a constructor for the Mapper class', function (done) {
+      var mapper = new Mapper(courier);
       expect(mapper).to.be.a(Mapper);
       done();
     });
 
-    it('has a function called getFieldsFromMapping that calls client.indices.getFieldMapping', function (done) {
-      sinon.spy(client.indices, 'getFieldMapping');
-      mapper.getFieldsFromMapping(source,function(){});
-      expect(client.indices.getFieldMapping.called).to.be(true);
-      client.indices.getFieldMapping.restore();
+    it('has getFieldsFromMapping function that returns a mapping', function (done) {
+      mapper.getFieldsFromMapping(source,function (err, mapping) {
+        expect(client.indices.getFieldMapping.called).to.be(true);
+        expect(mapping['foo.bar'].type).to.be('string');
+        done();
+      });
+    });
+
+    it('has getFieldsFromCache that returns an error for uncached indices', function (done) {
+      source = courier.createSource('search')
+        .index('invalid')
+        .size(5);
+
+      mapper.getFieldsFromCache(source,function (err, mapping) {
+        expect(client.getSource.called).to.be(true);
+        expect(err).to.be('Error: Not Found');
+        done();
+      });
+    });
+
+    it('has getFieldsFromCache that returns a mapping', function (done) {
+      mapper.getFieldsFromCache(source,function (err, mapping) {
+        expect(client.getSource.called).to.be(true);
+        expect(mapping['foo.bar'].type).to.be('string');
+        done();
+      });
+    });
+
+    it('has a getFieldsFromObject function', function (done) {
+      expect(mapper.getFieldsFromObject).to.be.a('function');
       done();
     });
 
+    it('has a getFields that returns a mapping from cache', function (done) {
+      mapper.getFields(source, function (err, mapping) {
+        expect(client.getSource.called).to.be(true);
+        expect(client.indices.getFieldMapping.called).to.be(false);
+        expect(mapping['foo.bar'].type).to.be('string');
+        done();
+      });
+    });
+
+    it('has getFields that throws an error for invalid indices', function (done) {
+      source = courier.createSource('search')
+        .index('invalid')
+        .size(5);
+      try {
+        mapper.getFields(source, function (err, mapping) {
+          // Should not be called
+          expect('the callback').to.be('not executed');
+          done();
+        });
+      } catch (e) {
+        expect(true).to.be(true);
+        done();
+      }
+    });
+
+    it('has a clearCache that calls client.delete', function (done) {
+      mapper.clearCache(source, function () {
+        expect(client.delete.called).to.be(true);
+        done();
+      });
+    });
 
   });