Skip to content

Commit

Permalink
[Canvas] Remove dependency on legacy expressions APIs (#74885)
Browse files Browse the repository at this point in the history
* Remove legacy types and function registration

* Pull server interpreter functions routes into Canvas and update them to use new expressions API

* Clean up comment

* Removing boom and doing more cleanup

* Add functions test and refactor other router tests

* Adding a type and refactoring a forgotten test

* more tests

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
poffdeluxe and elasticmachine authored Aug 18, 2020
1 parent f02fad4 commit e6e7354
Show file tree
Hide file tree
Showing 30 changed files with 306 additions and 199 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/canvas/common/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,4 @@ export const API_ROUTE_SHAREABLE_RUNTIME = '/public/canvas/runtime';
export const API_ROUTE_SHAREABLE_RUNTIME_DOWNLOAD = `/public/canvas/${SHAREABLE_RUNTIME_NAME}.js`;
export const CANVAS_EMBEDDABLE_CLASSNAME = `canvasEmbeddable`;
export const CONTEXT_MENU_TOP_BORDER_CLASSNAME = 'canvasContextMenu--topBorder';
export const API_ROUTE_FUNCTIONS = `${API_ROUTE}/fns`;
2 changes: 1 addition & 1 deletion x-pack/plugins/canvas/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"configPath": ["xpack", "canvas"],
"server": true,
"ui": true,
"requiredPlugins": ["data", "embeddable", "expressions", "features", "home", "inspector", "uiActions"],
"requiredPlugins": ["bfetch", "data", "embeddable", "expressions", "features", "home", "inspector", "uiActions"],
"optionalPlugins": ["usageCollection"],
"requiredBundles": ["kibanaReact", "maps", "lens", "visualizations", "kibanaUtils", "kibanaLegacy", "discover", "savedObjects", "reporting"]
}
2 changes: 1 addition & 1 deletion x-pack/plugins/canvas/public/application.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export const initializeCanvas = async (
const canvasFunctions = initFunctions({
timefilter: setupPlugins.data.query.timefilter.timefilter,
prependBasePath: coreSetup.http.basePath.prepend,
typesRegistry: setupPlugins.expressions.__LEGACY.types,
types: setupPlugins.expressions.getTypes(),
});

for (const fn of canvasFunctions) {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/canvas/public/functions/filters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export function filtersFunctionFactory(initialize: InitializeArguments): () => F
const filterAST = fromExpression(filterExpression);
return interpretAst(filterAST, getWorkpadVariablesAsObject(getState()));
} else {
const filterType = initialize.typesRegistry.get('filter');
const filterType = initialize.types.filter;
return filterType?.from(null, {});
}
},
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/canvas/public/functions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { CanvasSetupDeps, CoreSetup } from '../plugin';

export interface InitializeArguments {
prependBasePath: CoreSetup['http']['basePath']['prepend'];
typesRegistry: CanvasSetupDeps['expressions']['__LEGACY']['types'];
types: ReturnType<CanvasSetupDeps['expressions']['getTypes']>;
timefilter: CanvasSetupDeps['data']['query']['timefilter']['timefilter'];
}

Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/canvas/public/functions/to.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function toFunctionFactory(initialize: InitializeArguments): () => ToFunc
throw errors.missingType();
}

return castProvider(initialize.typesRegistry.toJS())(input, args.type);
return castProvider(initialize.types)(input, args.type);
},
};
};
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/canvas/public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { UiActionsStart } from '../../../../src/plugins/ui_actions/public';
import { EmbeddableStart } from '../../../../src/plugins/embeddable/public';
import { UsageCollectionSetup } from '../../../../src/plugins/usage_collection/public';
import { Start as InspectorStart } from '../../../../src/plugins/inspector/public';
import { BfetchPublicSetup } from '../../../../src/plugins/bfetch/public';
// @ts-expect-error untyped local
import { argTypeSpecs } from './expression_types/arg_types';
import { transitions } from './transitions';
Expand All @@ -41,6 +42,7 @@ export interface CanvasSetupDeps {
expressions: ExpressionsSetup;
home: HomePublicPluginSetup;
usageCollection?: UsageCollectionSetup;
bfetch: BfetchPublicSetup;
}

