Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expressions/migrations2 #81281

Merged
merged 7 commits into from
Oct 30, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export declare class Executor<Context extends Record<string, unknown> = Record<s
| [getType(name)](./kibana-plugin-plugins-expressions-public.executor.gettype.md) | | |
| [getTypes()](./kibana-plugin-plugins-expressions-public.executor.gettypes.md) | | |
| [inject(ast, references)](./kibana-plugin-plugins-expressions-public.executor.inject.md) | | |
| [migrate(ast, version)](./kibana-plugin-plugins-expressions-public.executor.migrate.md) | | |
| [migrateToLatest(ast, version)](./kibana-plugin-plugins-expressions-public.executor.migratetolatest.md) | | |
| [registerFunction(functionDefinition)](./kibana-plugin-plugins-expressions-public.executor.registerfunction.md) | | |
| [registerType(typeDefinition)](./kibana-plugin-plugins-expressions-public.executor.registertype.md) | | |
| [run(ast, input, context, options)](./kibana-plugin-plugins-expressions-public.executor.run.md) | | Execute expression and return result. |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-expressions-public](./kibana-plugin-plugins-expressions-public.md) &gt; [Executor](./kibana-plugin-plugins-expressions-public.executor.md) &gt; [migrate](./kibana-plugin-plugins-expressions-public.executor.migrate.md)

## Executor.migrate() method

<b>Signature:</b>

```typescript
migrate(ast: SerializableState, version: string): ExpressionAstExpression;
```

## Parameters

| Parameter | Type | Description |
| --- | --- | --- |
| ast | <code>SerializableState</code> | |
| version | <code>string</code> | |

<b>Returns:</b>

`ExpressionAstExpression`

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-expressions-public](./kibana-plugin-plugins-expressions-public.md) &gt; [Executor](./kibana-plugin-plugins-expressions-public.executor.md) &gt; [migrateToLatest](./kibana-plugin-plugins-expressions-public.executor.migratetolatest.md)

## Executor.migrateToLatest() method

<b>Signature:</b>

```typescript
migrateToLatest(ast: unknown, version: string): ExpressionAstExpression;
```

## Parameters

| Parameter | Type | Description |
| --- | --- | --- |
| ast | <code>unknown</code> | |
| version | <code>string</code> | |

<b>Returns:</b>

`ExpressionAstExpression`

Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export declare class ExpressionFunction implements PersistableState<ExpressionAs
| [help](./kibana-plugin-plugins-expressions-public.expressionfunction.help.md) | | <code>string</code> | A short help text. |
| [inject](./kibana-plugin-plugins-expressions-public.expressionfunction.inject.md) | | <code>(state: ExpressionAstFunction['arguments'], references: SavedObjectReference[]) =&gt; ExpressionAstFunction['arguments']</code> | |
| [inputTypes](./kibana-plugin-plugins-expressions-public.expressionfunction.inputtypes.md) | | <code>string[] &#124; undefined</code> | Type of inputs that this function supports. |
| [migrations](./kibana-plugin-plugins-expressions-public.expressionfunction.migrations.md) | | <code>{</code><br/><code> [key: string]: (state: SerializableState) =&gt; SerializableState;</code><br/><code> }</code> | |
| [name](./kibana-plugin-plugins-expressions-public.expressionfunction.name.md) | | <code>string</code> | Name of function |
| [telemetry](./kibana-plugin-plugins-expressions-public.expressionfunction.telemetry.md) | | <code>(state: ExpressionAstFunction['arguments'], telemetryData: Record&lt;string, any&gt;) =&gt; Record&lt;string, any&gt;</code> | |
| [type](./kibana-plugin-plugins-expressions-public.expressionfunction.type.md) | | <code>string</code> | Return type of function. This SHOULD be supplied. We use it for UI and autocomplete hinting. We may also use it for optimizations in the future. |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-expressions-public](./kibana-plugin-plugins-expressions-public.md) &gt; [ExpressionFunction](./kibana-plugin-plugins-expressions-public.expressionfunction.md) &gt; [migrations](./kibana-plugin-plugins-expressions-public.expressionfunction.migrations.md)

## ExpressionFunction.migrations property

<b>Signature:</b>

