Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Reporting] Clean up any usage, reorganize server route files #110740

Merged
merged 8 commits into from
Sep 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ export class HeadlessChromiumDriverFactory {
{ viewport, browserTimezone }: { viewport: ViewportConfig; browserTimezone?: string },
pLogger: LevelLogger
): Rx.Observable<{ driver: HeadlessChromiumDriver; exit$: Rx.Observable<never> }> {
return Rx.Observable.create(async (observer: InnerSubscriber<any, any>) => {
// FIXME: 'create' is deprecated
return Rx.Observable.create(async (observer: InnerSubscriber<unknown, unknown>) => {
const logger = pLogger.clone(['browser-driver']);
logger.info(`Creating browser page driver`);

Expand Down
56 changes: 45 additions & 11 deletions x-pack/plugins/reporting/server/config/ui_settings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,23 +49,57 @@ test('throws validation error if provided with data over max size', () => {
});

test('throws validation error if provided with non-image data', () => {
const invalidErrorMatcher = /try a different image/;

expect(() => PdfLogoSchema.validate('')).toThrowError(invalidErrorMatcher);
expect(() => PdfLogoSchema.validate(true)).toThrow(invalidErrorMatcher);
expect(() => PdfLogoSchema.validate(false)).toThrow(invalidErrorMatcher);
expect(() => PdfLogoSchema.validate({})).toThrow(invalidErrorMatcher);
expect(() => PdfLogoSchema.validate([])).toThrow(invalidErrorMatcher);
expect(() => PdfLogoSchema.validate(0)).toThrow(invalidErrorMatcher);
expect(() => PdfLogoSchema.validate(0x00f)).toThrow(invalidErrorMatcher);
expect(() => PdfLogoSchema.validate('')).toThrowErrorMatchingInlineSnapshot(`
"types that failed validation:
- [0]: Sorry, that file will not work. Please try a different image file.
- [1]: expected value to equal [null]"
`);
expect(() => PdfLogoSchema.validate(true)).toThrowErrorMatchingInlineSnapshot(`
"types that failed validation:
- [0]: expected value of type [string] but got [boolean]
- [1]: expected value to equal [null]"
`);
expect(() => PdfLogoSchema.validate(false)).toThrowErrorMatchingInlineSnapshot(`
"types that failed validation:
- [0]: expected value of type [string] but got [boolean]
- [1]: expected value to equal [null]"
`);
expect(() => PdfLogoSchema.validate({})).toThrowErrorMatchingInlineSnapshot(`
"types that failed validation:
- [0]: expected value of type [string] but got [Object]
- [1]: expected value to equal [null]"
`);
expect(() => PdfLogoSchema.validate([])).toThrowErrorMatchingInlineSnapshot(`
"types that failed validation:
- [0]: expected value of type [string] but got [Array]
- [1]: expected value to equal [null]"
`);
expect(() => PdfLogoSchema.validate(0)).toThrowErrorMatchingInlineSnapshot(`
"types that failed validation:
- [0]: expected value of type [string] but got [number]
- [1]: expected value to equal [null]"
`);
expect(() => PdfLogoSchema.validate(0x00f)).toThrowErrorMatchingInlineSnapshot(`
"types that failed validation:
- [0]: expected value of type [string] but got [number]
- [1]: expected value to equal [null]"
`);

const csvString =
`data:text/csv;base64,Il9pZCIsIl9pbmRleCIsIl9zY29yZSIsIl90eXBlIiwiZm9vLmJhciIsImZvby5iYXIua2V5d29yZCIKZjY1QU9IZ0J5bFZmWW04W` +
`TRvb1EsYmVlLDEsIi0iLGJheixiYXoKbks1QU9IZ0J5bFZmWW04WTdZcUcsYmVlLDEsIi0iLGJvbyxib28K`;
expect(() => PdfLogoSchema.validate(csvString)).toThrow(invalidErrorMatcher);
expect(() => PdfLogoSchema.validate(csvString)).toThrowErrorMatchingInlineSnapshot(`
"types that failed validation:
- [0]: Sorry, that file will not work. Please try a different image file.
- [1]: expected value to equal [null]"
`);

const scriptString =
`data:application/octet-stream;base64,QEVDSE8gT0ZGCldFRUtPRllSLkNPTSB8IEZJTkQgIlRoaXMgaXMiID4gVEVNUC5CQV` +
`QKRUNITz5USElTLkJBVCBTRVQgV0VFSz0lJTMKQ0FMTCBURU1QLkJBVApERUwgIFRFTVAuQkFUCkRFTCAgVEhJUy5CQVQKRUNITyBXZWVrICVXRUVLJQo=`;
expect(() => PdfLogoSchema.validate(scriptString)).toThrow(invalidErrorMatcher);
expect(() => PdfLogoSchema.validate(scriptString)).toThrowErrorMatchingInlineSnapshot(`
"types that failed validation:
- [0]: Sorry, that file will not work. Please try a different image file.
- [1]: expected value to equal [null]"
`);
});
8 changes: 5 additions & 3 deletions x-pack/plugins/reporting/server/config/ui_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const maxLogoSizeInBase64 = kbToBase64Length(200);
const dataurlRegex = /^data:([a-z]+\/[a-z0-9-+.]+)(;[a-z-]+=[a-z0-9-]+)?(;([a-z0-9]+))?,/;
const imageTypes = ['image/svg+xml', 'image/jpeg', 'image/png', 'image/gif'];

const isImageData = (str: any): boolean => {
const isImageData = (str: string) => {
const matches = str.match(dataurlRegex);

if (!matches) {
Expand All @@ -33,7 +33,7 @@ const isImageData = (str: any): boolean => {
return true;
};

const validatePdfLogoBase64String = (str: any) => {
const validatePdfLogoBase64String = (str: string) => {
if (typeof str !== 'string' || !isImageData(str)) {
return i18n.translate('xpack.reporting.uiSettings.validate.customLogo.badFile', {
defaultMessage: `Sorry, that file will not work. Please try a different image file.`,
Expand All @@ -46,7 +46,9 @@ const validatePdfLogoBase64String = (str: any) => {
}
};

export const PdfLogoSchema = schema.nullable(schema.any({ validate: validatePdfLogoBase64String }));
export const PdfLogoSchema = schema.nullable(
schema.string({ validate: validatePdfLogoBase64String })
);

export function registerUiSettings(core: CoreSetup<object, unknown>) {
core.uiSettings.register({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,10 @@ export class PdfMaker {
this._addContents(contents);
}

addImage(image: Buffer, opts = { title: '', description: '' }) {
addImage(
image: Buffer,
opts: { title?: string; description?: string } = { title: '', description: '' }
) {
const size = this._layout.getPdfImageSize();
const img = {
image: `data:image/png;base64,${image.toString('base64')}`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ export async function generatePdfObservableFactory(reporting: ReportingCore) {
tracker.startAddImage();
tracker.endAddImage();
pdfOutput.addImage(screenshot.data, {
title: screenshot.title,
description: screenshot.description,
title: screenshot.title ?? undefined,
description: screenshot.description ?? undefined,
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ export async function generatePdfObservableFactory(reporting: ReportingCore) {
tracker.startAddImage();
tracker.endAddImage();
pdfOutput.addImage(screenshot.data, {
title: screenshot.title,
description: screenshot.description,
title: screenshot.title ?? undefined,
description: screenshot.description ?? undefined,
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const getElementPositionAndAttributes = async (
},
attributes: Object.keys(attributes).reduce((result: AttributesMap, key) => {
const attribute = attributes[key];
(result as any)[key] = element.getAttribute(attribute);
result[key] = element.getAttribute(attribute);
return result;
}, {} as AttributesMap),
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,10 @@
import { HeadlessChromiumDriver } from '../../browsers';
import {
createMockBrowserDriverFactory,
createMockConfig,
createMockConfigSchema,
createMockLayoutInstance,
createMockLevelLogger,
createMockReportingCore,
} from '../../test_helpers';
import { LayoutInstance } from '../layouts';
import { getScreenshots } from './get_screenshots';

describe('getScreenshots', () => {
Expand All @@ -35,17 +32,12 @@ describe('getScreenshots', () => {
},
];

let layout: LayoutInstance;
let logger: ReturnType<typeof createMockLevelLogger>;
let browser: jest.Mocked<HeadlessChromiumDriver>;

beforeEach(async () => {
const schema = createMockConfigSchema();
const config = createMockConfig(schema);
const captureConfig = config.get('capture');
const core = await createMockReportingCore(schema);
const core = await createMockReportingCore(createMockConfigSchema());

layout = createMockLayoutInstance(captureConfig);
logger = createMockLevelLogger();

await createMockBrowserDriverFactory(core, logger, {
Expand All @@ -71,7 +63,7 @@ describe('getScreenshots', () => {
});

it('should return screenshots', async () => {
await expect(getScreenshots(browser, layout, elementsPositionAndAttributes, logger)).resolves
await expect(getScreenshots(browser, elementsPositionAndAttributes, logger)).resolves
.toMatchInlineSnapshot(`
Array [
Object {
Expand Down Expand Up @@ -117,7 +109,7 @@ describe('getScreenshots', () => {
});

it('should forward elements positions', async () => {
await getScreenshots(browser, layout, elementsPositionAndAttributes, logger);
await getScreenshots(browser, elementsPositionAndAttributes, logger);

expect(browser.screenshot).toHaveBeenCalledTimes(2);
expect(browser.screenshot).toHaveBeenNthCalledWith(
Expand All @@ -134,7 +126,7 @@ describe('getScreenshots', () => {
browser.screenshot.mockResolvedValue(Buffer.from(''));

await expect(
getScreenshots(browser, layout, elementsPositionAndAttributes, logger)
getScreenshots(browser, elementsPositionAndAttributes, logger)
).rejects.toBeInstanceOf(Error);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@
import { i18n } from '@kbn/i18n';
import { LevelLogger, startTrace } from '../';
import { HeadlessChromiumDriver } from '../../browsers';
import { LayoutInstance } from '../layouts';
import { ElementsPositionAndAttribute, Screenshot } from './';

export const getScreenshots = async (
browser: HeadlessChromiumDriver,
layout: LayoutInstance,
elementsPositionAndAttributes: ElementsPositionAndAttribute[],
logger: LevelLogger
): Promise<Screenshot[]> => {
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/reporting/server/lib/screenshots/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export interface ScreenshotObservableOpts {
}

export interface AttributesMap {
[key: string]: any;
[key: string]: string | null;
}

export interface ElementPosition {
Expand All @@ -45,8 +45,8 @@ export interface ElementsPositionAndAttribute {

export interface Screenshot {
data: Buffer;
title: string;
description: string;
title: string | null;
description: string | null;
}

export interface ScreenshotResults {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export function getScreenshots$(
const elements = data.elementsPositionAndAttributes
? data.elementsPositionAndAttributes
: getDefaultElementPosition(layout.getViewport(1));
const screenshots = await getScreenshots(driver, layout, elements, logger);
const screenshots = await getScreenshots(driver, elements, logger);
const { timeRange, error: setupError } = data;
return {
timeRange,
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/reporting/server/lib/tasks/execute_report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ interface TaskExecutor extends Pick<ExportTypeDefinition, 'jobContentEncoding'>
jobExecutor: RunTaskFn<BasePayload>;
}

function isOutput(output: any): output is CompletedReportOutput {
return output?.size != null;
function isOutput(output: CompletedReportOutput | Error): output is CompletedReportOutput {
return (output as CompletedReportOutput).size != null;
}

function reportFromTask(task: ReportTaskParams) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('POST /diagnose/screenshot', () => {
toPromise: () => (resp instanceof Error ? Promise.reject(resp) : Promise.resolve(resp)),
}),
}));
(generatePngObservableFactory as any).mockResolvedValue(generateMock);
(generatePngObservableFactory as jest.Mock).mockResolvedValue(generateMock);
};

const config = createMockConfigSchema({ queue: { timeout: 120000 } });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@
* 2.0.
*/

import { Writable } from 'stream';
import { schema } from '@kbn/config-schema';
import { KibanaRequest } from 'src/core/server';
import { ReportingCore } from '../';
import { runTaskFnFactory } from '../export_types/csv_searchsource_immediate/execute_job';
import { JobParamsDownloadCSV } from '../export_types/csv_searchsource_immediate/types';
import { LevelLogger as Logger } from '../lib';
import { TaskRunResult } from '../lib/tasks';
import { authorizedUserPreRouting } from './lib/authorized_user_pre_routing';
import { HandlerErrorFunction } from './types';
import { Writable } from 'stream';
import { ReportingCore } from '../../';
import { runTaskFnFactory } from '../../export_types/csv_searchsource_immediate/execute_job';
import { JobParamsDownloadCSV } from '../../export_types/csv_searchsource_immediate/types';
import { LevelLogger as Logger } from '../../lib';
import { TaskRunResult } from '../../lib/tasks';
import { authorizedUserPreRouting } from '../lib/authorized_user_pre_routing';
import { RequestHandler } from '../lib/request_handler';

const API_BASE_URL_V1 = '/api/reporting/v1';
const API_BASE_GENERATE_V1 = `${API_BASE_URL_V1}/generate`;
Expand All @@ -32,7 +32,6 @@ export type CsvFromSavedObjectRequest = KibanaRequest<unknown, unknown, JobParam
*/
export function registerGenerateCsvFromSavedObjectImmediate(
reporting: ReportingCore,
handleError: HandlerErrorFunction,
parentLogger: Logger
) {
const setupDeps = reporting.getPluginSetupDeps();
Expand Down Expand Up @@ -64,9 +63,10 @@ export function registerGenerateCsvFromSavedObjectImmediate(
},
authorizedUserPreRouting(
reporting,
async (_user, context, req: CsvFromSavedObjectRequest, res) => {
async (user, context, req: CsvFromSavedObjectRequest, res) => {
const logger = parentLogger.clone(['csv_searchsource_immediate']);
const runTaskFn = runTaskFnFactory(reporting, logger);
const requestHandler = new RequestHandler(reporting, user, context, req, res, logger);

try {
let buffer = Buffer.from('');
Expand Down Expand Up @@ -107,7 +107,7 @@ export function registerGenerateCsvFromSavedObjectImmediate(
});
} catch (err) {
logger.error(err);
return handleError(res, err);
return requestHandler.handleError(err);
}
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,16 @@

import { schema } from '@kbn/config-schema';
import rison from 'rison-node';
import { ReportingCore } from '../';
import { API_BASE_URL } from '../../common/constants';
import { BaseParams } from '../types';
import { authorizedUserPreRouting } from './lib/authorized_user_pre_routing';
import { HandlerErrorFunction, HandlerFunction } from './types';
import { ReportingCore } from '../..';
import { API_BASE_URL } from '../../../common/constants';
import { LevelLogger } from '../../lib';
import { BaseParams } from '../../types';
import { authorizedUserPreRouting } from '../lib/authorized_user_pre_routing';
import { RequestHandler } from '../lib/request_handler';

const BASE_GENERATE = `${API_BASE_URL}/generate`;

export function registerGenerateFromJobParams(
reporting: ReportingCore,
handler: HandlerFunction,
handleError: HandlerErrorFunction
) {
export function registerJobGenerationRoutes(reporting: ReportingCore, logger: LevelLogger) {
const setupDeps = reporting.getPluginSetupDeps();
const { router } = setupDeps;

Expand Down Expand Up @@ -62,7 +59,6 @@ export function registerGenerateFromJobParams(
});
}

const { exportType } = req.params;
let jobParams;

try {
Expand All @@ -80,10 +76,12 @@ export function registerGenerateFromJobParams(
});
}

const requestHandler = new RequestHandler(reporting, user, context, req, res, logger);

try {
return await handler(user, exportType, jobParams, context, req, res);
return await requestHandler.handleGenerateRequest(req.params.exportType, jobParams);
} catch (err) {
return handleError(res, err);
return requestHandler.handleError(err);
}
})
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ import { of } from 'rxjs';
import { ElasticsearchClient } from 'kibana/server';
import { setupServer } from 'src/core/server/test_utils';
import supertest from 'supertest';
import { ReportingCore } from '..';
import { ExportTypesRegistry } from '../lib/export_types_registry';
import { createMockLevelLogger, createMockReportingCore } from '../test_helpers';
import { ReportingCore } from '../..';
import { ExportTypesRegistry } from '../../lib/export_types_registry';
import { createMockLevelLogger, createMockReportingCore } from '../../test_helpers';
import {
createMockConfigSchema,
createMockPluginSetup,
} from '../test_helpers/create_mock_reportingplugin';
import { registerJobGenerationRoutes } from './generation';
import type { ReportingRequestHandlerContext } from '../types';
} from '../../test_helpers/create_mock_reportingplugin';
import type { ReportingRequestHandlerContext } from '../../types';
import { registerJobGenerationRoutes } from './generate_from_jobparams';

type SetupServerReturn = UnwrapPromise<ReturnType<typeof setupServer>>;

Expand Down
Loading