Skip to content

Commit

Permalink
[Reporting ] run task function simplifications of image export types (#…
Browse files Browse the repository at this point in the history
…174410)

## Summary

This PR cleans up extra layers of abstraction in image export types that
could complicate progress of the proposal of "auto" timeouts. See
#131852

## Changes
Minor code cleanup of the "runTask" functions of export types that
create screenshots, by removing the `generatePdf*` / `generatePng`
helper callback functions and inlining the work those modules were
doing.

The helper modules were an integral part of the screenshotting
pipelines, but in the unit tests they were completely mocked. Now that
we have a proper mock utility of the `screenshotting` plugin start
contract, we no longer need mockable code in a separate layer of the
pipelines.

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
tsullivan and kibanamachine authored Jan 10, 2024
1 parent 88b74ba commit 5a8b7af
Show file tree
Hide file tree
Showing 10 changed files with 251 additions and 376 deletions.
58 changes: 0 additions & 58 deletions packages/kbn-reporting/export_types/pdf/generate_pdf.ts

This file was deleted.

75 changes: 0 additions & 75 deletions packages/kbn-reporting/export_types/pdf/generate_pdf_v2.ts

This file was deleted.

42 changes: 23 additions & 19 deletions packages/kbn-reporting/export_types/pdf/printable_pdf.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,16 @@
* Side Public License, v 1.
*/

import { of } from 'rxjs';
import * as Rx from 'rxjs';
import { Writable } from 'stream';

import { coreMock, elasticsearchServiceMock, loggingSystemMock } from '@kbn/core/server/mocks';
import { CancellationToken } from '@kbn/reporting-common';
import { TaskPayloadPDF } from '@kbn/reporting-export-types-pdf-common';
import { createMockConfigSchema } from '@kbn/reporting-mocks-server';
import { cryptoFactory } from '@kbn/reporting-server';
import { ScreenshottingStart } from '@kbn/screenshotting-plugin/server';

import { createMockScreenshottingStart } from '@kbn/screenshotting-plugin/server/mock';
import { PdfV1ExportType } from '.';
import { generatePdfObservable } from './generate_pdf';

jest.mock('./generate_pdf');

let content: string;
let mockPdfExportType: PdfV1ExportType;
Expand All @@ -34,6 +30,9 @@ const encryptHeaders = async (headers: Record<string, string>) => {
return await crypto.encrypt(headers);
};

const screenshottingMock = createMockScreenshottingStart();
const getScreenshotsSpy = jest.spyOn(screenshottingMock, 'getScreenshots');
const testContent = 'raw string from get_screenhots';
const getBasePayload = (baseObj: any) => baseObj as TaskPayloadPDF;

beforeEach(async () => {
Expand All @@ -54,15 +53,20 @@ beforeEach(async () => {
esClient: elasticsearchServiceMock.createClusterClient(),
savedObjects: mockCoreStart.savedObjects,
uiSettings: mockCoreStart.uiSettings,
screenshotting: {} as unknown as ScreenshottingStart,
screenshotting: screenshottingMock,
});
getScreenshotsSpy.mockImplementation(() => {
return Rx.of({
metrics: { cpu: 0, pages: 1 },
data: Buffer.from(testContent),
errors: [],
renderErrors: [],
});
});
});

afterEach(() => (generatePdfObservable as jest.Mock).mockReset());