```typescript
migrations: {
[key: string]: (state: SerializableState) => SerializableState;
};
```
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export declare class ExpressionsService implements PersistableState<ExpressionAs
| [getType](./kibana-plugin-plugins-expressions-public.expressionsservice.gettype.md) | | <code>ExpressionsServiceStart['getType']</code> | |
| [getTypes](./kibana-plugin-plugins-expressions-public.expressionsservice.gettypes.md) | | <code>() =&gt; ReturnType&lt;Executor['getTypes']&gt;</code> | Returns POJO map of all registered expression types, where keys are names of the types and values are <code>ExpressionType</code> instances. |
| [inject](./kibana-plugin-plugins-expressions-public.expressionsservice.inject.md) | | <code>(state: ExpressionAstExpression, references: SavedObjectReference[]) =&gt; ExpressionAstExpression</code> | Injects saved object references into expression AST |
| [migrate](./kibana-plugin-plugins-expressions-public.expressionsservice.migrate.md) | | <code>(state: SerializableState, version: string) =&gt; ExpressionAstExpression</code> | Injects saved object references into expression AST |
| [migrateToLatest](./kibana-plugin-plugins-expressions-public.expressionsservice.migratetolatest.md) | | <code>(state: unknown, version: string) =&gt; ExpressionAstExpression</code> | Injects saved object references into expression AST |
| [registerFunction](./kibana-plugin-plugins-expressions-public.expressionsservice.registerfunction.md) | | <code>(functionDefinition: AnyExpressionFunctionDefinition &#124; (() =&gt; AnyExpressionFunctionDefinition)) =&gt; void</code> | Register an expression function, which will be possible to execute as part of the expression pipeline.<!-- -->Below we register a function which simply sleeps for given number of milliseconds to delay the execution and outputs its input as-is.
```ts
expressions.registerFunction({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-expressions-public](./kibana-plugin-plugins-expressions-public.md) &gt; [ExpressionsService](./kibana-plugin-plugins-expressions-public.expressionsservice.md) &gt; [migrate](./kibana-plugin-plugins-expressions-public.expressionsservice.migrate.md)

## ExpressionsService.migrate property

Injects saved object references into expression AST

<b>Signature:</b>

```typescript
readonly migrate: (state: SerializableState, version: string) => ExpressionAstExpression;
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-expressions-public](./kibana-plugin-plugins-expressions-public.md) &gt; [ExpressionsService](./kibana-plugin-plugins-expressions-public.expressionsservice.md) &gt; [migrateToLatest](./kibana-plugin-plugins-expressions-public.expressionsservice.migratetolatest.md)

## ExpressionsService.migrateToLatest property

Injects saved object references into expression AST

<b>Signature:</b>

```typescript
readonly migrateToLatest: (state: unknown, version: string) => ExpressionAstExpression;
```
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export declare class Executor<Context extends Record<string, unknown> = Record<s
| [getType(name)](./kibana-plugin-plugins-expressions-server.executor.gettype.md) | | |
| [getTypes()](./kibana-plugin-plugins-expressions-server.executor.gettypes.md) | | |
| [inject(ast, references)](./kibana-plugin-plugins-expressions-server.executor.inject.md) | | |
| [migrate(ast, version)](./kibana-plugin-plugins-expressions-server.executor.migrate.md) | | |
| [migrateToLatest(ast, version)](./kibana-plugin-plugins-expressions-server.executor.migratetolatest.md) | | |
| [registerFunction(functionDefinition)](./kibana-plugin-plugins-expressions-server.executor.registerfunction.md) | | |
| [registerType(typeDefinition)](./kibana-plugin-plugins-expressions-server.executor.registertype.md) | | |
| [run(ast, input, context, options)](./kibana-plugin-plugins-expressions-server.executor.run.md) | | Execute expression and return result. |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-expressions-server](./kibana-plugin-plugins-expressions-server.md) &gt; [Executor](./kibana-plugin-plugins-expressions-server.executor.md) &gt; [migrate](./kibana-plugin-plugins-expressions-server.executor.migrate.md)

## Executor.migrate() method

<b>Signature:</b>

```typescript
migrate(ast: SerializableState, version: string): ExpressionAstExpression;
```

## Parameters

| Parameter | Type | Description |
| --- | --- | --- |
| ast | <code>SerializableState</code> | |
| version | <code>string</code> | |

<b>Returns:</b>

`ExpressionAstExpression`

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-expressions-server](./kibana-plugin-plugins-expressions-server.md) &gt; [Executor](./kibana-plugin-plugins-expressions-server.executor.md) &gt; [migrateToLatest](./kibana-plugin-plugins-expressions-server.executor.migratetolatest.md)

## Executor.migrateToLatest() method

<b>Signature:</b>

```typescript
migrateToLatest(ast: unknown, version: string): ExpressionAstExpression;
```

## Parameters

| Parameter | Type | Description |
| --- | --- | --- |
| ast | <code>unknown</code> | |
| version | <code>string</code> | |

<b>Returns:</b>

