github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-23 17:28:26 -04:00
Code Issues Projects Releases Packages Wiki Activity

[object.routes] move away from precomputed routes

Browse source 
A circular dependency was discovered in the Doc AppController tests, because it was stubbing
$route and required the IndexPattern class. The dependency path was something like:

$route -> IndexPattern -> Field -> kbnUrl -> $route

While investigating solutions for this I noticed two things: we require kbnUrl both
in the object classes where urls are precomputed and in the controllers/directives
where we redirect/change the url, we needed error-prone logic to recompute the urls
when their dependencies change.

To simplify things and solve the circular dep, the indexPattern and field objects now just
expose a .routes property that has named route patterns. These patterns expect to receive
the object they describe in order to create the route. kbnUrl was also updated with helper
functions for this:

```
kbnUrl.getRouteUrl(indexPattern, 'edit') -> '#/settings/indices/logstash-*'
kbnUrl.redirectToRoute(indexPattern, 'indexedFields')
kbnUrl.changeToRoute(field, 'edit')
```
This commit is contained in:
Spencer Alger 2015-04-30 02:53:59 -07:00
parent e4b07e8689
commit 9caa81482c
10 changed files with 166 additions and 111 deletions
Download patch file Download diff file Expand all files Collapse all files

4
src/kibana/components/field_editor/field_editor.js
View file

