Merge pull request #1931 from spenceralger/dependable-agg-ids

Dependable agg ids

src/kibana/components/vis/_agg_config.js

@@ -6,7 +6,7 @@ define(function (require) {
     function AggConfig(vis, opts) {
       var self = this;
 
-      self.id = _.uniqueId('agg_');
+      self.id = opts.id || AggConfig.nextId(vis.aggs);
       self.vis = vis;
       self._opts = opts = (opts || {});
 
@@ -26,6 +26,39 @@ define(function (require) {
       self.fillDefaults(opts.params);
     }
 
+    /**
+     * Ensure that all of the objects in the list have ids, the objects
+     * and list are modified by reference.
+     *
+     * @param  {array[object]} list - a list of objects, objects can be anything really
+     * @return {array} - the list that was passed in
+     */
+    AggConfig.ensureIds = function (list) {
+      var have = [];
+      var haveNot = [];
+      list.forEach(function (obj) {
+        (obj.id ? have : haveNot).push(obj);
+      });
+
+      var nextId = AggConfig.nextId(have);
+      haveNot.forEach(function (obj) {
+        obj.id = nextId++;
+      });
+
+      return list;
+    };
+
+    /**
+     * Calculate the next id based on the ids in this list
+     *
+     * @return {array} list - a list of objects with id properties
+     */
+    AggConfig.nextId = function (list) {
+      return 1 + list.reduce(function (max, obj) {
+        return Math.max(max, obj.id || 0);
+      }, 0);
+    };
+
     /**
      * Write the current values to this.params, filling in the defaults as we go
      *
@@ -118,6 +151,7 @@ define(function (require) {
       }, {});
 
       return {
+        id: self.id,
         type: self.type && self.type.name,
         schema: self.schema && self.schema.name,
         params: outParams

src/kibana/components/vis/_agg_configs.js

@@ -9,11 +9,12 @@ define(function (require) {
       var self = this;
       this.vis = vis;
 
+      configStates = AggConfig.ensureIds(configStates || []);
 
       AggConfigs.Super.call(this, {
         index: ['id'],
         group: ['schema.group', 'type.name', 'schema.name'],
-        initialSet: (configStates || []).map(function (aggConfigState) {
+        initialSet: configStates.map(function (aggConfigState) {
           if (aggConfigState instanceof AggConfig) return aggConfigState;
           return new AggConfig(vis, aggConfigState);
         })
@@ -33,8 +34,9 @@ define(function (require) {
         .each(function (schema) {
           if (!self.bySchemaName[schema.name]) {
             var defaults = schema.defaults.slice(0, schema.max);
-            _.each(defaults, function (def) {
-              self.push(new AggConfig(vis, def));
+            _.each(defaults, function (defaultState) {
+              var state = _.defaults({ id: AggConfig.nextId(self) }, defaultState);
+              self.push(new AggConfig(vis, state));
             });
           }
         });

src/kibana/plugins/discover/_segmented_fetch.js

@@ -315,30 +315,30 @@ define(function (require) {
 
       if (!resp.aggregations) return;
 
-      var aggKey = _.find(Object.keys(resp.aggregations), function (key) {
-        return key.substr(0, 4) === 'agg_';
-      });
-
-      if (!aggKey) throw new Error('aggKey not found in response: ' + Object.keys(resp.aggregations));
+      Object.keys(resp.aggregations).forEach(function (aggKey) {
 
-      // start merging aggregations
-      if (!requestStats.aggregations) {
-        requestStats.aggregations = {};
-        requestStats.aggregations[aggKey] = {
-          buckets: []
-        };
-        requestStats._bucketIndex = {};
-      }
+        if (!requestStats.aggregations) {
+          // start merging aggregations
+          requestStats.aggregations = {};
+          requestStats._bucketIndex = {};
+        }
 
-      resp.aggregations[aggKey].buckets.forEach(function (bucket) {
-        var mbucket = requestStats._bucketIndex[bucket.key];
-        if (mbucket) {
-          mbucket.doc_count += bucket.doc_count;
-          return;
+        if (!requestStats.aggregations[aggKey]) {
+          requestStats.aggregations[aggKey] = {
+            buckets: []
+          };
         }
 
-        mbucket = requestStats._bucketIndex[bucket.key] = bucket;
-        requestStats.aggregations[aggKey].buckets.push(mbucket);
+        resp.aggregations[aggKey].buckets.forEach(function (bucket) {
+          var mbucket = requestStats._bucketIndex[bucket.key];
+          if (mbucket) {
+            mbucket.doc_count += bucket.doc_count;
+            return;
+          }
+
+          mbucket = requestStats._bucketIndex[bucket.key] = bucket;
+          requestStats.aggregations[aggKey].buckets.push(mbucket);
+        });
       });
     }
 

src/kibana/plugins/visualize/editor/nesting_indicator.js

@@ -11,7 +11,7 @@ define(function (require) {
       var colorPool = Private(require('components/vislib/components/color/color_palette'))(100);
       var assigned = {};
       return function (item) {
-        var key = item.$$hashKey;
+        var key = item.id || item.$$hashKey;
         if (!key) throw new Error('expected an item that is part of an ngRepeat');
 
         if (!assigned[key]) {

test/unit/specs/components/vis/_agg_config.js

@@ -3,12 +3,14 @@ define(function (require) {
     var sinon = require('test_utils/auto_release_sinon');
 
     var Vis;
+    var AggType;
     var AggConfig;
     var indexPattern;
 
     beforeEach(module('kibana'));
     beforeEach(inject(function (Private) {
       Vis = Private(require('components/vis/vis'));
+      AggType = Private(require('components/agg_types/_agg_type'));
       AggConfig = Private(require('components/vis/_agg_config'));
       indexPattern = Private(require('fixtures/stubbed_logstash_index_pattern'));
     }));
@@ -50,7 +52,6 @@ define(function (require) {
         expect(dsl).to.have.property('date_histogram');
       });
 
-
       it('uses the params from #write() output as the agg params', function () {
         var vis = new Vis(indexPattern, {
           type: 'histogram',
@@ -102,5 +103,108 @@ define(function (require) {
         expect(dsl.aggs[avgConfig.id].avg).to.be(football);
       });
     });
+
+    describe('::ensureIds', function () {
+      it('accepts an array of objects and assigns ids to them', function () {
+        var objs = [
+          {},
+          {},
+          {},
+          {}
+        ];
+        AggConfig.ensureIds(objs);
+        expect(objs[0]).to.have.property('id', 1);
+        expect(objs[1]).to.have.property('id', 2);
+        expect(objs[2]).to.have.property('id', 3);
+        expect(objs[3]).to.have.property('id', 4);
+      });
+
+      it('assigns ids relative to the other items in the list', function () {
+        var objs = [
+          { id: 100 },
+          {},
+        ];
+        AggConfig.ensureIds(objs);
+        expect(objs[0]).to.have.property('id', 100);
+        expect(objs[1]).to.have.property('id', 101);
+      });
+
+      it('assigns ids relative to the other items in the list', function () {
+        var objs = [
+          { id: 100 },
+          { id: 200 },
+          { id: 500 },
+          { id: 350 },
+          {},
+        ];
+        AggConfig.ensureIds(objs);
+        expect(objs[0]).to.have.property('id', 100);
+        expect(objs[1]).to.have.property('id', 200);
+        expect(objs[2]).to.have.property('id', 500);
+        expect(objs[3]).to.have.property('id', 350);
+        expect(objs[4]).to.have.property('id', 501);
+      });
+
+      it('uses ::nextId to get the starting value', function () {
+        sinon.stub(AggConfig, 'nextId').returns(534);
+        var objs = AggConfig.ensureIds([{}]);
+        expect(objs[0]).to.have.property('id', 534);
+      });
+
+      it('only calls ::nextId once', function () {
+        var start = 420;
+        sinon.stub(AggConfig, 'nextId').returns(start);
+        var objs = AggConfig.ensureIds([{}, {}, {}, {}, {}, {}, {}]);
+
+        expect(AggConfig.nextId).to.have.property('callCount', 1);
+        objs.forEach(function (obj, i) {
+          expect(obj).to.have.property('id', start + i);
+        });
+      });
+    });
+
+    describe('::nextId', function () {
+      it('accepts a list of objects and picks the next id', function () {
+        var next = AggConfig.nextId([ {id: 100}, {id: 500} ]);
+        expect(next).to.be(501);
+      });
+
+      it('handles an empty list', function () {
+        var next = AggConfig.nextId([]);
+        expect(next).to.be(1);
+      });
+
+      it('fails when the list is not defined', function () {
+        expect(function () {
+          AggConfig.nextId();
+        }).to.throwError();
+      });
+    });
+
+    describe('#toJSON', function () {
+      it('includes the aggs id, params, type and schema', function () {
+        var vis = new Vis(indexPattern, {
+          type: 'histogram',
+          aggs: [
+            {
+              type: 'date_histogram',
+              schema: 'segment'
+            }
+          ]
+        });
+
+        var aggConfig = vis.aggs.byTypeName.date_histogram[0];
+        expect(aggConfig.id).to.be(1);
+        expect(aggConfig.params).to.be.an('object');
+        expect(aggConfig.type).to.be.an(AggType).and.have.property('name', 'date_histogram');
+        expect(aggConfig.schema).to.be.an('object').and.have.property('name', 'segment');
+
+        var state = aggConfig.toJSON();
+        expect(state).to.have.property('id', 1);
+        expect(state.params).to.be.an('object');
+        expect(state).to.have.property('type', 'date_histogram');
+        expect(state).to.have.property('schema', 'segment');
+      });
+    });
   }];
 });

test/unit/specs/components/vis/_agg_configs.js

@@ -54,7 +54,7 @@ define(function (require) {
             type: 'date_histogram',
             schema: 'segment'
           },
-          new AggConfig({
+          new AggConfig(vis, {
             type: 'terms',
             schema: 'split'
           })
@@ -64,6 +64,29 @@ define(function (require) {
         expect(SpiedAggConfig).to.have.property('callCount', 3);
       });
 
+      it('attemps to ensure that all states have an id', function () {
+        var vis = new Vis(indexPattern, {
+          type: 'histogram',
+          aggs: []
+        });
+
+        var states = [
+          {
+            type: 'date_histogram',
+            schema: 'segment'
+          },
+          {
+            type: 'terms',
+            schema: 'split'
+          }
+        ];
+
+        var spy = sinon.spy(SpiedAggConfig, 'ensureIds');
+        var ac = new AggConfigs(vis, states);
+        expect(spy.callCount).to.be(1);
+        expect(spy.firstCall.args[0]).to.be(states);
+      });
+
       describe('defaults', function () {
         var vis;
         beforeEach(function () {