`ExpressionAstExpression`

Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export declare class ExpressionFunction implements PersistableState<ExpressionAs
| [help](./kibana-plugin-plugins-expressions-server.expressionfunction.help.md) | | <code>string</code> | A short help text. |
| [inject](./kibana-plugin-plugins-expressions-server.expressionfunction.inject.md) | | <code>(state: ExpressionAstFunction['arguments'], references: SavedObjectReference[]) =&gt; ExpressionAstFunction['arguments']</code> | |
| [inputTypes](./kibana-plugin-plugins-expressions-server.expressionfunction.inputtypes.md) | | <code>string[] &#124; undefined</code> | Type of inputs that this function supports. |
| [migrations](./kibana-plugin-plugins-expressions-server.expressionfunction.migrations.md) | | <code>{</code><br/><code> [key: string]: (state: SerializableState) =&gt; SerializableState;</code><br/><code> }</code> | |
| [name](./kibana-plugin-plugins-expressions-server.expressionfunction.name.md) | | <code>string</code> | Name of function |
| [telemetry](./kibana-plugin-plugins-expressions-server.expressionfunction.telemetry.md) | | <code>(state: ExpressionAstFunction['arguments'], telemetryData: Record&lt;string, any&gt;) =&gt; Record&lt;string, any&gt;</code> | |
| [type](./kibana-plugin-plugins-expressions-server.expressionfunction.type.md) | | <code>string</code> | Return type of function. This SHOULD be supplied. We use it for UI and autocomplete hinting. We may also use it for optimizations in the future. |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-expressions-server](./kibana-plugin-plugins-expressions-server.md) &gt; [ExpressionFunction](./kibana-plugin-plugins-expressions-server.expressionfunction.md) &gt; [migrations](./kibana-plugin-plugins-expressions-server.expressionfunction.migrations.md)

## ExpressionFunction.migrations property

<b>Signature:</b>

```typescript
migrations: {
[key: string]: (state: SerializableState) => SerializableState;
};
```
17 changes: 16 additions & 1 deletion src/plugins/expressions/common/executor/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { typeSpecs } from '../expression_types/specs';
import { functionSpecs } from '../expression_functions/specs';
import { getByAlias } from '../util';
import { SavedObjectReference } from '../../../../core/types';
import { PersistableState } from '../../../kibana_utils/common';
import { PersistableState, SerializableState } from '../../../kibana_utils/common';

