Merge branch '7.x' into backport/7.x/pr-66164

.ci/Jenkinsfile_visual_baseline

@@ -4,20 +4,22 @@ library 'kibana-pipeline-library'
 kibanaLibrary.load()
 
 kibanaPipeline(timeoutMinutes: 120) {
-  catchError {
-    parallel([
-      'oss-visualRegression': {
-        workers.ci(name: 'oss-visualRegression', size: 's', ramDisk: false) {
-          kibanaPipeline.functionalTestProcess('oss-visualRegression', './test/scripts/jenkins_visual_regression.sh')(1)
-        }
-      },
-      'xpack-visualRegression': {
-        workers.ci(name: 'xpack-visualRegression', size: 's', ramDisk: false) {
-          kibanaPipeline.functionalTestProcess('xpack-visualRegression', './test/scripts/jenkins_xpack_visual_regression.sh')(1)
-        }
-      },
-    ])
-  }
+  ciStats.trackBuild {
+    catchError {
+      parallel([
+        'oss-visualRegression': {
+          workers.ci(name: 'oss-visualRegression', size: 's', ramDisk: false) {
+            kibanaPipeline.functionalTestProcess('oss-visualRegression', './test/scripts/jenkins_visual_regression.sh')(1)
+          }
+        },
+        'xpack-visualRegression': {
+          workers.ci(name: 'xpack-visualRegression', size: 's', ramDisk: false) {
+            kibanaPipeline.functionalTestProcess('xpack-visualRegression', './test/scripts/jenkins_xpack_visual_regression.sh')(1)
+          }
+        },
+      ])
+    }
 
-  kibanaPipeline.sendMail()
+    kibanaPipeline.sendMail()
+  }
 }

examples/alerting_example/public/alert_types/always_firing.tsx

@@ -51,6 +51,7 @@ export function getAlertType(): AlertTypeModel {
       }
       return validationResult;
     },
+    requiresAppContext: false,
   };
 }
 

examples/alerting_example/public/alert_types/astros.tsx

@@ -99,6 +99,7 @@ export function getAlertType(): AlertTypeModel {
 
       return validationResult;
     },
+    requiresAppContext: false,
   };
 }
 

examples/alerting_example/server/alert_types/always_firing.ts

@@ -20,7 +20,7 @@
 import uuid from 'uuid';
 import { range } from 'lodash';
 import { AlertType } from '../../../../x-pack/plugins/alerting/server';
-import { DEFAULT_INSTANCES_TO_GENERATE } from '../../common/constants';
+import { DEFAULT_INSTANCES_TO_GENERATE, ALERTING_EXAMPLE_APP_ID } from '../../common/constants';
 
 export const alertType: AlertType = {
   id: 'example.always-firing',
@@ -43,4 +43,5 @@ export const alertType: AlertType = {
       count,
     };
   },
+  producer: ALERTING_EXAMPLE_APP_ID,
 };

examples/alerting_example/server/alert_types/astros.ts

@@ -19,7 +19,7 @@
 
 import axios from 'axios';
 import { AlertType } from '../../../../x-pack/plugins/alerting/server';
-import { Operator, Craft } from '../../common/constants';
+import { Operator, Craft, ALERTING_EXAMPLE_APP_ID } from '../../common/constants';
 
 interface PeopleInSpace {
   people: Array<{
@@ -79,4 +79,5 @@ export const alertType: AlertType = {
       peopleInSpace,
     };
   },
+  producer: ALERTING_EXAMPLE_APP_ID,
 };

packages/kbn-dev-utils/src/proc_runner/proc_runner.ts

@@ -18,17 +18,22 @@
  */
 
 import moment from 'moment';
-import { filter, first, catchError } from 'rxjs/operators';
+import * as Rx from 'rxjs';
+import { filter, first, catchError, map } from 'rxjs/operators';
 import exitHook from 'exit-hook';
 
 import { ToolingLog } from '../tooling_log';
 import { createCliError } from './errors';
 import { Proc, ProcOptions, startProc } from './proc';
 
