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.

Sign up for GitHub

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

Clean up TSVB turn Joi schema into kbn schema #65482

Merged
merged 5 commits into from
May 8, 2020
Merged

Clean up TSVB turn Joi schema into kbn schema #65482

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5d0a59e
Clean up TSVB turn Joi schema into kbn schema
DianaDerevyankina May 6, 2020
8eb5bc9
Return validationTelemerty and logging part back
DianaDerevyankina May 6, 2020
d230c17
Merge branch 'master' into 57342
DianaDerevyankina May 7, 2020
507a21b
Add schema.maybe() where it was missed
DianaDerevyankina May 7, 2020
90ebf9e
Merge branch 'DianaDerevyankina' into 57342
DianaDerevyankina May 8, 2020
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
14 changes: 2 additions & 12 deletions src/plugins/vis_type_timeseries/server/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import { Observable } from 'rxjs';
import { Server } from 'hapi';
import { VisTypeTimeseriesConfig } from './config';
import { getVisData, GetVisData, GetVisDataOptions } from './lib/get_vis_data';
import { ValidationTelemetryService } from './validation_telemetry';
import { UsageCollectionSetup } from '../../usage_collection/server';
import { visDataRoutes } from './routes/vis';
// @ts-ignore
Expand Down Expand Up @@ -66,11 +65,8 @@ export interface Framework {
}

