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

allow negative signs in duration #284

Merged
merged 1 commit into from
Feb 19, 2024
+32 −21
Merged

allow negative signs in duration #284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 26 additions & 14 deletions server/utils/__tests__/validationHelper.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@

import { ReportDefinitionSchemaType, ReportSchemaType } from '../../model';
import {
DELIVERY_TYPE,
FORMAT,
REPORT_TYPE,
TRIGGER_TYPE,
} from '../../routes/utils/constants';
import { isValidRelativeUrl, validateReport, validateReportDefinition } from '../validationHelper';
import {
isValidRelativeUrl,
regexDuration,
validateReport,
validateReportDefinition,
} from '../validationHelper';

const SAMPLE_SAVED_OBJECT_ID = '3ba638e0-b894-11e8-a6d9-e546fe2bba5f';
const createReportDefinitionInput: ReportDefinitionSchemaType = {
Expand All @@ -31,7 +35,7 @@
configIds: [],
title: 'title',
textDescription: 'text description',
htmlDescription: 'html description'
htmlDescription: 'html description',
},
trigger: {
trigger_type: TRIGGER_TYPE.onDemand,
Expand Down Expand Up @@ -63,12 +67,12 @@
configIds: [],
title: 'title',
textDescription: 'text description',
htmlDescription: 'html description'
htmlDescription: 'html description',
},
trigger: {
trigger_type: TRIGGER_TYPE.onDemand,
},
}
};

const createReportDefinitionNotebookInput: ReportDefinitionSchemaType = {
report_params: {
Expand All @@ -88,12 +92,12 @@
configIds: [],
title: 'title',
textDescription: 'text description',
htmlDescription: 'html description'
htmlDescription: 'html description',
},
trigger: {
trigger_type: TRIGGER_TYPE.onDemand,
},
}
};

const createReportDefinitionNotebookPostNavBarInput: ReportDefinitionSchemaType = {
report_params: {
Expand All @@ -113,12 +117,12 @@
configIds: [],
title: 'title',
textDescription: 'text description',
htmlDescription: 'html description'
htmlDescription: 'html description',
},
trigger: {
trigger_type: TRIGGER_TYPE.onDemand,
},
}
};

describe('test input validation', () => {
test('create report with correct saved object id', async () => {
Expand Down Expand Up @@ -189,7 +193,7 @@
});

test('validation against query_url', async () => {
const urls: [string, boolean][] = [
const urls: Array<[string, boolean]> = [
['/app/dashboards#/view/7adfa750-4c81-11e8-b3d7-01146121b73d?_g=', true],
[
'/_plugin/kibana/app/dashboards#/view/7adfa750-4c81-11e8-b3d7-01146121b73d?_g=',
Expand Down Expand Up @@ -223,15 +227,23 @@
'/app/data-explorer/discover?security_tenant=private#/view/571aaf70-4c88-11e8-b3d7-01146121b73d',
true,
],
[
'/app/discoverLegacy#/view/571aaf70-4c88-11e8-b3d7-01146121b73d',
true,
],
['/app/discoverLegacy#/view/571aaf70-4c88-11e8-b3d7-01146121b73d', true],
];
expect(urls.map((url) => isValidRelativeUrl(url[0]))).toEqual(
urls.map((url) => url[1])
);
});

test('validate ISO 8601 durations', () => {
const durations: Array<[string, boolean]> = [
['PT30M', true],
['PT-30M', true],
['PT-2H-30M', true],
];
expect(
durations.map((duration) => regexDuration.test(duration[0]))
).toEqual(durations.map((duration) => duration[1]));
});
});

// TODO: merge this with other mock clients used in testing, to create some mock helpers file
Expand All @@ -239,12 +251,12 @@
const client = {
callAsCurrentUser: jest
.fn()
.mockImplementation((endpoint: string, params: any) => {

Check warning on line 254 in server/utils/__tests__/validationHelper.test.ts

View workflow job for this annotation

GitHub Actions / Lint 

Unexpected any. Specify a different type
switch (endpoint) {
case 'exists':
return mockSavedObjectIds.includes(params.id);
default:
fail('Fail due to unexpected function call on client');

Check failure on line 259 in server/utils/__tests__/validationHelper.test.ts

View workflow job for this annotation

GitHub Actions / Lint 

Illegal usage of `fail`, prefer throwing an error, or the `done.fail` callback
}
}),
};
Expand Down
13 changes: 6 additions & 7 deletions server/utils/validationHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { RequestParams } from '@elastic/elasticsearch';
import { RequestParams } from '@opensearch-project/opensearch';
import path from 'path';
import { ILegacyScopedClusterClient } from '../../../../src/core/server';
import {
Expand All @@ -15,7 +15,7 @@ import {
import { REPORT_TYPE } from '../../server/routes/utils/constants';

export const isValidRelativeUrl = (relativeUrl: string) => {
let normalizedRelativeUrl = relativeUrl
let normalizedRelativeUrl = relativeUrl;
if (
!relativeUrl.includes('observability#/notebooks') &&
!relativeUrl.includes('notebooks-dashboards')
Expand All @@ -34,15 +34,15 @@ export const isValidRelativeUrl = (relativeUrl: string) => {
* moment.js isValid() API fails to validate time duration, so use regex
* https://github.com/moment/moment/issues/1805
**/
export const regexDuration = /^(-?)P(?=\d|T\d)(?:(\d+)Y)?(?:(\d+)M)?(?:(\d+)([DW]))?(?:T(?:(\d+)H)?(?:(\d+)M)?(?:(\d+(?:\.\d+)?)S)?)?$/;
export const regexDuration = /^([-+]?)P(?=\d|T[-+]?\d)(?:([-+]?\d+)Y)?(?:([-+]?\d+)M)?(?:([-+]?\d+)([DW]))?(?:T(?:([-+]?\d+)H)?(?:([-+]?\d+)M)?(?:([-+]?\d+(?:\.\d+)?)S)?)?$/;
export const regexEmailAddress = /\S+@\S+\.\S+/;
export const regexReportName = /^[\w\-\s\(\)\[\]\,\_\-+]+$/;
export const regexRelativeUrl = /^\/(_plugin\/kibana\/|_dashboards\/)?app\/(dashboards|visualize|discover|discoverLegacy|data-explorer\/discover\/?|observability-dashboards|observability-notebooks|notebooks-dashboards\?view=output_only(&security_tenant=.+)?)(\?security_tenant=.+)?#\/(notebooks\/|view\/|edit\/)?[^\/]+$/;

export const validateReport = async (
client: ILegacyScopedClusterClient,
report: ReportSchemaType,
basePath: String
basePath: string
) => {
report.query_url = report.query_url.replace(basePath, '');
report.report_definition.report_params.core_params.base_url = report.report_definition.report_params.core_params.base_url.replace(
Expand All @@ -66,7 +66,7 @@ export const validateReport = async (
export const validateReportDefinition = async (
client: ILegacyScopedClusterClient,
reportDefinition: ReportDefinitionSchemaType,
basePath: String
basePath: string
) => {
reportDefinition.report_params.core_params.base_url = reportDefinition.report_params.core_params.base_url.replace(
basePath,
Expand Down Expand Up @@ -115,8 +115,7 @@ const validateSavedObject = async (
if (getType(source) === 'notebook') {
// no backend check for notebooks because we would just be checking against the notebooks api again
exist = true;
}
else {
} else {
savedObjectId = `${getType(source)}:${getId(url)}`;
const params: RequestParams.Exists = {
index: '.kibana',
Expand Down
Loading