allow negative signs in duration (#284) (#300)

(cherry picked from commit 69f6da8) Signed-off-by: Joshua Li <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Feb 20, 2024 · 2a77b4f · 2a77b4f
1 parent 73628ad
commit 2a77b4f
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 21 deletions.
diff --git a/server/utils/__tests__/validationHelper.test.ts b/server/utils/__tests__/validationHelper.test.ts
@@ -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 = {
@@ -31,7 +35,7 @@ const createReportDefinitionInput: ReportDefinitionSchemaType = {
     configIds: [],
     title: 'title',
     textDescription: 'text description',
-    htmlDescription: 'html description'
+    htmlDescription: 'html description',
   },
   trigger: {
     trigger_type: TRIGGER_TYPE.onDemand,
@@ -63,12 +67,12 @@ const createReportDefinitionNotebookLegacyInput: ReportDefinitionSchemaType = {
     configIds: [],
     title: 'title',
     textDescription: 'text description',
-    htmlDescription: 'html description'
+    htmlDescription: 'html description',
   },
   trigger: {
     trigger_type: TRIGGER_TYPE.onDemand,
   },
-}
+};
 
 const createReportDefinitionNotebookInput: ReportDefinitionSchemaType = {
   report_params: {
@@ -88,12 +92,12 @@ const createReportDefinitionNotebookInput: ReportDefinitionSchemaType = {
     configIds: [],
     title: 'title',
     textDescription: 'text description',
-    htmlDescription: 'html description'
+    htmlDescription: 'html description',
   },
   trigger: {
     trigger_type: TRIGGER_TYPE.onDemand,
   },
-}
+};
 
 const createReportDefinitionNotebookPostNavBarInput: ReportDefinitionSchemaType = {
   report_params: {
@@ -113,12 +117,12 @@ const createReportDefinitionNotebookPostNavBarInput: ReportDefinitionSchemaType
     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 () => {
@@ -189,7 +193,7 @@ describe('test input validation', () => {
   });
 
   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=',
@@ -223,15 +227,23 @@ describe('test input validation', () => {
         '/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

diff --git a/server/utils/validationHelper.ts b/server/utils/validationHelper.ts
@@ -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 {
@@ -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')
@@ -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(
@@ -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,
@@ -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',