Skip to content

Commit

Permalink
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

Signed-off-by: Anan Zhuang <ananzh@amazon.com>
  • Loading branch information
ananzh authored and AMoo-Miki committed May 16, 2023
1 parent 5dd356f commit b413a1e
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 431 deletions.
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@
"pegjs": "0.10.0",
"proxy-from-env": "1.0.0",
"query-string": "^6.13.2",
"re2": "1.18.0",
"react": "^16.14.0",
"react-dom": "^16.12.0",
"react-input-range": "^1.3.0",
Expand Down
115 changes: 77 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 @@ -44,9 +44,9 @@ async function setup() {

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 @@ -56,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-108.tar.gz,
"destination": <absolute path>/.native_modules/mock-native-module/linux-arm64.tar.gz,
"log": <ToolingLog>,
"retries": 3,
"sha256": "9ec1c0485ad1e0356ba12ae267d7fe485915a40d0bd74019ce6e7ba109912512",
"url": "https://d1v1sj258etie.cloudfront.net/node-re2/releases/download/1.18.0/linux-arm64-108.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.18.0',
destinationPath: 'node_modules/re2/build/Release/re2.node',
extractMethod: 'gunzip',
archives: {
'darwin-x64': {
url: 'https://github.com/uhop/node-re2/releases/download/1.18.0/darwin-x64-108.gz',
sha256: '1fbe31075a86b44b26a3f188ccc6145600b12a1e9096af97076c9f690065137e',
},
'linux-x64': {
url: 'https://github.com/uhop/node-re2/releases/download/1.18.0/linux-x64-108.gz',
sha256: 'f256e25870feb6c371585aca321eb865c3592ab3bb9547591b5af9513c7ac008',
},
'linux-arm64': {
url:
'https://d1v1sj258etie.cloudfront.net/node-re2/releases/download/1.18.0/linux-arm64-108.tar.gz',
sha256: '9ec1c0485ad1e0356ba12ae267d7fe485915a40d0bd74019ce6e7ba109912512',
overriddenExtractMethod: 'untar',
overriddenDestinationPath: 'node_modules/re2/build/Release',
},
'win32-x64': {
url: 'https://github.com/uhop/node-re2/releases/download/1.18.0/win32-x64-108.gz',
sha256: 'a9be9c18995b687724bc0c8a41ff0b4bef836d0165fe4fff42589220240cabf9',
},
},
},
];
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 b413a1e

Please sign in to comment.