From 6ecd79fb5694b64e01065dcc14da6a6a34f64f95 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Mon, 3 Aug 2020 16:37:50 -0700 Subject: [PATCH 01/23] [Reporting/Config] use better schema methods --- .../public/components/report_listing.test.tsx | 5 +-- .../public/components/report_listing.tsx | 12 +++++-- x-pack/plugins/reporting/public/plugin.tsx | 4 +-- .../browsers/chromium/driver_factory/index.ts | 4 ++- .../reporting/server/config/schema.test.ts | 35 +++++++++++++++++++ .../plugins/reporting/server/config/schema.ts | 29 ++++++--------- .../export_types/csv/generate_csv/index.ts | 12 +++++-- .../lib/screenshots/get_number_of_items.ts | 4 ++- .../server/lib/screenshots/observable.test.ts | 5 +-- .../server/lib/screenshots/open_url.ts | 8 ++--- .../server/lib/screenshots/wait_for_render.ts | 7 +++- .../screenshots/wait_for_visualizations.ts | 17 ++++----- .../server/lib/store/index_timestamp.ts | 1 - .../reporting/server/lib/store/store.ts | 4 ++- .../validate/validate_max_content_length.ts | 15 +++++--- .../create_mock_browserdriverfactory.ts | 9 +++-- 16 files changed, 116 insertions(+), 55 deletions(-) diff --git a/x-pack/plugins/reporting/public/components/report_listing.test.tsx b/x-pack/plugins/reporting/public/components/report_listing.test.tsx index cacae943687e1..aa35acb175664 100644 --- a/x-pack/plugins/reporting/public/components/report_listing.test.tsx +++ b/x-pack/plugins/reporting/public/components/report_listing.test.tsx @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import moment from 'moment'; import React from 'react'; import { Observable } from 'rxjs'; import { mountWithIntl } from 'test_utils/enzyme_helpers'; @@ -53,11 +54,11 @@ const toasts = { const mockPollConfig = { jobCompletionNotifier: { - interval: 5000, + interval: moment.duration(5, 's'), intervalErrorMultiplier: 3, }, jobsRefresh: { - interval: 5000, + interval: moment.duration(5, 's'), intervalErrorMultiplier: 3, }, }; diff --git a/x-pack/plugins/reporting/public/components/report_listing.tsx b/x-pack/plugins/reporting/public/components/report_listing.tsx index afcae93a8db16..24c331198e75a 100644 --- a/x-pack/plugins/reporting/public/components/report_listing.tsx +++ b/x-pack/plugins/reporting/public/components/report_listing.tsx @@ -165,17 +165,23 @@ class ReportListingUi extends Component { public componentDidMount() { this.mounted = true; + const { pollConfig, license$ } = this.props; + + const pollFrequencyInMillis = + typeof pollConfig.jobsRefresh.interval === 'number' + ? pollConfig.jobsRefresh.interval + : pollConfig.jobsRefresh.interval.asMilliseconds(); this.poller = new Poller({ functionToPoll: () => { return this.fetchJobs(); }, - pollFrequencyInMillis: this.props.pollConfig.jobsRefresh.interval, + pollFrequencyInMillis, trailing: false, continuePollingOnError: true, - pollFrequencyErrorMultiplier: this.props.pollConfig.jobsRefresh.intervalErrorMultiplier, + pollFrequencyErrorMultiplier: pollConfig.jobsRefresh.intervalErrorMultiplier, }); this.poller.start(); - this.licenseSubscription = this.props.license$.subscribe(this.licenseHandler); + this.licenseSubscription = license$.subscribe(this.licenseHandler); } private licenseHandler = (license: ILicense) => { diff --git a/x-pack/plugins/reporting/public/plugin.tsx b/x-pack/plugins/reporting/public/plugin.tsx index d003d4c581699..1f2efbfcba889 100644 --- a/x-pack/plugins/reporting/public/plugin.tsx +++ b/x-pack/plugins/reporting/public/plugin.tsx @@ -158,8 +158,8 @@ export class ReportingPublicPlugin implements Plugin { const { http, notifications } = core; const apiClient = new ReportingAPIClient(http); const streamHandler = new StreamHandler(notifications, apiClient); - const { interval } = this.config.poll.jobsRefresh; - + const { interval: intervalRaw } = this.config.poll.jobsRefresh; + const interval = typeof intervalRaw === 'number' ? intervalRaw : intervalRaw.asMilliseconds(); Rx.timer(0, interval) .pipe( takeUntil(this.stop$), // stop the interval when stop method is called diff --git a/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts b/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts index 809bfb57dd4fa..85c70b727110d 100644 --- a/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts +++ b/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts @@ -112,7 +112,9 @@ export class HeadlessChromiumDriverFactory { // Set the default timeout for all navigation methods to the openUrl timeout (30 seconds) // All waitFor methods have their own timeout config passed in to them - page.setDefaultTimeout(this.captureConfig.timeouts.openUrl); + const timeoutRaw = this.captureConfig.timeouts.openUrl; + const timeout = typeof timeoutRaw === 'number' ? timeoutRaw : timeoutRaw.asMilliseconds(); + page.setDefaultTimeout(timeout); logger.debug(`Browser page driver created`); } catch (err) { diff --git a/x-pack/plugins/reporting/server/config/schema.test.ts b/x-pack/plugins/reporting/server/config/schema.test.ts index 69e4d443cf040..2eef5353e2297 100644 --- a/x-pack/plugins/reporting/server/config/schema.test.ts +++ b/x-pack/plugins/reporting/server/config/schema.test.ts @@ -105,6 +105,41 @@ describe('Reporting Config Schema', () => { }); }); + it('allows Duration values for certain keys', () => { + expect(ConfigSchema.validate({ queue: { timeout: '2m' } }).queue.timeout).toMatchInlineSnapshot( + `"PT2M"` + ); + + expect( + ConfigSchema.validate({ capture: { loadDelay: '3s' } }).capture.loadDelay + ).toMatchInlineSnapshot(`"PT3S"`); + + expect( + ConfigSchema.validate({ + capture: { timeouts: { openUrl: '1m', waitForElements: '30s', renderComplete: '10s' } }, + }).capture.timeouts + ).toMatchInlineSnapshot(` + Object { + "openUrl": "PT1M", + "renderComplete": "PT10S", + "waitForElements": "PT30S", + } + `); + + expect( + ConfigSchema.validate({ csv: { scroll: { duration: '1m' } } }).csv.scroll.duration + ).toMatchInlineSnapshot(`"1m"`); + }); + + it('allows ByteSizeValue values for certain keys', () => { + expect(ConfigSchema.validate({ csv: { maxSizeBytes: '12mb' } }).csv.maxSizeBytes) + .toMatchInlineSnapshot(` + ByteSizeValue { + "valueInBytes": 12582912, + } + `); + }); + it(`allows optional settings`, () => { // encryption key expect( diff --git a/x-pack/plugins/reporting/server/config/schema.ts b/x-pack/plugins/reporting/server/config/schema.ts index a81ffd754946b..714dd912e0eae 100644 --- a/x-pack/plugins/reporting/server/config/schema.ts +++ b/x-pack/plugins/reporting/server/config/schema.ts @@ -5,7 +5,6 @@ */ import { schema, TypeOf } from '@kbn/config-schema'; -import moment from 'moment'; const KibanaServerSchema = schema.object({ hostname: schema.maybe( @@ -31,11 +30,11 @@ const KibanaServerSchema = schema.object({ }); // default values are all dynamic in createConfig$ const QueueSchema = schema.object({ - indexInterval: schema.string({ defaultValue: 'week' }), + indexInterval: schema.string({ defaultValue: 'week' }), // no schema.duration: used to create weekly indices pollEnabled: schema.boolean({ defaultValue: true }), pollInterval: schema.number({ defaultValue: 3000 }), pollIntervalErrorMultiplier: schema.number({ defaultValue: 10 }), - timeout: schema.number({ defaultValue: moment.duration(2, 'm').asMilliseconds() }), + timeout: schema.oneOf([schema.number(), schema.duration()], { defaultValue: 120000 }), }); const RulesSchema = schema.object({ @@ -46,9 +45,9 @@ const RulesSchema = schema.object({ const CaptureSchema = schema.object({ timeouts: schema.object({ - openUrl: schema.number({ defaultValue: 60000 }), - waitForElements: schema.number({ defaultValue: 30000 }), - renderComplete: schema.number({ defaultValue: 30000 }), + openUrl: schema.oneOf([schema.number(), schema.duration()], { defaultValue: 30000 }), + waitForElements: schema.oneOf([schema.number(), schema.duration()], { defaultValue: 30000 }), + renderComplete: schema.oneOf([schema.number(), schema.duration()], { defaultValue: 30000 }), }), networkPolicy: schema.object({ enabled: schema.boolean({ defaultValue: true }), @@ -68,9 +67,7 @@ const CaptureSchema = schema.object({ width: schema.number({ defaultValue: 1950 }), height: schema.number({ defaultValue: 1200 }), }), - loadDelay: schema.number({ - defaultValue: moment.duration(3, 's').asMilliseconds(), - }), // TODO: use schema.duration + loadDelay: schema.oneOf([schema.number(), schema.duration()], { defaultValue: 3000 }), browser: schema.object({ autoDownload: schema.conditional( schema.contextRef('dist'), @@ -116,9 +113,9 @@ const CsvSchema = schema.object({ checkForFormulas: schema.boolean({ defaultValue: true }), escapeFormulaValues: schema.boolean({ defaultValue: false }), enablePanelActionDownload: schema.boolean({ defaultValue: true }), - maxSizeBytes: schema.number({ - defaultValue: 1024 * 1024 * 10, // 10MB - }), // TODO: use schema.byteSize + maxSizeBytes: schema.oneOf([schema.number(), schema.byteSize()], { + defaultValue: 1024 * 1024 * 10, + }), useByteOrderMarkEncoding: schema.boolean({ defaultValue: false }), scroll: schema.object({ duration: schema.string({ @@ -148,15 +145,11 @@ const IndexSchema = schema.string({ defaultValue: '.reporting' }); const PollSchema = schema.object({ jobCompletionNotifier: schema.object({ - interval: schema.number({ - defaultValue: moment.duration(10, 's').asMilliseconds(), - }), // TODO: use schema.duration + interval: schema.oneOf([schema.number(), schema.duration()], { defaultValue: 10000 }), intervalErrorMultiplier: schema.number({ defaultValue: 5 }), }), jobsRefresh: schema.object({ - interval: schema.number({ - defaultValue: moment.duration(5, 's').asMilliseconds(), - }), // TODO: use schema.duration + interval: schema.oneOf([schema.number(), schema.duration()], { defaultValue: 5000 }), intervalErrorMultiplier: schema.number({ defaultValue: 5 }), }), }); diff --git a/x-pack/plugins/reporting/server/export_types/csv/generate_csv/index.ts b/x-pack/plugins/reporting/server/export_types/csv/generate_csv/index.ts index 06aa2434afc3f..5e8c36e9fe328 100644 --- a/x-pack/plugins/reporting/server/export_types/csv/generate_csv/index.ts +++ b/x-pack/plugins/reporting/server/export_types/csv/generate_csv/index.ts @@ -4,13 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ +import { ByteSizeValue } from '@kbn/config-schema'; import { i18n } from '@kbn/i18n'; import { IUiSettingsClient } from 'src/core/server'; -import { getFieldFormats } from '../../../services'; import { ReportingConfig } from '../../../'; import { CancellationToken } from '../../../../../../plugins/reporting/common'; import { CSV_BOM_CHARS } from '../../../../common/constants'; import { LevelLogger } from '../../../lib'; +import { getFieldFormats } from '../../../services'; import { IndexPatternSavedObject, SavedSearchGeneratorResult } from '../types'; import { checkIfRowsHaveFormulas } from './check_cells_for_formulas'; import { createEscapeValue } from './escape_value'; @@ -46,6 +47,13 @@ export interface GenerateCsvParams { conflictedTypesFields: string[]; } +const getBytes = (sizeBytes: number | ByteSizeValue): number => { + if (typeof sizeBytes === 'number') { + return sizeBytes; + } + return sizeBytes.getValueInBytes(); +}; + export function createGenerateCsv(logger: LevelLogger) { const hitIterator = createHitIterator(logger); @@ -64,7 +72,7 @@ export function createGenerateCsv(logger: LevelLogger) { ); const escapeValue = createEscapeValue(settings.quoteValues, settings.escapeFormulaValues); const bom = config.get('csv', 'useByteOrderMarkEncoding') ? CSV_BOM_CHARS : ''; - const builder = new MaxSizeStringBuilder(settings.maxSizeBytes, bom); + const builder = new MaxSizeStringBuilder(getBytes(settings.maxSizeBytes), bom); const { fields, metaFields, conflictedTypesFields } = job; const header = `${fields.map(escapeValue).join(settings.separator)}\n`; diff --git a/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.ts b/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.ts index 49c690e8c024d..501fa3b0c8175 100644 --- a/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.ts +++ b/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.ts @@ -31,9 +31,11 @@ export const getNumberOfItems = async ( // the dashboard is using the `itemsCountAttribute` attribute to let us // know how many items to expect since gridster incrementally adds panels // we have to use this hint to wait for all of them + const timeoutRaw = captureConfig.timeouts.waitForElements; + const timeout = typeof timeoutRaw === 'number' ? timeoutRaw : timeoutRaw.asMilliseconds(); await browser.waitForSelector( `${renderCompleteSelector},[${itemsCountAttribute}]`, - { timeout: captureConfig.timeouts.waitForElements }, + { timeout }, { context: CONTEXT_READMETADATA }, logger ); diff --git a/x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts b/x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts index b25e8fab3abcf..b9612cffadbed 100644 --- a/x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts +++ b/x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts @@ -15,10 +15,11 @@ jest.mock('../../browsers/chromium/puppeteer', () => ({ }), })); +import moment from 'moment'; import * as Rx from 'rxjs'; +import { LevelLogger } from '../'; import { loggingSystemMock } from '../../../../../../src/core/server/mocks'; import { HeadlessChromiumDriver } from '../../browsers'; -import { LevelLogger } from '../'; import { createMockBrowserDriverFactory, createMockLayoutInstance } from '../../test_helpers'; import { CaptureConfig, ConditionalHeaders, ElementsPositionAndAttribute } from '../../types'; import * as contexts from './constants'; @@ -30,7 +31,7 @@ import { screenshotsObservableFactory } from './observable'; const mockLogger = jest.fn(loggingSystemMock.create); const logger = new LevelLogger(mockLogger()); -const mockConfig = { timeouts: { openUrl: 13 } } as CaptureConfig; +const mockConfig = { timeouts: { openUrl: moment.duration(13, 'ms') } } as CaptureConfig; const mockLayout = createMockLayoutInstance(mockConfig); /* diff --git a/x-pack/plugins/reporting/server/lib/screenshots/open_url.ts b/x-pack/plugins/reporting/server/lib/screenshots/open_url.ts index c21ef3b91fab3..aec218791ffa5 100644 --- a/x-pack/plugins/reporting/server/lib/screenshots/open_url.ts +++ b/x-pack/plugins/reporting/server/lib/screenshots/open_url.ts @@ -19,13 +19,11 @@ export const openUrl = async ( ): Promise => { const endTrace = startTrace('open_url', 'wait'); try { + const timeoutRaw = captureConfig.timeouts.openUrl; + const timeout = typeof timeoutRaw === 'number' ? timeoutRaw : timeoutRaw.asMilliseconds(); await browser.open( url, - { - conditionalHeaders, - waitForSelector: pageLoadSelector, - timeout: captureConfig.timeouts.openUrl, - }, + { conditionalHeaders, waitForSelector: pageLoadSelector, timeout }, logger ); } catch (err) { diff --git a/x-pack/plugins/reporting/server/lib/screenshots/wait_for_render.ts b/x-pack/plugins/reporting/server/lib/screenshots/wait_for_render.ts index f36a7b6f73664..5cf2b0b8cb678 100644 --- a/x-pack/plugins/reporting/server/lib/screenshots/wait_for_render.ts +++ b/x-pack/plugins/reporting/server/lib/screenshots/wait_for_render.ts @@ -5,12 +5,17 @@ */ import { i18n } from '@kbn/i18n'; +import { Duration } from 'moment'; import { HeadlessChromiumDriver } from '../../browsers'; import { CaptureConfig } from '../../types'; import { LevelLogger, startTrace } from '../'; import { LayoutInstance } from '../layouts'; import { CONTEXT_WAITFORRENDER } from './constants'; +const toMilliseconds = (rawValue: number | Duration) => { + return typeof rawValue === 'number' ? rawValue : rawValue.asMilliseconds(); +}; + export const waitForRenderComplete = async ( captureConfig: CaptureConfig, browser: HeadlessChromiumDriver, @@ -67,7 +72,7 @@ export const waitForRenderComplete = async ( return Promise.all(renderedTasks).then(hackyWaitForVisualizations); }, - args: [layout.selectors.renderComplete, captureConfig.loadDelay], + args: [layout.selectors.renderComplete, toMilliseconds(captureConfig.loadDelay)], }, { context: CONTEXT_WAITFORRENDER }, logger diff --git a/x-pack/plugins/reporting/server/lib/screenshots/wait_for_visualizations.ts b/x-pack/plugins/reporting/server/lib/screenshots/wait_for_visualizations.ts index 779d00442522d..7698b6e3ae068 100644 --- a/x-pack/plugins/reporting/server/lib/screenshots/wait_for_visualizations.ts +++ b/x-pack/plugins/reporting/server/lib/screenshots/wait_for_visualizations.ts @@ -25,7 +25,7 @@ const getCompletedItemsCount = ({ renderCompleteSelector }: SelectorArgs) => { export const waitForVisualizations = async ( captureConfig: CaptureConfig, browser: HeadlessChromiumDriver, - itemsCount: number, + toEqual: number, layout: LayoutInstance, logger: LevelLogger ): Promise => { @@ -35,29 +35,26 @@ export const waitForVisualizations = async ( logger.debug( i18n.translate('xpack.reporting.screencapture.waitingForRenderedElements', { defaultMessage: `waiting for {itemsCount} rendered elements to be in the DOM`, - values: { itemsCount }, + values: { itemsCount: toEqual }, }) ); try { + const timeoutRaw = captureConfig.timeouts.renderComplete; + const timeout = typeof timeoutRaw === 'number' ? timeoutRaw : timeoutRaw.asMilliseconds(); await browser.waitFor( - { - fn: getCompletedItemsCount, - args: [{ renderCompleteSelector }], - toEqual: itemsCount, - timeout: captureConfig.timeouts.renderComplete, - }, + { fn: getCompletedItemsCount, args: [{ renderCompleteSelector }], toEqual, timeout }, { context: CONTEXT_WAITFORELEMENTSTOBEINDOM }, logger ); - logger.debug(`found ${itemsCount} rendered elements in the DOM`); + logger.debug(`found ${toEqual} rendered elements in the DOM`); } catch (err) { throw new Error( i18n.translate('xpack.reporting.screencapture.couldntFinishRendering', { defaultMessage: `An error occurred when trying to wait for {count} visualizations to finish rendering. You may need to increase '{configKey}'. {error}`, values: { - count: itemsCount, + count: toEqual, configKey: 'xpack.reporting.capture.timeouts.renderComplete', error: err, }, diff --git a/x-pack/plugins/reporting/server/lib/store/index_timestamp.ts b/x-pack/plugins/reporting/server/lib/store/index_timestamp.ts index 71ce0b1e572f8..7b8b851f5bd72 100644 --- a/x-pack/plugins/reporting/server/lib/store/index_timestamp.ts +++ b/x-pack/plugins/reporting/server/lib/store/index_timestamp.ts @@ -8,7 +8,6 @@ import moment, { unitOfTime } from 'moment'; export const intervals = ['year', 'month', 'week', 'day', 'hour', 'minute']; -// TODO: This helper function can be removed by using `schema.duration` objects in the reporting config schema export function indexTimestamp(intervalStr: string, separator = '-') { const startOf = intervalStr as unitOfTime.StartOf; if (separator.match(/[a-z]/i)) throw new Error('Interval separator can not be a letter'); diff --git a/x-pack/plugins/reporting/server/lib/store/store.ts b/x-pack/plugins/reporting/server/lib/store/store.ts index 88f6fa418a1b3..12a9f4db5299d 100644 --- a/x-pack/plugins/reporting/server/lib/store/store.ts +++ b/x-pack/plugins/reporting/server/lib/store/store.ts @@ -48,8 +48,10 @@ export class ReportingStore { this.client = elasticsearch.legacy.client; this.indexPrefix = config.get('index'); this.indexInterval = config.get('queue', 'indexInterval'); + + const timeoutRaw = config.get('queue', 'timeout'); this.jobSettings = { - timeout: config.get('queue', 'timeout'), + timeout: typeof timeoutRaw === 'number' ? timeoutRaw : timeoutRaw.asMilliseconds(), browser_type: config.get('capture', 'browser', 'type'), max_attempts: config.get('capture', 'maxAttempts'), priority: 10, // unused diff --git a/x-pack/plugins/reporting/server/lib/validate/validate_max_content_length.ts b/x-pack/plugins/reporting/server/lib/validate/validate_max_content_length.ts index c38c6e5297854..458462ebcf69f 100644 --- a/x-pack/plugins/reporting/server/lib/validate/validate_max_content_length.ts +++ b/x-pack/plugins/reporting/server/lib/validate/validate_max_content_length.ts @@ -5,10 +5,11 @@ */ import numeral from '@elastic/numeral'; +import { ByteSizeValue } from '@kbn/config-schema'; import { ElasticsearchServiceSetup } from 'kibana/server'; import { defaults, get } from 'lodash'; -import { ReportingConfig } from '../../'; import { LevelLogger } from '../'; +import { ReportingConfig } from '../../'; const KIBANA_MAX_SIZE_BYTES_PATH = 'csv.maxSizeBytes'; const ES_MAX_SIZE_BYTES_PATH = 'http.max_content_length'; @@ -27,10 +28,16 @@ export async function validateMaxContentLength( const elasticClusterSettings = defaults({}, persistent, transient, defaultSettings); const elasticSearchMaxContent = get(elasticClusterSettings, 'http.max_content_length', '100mb'); - const elasticSearchMaxContentBytes = numeral().unformat(elasticSearchMaxContent.toUpperCase()); - const kibanaMaxContentBytes = config.get('csv', 'maxSizeBytes'); + const elasticSearchMaxContentBytes = new ByteSizeValue( + numeral().unformat(elasticSearchMaxContent.toUpperCase()) + ); + const kibanaMaxContentBytesRaw = config.get('csv', 'maxSizeBytes'); + const kibanaMaxContentBytes = + typeof kibanaMaxContentBytesRaw === 'number' + ? new ByteSizeValue(kibanaMaxContentBytesRaw) + : kibanaMaxContentBytesRaw; - if (kibanaMaxContentBytes > elasticSearchMaxContentBytes) { + if (kibanaMaxContentBytes.isGreaterThan(elasticSearchMaxContentBytes)) { // TODO this should simply throw an error and let the handler conver it to a warning mesasge. See validateServerHost. logger.warning( `xpack.reporting.${KIBANA_MAX_SIZE_BYTES_PATH} (${kibanaMaxContentBytes}) is higher than ElasticSearch's ${ES_MAX_SIZE_BYTES_PATH} (${elasticSearchMaxContentBytes}). ` + diff --git a/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts b/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts index 08313e6892f3c..ae6d2d1450475 100644 --- a/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts +++ b/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts @@ -6,6 +6,7 @@ import { Page } from 'puppeteer'; import * as Rx from 'rxjs'; +import moment from 'moment'; import { chromium, HeadlessChromiumDriver, HeadlessChromiumDriverFactory } from '../browsers'; import { LevelLogger } from '../lib'; import * as contexts from '../lib/screenshots/constants'; @@ -95,7 +96,11 @@ export const createMockBrowserDriverFactory = async ( opts: Partial = {} ): Promise => { const captureConfig: CaptureConfig = { - timeouts: { openUrl: 30000, waitForElements: 30000, renderComplete: 30000 }, + timeouts: { + openUrl: moment.duration(60, 's'), + waitForElements: moment.duration(30, 's'), + renderComplete: moment.duration(30, 's'), + }, browser: { type: 'chromium', chromium: { @@ -107,7 +112,7 @@ export const createMockBrowserDriverFactory = async ( }, networkPolicy: { enabled: true, rules: [] }, viewport: { width: 800, height: 600 }, - loadDelay: 2000, + loadDelay: moment.duration(2, 's'), zoom: 2, maxAttempts: 1, }; From 0472d4533b26d90aadedf80e73948b67b5950bcc Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Mon, 17 Aug 2020 13:41:01 -0700 Subject: [PATCH 02/23] add createMockConfig --- .../reporting/server/config/schema.test.ts | 4 -- .../plugins/reporting/server/config/schema.ts | 2 +- .../common/get_conditional_headers.test.ts | 56 ++++++----------- .../common/get_custom_logo.test.ts | 16 ++--- .../export_types/common/get_full_urls.test.ts | 13 +--- .../export_types/csv/execute_job.test.ts | 2 + .../printable_pdf/execute_job/index.test.ts | 22 +++---- .../server/lib/create_worker.test.ts | 30 +++++----- .../server/lib/screenshots/observable.test.ts | 41 +++++++++---- .../reporting/server/lib/store/store.test.ts | 39 ++++++------ .../reporting/server/lib/store/store.ts | 4 +- ...js => validate_max_content_length.test.ts} | 15 ++--- .../plugins/reporting/server/plugin.test.ts | 4 +- .../server/routes/generation.test.ts | 3 +- .../reporting/server/routes/jobs.test.ts | 11 +--- .../lib/authorized_user_pre_routing.test.ts | 23 ++++--- .../create_mock_reportingplugin.ts | 60 +++++++++++++++++-- .../reporting/server/test_helpers/index.ts | 9 ++- .../usage/reporting_usage_collector.test.ts | 12 ++-- 19 files changed, 200 insertions(+), 166 deletions(-) rename x-pack/plugins/reporting/server/lib/validate/{validate_max_content_length.test.js => validate_max_content_length.test.ts} (83%) diff --git a/x-pack/plugins/reporting/server/config/schema.test.ts b/x-pack/plugins/reporting/server/config/schema.test.ts index 2eef5353e2297..70965bc1a120a 100644 --- a/x-pack/plugins/reporting/server/config/schema.test.ts +++ b/x-pack/plugins/reporting/server/config/schema.test.ts @@ -125,10 +125,6 @@ describe('Reporting Config Schema', () => { "waitForElements": "PT30S", } `); - - expect( - ConfigSchema.validate({ csv: { scroll: { duration: '1m' } } }).csv.scroll.duration - ).toMatchInlineSnapshot(`"1m"`); }); it('allows ByteSizeValue values for certain keys', () => { diff --git a/x-pack/plugins/reporting/server/config/schema.ts b/x-pack/plugins/reporting/server/config/schema.ts index 714dd912e0eae..85e0ae8bac04d 100644 --- a/x-pack/plugins/reporting/server/config/schema.ts +++ b/x-pack/plugins/reporting/server/config/schema.ts @@ -30,7 +30,7 @@ const KibanaServerSchema = schema.object({ }); // default values are all dynamic in createConfig$ const QueueSchema = schema.object({ - indexInterval: schema.string({ defaultValue: 'week' }), // no schema.duration: used to create weekly indices + indexInterval: schema.string({ defaultValue: 'week' }), pollEnabled: schema.boolean({ defaultValue: true }), pollInterval: schema.number({ defaultValue: 3000 }), pollIntervalErrorMultiplier: schema.number({ defaultValue: 10 }), diff --git a/x-pack/plugins/reporting/server/export_types/common/get_conditional_headers.test.ts b/x-pack/plugins/reporting/server/export_types/common/get_conditional_headers.test.ts index 0372d515c21a8..b68d90189c21d 100644 --- a/x-pack/plugins/reporting/server/export_types/common/get_conditional_headers.test.ts +++ b/x-pack/plugins/reporting/server/export_types/common/get_conditional_headers.test.ts @@ -4,10 +4,13 @@ * you may not use this file except in compliance with the Elastic License. */ -import sinon from 'sinon'; import { ReportingConfig } from '../../'; import { ReportingCore } from '../../core'; -import { createMockReportingCore } from '../../test_helpers'; +import { + createMockConfig, + createMockConfigSchema, + createMockReportingCore, +} from '../../test_helpers'; import { ScheduledTaskParams } from '../../types'; import { ScheduledTaskParamsPDF } from '../printable_pdf/types'; import { getConditionalHeaders, getCustomLogo } from './'; @@ -15,17 +18,10 @@ import { getConditionalHeaders, getCustomLogo } from './'; let mockConfig: ReportingConfig; let mockReportingPlugin: ReportingCore; -const getMockConfig = (mockConfigGet: sinon.SinonStub) => ({ - get: mockConfigGet, - kbnConfig: { get: mockConfigGet }, -}); - beforeEach(async () => { - const mockConfigGet = sinon - .stub() - .withArgs('kibanaServer', 'hostname') - .returns('custom-hostname'); - mockConfig = getMockConfig(mockConfigGet); + const reportingConfig = { kibanaServer: { hostname: 'custom-hostname' } }; + const mockSchema = createMockConfigSchema(reportingConfig); + mockConfig = createMockConfig(mockSchema); mockReportingPlugin = await createMockReportingCore(mockConfig); }); @@ -36,7 +32,7 @@ describe('conditions', () => { baz: 'quix', }; - const conditionalHeaders = await getConditionalHeaders({ + const conditionalHeaders = getConditionalHeaders({ job: {} as ScheduledTaskParams, filteredHeaders: permittedHeaders, config: mockConfig, @@ -63,7 +59,7 @@ test('uses basePath from job when creating saved object service', async () => { foo: 'bar', baz: 'quix', }; - const conditionalHeaders = await getConditionalHeaders({ + const conditionalHeaders = getConditionalHeaders({ job: {} as ScheduledTaskParams, filteredHeaders: permittedHeaders, config: mockConfig, @@ -84,16 +80,15 @@ test(`uses basePath from server if job doesn't have a basePath when creating sav const mockGetSavedObjectsClient = jest.fn(); mockReportingPlugin.getSavedObjectsClient = mockGetSavedObjectsClient; - const mockConfigGet = sinon.stub(); - mockConfigGet.withArgs('kibanaServer', 'hostname').returns('localhost'); - mockConfigGet.withArgs('server', 'basePath').returns('/sbp'); - mockConfig = getMockConfig(mockConfigGet); + const reportingConfig = { kibanaServer: { hostname: 'localhost' }, server: { basePath: '/sbp' } }; + const mockSchema = createMockConfigSchema(reportingConfig); + mockConfig = createMockConfig(mockSchema); const permittedHeaders = { foo: 'bar', baz: 'quix', }; - const conditionalHeaders = await getConditionalHeaders({ + const conditionalHeaders = getConditionalHeaders({ job: {} as ScheduledTaskParams, filteredHeaders: permittedHeaders, config: mockConfig, @@ -134,25 +129,12 @@ test(`uses basePath from server if job doesn't have a basePath when creating sav }); describe('config formatting', () => { - test(`lowercases server.host`, async () => { - const mockConfigGet = sinon.stub().withArgs('server', 'host').returns('COOL-HOSTNAME'); - mockConfig = getMockConfig(mockConfigGet); - - const conditionalHeaders = await getConditionalHeaders({ - job: {} as ScheduledTaskParams, - filteredHeaders: {}, - config: mockConfig, - }); - expect(conditionalHeaders.conditions.hostname).toEqual('cool-hostname'); - }); - test(`lowercases kibanaServer.hostname`, async () => { - const mockConfigGet = sinon - .stub() - .withArgs('kibanaServer', 'hostname') - .returns('GREAT-HOSTNAME'); - mockConfig = getMockConfig(mockConfigGet); - const conditionalHeaders = await getConditionalHeaders({ + const reportingConfig = { kibanaServer: { hostname: 'GREAT-HOSTNAME' } }; + const mockSchema = createMockConfigSchema(reportingConfig); + mockConfig = createMockConfig(mockSchema); + + const conditionalHeaders = getConditionalHeaders({ job: { title: 'cool-job-bro', type: 'csv', diff --git a/x-pack/plugins/reporting/server/export_types/common/get_custom_logo.test.ts b/x-pack/plugins/reporting/server/export_types/common/get_custom_logo.test.ts index a3d65a1398a20..05d98cd3d6203 100644 --- a/x-pack/plugins/reporting/server/export_types/common/get_custom_logo.test.ts +++ b/x-pack/plugins/reporting/server/export_types/common/get_custom_logo.test.ts @@ -4,18 +4,20 @@ * you may not use this file except in compliance with the Elastic License. */ -import { ReportingCore } from '../../core'; -import { createMockReportingCore } from '../../test_helpers'; +import { ReportingConfig, ReportingCore } from '../../'; +import { + createMockConfig, + createMockConfigSchema, + createMockReportingCore, +} from '../../test_helpers'; import { ScheduledTaskParamsPDF } from '../printable_pdf/types'; import { getConditionalHeaders, getCustomLogo } from './'; -const mockConfigGet = jest.fn().mockImplementation((key: string) => { - return 'localhost'; -}); -const mockConfig = { get: mockConfigGet, kbnConfig: { get: mockConfigGet } }; - +let mockConfig: ReportingConfig; let mockReportingPlugin: ReportingCore; + beforeEach(async () => { + mockConfig = createMockConfig(createMockConfigSchema()); mockReportingPlugin = await createMockReportingCore(mockConfig); }); diff --git a/x-pack/plugins/reporting/server/export_types/common/get_full_urls.test.ts b/x-pack/plugins/reporting/server/export_types/common/get_full_urls.test.ts index 73d7c7b03c128..13c3b5070d033 100644 --- a/x-pack/plugins/reporting/server/export_types/common/get_full_urls.test.ts +++ b/x-pack/plugins/reporting/server/export_types/common/get_full_urls.test.ts @@ -4,7 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ +import _ from 'lodash'; import { ReportingConfig } from '../../'; +import { createMockConfig } from '../../test_helpers'; import { ScheduledTaskParamsPNG } from '../png/types'; import { ScheduledTaskParamsPDF } from '../printable_pdf/types'; import { getFullUrls } from './get_full_urls'; @@ -15,12 +17,6 @@ interface FullUrlsOpts { } let mockConfig: ReportingConfig; -const getMockConfig = (mockConfigGet: jest.Mock) => { - return { - get: mockConfigGet, - kbnConfig: { get: mockConfigGet }, - }; -}; beforeEach(() => { const reportingConfig: Record = { @@ -29,10 +25,7 @@ beforeEach(() => { 'kibanaServer.protocol': 'http', 'server.basePath': '/sbp', }; - const mockConfigGet = jest.fn().mockImplementation((...keys: string[]) => { - return reportingConfig[keys.join('.') as string]; - }); - mockConfig = getMockConfig(mockConfigGet); + mockConfig = createMockConfig(reportingConfig); }); const getMockJob = (base: object) => base as ScheduledTaskParamsPNG & ScheduledTaskParamsPDF; diff --git a/x-pack/plugins/reporting/server/export_types/csv/execute_job.test.ts b/x-pack/plugins/reporting/server/export_types/csv/execute_job.test.ts index 5eeef0f9906dd..16ddf5869285c 100644 --- a/x-pack/plugins/reporting/server/export_types/csv/execute_job.test.ts +++ b/x-pack/plugins/reporting/server/export_types/csv/execute_job.test.ts @@ -22,6 +22,7 @@ import { setFieldFormats } from '../../services'; import { createMockReportingCore } from '../../test_helpers'; import { runTaskFnFactory } from './execute_job'; import { ScheduledTaskParamsCSV } from './types'; +import moment from 'moment'; const delay = (ms: number) => new Promise((resolve) => setTimeout(() => resolve(), ms)); @@ -73,6 +74,7 @@ describe('CSV Execute Job', function () { beforeEach(async function () { configGetStub = sinon.stub(); + configGetStub.withArgs('queue', 'timeout').returns(moment.duration('2m')); configGetStub.withArgs('index').returns('.reporting-foo-test'); configGetStub.withArgs('encryptionKey').returns(encryptionKey); configGetStub.withArgs('csv', 'maxSizeBytes').returns(1024 * 1000); // 1mB diff --git a/x-pack/plugins/reporting/server/export_types/printable_pdf/execute_job/index.test.ts b/x-pack/plugins/reporting/server/export_types/printable_pdf/execute_job/index.test.ts index 443db1b57fe48..ecce74f0d20a7 100644 --- a/x-pack/plugins/reporting/server/export_types/printable_pdf/execute_job/index.test.ts +++ b/x-pack/plugins/reporting/server/export_types/printable_pdf/execute_job/index.test.ts @@ -10,7 +10,11 @@ import * as Rx from 'rxjs'; import { ReportingCore } from '../../../'; import { CancellationToken } from '../../../../common'; import { cryptoFactory, LevelLogger } from '../../../lib'; -import { createMockReportingCore } from '../../../test_helpers'; +import { + createMockConfig, + createMockConfigSchema, + createMockReportingCore, +} from '../../../test_helpers'; import { generatePdfObservableFactory } from '../lib/generate_pdf'; import { ScheduledTaskParamsPDF } from '../types'; import { runTaskFnFactory } from './'; @@ -39,20 +43,16 @@ const encryptHeaders = async (headers: Record) => { const getScheduledTaskParams = (baseObj: any) => baseObj as ScheduledTaskParamsPDF; beforeEach(async () => { - const kbnConfig = { - 'server.basePath': '/sbp', - }; const reportingConfig = { + 'server.basePath': '/sbp', index: '.reports-test', encryptionKey: mockEncryptionKey, 'kibanaServer.hostname': 'localhost', 'kibanaServer.port': 5601, 'kibanaServer.protocol': 'http', }; - const mockReportingConfig = { - get: (...keys: string[]) => (reportingConfig as any)[keys.join('.')], - kbnConfig: { get: (...keys: string[]) => (kbnConfig as any)[keys.join('.')] }, - }; + const mockSchema = createMockConfigSchema(reportingConfig); + const mockReportingConfig = createMockConfig(mockSchema); mockReporting = await createMockReportingCore(mockReportingConfig); @@ -79,7 +79,7 @@ test(`passes browserTimezone to generatePdf`, async () => { const generatePdfObservable = (await generatePdfObservableFactory(mockReporting)) as jest.Mock; generatePdfObservable.mockReturnValue(Rx.of(Buffer.from(''))); - const runTask = await runTaskFnFactory(mockReporting, getMockLogger()); + const runTask = runTaskFnFactory(mockReporting, getMockLogger()); const browserTimezone = 'UTC'; await runTask( 'pdfJobId', @@ -98,7 +98,7 @@ test(`passes browserTimezone to generatePdf`, async () => { test(`returns content_type of application/pdf`, async () => { const logger = getMockLogger(); - const runTask = await runTaskFnFactory(mockReporting, logger); + const runTask = runTaskFnFactory(mockReporting, logger); const encryptedHeaders = await encryptHeaders({}); const generatePdfObservable = await generatePdfObservableFactory(mockReporting); @@ -117,7 +117,7 @@ test(`returns content of generatePdf getBuffer base64 encoded`, async () => { const generatePdfObservable = await generatePdfObservableFactory(mockReporting); (generatePdfObservable as jest.Mock).mockReturnValue(Rx.of({ buffer: Buffer.from(testContent) })); - const runTask = await runTaskFnFactory(mockReporting, getMockLogger()); + const runTask = runTaskFnFactory(mockReporting, getMockLogger()); const encryptedHeaders = await encryptHeaders({}); const { content } = await runTask( 'pdfJobId', diff --git a/x-pack/plugins/reporting/server/lib/create_worker.test.ts b/x-pack/plugins/reporting/server/lib/create_worker.test.ts index 85188c07eeb20..1fcd750849331 100644 --- a/x-pack/plugins/reporting/server/lib/create_worker.test.ts +++ b/x-pack/plugins/reporting/server/lib/create_worker.test.ts @@ -6,7 +6,12 @@ import * as sinon from 'sinon'; import { ReportingConfig, ReportingCore } from '../../server'; -import { createMockReportingCore } from '../test_helpers'; +import { + createMockConfig, + createMockConfigSchema, + createMockLevelLogger, + createMockReportingCore, +} from '../test_helpers'; import { createWorkerFactory } from './create_worker'; // @ts-ignore import { Esqueue } from './esqueue'; @@ -14,16 +19,13 @@ import { Esqueue } from './esqueue'; import { ClientMock } from './esqueue/__tests__/fixtures/legacy_elasticsearch'; import { ExportTypesRegistry } from './export_types_registry'; -const configGetStub = sinon.stub(); -configGetStub.withArgs('queue').returns({ - pollInterval: 3300, - pollIntervalErrorMultiplier: 10, -}); -configGetStub.withArgs('server', 'name').returns('test-server-123'); -configGetStub.withArgs('server', 'uuid').returns('g9ymiujthvy6v8yrh7567g6fwzgzftzfr'); +const logger = createMockLevelLogger(); +const reportingConfig = { + queue: { pollInterval: 3300, pollIntervalErrorMultiplier: 10 }, + server: { name: 'test-server-123', uuid: 'g9ymiujthvy6v8yrh7567g6fwzgzftzfr' }, +}; const executeJobFactoryStub = sinon.stub(); -const getMockLogger = sinon.stub(); const getMockExportTypesRegistry = ( exportTypes: any[] = [{ runTaskFnFactory: executeJobFactoryStub }] @@ -39,18 +41,18 @@ describe('Create Worker', () => { let client: ClientMock; beforeEach(async () => { - mockConfig = { get: configGetStub, kbnConfig: { get: configGetStub } }; + const mockSchema = createMockConfigSchema(reportingConfig); + mockConfig = createMockConfig(mockSchema); mockReporting = await createMockReportingCore(mockConfig); mockReporting.getExportTypesRegistry = () => getMockExportTypesRegistry(); - // @ts-ignore over-riding config manually - mockReporting.config = mockConfig; + client = new ClientMock(); queue = new Esqueue('reporting-queue', { client }); executeJobFactoryStub.reset(); }); test('Creates a single Esqueue worker for Reporting', async () => { - const createWorker = createWorkerFactory(mockReporting, getMockLogger()); + const createWorker = createWorkerFactory(mockReporting, logger); const registerWorkerSpy = sinon.spy(queue, 'registerWorker'); await createWorker(queue); @@ -82,7 +84,7 @@ Object { { runTaskFnFactory: executeJobFactoryStub }, ]); mockReporting.getExportTypesRegistry = () => exportTypesRegistry; - const createWorker = createWorkerFactory(mockReporting, getMockLogger()); + const createWorker = createWorkerFactory(mockReporting, logger); const registerWorkerSpy = sinon.spy(queue, 'registerWorker'); await createWorker(queue); diff --git a/x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts b/x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts index b9612cffadbed..a35c564126eec 100644 --- a/x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts +++ b/x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts @@ -17,22 +17,37 @@ jest.mock('../../browsers/chromium/puppeteer', () => ({ import moment from 'moment'; import * as Rx from 'rxjs'; -import { LevelLogger } from '../'; -import { loggingSystemMock } from '../../../../../../src/core/server/mocks'; import { HeadlessChromiumDriver } from '../../browsers'; -import { createMockBrowserDriverFactory, createMockLayoutInstance } from '../../test_helpers'; -import { CaptureConfig, ConditionalHeaders, ElementsPositionAndAttribute } from '../../types'; +import { + createMockBrowserDriverFactory, + createMockConfig, + createMockConfigSchema, + createMockLayoutInstance, + createMockLevelLogger, +} from '../../test_helpers'; +import { ConditionalHeaders, ElementsPositionAndAttribute } from '../../types'; import * as contexts from './constants'; import { screenshotsObservableFactory } from './observable'; /* * Mocks */ -const mockLogger = jest.fn(loggingSystemMock.create); -const logger = new LevelLogger(mockLogger()); +const logger = createMockLevelLogger(); -const mockConfig = { timeouts: { openUrl: moment.duration(13, 'ms') } } as CaptureConfig; -const mockLayout = createMockLayoutInstance(mockConfig); +const reportingConfig = { + capture: { + loadDelay: moment.duration(2, 's'), + timeouts: { + openUrl: moment.duration(2, 'm'), + waitForElements: moment.duration(20, 's'), + renderComplete: moment.duration(10, 's'), + }, + }, +}; +const mockSchema = createMockConfigSchema(reportingConfig); +const mockConfig = createMockConfig(mockSchema); +const captureConfig = mockConfig.get('capture'); +const mockLayout = createMockLayoutInstance(captureConfig); /* * Tests @@ -45,7 +60,7 @@ describe('Screenshot Observable Pipeline', () => { }); it('pipelines a single url into screenshot and timeRange', async () => { - const getScreenshots$ = screenshotsObservableFactory(mockConfig, mockBrowserDriverFactory); + const getScreenshots$ = screenshotsObservableFactory(captureConfig, mockBrowserDriverFactory); const result = await getScreenshots$({ logger, urls: ['/welcome/home/start/index.htm'], @@ -106,7 +121,7 @@ describe('Screenshot Observable Pipeline', () => { }); // test - const getScreenshots$ = screenshotsObservableFactory(mockConfig, mockBrowserDriverFactory); + const getScreenshots$ = screenshotsObservableFactory(captureConfig, mockBrowserDriverFactory); const result = await getScreenshots$({ logger, urls: ['/welcome/home/start/index2.htm', '/welcome/home/start/index.php3?page=./home.php'], @@ -205,7 +220,7 @@ describe('Screenshot Observable Pipeline', () => { }); // test - const getScreenshots$ = screenshotsObservableFactory(mockConfig, mockBrowserDriverFactory); + const getScreenshots$ = screenshotsObservableFactory(captureConfig, mockBrowserDriverFactory); const getScreenshot = async () => { return await getScreenshots$({ logger, @@ -300,7 +315,7 @@ describe('Screenshot Observable Pipeline', () => { }); // test - const getScreenshots$ = screenshotsObservableFactory(mockConfig, mockBrowserDriverFactory); + const getScreenshots$ = screenshotsObservableFactory(captureConfig, mockBrowserDriverFactory); const getScreenshot = async () => { return await getScreenshots$({ logger, @@ -333,7 +348,7 @@ describe('Screenshot Observable Pipeline', () => { mockLayout.getViewport = () => null; // test - const getScreenshots$ = screenshotsObservableFactory(mockConfig, mockBrowserDriverFactory); + const getScreenshots$ = screenshotsObservableFactory(captureConfig, mockBrowserDriverFactory); const getScreenshot = async () => { return await getScreenshots$({ logger, diff --git a/x-pack/plugins/reporting/server/lib/store/store.test.ts b/x-pack/plugins/reporting/server/lib/store/store.test.ts index e6c4eb7346460..8dc4edd200052 100644 --- a/x-pack/plugins/reporting/server/lib/store/store.test.ts +++ b/x-pack/plugins/reporting/server/lib/store/store.test.ts @@ -7,16 +7,15 @@ import sinon from 'sinon'; import { ElasticsearchServiceSetup } from 'src/core/server'; import { ReportingConfig, ReportingCore } from '../..'; -import { createMockReportingCore } from '../../test_helpers'; -import { createMockLevelLogger } from '../../test_helpers/create_mock_levellogger'; +import { + createMockConfig, + createMockConfigSchema, + createMockLevelLogger, + createMockReportingCore, +} from '../../test_helpers'; import { Report } from './report'; import { ReportingStore } from './store'; -const getMockConfig = (mockConfigGet: sinon.SinonStub) => ({ - get: mockConfigGet, - kbnConfig: { get: mockConfigGet }, -}); - describe('ReportingStore', () => { const mockLogger = createMockLevelLogger(); let mockConfig: ReportingConfig; @@ -26,10 +25,12 @@ describe('ReportingStore', () => { const mockElasticsearch = { legacy: { client: { callAsInternalUser: callClusterStub } } }; beforeEach(async () => { - const mockConfigGet = sinon.stub(); - mockConfigGet.withArgs('index').returns('.reporting-test'); - mockConfigGet.withArgs('queue', 'indexInterval').returns('week'); - mockConfig = getMockConfig(mockConfigGet); + const reportingConfig = { + index: '.reporting-test', + queue: { indexInterval: 'week' }, + }; + const mockSchema = createMockConfigSchema(reportingConfig); + mockConfig = createMockConfig(mockSchema); mockCore = await createMockReportingCore(mockConfig); callClusterStub.reset(); @@ -68,15 +69,17 @@ describe('ReportingStore', () => { priority: 10, started_at: undefined, status: 'pending', - timeout: undefined, + timeout: 120000, }); }); it('throws if options has invalid indexInterval', async () => { - const mockConfigGet = sinon.stub(); - mockConfigGet.withArgs('index').returns('.reporting-test'); - mockConfigGet.withArgs('queue', 'indexInterval').returns('centurially'); - mockConfig = getMockConfig(mockConfigGet); + const reportingConfig = { + index: '.reporting-test', + queue: { indexInterval: 'centurially' }, + }; + const mockSchema = createMockConfigSchema(reportingConfig); + mockConfig = createMockConfig(mockSchema); mockCore = await createMockReportingCore(mockConfig); const store = new ReportingStore(mockCore, mockLogger); @@ -160,7 +163,7 @@ describe('ReportingStore', () => { priority: 10, started_at: undefined, status: 'pending', - timeout: undefined, + timeout: 120000, }); }); @@ -191,7 +194,7 @@ describe('ReportingStore', () => { priority: 10, started_at: undefined, status: 'pending', - timeout: undefined, + timeout: 120000, }); }); }); diff --git a/x-pack/plugins/reporting/server/lib/store/store.ts b/x-pack/plugins/reporting/server/lib/store/store.ts index 12a9f4db5299d..4ef0c5c76ac20 100644 --- a/x-pack/plugins/reporting/server/lib/store/store.ts +++ b/x-pack/plugins/reporting/server/lib/store/store.ts @@ -42,6 +42,8 @@ export class ReportingStore { private logger: LevelLogger; constructor(reporting: ReportingCore, logger: LevelLogger) { + this.logger = logger; + const config = reporting.getConfig(); const elasticsearch = reporting.getElasticsearchService(); @@ -56,8 +58,6 @@ export class ReportingStore { max_attempts: config.get('capture', 'maxAttempts'), priority: 10, // unused }; - - this.logger = logger; } private async createIndex(indexName: string) { diff --git a/x-pack/plugins/reporting/server/lib/validate/validate_max_content_length.test.js b/x-pack/plugins/reporting/server/lib/validate/validate_max_content_length.test.ts similarity index 83% rename from x-pack/plugins/reporting/server/lib/validate/validate_max_content_length.test.js rename to x-pack/plugins/reporting/server/lib/validate/validate_max_content_length.test.ts index f358021560cff..26d3f29ad9ac5 100644 --- a/x-pack/plugins/reporting/server/lib/validate/validate_max_content_length.test.js +++ b/x-pack/plugins/reporting/server/lib/validate/validate_max_content_length.test.ts @@ -10,8 +10,9 @@ import { validateMaxContentLength } from './validate_max_content_length'; const FIVE_HUNDRED_MEGABYTES = 524288000; const ONE_HUNDRED_MEGABYTES = 104857600; +let elasticsearch: any; describe('Reporting: Validate Max Content Length', () => { - const elasticsearch = { + elasticsearch = { legacy: { client: { callAsInternalUser: () => ({ @@ -25,7 +26,7 @@ describe('Reporting: Validate Max Content Length', () => { }, }; - const logger = { + const logger: any = { warning: sinon.spy(), }; @@ -34,8 +35,8 @@ describe('Reporting: Validate Max Content Length', () => { }); it('should log warning messages when reporting has a higher max-size than elasticsearch', async () => { - const config = { get: sinon.stub().returns(FIVE_HUNDRED_MEGABYTES) }; - const elasticsearch = { + const config: any = { get: sinon.stub().returns(FIVE_HUNDRED_MEGABYTES) }; + elasticsearch = { legacy: { client: { callAsInternalUser: () => ({ @@ -53,11 +54,11 @@ describe('Reporting: Validate Max Content Length', () => { sinon.assert.calledWithMatch( logger.warning, - `xpack.reporting.csv.maxSizeBytes (524288000) is higher` + `xpack.reporting.csv.maxSizeBytes (500mb) is higher` ); sinon.assert.calledWithMatch( logger.warning, - `than ElasticSearch's http.max_content_length (104857600)` + `than ElasticSearch's http.max_content_length (100mb)` ); sinon.assert.calledWithMatch( logger.warning, @@ -70,7 +71,7 @@ describe('Reporting: Validate Max Content Length', () => { }); it('should do nothing when reporting has the same max-size as elasticsearch', async () => { - const config = { get: sinon.stub().returns(ONE_HUNDRED_MEGABYTES) }; + const config: any = { get: sinon.stub().returns(ONE_HUNDRED_MEGABYTES) }; expect( async () => await validateMaxContentLength(config, elasticsearch, logger.warning) diff --git a/x-pack/plugins/reporting/server/plugin.test.ts b/x-pack/plugins/reporting/server/plugin.test.ts index 420fa8347cdeb..b2eeac96e1129 100644 --- a/x-pack/plugins/reporting/server/plugin.test.ts +++ b/x-pack/plugins/reporting/server/plugin.test.ts @@ -31,8 +31,8 @@ describe('Reporting Plugin', () => { beforeEach(async () => { configSchema = createMockConfigSchema(); initContext = coreMock.createPluginInitializerContext(configSchema); - coreSetup = await coreMock.createSetup(configSchema); - coreStart = await coreMock.createStart(); + coreSetup = coreMock.createSetup(configSchema); + coreStart = coreMock.createStart(); pluginSetup = ({ licensing: {}, usageCollection: { diff --git a/x-pack/plugins/reporting/server/routes/generation.test.ts b/x-pack/plugins/reporting/server/routes/generation.test.ts index cef4da9aabbd4..c0a45f164ce67 100644 --- a/x-pack/plugins/reporting/server/routes/generation.test.ts +++ b/x-pack/plugins/reporting/server/routes/generation.test.ts @@ -11,8 +11,7 @@ import { setupServer } from 'src/core/server/test_utils'; import supertest from 'supertest'; import { ReportingCore } from '..'; import { ExportTypesRegistry } from '../lib/export_types_registry'; -import { createMockReportingCore } from '../test_helpers'; -import { createMockLevelLogger } from '../test_helpers/create_mock_levellogger'; +import { createMockLevelLogger, createMockReportingCore } from '../test_helpers'; import { registerJobGenerationRoutes } from './generation'; type SetupServerReturn = UnwrapPromise>; diff --git a/x-pack/plugins/reporting/server/routes/jobs.test.ts b/x-pack/plugins/reporting/server/routes/jobs.test.ts index 2957bc76f4682..187c69f4a72ef 100644 --- a/x-pack/plugins/reporting/server/routes/jobs.test.ts +++ b/x-pack/plugins/reporting/server/routes/jobs.test.ts @@ -10,9 +10,8 @@ import { setupServer } from 'src/core/server/test_utils'; import supertest from 'supertest'; import { ReportingCore } from '..'; import { ReportingInternalSetup } from '../core'; -import { LevelLogger } from '../lib'; import { ExportTypesRegistry } from '../lib/export_types_registry'; -import { createMockReportingCore } from '../test_helpers'; +import { createMockConfig, createMockConfigSchema, createMockReportingCore } from '../test_helpers'; import { ExportTypeDefinition } from '../types'; import { registerJobInfoRoutes } from './jobs'; @@ -25,11 +24,7 @@ describe('GET /api/reporting/jobs/download', () => { let exportTypesRegistry: ExportTypesRegistry; let core: ReportingCore; - const config = { get: jest.fn(), kbnConfig: { get: jest.fn() } }; - const mockLogger = ({ - error: jest.fn(), - debug: jest.fn(), - } as unknown) as jest.Mocked; + const config = createMockConfig(createMockConfigSchema()); const getHits = (...sources: any) => { return { @@ -86,8 +81,6 @@ describe('GET /api/reporting/jobs/download', () => { }); afterEach(async () => { - mockLogger.debug.mockReset(); - mockLogger.error.mockReset(); await server.stop(); }); diff --git a/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.test.ts b/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.test.ts index 50780a577af02..932ebfdd22bbc 100644 --- a/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.test.ts +++ b/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.test.ts @@ -4,24 +4,21 @@ * you may not use this file except in compliance with the Elastic License. */ -import { KibanaRequest, RequestHandlerContext, KibanaResponseFactory } from 'kibana/server'; +import { KibanaRequest, KibanaResponseFactory, RequestHandlerContext } from 'kibana/server'; import { coreMock, httpServerMock } from 'src/core/server/mocks'; import { ReportingCore } from '../../'; -import { createMockReportingCore } from '../../test_helpers'; -import { authorizedUserPreRoutingFactory } from './authorized_user_pre_routing'; import { ReportingInternalSetup } from '../../core'; +import { + createMockConfig, + createMockConfigSchema, + createMockReportingCore, +} from '../../test_helpers'; +import { authorizedUserPreRoutingFactory } from './authorized_user_pre_routing'; let mockCore: ReportingCore; -const kbnConfig = { - 'server.basePath': '/sbp', -}; -const reportingConfig = { - 'roles.allow': ['reporting_user'], -}; -const mockReportingConfig = { - get: (...keys: string[]) => (reportingConfig as any)[keys.join('.')] || 'whoah!', - kbnConfig: { get: (...keys: string[]) => (kbnConfig as any)[keys.join('.')] }, -}; +const mockConfig: any = { 'server.basePath': '/sbp', 'roles.allow': ['reporting_user'] }; +const mockReportingConfigSchema = createMockConfigSchema(mockConfig); +const mockReportingConfig = createMockConfig(mockReportingConfigSchema); const getMockContext = () => (({ diff --git a/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts b/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts index c508ee6974ca0..40eb0e524ddb9 100644 --- a/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts +++ b/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts @@ -10,7 +10,9 @@ jest.mock('../browsers'); jest.mock('../lib/create_queue'); jest.mock('../lib/validate'); +import _ from 'lodash'; import * as Rx from 'rxjs'; +import { ReportingConfigType } from '../config'; import { ReportingConfig, ReportingCore } from '../'; import { chromium, @@ -56,12 +58,58 @@ const createMockPluginStart = ( }; }; -export const createMockConfigSchema = (overrides?: any) => ({ - index: '.reporting', - kibanaServer: { hostname: 'localhost', port: '80' }, - capture: { browser: { chromium: { disableSandbox: true } } }, - ...overrides, -}); +interface ReportingConfigTestType { + index: string; + encryptionKey: string; + queue: Partial; + kibanaServer: Partial; + csv: Partial; + capture: any; + server?: any; +} + +export const createMockConfigSchema = ( + overrides: Partial = {} +): ReportingConfigTestType => { + // deeply merge the defaults and the provided partial schema + return { + index: '.reporting', + encryptionKey: 'cool-encryption-key-where-did-you-find-it', + ...overrides, + kibanaServer: { + hostname: 'localhost', + port: 80, + ...overrides.kibanaServer, + }, + capture: { + browser: { + chromium: { + disableSandbox: true, + }, + }, + ...overrides.capture, + }, + queue: { + timeout: 120000, + ...overrides.queue, + }, + csv: { + ...overrides.csv, + }, + }; +}; + +export const createMockConfig = ( + reportingConfig: Partial +): ReportingConfig => { + const mockConfigGet = jest.fn().mockImplementation((...keys: string[]) => { + return _.get(reportingConfig, keys.join('.')); + }); + return { + get: mockConfigGet, + kbnConfig: { get: mockConfigGet }, + }; +}; export const createMockStartDeps = (startMock?: any): ReportingStartDeps => ({ data: startMock.data, diff --git a/x-pack/plugins/reporting/server/test_helpers/index.ts b/x-pack/plugins/reporting/server/test_helpers/index.ts index b37b447dc05a9..96357dc915eef 100644 --- a/x-pack/plugins/reporting/server/test_helpers/index.ts +++ b/x-pack/plugins/reporting/server/test_helpers/index.ts @@ -4,7 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ -export { createMockServer } from './create_mock_server'; -export { createMockReportingCore, createMockConfigSchema } from './create_mock_reportingplugin'; export { createMockBrowserDriverFactory } from './create_mock_browserdriverfactory'; export { createMockLayoutInstance } from './create_mock_layoutinstance'; +export { createMockLevelLogger } from './create_mock_levellogger'; +export { + createMockConfig, + createMockConfigSchema, + createMockReportingCore, +} from './create_mock_reportingplugin'; +export { createMockServer } from './create_mock_server'; diff --git a/x-pack/plugins/reporting/server/usage/reporting_usage_collector.test.ts b/x-pack/plugins/reporting/server/usage/reporting_usage_collector.test.ts index ed2abef2542de..fc2dce441c621 100644 --- a/x-pack/plugins/reporting/server/usage/reporting_usage_collector.test.ts +++ b/x-pack/plugins/reporting/server/usage/reporting_usage_collector.test.ts @@ -8,8 +8,8 @@ import * as Rx from 'rxjs'; import sinon from 'sinon'; import { UsageCollectionSetup } from 'src/plugins/usage_collection/server'; import { ReportingConfig, ReportingCore } from '../'; -import { createMockReportingCore } from '../test_helpers'; import { getExportTypesRegistry } from '../lib/export_types_registry'; +import { createMockConfig, createMockConfigSchema, createMockReportingCore } from '../test_helpers'; import { ReportingSetupDeps } from '../types'; import { FeaturesAvailability } from './'; import { @@ -54,17 +54,13 @@ function getPluginsMock( } as unknown) as ReportingSetupDeps & { usageCollection: UsageCollectionSetup }; } -const getMockReportingConfig = () => ({ - get: () => {}, - kbnConfig: { get: () => '' }, -}); const getResponseMock = (base = {}) => base; describe('license checks', () => { let mockConfig: ReportingConfig; let mockCore: ReportingCore; beforeAll(async () => { - mockConfig = getMockReportingConfig(); + mockConfig = createMockConfig(createMockConfigSchema()); mockCore = await createMockReportingCore(mockConfig); }); @@ -189,7 +185,7 @@ describe('data modeling', () => { let mockConfig: ReportingConfig; let mockCore: ReportingCore; beforeAll(async () => { - mockConfig = getMockReportingConfig(); + mockConfig = createMockConfig(createMockConfigSchema()); mockCore = await createMockReportingCore(mockConfig); }); test('with normal looking usage data', async () => { @@ -455,7 +451,7 @@ describe('data modeling', () => { describe('Ready for collection observable', () => { test('converts observable to promise', async () => { - const mockConfig = getMockReportingConfig(); + const mockConfig = createMockConfig(createMockConfigSchema()); const mockReporting = await createMockReportingCore(mockConfig); const usageCollection = getMockUsageCollection(); From a4322765850bf727b22ca2b9edd6e47301fd818f Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 19 Aug 2020 15:47:45 -0700 Subject: [PATCH 03/23] update documentation --- docs/settings/reporting-settings.asciidoc | 25 +++++++++++------------ 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/docs/settings/reporting-settings.asciidoc b/docs/settings/reporting-settings.asciidoc index 0b6f94e86a39f..13b2d928eedaa 100644 --- a/docs/settings/reporting-settings.asciidoc +++ b/docs/settings/reporting-settings.asciidoc @@ -104,15 +104,15 @@ security is enabled, `xpack.security.encryptionKey`. [cols="2*<"] |=== | `xpack.reporting.queue.pollInterval` - | Specifies the number of milliseconds that the reporting poller waits between polling the - index for any pending Reporting jobs. Defaults to `3000` (3 seconds). + | (time value) Specifies the time that the reporting poller waits between polling the + index for any pending Reporting jobs. Defaults to `3s`. | [[xpack-reporting-q-timeout]] `xpack.reporting.queue.timeout` - | How long each worker has to produce a report. If your machine is slow or under - heavy load, you might need to increase this timeout. Specified in milliseconds. + | (time value) How long each worker has to produce a report. If your machine is slow or under + heavy load, you might need to increase this timeout. If a Reporting job execution time goes over this time limit, the job will be marked as a failure and there will not be a download available. - Defaults to `120000` (two minutes). + Defaults to `2m`. |=== @@ -126,22 +126,21 @@ control the capturing process. [cols="2*<"] |=== | `xpack.reporting.capture.timeouts.openUrl` - | Specify how long to allow the Reporting browser to wait for the "Loading..." screen + | (time value) Specify how long to allow the Reporting browser to wait for the "Loading..." screen to dismiss and find the initial data for the Kibana page. If the time is exceeded, a page screenshot is captured showing the current state, and the download link shows a warning message. - Defaults to `60000` (1 minute). + Defaults to `1m`. | `xpack.reporting.capture.timeouts.waitForElements` | Specify how long to allow the Reporting browser to wait for all visualization panels to load on the Kibana page. If the time is exceeded, a page screenshot - is captured showing the current state, and the download link shows a warning message. Defaults to `30000` (30 - seconds). + is captured showing the current state, and the download link shows a warning message. Defaults to `30s`. | `xpack.reporting.capture.timeouts.renderComplete` | Specify how long to allow the Reporting browser to wait for all visualizations to fetch and render the data. If the time is exceeded, a page screenshot is captured showing the current state, and the download link shows a warning message. Defaults to - `30000` (30 seconds). + `30s`. |=== @@ -160,11 +159,11 @@ available, but there will likely be errors in the visualizations in the report. job, as many times as this setting. Defaults to `3`. | `xpack.reporting.capture.loadDelay` - | When visualizations are not evented, this is the amount of time before + | (time value) When visualizations are not evented, this is the amount of time before taking a screenshot. All visualizations that ship with {kib} are evented, so this setting should not have much effect. If you are seeing empty images instead of visualizations, try increasing this value. - Defaults to `3000` (3 seconds). + Defaults to `3s`. | [[xpack-reporting-browser]] `xpack.reporting.capture.browser.type` | Specifies the browser to use to capture screenshots. This setting exists for @@ -216,7 +215,7 @@ When `xpack.reporting.capture.browser.type` is set to `chromium` (default) you c Defaults to `500`. | `xpack.reporting.csv.scroll.duration` - | Amount of time allowed before {kib} cleans the scroll context during a CSV export. + | (time value) Amount of time allowed before {kib} cleans the scroll context during a CSV export. Defaults to `30s`. | `xpack.reporting.csv.checkForFormulas` From ff007386e7aab9ceb6da3e5f5a2cd38f60b06061 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 19 Aug 2020 16:36:43 -0700 Subject: [PATCH 04/23] fix observable.test --- .../server/lib/screenshots/get_number_of_items.ts | 1 + .../reporting/server/lib/screenshots/inject_css.ts | 1 + .../reporting/server/lib/screenshots/open_url.ts | 1 + .../lib/screenshots/wait_for_visualizations.ts | 1 + .../create_mock_browserdriverfactory.ts | 13 ++++++------- 5 files changed, 10 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.ts b/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.ts index 501fa3b0c8175..52c35e29eb995 100644 --- a/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.ts +++ b/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.ts @@ -61,6 +61,7 @@ export const getNumberOfItems = async ( logger ); } catch (err) { + logger.error(err); throw new Error( i18n.translate('xpack.reporting.screencapture.readVisualizationsError', { defaultMessage: `An error occurred when trying to read the page for visualization panel info. You may need to increase '{configKey}'. {error}`, diff --git a/x-pack/plugins/reporting/server/lib/screenshots/inject_css.ts b/x-pack/plugins/reporting/server/lib/screenshots/inject_css.ts index f893951815e9e..2fc711d4d6f07 100644 --- a/x-pack/plugins/reporting/server/lib/screenshots/inject_css.ts +++ b/x-pack/plugins/reporting/server/lib/screenshots/inject_css.ts @@ -43,6 +43,7 @@ export const injectCustomCss = async ( logger ); } catch (err) { + logger.error(err); throw new Error( i18n.translate('xpack.reporting.screencapture.injectCss', { defaultMessage: `An error occurred when trying to update Kibana CSS for reporting. {error}`, diff --git a/x-pack/plugins/reporting/server/lib/screenshots/open_url.ts b/x-pack/plugins/reporting/server/lib/screenshots/open_url.ts index aec218791ffa5..bd4a7f339927f 100644 --- a/x-pack/plugins/reporting/server/lib/screenshots/open_url.ts +++ b/x-pack/plugins/reporting/server/lib/screenshots/open_url.ts @@ -27,6 +27,7 @@ export const openUrl = async ( logger ); } catch (err) { + logger.error(err); throw new Error( i18n.translate('xpack.reporting.screencapture.couldntLoadKibana', { defaultMessage: `An error occurred when trying to open the Kibana URL. You may need to increase '{configKey}'. {error}`, diff --git a/x-pack/plugins/reporting/server/lib/screenshots/wait_for_visualizations.ts b/x-pack/plugins/reporting/server/lib/screenshots/wait_for_visualizations.ts index 7698b6e3ae068..03b52718a9e14 100644 --- a/x-pack/plugins/reporting/server/lib/screenshots/wait_for_visualizations.ts +++ b/x-pack/plugins/reporting/server/lib/screenshots/wait_for_visualizations.ts @@ -50,6 +50,7 @@ export const waitForVisualizations = async ( logger.debug(`found ${toEqual} rendered elements in the DOM`); } catch (err) { + logger.error(err); throw new Error( i18n.translate('xpack.reporting.screencapture.couldntFinishRendering', { defaultMessage: `An error occurred when trying to wait for {count} visualizations to finish rendering. You may need to increase '{configKey}'. {error}`, diff --git a/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts b/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts index ae6d2d1450475..e2c19aeb939f1 100644 --- a/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts +++ b/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts @@ -4,9 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ +import moment from 'moment'; import { Page } from 'puppeteer'; import * as Rx from 'rxjs'; -import moment from 'moment'; import { chromium, HeadlessChromiumDriver, HeadlessChromiumDriverFactory } from '../browsers'; import { LevelLogger } from '../lib'; import * as contexts from '../lib/screenshots/constants'; @@ -15,6 +15,7 @@ import { CaptureConfig, ElementsPositionAndAttribute } from '../types'; interface CreateMockBrowserDriverFactoryOpts { evaluate: jest.Mock, any[]>; waitForSelector: jest.Mock, any[]>; + waitFor: jest.Mock, any[]>; screenshot: jest.Mock, any[]>; open: jest.Mock, any[]>; getCreatePage: (driver: HeadlessChromiumDriver) => jest.Mock; @@ -86,6 +87,7 @@ const getCreatePage = (driver: HeadlessChromiumDriver) => const defaultOpts: CreateMockBrowserDriverFactoryOpts = { evaluate: mockBrowserEvaluate, waitForSelector: mockWaitForSelector, + waitFor: jest.fn(), screenshot: mockScreenshot, open: jest.fn(), getCreatePage, @@ -118,12 +120,8 @@ export const createMockBrowserDriverFactory = async ( }; const binaryPath = '/usr/local/share/common/secure/super_awesome_binary'; - const mockBrowserDriverFactory = await chromium.createDriverFactory( - binaryPath, - captureConfig, - logger - ); - const mockPage = {} as Page; + const mockBrowserDriverFactory = chromium.createDriverFactory(binaryPath, captureConfig, logger); + const mockPage = ({ setViewport: () => {} } as unknown) as Page; const mockBrowserDriver = new HeadlessChromiumDriver(mockPage, { inspect: true, networkPolicy: captureConfig.networkPolicy, @@ -131,6 +129,7 @@ export const createMockBrowserDriverFactory = async ( // mock the driver methods as either default mocks or passed-in mockBrowserDriver.waitForSelector = opts.waitForSelector ? opts.waitForSelector : defaultOpts.waitForSelector; // prettier-ignore + mockBrowserDriver.waitFor = opts.waitFor ? opts.waitFor : defaultOpts.waitFor; mockBrowserDriver.evaluate = opts.evaluate ? opts.evaluate : defaultOpts.evaluate; mockBrowserDriver.screenshot = opts.screenshot ? opts.screenshot : defaultOpts.screenshot; mockBrowserDriver.open = opts.open ? opts.open : defaultOpts.open; From a40626b9e53e2e4cba7d05b7a9118b7efc7be6be Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Thu, 20 Aug 2020 12:53:02 -0700 Subject: [PATCH 05/23] add docs links to common options page --- docs/settings/reporting-settings.asciidoc | 47 ++++++++++------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/docs/settings/reporting-settings.asciidoc b/docs/settings/reporting-settings.asciidoc index 13b2d928eedaa..76a1157e9cc79 100644 --- a/docs/settings/reporting-settings.asciidoc +++ b/docs/settings/reporting-settings.asciidoc @@ -104,15 +104,13 @@ security is enabled, `xpack.security.encryptionKey`. [cols="2*<"] |=== | `xpack.reporting.queue.pollInterval` - | (time value) Specifies the time that the reporting poller waits between polling the - index for any pending Reporting jobs. Defaults to `3s`. + | ({ref}/common-options.html#time-units[time value]) Specifies the time that the reporting poller waits between polling the index for any + pending Reporting jobs. Can be specified as number of milliseconds. Defaults to `3s`. | [[xpack-reporting-q-timeout]] `xpack.reporting.queue.timeout` - | (time value) How long each worker has to produce a report. If your machine is slow or under - heavy load, you might need to increase this timeout. - If a Reporting job execution time goes over this time limit, the job will be - marked as a failure and there will not be a download available. - Defaults to `2m`. +| ({ref}/common-options.html#time-units[time value]) How long each worker has to produce a report. If your machine is slow or under heavy +load, you might need to increase this timeout. If a Reporting job execution time goes over this time limit, the job will be marked as a +failure and there will not be a download available. Can be specified as number of milliseconds. Defaults to `2m`. |=== @@ -126,21 +124,19 @@ control the capturing process. [cols="2*<"] |=== | `xpack.reporting.capture.timeouts.openUrl` - | (time value) Specify how long to allow the Reporting browser to wait for the "Loading..." screen - to dismiss and find the initial data for the Kibana page. If the time is - exceeded, a page screenshot is captured showing the current state, and the download link shows a warning message. - Defaults to `1m`. + | ({ref}/common-options.html#time-units[time value]) Specify how long to allow the Reporting browser to wait for the "Loading..." screen + to dismiss and find the initial data for the Kibana page. If the time is exceeded, a page screenshot is captured showing the current + state, and the download link shows a warning message. Can be specified as number of milliseconds. Defaults to `1m`. | `xpack.reporting.capture.timeouts.waitForElements` - | Specify how long to allow the Reporting browser to wait for all visualization - panels to load on the Kibana page. If the time is exceeded, a page screenshot - is captured showing the current state, and the download link shows a warning message. Defaults to `30s`. + | ({ref}/common-options.html#time-units[time value]) Specify how long to allow the Reporting browser to wait for all visualization panels + to load on the Kibana page. If the time is exceeded, a page screenshot is captured showing the current state, and the download link shows + a warning message. Can be specified as number of milliseconds. Defaults to `30s`. | `xpack.reporting.capture.timeouts.renderComplete` - | Specify how long to allow the Reporting browser to wait for all visualizations to - fetch and render the data. If the time is exceeded, a - page screenshot is captured showing the current state, and the download link shows a warning message. Defaults to - `30s`. + | ({ref}/common-options.html#time-units[time value]) Specify how long to allow the Reporting browser to wait for all visualizations to + fetch and render the data. If the time is exceeded, a page screenshot is captured showing the current state, and the download link shows a + warning message. Can be specified as number of milliseconds. Defaults to `30s`. |=== @@ -159,11 +155,9 @@ available, but there will likely be errors in the visualizations in the report. job, as many times as this setting. Defaults to `3`. | `xpack.reporting.capture.loadDelay` - | (time value) When visualizations are not evented, this is the amount of time before - taking a screenshot. All visualizations that ship with {kib} are evented, so this - setting should not have much effect. If you are seeing empty images instead of - visualizations, try increasing this value. - Defaults to `3s`. + | ({ref}/common-options.html#time-units[time value]) When visualizations are not evented, this is the amount of time before taking a + screenshot. All visualizations that ship with {kib} are evented, so this setting should not have much effect. If you are seeing empty + images instead of visualizations, try increasing this value. Defaults to `3s`. | [[xpack-reporting-browser]] `xpack.reporting.capture.browser.type` | Specifies the browser to use to capture screenshots. This setting exists for @@ -205,9 +199,8 @@ When `xpack.reporting.capture.browser.type` is set to `chromium` (default) you c [cols="2*<"] |=== | [[xpack-reporting-csv]] `xpack.reporting.csv.maxSizeBytes` - | The maximum size of a CSV file before being truncated. This setting exists to prevent - large exports from causing performance and storage issues. - Defaults to `10485760` (10mB). + | ({ref}/common-options.html#byte-units[bytes value]) The maximum size of a CSV file before being truncated. This setting exists to + prevent large exports from causing performance and storage issues. Can be specified as number of bytes. Defaults to `10mb`. | `xpack.reporting.csv.scroll.size` | Number of documents retrieved from {es} for each scroll iteration during a CSV @@ -215,7 +208,7 @@ When `xpack.reporting.capture.browser.type` is set to `chromium` (default) you c Defaults to `500`. | `xpack.reporting.csv.scroll.duration` - | (time value) Amount of time allowed before {kib} cleans the scroll context during a CSV export. + | ({ref}/common-options.html#time-units[time value]) Amount of time allowed before {kib} cleans the scroll context during a CSV export. Defaults to `30s`. | `xpack.reporting.csv.checkForFormulas` From 88af5e29e6e620b547b6646f10bfafa7ea0fdc63 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Thu, 20 Aug 2020 13:05:39 -0700 Subject: [PATCH 06/23] make the schema match the docs --- x-pack/plugins/reporting/server/config/schema.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/reporting/server/config/schema.ts b/x-pack/plugins/reporting/server/config/schema.ts index 85e0ae8bac04d..3e4efbd150847 100644 --- a/x-pack/plugins/reporting/server/config/schema.ts +++ b/x-pack/plugins/reporting/server/config/schema.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { schema, TypeOf } from '@kbn/config-schema'; +import { schema, ByteSizeValue, TypeOf } from '@kbn/config-schema'; const KibanaServerSchema = schema.object({ hostname: schema.maybe( @@ -114,7 +114,7 @@ const CsvSchema = schema.object({ escapeFormulaValues: schema.boolean({ defaultValue: false }), enablePanelActionDownload: schema.boolean({ defaultValue: true }), maxSizeBytes: schema.oneOf([schema.number(), schema.byteSize()], { - defaultValue: 1024 * 1024 * 10, + defaultValue: ByteSizeValue.parse('10mb').getValueInBytes(), }), useByteOrderMarkEncoding: schema.boolean({ defaultValue: false }), scroll: schema.object({ From 7b221f580a7f9e1a0fad013148234d31a9f62d4f Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Mon, 31 Aug 2020 10:29:22 -0700 Subject: [PATCH 07/23] wording edits per feedback --- docs/settings/reporting-settings.asciidoc | 28 +++++++++++------------ 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/docs/settings/reporting-settings.asciidoc b/docs/settings/reporting-settings.asciidoc index 76a1157e9cc79..594db854dc8c8 100644 --- a/docs/settings/reporting-settings.asciidoc +++ b/docs/settings/reporting-settings.asciidoc @@ -104,13 +104,13 @@ security is enabled, `xpack.security.encryptionKey`. [cols="2*<"] |=== | `xpack.reporting.queue.pollInterval` - | ({ref}/common-options.html#time-units[time value]) Specifies the time that the reporting poller waits between polling the index for any + | Specify the {ref}/common-options.html#time-units[time] that the reporting poller waits between polling the index for any pending Reporting jobs. Can be specified as number of milliseconds. Defaults to `3s`. | [[xpack-reporting-q-timeout]] `xpack.reporting.queue.timeout` -| ({ref}/common-options.html#time-units[time value]) How long each worker has to produce a report. If your machine is slow or under heavy -load, you might need to increase this timeout. If a Reporting job execution time goes over this time limit, the job will be marked as a -failure and there will not be a download available. Can be specified as number of milliseconds. Defaults to `2m`. +| {ref}/common-options.html#time-units[How long] each worker has to produce a report. If your machine is slow or under heavy +load, you might need to increase this timeout. If a Reporting job execution goes over this time limit, the job is marked as a +failure and no download will be available. Can be specified as number of milliseconds. Defaults to `2m`. |=== @@ -124,17 +124,17 @@ control the capturing process. [cols="2*<"] |=== | `xpack.reporting.capture.timeouts.openUrl` - | ({ref}/common-options.html#time-units[time value]) Specify how long to allow the Reporting browser to wait for the "Loading..." screen - to dismiss and find the initial data for the Kibana page. If the time is exceeded, a page screenshot is captured showing the current + | Specify the {ref}/common-options.html#time-units[time] to allow the Reporting browser to wait for the "Loading..." screen + to dismiss and find the initial data for the page. If the time is exceeded, a page screenshot is captured showing the current state, and the download link shows a warning message. Can be specified as number of milliseconds. Defaults to `1m`. | `xpack.reporting.capture.timeouts.waitForElements` - | ({ref}/common-options.html#time-units[time value]) Specify how long to allow the Reporting browser to wait for all visualization panels - to load on the Kibana page. If the time is exceeded, a page screenshot is captured showing the current state, and the download link shows + | Specify the {ref}/common-options.html#time-units[time] to allow the Reporting browser to wait for all visualization panels + to load on the page. If the time is exceeded, a page screenshot is captured showing the current state, and the download link shows a warning message. Can be specified as number of milliseconds. Defaults to `30s`. | `xpack.reporting.capture.timeouts.renderComplete` - | ({ref}/common-options.html#time-units[time value]) Specify how long to allow the Reporting browser to wait for all visualizations to + | Specify the {ref}/common-options.html#time-units[time] to allow the Reporting browser to wait for all visualizations to fetch and render the data. If the time is exceeded, a page screenshot is captured showing the current state, and the download link shows a warning message. Can be specified as number of milliseconds. Defaults to `30s`. @@ -155,9 +155,9 @@ available, but there will likely be errors in the visualizations in the report. job, as many times as this setting. Defaults to `3`. | `xpack.reporting.capture.loadDelay` - | ({ref}/common-options.html#time-units[time value]) When visualizations are not evented, this is the amount of time before taking a - screenshot. All visualizations that ship with {kib} are evented, so this setting should not have much effect. If you are seeing empty - images instead of visualizations, try increasing this value. Defaults to `3s`. + | Specify the {ref}/common-options.html#time-units[amount of time] before taking a screenshot when visualizations are not evented. + All visualizations that ship with {kib} are evented, so this setting should not have much effect. If you are seeing empty images + instead of visualizations, try increasing this value. Defaults to `3s`. | [[xpack-reporting-browser]] `xpack.reporting.capture.browser.type` | Specifies the browser to use to capture screenshots. This setting exists for @@ -199,7 +199,7 @@ When `xpack.reporting.capture.browser.type` is set to `chromium` (default) you c [cols="2*<"] |=== | [[xpack-reporting-csv]] `xpack.reporting.csv.maxSizeBytes` - | ({ref}/common-options.html#byte-units[bytes value]) The maximum size of a CSV file before being truncated. This setting exists to + | The maximum {ref}/common-options.html#byte-units[byte size] of a CSV file before being truncated. This setting exists to prevent large exports from causing performance and storage issues. Can be specified as number of bytes. Defaults to `10mb`. | `xpack.reporting.csv.scroll.size` @@ -208,7 +208,7 @@ When `xpack.reporting.capture.browser.type` is set to `chromium` (default) you c Defaults to `500`. | `xpack.reporting.csv.scroll.duration` - | ({ref}/common-options.html#time-units[time value]) Amount of time allowed before {kib} cleans the scroll context during a CSV export. + | Amount of {ref}/common-options.html#time-units[time] allowed before {kib} cleans the scroll context during a CSV export. Defaults to `30s`. | `xpack.reporting.csv.checkForFormulas` From 7afa56594acf5976cdfe2ed9872cbfb83e37c5a0 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Mon, 31 Aug 2020 10:37:54 -0700 Subject: [PATCH 08/23] self edits --- docs/settings/reporting-settings.asciidoc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/settings/reporting-settings.asciidoc b/docs/settings/reporting-settings.asciidoc index 594db854dc8c8..eed9e8f7ded22 100644 --- a/docs/settings/reporting-settings.asciidoc +++ b/docs/settings/reporting-settings.asciidoc @@ -125,17 +125,17 @@ control the capturing process. |=== | `xpack.reporting.capture.timeouts.openUrl` | Specify the {ref}/common-options.html#time-units[time] to allow the Reporting browser to wait for the "Loading..." screen - to dismiss and find the initial data for the page. If the time is exceeded, a page screenshot is captured showing the current - state, and the download link shows a warning message. Can be specified as number of milliseconds. Defaults to `1m`. + to dismiss and find the initial data for the page. If the time is exceeded, a screenshot is captured showing the current + page, and the download link shows a warning message. Can be specified as number of milliseconds. Defaults to `1m`. | `xpack.reporting.capture.timeouts.waitForElements` | Specify the {ref}/common-options.html#time-units[time] to allow the Reporting browser to wait for all visualization panels - to load on the page. If the time is exceeded, a page screenshot is captured showing the current state, and the download link shows + to load on the page. If the time is exceeded, a screenshot is captured showing the current page, and the download link shows a warning message. Can be specified as number of milliseconds. Defaults to `30s`. | `xpack.reporting.capture.timeouts.renderComplete` | Specify the {ref}/common-options.html#time-units[time] to allow the Reporting browser to wait for all visualizations to - fetch and render the data. If the time is exceeded, a page screenshot is captured showing the current state, and the download link shows a + fetch and render the data. If the time is exceeded, a screenshot is captured showing the current page, and the download link shows a warning message. Can be specified as number of milliseconds. Defaults to `30s`. |=== From 05debe20cd5d6c115f5be83f28f219a6704625d0 Mon Sep 17 00:00:00 2001 From: Tim Sullivan Date: Thu, 3 Sep 2020 09:57:55 -0700 Subject: [PATCH 09/23] todo comment --- .../reporting/server/lib/validate/validate_max_content_length.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/reporting/server/lib/validate/validate_max_content_length.ts b/x-pack/plugins/reporting/server/lib/validate/validate_max_content_length.ts index 458462ebcf69f..3e920c721f74f 100644 --- a/x-pack/plugins/reporting/server/lib/validate/validate_max_content_length.ts +++ b/x-pack/plugins/reporting/server/lib/validate/validate_max_content_length.ts @@ -28,6 +28,7 @@ export async function validateMaxContentLength( const elasticClusterSettings = defaults({}, persistent, transient, defaultSettings); const elasticSearchMaxContent = get(elasticClusterSettings, 'http.max_content_length', '100mb'); + // TODO: use ByteSizeValue.parse instead of numeral const elasticSearchMaxContentBytes = new ByteSizeValue( numeral().unformat(elasticSearchMaxContent.toUpperCase()) ); From a2fab88018ccbb62f5c3f00dfc56bb81142c4513 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 9 Sep 2020 17:00:17 -0700 Subject: [PATCH 10/23] fix tests --- .../server/routes/diagnostic/browser.test.ts | 10 ++++- .../server/routes/diagnostic/browser.ts | 37 +++++++++++-------- .../server/routes/diagnostic/config.test.ts | 10 ++++- .../routes/diagnostic/screenshot.test.ts | 6 ++- 4 files changed, 44 insertions(+), 19 deletions(-) diff --git a/x-pack/plugins/reporting/server/routes/diagnostic/browser.test.ts b/x-pack/plugins/reporting/server/routes/diagnostic/browser.test.ts index f92fbfc7013cf..71ca0661a42a9 100644 --- a/x-pack/plugins/reporting/server/routes/diagnostic/browser.test.ts +++ b/x-pack/plugins/reporting/server/routes/diagnostic/browser.test.ts @@ -33,7 +33,15 @@ describe('POST /diagnose/browser', () => { const mockedCreateInterface: any = createInterface; const config = { - get: jest.fn().mockImplementation(() => ({})), + get: jest.fn().mockImplementation((...keys) => { + const key = keys.join('.'); + switch (key) { + case 'queue.timeout': + return 120000; + case 'capture.browser.chromium.proxy': + return { enabled: false }; + } + }), kbnConfig: { get: jest.fn() }, }; diff --git a/x-pack/plugins/reporting/server/routes/diagnostic/browser.ts b/x-pack/plugins/reporting/server/routes/diagnostic/browser.ts index 24b85220defb4..33620bc9a0038 100644 --- a/x-pack/plugins/reporting/server/routes/diagnostic/browser.ts +++ b/x-pack/plugins/reporting/server/routes/diagnostic/browser.ts @@ -54,25 +54,30 @@ export const registerDiagnoseBrowser = (reporting: ReportingCore, logger: Logger validate: {}, }, userHandler(async (user, context, req, res) => { - const logs = await browserStartLogs(reporting, logger).toPromise(); - const knownIssues = Object.keys(logsToHelpMap) as Array; + try { + const logs = await browserStartLogs(reporting, logger).toPromise(); + const knownIssues = Object.keys(logsToHelpMap) as Array; - const boundSuccessfully = logs.includes(`DevTools listening on`); - const help = knownIssues.reduce((helpTexts: string[], knownIssue) => { - const helpText = logsToHelpMap[knownIssue]; - if (logs.includes(knownIssue)) { - helpTexts.push(helpText); - } - return helpTexts; - }, []); + const boundSuccessfully = logs.includes(`DevTools listening on`); + const help = knownIssues.reduce((helpTexts: string[], knownIssue) => { + const helpText = logsToHelpMap[knownIssue]; + if (logs.includes(knownIssue)) { + helpTexts.push(helpText); + } + return helpTexts; + }, []); - const response: DiagnosticResponse = { - success: boundSuccessfully && !help.length, - help, - logs, - }; + const response: DiagnosticResponse = { + success: boundSuccessfully && !help.length, + help, + logs, + }; - return res.ok({ body: response }); + return res.ok({ body: response }); + } catch (err) { + logger.error(err); + return res.custom({ statusCode: 500 }); + } }) ); }; diff --git a/x-pack/plugins/reporting/server/routes/diagnostic/config.test.ts b/x-pack/plugins/reporting/server/routes/diagnostic/config.test.ts index 624397246656d..a112d04f38c7b 100644 --- a/x-pack/plugins/reporting/server/routes/diagnostic/config.test.ts +++ b/x-pack/plugins/reporting/server/routes/diagnostic/config.test.ts @@ -35,7 +35,15 @@ describe('POST /diagnose/config', () => { } as unknown) as any; config = { - get: jest.fn(), + get: jest.fn().mockImplementation((...keys) => { + const key = keys.join('.'); + switch (key) { + case 'queue.timeout': + return 120000; + case 'csv.maxSizeBytes': + return 1024; + } + }), kbnConfig: { get: jest.fn() }, }; diff --git a/x-pack/plugins/reporting/server/routes/diagnostic/screenshot.test.ts b/x-pack/plugins/reporting/server/routes/diagnostic/screenshot.test.ts index ec4ab0446ae5f..287da0d2ed5ec 100644 --- a/x-pack/plugins/reporting/server/routes/diagnostic/screenshot.test.ts +++ b/x-pack/plugins/reporting/server/routes/diagnostic/screenshot.test.ts @@ -33,7 +33,11 @@ describe('POST /diagnose/screenshot', () => { }; const config = { - get: jest.fn(), + get: jest.fn().mockImplementation((...keys) => { + if (keys.join('.') === 'queue.timeout') { + return 120000; + } + }), kbnConfig: { get: jest.fn() }, }; const mockLogger = createMockLevelLogger(); From 5319844dc27b15a5624cb1363fd497749dc0fe66 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 16 Sep 2020 15:00:10 -0700 Subject: [PATCH 11/23] feedback change 1 --- .../reporting/public/components/report_listing.test.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/reporting/public/components/report_listing.test.tsx b/x-pack/plugins/reporting/public/components/report_listing.test.tsx index aa35acb175664..36167f569ee08 100644 --- a/x-pack/plugins/reporting/public/components/report_listing.test.tsx +++ b/x-pack/plugins/reporting/public/components/report_listing.test.tsx @@ -52,13 +52,14 @@ const toasts = { addDanger: jest.fn(), } as any; +const fiveSeconds = moment.duration(5, 's'); const mockPollConfig = { jobCompletionNotifier: { - interval: moment.duration(5, 's'), + interval: fiveSeconds, intervalErrorMultiplier: 3, }, jobsRefresh: { - interval: moment.duration(5, 's'), + interval: fiveSeconds, intervalErrorMultiplier: 3, }, }; From 2fdaf88fbd20d7a16e0d8108a93c386025bfdbdc Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 16 Sep 2020 15:30:33 -0700 Subject: [PATCH 12/23] schema utils --- .../plugins/reporting/common/schema_utils.ts | 36 +++++++++++++++++++ .../public/components/report_listing.tsx | 8 ++--- x-pack/plugins/reporting/public/plugin.tsx | 4 +-- .../browsers/chromium/driver_factory/index.ts | 5 ++- .../export_types/csv/generate_csv/index.ts | 11 ++---- .../lib/screenshots/get_number_of_items.ts | 6 ++-- .../server/lib/screenshots/open_url.ts | 6 ++-- .../server/lib/screenshots/wait_for_render.ts | 8 ++--- .../screenshots/wait_for_visualizations.ts | 6 ++-- .../reporting/server/lib/store/store.ts | 4 +-- .../server/routes/diagnostic/config.ts | 7 ++-- 11 files changed, 60 insertions(+), 41 deletions(-) create mode 100644 x-pack/plugins/reporting/common/schema_utils.ts diff --git a/x-pack/plugins/reporting/common/schema_utils.ts b/x-pack/plugins/reporting/common/schema_utils.ts new file mode 100644 index 0000000000000..1f924ab272bcc --- /dev/null +++ b/x-pack/plugins/reporting/common/schema_utils.ts @@ -0,0 +1,36 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { ByteSizeValue } from '@kbn/config-schema'; +import moment from 'moment'; + +/* + * For cleaner code: use these functions when a config schema value could be + * one type or another. This allows you to treat the value as one type. + */ + +export const durationToNumber = (value: number | moment.Duration): number => { + if (typeof value === 'number') { + return value; + } + return value.asMilliseconds(); +}; + +export const numberToByteSizeValue = (value: number | ByteSizeValue) => { + if (typeof value === 'number') { + return new ByteSizeValue(value); + } + + return value; +}; + +export const byteSizeValueToNumber = (value: number | ByteSizeValue) => { + if (typeof value === 'number') { + return value; + } + + return value.getValueInBytes(); +}; diff --git a/x-pack/plugins/reporting/public/components/report_listing.tsx b/x-pack/plugins/reporting/public/components/report_listing.tsx index fc964fc599c62..a09ec22a93797 100644 --- a/x-pack/plugins/reporting/public/components/report_listing.tsx +++ b/x-pack/plugins/reporting/public/components/report_listing.tsx @@ -6,8 +6,8 @@ import { EuiBasicTable, - EuiFlexItem, EuiFlexGroup, + EuiFlexItem, EuiPageContent, EuiSpacer, EuiText, @@ -23,6 +23,7 @@ import { Subscription } from 'rxjs'; import { ApplicationStart, ToastsSetup } from 'src/core/public'; import { ILicense, LicensingPluginSetup } from '../../../licensing/public'; import { Poller } from '../../common/poller'; +import { durationToNumber } from '../../common/schema_utils'; import { JobStatuses } from '../../constants'; import { checkLicense } from '../lib/license_check'; import { JobQueueEntry, ReportingAPIClient } from '../lib/reporting_api_client'; @@ -185,10 +186,7 @@ class ReportListingUi extends Component { this.mounted = true; const { pollConfig, license$ } = this.props; - const pollFrequencyInMillis = - typeof pollConfig.jobsRefresh.interval === 'number' - ? pollConfig.jobsRefresh.interval - : pollConfig.jobsRefresh.interval.asMilliseconds(); + const pollFrequencyInMillis = durationToNumber(pollConfig.jobsRefresh.interval); this.poller = new Poller({ functionToPoll: () => { return this.fetchJobs(); diff --git a/x-pack/plugins/reporting/public/plugin.tsx b/x-pack/plugins/reporting/public/plugin.tsx index 1f2efbfcba889..a134377e194b8 100644 --- a/x-pack/plugins/reporting/public/plugin.tsx +++ b/x-pack/plugins/reporting/public/plugin.tsx @@ -26,6 +26,7 @@ import { import { ManagementSetup } from '../../../../src/plugins/management/public'; import { SharePluginSetup } from '../../../../src/plugins/share/public'; import { LicensingPluginSetup } from '../../licensing/public'; +import { durationToNumber } from '../common/schema_utils'; import { JobId, JobStatusBuckets, ReportingConfigType } from '../common/types'; import { JOB_COMPLETION_NOTIFICATIONS_SESSION_KEY } from '../constants'; import { getGeneralErrorToast } from './components'; @@ -158,8 +159,7 @@ export class ReportingPublicPlugin implements Plugin { const { http, notifications } = core; const apiClient = new ReportingAPIClient(http); const streamHandler = new StreamHandler(notifications, apiClient); - const { interval: intervalRaw } = this.config.poll.jobsRefresh; - const interval = typeof intervalRaw === 'number' ? intervalRaw : intervalRaw.asMilliseconds(); + const interval = durationToNumber(this.config.poll.jobsRefresh.interval); Rx.timer(0, interval) .pipe( takeUntil(this.stop$), // stop the interval when stop method is called diff --git a/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts b/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts index 2ceb92a4fe399..6897f07c45e2b 100644 --- a/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts +++ b/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts @@ -21,6 +21,7 @@ import { InnerSubscriber } from 'rxjs/internal/InnerSubscriber'; import { ignoreElements, map, mergeMap, tap } from 'rxjs/operators'; import { getChromiumDisconnectedError } from '../'; import { BROWSER_TYPE } from '../../../../common/constants'; +import { durationToNumber } from '../../../../common/schema_utils'; import { CaptureConfig } from '../../../../server/types'; import { LevelLogger } from '../../../lib'; import { safeChildProcess } from '../../safe_child_process'; @@ -90,9 +91,7 @@ export class HeadlessChromiumDriverFactory { // Set the default timeout for all navigation methods to the openUrl timeout (30 seconds) // All waitFor methods have their own timeout config passed in to them - const timeoutRaw = this.captureConfig.timeouts.openUrl; - const timeout = typeof timeoutRaw === 'number' ? timeoutRaw : timeoutRaw.asMilliseconds(); - page.setDefaultTimeout(timeout); + page.setDefaultTimeout(durationToNumber(this.captureConfig.timeouts.openUrl)); logger.debug(`Browser page driver created`); } catch (err) { diff --git a/x-pack/plugins/reporting/server/export_types/csv/generate_csv/index.ts b/x-pack/plugins/reporting/server/export_types/csv/generate_csv/index.ts index 5e8c36e9fe328..e383f21143149 100644 --- a/x-pack/plugins/reporting/server/export_types/csv/generate_csv/index.ts +++ b/x-pack/plugins/reporting/server/export_types/csv/generate_csv/index.ts @@ -4,12 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ -import { ByteSizeValue } from '@kbn/config-schema'; import { i18n } from '@kbn/i18n'; import { IUiSettingsClient } from 'src/core/server'; import { ReportingConfig } from '../../../'; import { CancellationToken } from '../../../../../../plugins/reporting/common'; import { CSV_BOM_CHARS } from '../../../../common/constants'; +import { byteSizeValueToNumber } from '../../../../common/schema_utils'; import { LevelLogger } from '../../../lib'; import { getFieldFormats } from '../../../services'; import { IndexPatternSavedObject, SavedSearchGeneratorResult } from '../types'; @@ -47,13 +47,6 @@ export interface GenerateCsvParams { conflictedTypesFields: string[]; } -const getBytes = (sizeBytes: number | ByteSizeValue): number => { - if (typeof sizeBytes === 'number') { - return sizeBytes; - } - return sizeBytes.getValueInBytes(); -}; - export function createGenerateCsv(logger: LevelLogger) { const hitIterator = createHitIterator(logger); @@ -72,7 +65,7 @@ export function createGenerateCsv(logger: LevelLogger) { ); const escapeValue = createEscapeValue(settings.quoteValues, settings.escapeFormulaValues); const bom = config.get('csv', 'useByteOrderMarkEncoding') ? CSV_BOM_CHARS : ''; - const builder = new MaxSizeStringBuilder(getBytes(settings.maxSizeBytes), bom); + const builder = new MaxSizeStringBuilder(byteSizeValueToNumber(settings.maxSizeBytes), bom); const { fields, metaFields, conflictedTypesFields } = job; const header = `${fields.map(escapeValue).join(settings.separator)}\n`; diff --git a/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.ts b/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.ts index 52c35e29eb995..89cb4221c96b2 100644 --- a/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.ts +++ b/x-pack/plugins/reporting/server/lib/screenshots/get_number_of_items.ts @@ -5,9 +5,10 @@ */ import { i18n } from '@kbn/i18n'; +import { durationToNumber } from '../../../common/schema_utils'; +import { LevelLogger, startTrace } from '../'; import { HeadlessChromiumDriver } from '../../browsers'; import { CaptureConfig } from '../../types'; -import { LevelLogger, startTrace } from '../'; import { LayoutInstance } from '../layouts'; import { CONTEXT_GETNUMBEROFITEMS, CONTEXT_READMETADATA } from './constants'; @@ -31,8 +32,7 @@ export const getNumberOfItems = async ( // the dashboard is using the `itemsCountAttribute` attribute to let us // know how many items to expect since gridster incrementally adds panels // we have to use this hint to wait for all of them - const timeoutRaw = captureConfig.timeouts.waitForElements; - const timeout = typeof timeoutRaw === 'number' ? timeoutRaw : timeoutRaw.asMilliseconds(); + const timeout = durationToNumber(captureConfig.timeouts.waitForElements); await browser.waitForSelector( `${renderCompleteSelector},[${itemsCountAttribute}]`, { timeout }, diff --git a/x-pack/plugins/reporting/server/lib/screenshots/open_url.ts b/x-pack/plugins/reporting/server/lib/screenshots/open_url.ts index bd4a7f339927f..e28f50851f4d9 100644 --- a/x-pack/plugins/reporting/server/lib/screenshots/open_url.ts +++ b/x-pack/plugins/reporting/server/lib/screenshots/open_url.ts @@ -5,9 +5,10 @@ */ import { i18n } from '@kbn/i18n'; +import { durationToNumber } from '../../../common/schema_utils'; +import { LevelLogger, startTrace } from '../'; import { HeadlessChromiumDriver } from '../../browsers'; import { CaptureConfig, ConditionalHeaders } from '../../types'; -import { LevelLogger, startTrace } from '../'; export const openUrl = async ( captureConfig: CaptureConfig, @@ -19,8 +20,7 @@ export const openUrl = async ( ): Promise => { const endTrace = startTrace('open_url', 'wait'); try { - const timeoutRaw = captureConfig.timeouts.openUrl; - const timeout = typeof timeoutRaw === 'number' ? timeoutRaw : timeoutRaw.asMilliseconds(); + const timeout = durationToNumber(captureConfig.timeouts.openUrl); await browser.open( url, { conditionalHeaders, waitForSelector: pageLoadSelector, timeout }, diff --git a/x-pack/plugins/reporting/server/lib/screenshots/wait_for_render.ts b/x-pack/plugins/reporting/server/lib/screenshots/wait_for_render.ts index 5cf2b0b8cb678..edd4f71b2adac 100644 --- a/x-pack/plugins/reporting/server/lib/screenshots/wait_for_render.ts +++ b/x-pack/plugins/reporting/server/lib/screenshots/wait_for_render.ts @@ -5,17 +5,13 @@ */ import { i18n } from '@kbn/i18n'; -import { Duration } from 'moment'; +import { durationToNumber } from '../../../common/schema_utils'; import { HeadlessChromiumDriver } from '../../browsers'; import { CaptureConfig } from '../../types'; import { LevelLogger, startTrace } from '../'; import { LayoutInstance } from '../layouts'; import { CONTEXT_WAITFORRENDER } from './constants'; -const toMilliseconds = (rawValue: number | Duration) => { - return typeof rawValue === 'number' ? rawValue : rawValue.asMilliseconds(); -}; - export const waitForRenderComplete = async ( captureConfig: CaptureConfig, browser: HeadlessChromiumDriver, @@ -72,7 +68,7 @@ export const waitForRenderComplete = async ( return Promise.all(renderedTasks).then(hackyWaitForVisualizations); }, - args: [layout.selectors.renderComplete, toMilliseconds(captureConfig.loadDelay)], + args: [layout.selectors.renderComplete, durationToNumber(captureConfig.loadDelay)], }, { context: CONTEXT_WAITFORRENDER }, logger diff --git a/x-pack/plugins/reporting/server/lib/screenshots/wait_for_visualizations.ts b/x-pack/plugins/reporting/server/lib/screenshots/wait_for_visualizations.ts index 03b52718a9e14..5f86a2b3bf00b 100644 --- a/x-pack/plugins/reporting/server/lib/screenshots/wait_for_visualizations.ts +++ b/x-pack/plugins/reporting/server/lib/screenshots/wait_for_visualizations.ts @@ -5,8 +5,9 @@ */ import { i18n } from '@kbn/i18n'; -import { HeadlessChromiumDriver } from '../../browsers'; +import { durationToNumber } from '../../../common/schema_utils'; import { LevelLogger, startTrace } from '../'; +import { HeadlessChromiumDriver } from '../../browsers'; import { CaptureConfig } from '../../types'; import { LayoutInstance } from '../layouts'; import { CONTEXT_WAITFORELEMENTSTOBEINDOM } from './constants'; @@ -40,8 +41,7 @@ export const waitForVisualizations = async ( ); try { - const timeoutRaw = captureConfig.timeouts.renderComplete; - const timeout = typeof timeoutRaw === 'number' ? timeoutRaw : timeoutRaw.asMilliseconds(); + const timeout = durationToNumber(captureConfig.timeouts.renderComplete); await browser.waitFor( { fn: getCompletedItemsCount, args: [{ renderCompleteSelector }], toEqual, timeout }, { context: CONTEXT_WAITFORELEMENTSTOBEINDOM }, diff --git a/x-pack/plugins/reporting/server/lib/store/store.ts b/x-pack/plugins/reporting/server/lib/store/store.ts index c5e6c3798aded..ce28715e1ba46 100644 --- a/x-pack/plugins/reporting/server/lib/store/store.ts +++ b/x-pack/plugins/reporting/server/lib/store/store.ts @@ -5,6 +5,7 @@ */ import { ElasticsearchServiceSetup } from 'src/core/server'; +import { durationToNumber } from '../../../common/schema_utils'; import { LevelLogger, statuses } from '../'; import { ReportingCore } from '../../'; import { BaseParams, BaseParamsEncryptedFields, ReportingUser } from '../../types'; @@ -47,9 +48,8 @@ export class ReportingStore { this.indexPrefix = config.get('index'); this.indexInterval = config.get('queue', 'indexInterval'); - const timeoutRaw = config.get('queue', 'timeout'); this.jobSettings = { - timeout: typeof timeoutRaw === 'number' ? timeoutRaw : timeoutRaw.asMilliseconds(), + timeout: durationToNumber(config.get('queue', 'timeout')), browser_type: config.get('capture', 'browser', 'type'), max_attempts: config.get('capture', 'maxAttempts'), priority: 10, // unused diff --git a/x-pack/plugins/reporting/server/routes/diagnostic/config.ts b/x-pack/plugins/reporting/server/routes/diagnostic/config.ts index 6f23d406ff640..3ce46cf233472 100644 --- a/x-pack/plugins/reporting/server/routes/diagnostic/config.ts +++ b/x-pack/plugins/reporting/server/routes/diagnostic/config.ts @@ -9,6 +9,7 @@ import { i18n } from '@kbn/i18n'; import { defaults, get } from 'lodash'; import { ReportingCore } from '../..'; import { API_DIAGNOSE_URL } from '../../../common/constants'; +import { numberToByteSizeValue } from '../../../common/schema_utils'; import { LevelLogger as Logger } from '../../lib'; import { DiagnosticResponse } from '../../types'; import { authorizedUserPreRoutingFactory } from '../lib/authorized_user_pre_routing'; @@ -43,11 +44,7 @@ export const registerDiagnoseConfig = (reporting: ReportingCore, logger: Logger) '100mb' ); const elasticSearchMaxContentBytes = ByteSizeValue.parse(elasticSearchMaxContent); - const kibanaMaxContentBytesRaw = config.get('csv', 'maxSizeBytes'); - const kibanaMaxContentBytes = - typeof kibanaMaxContentBytesRaw === 'number' - ? new ByteSizeValue(kibanaMaxContentBytesRaw) - : kibanaMaxContentBytesRaw; + const kibanaMaxContentBytes = numberToByteSizeValue(config.get('csv', 'maxSizeBytes')); if (kibanaMaxContentBytes.isGreaterThan(elasticSearchMaxContentBytes)) { const maxContentSizeWarning = i18n.translate( From d9bed34aa591106e7ddcddd96d0ca376dabc6717 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 16 Sep 2020 15:51:01 -0700 Subject: [PATCH 13/23] fix goof --- x-pack/plugins/reporting/server/lib/store/store.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/reporting/server/lib/store/store.ts b/x-pack/plugins/reporting/server/lib/store/store.ts index ce28715e1ba46..0aae8b567bcdb 100644 --- a/x-pack/plugins/reporting/server/lib/store/store.ts +++ b/x-pack/plugins/reporting/server/lib/store/store.ts @@ -39,21 +39,20 @@ export class ReportingStore { private logger: LevelLogger; constructor(reporting: ReportingCore, logger: LevelLogger) { - this.logger = logger; - const config = reporting.getConfig(); const elasticsearch = reporting.getElasticsearchService(); this.client = elasticsearch.legacy.client; this.indexPrefix = config.get('index'); this.indexInterval = config.get('queue', 'indexInterval'); - this.jobSettings = { timeout: durationToNumber(config.get('queue', 'timeout')), browser_type: config.get('capture', 'browser', 'type'), max_attempts: config.get('capture', 'maxAttempts'), priority: 10, // unused }; + + this.logger = logger; } private async createIndex(indexName: string) { From 89c0b89f859a10b165075f0a369b759fa482f2ea Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 16 Sep 2020 16:08:11 -0700 Subject: [PATCH 14/23] use objects for the defaults when available --- .../plugins/reporting/server/config/schema.ts | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/reporting/server/config/schema.ts b/x-pack/plugins/reporting/server/config/schema.ts index 3e4efbd150847..4638cdaf0ef05 100644 --- a/x-pack/plugins/reporting/server/config/schema.ts +++ b/x-pack/plugins/reporting/server/config/schema.ts @@ -4,7 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ -import { schema, ByteSizeValue, TypeOf } from '@kbn/config-schema'; +import { ByteSizeValue, schema, TypeOf } from '@kbn/config-schema'; +import moment from 'moment'; const KibanaServerSchema = schema.object({ hostname: schema.maybe( @@ -45,9 +46,15 @@ const RulesSchema = schema.object({ const CaptureSchema = schema.object({ timeouts: schema.object({ - openUrl: schema.oneOf([schema.number(), schema.duration()], { defaultValue: 30000 }), - waitForElements: schema.oneOf([schema.number(), schema.duration()], { defaultValue: 30000 }), - renderComplete: schema.oneOf([schema.number(), schema.duration()], { defaultValue: 30000 }), + openUrl: schema.oneOf([schema.number(), schema.duration()], { + defaultValue: moment.duration('1m'), + }), + waitForElements: schema.oneOf([schema.number(), schema.duration()], { + defaultValue: moment.duration('30s'), + }), + renderComplete: schema.oneOf([schema.number(), schema.duration()], { + defaultValue: moment.duration('30s'), + }), }), networkPolicy: schema.object({ enabled: schema.boolean({ defaultValue: true }), @@ -67,7 +74,9 @@ const CaptureSchema = schema.object({ width: schema.number({ defaultValue: 1950 }), height: schema.number({ defaultValue: 1200 }), }), - loadDelay: schema.oneOf([schema.number(), schema.duration()], { defaultValue: 3000 }), + loadDelay: schema.oneOf([schema.number(), schema.duration()], { + defaultValue: moment.duration('3s'), + }), browser: schema.object({ autoDownload: schema.conditional( schema.contextRef('dist'), @@ -114,12 +123,12 @@ const CsvSchema = schema.object({ escapeFormulaValues: schema.boolean({ defaultValue: false }), enablePanelActionDownload: schema.boolean({ defaultValue: true }), maxSizeBytes: schema.oneOf([schema.number(), schema.byteSize()], { - defaultValue: ByteSizeValue.parse('10mb').getValueInBytes(), + defaultValue: ByteSizeValue.parse('10mb'), }), useByteOrderMarkEncoding: schema.boolean({ defaultValue: false }), scroll: schema.object({ duration: schema.string({ - defaultValue: '30s', + defaultValue: '30s', // this value is passed directly to ES, so string only format is preferred validate(value) { if (!/^[0-9]+(d|h|m|s|ms|micros|nanos)$/.test(value)) { return 'must be a duration string'; @@ -145,11 +154,15 @@ const IndexSchema = schema.string({ defaultValue: '.reporting' }); const PollSchema = schema.object({ jobCompletionNotifier: schema.object({ - interval: schema.oneOf([schema.number(), schema.duration()], { defaultValue: 10000 }), + interval: schema.oneOf([schema.number(), schema.duration()], { + defaultValue: moment.duration('10s'), + }), intervalErrorMultiplier: schema.number({ defaultValue: 5 }), }), jobsRefresh: schema.object({ - interval: schema.oneOf([schema.number(), schema.duration()], { defaultValue: 5000 }), + interval: schema.oneOf([schema.number(), schema.duration()], { + defaultValue: moment.duration('5s'), + }), intervalErrorMultiplier: schema.number({ defaultValue: 5 }), }), }); From d06391ec1040785c3e69f75209a3de854346d7c1 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 16 Sep 2020 16:08:18 -0700 Subject: [PATCH 15/23] fix pollInterval --- x-pack/plugins/reporting/server/config/schema.ts | 8 ++++++-- x-pack/plugins/reporting/server/lib/create_worker.ts | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/reporting/server/config/schema.ts b/x-pack/plugins/reporting/server/config/schema.ts index 4638cdaf0ef05..d34e0239d4fee 100644 --- a/x-pack/plugins/reporting/server/config/schema.ts +++ b/x-pack/plugins/reporting/server/config/schema.ts @@ -33,9 +33,13 @@ const KibanaServerSchema = schema.object({ const QueueSchema = schema.object({ indexInterval: schema.string({ defaultValue: 'week' }), pollEnabled: schema.boolean({ defaultValue: true }), - pollInterval: schema.number({ defaultValue: 3000 }), + pollInterval: schema.oneOf([schema.number(), schema.duration()], { + defaultValue: moment.duration('3s'), + }), pollIntervalErrorMultiplier: schema.number({ defaultValue: 10 }), - timeout: schema.oneOf([schema.number(), schema.duration()], { defaultValue: 120000 }), + timeout: schema.oneOf([schema.number(), schema.duration()], { + defaultValue: moment.duration('2m'), + }), }); const RulesSchema = schema.object({ diff --git a/x-pack/plugins/reporting/server/lib/create_worker.ts b/x-pack/plugins/reporting/server/lib/create_worker.ts index dd5c560455274..c1c88dd8a54ba 100644 --- a/x-pack/plugins/reporting/server/lib/create_worker.ts +++ b/x-pack/plugins/reporting/server/lib/create_worker.ts @@ -6,6 +6,7 @@ import { CancellationToken } from '../../common'; import { PLUGIN_ID } from '../../common/constants'; +import { durationToNumber } from '../../common/schema_utils'; import { ReportingCore } from '../../server'; import { LevelLogger } from '../../server/lib'; import { ExportTypeDefinition, JobSource, RunTaskFn } from '../../server/types'; @@ -57,7 +58,7 @@ export function createWorkerFactory(reporting: ReportingCore, log const workerOptions = { kibanaName, kibanaId, - interval: queueConfig.pollInterval, + interval: durationToNumber(queueConfig.pollInterval), intervalErrorMultiplier: queueConfig.pollIntervalErrorMultiplier, }; const worker = queue.registerWorker(PLUGIN_ID, workerFn, workerOptions); From 5bc26d9ce7f4f547d746748ccd0499eebf13cc48 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 16 Sep 2020 16:30:44 -0700 Subject: [PATCH 16/23] fix snapshots --- .../reporting/server/config/schema.test.ts | 282 ++++++++++++------ 1 file changed, 196 insertions(+), 86 deletions(-) diff --git a/x-pack/plugins/reporting/server/config/schema.test.ts b/x-pack/plugins/reporting/server/config/schema.test.ts index 70965bc1a120a..417bf98963a69 100644 --- a/x-pack/plugins/reporting/server/config/schema.test.ts +++ b/x-pack/plugins/reporting/server/config/schema.test.ts @@ -8,101 +8,211 @@ import { ConfigSchema } from './schema'; describe('Reporting Config Schema', () => { it(`context {"dev":false,"dist":false} produces correct config`, () => { - expect(ConfigSchema.validate({}, { dev: false, dist: false })).toMatchObject({ - capture: { - browser: { - autoDownload: true, - chromium: { proxy: { enabled: false } }, - type: 'chromium', + expect(ConfigSchema.validate({}, { dev: false, dist: false })).toMatchInlineSnapshot(` + Object { + "capture": Object { + "browser": Object { + "autoDownload": true, + "chromium": Object { + "proxy": Object { + "enabled": false, + }, + }, + "type": "chromium", + }, + "loadDelay": "P0D", + "maxAttempts": 1, + "networkPolicy": Object { + "enabled": true, + "rules": Array [ + Object { + "allow": true, + "host": undefined, + "protocol": "http:", + }, + Object { + "allow": true, + "host": undefined, + "protocol": "https:", + }, + Object { + "allow": true, + "host": undefined, + "protocol": "ws:", + }, + Object { + "allow": true, + "host": undefined, + "protocol": "wss:", + }, + Object { + "allow": true, + "host": undefined, + "protocol": "data:", + }, + Object { + "allow": false, + "host": undefined, + "protocol": undefined, + }, + ], + }, + "timeouts": Object { + "openUrl": "P0D", + "renderComplete": "P0D", + "waitForElements": "P0D", + }, + "viewport": Object { + "height": 1200, + "width": 1950, + }, + "zoom": 2, + }, + "csv": Object { + "checkForFormulas": true, + "enablePanelActionDownload": true, + "escapeFormulaValues": false, + "maxSizeBytes": ByteSizeValue { + "valueInBytes": 10485760, + }, + "scroll": Object { + "duration": "30s", + "size": 500, + }, + "useByteOrderMarkEncoding": false, }, - loadDelay: 3000, - maxAttempts: 1, - networkPolicy: { - enabled: true, - rules: [ - { allow: true, host: undefined, protocol: 'http:' }, - { allow: true, host: undefined, protocol: 'https:' }, - { allow: true, host: undefined, protocol: 'ws:' }, - { allow: true, host: undefined, protocol: 'wss:' }, - { allow: true, host: undefined, protocol: 'data:' }, - { allow: false, host: undefined, protocol: undefined }, + "enabled": true, + "encryptionKey": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "index": ".reporting", + "kibanaServer": Object {}, + "poll": Object { + "jobCompletionNotifier": Object { + "interval": "P0D", + "intervalErrorMultiplier": 5, + }, + "jobsRefresh": Object { + "interval": "P0D", + "intervalErrorMultiplier": 5, + }, + }, + "queue": Object { + "indexInterval": "week", + "pollEnabled": true, + "pollInterval": "P0D", + "pollIntervalErrorMultiplier": 10, + "timeout": "P0D", + }, + "roles": Object { + "allow": Array [ + "reporting_user", ], }, - viewport: { height: 1200, width: 1950 }, - zoom: 2, - }, - csv: { - checkForFormulas: true, - enablePanelActionDownload: true, - maxSizeBytes: 10485760, - scroll: { duration: '30s', size: 500 }, - }, - encryptionKey: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', - index: '.reporting', - kibanaServer: {}, - poll: { - jobCompletionNotifier: { interval: 10000, intervalErrorMultiplier: 5 }, - jobsRefresh: { interval: 5000, intervalErrorMultiplier: 5 }, - }, - queue: { - indexInterval: 'week', - pollEnabled: true, - pollInterval: 3000, - pollIntervalErrorMultiplier: 10, - timeout: 120000, - }, - roles: { allow: ['reporting_user'] }, - }); + } + `); }); it(`context {"dev":false,"dist":true} produces correct config`, () => { - expect(ConfigSchema.validate({}, { dev: false, dist: true })).toMatchObject({ - capture: { - browser: { - autoDownload: false, - chromium: { - inspect: false, - proxy: { enabled: false }, - }, - type: 'chromium', + expect(ConfigSchema.validate({}, { dev: false, dist: true })).toMatchInlineSnapshot(` + Object { + "capture": Object { + "browser": Object { + "autoDownload": false, + "chromium": Object { + "inspect": false, + "proxy": Object { + "enabled": false, + }, + }, + "type": "chromium", + }, + "loadDelay": "P0D", + "maxAttempts": 3, + "networkPolicy": Object { + "enabled": true, + "rules": Array [ + Object { + "allow": true, + "host": undefined, + "protocol": "http:", + }, + Object { + "allow": true, + "host": undefined, + "protocol": "https:", + }, + Object { + "allow": true, + "host": undefined, + "protocol": "ws:", + }, + Object { + "allow": true, + "host": undefined, + "protocol": "wss:", + }, + Object { + "allow": true, + "host": undefined, + "protocol": "data:", + }, + Object { + "allow": false, + "host": undefined, + "protocol": undefined, + }, + ], + }, + "timeouts": Object { + "openUrl": "P0D", + "renderComplete": "P0D", + "waitForElements": "P0D", + }, + "viewport": Object { + "height": 1200, + "width": 1950, + }, + "zoom": 2, + }, + "csv": Object { + "checkForFormulas": true, + "enablePanelActionDownload": true, + "escapeFormulaValues": false, + "maxSizeBytes": ByteSizeValue { + "valueInBytes": 10485760, + }, + "scroll": Object { + "duration": "30s", + "size": 500, + }, + "useByteOrderMarkEncoding": false, }, - loadDelay: 3000, - maxAttempts: 3, - networkPolicy: { - enabled: true, - rules: [ - { allow: true, host: undefined, protocol: 'http:' }, - { allow: true, host: undefined, protocol: 'https:' }, - { allow: true, host: undefined, protocol: 'ws:' }, - { allow: true, host: undefined, protocol: 'wss:' }, - { allow: true, host: undefined, protocol: 'data:' }, - { allow: false, host: undefined, protocol: undefined }, + "enabled": true, + "index": ".reporting", + "kibanaServer": Object {}, + "poll": Object { + "jobCompletionNotifier": Object { + "interval": "P0D", + "intervalErrorMultiplier": 5, + }, + "jobsRefresh": Object { + "interval": "P0D", + "intervalErrorMultiplier": 5, + }, + }, + "queue": Object { + "indexInterval": "week", + "pollEnabled": true, + "pollInterval": "P0D", + "pollIntervalErrorMultiplier": 10, + "timeout": "P0D", + }, + "roles": Object { + "allow": Array [ + "reporting_user", ], }, - viewport: { height: 1200, width: 1950 }, - zoom: 2, - }, - csv: { - checkForFormulas: true, - enablePanelActionDownload: true, - maxSizeBytes: 10485760, - scroll: { duration: '30s', size: 500 }, - }, - index: '.reporting', - kibanaServer: {}, - poll: { - jobCompletionNotifier: { interval: 10000, intervalErrorMultiplier: 5 }, - jobsRefresh: { interval: 5000, intervalErrorMultiplier: 5 }, - }, - queue: { - indexInterval: 'week', - pollEnabled: true, - pollInterval: 3000, - pollIntervalErrorMultiplier: 10, - timeout: 120000, - }, - roles: { allow: ['reporting_user'] }, - }); + } + `); }); it('allows Duration values for certain keys', () => { From 79aef77618185e2dbd75a90765ca8e3d87ec153e Mon Sep 17 00:00:00 2001 From: Tim Sullivan Date: Wed, 16 Sep 2020 16:33:19 -0700 Subject: [PATCH 17/23] Update report_listing.tsx --- x-pack/plugins/reporting/public/components/report_listing.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/reporting/public/components/report_listing.tsx b/x-pack/plugins/reporting/public/components/report_listing.tsx index a09ec22a93797..f326d365351f2 100644 --- a/x-pack/plugins/reporting/public/components/report_listing.tsx +++ b/x-pack/plugins/reporting/public/components/report_listing.tsx @@ -185,7 +185,6 @@ class ReportListingUi extends Component { public componentDidMount() { this.mounted = true; const { pollConfig, license$ } = this.props; - const pollFrequencyInMillis = durationToNumber(pollConfig.jobsRefresh.interval); this.poller = new Poller({ functionToPoll: () => { From e5f6a512c74afad83eaa254e9b6153c2f769aa86 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 16 Sep 2020 17:00:38 -0700 Subject: [PATCH 18/23] call new ByteSizeValue on server side only --- x-pack/plugins/reporting/common/schema_utils.ts | 8 -------- .../plugins/reporting/server/routes/diagnostic/config.ts | 9 ++++++++- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/reporting/common/schema_utils.ts b/x-pack/plugins/reporting/common/schema_utils.ts index 1f924ab272bcc..f9b5c90e3c366 100644 --- a/x-pack/plugins/reporting/common/schema_utils.ts +++ b/x-pack/plugins/reporting/common/schema_utils.ts @@ -19,14 +19,6 @@ export const durationToNumber = (value: number | moment.Duration): number => { return value.asMilliseconds(); }; -export const numberToByteSizeValue = (value: number | ByteSizeValue) => { - if (typeof value === 'number') { - return new ByteSizeValue(value); - } - - return value; -}; - export const byteSizeValueToNumber = (value: number | ByteSizeValue) => { if (typeof value === 'number') { return value; diff --git a/x-pack/plugins/reporting/server/routes/diagnostic/config.ts b/x-pack/plugins/reporting/server/routes/diagnostic/config.ts index 3ce46cf233472..95c3a05bbf680 100644 --- a/x-pack/plugins/reporting/server/routes/diagnostic/config.ts +++ b/x-pack/plugins/reporting/server/routes/diagnostic/config.ts @@ -9,7 +9,6 @@ import { i18n } from '@kbn/i18n'; import { defaults, get } from 'lodash'; import { ReportingCore } from '../..'; import { API_DIAGNOSE_URL } from '../../../common/constants'; -import { numberToByteSizeValue } from '../../../common/schema_utils'; import { LevelLogger as Logger } from '../../lib'; import { DiagnosticResponse } from '../../types'; import { authorizedUserPreRoutingFactory } from '../lib/authorized_user_pre_routing'; @@ -17,6 +16,14 @@ import { authorizedUserPreRoutingFactory } from '../lib/authorized_user_pre_rout const KIBANA_MAX_SIZE_BYTES_PATH = 'csv.maxSizeBytes'; const ES_MAX_SIZE_BYTES_PATH = 'http.max_content_length'; +const numberToByteSizeValue = (value: number | ByteSizeValue) => { + if (typeof value === 'number') { + return new ByteSizeValue(value); + } + + return value; +}; + export const registerDiagnoseConfig = (reporting: ReportingCore, logger: Logger) => { const setupDeps = reporting.getPluginSetupDeps(); const userHandler = authorizedUserPreRoutingFactory(reporting); From 729d1e7bae53b62f23f305151b9799635c477392 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 16 Sep 2020 21:28:43 -0700 Subject: [PATCH 19/23] revert xpack.reporting.poll --- x-pack/plugins/reporting/server/config/index.ts | 2 ++ x-pack/plugins/reporting/server/config/schema.ts | 14 ++++++-------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/reporting/server/config/index.ts b/x-pack/plugins/reporting/server/config/index.ts index a89b952702e1b..b9c6f8e7591e3 100644 --- a/x-pack/plugins/reporting/server/config/index.ts +++ b/x-pack/plugins/reporting/server/config/index.ts @@ -17,6 +17,8 @@ export const config: PluginConfigDescriptor = { unused('capture.concurrency'), unused('capture.settleTime'), unused('capture.timeout'), + unused('poll.jobCompletionNotifier.intervalErrorMultiplier'), + unused('poll.jobsRefresh.intervalErrorMultiplier'), unused('kibanaApp'), ], }; diff --git a/x-pack/plugins/reporting/server/config/schema.ts b/x-pack/plugins/reporting/server/config/schema.ts index d34e0239d4fee..b6da844c9c558 100644 --- a/x-pack/plugins/reporting/server/config/schema.ts +++ b/x-pack/plugins/reporting/server/config/schema.ts @@ -156,18 +156,16 @@ const RolesSchema = schema.object({ const IndexSchema = schema.string({ defaultValue: '.reporting' }); +// Browser side polling: job completion notifier, management table auto-refresh +// NOTE: can not use schema.duration, a bug prevents it being passed to the browser correctly const PollSchema = schema.object({ jobCompletionNotifier: schema.object({ - interval: schema.oneOf([schema.number(), schema.duration()], { - defaultValue: moment.duration('10s'), - }), - intervalErrorMultiplier: schema.number({ defaultValue: 5 }), + interval: schema.number({ defaultValue: 10000 }), + intervalErrorMultiplier: schema.number({ defaultValue: 5 }), // unused }), jobsRefresh: schema.object({ - interval: schema.oneOf([schema.number(), schema.duration()], { - defaultValue: moment.duration('5s'), - }), - intervalErrorMultiplier: schema.number({ defaultValue: 5 }), + interval: schema.number({ defaultValue: 5000 }), + intervalErrorMultiplier: schema.number({ defaultValue: 5 }), // unused }), }); From 31342891c3d25357e41a7c6d0073db213194442b Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Thu, 17 Sep 2020 09:44:39 -0700 Subject: [PATCH 20/23] fix ts --- .../reporting/public/components/report_listing.test.tsx | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/reporting/public/components/report_listing.test.tsx b/x-pack/plugins/reporting/public/components/report_listing.test.tsx index 36167f569ee08..03b66ce8fe3fb 100644 --- a/x-pack/plugins/reporting/public/components/report_listing.test.tsx +++ b/x-pack/plugins/reporting/public/components/report_listing.test.tsx @@ -4,12 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ -import moment from 'moment'; import React from 'react'; import { Observable } from 'rxjs'; import { mountWithIntl } from 'test_utils/enzyme_helpers'; import { ILicense } from '../../../licensing/public'; import { ReportingAPIClient } from '../lib/reporting_api_client'; +import { ReportListing } from './report_listing'; jest.mock('@elastic/eui/lib/services/accessibility/html_id_generator', () => { return { @@ -17,8 +17,6 @@ jest.mock('@elastic/eui/lib/services/accessibility/html_id_generator', () => { }; }); -import { ReportListing } from './report_listing'; - const reportingAPIClient = { list: () => Promise.resolve([ @@ -52,14 +50,13 @@ const toasts = { addDanger: jest.fn(), } as any; -const fiveSeconds = moment.duration(5, 's'); const mockPollConfig = { jobCompletionNotifier: { - interval: fiveSeconds, + interval: 5000, intervalErrorMultiplier: 3, }, jobsRefresh: { - interval: fiveSeconds, + interval: 5000, intervalErrorMultiplier: 3, }, }; From a09f95655b9f04c005e23280b5dd3785570f02bb Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Thu, 17 Sep 2020 09:47:45 -0700 Subject: [PATCH 21/23] fix snapshot --- x-pack/plugins/reporting/server/config/schema.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/reporting/server/config/schema.test.ts b/x-pack/plugins/reporting/server/config/schema.test.ts index 417bf98963a69..6095fcaf7963f 100644 --- a/x-pack/plugins/reporting/server/config/schema.test.ts +++ b/x-pack/plugins/reporting/server/config/schema.test.ts @@ -87,11 +87,11 @@ describe('Reporting Config Schema', () => { "kibanaServer": Object {}, "poll": Object { "jobCompletionNotifier": Object { - "interval": "P0D", + "interval": 10000, "intervalErrorMultiplier": 5, }, "jobsRefresh": Object { - "interval": "P0D", + "interval": 5000, "intervalErrorMultiplier": 5, }, }, @@ -191,11 +191,11 @@ describe('Reporting Config Schema', () => { "kibanaServer": Object {}, "poll": Object { "jobCompletionNotifier": Object { - "interval": "P0D", + "interval": 10000, "intervalErrorMultiplier": 5, }, "jobsRefresh": Object { - "interval": "P0D", + "interval": 5000, "intervalErrorMultiplier": 5, }, }, From e78fae2c8c80d5279e2e5b32a90428641f2f10fc Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Thu, 17 Sep 2020 10:00:35 -0700 Subject: [PATCH 22/23] use correct input for duration --- .../reporting/server/config/schema.test.ts | 24 +++++++++---------- .../plugins/reporting/server/config/schema.ts | 12 +++++----- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/reporting/server/config/schema.test.ts b/x-pack/plugins/reporting/server/config/schema.test.ts index 6095fcaf7963f..9fc3d4329879e 100644 --- a/x-pack/plugins/reporting/server/config/schema.test.ts +++ b/x-pack/plugins/reporting/server/config/schema.test.ts @@ -20,7 +20,7 @@ describe('Reporting Config Schema', () => { }, "type": "chromium", }, - "loadDelay": "P0D", + "loadDelay": "PT3S", "maxAttempts": 1, "networkPolicy": Object { "enabled": true, @@ -58,9 +58,9 @@ describe('Reporting Config Schema', () => { ], }, "timeouts": Object { - "openUrl": "P0D", - "renderComplete": "P0D", - "waitForElements": "P0D", + "openUrl": "PT1M", + "renderComplete": "PT30S", + "waitForElements": "PT30S", }, "viewport": Object { "height": 1200, @@ -98,9 +98,9 @@ describe('Reporting Config Schema', () => { "queue": Object { "indexInterval": "week", "pollEnabled": true, - "pollInterval": "P0D", + "pollInterval": "PT3S", "pollIntervalErrorMultiplier": 10, - "timeout": "P0D", + "timeout": "PT2M", }, "roles": Object { "allow": Array [ @@ -125,7 +125,7 @@ describe('Reporting Config Schema', () => { }, "type": "chromium", }, - "loadDelay": "P0D", + "loadDelay": "PT3S", "maxAttempts": 3, "networkPolicy": Object { "enabled": true, @@ -163,9 +163,9 @@ describe('Reporting Config Schema', () => { ], }, "timeouts": Object { - "openUrl": "P0D", - "renderComplete": "P0D", - "waitForElements": "P0D", + "openUrl": "PT1M", + "renderComplete": "PT30S", + "waitForElements": "PT30S", }, "viewport": Object { "height": 1200, @@ -202,9 +202,9 @@ describe('Reporting Config Schema', () => { "queue": Object { "indexInterval": "week", "pollEnabled": true, - "pollInterval": "P0D", + "pollInterval": "PT3S", "pollIntervalErrorMultiplier": 10, - "timeout": "P0D", + "timeout": "PT2M", }, "roles": Object { "allow": Array [ diff --git a/x-pack/plugins/reporting/server/config/schema.ts b/x-pack/plugins/reporting/server/config/schema.ts index b6da844c9c558..8276e8b49d348 100644 --- a/x-pack/plugins/reporting/server/config/schema.ts +++ b/x-pack/plugins/reporting/server/config/schema.ts @@ -34,11 +34,11 @@ const QueueSchema = schema.object({ indexInterval: schema.string({ defaultValue: 'week' }), pollEnabled: schema.boolean({ defaultValue: true }), pollInterval: schema.oneOf([schema.number(), schema.duration()], { - defaultValue: moment.duration('3s'), + defaultValue: moment.duration({ seconds: 3 }), }), pollIntervalErrorMultiplier: schema.number({ defaultValue: 10 }), timeout: schema.oneOf([schema.number(), schema.duration()], { - defaultValue: moment.duration('2m'), + defaultValue: moment.duration({ minutes: 2 }), }), }); @@ -51,13 +51,13 @@ const RulesSchema = schema.object({ const CaptureSchema = schema.object({ timeouts: schema.object({ openUrl: schema.oneOf([schema.number(), schema.duration()], { - defaultValue: moment.duration('1m'), + defaultValue: moment.duration({ minutes: 1 }), }), waitForElements: schema.oneOf([schema.number(), schema.duration()], { - defaultValue: moment.duration('30s'), + defaultValue: moment.duration({ seconds: 30 }), }), renderComplete: schema.oneOf([schema.number(), schema.duration()], { - defaultValue: moment.duration('30s'), + defaultValue: moment.duration({ seconds: 30 }), }), }), networkPolicy: schema.object({ @@ -79,7 +79,7 @@ const CaptureSchema = schema.object({ height: schema.number({ defaultValue: 1200 }), }), loadDelay: schema.oneOf([schema.number(), schema.duration()], { - defaultValue: moment.duration('3s'), + defaultValue: moment.duration({ seconds: 3 }), }), browser: schema.object({ autoDownload: schema.conditional( From 79566628f979c312b5c1af7eb9d6a13a31c93811 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Thu, 17 Sep 2020 10:04:20 -0700 Subject: [PATCH 23/23] revert reorganize imports --- .../reporting/public/components/report_listing.test.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/reporting/public/components/report_listing.test.tsx b/x-pack/plugins/reporting/public/components/report_listing.test.tsx index 03b66ce8fe3fb..cacae943687e1 100644 --- a/x-pack/plugins/reporting/public/components/report_listing.test.tsx +++ b/x-pack/plugins/reporting/public/components/report_listing.test.tsx @@ -9,7 +9,6 @@ import { Observable } from 'rxjs'; import { mountWithIntl } from 'test_utils/enzyme_helpers'; import { ILicense } from '../../../licensing/public'; import { ReportingAPIClient } from '../lib/reporting_api_client'; -import { ReportListing } from './report_listing'; jest.mock('@elastic/eui/lib/services/accessibility/html_id_generator', () => { return { @@ -17,6 +16,8 @@ jest.mock('@elastic/eui/lib/services/accessibility/html_id_generator', () => { }; }); +import { ReportListing } from './report_listing'; + const reportingAPIClient = { list: () => Promise.resolve([