From 6537b15d949ce913b323496ceb4f2cf5d1b487ae Mon Sep 17 00:00:00 2001 From: Stacey Gammon Date: Wed, 13 May 2020 11:10:19 -0400 Subject: [PATCH 1/3] add section on why to prefer async over sync accessor (#66284) --- src/core/CONVENTIONS.md | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/core/CONVENTIONS.md b/src/core/CONVENTIONS.md index a82cc27839a1d..c124169580337 100644 --- a/src/core/CONVENTIONS.md +++ b/src/core/CONVENTIONS.md @@ -201,6 +201,36 @@ export class MyPlugin implements Plugin { } ``` +Prefer the pattern shown above, using `core.getStartServices()`, rather than store local references retrieved from `start`. + +**Bad:** +```ts +export class MyPlugin implements Plugin { + // Anti pattern + private coreStart?: CoreStart; + private depsStart?: DepsStart; + + public setup(core) { + core.application.register({ + id: 'my-app', + async mount(params) { + const { renderApp } = await import('./application/my_app'); + // Anti pattern - use `core.getStartServices()` instead! + return renderApp(this.coreStart, this.depsStart, params); + } + }); + } + + public start(core, deps) { + // Anti pattern + this.coreStart = core; + this.depsStart = deps; + } +} +``` + +The main reason to prefer the provided async accessor, is that it doesn't requires the developer to understand and reason about when that function can be called. Having an API that fails sometimes isn't a good API design, and it makes accurately testing this difficult. + #### Services Service structure should mirror the plugin lifecycle to make reasoning about how the service is executed more clear. From ff1d1298138ec9730819b27a21749d77ebaf2142 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cau=C3=AA=20Marcondes?= <55978943+cauemarcondes@users.noreply.github.com> Date: Wed, 13 May 2020 16:12:22 +0100 Subject: [PATCH 2/3] Fixing jest when running it with --watchAll (#66384) --- x-pack/dev-tools/jest/create_jest_config.js | 7 +------ x-pack/test_utils/jest/config.js | 1 - 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/x-pack/dev-tools/jest/create_jest_config.js b/x-pack/dev-tools/jest/create_jest_config.js index 4f1251321b005..75f8a3c156e01 100644 --- a/x-pack/dev-tools/jest/create_jest_config.js +++ b/x-pack/dev-tools/jest/create_jest_config.js @@ -8,12 +8,7 @@ export function createJestConfig({ kibanaDirectory, xPackKibanaDirectory }) { const fileMockPath = `${kibanaDirectory}/src/dev/jest/mocks/file_mock.js`; return { rootDir: xPackKibanaDirectory, - roots: [ - '/plugins', - '/legacy/plugins', - '/legacy/server', - '/test_utils/jest/contract_tests', - ], + roots: ['/plugins', '/legacy/plugins', '/legacy/server'], moduleFileExtensions: ['js', 'json', 'ts', 'tsx'], moduleNameMapper: { '@elastic/eui$': `${kibanaDirectory}/node_modules/@elastic/eui/test-env`, diff --git a/x-pack/test_utils/jest/config.js b/x-pack/test_utils/jest/config.js index 6ca2818f9e6af..66b88cbdeba17 100644 --- a/x-pack/test_utils/jest/config.js +++ b/x-pack/test_utils/jest/config.js @@ -13,7 +13,6 @@ export default { '/legacy/server', '/legacy/common', '/test_utils/jest/integration_tests', - '/test_utils/jest/contract_tests', ], collectCoverageFrom: ['legacy/plugins/**/*.js', 'legacy/common/**/*.js', 'legacy/server/**/*.js'], moduleNameMapper: { From 9ab73efd186ff3dd5f7f7ee41d90a1ace0dd917c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Wed, 13 May 2020 16:35:22 +0100 Subject: [PATCH 3/3] Remove Newsfeed leftovers from the Legacy codebase (#66084) --- .github/CODEOWNERS | 1 + src/legacy/core_plugins/newsfeed/index.ts | 71 ---------- src/legacy/core_plugins/newsfeed/package.json | 4 - .../core_plugins/newsfeed/public/index.scss | 3 - .../components/header_alert/_index.scss | 27 ---- .../components/header_alert/header_alert.tsx | 76 ----------- .../plugins/newsfeed/common/constants.ts | 19 +-- src/plugins/newsfeed/kibana.json | 2 +- .../public/components/flyout_list.tsx | 4 +- .../components/newsfeed_header_nav_button.tsx | 2 +- src/plugins/newsfeed/public/lib/api.test.ts | 38 +++--- src/plugins/newsfeed/public/lib/api.ts | 14 +- src/plugins/newsfeed/public/plugin.tsx | 25 ++-- src/plugins/newsfeed/{ => public}/types.ts | 18 ++- src/plugins/newsfeed/server/config.ts | 44 ++++++ src/plugins/newsfeed/server/index.ts | 36 +++++ .../{constants.ts => server/plugin.ts} | 12 +- .../fixtures/plugins/newsfeed/kibana.json | 6 + .../plugins/newsfeed/newsfeed_simulation.ts | 126 ------------------ .../fixtures/plugins/newsfeed/package.json | 7 - .../fixtures/plugins/newsfeed/server/index.ts | 10 +- .../plugins/newsfeed/server/plugin.ts | 97 ++++++++++++++ 22 files changed, 256 insertions(+), 386 deletions(-) delete mode 100644 src/legacy/core_plugins/newsfeed/index.ts delete mode 100644 src/legacy/core_plugins/newsfeed/package.json delete mode 100644 src/legacy/core_plugins/newsfeed/public/index.scss delete mode 100644 src/legacy/core_plugins/newsfeed/public/np_ready/components/header_alert/_index.scss delete mode 100644 src/legacy/core_plugins/newsfeed/public/np_ready/components/header_alert/header_alert.tsx rename test/common/fixtures/plugins/newsfeed/index.ts => src/plugins/newsfeed/common/constants.ts (65%) rename src/plugins/newsfeed/{ => public}/types.ts (80%) create mode 100644 src/plugins/newsfeed/server/config.ts create mode 100644 src/plugins/newsfeed/server/index.ts rename src/plugins/newsfeed/{constants.ts => server/plugin.ts} (81%) create mode 100644 test/common/fixtures/plugins/newsfeed/kibana.json delete mode 100644 test/common/fixtures/plugins/newsfeed/newsfeed_simulation.ts delete mode 100644 test/common/fixtures/plugins/newsfeed/package.json rename src/legacy/core_plugins/newsfeed/constants.ts => test/common/fixtures/plugins/newsfeed/server/index.ts (76%) create mode 100644 test/common/fixtures/plugins/newsfeed/server/plugin.ts diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 477089e48367f..75a415ee7e128 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -163,6 +163,7 @@ /packages/kbn-analytics/ @elastic/pulse /src/legacy/core_plugins/ui_metric/ @elastic/pulse /src/plugins/kibana_usage_collection/ @elastic/pulse +/src/plugins/newsfeed/ @elastic/pulse /src/plugins/telemetry/ @elastic/pulse /src/plugins/telemetry_collection_manager/ @elastic/pulse /src/plugins/telemetry_management_section/ @elastic/pulse diff --git a/src/legacy/core_plugins/newsfeed/index.ts b/src/legacy/core_plugins/newsfeed/index.ts deleted file mode 100644 index cf8852be09a1e..0000000000000 --- a/src/legacy/core_plugins/newsfeed/index.ts +++ /dev/null @@ -1,71 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { resolve } from 'path'; -import { LegacyPluginApi, LegacyPluginSpec, ArrayOrItem } from 'src/legacy/plugin_discovery/types'; -import { Legacy } from 'kibana'; -import { NewsfeedPluginInjectedConfig } from '../../../plugins/newsfeed/types'; -import { - PLUGIN_ID, - DEFAULT_SERVICE_URLROOT, - DEV_SERVICE_URLROOT, - DEFAULT_SERVICE_PATH, -} from './constants'; - -// eslint-disable-next-line import/no-default-export -export default function(kibana: LegacyPluginApi): ArrayOrItem { - const pluginSpec: Legacy.PluginSpecOptions = { - id: PLUGIN_ID, - config(Joi: any) { - // NewsfeedPluginInjectedConfig in Joi form - return Joi.object({ - enabled: Joi.boolean().default(true), - service: Joi.object({ - pathTemplate: Joi.string().default(DEFAULT_SERVICE_PATH), - urlRoot: Joi.when('$prod', { - is: true, - then: Joi.string().default(DEFAULT_SERVICE_URLROOT), - otherwise: Joi.string().default(DEV_SERVICE_URLROOT), - }), - }).default(), - defaultLanguage: Joi.string().default('en'), - mainInterval: Joi.number().default(120 * 1000), // (2min) How often to retry failed fetches, and/or check if newsfeed items need to be refreshed from remote - fetchInterval: Joi.number().default(86400 * 1000), // (1day) How often to fetch remote and reset the last fetched time - }).default(); - }, - uiExports: { - styleSheetPaths: resolve(__dirname, 'public/index.scss'), - injectDefaultVars(server): NewsfeedPluginInjectedConfig { - const config = server.config(); - return { - newsfeed: { - service: { - pathTemplate: config.get('newsfeed.service.pathTemplate') as string, - urlRoot: config.get('newsfeed.service.urlRoot') as string, - }, - defaultLanguage: config.get('newsfeed.defaultLanguage') as string, - mainInterval: config.get('newsfeed.mainInterval') as number, - fetchInterval: config.get('newsfeed.fetchInterval') as number, - }, - }; - }, - }, - }; - return new kibana.Plugin(pluginSpec); -} diff --git a/src/legacy/core_plugins/newsfeed/package.json b/src/legacy/core_plugins/newsfeed/package.json deleted file mode 100644 index d4d753f32b0f9..0000000000000 --- a/src/legacy/core_plugins/newsfeed/package.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "name": "newsfeed", - "version": "kibana" -} diff --git a/src/legacy/core_plugins/newsfeed/public/index.scss b/src/legacy/core_plugins/newsfeed/public/index.scss deleted file mode 100644 index a77132379041c..0000000000000 --- a/src/legacy/core_plugins/newsfeed/public/index.scss +++ /dev/null @@ -1,3 +0,0 @@ -@import 'src/legacy/ui/public/styles/styling_constants'; - -@import './np_ready/components/header_alert/_index'; diff --git a/src/legacy/core_plugins/newsfeed/public/np_ready/components/header_alert/_index.scss b/src/legacy/core_plugins/newsfeed/public/np_ready/components/header_alert/_index.scss deleted file mode 100644 index e25dbd25daaf5..0000000000000 --- a/src/legacy/core_plugins/newsfeed/public/np_ready/components/header_alert/_index.scss +++ /dev/null @@ -1,27 +0,0 @@ -@import '@elastic/eui/src/components/header/variables'; - -.kbnNews__flyout { - top: $euiHeaderChildSize + 1px; - height: calc(100% - #{$euiHeaderChildSize}); -} - -.kbnNewsFeed__headerAlert.euiHeaderAlert { - margin-bottom: $euiSizeL; - padding: 0 $euiSizeS $euiSizeL; - border-bottom: $euiBorderThin; - border-top: none; - - .euiHeaderAlert__title { - @include euiTitle('xs'); - margin-bottom: $euiSizeS; - } - - .euiHeaderAlert__text { - @include euiFontSizeS; - margin-bottom: $euiSize; - } - - .euiHeaderAlert__action { - @include euiFontSizeS; - } -} diff --git a/src/legacy/core_plugins/newsfeed/public/np_ready/components/header_alert/header_alert.tsx b/src/legacy/core_plugins/newsfeed/public/np_ready/components/header_alert/header_alert.tsx deleted file mode 100644 index c3c3e4144fca8..0000000000000 --- a/src/legacy/core_plugins/newsfeed/public/np_ready/components/header_alert/header_alert.tsx +++ /dev/null @@ -1,76 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import React from 'react'; -import PropTypes from 'prop-types'; -import classNames from 'classnames'; - -import { EuiFlexGroup, EuiFlexItem, EuiI18n } from '@elastic/eui'; - -interface IEuiHeaderAlertProps { - action: JSX.Element; - className?: string; - date: string; - text: string; - title: string; - badge?: JSX.Element; - rest?: string[]; -} - -export const EuiHeaderAlert = ({ - action, - className, - date, - text, - title, - badge, - ...rest -}: IEuiHeaderAlertProps) => { - const classes = classNames('euiHeaderAlert', 'kbnNewsFeed__headerAlert', className); - - const badgeContent = badge || null; - - return ( - - {(dismiss: any) => ( -
- - -
{date}
- - {badgeContent} - - -
{title}
-
{text}
-
{action}
-
- )} - - ); -}; - -EuiHeaderAlert.propTypes = { - action: PropTypes.node, - className: PropTypes.string, - date: PropTypes.node.isRequired, - text: PropTypes.node, - title: PropTypes.node.isRequired, - badge: PropTypes.node, -}; diff --git a/test/common/fixtures/plugins/newsfeed/index.ts b/src/plugins/newsfeed/common/constants.ts similarity index 65% rename from test/common/fixtures/plugins/newsfeed/index.ts rename to src/plugins/newsfeed/common/constants.ts index beee9bb5c6069..6bc95873a342d 100644 --- a/test/common/fixtures/plugins/newsfeed/index.ts +++ b/src/plugins/newsfeed/common/constants.ts @@ -17,17 +17,10 @@ * under the License. */ -import Hapi from 'hapi'; -import { initPlugin as initNewsfeed } from './newsfeed_simulation'; +export const NEWSFEED_FALLBACK_LANGUAGE = 'en'; +export const NEWSFEED_LAST_FETCH_STORAGE_KEY = 'newsfeed.lastfetchtime'; +export const NEWSFEED_HASH_SET_STORAGE_KEY = 'newsfeed.hashes'; -const NAME = 'newsfeed-FTS-external-service-simulators'; - -// eslint-disable-next-line import/no-default-export -export default function(kibana: any) { - return new kibana.Plugin({ - name: NAME, - init: (server: Hapi.Server) => { - initNewsfeed(server, `/api/_${NAME}`); - }, - }); -} +export const NEWSFEED_DEFAULT_SERVICE_BASE_URL = 'https://feeds.elastic.co'; +export const NEWSFEED_DEV_SERVICE_BASE_URL = 'https://feeds-staging.elastic.co'; +export const NEWSFEED_DEFAULT_SERVICE_PATH = '/kibana/v{VERSION}.json'; diff --git a/src/plugins/newsfeed/kibana.json b/src/plugins/newsfeed/kibana.json index 9d49b42424a06..b9f37b67f6921 100644 --- a/src/plugins/newsfeed/kibana.json +++ b/src/plugins/newsfeed/kibana.json @@ -1,6 +1,6 @@ { "id": "newsfeed", "version": "kibana", - "server": false, + "server": true, "ui": true } diff --git a/src/plugins/newsfeed/public/components/flyout_list.tsx b/src/plugins/newsfeed/public/components/flyout_list.tsx index bd554d7d98b7d..6e9444c950107 100644 --- a/src/plugins/newsfeed/public/components/flyout_list.tsx +++ b/src/plugins/newsfeed/public/components/flyout_list.tsx @@ -29,11 +29,11 @@ import { EuiButtonEmpty, EuiText, EuiBadge, + EuiHeaderAlert, } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n/react'; -import { EuiHeaderAlert } from '../../../../legacy/core_plugins/newsfeed/public/np_ready/components/header_alert/header_alert'; import { NewsfeedContext } from './newsfeed_header_nav_button'; -import { NewsfeedItem } from '../../types'; +import { NewsfeedItem } from '../types'; import { NewsEmptyPrompt } from './empty_news'; import { NewsLoadingPrompt } from './loading_news'; diff --git a/src/plugins/newsfeed/public/components/newsfeed_header_nav_button.tsx b/src/plugins/newsfeed/public/components/newsfeed_header_nav_button.tsx index da042f0fce7b6..fd938e9071074 100644 --- a/src/plugins/newsfeed/public/components/newsfeed_header_nav_button.tsx +++ b/src/plugins/newsfeed/public/components/newsfeed_header_nav_button.tsx @@ -21,7 +21,7 @@ import React, { useState, Fragment, useEffect } from 'react'; import * as Rx from 'rxjs'; import { EuiHeaderSectionItemButton, EuiIcon, EuiNotificationBadge } from '@elastic/eui'; import { NewsfeedFlyout } from './flyout_list'; -import { FetchResult } from '../../types'; +import { FetchResult } from '../types'; export interface INewsfeedContext { setFlyoutVisible: React.Dispatch>; diff --git a/src/plugins/newsfeed/public/lib/api.test.ts b/src/plugins/newsfeed/public/lib/api.test.ts index 9d64dd26da047..5db578f1bd4e9 100644 --- a/src/plugins/newsfeed/public/lib/api.test.ts +++ b/src/plugins/newsfeed/public/lib/api.test.ts @@ -22,8 +22,11 @@ import { interval, race } from 'rxjs'; import sinon, { stub } from 'sinon'; import moment from 'moment'; import { HttpSetup } from 'src/core/public'; -import { NEWSFEED_HASH_SET_STORAGE_KEY, NEWSFEED_LAST_FETCH_STORAGE_KEY } from '../../constants'; -import { ApiItem, NewsfeedItem, NewsfeedPluginInjectedConfig } from '../../types'; +import { + NEWSFEED_HASH_SET_STORAGE_KEY, + NEWSFEED_LAST_FETCH_STORAGE_KEY, +} from '../../common/constants'; +import { ApiItem, NewsfeedItem, NewsfeedPluginBrowserConfig } from '../types'; import { NewsfeedApiDriver, getApi } from './api'; const localStorageGet = sinon.stub(); @@ -458,7 +461,7 @@ describe('getApi', () => { } return Promise.reject('wrong args!'); }; - let configMock: NewsfeedPluginInjectedConfig; + let configMock: NewsfeedPluginBrowserConfig; afterEach(() => { jest.resetAllMocks(); @@ -466,15 +469,12 @@ describe('getApi', () => { beforeEach(() => { configMock = { - newsfeed: { - service: { - urlRoot: 'http://fakenews.co', - pathTemplate: '/kibana-test/v{VERSION}.json', - }, - defaultLanguage: 'en', - mainInterval: 86400000, - fetchInterval: 86400000, + service: { + urlRoot: 'http://fakenews.co', + pathTemplate: '/kibana-test/v{VERSION}.json', }, + mainInterval: moment.duration(86400000), + fetchInterval: moment.duration(86400000), }; httpMock = ({ fetch: mockHttpGet, @@ -483,7 +483,7 @@ describe('getApi', () => { it('creates a result', done => { mockHttpGet.mockImplementationOnce(() => Promise.resolve({ items: [] })); - getApi(httpMock, configMock.newsfeed, '6.8.2').subscribe(result => { + getApi(httpMock, configMock, '6.8.2').subscribe(result => { expect(result).toMatchInlineSnapshot(` Object { "error": null, @@ -528,7 +528,7 @@ describe('getApi', () => { mockHttpGet.mockImplementationOnce(getHttpMockWithItems(mockApiItems)); - getApi(httpMock, configMock.newsfeed, '6.8.2').subscribe(result => { + getApi(httpMock, configMock, '6.8.2').subscribe(result => { expect(result).toMatchInlineSnapshot(` Object { "error": null, @@ -568,7 +568,7 @@ describe('getApi', () => { }, ]; mockHttpGet.mockImplementationOnce(getHttpMockWithItems(mockApiItems)); - getApi(httpMock, configMock.newsfeed, '6.8.2').subscribe(result => { + getApi(httpMock, configMock, '6.8.2').subscribe(result => { expect(result).toMatchInlineSnapshot(` Object { "error": null, @@ -595,7 +595,7 @@ describe('getApi', () => { it('forwards an error', done => { mockHttpGet.mockImplementationOnce((arg1, arg2) => Promise.reject('sorry, try again later!')); - getApi(httpMock, configMock.newsfeed, '6.8.2').subscribe(result => { + getApi(httpMock, configMock, '6.8.2').subscribe(result => { expect(result).toMatchInlineSnapshot(` Object { "error": "sorry, try again later!", @@ -623,14 +623,14 @@ describe('getApi', () => { ]; it("retries until fetch doesn't error", done => { - configMock.newsfeed.mainInterval = 10; // fast retry for testing + configMock.mainInterval = moment.duration(10); // fast retry for testing mockHttpGet .mockImplementationOnce(() => Promise.reject('Sorry, try again later!')) .mockImplementationOnce(() => Promise.reject('Sorry, internal server error!')) .mockImplementationOnce(() => Promise.reject("Sorry, it's too cold to go outside!")) .mockImplementationOnce(getHttpMockWithItems(successItems)); - getApi(httpMock, configMock.newsfeed, '6.8.2') + getApi(httpMock, configMock, '6.8.2') .pipe(take(4), toArray()) .subscribe(result => { expect(result).toMatchInlineSnapshot(` @@ -677,13 +677,13 @@ describe('getApi', () => { }); it("doesn't retry if fetch succeeds", done => { - configMock.newsfeed.mainInterval = 10; // fast retry for testing + configMock.mainInterval = moment.duration(10); // fast retry for testing mockHttpGet.mockImplementation(getHttpMockWithItems(successItems)); const timeout$ = interval(1000); // lets us capture some results after a short time let timesFetched = 0; - const get$ = getApi(httpMock, configMock.newsfeed, '6.8.2').pipe( + const get$ = getApi(httpMock, configMock, '6.8.2').pipe( tap(() => { timesFetched++; }) diff --git a/src/plugins/newsfeed/public/lib/api.ts b/src/plugins/newsfeed/public/lib/api.ts index bfeff4aa3e37b..2924f3d340662 100644 --- a/src/plugins/newsfeed/public/lib/api.ts +++ b/src/plugins/newsfeed/public/lib/api.ts @@ -26,10 +26,10 @@ import { NEWSFEED_FALLBACK_LANGUAGE, NEWSFEED_LAST_FETCH_STORAGE_KEY, NEWSFEED_HASH_SET_STORAGE_KEY, -} from '../../constants'; -import { NewsfeedPluginInjectedConfig, ApiItem, NewsfeedItem, FetchResult } from '../../types'; +} from '../../common/constants'; +import { ApiItem, NewsfeedItem, FetchResult, NewsfeedPluginBrowserConfig } from '../types'; -type ApiConfig = NewsfeedPluginInjectedConfig['newsfeed']['service']; +type ApiConfig = NewsfeedPluginBrowserConfig['service']; export class NewsfeedApiDriver { private readonly loadedTime = moment().utc(); // the date is compared to time in UTC format coming from the service @@ -167,14 +167,14 @@ export class NewsfeedApiDriver { */ export function getApi( http: HttpSetup, - config: NewsfeedPluginInjectedConfig['newsfeed'], + config: NewsfeedPluginBrowserConfig, kibanaVersion: string ): Rx.Observable { - const userLanguage = i18n.getLocale() || config.defaultLanguage; - const fetchInterval = config.fetchInterval; + const userLanguage = i18n.getLocale(); + const fetchInterval = config.fetchInterval.asMilliseconds(); const driver = new NewsfeedApiDriver(kibanaVersion, userLanguage, fetchInterval); - return Rx.timer(0, config.mainInterval).pipe( + return Rx.timer(0, config.mainInterval.asMilliseconds()).pipe( filter(() => driver.shouldFetch()), mergeMap(() => driver.fetchNewsfeedItems(http, config.service).pipe( diff --git a/src/plugins/newsfeed/public/plugin.tsx b/src/plugins/newsfeed/public/plugin.tsx index d21cf75a1a65e..e61070ab184f3 100644 --- a/src/plugins/newsfeed/public/plugin.tsx +++ b/src/plugins/newsfeed/public/plugin.tsx @@ -21,9 +21,10 @@ import * as Rx from 'rxjs'; import { catchError, takeUntil } from 'rxjs/operators'; import ReactDOM from 'react-dom'; import React from 'react'; +import moment from 'moment'; import { I18nProvider } from '@kbn/i18n/react'; import { PluginInitializerContext, CoreSetup, CoreStart, Plugin } from 'src/core/public'; -import { FetchResult, NewsfeedPluginInjectedConfig } from '../types'; +import { NewsfeedPluginBrowserConfig } from './types'; import { NewsfeedNavButton, NewsfeedApiFetchResult } from './components/newsfeed_header_nav_button'; import { getApi } from './lib/api'; @@ -32,10 +33,18 @@ export type Start = object; export class NewsfeedPublicPlugin implements Plugin { private readonly kibanaVersion: string; + private readonly config: NewsfeedPluginBrowserConfig; private readonly stop$ = new Rx.ReplaySubject(1); - constructor(initializerContext: PluginInitializerContext) { + constructor(initializerContext: PluginInitializerContext) { this.kibanaVersion = initializerContext.env.packageInfo.version; + const config = initializerContext.config.get(); + this.config = Object.freeze({ + ...config, + // We need wrap them in moment.duration because exposeToBrowser stringifies it. + mainInterval: moment.duration(config.mainInterval), + fetchInterval: moment.duration(config.fetchInterval), + }); } public setup(core: CoreSetup): Setup { @@ -57,16 +66,8 @@ export class NewsfeedPublicPlugin implements Plugin { } private fetchNewsfeed(core: CoreStart) { - const { http, injectedMetadata } = core; - const config = injectedMetadata.getInjectedVar('newsfeed') as - | NewsfeedPluginInjectedConfig['newsfeed'] - | undefined; - - if (!config) { - // running in new platform, injected metadata not available - return new Rx.Observable(); - } - return getApi(http, config, this.kibanaVersion).pipe( + const { http } = core; + return getApi(http, this.config, this.kibanaVersion).pipe( takeUntil(this.stop$), // stop the interval when stop method is called catchError(() => Rx.of(null)) // do not throw error ); diff --git a/src/plugins/newsfeed/types.ts b/src/plugins/newsfeed/public/types.ts similarity index 80% rename from src/plugins/newsfeed/types.ts rename to src/plugins/newsfeed/public/types.ts index 78485c6ee4f59..a9dab6791d7aa 100644 --- a/src/plugins/newsfeed/types.ts +++ b/src/plugins/newsfeed/public/types.ts @@ -17,18 +17,16 @@ * under the License. */ -import { Moment } from 'moment'; +import { Duration, Moment } from 'moment'; -export interface NewsfeedPluginInjectedConfig { - newsfeed: { - service: { - urlRoot: string; - pathTemplate: string; - }; - defaultLanguage: string; - mainInterval: number; // how often to check last updated time - fetchInterval: number; // how often to fetch remote service and set last updated +// Ideally, we may want to obtain the type from the configSchema and exposeToBrowser keys... +export interface NewsfeedPluginBrowserConfig { + service: { + urlRoot: string; + pathTemplate: string; }; + mainInterval: Duration; // how often to check last updated time + fetchInterval: Duration; // how often to fetch remote service and set last updated } export interface ApiItem { diff --git a/src/plugins/newsfeed/server/config.ts b/src/plugins/newsfeed/server/config.ts new file mode 100644 index 0000000000000..e42066f4d20ec --- /dev/null +++ b/src/plugins/newsfeed/server/config.ts @@ -0,0 +1,44 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { schema, TypeOf } from '@kbn/config-schema'; +import { + NEWSFEED_DEFAULT_SERVICE_PATH, + NEWSFEED_DEFAULT_SERVICE_BASE_URL, + NEWSFEED_DEV_SERVICE_BASE_URL, + NEWSFEED_FALLBACK_LANGUAGE, +} from '../common/constants'; + +export const configSchema = schema.object({ + enabled: schema.boolean({ defaultValue: true }), + service: schema.object({ + pathTemplate: schema.string({ defaultValue: NEWSFEED_DEFAULT_SERVICE_PATH }), + urlRoot: schema.conditional( + schema.contextRef('prod'), + schema.literal(true), // Point to staging if it's not a production release + schema.string({ defaultValue: NEWSFEED_DEFAULT_SERVICE_BASE_URL }), + schema.string({ defaultValue: NEWSFEED_DEV_SERVICE_BASE_URL }) + ), + }), + defaultLanguage: schema.string({ defaultValue: NEWSFEED_FALLBACK_LANGUAGE }), // TODO: Deprecate since no longer used + mainInterval: schema.duration({ defaultValue: '2m' }), // (2min) How often to retry failed fetches, and/or check if newsfeed items need to be refreshed from remote + fetchInterval: schema.duration({ defaultValue: '1d' }), // (1day) How often to fetch remote and reset the last fetched time +}); + +export type NewsfeedConfigType = TypeOf; diff --git a/src/plugins/newsfeed/server/index.ts b/src/plugins/newsfeed/server/index.ts new file mode 100644 index 0000000000000..e62bb95df8862 --- /dev/null +++ b/src/plugins/newsfeed/server/index.ts @@ -0,0 +1,36 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { PluginConfigDescriptor } from 'kibana/server'; +import { NewsfeedPlugin } from './plugin'; +import { configSchema, NewsfeedConfigType } from './config'; + +export const config: PluginConfigDescriptor = { + schema: configSchema, + exposeToBrowser: { + service: true, + mainInterval: true, + fetchInterval: true, + }, + deprecations: ({ unused }) => [unused('defaultLanguage')], +}; + +export function plugin() { + return new NewsfeedPlugin(); +} diff --git a/src/plugins/newsfeed/constants.ts b/src/plugins/newsfeed/server/plugin.ts similarity index 81% rename from src/plugins/newsfeed/constants.ts rename to src/plugins/newsfeed/server/plugin.ts index ddcbbb6cb1dbe..60b69b3977bf5 100644 --- a/src/plugins/newsfeed/constants.ts +++ b/src/plugins/newsfeed/server/plugin.ts @@ -17,6 +17,12 @@ * under the License. */ -export const NEWSFEED_FALLBACK_LANGUAGE = 'en'; -export const NEWSFEED_LAST_FETCH_STORAGE_KEY = 'newsfeed.lastfetchtime'; -export const NEWSFEED_HASH_SET_STORAGE_KEY = 'newsfeed.hashes'; +import { Plugin } from 'kibana/server'; + +export class NewsfeedPlugin implements Plugin { + public setup() {} + + public start() {} + + public stop() {} +} diff --git a/test/common/fixtures/plugins/newsfeed/kibana.json b/test/common/fixtures/plugins/newsfeed/kibana.json new file mode 100644 index 0000000000000..110b53fc6b2e9 --- /dev/null +++ b/test/common/fixtures/plugins/newsfeed/kibana.json @@ -0,0 +1,6 @@ +{ + "id": "newsfeed-fixtures", + "version": "kibana", + "server": true, + "ui": false +} diff --git a/test/common/fixtures/plugins/newsfeed/newsfeed_simulation.ts b/test/common/fixtures/plugins/newsfeed/newsfeed_simulation.ts deleted file mode 100644 index 5aa44b48f9d59..0000000000000 --- a/test/common/fixtures/plugins/newsfeed/newsfeed_simulation.ts +++ /dev/null @@ -1,126 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import Hapi from 'hapi'; - -interface WebhookRequest extends Hapi.Request { - payload: string; -} - -export async function initPlugin(server: Hapi.Server, path: string) { - server.route({ - method: ['GET'], - path: `${path}/kibana/v{version}.json`, - options: { - cors: { - origin: ['*'], - additionalHeaders: [ - 'Sec-Fetch-Mode', - 'Access-Control-Request-Method', - 'Access-Control-Request-Headers', - 'cache-control', - 'x-requested-with', - 'Origin', - 'User-Agent', - 'DNT', - 'content-type', - 'kbn-version', - ], - }, - }, - handler: newsfeedHandler as Hapi.Lifecycle.Method, - }); - - server.route({ - method: ['GET'], - path: `${path}/kibana/crash.json`, - options: { - cors: { - origin: ['*'], - additionalHeaders: [ - 'Sec-Fetch-Mode', - 'Access-Control-Request-Method', - 'Access-Control-Request-Headers', - 'cache-control', - 'x-requested-with', - 'Origin', - 'User-Agent', - 'DNT', - 'content-type', - 'kbn-version', - ], - }, - }, - handler() { - throw new Error('Internal server error'); - }, - }); -} - -function newsfeedHandler(request: WebhookRequest, h: any) { - return htmlResponse(h, 200, JSON.stringify(mockNewsfeed(request.params.version))); -} - -const mockNewsfeed = (version: string) => ({ - items: [ - { - title: { en: `You are functionally testing the newsfeed widget with fixtures!` }, - description: { en: 'See test/common/fixtures/plugins/newsfeed/newsfeed_simulation' }, - link_text: { en: 'Generic feed-viewer could go here' }, - link_url: { en: 'https://feeds.elastic.co' }, - languages: null, - badge: null, - image_url: null, - publish_on: '2019-06-21T00:00:00', - expire_on: '2040-01-31T00:00:00', - hash: '39ca7d409c7eb25f4c69a5a6a11309b2f5ced7ca3f9b3a0109517126e0fd91ca', - }, - { - title: { en: 'Staging too!' }, - description: { en: 'Hello world' }, - link_text: { en: 'Generic feed-viewer could go here' }, - link_url: { en: 'https://feeds-staging.elastic.co' }, - languages: null, - badge: null, - image_url: null, - publish_on: '2019-06-21T00:00:00', - expire_on: '2040-01-31T00:00:00', - hash: 'db445c9443eb50ea2eb15f20edf89cf0f7dac2b058b11cafc2c8c288b6e4ce2a', - }, - { - title: { en: 'This item is expired!' }, - description: { en: 'This should not show up.' }, - link_text: { en: 'Generic feed-viewer could go here' }, - link_url: { en: 'https://feeds-staging.elastic.co' }, - languages: null, - badge: null, - image_url: null, - publish_on: '2019-06-21T00:00:00', - expire_on: '2019-12-31T00:00:00', - hash: 'db445c9443eb50ea2eb15f20edf89cf0f7dac2b058b11cafc2c8c288b6e4ce2a', - }, - ], -}); - -function htmlResponse(h: any, code: number, text: string) { - return h - .response(text) - .type('application/json') - .code(code); -} diff --git a/test/common/fixtures/plugins/newsfeed/package.json b/test/common/fixtures/plugins/newsfeed/package.json deleted file mode 100644 index 5291b1031b0a9..0000000000000 --- a/test/common/fixtures/plugins/newsfeed/package.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "name": "newsfeed-fixtures", - "version": "0.0.0", - "kibana": { - "version": "kibana" - } -} diff --git a/src/legacy/core_plugins/newsfeed/constants.ts b/test/common/fixtures/plugins/newsfeed/server/index.ts similarity index 76% rename from src/legacy/core_plugins/newsfeed/constants.ts rename to test/common/fixtures/plugins/newsfeed/server/index.ts index 55a0c51c2ac65..979aadfe86606 100644 --- a/src/legacy/core_plugins/newsfeed/constants.ts +++ b/test/common/fixtures/plugins/newsfeed/server/index.ts @@ -17,7 +17,9 @@ * under the License. */ -export const PLUGIN_ID = 'newsfeed'; -export const DEFAULT_SERVICE_URLROOT = 'https://feeds.elastic.co'; -export const DEV_SERVICE_URLROOT = 'https://feeds-staging.elastic.co'; -export const DEFAULT_SERVICE_PATH = '/kibana/v{VERSION}.json'; +import { PluginInitializerContext } from 'kibana/public'; +import { NewsFeedSimulatorPlugin } from './plugin'; + +export function plugin(initializerContext: PluginInitializerContext) { + return new NewsFeedSimulatorPlugin(initializerContext); +} diff --git a/test/common/fixtures/plugins/newsfeed/server/plugin.ts b/test/common/fixtures/plugins/newsfeed/server/plugin.ts new file mode 100644 index 0000000000000..60044d4e0b2df --- /dev/null +++ b/test/common/fixtures/plugins/newsfeed/server/plugin.ts @@ -0,0 +1,97 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { CoreSetup, Plugin } from 'kibana/server'; +import { PluginInitializerContext } from 'kibana/public'; + +export class NewsFeedSimulatorPlugin implements Plugin { + constructor(private readonly initializerContext: PluginInitializerContext) {} + + public setup({ http }: CoreSetup) { + const router = http.createRouter(); + const version = this.initializerContext.env.packageInfo.version; + + router.get( + { + path: `/api/_newsfeed-FTS-external-service-simulators/kibana/v${version}.json`, + validate: false, + options: { authRequired: false }, + }, + (context, req, res) => { + return res.ok({ body: this.mockNewsfeed() }); + } + ); + + router.get( + { + path: '/api/_newsfeed-FTS-external-service-simulators/kibana/crash.json', + validate: false, + options: { authRequired: false }, + }, + (context, req, res) => { + return res.internalError({ body: new Error('Internal server error') }); + } + ); + } + + public start() {} + + private mockNewsfeed() { + return { + items: [ + { + title: { en: `You are functionally testing the newsfeed widget with fixtures!` }, + description: { en: 'See test/common/fixtures/plugins/newsfeed/newsfeed_simulation' }, + link_text: { en: 'Generic feed-viewer could go here' }, + link_url: { en: 'https://feeds.elastic.co' }, + languages: null, + badge: null, + image_url: null, + publish_on: '2019-06-21T00:00:00', + expire_on: '2040-01-31T00:00:00', + hash: '39ca7d409c7eb25f4c69a5a6a11309b2f5ced7ca3f9b3a0109517126e0fd91ca', + }, + { + title: { en: 'Staging too!' }, + description: { en: 'Hello world' }, + link_text: { en: 'Generic feed-viewer could go here' }, + link_url: { en: 'https://feeds-staging.elastic.co' }, + languages: null, + badge: null, + image_url: null, + publish_on: '2019-06-21T00:00:00', + expire_on: '2040-01-31T00:00:00', + hash: 'db445c9443eb50ea2eb15f20edf89cf0f7dac2b058b11cafc2c8c288b6e4ce2a', + }, + { + title: { en: 'This item is expired!' }, + description: { en: 'This should not show up.' }, + link_text: { en: 'Generic feed-viewer could go here' }, + link_url: { en: 'https://feeds-staging.elastic.co' }, + languages: null, + badge: null, + image_url: null, + publish_on: '2019-06-21T00:00:00', + expire_on: '2019-12-31T00:00:00', + hash: 'db445c9443eb50ea2eb15f20edf89cf0f7dac2b058b11cafc2c8c288b6e4ce2a', + }, + ], + }; + } +}