export interface ExpressionExecOptions {
/**
Expand Down Expand Up @@ -257,6 +257,21 @@ export class Executor<Context extends Record<string, unknown> = Record<string, u
return telemetryData;
}

migrate(ast: SerializableState, version: string) {
return this.walkAst(cloneDeep(ast) as ExpressionAstExpression, (fn, link) => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we know the input is ExpressionAstExpression as we don't allow migrating that structure but just the actual functions within

link = fn.migrations[version](link) as ExpressionAstFunction;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and we expect the migration to return ExpressionAstFunction. thats not the general case with migrations, we might be working with unknown states where the only thing we know is that its serializable. When we do have more knowledge we'll need to do the casting.

ps: open for suggestions on how to better type safe this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the only other option that comes to mind is instead casting fn.migrations[version] as MigrateFunction<SerializableState, ExpressionAstFunction>, but that doesn't really make a difference here in terms of type safety, so I think we can go with the existing implementation for now

});
}

migrateToLatest(ast: unknown, version: string) {
return this.walkAst(cloneDeep(ast) as ExpressionAstExpression, (fn, link) => {
for (const key in Object.keys(fn.migrations)) {
if (key < version) continue;
link = fn.migrations[key](link) as ExpressionAstFunction;
}
});
}

public fork(): Executor<Context> {
const initialState = this.state.get();
const fork = new Executor<Context>(initialState);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { ExpressionValue } from '../expression_types/types';
import { ExecutionContext } from '../execution';
import { ExpressionAstFunction } from '../ast';
import { SavedObjectReference } from '../../../../core/types';
import { PersistableState } from '../../../kibana_utils/common';
import { PersistableState, SerializableState } from '../../../kibana_utils/common';

export class ExpressionFunction implements PersistableState<ExpressionAstFunction['arguments']> {
/**
Expand Down Expand Up @@ -76,6 +76,9 @@ export class ExpressionFunction implements PersistableState<ExpressionAstFunctio
state: ExpressionAstFunction['arguments'],
references: SavedObjectReference[]
) => ExpressionAstFunction['arguments'];
migrations: {
[key: string]: (state: SerializableState) => SerializableState;
};

constructor(functionDefinition: AnyExpressionFunctionDefinition) {
const {
Expand All @@ -91,6 +94,7 @@ export class ExpressionFunction implements PersistableState<ExpressionAstFunctio
telemetry,
inject,
extract,
migrations,
} = functionDefinition;

this.name = name;
Expand All @@ -104,6 +108,7 @@ export class ExpressionFunction implements PersistableState<ExpressionAstFunctio
this.telemetry = telemetry || ((s, c) => c);
this.inject = inject || identity;
this.extract = extract || ((s) => ({ state: s, references: [] }));
this.migrations = migrations || {};

for (const [key, arg] of Object.entries(args || {})) {
this.args[key] = new ExpressionFunctionParameter(key, arg);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import { i18n } from '@kbn/i18n';
import { ExpressionFunctionDefinition } from '../types';
import { ExpressionValueSearchContext } from '../../expression_types';
import { MigrateFunction } from '../../../../kibana_utils/common/persistable_state';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all changes to this file will be reverted, its just an example


const toArray = <T>(query: undefined | T | T[]): T[] =>
!query ? [] : Array.isArray(query) ? query : [query];
Expand All @@ -32,6 +33,23 @@ export type ExpressionFunctionKibana = ExpressionFunctionDefinition<
ExpressionValueSearchContext
>;

type MYSTATE_7_10_0 = {
name: 'kibana';
args: {};
};

type MYSTATE_7_10_1 = {
name: 'kibana_input';
args: {};
};

const MIGRATE7_10_1: MigrateFunction<MYSTATE_7_10_0, MYSTATE_7_10_1> = (state) => {
return {
name: 'kibana_input',
args: state.args,
};
};

export const kibana: ExpressionFunctionKibana = {
name: 'kibana',
type: 'kibana_context',
Expand All @@ -44,6 +62,10 @@ export const kibana: ExpressionFunctionKibana = {

args: {},

migrations: {
'7.10.1': (MIGRATE7_10_1 as any) as MigrateFunction,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the part i really don't like. any suggestions ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doesn't TS like this? Looks as if it should work without casting to me, but need to test it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe part of the issue is that in expression_function.ts you are re-defining the migration type instead of reusing MigrateFunction? (I assume that's what's being used behind the scenes here?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem is that MigrateFunction there is <SerializableSTate,SerializableState> where MIGRATE7_10_1 actually work with well defined types

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem is that MigrateFunction there is <SerializableSTate,SerializableState> where MIGRATE7_10_1 actually work with well defined types

Okay, after looking into this more, there are actually a couple problems. First, migrations isn't in ExpressionFunctionDefinition at all, which is masked by the casting.

More problematic is that ExpressionFunctionDefinition would need to provide a generic type param so you can specify the actual type(s) of your migrations, that way TS doesn't complain because the functions you provide are too specific.

The bummer is that this makes things more verbose and makes ExpressionFunctionDefinition even more complex, but usage would essentially look like:

interface Migrations extends Record<string, any> {
  '7.10.1': MigrateFunction<MYSTATE_7_10_0, MYSTATE_7_10_1>;
}

export type ExpressionFunctionKibana = ExpressionFunctionDefinition<
  'kibana',
  ExpressionValueSearchContext | null,
  object,
  ExpressionValueSearchContext,
  ExecutionContext,
  Migrations
>;

And then ExpressionFunctionDefinition would need to take a generic param along the lines of:

  Migrations extends Record<string, MigrateFunction> = Record<string, MigrateFunction>

I tested this and it worked for me locally, but LMK what you think. Either way if we are adding migrations to the functions, we should update ExpressionFunctionDefinition

},

fn(input, _, { search = {} }) {
const output: ExpressionValueSearchContext = {
// TODO: This spread is left here for legacy reasons, possibly Lens uses it.
Expand Down
22 changes: 21 additions & 1 deletion src/plugins/expressions/common/service/expressions_services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { ExecutionContract } from '../execution/execution_contract';
import { AnyExpressionTypeDefinition } from '../expression_types';
import { AnyExpressionFunctionDefinition } from '../expression_functions';
import { SavedObjectReference } from '../../../../core/types';
import { PersistableState } from '../../../kibana_utils/common';
import { PersistableState, SerializableState } from '../../../kibana_utils/common';

/**
* The public contract that `ExpressionsService` provides to other plugins
Expand Down Expand Up @@ -290,6 +290,26 @@ export class ExpressionsService implements PersistableState<ExpressionAstExpress
return this.executor.inject(state, references);
};

/**
* Injects saved object references into expression AST
* @param state expression AST to update
* @param references array of saved object references
* @returns new expression AST with references injected
*/
public readonly migrate = (state: SerializableState, version: string) => {
return this.executor.migrate(state, version);
};

/**
* Injects saved object references into expression AST
* @param state expression AST to update
* @param references array of saved object references
* @returns new expression AST with references injected
*/
public readonly migrateToLatest = (state: unknown, version: string) => {
return this.executor.migrateToLatest(state, version);
};

/**
* Returns Kibana Platform *setup* life-cycle contract. Useful to return the
* same contract on server-side and browser-side.
Expand Down
Loading