From 382a12c0fd2da62525c66a9a440a4c4fceafc4c7 Mon Sep 17 00:00:00 2001 From: Andrej Ocenas Date: Sat, 19 Jan 2019 16:58:26 +0100 Subject: [PATCH] Add loop counter for full refresh in playlist --- public/app/features/playlist/playlist_srv.ts | 22 ++-- .../playlist/specs/playlist_srv.test.ts | 103 ++++++++++++++++++ 2 files changed, 118 insertions(+), 7 deletions(-) create mode 100644 public/app/features/playlist/specs/playlist_srv.test.ts diff --git a/public/app/features/playlist/playlist_srv.ts b/public/app/features/playlist/playlist_srv.ts index 9d3b635a1e5..0a80ce0cdf0 100644 --- a/public/app/features/playlist/playlist_srv.ts +++ b/public/app/features/playlist/playlist_srv.ts @@ -4,12 +4,13 @@ import appEvents from 'app/core/app_events'; import _ from 'lodash'; import { toUrlParams } from 'app/core/utils/url'; -class PlaylistSrv { +export class PlaylistSrv { private cancelPromise: any; - private dashboards: any; + private dashboards: Array<{ uri: string }>; private index: number; - private interval: any; + private interval: number; private startUrl: string; + private numberOfLoops = 0; isPlaying: boolean; /** @ngInject */ @@ -20,8 +21,15 @@ class PlaylistSrv { const playedAllDashboards = this.index > this.dashboards.length - 1; if (playedAllDashboards) { - window.location.href = this.startUrl; - return; + this.numberOfLoops++; + + // This does full reload of the playlist to keep memory in check due to existing leaks but at the same time + // we do not want page to flicker after each full loop. + if (this.numberOfLoops >= 3) { + window.location.href = this.startUrl; + return; + } + this.index = 0; } const dash = this.dashboards[this.index]; @@ -46,8 +54,8 @@ class PlaylistSrv { this.index = 0; this.isPlaying = true; - this.backendSrv.get(`/api/playlists/${playlistId}`).then(playlist => { - this.backendSrv.get(`/api/playlists/${playlistId}/dashboards`).then(dashboards => { + return this.backendSrv.get(`/api/playlists/${playlistId}`).then(playlist => { + return this.backendSrv.get(`/api/playlists/${playlistId}/dashboards`).then(dashboards => { this.dashboards = dashboards; this.interval = kbn.interval_to_ms(playlist.interval); this.next(); diff --git a/public/app/features/playlist/specs/playlist_srv.test.ts b/public/app/features/playlist/specs/playlist_srv.test.ts new file mode 100644 index 00000000000..e6b7671c964 --- /dev/null +++ b/public/app/features/playlist/specs/playlist_srv.test.ts @@ -0,0 +1,103 @@ +import { PlaylistSrv } from '../playlist_srv'; + +const dashboards = [{ uri: 'dash1' }, { uri: 'dash2' }]; + +const createPlaylistSrv = (): [PlaylistSrv, { url: jest.MockInstance }] => { + const mockBackendSrv = { + get: jest.fn(url => { + switch (url) { + case '/api/playlists/1': + return Promise.resolve({ interval: '1s' }); + case '/api/playlists/1/dashboards': + return Promise.resolve(dashboards); + default: + throw new Error(`Unexpected url=${url}`); + } + }), + }; + + const mockLocation = { + url: jest.fn(), + search: () => ({}), + }; + + const mockTimeout = jest.fn(); + (mockTimeout as any).cancel = jest.fn(); + + return [new PlaylistSrv(mockLocation, mockTimeout, mockBackendSrv), mockLocation]; +}; + +const mockWindowLocation = (): [jest.MockInstance, () => void] => { + const oldLocation = window.location; + const hrefMock = jest.fn(); + + // JSDom defines window in a way that you cannot tamper with location so this seems to be the only way to change it. + // https://github.com/facebook/jest/issues/5124#issuecomment-446659510 + delete window.location; + window.location = {} as any; + + // Only mocking href as that is all this test needs, but otherwise there is lots of things missing, so keep that + // in mind if this is reused. + Object.defineProperty(window.location, 'href', { + set: hrefMock, + get: hrefMock, + }); + const unmock = () => { + window.location = oldLocation; + }; + return [hrefMock, unmock]; +}; + +describe('PlaylistSrv', () => { + let srv: PlaylistSrv; + let mockLocationService: { url: jest.MockInstance }; + let hrefMock: jest.MockInstance; + let unmockLocation: () => void; + const initialUrl = 'http://localhost/playlist'; + + beforeEach(() => { + [srv, mockLocationService] = createPlaylistSrv(); + [hrefMock, unmockLocation] = mockWindowLocation(); + + // This will be cached in the srv when start() is called + hrefMock.mockReturnValue(initialUrl); + }); + + afterEach(() => { + unmockLocation(); + }); + + it('runs all dashboards in cycle and reloads page after 3 cycles', async () => { + await srv.start(1); + + for (let i = 0; i < 6; i++) { + expect(mockLocationService.url).toHaveBeenLastCalledWith(`dashboard/${dashboards[i % 2].uri}?`); + srv.next(); + } + + expect(hrefMock).toHaveBeenCalledTimes(2); + expect(hrefMock).toHaveBeenLastCalledWith(initialUrl); + }); + + it('keeps the refresh counter value after restarting', async () => { + await srv.start(1); + + // 1 complete loop + for (let i = 0; i < 3; i++) { + expect(mockLocationService.url).toHaveBeenLastCalledWith(`dashboard/${dashboards[i % 2].uri}?`); + srv.next(); + } + + srv.stop(); + await srv.start(1); + + // Another 2 loops + for (let i = 0; i < 4; i++) { + expect(mockLocationService.url).toHaveBeenLastCalledWith(`dashboard/${dashboards[i % 2].uri}?`); + srv.next(); + } + + expect(hrefMock).toHaveBeenCalledTimes(3); + expect(hrefMock).toHaveBeenLastCalledWith(initialUrl); + }); +});