Skip to content

Commit

Permalink
[2.x backport] Replace re2 with RegExp in timeline and add unit tests
Browse files Browse the repository at this point in the history
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
opensearch-project#3901

Backport PR
opensearch-project#3908

Signed-off-by: Anan Zhuang <ananzh@amazon.com>
  • Loading branch information
ananzh committed Jun 1, 2023
1 parent 8af035d commit c51f671
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 138 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,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

Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
116 changes: 78 additions & 38 deletions src/dev/build/tasks/patch_native_modules_task.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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({
Expand All @@ -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 };
}
Expand All @@ -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'
);
});
});
69 changes: 18 additions & 51 deletions src/dev/build/tasks/patch_native_modules_task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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();
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}),
]);
});
});
Loading

0 comments on commit c51f671

Please sign in to comment.