maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[ML] Fix cleanup of mlAnomaliesTableService listeners in Time Series Viewer. (#25967)

Browse source 
- A missing call to componentWillUnmount() in the Single Series Viewer didn't properly clean up the listeners for mlAnomaliesTableService so when switching to the Anomaly Explorer the page would crash if the user hovered the Anomaly Table.
- This fixes it by calling ReactDOM.unmountComponentAtNode(element[0]); in element.on('$destroy', () => { ... }) to trigger the cleanup.
- Additionally, as a safety measure, mlChartTooltipService.show() now silently fails if target is undefined.
This commit is contained in:
Walter Rafelsberger 2018-11-21 11:32:23 +01:00 committed by GitHub
parent ef4fa62f51
commit ac9c375662
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 72 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
x-pack/plugins/ml/public/components/chart_tooltip/__tests__/chart_tooltip.js
View file

@ -15,4 +15,10 @@ describe('ML - mlChartTooltipService', () => {
expect(mlChartTooltipService.hide).to.be.a('function');
});
it('should fail silently when target is not defined', () => {
mlChartTooltipService.element = {};
expect(() => {
mlChartTooltipService.show('', undefined);
}).to.not.throwError('Call to show() should fail silently.');
});
});

2
x-pack/plugins/ml/public/components/chart_tooltip/chart_tooltip_service.js
View file

@ -18,7 +18,7 @@ export const mlChartTooltipService = {
};
mlChartTooltipService.show = function (contents, target, offset = { x: 0, y: 0 }) {
if (this.element === null) {
if (this.element === null || typeof target === 'undefined') {
return;
}

62
x-pack/plugins/ml/public/timeseriesexplorer/__tests__/timeseries_chart_directive.js Normal file
View file

@ -0,0 +1,62 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import ngMock from 'ng_mock';
import expect from 'expect.js';
import sinon from 'sinon';
import { TimeseriesChart } from '../timeseries_chart';
describe('ML - <ml-timeseries-chart>', () => {
let $scope;
let $compile;
let $element;
beforeEach(ngMock.module('kibana'));
beforeEach(() => {
ngMock.inject(function ($injector) {
$compile = $injector.get('$compile');
const $rootScope = $injector.get('$rootScope');
$scope = $rootScope.$new();
});
});
afterEach(() => {
$scope.$destroy();
});
it('Plain initialization doesn\'t throw an error', () => {
// this creates a dummy DOM element with class 'ml-timeseries-chart' as a direct child of
// the <body> tag so the directive can find it in the DOM to create the resizeChecker.
const mockClassedElement = document.createElement('div');
mockClassedElement.classList.add('ml-timeseries-chart');
document.getElementsByTagName('body')[0].append(mockClassedElement);
// spy the TimeseriesChart component's unmount method to be able to test if it was called
const componentWillUnmountSpy = sinon.spy(TimeseriesChart.prototype, 'componentWillUnmount');
$element = $compile('<ml-timeseries-chart show-forecast="true" />')($scope);
const scope = $element.isolateScope();
// sanity test to check if directive picked up the attribute for its scope
expect(scope.showForecast).to.equal(true);
// componentWillUnmount() should not have been called so far
expect(componentWillUnmountSpy.callCount).to.equal(0);
// remove $element to trigger $destroy() callback
$element.remove();
// componentWillUnmount() should now have been called once
expect(componentWillUnmountSpy.callCount).to.equal(1);
componentWillUnmountSpy.restore();
// clean up the dummy DOM element
mockClassedElement.parentNode.removeChild(mockClassedElement);
});
});

3
x-pack/plugins/ml/public/timeseriesexplorer/timeseries_chart_directive.js
View file

@ -90,6 +90,9 @@ module.directive('mlTimeseriesChart', function () {
element.on('$destroy', () => {
resizeChecker.destroy();
// unmountComponentAtNode() needs to be called so mlAnomaliesTableService listeners within
// the TimeseriesChart component get unwatched properly.
ReactDOM.unmountComponentAtNode(element[0]);
scope.$destroy();
});