export interface CanvasStartDeps {
Expand Down
39 changes: 37 additions & 2 deletions x-pack/plugins/canvas/public/services/expressions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,49 @@
*/

import { CanvasServiceFactory } from '.';
import { ExpressionsService } from '../../../../../src/plugins/expressions/common';
import {
ExpressionsService,
serializeProvider,
} from '../../../../../src/plugins/expressions/common';
import { API_ROUTE_FUNCTIONS } from '../../common/lib/constants';

export const expressionsServiceFactory: CanvasServiceFactory<ExpressionsService> = async (
coreSetup,
coreStart,
setupPlugins,
startPlugins
) => {
await setupPlugins.expressions.__LEGACY.loadLegacyServerFunctionWrappers();
const { expressions, bfetch } = setupPlugins;

let cached: Promise<void> | null = null;
const loadServerFunctionWrappers = async () => {
if (!cached) {
cached = (async () => {
const serverFunctionList = await coreSetup.http.get(API_ROUTE_FUNCTIONS);
const batchedFunction = bfetch.batchedFunction({ url: API_ROUTE_FUNCTIONS });
const { serialize } = serializeProvider(expressions.getTypes());

// For every sever-side function, register a client-side
// function that matches its definition, but which simply
// calls the server-side function endpoint.
Object.keys(serverFunctionList).forEach((functionName) => {
if (expressions.getFunction(functionName)) {
return;
}
const fn = () => ({
...serverFunctionList[functionName],
fn: (input: any, args: any) => {
return batchedFunction({ functionName, args, context: serialize(input) });
},
});
expressions.registerFunction(fn);
});
})();
}
return cached;
};

await loadServerFunctionWrappers();

return setupPlugins.expressions.fork();
};
3 changes: 2 additions & 1 deletion x-pack/plugins/canvas/public/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
import { getInitialState } from './state/initial_state';

import { CoreSetup } from '../../../../src/core/public';
import { API_ROUTE_FUNCTIONS } from '../common/lib/constants';
import { CanvasSetupDeps } from './plugin';

export async function createStore(core: CoreSetup, plugins: CanvasSetupDeps) {
Expand All @@ -31,7 +32,7 @@ export async function createFreshStore(core: CoreSetup, plugins: CanvasSetupDeps
const basePath = core.http.basePath.get();

// Retrieve server functions
const serverFunctionsResponse = await core.http.get(`/api/interpreter/fns`);
const serverFunctionsResponse = await core.http.get(API_ROUTE_FUNCTIONS);
const serverFunctions = Object.values(serverFunctionsResponse);

initialState.app = {
Expand Down
10 changes: 9 additions & 1 deletion x-pack/plugins/canvas/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { first } from 'rxjs/operators';
import { CoreSetup, PluginInitializerContext, Plugin, Logger, CoreStart } from 'src/core/server';
import { ExpressionsServerSetup } from 'src/plugins/expressions/server';
import { BfetchServerSetup } from 'src/plugins/bfetch/server';
import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
import { HomeServerPluginSetup } from 'src/plugins/home/server';
import { PluginSetupContract as FeaturesPluginSetup } from '../../features/server';
Expand All @@ -21,6 +22,7 @@ interface PluginsSetup {
expressions: ExpressionsServerSetup;
features: FeaturesPluginSetup;
home: HomeServerPluginSetup;
bfetch: BfetchServerSetup;
usageCollection?: UsageCollectionSetup;
}

Expand Down Expand Up @@ -67,7 +69,13 @@ export class CanvasPlugin implements Plugin {

const canvasRouter = coreSetup.http.createRouter();

initRoutes({ router: canvasRouter, logger: this.logger });
initRoutes({
router: canvasRouter,
expressions: plugins.expressions,
bfetch: plugins.bfetch,
elasticsearch: coreSetup.elasticsearch,
logger: this.logger,
});

loadSampleData(
plugins.home.sampleData.addSavedObjectsToSampleDataset,
Expand Down
19 changes: 5 additions & 14 deletions x-pack/plugins/canvas/server/routes/custom_elements/create.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,11 @@
*/

import sinon from 'sinon';
import {
savedObjectsClientMock,
httpServiceMock,
httpServerMock,
loggingSystemMock,
} from 'src/core/server/mocks';
import { savedObjectsClientMock, httpServerMock } from 'src/core/server/mocks';
import { CUSTOM_ELEMENT_TYPE } from '../../../common/lib/constants';
import { initializeCreateCustomElementRoute } from './create';
import { kibanaResponseFactory, RequestHandlerContext, RequestHandler } from 'src/core/server';
import { getMockedRouterDeps } from '../test_helpers';

const mockRouteContext = ({
core: {
Expand All @@ -36,15 +32,10 @@ describe('POST custom element', () => {
beforeEach(() => {
clock = sinon.useFakeTimers(now);

const httpService = httpServiceMock.createSetupContract();
const routerDeps = getMockedRouterDeps();
initializeCreateCustomElementRoute(routerDeps);

const router = httpService.createRouter();
initializeCreateCustomElementRoute({
router,
logger: loggingSystemMock.create().get(),
});

routeHandler = router.post.mock.calls[0][1];
routeHandler = routerDeps.router.post.mock.calls[0][1];
});

afterEach(() => {
Expand Down
18 changes: 5 additions & 13 deletions x-pack/plugins/canvas/server/routes/custom_elements/delete.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,8 @@
import { CUSTOM_ELEMENT_TYPE } from '../../../common/lib/constants';
import { initializeDeleteCustomElementRoute } from './delete';
import { kibanaResponseFactory, RequestHandlerContext, RequestHandler } from 'src/core/server';
import {
savedObjectsClientMock,
httpServiceMock,
httpServerMock,
loggingSystemMock,
} from 'src/core/server/mocks';
import { savedObjectsClientMock, httpServerMock } from 'src/core/server/mocks';
import { getMockedRouterDeps } from '../test_helpers';

const mockRouteContext = ({
core: {
Expand All @@ -26,14 +22,10 @@ describe('DELETE custom element', () => {
let routeHandler: RequestHandler<any, any, any>;

beforeEach(() => {
const httpService = httpServiceMock.createSetupContract();
const router = httpService.createRouter();
initializeDeleteCustomElementRoute({
router,
logger: loggingSystemMock.create().get(),
});
const routerDeps = getMockedRouterDeps();
initializeDeleteCustomElementRoute(routerDeps);

routeHandler = router.delete.mock.calls[0][1];
routeHandler = routerDeps.router.delete.mock.calls[0][1];
});

it(`returns 200 ok when the custom element is deleted`, async () => {
Expand Down
18 changes: 5 additions & 13 deletions x-pack/plugins/canvas/server/routes/custom_elements/find.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,8 @@

import { initializeFindCustomElementsRoute } from './find';
import { kibanaResponseFactory, RequestHandlerContext, RequestHandler } from 'src/core/server';
import {
savedObjectsClientMock,
httpServiceMock,
httpServerMock,
loggingSystemMock,
} from 'src/core/server/mocks';
import { savedObjectsClientMock, httpServerMock } from 'src/core/server/mocks';
import { getMockedRouterDeps } from '../test_helpers';

const mockRouteContext = ({
core: {
Expand All @@ -25,14 +21,10 @@ describe('Find custom element', () => {
let routeHandler: RequestHandler<any, any, any>;

beforeEach(() => {
const httpService = httpServiceMock.createSetupContract();
const router = httpService.createRouter();
initializeFindCustomElementsRoute({
router,
logger: loggingSystemMock.create().get(),
});
const routerDeps = getMockedRouterDeps();
initializeFindCustomElementsRoute(routerDeps);

routeHandler = router.get.mock.calls[0][1];
routeHandler = routerDeps.router.get.mock.calls[0][1];
});

it(`returns 200 with the found custom elements`, async () => {
Expand Down
18 changes: 5 additions & 13 deletions x-pack/plugins/canvas/server/routes/custom_elements/get.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,8 @@
import { CUSTOM_ELEMENT_TYPE } from '../../../common/lib/constants';
import { initializeGetCustomElementRoute } from './get';
import { kibanaResponseFactory, RequestHandlerContext, RequestHandler } from 'src/core/server';
import {
savedObjectsClientMock,
httpServiceMock,
httpServerMock,
loggingSystemMock,
} from 'src/core/server/mocks';
import { savedObjectsClientMock, httpServerMock } from 'src/core/server/mocks';
import { getMockedRouterDeps } from '../test_helpers';

const mockRouteContext = ({
core: {
Expand All @@ -26,14 +22,10 @@ describe('GET custom element', () => {
let routeHandler: RequestHandler<any, any, any>;

beforeEach(() => {
const httpService = httpServiceMock.createSetupContract();
const router = httpService.createRouter();
initializeGetCustomElementRoute({
router,
logger: loggingSystemMock.create().get(),
});
const routerDeps = getMockedRouterDeps();
initializeGetCustomElementRoute(routerDeps);

routeHandler = router.get.mock.calls[0][1];
routeHandler = routerDeps.router.get.mock.calls[0][1];
});

it(`returns 200 when the custom element is found`, async () => {
Expand Down
18 changes: 5 additions & 13 deletions x-pack/plugins/canvas/server/routes/custom_elements/update.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,9 @@ import { CustomElement } from '../../../types';
import { CUSTOM_ELEMENT_TYPE } from '../../../common/lib/constants';
import { initializeUpdateCustomElementRoute } from './update';
import { kibanaResponseFactory, RequestHandlerContext, RequestHandler } from 'src/core/server';
import {
savedObjectsClientMock,
httpServiceMock,
httpServerMock,
loggingSystemMock,
} from 'src/core/server/mocks';
import { savedObjectsClientMock, httpServerMock } from 'src/core/server/mocks';
import { okResponse } from '../ok_response';
import { getMockedRouterDeps } from '../test_helpers';

const mockRouteContext = ({
core: {
Expand Down Expand Up @@ -51,14 +47,10 @@ describe('PUT custom element', () => {
beforeEach(() => {
clock = sinon.useFakeTimers(now);

const httpService = httpServiceMock.createSetupContract();
const router = httpService.createRouter();
initializeUpdateCustomElementRoute({
router,
logger: loggingSystemMock.create().get(),
});
const routerDeps = getMockedRouterDeps();
initializeUpdateCustomElementRoute(routerDeps);

routeHandler = router.put.mock.calls[0][1];
routeHandler = routerDeps.router.put.mock.calls[0][1];
});

afterEach(() => {
Expand Down
18 changes: 5 additions & 13 deletions x-pack/plugins/canvas/server/routes/es_fields/es_fields.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,8 @@

import { initializeESFieldsRoute } from './es_fields';
import { kibanaResponseFactory, RequestHandlerContext, RequestHandler } from 'src/core/server';
import {
httpServiceMock,
httpServerMock,
loggingSystemMock,
elasticsearchServiceMock,
} from 'src/core/server/mocks';
import { httpServerMock, elasticsearchServiceMock } from 'src/core/server/mocks';
import { getMockedRouterDeps } from '../test_helpers';

const mockRouteContext = ({
core: {
Expand All @@ -27,14 +23,10 @@ describe('Retrieve ES Fields', () => {
let routeHandler: RequestHandler<any, any, any>;

beforeEach(() => {
const httpService = httpServiceMock.createSetupContract();
const router = httpService.createRouter();
initializeESFieldsRoute({
router,
logger: loggingSystemMock.create().get(),
});
const routerDeps = getMockedRouterDeps();
initializeESFieldsRoute(routerDeps);

routeHandler = router.get.mock.calls[0][1];
routeHandler = routerDeps.router.get.mock.calls[0][1];
});

it(`returns 200 with fields from existing index/index pattern`, async () => {
Expand Down
Loading

0 comments on commit e6e7354

Please sign in to comment.