export class VisTypeTimeseriesPlugin implements Plugin<VisTypeTimeseriesSetup> {
private validationTelementryService: ValidationTelemetryService;

constructor(private readonly initializerContext: PluginInitializerContext) {
this.initializerContext = initializerContext;
this.validationTelementryService = new ValidationTelemetryService();
}

public setup(core: CoreSetup, plugins: VisTypeTimeseriesPluginSetupDependencies) {
Expand All @@ -92,15 +88,9 @@ export class VisTypeTimeseriesPlugin implements Plugin<VisTypeTimeseriesSetup> {
searchStrategyRegistry,
};

(async () => {
const validationTelemetry = await this.validationTelementryService.setup(core, {
...plugins,
globalConfig$,
});
visDataRoutes(router, framework, validationTelemetry);
visDataRoutes(router, framework);

fieldsRoutes(framework);
})();
fieldsRoutes(framework);

return {
getVisData: async (
Expand Down
208 changes: 102 additions & 106 deletions src/plugins/vis_type_timeseries/server/routes/post_vis_schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,59 +17,65 @@
* under the License.
*/

import Joi from 'joi';
const stringOptionalNullable = Joi.string()
.allow('', null)
.optional();
const stringRequired = Joi.string()
.allow('')
.required();
const arrayNullable = Joi.array().allow(null);
const numberIntegerOptional = Joi.number()
.integer()
.optional();
const numberIntegerRequired = Joi.number()
.integer()
.required();
const numberOptional = Joi.number().optional();
const queryObject = Joi.object({
language: Joi.string().allow(''),
query: Joi.string().allow(''),
import { schema } from '@kbn/config-schema';
import { TypeOptions } from '@kbn/config-schema/target/types/types';

const stringOptionalNullable = schema.nullable(schema.string());
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this should be schema.maybe(schema.nullable(schema.string()));

The current schema just gives string | null.


const stringRequired = schema.string();

const arrayNullable = schema.arrayOf(schema.nullable(schema.any()));

const validateInteger: TypeOptions<number>['validate'] = value => {
if (!Number.isInteger(value)) {
return `${value} is not an integer`;
}
};
const numberIntegerOptional = schema.maybe(schema.number({ validate: validateInteger }));
const numberIntegerRequired = schema.number({ validate: validateInteger });

const numberOptional = schema.maybe(schema.number());

const queryObject = schema.object({
language: schema.string(),
query: schema.string(),
});
const stringOrNumberOptionalNullable = Joi.alternatives([stringOptionalNullable, numberOptional]);
const numberOptionalOrEmptyString = Joi.alternatives(numberOptional, Joi.string().valid(''));
const stringOrNumberOptionalNullable = schema.nullable(
schema.oneOf([stringOptionalNullable, numberOptional])
);
const numberOptionalOrEmptyString = schema.maybe(
schema.oneOf([numberOptional, schema.literal('')])
);

const annotationsItems = Joi.object({
const annotationsItems = schema.object({
color: stringOptionalNullable,
fields: stringOptionalNullable,
hidden: Joi.boolean().optional(),
hidden: schema.maybe(schema.boolean()),
icon: stringOptionalNullable,
id: stringOptionalNullable,
ignore_global_filters: numberIntegerOptional,
ignore_panel_filters: numberIntegerOptional,
index_pattern: stringOptionalNullable,
query_string: queryObject.optional(),
query_string: schema.maybe(queryObject),
template: stringOptionalNullable,
time_field: stringOptionalNullable,
});

const backgroundColorRulesItems = Joi.object({
value: Joi.number()
.allow(null)
.optional(),
const backgroundColorRulesItems = schema.object({
value: schema.nullable(schema.number()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this needs a schema.maybe wrapper as well - in the Joi schema this was optional.

id: stringOptionalNullable,
background_color: stringOptionalNullable,
color: stringOptionalNullable,
});

const gaugeColorRulesItems = Joi.object({
const gaugeColorRulesItems = schema.object({
gauge: stringOptionalNullable,
text: stringOptionalNullable,
id: stringOptionalNullable,
operator: stringOptionalNullable,
value: Joi.number(),
value: schema.number(),
});
const metricsItems = Joi.object({
const metricsItems = schema.object({
field: stringOptionalNullable,
id: stringRequired,
metric_agg: stringOptionalNullable,
Expand All @@ -84,82 +90,83 @@ const metricsItems = Joi.object({
beta: numberOptional,
gamma: numberOptional,
period: numberOptional,
multiplicative: Joi.boolean(),
multiplicative: schema.maybe(schema.boolean()),
window: numberOptional,
function: stringOptionalNullable,
script: stringOptionalNullable,
variables: Joi.array()
.items(
Joi.object({
variables: schema.maybe(
schema.arrayOf(
schema.object({
field: stringOptionalNullable,
id: stringRequired,
name: stringOptionalNullable,
})
)
.optional(),
percentiles: Joi.array()
.items(
Joi.object({
),
percentiles: schema.maybe(
schema.arrayOf(
schema.object({
id: stringRequired,
field: stringOptionalNullable,
mode: Joi.string().allow('line', 'band'),
shade: Joi.alternatives(numberOptional, stringOptionalNullable),
value: Joi.alternatives(numberOptional, stringOptionalNullable),
mode: schema.oneOf([schema.literal('line'), schema.literal('band')]),
shade: schema.oneOf([numberOptional, stringOptionalNullable]),
value: schema.oneOf([numberOptional, stringOptionalNullable]),
percentile: stringOptionalNullable,
})
)
.optional(),
),
type: stringRequired,
value: stringOptionalNullable,
values: Joi.array()
.items(Joi.string().allow('', null))
.allow(null)
.optional(),
values: schema.nullable(schema.arrayOf(schema.nullable(schema.string()))),
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

});

const splitFiltersItems = Joi.object({
const splitFiltersItems = schema.object({
id: stringOptionalNullable,
color: stringOptionalNullable,
filter: Joi.object({
language: Joi.string().allow(''),
query: Joi.string().allow(''),
}).optional(),
filter: schema.maybe(
schema.object({
language: schema.string(),
query: schema.string(),
})
),
label: stringOptionalNullable,
});

const seriesItems = Joi.object({
const seriesItems = schema.object({
aggregate_by: stringOptionalNullable,
aggregate_function: stringOptionalNullable,
axis_position: stringRequired,
axis_max: stringOrNumberOptionalNullable,
axis_min: stringOrNumberOptionalNullable,
chart_type: stringRequired,
color: stringRequired,
color_rules: Joi.array()
.items(
Joi.object({
color_rules: schema.maybe(
schema.arrayOf(
schema.object({
value: numberOptional,
id: stringRequired,
text: stringOptionalNullable,
operator: stringOptionalNullable,
})
)
.optional(),
),
fill: numberOptionalOrEmptyString,
filter: Joi.alternatives(
Joi.object({
query: stringRequired,
language: stringOptionalNullable,
}).optional(),
Joi.string().valid('')
filter: schema.maybe(
schema.oneOf([
schema.object({
query: stringRequired,
language: stringOptionalNullable,
}),
schema.literal(''),
])
),
formatter: stringRequired,
hide_in_legend: numberIntegerOptional,
hidden: Joi.boolean().optional(),
hidden: schema.maybe(schema.boolean()),
id: stringRequired,
label: stringOptionalNullable,
line_width: numberOptionalOrEmptyString,
metrics: Joi.array().items(metricsItems),
metrics: schema.arrayOf(metricsItems),
offset_time: stringOptionalNullable,
override_index_pattern: numberOptional,
point_size: numberOptionalOrEmptyString,
Expand All @@ -170,9 +177,7 @@ const seriesItems = Joi.object({
series_interval: stringOptionalNullable,
series_drop_last_bucket: numberIntegerOptional,
split_color_mode: stringOptionalNullable,
split_filters: Joi.array()
.items(splitFiltersItems)
.optional(),
split_filters: schema.maybe(schema.arrayOf(splitFiltersItems)),
split_mode: stringRequired,
stacked: stringRequired,
steps: numberIntegerOptional,
Expand All @@ -189,38 +194,34 @@ const seriesItems = Joi.object({
var_name: stringOptionalNullable,
});

export const visPayloadSchema = Joi.object({
export const visPayloadSchema = schema.object({
filters: arrayNullable,
panels: Joi.array().items(
Joi.object({
annotations: Joi.array()
.items(annotationsItems)
.optional(),
panels: schema.arrayOf(
schema.object({
annotations: schema.maybe(schema.arrayOf(annotationsItems)),
axis_formatter: stringRequired,
axis_position: stringRequired,
axis_scale: stringRequired,
axis_min: stringOrNumberOptionalNullable,
axis_max: stringOrNumberOptionalNullable,
bar_color_rules: arrayNullable.optional(),
bar_color_rules: schema.maybe(arrayNullable),
background_color: stringOptionalNullable,
background_color_rules: Joi.array()
.items(backgroundColorRulesItems)
.optional(),
background_color_rules: schema.maybe(schema.arrayOf(backgroundColorRulesItems)),
default_index_pattern: stringOptionalNullable,
default_timefield: stringOptionalNullable,
drilldown_url: stringOptionalNullable,
drop_last_bucket: numberIntegerOptional,
filter: Joi.alternatives(
stringOptionalNullable,
Joi.object({
language: stringOptionalNullable,
query: stringOptionalNullable,
})
filter: schema.nullable(
schema.oneOf([
stringOptionalNullable,
schema.object({
language: stringOptionalNullable,
query: stringOptionalNullable,
}),
])
),
gauge_color_rules: Joi.array()
.items(gaugeColorRulesItems)
.optional(),
gauge_width: [stringOptionalNullable, numberOptional],
gauge_color_rules: schema.maybe(schema.arrayOf(gaugeColorRulesItems)),
gauge_width: schema.nullable(schema.oneOf([stringOptionalNullable, numberOptional])),
gauge_inner_color: stringOptionalNullable,
gauge_inner_width: stringOrNumberOptionalNullable,
gauge_style: stringOptionalNullable,
Expand All @@ -230,7 +231,7 @@ export const visPayloadSchema = Joi.object({
ignore_global_filter: numberOptional,
index_pattern: stringRequired,
interval: stringRequired,
isModelInvalid: Joi.boolean().optional(),
isModelInvalid: schema.maybe(schema.boolean()),
legend_position: stringOptionalNullable,
markdown: stringOptionalNullable,
markdown_scrollbars: numberIntegerOptional,
Expand All @@ -242,9 +243,7 @@ export const visPayloadSchema = Joi.object({
pivot_label: stringOptionalNullable,
pivot_type: stringOptionalNullable,
pivot_rows: stringOptionalNullable,
series: Joi.array()
.items(seriesItems)
.required(),
series: schema.arrayOf(seriesItems),
show_grid: numberIntegerRequired,
show_legend: numberIntegerRequired,
time_field: stringOptionalNullable,
Expand All @@ -253,22 +252,19 @@ export const visPayloadSchema = Joi.object({
})
),
// general
query: Joi.array()
.items(queryObject)
.allow(null)
.required(),
state: Joi.object({
sort: Joi.object({
column: stringRequired,
order: Joi.string()
.valid(['asc', 'desc'])
.required(),
}).optional(),
}).required(),
savedObjectId: Joi.string().optional(),
timerange: Joi.object({
query: schema.maybe(schema.arrayOf(schema.nullable(queryObject))),
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the query is required, so the maybe should be removed here. Also when I'm reading the Joi schema correctly, query is allowed to be null, not the items within the array. That's not the same thing, Joi would allow query: null but fail on query: [null] while the new version would do the exact opposite.

So it should be schema.nullable(schema.arrayOf(queryObject))

state: schema.object({
sort: schema.maybe(
schema.object({
column: stringRequired,
order: schema.oneOf([schema.literal('asc'), schema.literal('desc')]),
})
),
}),
savedObjectId: schema.maybe(schema.string()),
timerange: schema.object({
timezone: stringRequired,
min: stringRequired,
max: stringRequired,
}).required(),
}),
});
Loading