[2.x backport] Replace re2 with RegExp in timeline and add unit tests (…

…#4203) Remove re2 usage and replace it with JavaScript built-in RegExp object. Also add more unit tests to make sure that using RegExp has same expressions as using re2 library. Issue Resolve #3901 Backport PR #3908 Signed-off-by: Anan Zhuang <[email protected]>
opensearch-project · Jun 15, 2023 · b19344a · b19344a
1 parent ec07e4b
commit b19344a
Show file tree

Hide file tree

Showing 7 changed files with 125 additions and 137 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -126,6 +126,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Add disablePrototypePoisoningProtection configuration to prevent JS client from erroring when cluster utilizes JS reserved words ([#2992](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2992))
 - [Multiple DataSource] Add support for SigV4 authentication ([#3058](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3058))
 - [Multiple DataSource] Refactor test connection to support SigV4 auth type ([#3456](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3456))
+- Replace re2 with RegExp in timeline and add unit tests ([#3908](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3908))
 
 ### 🐛 Bug Fixes
 

diff --git a/package.json b/package.json
@@ -204,7 +204,6 @@
     "pegjs": "0.10.0",
     "proxy-from-env": "1.0.0",
     "query-string": "^6.13.2",
-    "re2": "1.17.4",
     "react": "^16.14.0",
     "react-dom": "^16.12.0",
     "react-input-range": "^1.3.0",

diff --git a/src/dev/build/tasks/patch_native_modules_task.test.ts b/src/dev/build/tasks/patch_native_modules_task.test.ts
@@ -9,8 +9,8 @@ import {
   createAnyInstanceSerializer,
   createAbsolutePathSerializer,
 } from '@osd/dev-utils';
-import { Build, Config } from '../lib';
-import { PatchNativeModules } from './patch_native_modules_task';
+import { Build, Config, read, download, untar, gunzip } from '../lib';
+import { createPatchNativeModulesTask } from './patch_native_modules_task';
 
 const log = new ToolingLog();
 const testWriter = new ToolingLogCollectingWriter();
@@ -19,16 +19,16 @@ expect.addSnapshotSerializer(createAnyInstanceSerializer(Config));
 expect.addSnapshotSerializer(createAnyInstanceSerializer(ToolingLog));
 expect.addSnapshotSerializer(createAbsolutePathSerializer());
 
-jest.mock('../lib/download');
-jest.mock('../lib/fs', () => ({
-  ...jest.requireActual('../lib/fs'),
-  untar: jest.fn(),
-  gunzip: jest.fn(),
-}));
-
-const { untar } = jest.requireMock('../lib/fs');
-const { gunzip } = jest.requireMock('../lib/fs');
-const { download } = jest.requireMock('../lib/download');
+jest.mock('../lib', () => {
+  const originalModule = jest.requireActual('../lib');
+  return {
+    ...originalModule,
+    download: jest.fn(),
+    gunzip: jest.fn(),
+    untar: jest.fn(),
+    read: jest.fn(),
+  };
+});
 
 async function setup() {
   const config = await Config.create({
@@ -38,14 +38,15 @@ async function setup() {
       linux: false,
       linuxArm: false,
       darwin: false,
+      windows: false,
     },
   });
 
   const build = new Build(config);
 
-  download.mockImplementation(() => {});
-  untar.mockImplementation(() => {});
-  gunzip.mockImplementation(() => {});
+  (read as jest.MockedFunction<typeof read>).mockImplementation(async () => {
+    return JSON.stringify({ version: mockPackage.version });
+  });
 
   return { config, build };
 }
@@ -55,38 +56,77 @@ beforeEach(() => {
   jest.clearAllMocks();
 });
 
-it('patch native modules task downloads the correct platform package', async () => {
-  const { config, build } = await setup();
-  config.targetPlatforms.linuxArm = true;
-  await PatchNativeModules.run(config, log, build);
-  expect(download.mock.calls.length).toBe(1);
-  expect(download.mock.calls).toMatchInlineSnapshot(`
+const mockPackage = {
+  name: 'mock-native-module',
+  version: '1.0.0',
+  destinationPath: 'path/to/destination',
+  extractMethod: 'untar',
+  archives: {
+    'linux-arm64': {
+      url: 'https://example.com/mock-native-module/linux-arm64.tar.gz',
+      sha256: 'mock-sha256',
+    },
+    'linux-x64': {
+      url: 'https://example.com/mock-native-module/linux-x64.gz',
+      sha256: 'mock-sha256',
+    },
+  },
+};
+
+describe('patch native modules task', () => {
+  it('patch native modules task downloads the correct platform package', async () => {
+    const { config, build } = await setup();
+    config.targetPlatforms.linuxArm = true;
+    const PatchNativeModulesWithMock = createPatchNativeModulesTask([mockPackage]);
+    await PatchNativeModulesWithMock.run(config, log, build);
+    expect((download as jest.MockedFunction<typeof download>).mock.calls.length).toBe(1);
+    expect((download as jest.MockedFunction<typeof download>).mock.calls).toMatchInlineSnapshot(`
     Array [
       Array [
         Object {
-          "destination": <absolute path>/.native_modules/re2/linux-arm64-83.tar.gz,
+          "destination": <absolute path>/.native_modules/mock-native-module/linux-arm64.tar.gz,
           "log": <ToolingLog>,
           "retries": 3,
-          "sha256": "d86ced75b794fbf518b90908847b3c09a50f3ff5a2815aa30f53080f926a2873",
-          "url": "https://d1v1sj258etie.cloudfront.net/node-re2/releases/download/1.17.4/linux-arm64-83.tar.gz",
+          "sha256": "mock-sha256",
+          "url": "https://example.com/mock-native-module/linux-arm64.tar.gz",
         },
       ],
     ]
   `);
-});
+  });
 
-it('for .tar.gz artifact, patch native modules task unzip it via untar', async () => {
-  const { config, build } = await setup();
-  config.targetPlatforms.linuxArm = true;
-  await PatchNativeModules.run(config, log, build);
-  expect(untar.mock.calls.length).toBe(1);
-  expect(gunzip.mock.calls.length).toBe(0);
-});
+  it('for .tar.gz artifact, patch native modules task unzip it via untar', async () => {
+    const { config, build } = await setup();
+    config.targetPlatforms.linuxArm = true;
+    const PatchNativeModulesWithMock = createPatchNativeModulesTask([mockPackage]);
+    await PatchNativeModulesWithMock.run(config, log, build);
+    expect(untar).toHaveBeenCalled();
+    expect(gunzip).not.toHaveBeenCalled();
+  });
 
-it('for .gz artifact, patch native modules task unzip it via gunzip', async () => {
-  const { config, build } = await setup();
-  config.targetPlatforms.linux = true;
-  await PatchNativeModules.run(config, log, build);
-  expect(untar.mock.calls.length).toBe(0);
-  expect(gunzip.mock.calls.length).toBe(1);
+  it('for .gz artifact, patch native modules task unzip it via gunzip', async () => {
+    const mockPackageGZ = {
+      ...mockPackage,
+      extractMethod: 'gunzip',
+    };
+    const { config, build } = await setup();
+    config.targetPlatforms.linux = true;
+    const PatchNativeModulesWithMock = createPatchNativeModulesTask([mockPackageGZ]);
+    await PatchNativeModulesWithMock.run(config, log, build);
+    expect(gunzip).toHaveBeenCalled();
+    expect(untar).not.toHaveBeenCalled();
+  });
+
+  it('throws error for unsupported extract methods', async () => {
+    const mockPackageUnsupported = {
+      ...mockPackage,
+      extractMethod: 'unsupported',
+    };
+    const { config, build } = await setup();
+    config.targetPlatforms.linux = true;
+    const PatchNativeModulesWithMock = createPatchNativeModulesTask([mockPackageUnsupported]);
+    await expect(PatchNativeModulesWithMock.run(config, log, build)).rejects.toThrow(
+      'Extract method of unsupported is not supported'
+    );
+  });
 });
diff --git a/src/dev/build/tasks/patch_native_modules_task.ts b/src/dev/build/tasks/patch_native_modules_task.ts
@@ -52,45 +52,7 @@ interface Package {
   >;
 }
 
-/* Process for updating URLs and checksums after bumping the version of `re2` or NodeJS:
- *   1. Match the `version` with the version in the yarn.lock file.
- *   2. Match the module version, the digits at the end of the filename, with the output of
- *      `node -p process.versions.modules`.
- *   3. Confirm that the URLs exist for each platform-architecture combo on
- *      https://github.com/uhop/node-re2/releases/tag/[VERSION]; reach out to maintainers for ARM
- *      releases of `re2` as they currently don't have an official ARM release.
- *   4. Generate new checksums for each artifact by downloading each one and calling
- *      `shasum -a 256` or `sha256sum` on the downloaded file.
- */
-const packages: Package[] = [
-  {
-    name: 're2',
-    version: '1.17.4',
-    destinationPath: 'node_modules/re2/build/Release/re2.node',
-    extractMethod: 'gunzip',
-    archives: {
-      'darwin-x64': {
-        url: 'https://github.com/uhop/node-re2/releases/download/1.17.4/darwin-x64-83.gz',
-        sha256: '9112ed93c1544ecc6397f7ff20bd2b28f3b04c7fbb54024e10f9a376a132a87d',
-      },
-      'linux-x64': {
-        url: 'https://github.com/uhop/node-re2/releases/download/1.17.4/linux-x64-83.gz',
-        sha256: '86e03540783a18c41f81df0aec320b1f64aca6cbd3a87fc1b7a9b4109c5f5986',
-      },
-      'linux-arm64': {
-        url:
-          'https://d1v1sj258etie.cloudfront.net/node-re2/releases/download/1.17.4/linux-arm64-83.tar.gz',
-        sha256: 'd86ced75b794fbf518b90908847b3c09a50f3ff5a2815aa30f53080f926a2873',
-        overriddenExtractMethod: 'untar',
-        overriddenDestinationPath: 'node_modules/re2/build/Release',
-      },
-      'win32-x64': {
-        url: 'https://github.com/uhop/node-re2/releases/download/1.17.4/win32-x64-83.gz',
-        sha256: '2f842d9757288afd4bd5dec0e7b370a4c3e89ac98050598b17abb9e8e00e3294',
-      },
-    },
-  },
-];
+export const packages: Package[] = [];
 
 async function getInstalledVersion(config: Config, packageName: string) {
   const packageJSONPath = config.resolveFromRepo(
@@ -145,15 +107,20 @@ async function patchModule(
   }
 }
 
-export const PatchNativeModules: Task = {
-  description: 'Patching platform-specific native modules',
-  async run(config, log, build) {
-    for (const pkg of packages) {
-      await Promise.all(
-        config.getTargetPlatforms().map(async (platform) => {
-          await patchModule(config, log, build, platform, pkg);
-        })
-      );
-    }
-  },
-};
+export function createPatchNativeModulesTask(customPackages?: Package[]): Task {
+  return {
+    description: 'Patching platform-specific native modules',
+    async run(config, log, build) {
+      const targetPackages = customPackages || packages;
+      for (const pkg of targetPackages) {
+        await Promise.all(
+          config.getTargetPlatforms().map(async (platform) => {
+            await patchModule(config, log, build, platform, pkg);
+          })
+        );
+      }
+    },
+  };
+}
+
+export const PatchNativeModules = createPatchNativeModulesTask();
diff --git a/src/plugins/vis_type_timeline/server/series_functions/label.js b/src/plugins/vis_type_timeline/server/series_functions/label.js
@@ -62,10 +62,8 @@ export default new Chainable('label', {
     const config = args.byName;
     return alter(args, function (eachSeries) {
       if (config.regex) {
-        // not using a standard `import` so that if there's an issue with the re2 native module
-        // that it doesn't prevent OpenSearch Dashboards from starting up and we only have an issue using Timeline labels
-        const RE2 = require('re2');
-        eachSeries.label = eachSeries.label.replace(new RE2(config.regex), config.label);
+        const regex = new RegExp(config.regex);
+        eachSeries.label = eachSeries.label.replace(regex, config.label);
       } else {
         eachSeries.label = config.label;
       }

diff --git a/src/plugins/vis_type_timeline/server/series_functions/label.test.js b/src/plugins/vis_type_timeline/server/series_functions/label.test.js
@@ -51,4 +51,24 @@ describe('label.js', () => {
       expect(r.output.list[0].label).to.equal('beerative');
     });
   });
+
+  it('can use a regex to capture groups to modify series label', () => {
+    return invoke(fn, [seriesList, 'beer$2', '(N)(egative)']).then((r) => {
+      expect(r.output.list[0].label).to.equal('beeregative');
+    });
+  });
+
+  it('can handle different regex patterns', () => {
+    const seriesListCopy1 = JSON.parse(JSON.stringify(seriesList));
+    const seriesListCopy2 = JSON.parse(JSON.stringify(seriesList));
+
+    return Promise.all([
+      invoke(fn, [seriesListCopy1, 'beer$1 - $2', '(N)(egative)']).then((r) => {
+        expect(r.output.list[0].label).to.equal('beerN - egative');
+      }),
+      invoke(fn, [seriesListCopy2, 'beer$1_$2', '(N)(eg.*)']).then((r) => {
+        expect(r.output.list[0].label).to.equal('beerN_egative');
+      }),
+    ]);
+  });
 });
diff --git a/yarn.lock b/yarn.lock
@@ -4342,7 +4342,7 @@ agentkeepalive@^3.4.1:
   dependencies:
     humanize-ms "^1.2.1"
 
-agentkeepalive@^4.1.3:
+agentkeepalive@^4.1.3, agentkeepalive@^4.2.1:
   version "4.3.0"
   resolved "https://registry.yarnpkg.com/agentkeepalive/-/agentkeepalive-4.3.0.tgz#bb999ff07412653c1803b3ced35e50729830a255"
   integrity sha512-7Epl1Blf4Sy37j4v9f9FjICCh4+KAQOyXgHEwlyBiAQLbhKdq/i2QQU3amQalS/wPhdPzDXPL5DMR5bkn+YeWg==
@@ -4351,15 +4351,6 @@ agentkeepalive@^4.1.3:
     depd "^2.0.0"
     humanize-ms "^1.2.1"
 
-agentkeepalive@^4.2.1:
-  version "4.2.1"
-  resolved "https://registry.yarnpkg.com/agentkeepalive/-/agentkeepalive-4.2.1.tgz#a7975cbb9f83b367f06c90cc51ff28fe7d499717"
-  integrity sha512-Zn4cw2NEqd+9fiSVWMscnjyQ1a8Yfoc5oBajLeo5w+YBHgDUcEBY2hS4YpTz6iN5f/2zQiktcuM6tS8x1p9dpA==
-  dependencies:
-    debug "^4.1.0"
-    depd "^1.1.2"
-    humanize-ms "^1.2.1"
-
 aggregate-error@^3.0.0:
   version "3.1.0"
   resolved "https://registry.yarnpkg.com/aggregate-error/-/aggregate-error-3.1.0.tgz#92670ff50f5359bdb7a3e0d40d0ec30c5737687a"
@@ -7129,7 +7120,7 @@ delayed-stream@~1.0.0:
 delegates@^1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/delegates/-/delegates-1.0.0.tgz#84c6e159b81904fdca59a0ef44cd870d31250f9a"
-  integrity sha1-hMbhWbgZBP3KWaDvRM2HDTElD5o=
+  integrity sha512-bd2L678uiWATM6m5Z1VzNCErI3jiGzt6HGY8OVICs40JQq/HALfbyNJmp0UDakEY4pMMaN0Ly5om/B1VI/+xfQ==
 
 delete-empty@^2.0.0:
   version "2.0.0"
@@ -7140,11 +7131,6 @@ delete-empty@^2.0.0:
     relative "^3.0.2"
     rimraf "^2.6.2"
 
-depd@^1.1.2:
-  version "1.1.2"
-  resolved "https://registry.yarnpkg.com/depd/-/depd-1.1.2.tgz#9bcd52e14c097763e749b274c4346ed2e560b5a9"
-  integrity sha1-m81S4UwJd2PnSbJ0xDRu0uVgtak=
-
 depd@^2.0.0:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/depd/-/depd-2.0.0.tgz#b696163cc757560d09cf22cc8fad1571b79e76df"
@@ -10246,11 +10232,6 @@ inquirer@^7.0.0, inquirer@^7.3.3:
     strip-ansi "^6.0.0"
     through "^2.3.6"
 
-install-artifact-from-github@^1.3.0:
-  version "1.3.1"
-  resolved "https://registry.yarnpkg.com/install-artifact-from-github/-/install-artifact-from-github-1.3.1.tgz#eefaad9af35d632e5d912ad1569c1de38c3c2462"
-  integrity sha512-3l3Bymg2eKDsN5wQuMfgGEj2x6l5MCAv0zPL6rxHESufFVlEAKW/6oY9F1aGgvY/EgWm5+eWGRjINveL4X7Hgg==
-
 internal-slot@^1.0.3:
   version "1.0.3"
   resolved "https://registry.yarnpkg.com/internal-slot/-/internal-slot-1.0.3.tgz#7347e307deeea2faac2ac6205d4bc7d34967f59c"
@@ -12729,26 +12710,17 @@ minipass-sized@^1.0.3:
   dependencies:
     minipass "^3.0.0"
 
-minipass@^3.0.0, minipass@^3.1.1:
-  version "3.1.6"
-  resolved "https://registry.yarnpkg.com/minipass/-/minipass-3.1.6.tgz#3b8150aa688a711a1521af5e8779c1d3bb4f45ee"
-  integrity sha512-rty5kpw9/z8SX9dmxblFA6edItUmwJgMeYDZRrwlIVN27i8gysGbznJwUggw2V/FVqFSDdWy040ZPS811DYAqQ==
-  dependencies:
-    yallist "^4.0.0"
-
-minipass@^3.1.0, minipass@^3.1.3, minipass@^3.1.6:
+minipass@^3.0.0, minipass@^3.1.0, minipass@^3.1.1, minipass@^3.1.3, minipass@^3.1.6:
   version "3.3.6"
   resolved "https://registry.yarnpkg.com/minipass/-/minipass-3.3.6.tgz#7bba384db3a1520d18c9c0e5251c3444e95dd94a"
   integrity sha512-DxiNidxSEK+tHG6zOIklvNOwm3hvCrbUrdtzY74U6HKTJxvIDfOUL5W5P2Ghd3DTkhhKPYGqeNUIh5qcM4YBfw==
   dependencies:
     yallist "^4.0.0"
 
 minipass@^4.0.0:
-  version "4.0.0"
-  resolved "https://registry.yarnpkg.com/minipass/-/minipass-4.0.0.tgz#7cebb0f9fa7d56f0c5b17853cbe28838a8dbbd3b"
-  integrity sha512-g2Uuh2jEKoht+zvO6vJqXmYpflPqzRBT+Th2h01DKh5z7wbY/AZ2gCQ78cP70YoHPyFdY30YBV5WxgLOEwOykw==
-  dependencies:
-    yallist "^4.0.0"
+  version "4.2.8"
+  resolved "https://registry.yarnpkg.com/minipass/-/minipass-4.2.8.tgz#f0010f64393ecfc1d1ccb5f582bcaf45f48e1a3a"
+  integrity sha512-fNzuVyifolSLFL4NzpF+wEF4qrgqaaKX0haXPQEdQ7NKAN+WecoKMHV09YcuL/DHxrUsYQOK3MiuDf7Ip2OXfQ==
 
 minipass@^5.0.0:
   version "5.0.0"
@@ -14599,15 +14571,6 @@ re-reselect@^3.4.0:
   resolved "https://registry.yarnpkg.com/re-reselect/-/re-reselect-3.4.0.tgz#0f2303f3c84394f57f0cd31fea08a1ca4840a7cd"
   integrity sha512-JsecfN+JlckncVXTWFWjn0Vk6uInl8GSf4eEd9tTk5qXHlgqkPdILpnYpgZcISXNYAzvfvsCZviaDk8AxyS5sg==
 
-[email protected]:
-  version "1.17.4"
-  resolved "https://registry.yarnpkg.com/re2/-/re2-1.17.4.tgz#7bf29290bdde963014e77bd2c2e799a6d788386e"
-  integrity sha512-xyZ4h5PqE8I9tAxTh3G0UttcK5ufrcUxReFjGzfX61vtanNbS1XZHjnwRSyPcLgChI4KLxVgOT/ioZXnUAdoTA==
-  dependencies:
-    install-artifact-from-github "^1.3.0"
-    nan "^2.15.0"
-    node-gyp "^8.4.1"
-
 react-ace@^7.0.5:
   version "7.0.5"
   resolved "https://registry.yarnpkg.com/react-ace/-/react-ace-7.0.5.tgz#798299fd52ddf3a3dcc92afc5865538463544f01"