moved discover state logic into SyncedSearch factory, closes #62, closes #44

src/kibana/apps/discover/controllers/discover.js

@@ -1,5 +1,6 @@
 define(function (require) {
   var _ = require('utils/mixins');
+  var angular = require('angular');
 
   var settingsHtml = require('text!../partials/settings.html');
 
@@ -11,9 +12,10 @@ define(function (require) {
   ]);
 
   require('directives/timepicker');
-  require('services/state');
   require('directives/fixed_scroll');
   require('filters/moment');
+  require('apps/settings/services/index_patterns');
+  require('factories/synced_state');
 
   require('routes')
   .when('/discover/:id?', {
@@ -23,20 +25,8 @@ define(function (require) {
       savedSearch: function (savedSearches, $route) {
         return savedSearches.get($route.current.params.id);
       },
-      patternList: function (es, configFile, $location, $q) {
-        // TODO: This is inefficient because it pulls down all of the cached mappings for every
-        // configured pattern instead of only the currently selected one.
-        return es.search({
-          index: configFile.kibanaIndex,
-          type: 'mapping',
-          size: 50000,
-          body: {
-            query: {match_all: {}},
-          }
-        })
-        .then(function (res) {
-          return res.hits.hits;
-        });
+      indexPatternList: function (indexPatterns) {
+        return indexPatterns.getIds();
       }
     }
   });
@@ -50,59 +40,70 @@ define(function (require) {
     { display: 'Yearly', val: 'yearly' }
   ];
 
-  app.controller('discover', function ($scope, config, $q, $route, savedSearches, courier, createNotifier, $location,
-    state, es, configFile) {
-    var notify = createNotifier({
+  app.controller('discover', function ($scope, config, $route, savedSearches, Notifier, $location, SyncedState) {
+    var notify = new Notifier({
       location: 'Discover'
     });
 
     // the saved savedSearch
     var savedSearch = $route.current.locals.savedSearch;
 
+    // list of indexPattern id's
+    var indexPatternList = $route.current.locals.indexPatternList;
+
     // the actual courier.SearchSource
     var searchSource = savedSearch.searchSource;
 
     /* Manage state & url state */
     var initialQuery = searchSource.get('query');
 
-
-    function loadState() {
-      $scope.state = state.get();
-      $scope.state = _.defaults($scope.state, {
-        query: initialQuery ? initialQuery.query_string.query : '',
-        columns: ['_source'],
-        sort: ['_score', 'desc'],
-        index: config.get('defaultIndex')
-      });
-    }
-
-    loadState();
+    var $state = $scope.state = new SyncedState({
+      query: initialQuery ? initialQuery.query_string.query : '',
+      columns: ['_source'],
+      sort: ['_score', 'desc'],
+      index: config.get('defaultIndex')
+    });
 
     // Check that we have any index patterns before going further, and that index being requested
     // exists.
-    if (!$route.current.locals.patternList.length ||
-      !_.find($route.current.locals.patternList, {_id: $scope.state.index})) {
+    if (!indexPatternList || !_.contains(indexPatternList, $state.index)) {
       $location.path('/settings/indices');
       return;
     }
 
-    function init() {
-      setFields();
-      updateDataSource();
-    }
-
     $scope.opts = {
       // number of records to fetch, then paginate through
       sampleSize: 500,
       // max length for summaries in the table
       maxSummaryLength: 100,
       // Index to match
-      index: $scope.state.index,
+      index: $state.index,
       savedSearch: savedSearch,
-      patternList: $route.current.locals.patternList,
+      indexPatternList: indexPatternList,
       time: {}
     };
 
+    var onStateChange = function () {
+      updateDataSource();
+      searchSource.fetch();
+    };
+
+    var init = _.once(function () {
+      return setFields()
+      .then(updateDataSource)
+      .then(function () {
+        // changes to state.columns don't require a refresh
+        var ignore = ['columns'];
+
+        $state.onUpdate().then(function filterStateUpdate(changed) {
+          if (_.difference(changed, ignore).length) onStateChange();
+          $state.onUpdate().then(filterStateUpdate);
+        });
+
+        $scope.$emit('application.load');
+      });
+    });
+
     $scope.opts.saveDataSource = function () {
       savedSearch.id = savedSearch.title;
 
@@ -124,32 +125,10 @@ define(function (require) {
 
     // the index to use when they don't specify one
     $scope.$on('change:config.defaultIndex', function (event, val) {
-      if (!$scope.opts.index) {
-        $scope.opts.index = val;
-        $scope.fetch();
-      }
+      if (!$state.index) $state.index = val;
     });
 
-    // If the URL changes, we re-fetch, no matter what changes.
-    $scope.$on('$locationChangeSuccess', function () {
-      $scope.state = state.get();
-
-      // We have no state, don't try to refresh until we do
-      if (_.isEmpty($scope.state)) return;
-
-      updateDataSource();
-      // TODO: fetch just this savedSearch
-      courier.fetch();
-    });
-
-        // the index to use when they don't specify one
-    $scope.$watch('opts.index', function (val) {
-      if (!val) return;
-      updateDataSource();
-      $scope.fetch();
-    });
-
-    // Bind a result handler. Any time scope.fetch() is executed this gets called
+    // Bind a result handler. Any time searchSource.fetch() is executed this gets called
     // with the results
     searchSource.onResults().then(function onResults(resp) {
       $scope.rows = resp.hits.hits;
@@ -171,19 +150,26 @@ define(function (require) {
       console.log('An error');
     });
 
-    $scope.$on('$destroy', savedSearch.destroy);
+    $scope.$on('$destroy', _.bindKey(searchSource, 'destroy'));
 
-    $scope.getSort = function () {
-      return $scope.state.sort;
+    $scope.fetch = function () {
+      var changed = $state.commit();
+      // when none of the fields updated, we need to call fetch ourselves
+      if (changed.length === 0) onStateChange();
     };
 
-    $scope.setSort = function (field, order) {
-      var sort = {};
-      sort[field] = order;
-      searchSource.sort([sort]);
-      $scope.state.sort = [field, order];
-      $scope.fetch();
-    };
+    // $scope.$watch('state.index', $scope.fetch);
+    // $scope.$watch('state.query', $scope.fetch);
+    $scope.$watch('state.sort', function (sort) {
+      if (!sort) return;
+
+      // get the current sort from {key: val} to ["key", "val"];
+      var currentSort = _.pairs(searchSource.get('sort')).pop();
+
+      // if the searchSource doesn't know, tell it so
+      if (!angular.equals(sort, currentSort)) onStateChange();
+    });
+    // $scope.$watch('state.columns', $scope.fetch);
 
     $scope.toggleConfig = function () {
       // Close if already open
@@ -205,7 +191,7 @@ define(function (require) {
     };
 
     $scope.resetQuery = function () {
-      $scope.state.query = initialQuery ? initialQuery.query_string.query : '';
+      $state.query = initialQuery ? initialQuery.query_string.query : '';
       $scope.fetch();
     };
 
@@ -214,7 +200,7 @@ define(function (require) {
         // set the index on the savedSearch
         searchSource.index($scope.opts.index);
 
-        $scope.state.index = $scope.opts.index;
+        $state.index = $scope.opts.index;
         delete $scope.fields;
         delete $scope.columns;
 
@@ -224,17 +210,14 @@ define(function (require) {
         //$scope.state.columns = $scope.fields = null;
       }
 
-      var sort = {};
-      sort[$scope.state.sort[0]] = $scope.state.sort[1];
-
       searchSource
         .size($scope.opts.sampleSize)
+        .sort(_.zipObject([$state.sort]))
         .query(!$scope.state.query ? null : {
           query_string: {
             query: $scope.state.query
           }
-        })
-        .sort([sort]);
+        });
 
       if (!!$scope.opts.timefield) {
         searchSource
@@ -250,11 +233,6 @@ define(function (require) {
       }
     }
 
-    $scope.fetch = function () {
-      // We only set the state on data refresh
-      state.set($scope.state);
-    };
-
     // This is a hacky optimization for comparing the contents of a large array to a short one.
     function arrayToKeys(array, value) {
       var obj = {};
@@ -265,46 +243,46 @@ define(function (require) {
     }
 
     function setFields() {
-      var fields = _.findLast($scope.opts.patternList, {_id: $scope.opts.index})._source;
+      return searchSource.getFields($scope.opts.index)
+      .then(function (fields) {
+        var currentState = _.transform($scope.fields || [], function (current, field) {
+          current[field.name] = {
+            display: field.display
+          };
+        }, {});
 
-      var currentState = _.transform($scope.fields || [], function (current, field) {
-        current[field.name] = {
-          display: field.display
-        };
-      }, {});
+        if (!fields) return;
 
-      if (!fields) return;
+        var columnObjects = arrayToKeys($scope.state.columns);
 
-      var columnObjects = arrayToKeys($scope.state.columns);
+        $scope.fields = [];
+        $scope.state.columns = $scope.state.columns || [];
 
-      $scope.fields = [];
-      $scope.state.columns = $scope.state.columns || [];
+        // Inject source into list;
+        $scope.fields.push({name: '_source', type: 'source', display: false});
 
-      // Inject source into list;
-      $scope.fields.push({name: '_source', type: 'source', display: false});
+        _(fields)
+          .keys()
+          .sort()
+          .each(function (name) {
+            var field = fields[name];
+            field.name = name;
 
-      _(fields)
-        .keys()
-        .sort()
-        .each(function (name) {
-          var field = fields[name];
-          field.name = name;
+            _.defaults(field, currentState[name]);
+            $scope.fields.push(_.defaults(field, {display: columnObjects[name] || false}));
+          });
 
-          _.defaults(field, currentState[name]);
-          $scope.fields.push(_.defaults(field, {display: columnObjects[name] || false}));
-        });
-
-      // TODO: timefield should be associated with the index pattern, this is a hack
-      // to pick the first date field and use it.
-      var timefields = _.find($scope.fields, {type: 'date'});
-      if (!!timefields) {
-        $scope.opts.timefield = timefields.name;
-      } else {
-        delete $scope.opts.timefield;
-      }
-
-      refreshColumns();
+        // TODO: timefield should be associated with the index pattern, this is a hack
+        // to pick the first date field and use it.
+        var timefields = _.find($scope.fields, {type: 'date'});
+        if (!!timefields) {
+          $scope.opts.timefield = timefields.name;
+        } else {
+          delete $scope.opts.timefield;
+        }
 
+        refreshColumns();
+      });
     }
 
     // TODO: On array fields, negating does not negate the combination, rather all terms
@@ -353,7 +331,10 @@ define(function (require) {
       // If no columns remain, use _source
       if (!$scope.state.columns.length) {
         $scope.toggleField('_source');
+        return;
       }
+
+      $state.commit();
     }
 
     // TODO: Move to utility class
@@ -373,6 +354,5 @@ define(function (require) {
     };
 
     init();
-    $scope.$emit('application.load');
   });
 });

src/kibana/apps/discover/index.html

@@ -41,10 +41,9 @@
           <kbn-table class="table table-condensed"
             rows="rows"
             columns="state.columns"
+            sorting="state.sort"
             refresh="fetch"
-            max-length="opts.maxSummaryLength"
-            get-sort="getSort"
-            set-sort="setSort">
+            max-length="opts.maxSummaryLength">
           </kbn-table>
         </div>
       </div>

src/kibana/apps/discover/partials/settings.html

@@ -17,7 +17,7 @@
       <select
         class="form-control"
         ng-model="opts.index"
-        ng-options="obj._id as obj._id for obj in opts.patternList">
+        ng-options="id as id for id in opts.indexPatternList">
       </select>
       <small>
         Time field: <strong>{{opts.timefield || 'not configured'}}</strong>

src/kibana/controllers/kibana.js

@@ -36,10 +36,12 @@ define(function (require) {
       var orig = Notifier.prototype.fatal;
       return function () {
         orig.apply(this, arguments);
-        $scope.$on('$routeChangeStart', function (event, next) {
+        function forceReload(event, next) {
           // reload using the current route, force re-get
           window.location.reload(false);
-        });
+        }
+        $scope.$on('$routeUpdate', forceReload);
+        $scope.$on('$routeChangeStart', forceReload);
         Notifier.prototype.fatal = orig;
       };
     }());
@@ -54,7 +56,17 @@ define(function (require) {
       config.init()
     ]).then(function () {
       $injector.invoke(function ($rootScope, courier, config, configFile, $timeout, $location) {
-        $scope.apps = configFile.apps;
+        // get/set last path for an app
+        var lastPathFor = function (app, path) {
+          var key = 'lastPath:' + app.id;
+          if (path === void 0) return localStorage.getItem(key);
+          else return localStorage.setItem(key, path);
+        };
+
+        $scope.apps = configFile.apps.map(function (app) {
+          app.lastPath = lastPathFor(app);
+          return app;
+        });
 
         function updateAppData() {
           var route = $location.path().split(/\//);
@@ -62,6 +74,7 @@ define(function (require) {
 
           // Record the last URL w/ state of the app, use for tab.
           app.lastPath = $location.url().substring(1);
+          lastPathFor(app, app.lastPath);
 
           // Set class of container to application-<whateverApp>
           $scope.activeApp = route ? route[1] : null;

src/kibana/directives/table.js

@@ -16,17 +16,17 @@ define(function (require) {
       restrict: 'A',
       scope: {
         columns: '=',
-        getSort: '=',
-        setSort: '=',
+        sorting: '='
       },
       template: headerHtml,
       controller: function ($scope) {
-
         $scope.headerClass = function (column) {
-          if (!$scope.getSort) return [];
-          var sort = $scope.getSort();
-          if (column === sort[0]) {
-            return ['fa', sort[1] === 'asc' ? 'fa-sort-up' : 'fa-sort-down'];
+          var sorting = $scope.sorting;
+
+          if (!sorting) return [];
+
+          if (column === sorting[0]) {
+            return ['fa', sorting[1] === 'asc' ? 'fa-sort-up' : 'fa-sort-down'];
           } else {
             return ['fa', 'fa-sort', 'table-header-sortchange'];
           }
@@ -43,9 +43,8 @@ define(function (require) {
         };
 
         $scope.sort = function (column) {
-          var sort = $scope.getSort();
-          console.log('dir', sort);
-          $scope.setSort(column, sort[1] === 'asc' ? 'desc' : 'asc');
+          var sorting = $scope.sorting || [];
+          $scope.sorting = [column, sorting[1] === 'asc' ? 'desc' : 'asc'];
         };
 
       }
@@ -79,9 +78,8 @@ define(function (require) {
       scope: {
         columns: '=',
         rows: '=',
+        sorting: '=',
         refresh: '=',
-        getSort: '=',
-        setSort: '=',
         maxLength: '=?',
         mapping: '=?'
       },

src/kibana/factories/synced_state.js

@@ -0,0 +1,103 @@
+define(function (require) {
+  var module = require('modules').get('kibana/factories');
+  var angular = require('angular');
+  var _ = require('lodash');
+  var rison = require('utils/rison');
+
+  module.factory('SyncedState', function ($rootScope, $route, $location, Promise) {
+    function SyncedState(defaults) {
+      var state = this;
+
+      var updateHandlers = [];
+      var abortHandlers = [];
+
+      // serialize the defaults so that they are easily used in onPossibleUpdate
+      var defaultRison = rison.encode(defaults);
+
+      // store the route matching regex so we can determine if we match later down the road
+      var routeRegex = $route.current.$$route.regexp;
+      // this will be used to store the state in local storage
+      var routeName = $route.current.$$route.originalPath;
+
+      var set = function (obj) {
+        var changed = [];
+        // all the keys that the object will have at the end
+        var newKeys = Object.keys(obj).concat(baseKeys);
+
+        // the keys that got removed
+        _.difference(Object.keys(state), newKeys).forEach(function (key) {
+          delete state[key];
+          changed.push(key);
+        });
+
+        newKeys.forEach(function (key) {
+          // don't overwrite object methods
+          if (typeof state[key] !== 'function') {
+            if (!angular.equals(state[key], obj[key])) {
+              state[key] = obj[key];
+              changed.push(key);
+            }
+          }
+        });
+
+        if (changed.length) {
+          updateHandlers.splice(0).forEach(function (handler, i, list) {
+            // micro optimizations!
+            handler(list.length > 1 ? _.clone(changed) : changed);
+          });
+        }
+
+        return changed;
+      };
+
+      var onPossibleUpdate = function (qs) {
+        if (routeRegex.test($location.path())) {
+          qs = qs || $location.search();
+
+          if (!qs._r) {
+            qs._r = defaultRison;
+            $location.search(qs);
+          }
+
+          return set(rison.decode(qs._r));
+        }
+      };
+
+      var unwatch = [];
+      unwatch.push($rootScope.$on('$locationChangeSuccess', _.partial(onPossibleUpdate, null)));
+      unwatch.push($rootScope.$on('$locationUpdate', _.partial(onPossibleUpdate, null)));
+
+      this.onUpdate = function () {
+        var defer = Promise.defer();
+
+        updateHandlers.push = defer.resolve;
+        abortHandlers.push = defer.reject;
+
+        return defer.promise;
+      };
+
+      /**
+       * Commit the state as a history item
+       */
+      this.commit = function () {
+        var qs = $location.search();
+        qs._r = rison.encode(this);
+        $location.search(qs);
+        return onPossibleUpdate(qs) || [];
+      };
+
+      this.destroy = function () {
+        unwatch.splice(0).concat(abortHandlers.splice(0)).forEach(function (fn) { fn(); });
+      };
+
+      // track the "known" keys that state objects have
+      var baseKeys = Object.keys(this);
+
+      // set the defaults on state
+      onPossibleUpdate();
+    }
+
+    return SyncedState;
+  });
+
+});

src/kibana/partials/table.html

@@ -1,5 +1,5 @@
 <table class="table">
-  <thead kbn-table-header columns="columns" get-sort="getSort" set-sort="setSort"></thead>
+  <thead kbn-table-header columns="columns" sorting="sorting"</thead>
   <tbody></tbody>
 </table>
 <kbn-infinite-scroll more="addRows"></kbn-infinite-scroll>

src/kibana/require.config.js

@@ -45,7 +45,10 @@ require.config({
     'angular-mocks': ['angular'],
     'elasticsearch': ['angular'],
     'angular-bootstrap': ['angular'],
-    'angular-bindonce': ['angular']
+    'angular-bindonce': ['angular'],
+    'utils/rison': {
+      exports: 'rison'
+    }
   },
   waitSeconds: 60
 });

src/kibana/services/state.js

@@ -1,20 +0,0 @@
-define(function (require) {
-  var _ = require('lodash');
-  require('utils/rison');
-
-  require('modules')
-    .get('kibana/services')
-    .service('state', function ($location) {
-      this.set = function (state) {
-        var search = $location.search();
-        search._r = rison.encode(state);
-        $location.search(search);
-        return search;
-      };
-
-      this.get = function () {
-        var search = $location.search();
-        return _.isUndefined(search._r) ? {} : rison.decode(search._r);
-      };
-    });
-});

test/unit/specs/services/state.js

@@ -1,52 +0,0 @@
-define(function (require) {
-  var angular = require('angular');
-  var mocks = require('angular-mocks');
-  var _ = require('lodash');
-  var $ = require('jquery');
-
-  // Load the kibana app dependencies.
-  require('angular-route');
-
-  // Load the code for the directive
-  require('services/state');
-
-  describe('State service', function () {
-    var state, location;
-
-    beforeEach(function () {
-      module('kibana/services');
-
-      // Create the scope
-      inject(function (_state_, $location) {
-
-        state = _state_;
-        location = $location;
-
-      });
-
-    });
-
-    afterEach(function () {
-      location.search({});
-    });
-
-    it('should have no state by default', function (done) {
-      expect(state.get()).to.eql({});
-      done();
-    });
-
-    it('should have a set(Object) that writes state to the search string', function (done) {
-      state.set({foo: 'bar'});
-      expect(location.search()._r).to.be('(foo:bar)');
-      done();
-    });
-
-    it('should have a get() that deserializes rison from the search string', function (done) {
-      location.search({_r: '(foo:bar)'});
-      expect(state.get()).to.eql({foo: 'bar'});
-      done();
-    });
-
-  });
-
-});