Skip to content

Commit

Permalink
Merge pull request #3 from tsullivan/screenshot-mode/create-plugin
Browse files Browse the repository at this point in the history
move require() to screenshotMode plugin
  • Loading branch information
jloleysens authored May 17, 2021
2 parents 2b10497 + 7dc5c0f commit 76242a6
Show file tree
Hide file tree
Showing 12 changed files with 72 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ export const KBN_SCREENSHOT_MODE_ENABLED_KEY = '__KBN_SCREENSHOT_MODE_ENABLED_KE
* localStorage. The ability to set a value in localStorage enables more convenient development and testing
* in functionality that needs to detect screenshot mode.
*/
export const getScreenshotMode = (): unknown => {
export const getScreenshotMode = (): boolean => {
return (
((window as unknown) as Record<string, unknown>)[KBN_SCREENSHOT_MODE_ENABLED_KEY] ||
Boolean(((window as unknown) as Record<string, unknown>)[KBN_SCREENSHOT_MODE_ENABLED_KEY]) ||
window.localStorage.getItem(KBN_SCREENSHOT_MODE_ENABLED_KEY) === 'true'
);
};
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/screenshot_mode/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ export { setScreenshotModeEnabled, KBN_SCREENSHOT_MODE_HEADER } from '../common'

export { ScreenshotModeRequestHandlerContext } from './types';

export { ScreenshotModePluginSetup } from './plugin';

export function plugin() {
return new ScreenshotModePlugin();
}
15 changes: 14 additions & 1 deletion src/plugins/screenshot_mode/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ import { Plugin, CoreSetup } from '../../../core/server';
import { KBN_SCREENSHOT_MODE_HEADER } from '../common';
import { ScreenshotModeRequestHandlerContext } from './types';

export class ScreenshotModePlugin implements Plugin {
export interface ScreenshotModePluginSetup {
setScreenshotModeEnabled: () => void;
}

export class ScreenshotModePlugin implements Plugin<ScreenshotModePluginSetup> {
public setup(core: CoreSetup) {
core.http.registerRouteHandlerContext<ScreenshotModeRequestHandlerContext, 'screenshotMode'>(
'screenshotMode',
Expand All @@ -22,6 +26,15 @@ export class ScreenshotModePlugin implements Plugin {
};
}
);

// We use "require" here to ensure the import does not have external references due to code bundling that
// commonly happens during transpiling which would be missing in the environment puppeteer creates.
// eslint-disable-next-line @typescript-eslint/no-var-requires
const { setScreenshotModeEnabled } = require('../');

return {
setScreenshotModeEnabled,
};
}

public start() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,15 @@ import { map, truncate } from 'lodash';
import open from 'opn';
import puppeteer, { ElementHandle, EvaluateFn, SerializableOrJSHandle } from 'puppeteer';
import { parse as parseUrl } from 'url';
import { KBN_SCREENSHOT_MODE_HEADER } from '../../../../../../../src/plugins/screenshot_mode/server';
import { getDisallowedOutgoingUrlError } from '../';
import { ReportingCore } from '../../..';
import { KBN_SCREENSHOT_MODE_HEADER } from '../../../../../../../src/plugins/screenshot_mode/server';
import { ConditionalHeaders, ConditionalHeadersConditions } from '../../../export_types/common';
import { LevelLogger } from '../../../lib';
import { ViewZoomWidthHeight } from '../../../lib/layouts/layout';
import { ElementPosition } from '../../../lib/screenshots';
import { allowRequest, NetworkPolicy } from '../../network_policy';

// We use "require" here to ensure the import does not have external references due to code bundling that
// commonly happens during transpiling which would be missing in the environment puppeteer creates.
// eslint-disable-next-line @typescript-eslint/no-var-requires
const setScreenshotModeEnabled = require('../../../../../../../src/plugins/screenshot_mode/server')
.setScreenshotModeEnabled;

export interface ChromiumDriverOptions {
inspect: boolean;
networkPolicy: NetworkPolicy;
Expand Down Expand Up @@ -66,8 +61,14 @@ export class HeadlessChromiumDriver {

private listenersAttached = false;
private interceptedCount = 0;
private core: ReportingCore;

constructor(page: puppeteer.Page, { inspect, networkPolicy }: ChromiumDriverOptions) {
constructor(
core: ReportingCore,
page: puppeteer.Page,
{ inspect, networkPolicy }: ChromiumDriverOptions
) {
this.core = core;
this.page = page;
this.inspect = inspect;
this.networkPolicy = networkPolicy;
Expand Down Expand Up @@ -105,7 +106,8 @@ export class HeadlessChromiumDriver {
// Reset intercepted request count
this.interceptedCount = 0;

await this.page.evaluateOnNewDocument(setScreenshotModeEnabled);
const enableScreenshotMode = this.core.enableScreenshotMode();
await this.page.evaluateOnNewDocument(enableScreenshotMode);
await this.page.setRequestInterception(true);

this.registerListeners(conditionalHeaders, logger);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import * as Rx from 'rxjs';
import { InnerSubscriber } from 'rxjs/internal/InnerSubscriber';
import { ignoreElements, map, mergeMap, tap } from 'rxjs/operators';
import { getChromiumDisconnectedError } from '../';
import { ReportingCore } from '../../..';
import { BROWSER_TYPE } from '../../../../common/constants';
import { durationToNumber } from '../../../../common/schema_utils';
import { CaptureConfig } from '../../../../server/types';
Expand All @@ -32,11 +33,14 @@ export class HeadlessChromiumDriverFactory {
private browserConfig: BrowserConfig;
private userDataDir: string;
private getChromiumArgs: (viewport: ViewportConfig) => string[];
private core: ReportingCore;

constructor(binaryPath: string, captureConfig: CaptureConfig, logger: LevelLogger) {
constructor(core: ReportingCore, binaryPath: string, logger: LevelLogger) {
this.core = core;
this.binaryPath = binaryPath;
this.captureConfig = captureConfig;
this.browserConfig = captureConfig.browser.chromium;
const config = core.getConfig();
this.captureConfig = config.get('capture');
this.browserConfig = this.captureConfig.browser.chromium;

if (this.browserConfig.disableSandbox) {
logger.warning(`Enabling the Chromium sandbox provides an additional layer of protection.`);
Expand Down Expand Up @@ -138,7 +142,7 @@ export class HeadlessChromiumDriverFactory {
this.getProcessLogger(browser, logger).subscribe();

// HeadlessChromiumDriver: object to "drive" a browser page
const driver = new HeadlessChromiumDriver(page, {
const driver = new HeadlessChromiumDriver(this.core, page, {
inspect: !!this.browserConfig.inspect,
networkPolicy: this.captureConfig.networkPolicy,
});
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/reporting/server/browsers/chromium/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@

import { i18n } from '@kbn/i18n';
import { BrowserDownload } from '../';
import { CaptureConfig } from '../../../server/types';
import { ReportingCore } from '../../../server';
import { LevelLogger } from '../../lib';
import { HeadlessChromiumDriverFactory } from './driver_factory';
import { ChromiumArchivePaths } from './paths';

export const chromium: BrowserDownload = {
paths: new ChromiumArchivePaths(),
createDriverFactory: (binaryPath: string, captureConfig: CaptureConfig, logger: LevelLogger) =>
new HeadlessChromiumDriverFactory(binaryPath, captureConfig, logger),
createDriverFactory: (core: ReportingCore, binaryPath: string, logger: LevelLogger) =>
new HeadlessChromiumDriverFactory(core, binaryPath, logger),
};

export const getChromiumDisconnectedError = () =>
Expand Down
13 changes: 4 additions & 9 deletions x-pack/plugins/reporting/server/browsers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
*/

import { first } from 'rxjs/operators';
import { ReportingConfig } from '../';
import { ReportingCore } from '../';
import { LevelLogger } from '../lib';
import { CaptureConfig } from '../types';
import { chromium, ChromiumArchivePaths } from './chromium';
import { HeadlessChromiumDriverFactory } from './chromium/driver_factory';
import { installBrowser } from './install';
Expand All @@ -18,8 +17,8 @@ export { HeadlessChromiumDriver } from './chromium/driver';
export { HeadlessChromiumDriverFactory } from './chromium/driver_factory';

type CreateDriverFactory = (
core: ReportingCore,
binaryPath: string,
captureConfig: CaptureConfig,
logger: LevelLogger
) => HeadlessChromiumDriverFactory;

Expand All @@ -28,12 +27,8 @@ export interface BrowserDownload {
paths: ChromiumArchivePaths;
}

export const initializeBrowserDriverFactory = async (
config: ReportingConfig,
logger: LevelLogger
) => {
export const initializeBrowserDriverFactory = async (core: ReportingCore, logger: LevelLogger) => {
const { binaryPath$ } = installBrowser(logger);
const binaryPath = await binaryPath$.pipe(first()).toPromise();
const captureConfig = config.get('capture');
return chromium.createDriverFactory(binaryPath, captureConfig, logger);
return chromium.createDriverFactory(core, binaryPath, logger);
};
7 changes: 7 additions & 0 deletions x-pack/plugins/reporting/server/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import Hapi from '@hapi/hapi';
import * as Rx from 'rxjs';
import { first, map, take } from 'rxjs/operators';
import { ScreenshotModePluginSetup } from 'src/plugins/screenshot_mode/server';
import {
BasePath,
IClusterClient,
Expand Down Expand Up @@ -41,6 +42,7 @@ export interface ReportingInternalSetup {
security?: SecurityPluginSetup;
spaces?: SpacesPluginSetup;
taskManager: TaskManagerSetupContract;
screenshotMode: ScreenshotModePluginSetup;
logger: LevelLogger;
}

Expand Down Expand Up @@ -237,6 +239,11 @@ export class ReportingCore {
return screenshotsObservableFactory(config.get('capture'), browserDriverFactory);
}

public enableScreenshotMode() {
const { screenshotMode } = this.getPluginSetupDeps();
return screenshotMode.setScreenshotModeEnabled;
}

/*
* Gives synchronous access to the setupDeps
*/
Expand Down
20 changes: 12 additions & 8 deletions x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ jest.mock('puppeteer', () => ({

import moment from 'moment';
import * as Rx from 'rxjs';
import { ReportingCore } from '../..';
import { HeadlessChromiumDriver } from '../../browsers';
import { ConditionalHeaders } from '../../export_types/common';
import {
Expand All @@ -27,6 +28,7 @@ import {
createMockConfigSchema,
createMockLayoutInstance,
createMockLevelLogger,
createMockReportingCore,
} from '../../test_helpers';
import { ElementsPositionAndAttribute } from './';
import * as contexts from './constants';
Expand All @@ -37,7 +39,7 @@ import { screenshotsObservableFactory } from './observable';
*/
const logger = createMockLevelLogger();

const reportingConfig = {
const mockSchema = createMockConfigSchema({
capture: {
loadDelay: moment.duration(2, 's'),
timeouts: {
Expand All @@ -46,20 +48,22 @@ const reportingConfig = {
renderComplete: moment.duration(10, 's'),
},
},
};
const mockSchema = createMockConfigSchema(reportingConfig);
});
const mockConfig = createMockConfig(mockSchema);
const captureConfig = mockConfig.get('capture');
const mockLayout = createMockLayoutInstance(captureConfig);

let core: ReportingCore;

/*
* Tests
*/
describe('Screenshot Observable Pipeline', () => {
let mockBrowserDriverFactory: any;

beforeEach(async () => {
mockBrowserDriverFactory = await createMockBrowserDriverFactory(logger, {});
core = await createMockReportingCore(mockSchema);
mockBrowserDriverFactory = await createMockBrowserDriverFactory(core, logger, {});
});

it('pipelines a single url into screenshot and timeRange', async () => {
Expand Down Expand Up @@ -118,7 +122,7 @@ describe('Screenshot Observable Pipeline', () => {
const mockOpen = jest.fn();

// mocks
mockBrowserDriverFactory = await createMockBrowserDriverFactory(logger, {
mockBrowserDriverFactory = await createMockBrowserDriverFactory(core, logger, {
screenshot: mockScreenshot,
open: mockOpen,
});
Expand Down Expand Up @@ -218,7 +222,7 @@ describe('Screenshot Observable Pipeline', () => {
});

// mocks
mockBrowserDriverFactory = await createMockBrowserDriverFactory(logger, {
mockBrowserDriverFactory = await createMockBrowserDriverFactory(core, logger, {
waitForSelector: mockWaitForSelector,
});

Expand Down Expand Up @@ -312,7 +316,7 @@ describe('Screenshot Observable Pipeline', () => {
return Rx.never().toPromise();
});

mockBrowserDriverFactory = await createMockBrowserDriverFactory(logger, {
mockBrowserDriverFactory = await createMockBrowserDriverFactory(core, logger, {
getCreatePage: mockGetCreatePage,
waitForSelector: mockWaitForSelector,
});
Expand Down Expand Up @@ -345,7 +349,7 @@ describe('Screenshot Observable Pipeline', () => {
return Promise.resolve();
}
});
mockBrowserDriverFactory = await createMockBrowserDriverFactory(logger, {
mockBrowserDriverFactory = await createMockBrowserDriverFactory(core, logger, {
evaluate: mockBrowserEvaluate,
});
mockLayout.getViewport = () => null;
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/reporting/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,13 @@ export class ReportingPlugin
registerUiSettings(core);

const { http } = core;
const { features, licensing, security, spaces, taskManager } = plugins;
const { screenshotMode, features, licensing, security, spaces, taskManager } = plugins;

const router = http.createRouter<ReportingRequestHandlerContext>();
const basePath = http.basePath;

reportingCore.pluginSetup({
screenshotMode,
features,
licensing,
basePath,
Expand Down Expand Up @@ -91,9 +92,8 @@ export class ReportingPlugin
// async background start
(async () => {
await reportingCore.pluginSetsUp();
const config = reportingCore.getConfig();

const browserDriverFactory = await initializeBrowserDriverFactory(config, this.logger);
const browserDriverFactory = await initializeBrowserDriverFactory(reportingCore, this.logger);
const store = new ReportingStore(reportingCore, this.logger);

await reportingCore.pluginStart({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import moment from 'moment';
import { Page } from 'puppeteer';
import * as Rx from 'rxjs';
import { ReportingCore } from '..';
import { chromium, HeadlessChromiumDriver, HeadlessChromiumDriverFactory } from '../browsers';
import { LevelLogger } from '../lib';
import { ElementsPositionAndAttribute } from '../lib/screenshots';
Expand Down Expand Up @@ -96,6 +97,7 @@ const defaultOpts: CreateMockBrowserDriverFactoryOpts = {
};

export const createMockBrowserDriverFactory = async (
core: ReportingCore,
logger: LevelLogger,
opts: Partial<CreateMockBrowserDriverFactoryOpts> = {}
): Promise<HeadlessChromiumDriverFactory> => {
Expand All @@ -122,9 +124,9 @@ export const createMockBrowserDriverFactory = async (
};

const binaryPath = '/usr/local/share/common/secure/super_awesome_binary';
const mockBrowserDriverFactory = chromium.createDriverFactory(binaryPath, captureConfig, logger);
const mockBrowserDriverFactory = chromium.createDriverFactory(core, binaryPath, logger);
const mockPage = ({ setViewport: () => {} } as unknown) as Page;
const mockBrowserDriver = new HeadlessChromiumDriver(mockPage, {
const mockBrowserDriver = new HeadlessChromiumDriver(core, mockPage, {
inspect: true,
networkPolicy: captureConfig.networkPolicy,
});
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/reporting/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import type { IRouter, KibanaRequest, RequestHandlerContext } from 'src/core/server';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { DataPluginStart } from 'src/plugins/data/server/plugin';
import { ScreenshotModePluginSetup } from 'src/plugins/screenshot_mode/server';
import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
import { PluginSetupContract as FeaturesPluginSetup } from '../../features/server';
import { LicensingPluginSetup } from '../../licensing/server';
Expand All @@ -32,6 +33,7 @@ export interface ReportingSetupDeps {
spaces?: SpacesPluginSetup;
taskManager: TaskManagerSetupContract;
usageCollection?: UsageCollectionSetup;
screenshotMode: ScreenshotModePluginSetup;
}

export interface ReportingStartDeps {
Expand Down

0 comments on commit 76242a6

Please sign in to comment.