[Screenshot Mode] Prevent newsfeed plugin from fetching items (#101256)

* - updated newsfeed to depend on low-level screenshotmode plugin
- added new NeverFetch driver that will never resolve to true
  for new fetch requests
- added plugin-level test for expected behaviour

* added screenshotmode contract to start!

* address pr feedback

* removed old test and updated test description

* update use of jest.fn to tidy up code

* simplify jest tests and remove unnecessary async

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Jean-Louis Leysens 2021-06-15 13:56:41 +02:00 committed by GitHub
parent bcb941acea
commit 8b6608d284
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 194 additions and 36 deletions

View file

@ -2,5 +2,6 @@
"id": "newsfeed",
"version": "kibana",
"server": true,
"ui": true
"ui": true,
"requiredPlugins": ["screenshotMode"]
}

View file

@ -8,6 +8,7 @@
import { storageMock } from './storage.mock';
import { driverMock } from './driver.mock';
import { NeverFetchNewsfeedApiDriver } from './never_fetch_driver';
export const storageInstanceMock = storageMock.create();
jest.doMock('./storage', () => ({
@ -18,3 +19,7 @@ export const driverInstanceMock = driverMock.create();
jest.doMock('./driver', () => ({
NewsfeedApiDriver: jest.fn().mockImplementation(() => driverInstanceMock),
}));
jest.doMock('./never_fetch_driver', () => ({
NeverFetchNewsfeedApiDriver: jest.fn(() => new NeverFetchNewsfeedApiDriver()),
}));

View file

@ -7,12 +7,16 @@
*/
import { driverInstanceMock, storageInstanceMock } from './api.test.mocks';
import moment from 'moment';
import { getApi } from './api';
import { TestScheduler } from 'rxjs/testing';
import { FetchResult, NewsfeedPluginBrowserConfig } from '../types';
import { take } from 'rxjs/operators';
import { NewsfeedApiDriver as MockNewsfeedApiDriver } from './driver';
import { NeverFetchNewsfeedApiDriver as MockNeverFetchNewsfeedApiDriver } from './never_fetch_driver';
const kibanaVersion = '8.0.0';
const newsfeedId = 'test';
@ -46,6 +50,8 @@ describe('getApi', () => {
afterEach(() => {
storageInstanceMock.isAnyUnread$.mockReset();
driverInstanceMock.fetchNewsfeedItems.mockReset();
(MockNewsfeedApiDriver as jest.Mock).mockClear();
(MockNeverFetchNewsfeedApiDriver as jest.Mock).mockClear();
});
it('merges the newsfeed and unread observables', () => {
@ -60,7 +66,7 @@ describe('getApi', () => {
a: createFetchResult({ feedItems: ['item' as any] }),
})
);
const api = getApi(createConfig(1000), kibanaVersion, newsfeedId);
const api = getApi(createConfig(1000), kibanaVersion, newsfeedId, false);
expectObservable(api.fetchResults$.pipe(take(1))).toBe('(a|)', {
a: createFetchResult({
@ -83,7 +89,7 @@ describe('getApi', () => {
a: createFetchResult({ feedItems: ['item' as any] }),
})
);
const api = getApi(createConfig(2), kibanaVersion, newsfeedId);
const api = getApi(createConfig(2), kibanaVersion, newsfeedId, false);
expectObservable(api.fetchResults$.pipe(take(2))).toBe('a-(b|)', {
a: createFetchResult({
@ -111,7 +117,7 @@ describe('getApi', () => {
a: createFetchResult({}),
})
);
const api = getApi(createConfig(10), kibanaVersion, newsfeedId);
const api = getApi(createConfig(10), kibanaVersion, newsfeedId, false);
expectObservable(api.fetchResults$.pipe(take(2))).toBe('a--(b|)', {
a: createFetchResult({
@ -123,4 +129,16 @@ describe('getApi', () => {
});
});
});
it('uses the news feed API driver if in not screenshot mode', () => {
getApi(createConfig(10), kibanaVersion, newsfeedId, false);
expect(MockNewsfeedApiDriver).toHaveBeenCalled();
expect(MockNeverFetchNewsfeedApiDriver).not.toHaveBeenCalled();
});
it('uses the never fetch news feed API driver if in not screenshot mode', () => {
getApi(createConfig(10), kibanaVersion, newsfeedId, true);
expect(MockNewsfeedApiDriver).not.toHaveBeenCalled();
expect(MockNeverFetchNewsfeedApiDriver).toHaveBeenCalled();
});
});

View file

@ -11,6 +11,7 @@ import { map, catchError, filter, mergeMap, tap } from 'rxjs/operators';
import { i18n } from '@kbn/i18n';
import { FetchResult, NewsfeedPluginBrowserConfig } from '../types';
import { NewsfeedApiDriver } from './driver';
import { NeverFetchNewsfeedApiDriver } from './never_fetch_driver';
import { NewsfeedStorage } from './storage';
export enum NewsfeedApiEndpoint {
@ -40,13 +41,23 @@ export interface NewsfeedApi {
export function getApi(
config: NewsfeedPluginBrowserConfig,
kibanaVersion: string,
newsfeedId: string
newsfeedId: string,
isScreenshotMode: boolean
): NewsfeedApi {
const userLanguage = i18n.getLocale();
const fetchInterval = config.fetchInterval.asMilliseconds();
const mainInterval = config.mainInterval.asMilliseconds();
const storage = new NewsfeedStorage(newsfeedId);
const driver = new NewsfeedApiDriver(kibanaVersion, userLanguage, fetchInterval, storage);
const mainInterval = config.mainInterval.asMilliseconds();
const createNewsfeedApiDriver = () => {
if (isScreenshotMode) {
return new NeverFetchNewsfeedApiDriver();
}
const userLanguage = i18n.getLocale();
const fetchInterval = config.fetchInterval.asMilliseconds();
return new NewsfeedApiDriver(kibanaVersion, userLanguage, fetchInterval, storage);
};
const driver = createNewsfeedApiDriver();
const results$ = timer(0, mainInterval).pipe(
filter(() => driver.shouldFetch()),

View file

@ -10,6 +10,7 @@ import moment from 'moment';
import * as Rx from 'rxjs';
import { NEWSFEED_DEFAULT_SERVICE_BASE_URL } from '../../common/constants';
import { ApiItem, FetchResult, NewsfeedPluginBrowserConfig } from '../types';
import { INewsfeedApiDriver } from './types';
import { convertItems } from './convert_items';
import type { NewsfeedStorage } from './storage';
@ -19,7 +20,7 @@ interface NewsfeedResponse {
items: ApiItem[];
}
export class NewsfeedApiDriver {
export class NewsfeedApiDriver implements INewsfeedApiDriver {
private readonly kibanaVersion: string;
private readonly loadedTime = moment().utc(); // the date is compared to time in UTC format coming from the service

View file

@ -0,0 +1,25 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import type { Observable } from 'rxjs';
import { FetchResult } from '../types';
import { INewsfeedApiDriver } from './types';
/**
* NewsfeedApiDriver variant that never fetches results. This is useful for instances where Kibana is started
* without any user interaction like when generating a PDF or PNG report.
*/
export class NeverFetchNewsfeedApiDriver implements INewsfeedApiDriver {
shouldFetch(): boolean {
return false;
}
fetchNewsfeedItems(): Observable<FetchResult> {
throw new Error('Not implemented!');
}
}

View file

@ -0,0 +1,19 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import type { Observable } from 'rxjs';
import type { FetchResult, NewsfeedPluginBrowserConfig } from '../types';
export interface INewsfeedApiDriver {
/**
* Check whether newsfeed items should be (re-)fetched
*/
shouldFetch(): boolean;
fetchNewsfeedItems(config: NewsfeedPluginBrowserConfig['service']): Observable<FetchResult>;
}

View file

@ -0,0 +1,76 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import { take } from 'rxjs/operators';
import { coreMock } from '../../../core/public/mocks';
import { NewsfeedPublicPlugin } from './plugin';
import { NewsfeedApiEndpoint } from './lib/api';
describe('Newsfeed plugin', () => {
let plugin: NewsfeedPublicPlugin;
beforeAll(() => {
jest.useFakeTimers();
});
afterAll(() => {
jest.useRealTimers();
});
beforeEach(() => {
plugin = new NewsfeedPublicPlugin(coreMock.createPluginInitializerContext());
});
describe('#start', () => {
beforeEach(() => {
plugin.setup(coreMock.createSetup());
});
beforeEach(() => {
/**
* We assume for these tests that the newsfeed stream exposed by start will fetch newsfeed items
* on the first tick for new subscribers
*/
jest.spyOn(window, 'fetch');
});
afterEach(() => {
jest.clearAllMocks();
});
describe('base case', () => {
it('makes fetch requests', () => {
const startContract = plugin.start(coreMock.createStart(), {
screenshotMode: { isScreenshotMode: () => false },
});
const sub = startContract
.createNewsFeed$(NewsfeedApiEndpoint.KIBANA) // Any endpoint will do
.pipe(take(1))
.subscribe(() => {});
jest.runOnlyPendingTimers();
expect(window.fetch).toHaveBeenCalled();
sub.unsubscribe();
});
});
describe('when in screenshot mode', () => {
it('makes no fetch requests in screenshot mode', () => {
const startContract = plugin.start(coreMock.createStart(), {
screenshotMode: { isScreenshotMode: () => true },
});
const sub = startContract
.createNewsFeed$(NewsfeedApiEndpoint.KIBANA) // Any endpoint will do
.pipe(take(1))
.subscribe(() => {});
jest.runOnlyPendingTimers();
expect(window.fetch).not.toHaveBeenCalled();
sub.unsubscribe();
});
});
});
});

View file

@ -13,7 +13,7 @@ import React from 'react';
import moment from 'moment';
import { I18nProvider } from '@kbn/i18n/react';
import { PluginInitializerContext, CoreSetup, CoreStart, Plugin } from 'src/core/public';
import { NewsfeedPluginBrowserConfig } from './types';
import { NewsfeedPluginBrowserConfig, NewsfeedPluginStartDependencies } from './types';
import { NewsfeedNavButton } from './components/newsfeed_header_nav_button';
import { getApi, NewsfeedApi, NewsfeedApiEndpoint } from './lib/api';
@ -41,8 +41,10 @@ export class NewsfeedPublicPlugin
return {};
}
public start(core: CoreStart) {
const api = this.createNewsfeedApi(this.config, NewsfeedApiEndpoint.KIBANA);
public start(core: CoreStart, { screenshotMode }: NewsfeedPluginStartDependencies) {
const isScreenshotMode = screenshotMode.isScreenshotMode();
const api = this.createNewsfeedApi(this.config, NewsfeedApiEndpoint.KIBANA, isScreenshotMode);
core.chrome.navControls.registerRight({
order: 1000,
mount: (target) => this.mount(api, target),
@ -56,7 +58,7 @@ export class NewsfeedPublicPlugin
pathTemplate: `/${endpoint}/v{VERSION}.json`,
},
});
const { fetchResults$ } = this.createNewsfeedApi(config, endpoint);
const { fetchResults$ } = this.createNewsfeedApi(config, endpoint, isScreenshotMode);
return fetchResults$;
},
};
@ -68,9 +70,10 @@ export class NewsfeedPublicPlugin
private createNewsfeedApi(
config: NewsfeedPluginBrowserConfig,
newsfeedId: NewsfeedApiEndpoint
newsfeedId: NewsfeedApiEndpoint,
isScreenshotMode: boolean
): NewsfeedApi {
const api = getApi(config, this.kibanaVersion, newsfeedId);
const api = getApi(config, this.kibanaVersion, newsfeedId, isScreenshotMode);
return {
markAsRead: api.markAsRead,
fetchResults$: api.fetchResults$.pipe(

View file

@ -7,6 +7,10 @@
*/
import { Duration, Moment } from 'moment';
import type { ScreenshotModePluginStart } from 'src/plugins/screenshot_mode/public';
export interface NewsfeedPluginStartDependencies {
screenshotMode: ScreenshotModePluginStart;
}
// Ideally, we may want to obtain the type from the configSchema and exposeToBrowser keys...
export interface NewsfeedPluginBrowserConfig {

View file

@ -7,13 +7,9 @@
"declaration": true,
"declarationMap": true
},
"include": [
"public/**/*",
"server/**/*",
"common/*",
"../../../typings/**/*"
],
"include": ["public/**/*", "server/**/*", "common/*", "../../../typings/**/*"],
"references": [
{ "path": "../../core/tsconfig.json" }
{ "path": "../../core/tsconfig.json" },
{ "path": "../screenshot_mode/tsconfig.json" }
]
}

View file

@ -18,4 +18,4 @@ export {
KBN_SCREENSHOT_MODE_ENABLED_KEY,
} from '../common';
export { ScreenshotModePluginSetup } from './types';
export { ScreenshotModePluginSetup, ScreenshotModePluginStart } from './types';

View file

@ -21,7 +21,7 @@ describe('Screenshot mode public', () => {
setScreenshotModeDisabled();
});
describe('setup contract', () => {
describe('public contract', () => {
it('detects screenshot mode "true"', () => {
setScreenshotModeEnabled();
const screenshotMode = plugin.setup(coreMock.createSetup());
@ -34,10 +34,4 @@ describe('Screenshot mode public', () => {
expect(screenshotMode.isScreenshotMode()).toBe(false);
});
});
describe('start contract', () => {
it('returns nothing', () => {
expect(plugin.start(coreMock.createStart())).toBe(undefined);
});
});
});

View file

@ -8,18 +8,22 @@
import { CoreSetup, CoreStart, Plugin } from '../../../core/public';
import { ScreenshotModePluginSetup } from './types';
import { ScreenshotModePluginSetup, ScreenshotModePluginStart } from './types';
import { getScreenshotMode } from '../common';
export class ScreenshotModePlugin implements Plugin<ScreenshotModePluginSetup> {
private publicContract = Object.freeze({
isScreenshotMode: () => getScreenshotMode() === true,
});
public setup(core: CoreSetup): ScreenshotModePluginSetup {
return {
isScreenshotMode: () => getScreenshotMode() === true,
};
return this.publicContract;
}
public start(core: CoreStart) {}
public start(core: CoreStart): ScreenshotModePluginStart {
return this.publicContract;
}
public stop() {}
}

View file

@ -15,3 +15,4 @@ export interface IScreenshotModeService {
}
export type ScreenshotModePluginSetup = IScreenshotModeService;
export type ScreenshotModePluginStart = IScreenshotModeService;