test(`passes browserTimezone to generatePdf`, async () => {
const encryptedHeaders = await encryptHeaders({});
(generatePdfObservable as jest.Mock).mockReturnValue(of({ buffer: Buffer.from('') }));

const browserTimezone = 'UTC';
await mockPdfExportType.runTask(
Expand All @@ -76,17 +80,20 @@ test(`passes browserTimezone to generatePdf`, async () => {
stream
);

expect(generatePdfObservable).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({ browserTimezone: 'UTC' })
);
expect(getScreenshotsSpy).toHaveBeenCalledWith({
browserTimezone: 'UTC',
format: 'pdf',
headers: {},
layout: undefined,
logo: false,
title: undefined,
urls: ['http://localhost:80/mock-server-basepath/app/kibana#/something'],
});
});

test(`returns content_type of application/pdf`, async () => {
const encryptedHeaders = await encryptHeaders({});

(generatePdfObservable as jest.Mock).mockReturnValue(of({ buffer: Buffer.from('') }));

const { content_type: contentType } = await mockPdfExportType.runTask(
'pdfJobId',
getBasePayload({ objects: [], headers: encryptedHeaders }),
Expand All @@ -97,9 +104,6 @@ test(`returns content_type of application/pdf`, async () => {
});

test(`returns content of generatePdf getBuffer base64 encoded`, async () => {
const testContent = 'test content';
(generatePdfObservable as jest.Mock).mockReturnValue(of({ buffer: Buffer.from(testContent) }));

const encryptedHeaders = await encryptHeaders({});
await mockPdfExportType.runTask(
'pdfJobId',
Expand Down
60 changes: 40 additions & 20 deletions packages/kbn-reporting/export_types/pdf/printable_pdf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ import {
} from '@kbn/reporting-export-types-pdf-common';
import { ExportType, decryptJobHeaders } from '@kbn/reporting-server';

import { generatePdfObservable } from './generate_pdf';
import { validateUrls } from './validate_urls';
import { getCustomLogo } from './get_custom_logo';
import { getFullUrls } from './get_full_urls';
import { getTracker } from './pdf_tracker';
import { validateUrls } from './validate_urls';

/**
* @deprecated
Expand Down Expand Up @@ -76,15 +76,15 @@ export class PdfV1ExportType extends ExportType<JobParamsPDFDeprecated, TaskPayl
cancellationToken: CancellationToken,
stream: Writable
) => {
const jobLogger = this.logger.get(`execute-job:${jobId}`);
const logger = this.logger.get(`execute-job:${jobId}`);
const apmTrans = apm.startTransaction('execute-job-pdf', REPORTING_TRANSACTION_TYPE);
const apmGetAssets = apmTrans.startSpan('get-assets', 'setup');
let apmGeneratePdf: { end: () => void } | null | undefined;

const process$: Observable<TaskRunResult> = of(1).pipe(
mergeMap(() => decryptJobHeaders(this.config.encryptionKey, job.headers, jobLogger)),
mergeMap(() => decryptJobHeaders(this.config.encryptionKey, job.headers, logger)),
mergeMap(async (headers) => {
const fakeRequest = this.getFakeRequest(headers, job.spaceId, jobLogger);
const fakeRequest = this.getFakeRequest(headers, job.spaceId, logger);
const uiSettingsClient = await this.getUiSettingsClient(fakeRequest);
return getCustomLogo(uiSettingsClient, headers);
}),
Expand All @@ -96,27 +96,47 @@ export class PdfV1ExportType extends ExportType<JobParamsPDFDeprecated, TaskPayl

apmGeneratePdf = apmTrans.startSpan('generate-pdf-pipeline', 'execute');

return generatePdfObservable(
() =>
this.startDeps.screenshotting!.getScreenshots({
format: 'pdf',
title,
logo,
urls,
browserTimezone,
headers,
layout,
}),
{
const tracker = getTracker();
tracker.startScreenshots();

return this.startDeps
.screenshotting!.getScreenshots({
format: 'pdf',
title,
logo,
urls,
browserTimezone,
headers,
layout,
}
);
})
.pipe(
tap(({ metrics }) => {
if (metrics.cpu) {
tracker.setCpuUsage(metrics.cpu);
}
if (metrics.memory) {
tracker.setMemoryUsage(metrics.memory);
}
}),
mergeMap(async ({ data: buffer, errors, metrics, renderErrors }) => {
tracker.endScreenshots();
const warnings: string[] = [];
if (errors) {
warnings.push(...errors.map((error) => error.message));
}
if (renderErrors) {
warnings.push(...renderErrors);
}

tracker.end();

return {
buffer,
metrics,
warnings,
};
})
);
}),
tap(({ buffer }) => {
apmGeneratePdf?.end();
Expand All @@ -130,7 +150,7 @@ export class PdfV1ExportType extends ExportType<JobParamsPDFDeprecated, TaskPayl
warnings,
})),
catchError((err: any) => {
jobLogger.error(err);
logger.error(err);
return throwError(err);
})
);
Expand Down
Loading

0 comments on commit 5a8b7af

Please sign in to comment.