@ -7,7 +7,6 @@ define(function (require) {
.directive('fieldEditor', function (Private, $sce) {
var _ = require('lodash');
var fieldFormats = Private(require('registry/field_formats'));
var Field = Private(require('components/index_patterns/_field'));
var scriptingInfo = $sce.trustAsHtml(require('text!components/field_editor/scripting_info.html'));
var scriptingWarning = $sce.trustAsHtml(require('text!components/field_editor/scripting_warning.html'));
@ -110,8 +109,7 @@ define(function (require) {
}
function redirectAway() {
var route = self.indexPattern.routes[self.field.scripted ? 'scriptedFields' : 'indexedFields'];
kbnUrl.change(route);
kbnUrl.changeToRoute(self.indexPattern, self.field.scripted ? 'scriptedFields' : 'indexedFields');
}
function getFieldFormatType() {

16
src/kibana/components/index_patterns/_field.js
View file

@ -1,7 +1,6 @@
define(function (require) {
return function FieldObjectProvider(Private, shortDotsFilter, $rootScope, Notifier, kbnUrl) {
return function FieldObjectProvider(Private, shortDotsFilter, $rootScope, Notifier) {
var notify = new Notifier({ location: 'IndexPattern Field' });
var FieldFormat = Private(require('components/index_patterns/_field_format/FieldFormat'));
var fieldTypes = Private(require('components/index_patterns/_field_types'));
var fieldFormats = Private(require('registry/field_formats'));
@ -62,22 +61,13 @@ define(function (require) {
// computed values
obj.comp('indexPattern', indexPattern);
obj.comp('displayName', shortDotsFilter(spec.name));
obj.comp('editRoute', Field.getEditRoute(indexPattern, spec));
obj.comp('$$spec', spec);
return obj.create();
}
Field.getEditRoute = function (indexPattern, field) {
if (!field.name || !indexPattern.id) return;
return kbnUrl.eval(
'/settings/indices/{{index}}/field/{{field}}',
{
index: indexPattern.id,
field: field.name
}
);
Field.prototype.routes = {
edit: '/settings/indices/{{indexPattern.id}}/field/{{name}}'
};
return Field;

19
src/kibana/components/index_patterns/_index_pattern.js
View file

@ -5,7 +5,6 @@ define(function (require) {
var angular = require('angular');
var fieldformats = Private(require('registry/field_formats'));
var kbnUrl = Private(require('components/url/url'));
var getIds = Private(require('components/index_patterns/_get_ids'));
var mapper = Private(require('components/index_patterns/_mapper'));
var intervals = Private(require('components/index_patterns/_intervals'));
@ -209,16 +208,8 @@ define(function (require) {
return body;
};
// refresh the id and editRoute
function setId(id) {
self.id = id;
self.routes = !id ? {} : {
edit: kbnUrl.eval('/settings/indices/{{id}}', self),
addField: kbnUrl.eval('/settings/indices/{{id}}/create-field', self),
indexedFields: kbnUrl.eval('/settings/indices/{{id}}?_a=(tab:indexedFields)', self),
scriptedFields: kbnUrl.eval('/settings/indices/{{id}}?_a=(tab:scriptedFields)', self)
};
return self.id;
return self.id = id;
}
self.create = function () {
@ -271,6 +262,14 @@ define(function (require) {
self.formatHit = formatHit(self, fieldformats.getDefaultInstance('string'));
self.formatField = self.formatHit.formatField;
}
IndexPattern.prototype.routes = {
edit: '/settings/indices/{{id}}',
addField: '/settings/indices/{{id}}/create-field',
indexedFields: '/settings/indices/{{id}}?_a=(tab:indexedFields)',
scriptedFields: '/settings/indices/{{id}}?_a=(tab:scriptedFields)'
};
return IndexPattern;
};
});

192
src/kibana/components/url/url.js
View file

@ -1,88 +1,74 @@
define(function (require) {
var _ = require('lodash');
require('filters/uriescape');
require('filters/rison');
require('modules').get('kibana/url')
.service('kbnUrl', function (Private) { return Private(KbnUrlProvider); });
function KbnUrlProvider($route, $location, $rootScope, globalState, $parse, getAppState) {
var self = this;
var reloading;
var unbindListener;
var rison = require('utils/rison');
var _ = require('lodash');
require('filters/uriescape');
require('filters/rison');
/**
* Navigate to a url
*
* @param {String} url - the new url, can be a template. See #eval
* @param {Object} [paramObj] - optional set of parameters for the url template
* @return {undefined}
*/
self.change = function (url, paramObj) {
self._changeLocation('url', url, paramObj);
};
/**
* Same as #change except only changes the url's path,
* leaving the search string and such intact
*
* @param {String} path - the new path, can be a template. See #eval
* @param {Object} [paramObj] - optional set of parameters for the path template
* @return {undefined}
*/
self.changePath = function (path, paramObj) {
self._changeLocation('path', path, paramObj);
};
/**
* Same as #change except that it removes the current url from history
*
* @param {String} url - the new url, can be a template. See #eval
* @param {Object} [paramObj] - optional set of parameters for the url template
* @return {undefined}
*/
self.redirect = function (url, paramObj) {
self._changeLocation('url', url, paramObj, true);
};
/**
* Same as #redirect except only changes the url's path,
* leaving the search string and such intact
*
* @param {String} path - the new path, can be a template. See #eval
* @param {Object} [paramObj] - optional set of parameters for the path template
* @return {undefined}
*/
self.redirectPath = function (path, paramObj) {
self._changeLocation('path', path, paramObj, true);
};
self._changeLocation = function (type, url, paramObj, replace) {
var prev = {
path: $location.path(),
search: $location.search()
};
url = self.eval(url, paramObj);
$location[type](url);
if (replace) $location.replace();
var next = {
path: $location.path(),
search: $location.search()
};
if (self.shouldAutoReload(next, prev)) {
var appState = getAppState();
if (appState) appState.destroy();
reloading = $rootScope.$on('$locationChangeSuccess', function () {
// call the "unlisten" function returned by $on
reloading();
reloading = false;
$route.reload();
});
}
};
self.eval = function (url, paramObj) {
/**
* Evaluate a url template. templates can contain double-curly wrapped
* expressions that are evaluated in the context of the paramObj
*
* @param {String} template - the url template to evaluate
* @param {Object} [paramObj] - the variables to expose to the template
* @return {String} - the evaluated result
* @throws {Error} If any of the expressions can't be parsed.
*/
self.eval = function (template, paramObj) {
paramObj = paramObj || {};
return parseUrlPrams(url, paramObj);
};
self.getRoute = function () {
return $route.current && $route.current.$$route;
};
self.shouldAutoReload = function (next, prev) {
if (reloading) return false;
var route = self.getRoute();
if (!route) return false;
if (next.path !== prev.path) return false;
var reloadOnSearch = route.reloadOnSearch;
var searchSame = _.isEqual(next.search, prev.search);
return (reloadOnSearch && searchSame) || !reloadOnSearch;
};
function parseUrlPrams(url, paramObj) {
return url.replace(/\{\{([^\}]+)\}\}/g, function (match, expr) {
return template.replace(/\{\{([^\}]+)\}\}/g, function (match, expr) {
// remove filters
var key = expr.split('|')[0].trim();
@ -101,7 +87,91 @@ define(function (require) {
return $parse(expr)(paramObj);
});
}
};
/**
* convert an object's route to a full url.
*
* @param {Object} obj - any object that list's it's routes at obj.routes{}
* @param {string} route - the route name
* @return {string} - the computed url
*/
self.getRouteUrl = function (obj, route) {
var template = obj && obj.routes && obj.routes[route];
if (template) return '#' + self.eval(template, obj);
};
/**
* Similar to getRouteUrl, supports objects which list their routes,
* and redirects to the named route. See #redirect
*
* @param {Object} obj - any object that list's it's routes at obj.routes{}
* @param {string} route - the route name
* @return {undefined}
*/
self.redirectToRoute = function (obj, route) {
self.redirect(self.getRouteUrl(obj, route));
};
/**
* Similar to getRouteUrl, supports objects which list their routes,
* and changes the url to the named route. See #change
*
* @param {Object} obj - any object that list's it's routes at obj.routes{}
* @param {string} route - the route name
* @return {undefined}
*/
self.changeToRoute = function (obj, route) {
self.change(self.getRouteUrl(obj, route));
};
/////
// private api
/////
var reloading;
self._changeLocation = function (type, url, paramObj, replace) {
var prev = {
path: $location.path(),
search: $location.search()
};
url = self.eval(url, paramObj);
$location[type](url);
if (replace) $location.replace();
var next = {
path: $location.path(),
search: $location.search()
};
if (self._shouldAutoReload(next, prev)) {
var appState = getAppState();
if (appState) appState.destroy();
reloading = $rootScope.$on('$locationChangeSuccess', function () {
// call the "unlisten" function returned by $on
reloading();
reloading = false;
$route.reload();
});
}
};
self._shouldAutoReload = function (next, prev) {
if (reloading) return false;
var route = $route.current && $route.current.$$route;
if (!route) return false;
if (next.path !== prev.path) return false;
var reloadOnSearch = route.reloadOnSearch;
var searchSame = _.isEqual(next.search, prev.search);
return (reloadOnSearch && searchSame) || !reloadOnSearch;
};
}
return KbnUrlProvider;

1
src/kibana/plugins/settings/sections/indices/_edit.js
View file

@ -22,6 +22,7 @@ define(function (require) {
var $state = $scope.state = new AppState();
var refreshKibanaIndex = Private(require('plugins/settings/sections/indices/_refresh_kibana_index'));
$scope.kbnUrl = Private(require('components/url/url'));
$scope.indexPattern = $route.current.locals.indexPattern;
docTitle.change($scope.indexPattern.id);
var otherIds = _.without($route.current.locals.indexPatternIds, $scope.indexPattern.id);

2
src/kibana/plugins/settings/sections/indices/_field_controls.html
View file

@ -1,5 +1,5 @@
<div class="actions">
<a ng-href="#{{ field.editRoute }}" aria-label="Edit" class="btn btn-xs btn-default">
<a ng-href="{{ kbnUrl.getRouteUrl(field, 'edit') }}" aria-label="Edit" class="btn btn-xs btn-default">
<span class="sr-only">Edit</span>
<i aria-hidden="true" class="fa fa-pencil"></i>
</a>

7
src/kibana/plugins/settings/sections/indices/_field_editor.js
View file

@ -14,9 +14,10 @@ define(function (require) {
}
},
controllerAs: 'fieldSettings',
controller: function FieldEditorPageController($route, Private, Notifier, kbnUrl, docTitle) {
controller: function FieldEditorPageController($route, Private, Notifier, docTitle) {
var Field = Private(require('components/index_patterns/_field'));
var notify = new Notifier({ location: 'Field Editor' });
var kbnUrl = Private(require('components/url/url'));
this.mode = $route.current.mode;
@ -29,7 +30,7 @@ define(function (require) {
if (!this.field) {
notify.error(this.indexPattern + ' does not have a "' + fieldName + '" field.');
kbnUrl.redirect(this.indexPattern.routes.edit);
kbnUrl.redirectToRoute(this.indexPattern, 'edit');
return;
}
@ -46,7 +47,7 @@ define(function (require) {
docTitle.change([this.field.name || 'New Scripted Field', this.indexPattern.id]);
this.goBack = function () {
kbnUrl.change(this.indexPattern.editUrl);
kbnUrl.changeToRoute(this.indexPattern, 'edit');
};
}
});

9
test/unit/specs/apps/doc/doc.js
View file

@ -1,13 +1,11 @@
define(function (require) {
var angular = require('angular');
var $ = require('jquery');
// Load the kibana app dependencies.
require('angular-route');
require('plugins/doc/index');
var $scope, createController, $route, timefilter;
var $scope, createController, timefilter;
var init = function (index, type, id) {
@ -67,11 +65,8 @@ define(function (require) {
});
// Create the scope
inject(function ($rootScope, $controller, _$route_, _timefilter_) {
$route = _$route_;
inject(function ($rootScope, $controller, _timefilter_) {
$scope = $rootScope.$new();
timefilter = _timefilter_;
createController = function () {

25
test/unit/specs/services/url.js
View file

@ -2,7 +2,6 @@ define(function (require) {
var sinon = require('test_utils/auto_release_sinon');
var faker = require('faker');
var _ = require('lodash');
var rison = require('utils/rison');
// global vars, injected and mocked in init()
var kbnUrl;
@ -328,7 +327,7 @@ define(function (require) {
});
it('should call replace on $location', function () {
sinon.stub(kbnUrl, 'shouldAutoReload').returns(false);
sinon.stub(kbnUrl, '_shouldAutoReload').returns(false);
sinon.stub($location, 'replace');
expect($location.replace.callCount).to.be(0);
@ -363,7 +362,7 @@ define(function (require) {
});
it('should call replace on $location', function () {
sinon.stub(kbnUrl, 'shouldAutoReload').returns(false);
sinon.stub(kbnUrl, '_shouldAutoReload').returns(false);
sinon.stub($location, 'replace');
expect($location.replace.callCount).to.be(0);
@ -372,7 +371,7 @@ define(function (require) {
});
});
describe('shouldAutoReload', function () {
describe('_shouldAutoReload', function () {
var next;
var prev;
@ -390,7 +389,7 @@ define(function (require) {
it('returns false if the passed url doesn\'t match the current route', function () {
next.path = '/not current';
expect(kbnUrl.shouldAutoReload(next, prev)).to.be(false);
expect(kbnUrl._shouldAutoReload(next, prev)).to.be(false);
});
describe('if the passed url does match the route', function () {
@ -398,14 +397,14 @@ define(function (require) {
describe('and the path is the same', function () {
describe('and the search params are the same', function () {
it('returns true', function () {
expect(kbnUrl.shouldAutoReload(next, prev)).to.be(true);
expect(kbnUrl._shouldAutoReload(next, prev)).to.be(true);
});
});
describe('but the search params are different', function () {
it('returns false', function () {
next.search = {};
prev.search = { q: 'search term' };
expect(kbnUrl.shouldAutoReload(next, prev)).to.be(false);
expect(kbnUrl._shouldAutoReload(next, prev)).to.be(false);
});
});
});
@ -417,14 +416,14 @@ define(function (require) {
describe('and the search params are the same', function () {
it('returns false', function () {
expect(kbnUrl.shouldAutoReload(next, prev)).to.be(false);
expect(kbnUrl._shouldAutoReload(next, prev)).to.be(false);
});
});
describe('but the search params are different', function () {
it('returns false', function () {
next.search = {};
prev.search = { q: 'search term' };
expect(kbnUrl.shouldAutoReload(next, prev)).to.be(false);
expect(kbnUrl._shouldAutoReload(next, prev)).to.be(false);
});
});
});
@ -438,14 +437,14 @@ define(function (require) {
describe('and the path is the same', function () {
describe('and the search params are the same', function () {
it('returns true', function () {
expect(kbnUrl.shouldAutoReload(next, prev)).to.be(true);
expect(kbnUrl._shouldAutoReload(next, prev)).to.be(true);
});
});
describe('but the search params are different', function () {
it('returns true', function () {
next.search = {};
prev.search = { q: 'search term' };
expect(kbnUrl.shouldAutoReload(next, prev)).to.be(true);
expect(kbnUrl._shouldAutoReload(next, prev)).to.be(true);
});
});
});
@ -457,14 +456,14 @@ define(function (require) {
describe('and the search params are the same', function () {
it('returns false', function () {
expect(kbnUrl.shouldAutoReload(next, prev)).to.be(false);
expect(kbnUrl._shouldAutoReload(next, prev)).to.be(false);
});
});
describe('but the search params are different', function () {
it('returns false', function () {
next.search = {};
prev.search = { q: 'search term' };
expect(kbnUrl.shouldAutoReload(next, prev)).to.be(false);
expect(kbnUrl._shouldAutoReload(next, prev)).to.be(false);
});
});
});

2
test/utils/stub_index_pattern.js
View file

@ -3,6 +3,7 @@ define(function (require) {
var _ = require('lodash');
var sinon = require('sinon/sinon');
var IndexedArray = require('utils/indexed_array/index');
var IndexPattern = require('components/index_patterns/_index_pattern');
var flattenHit = require('components/index_patterns/_flatten_hit');
var getComputedFields = require('components/index_patterns/_get_computed_fields');
@ -19,6 +20,7 @@ define(function (require) {
this.flattenHit = _.partial(flattenHit, this);
this.metaFields = ['_id', '_type', '_source'];
this.fieldFormatMap = {};
this.routes = IndexPattern.prototype.routes;
this.fields = new IndexedArray({
index: ['name'],