+const SECOND = 1000;
+const MINUTE = 60 * SECOND;
+
 const noop = () => {};
 
 interface RunOptions extends ProcOptions {
   wait: true | RegExp;
+  waitTimeout?: number | false;
 }
 
 /**
@@ -71,6 +76,7 @@ export class ProcRunner {
       cwd = process.cwd(),
       stdin = undefined,
       wait = false,
+      waitTimeout = 15 * MINUTE,
       env = process.env,
     } = options;
 
@@ -97,8 +103,8 @@ export class ProcRunner {
     try {
       if (wait instanceof RegExp) {
         // wait for process to log matching line
-        await proc.lines$
-          .pipe(
+        await Rx.race(
+          proc.lines$.pipe(
             filter(line => wait.test(line)),
             first(),
             catchError(err => {
@@ -108,8 +114,18 @@ export class ProcRunner {
                 throw err;
               }
             })
-          )
-          .toPromise();
+          ),
+          waitTimeout === false
+            ? Rx.NEVER
+            : Rx.timer(waitTimeout).pipe(
+                map(() => {
+                  const sec = waitTimeout / SECOND;
+                  throw createCliError(
+                    `[${name}] failed to match pattern within ${sec} seconds [pattern=${wait}]`
+                  );
+                })
+              )
+        ).toPromise();
       }
 
       if (wait === true) {

packages/kbn-spec-to-console/lib/convert.js

@@ -37,13 +37,16 @@ module.exports = spec => {
   Object.keys(spec).forEach(api => {
     const source = spec[api];
 
-    if (source.url.paths.every(path => Boolean(path.deprecated))) {
-      return;
-    }
-
     if (!source.url) {
       return result;
     }
+
+    if (source.url.path) {
+      if (source.url.paths.every(path => Boolean(path.deprecated))) {
+        return;
+      }
+    }
+
     const convertedSpec = (result[api] = {});
     if (source.params) {
       const urlParams = convertParams(source.params);

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.

src/core/public/application/application_service.tsx

@@ -198,6 +198,7 @@ export class ApplicationService {
           appBasePath: basePath.prepend(app.appRoute!),
           mount: wrapMount(plugin, app),
           unmountBeforeMounting: false,
+          legacy: false,
         });
       },
       registerLegacyApp: app => {
@@ -232,6 +233,7 @@ export class ApplicationService {
           appBasePath,
           mount,
           unmountBeforeMounting: true,
+          legacy: true,
         });
       },
       registerAppUpdater: (appUpdater$: Observable<AppUpdater>) =>

src/core/public/application/integration_tests/router.test.tsx

@@ -27,7 +27,7 @@ import { createRenderer, createAppMounter, createLegacyAppMounter, getUnmounter
 import { AppStatus } from '../types';
 import { ScopedHistory } from '../scoped_history';
 
-describe('AppContainer', () => {
+describe('AppRouter', () => {
   let mounters: MockedMounterMap<EitherApp>;
   let globalHistory: History;
   let appStatuses$: BehaviorSubject<Map<string, AppStatus>>;
@@ -78,6 +78,16 @@ describe('AppContainer', () => {
           history.push('/subpath');
         },
       }),
+      createAppMounter({
+        appId: 'app5',
+        html: '<div>App 5</div>',
+        appRoute: '/app/my-app/app5',
+      }),
+      createAppMounter({
+        appId: 'app6',
+        html: '<div>App 6</div>',
+        appRoute: '/app/my-app/app6',
+      }),
     ] as Array<MockedMounterTuple<EitherApp>>);
     globalHistory = createMemoryHistory();
     appStatuses$ = mountersToAppStatus$();
@@ -282,6 +292,16 @@ describe('AppContainer', () => {
     expect(unmount).not.toHaveBeenCalled();
   });
 
+  it('allows multiple apps with the same `/app/appXXX` appRoute prefix', async () => {
+    await navigate('/app/my-app/app5/path');
+    expect(mounters.get('app5')!.mounter.mount).toHaveBeenCalledTimes(1);
+    expect(mounters.get('app6')!.mounter.mount).toHaveBeenCalledTimes(0);
+
+    await navigate('/app/my-app/app6/another-path');
+    expect(mounters.get('app5')!.mounter.mount).toHaveBeenCalledTimes(1);
+    expect(mounters.get('app6')!.mounter.mount).toHaveBeenCalledTimes(1);
+  });
+
   it('should not remount when when changing pages within app using hash history', async () => {
     globalHistory = createHashHistory();
     update = createRenderer(

src/core/public/application/integration_tests/utils.tsx

@@ -61,6 +61,7 @@ export const createAppMounter = ({
       mounter: {
         appRoute,
         appBasePath: appRoute,
+        legacy: false,
         mount: jest.fn(async (params: AppMountParameters) => {
           const { appBasePath: basename, element } = params;
           Object.assign(element, {
@@ -88,6 +89,7 @@ export const createLegacyAppMounter = (
       appRoute: `/app/${appId.split(':')[0]}`,
       appBasePath: `/app/${appId.split(':')[0]}`,
       unmountBeforeMounting: true,
+      legacy: true,
       mount: legacyMount,
     },
     unmount: jest.fn(),

src/core/public/application/types.ts

@@ -549,6 +549,7 @@ export type Mounter<T = App | LegacyApp> = SelectivePartial<
     appRoute: string;
     appBasePath: string;
     mount: T extends LegacyApp ? LegacyAppMounter : AppMounter;
+    legacy: boolean;
     unmountBeforeMounting: T extends LegacyApp ? true : boolean;
   },
   T extends LegacyApp ? never : 'unmountBeforeMounting'

src/core/public/application/ui/app_container.test.tsx

@@ -54,6 +54,7 @@ describe('AppContainer', () => {
     appBasePath: '/base-path',
     appRoute: '/some-route',
     unmountBeforeMounting: false,
+    legacy: false,
     mount: async ({ element }: AppMountParameters) => {
       await promise;
       const container = document.createElement('div');
@@ -138,9 +139,10 @@ describe('AppContainer', () => {
   it('should call setIsMounting(false) if mounting throws', async () => {
     const [waitPromise, resolvePromise] = createResolver();
     const mounter = {
-      appBasePath: '/base-path',
+      appBasePath: '/base-path/some-route',
       appRoute: '/some-route',
       unmountBeforeMounting: false,
+      legacy: false,
       mount: async ({ element }: AppMountParameters) => {
         await waitPromise;
         throw new Error(`Mounting failed!`);

src/core/public/application/ui/app_router.tsx

@@ -55,26 +55,25 @@ export const AppRouter: FunctionComponent<Props> = ({
   return (
     <Router history={history}>
       <Switch>
-        {[...mounters].flatMap(([appId, mounter]) =>
-          // Remove /app paths from the routes as they will be handled by the
-          // "named" route parameter `:appId` below
-          mounter.appBasePath.startsWith('/app')
-            ? []
-            : [
-                <Route
-                  key={mounter.appRoute}
-                  path={mounter.appRoute}
-                  render={({ match: { url } }) => (
-                    <AppContainer
-                      appPath={url}
-                      appStatus={appStatuses.get(appId) ?? AppStatus.inaccessible}
-                      createScopedHistory={createScopedHistory}
-                      {...{ appId, mounter, setAppLeaveHandler, setIsMounting }}
-                    />
-                  )}
-                />,
-              ]
-        )}
+        {[...mounters]
+          // legacy apps can have multiple sub-apps registered with the same route
+          // which needs additional logic that is handled in the catch-all route below
+          .filter(([_, mounter]) => !mounter.legacy)
+          .map(([appId, mounter]) => (
+            <Route
+              key={mounter.appRoute}
+              path={mounter.appRoute}
+              render={({ match: { url } }) => (
+                <AppContainer
+                  appPath={url}
+                  appStatus={appStatuses.get(appId) ?? AppStatus.inaccessible}
+                  createScopedHistory={createScopedHistory}
+                  {...{ appId, mounter, setAppLeaveHandler, setIsMounting }}
+                />
+              )}
+            />
+          ))}
+        {/* handler for legacy apps and used as a catch-all to display 404 page on not existing /app/appId apps*/}
         <Route
           path="/app/:appId"
           render={({