Skip to content

Commit

Permalink
fix: fixes in javascript transformation
Browse files Browse the repository at this point in the history
  • Loading branch information
kanishkkatara committed Jun 18, 2024
1 parent a80d403 commit 67e9277
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 50 deletions.
2 changes: 2 additions & 0 deletions src/util/customTransformer-v1.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ async function userTransformHandlerV1(
userTransformation.code,
libraryVersionIds,
userTransformation.versionId,
userTransformation.id,
userTransformation.workspaceId || events[0].metadata?.workspaceId,
credentials,
userTransformation.secrets || {},
testMode,
Expand Down
10 changes: 5 additions & 5 deletions src/util/customTransformer.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,15 +289,15 @@ async function userTransformHandler(
});
const credentialsMap = {};
if (testMode === false) {
events[0]?.credentials?.forEach((cred) => {
credentialsMap[cred.key] = cred.value;
(events[0]?.credentials || []).forEach((cred) => {
credentialsMap[cred.Key] = cred.Value;
});
} else {
credentials?.forEach((cred) => {
credentialsMap[cred.key] = cred.value;
(credentials || []).forEach((cred) => {
credentialsMap[cred.Key] = cred.Value;

Check warning on line 297 in src/util/customTransformer.js

View check run for this annotation

Codecov / codecov/patch

src/util/customTransformer.js#L297

Added line #L297 was not covered by tests
});
events.forEach((ev) => {
if (isNil(ev?.credentials)) {
if (isNil(ev.credentials)) {
ev.credentials = credentials;
}
});
Expand Down
51 changes: 38 additions & 13 deletions src/util/ivmFactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,16 @@ async function loadModule(isolateInternal, contextInternal, moduleName, moduleCo
return module;
}

async function createIvm(code, libraryVersionIds, versionId, credentials, secrets, testMode) {
async function createIvm(
code,
libraryVersionIds,
versionId,
transformationId,
workspaceId,
credentials,
secrets,
testMode,
) {
const createIvmStartTime = new Date();
const logs = [];
const libraries = await Promise.all(
Expand Down Expand Up @@ -243,16 +252,12 @@ async function createIvm(code, libraryVersionIds, versionId, credentials, secret
}),
);

await jail.set('_credential', function (...args) {
await jail.set('_credential', function (key) {
if (_.isNil(credentials) || !_.isObject(credentials)) {
logger.error('Error fetching credentials map');
stats.increment('credential_error', { versionId });
logger.error('Error fetching credentials map', versionId);
stats.increment('credential_error', { transformationId, workspaceId });
return undefined;

Check warning on line 259 in src/util/ivmFactory.js

View check run for this annotation

Codecov / codecov/patch

src/util/ivmFactory.js#L257-L259

Added lines #L257 - L259 were not covered by tests
}
const key = args[0][0];
if (_.isNil(key)) {
throw new Error('Key should be valid and defined');
}
return credentials[key];
});

Expand Down Expand Up @@ -337,9 +342,11 @@ async function createIvm(code, libraryVersionIds, versionId, credentials, secret
let credential = _credential;
delete _credential;
global.credential = function(...args) {
return credential([
...args.map(arg => new ivm.ExternalCopy(arg).copyInto())
]);
const key = args[0];
if (key === null || key === undefined) {
throw new Error('Key should be valid and defined'+ JSON.stringify(args));
}
return credential(new ivm.ExternalCopy(key).copyInto());
};
return new ivm.Reference(function forwardMainPromise(
Expand Down Expand Up @@ -432,10 +439,28 @@ async function compileUserLibrary(code) {
return evaluateModule(isolate, context, code);
}

async function getFactory(code, libraryVersionIds, versionId, credentials, secrets, testMode) {
async function getFactory(
code,
libraryVersionIds,
transformationId,
workspaceId,
versionId,
credentials,
secrets,
testMode,
) {
const factory = {
create: async () => {
return createIvm(code, libraryVersionIds, versionId, credentials, secrets, testMode);
return createIvm(
code,
libraryVersionIds,
versionId,
transformationId,
workspaceId,
credentials,
secrets,
testMode,
);
},
destroy: async (client) => {
client.fnRef.release();
Expand Down
4 changes: 2 additions & 2 deletions src/util/prometheus.js
Original file line number Diff line number Diff line change
Expand Up @@ -570,10 +570,10 @@ class Prometheus {
],
},
{
name: 'credentials_error',
name: 'credential_error',
help: 'Error in fetching credentials count',
type: 'counter',
labelNames: ['versionId'],
labelNames: ['transformationId', 'workspaceId'],
},

// Gauges
Expand Down
14 changes: 8 additions & 6 deletions test/__tests__/data/user_transformation_input_credentials.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,16 @@
},
"credentials": [
{
"key": "key1",
"value": "value1",
"isPublic": true
"Id": "id1",
"Key": "key1",
"Value": "value1",
"IsSecret": false
},
{
"key": "key2",
"value": "value2",
"isPublic": true
"Id": "id2",
"Key": "key2",
"Value": "value2",
"IsSecret": true
}
]
}
Expand Down
6 changes: 3 additions & 3 deletions test/__tests__/user_transformation.integration.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,14 +159,14 @@ describe("Function invocation & creation tests", () => {
versionId,
[],
trRevCode,
null,
[],
true
);
expect(response).toEqual(outputEvents);

// Test with language python; should return same output
trRevCode = contructTrRevCode(versionId, 'python');
response = await userTransformHandler(inputEvents, versionId, [], trRevCode, null, true);
response = await userTransformHandler(inputEvents, versionId, [], trRevCode, [], true);
expect(response).toEqual(outputEvents);
});

Expand All @@ -185,7 +185,7 @@ describe("Function invocation & creation tests", () => {
});

await expect(async () => {
await userTransformHandler(inputEvents, respBody.versionId, null, []);
await userTransformHandler(inputEvents, respBody.versionId, []);
}).rejects.toThrow(RetryRequestError);

// If function is not found, it will be created
Expand Down
24 changes: 15 additions & 9 deletions test/__tests__/user_transformation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1073,7 +1073,7 @@ describe("User transformation", () => {
trRevCode.versionId,
[libraryVersionId],
trRevCode,
null,
[],
true
);

Expand Down Expand Up @@ -1110,7 +1110,7 @@ describe("User transformation", () => {
trRevCode.versionId,
[],
trRevCode,
null,
[],
true
);
expect(output).toEqual(expectedData);
Expand All @@ -1128,7 +1128,7 @@ describe("User transformation", () => {
name,
code: `
export function transformEvent(event, metadata) {
event.credentialValue = credential("key1");
event.credentialValue = credential('key1');
return event;
}
`
Expand Down Expand Up @@ -1174,7 +1174,7 @@ describe("User transformation", () => {
expect(output[0].error).toMatch(/Key should be valid and defined/);
});

it(`Simple ${name} Test with credentials with multiple arguements for codeVersion 1`, async () => {
it(`Simple ${name} Test with credentials with multiple arguments for codeVersion 1`, async () => {
const versionId = randomID();

const inputData = require(`./data/${integration}_input_credentials.json`);
Expand Down Expand Up @@ -1214,7 +1214,10 @@ describe("User transformation", () => {
name,
code: `
export function transformEvent(event, metadata) {
event.credentialValue = credential(1);
event.credentialValueForNumkey = credential(1);
event.credentialValueForBoolkey = credential(true);
event.credentialValueForArraykey = credential([]);
event.credentialValueForObjkey = credential({});
return event;
}
`
Expand All @@ -1229,7 +1232,10 @@ describe("User transformation", () => {
expect(fetch).toHaveBeenCalledWith(
`https://api.rudderlabs.com/transformation/getByVersionId?versionId=${versionId}`
);
expect(output[0].transformedEvent.credentialValue).toEqual(undefined);
expect(output[0].transformedEvent.credentialValueForNumkey).toEqual(undefined);
expect(output[0].transformedEvent.credentialValueForBoolkey).toEqual(undefined);
expect(output[0].transformedEvent.credentialValueForArraykey).toEqual(undefined);
expect(output[0].transformedEvent.credentialValueForObjkey).toEqual(undefined);
});

it(`Simple ${name} Test with credentials without value for codeVersion 1`, async () => {
Expand Down Expand Up @@ -1707,7 +1713,7 @@ describe("Python transformations", () => {
trRevCode.versionId,
[],
trRevCode,
null,
[],
true
);
expect(output).toEqual(outputData);
Expand Down Expand Up @@ -1753,7 +1759,7 @@ describe("Python transformations", () => {
trRevCode.versionId,
Object.values(importNameLibraryVersionIdsMap),
trRevCode,
null,
[],
true
);
expect(output).toEqual(outputData);
Expand Down Expand Up @@ -1795,7 +1801,7 @@ describe("Python transformations", () => {
trRevCode.versionId,
Object.values(importNameLibraryVersionIdsMap),
trRevCode,
null,
[],
true
);
expect(output).toEqual(outputData);
Expand Down
24 changes: 12 additions & 12 deletions test/__tests__/user_transformation_fetch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe("User transformation fetch tests", () => {
`
};

const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);
expect(output).toEqual({
logs: [],
transformedEvents: expectedData.map(ev => (ev.transformedEvent))
Expand All @@ -67,7 +67,7 @@ describe("User transformation fetch tests", () => {
};
const errMsg = "ERROR";

const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);

expect(mockResolver).toHaveBeenCalledTimes(0);
output.transformedEvents.forEach(ev => {
Expand All @@ -92,7 +92,7 @@ describe("User transformation fetch tests", () => {
const errMsg = "ERROR";

mockResolver.mockResolvedValue([ '127.0.0.1' ]);
const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);

expect(mockResolver).toHaveBeenCalledTimes(inputData.length);
expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com');
Expand Down Expand Up @@ -122,7 +122,7 @@ describe("User transformation fetch tests", () => {
};
const errMsg = "invalid url, localhost requests are not allowed";

const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);

expect(mockResolver).toHaveBeenCalledTimes(0);
output.transformedEvents.forEach(ev => {
Expand Down Expand Up @@ -152,7 +152,7 @@ describe("User transformation fetch tests", () => {
const errMsg = "request to https://abc.xyz.com/dummyUrl failed, reason: Invalid IP address: unable to resolve IP address for abc.xyz.com";

mockResolver.mockRejectedValue('invalid host');
const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);

expect(mockResolver).toHaveBeenCalledTimes(inputData.length);
expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com');
Expand Down Expand Up @@ -182,7 +182,7 @@ describe("User transformation fetch tests", () => {
};
const errMsg = "invalid protocol, only http and https are supported";

const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);

output.transformedEvents.forEach(ev => {
expect(ev.errMsg).toEqual(errMsg);
Expand All @@ -204,7 +204,7 @@ describe("User transformation fetch tests", () => {
versionId,
};

const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);
expect(output).toEqual({
logs: [],
transformedEvents: expectedData.map(ev => (ev.transformedEvent))
Expand All @@ -226,7 +226,7 @@ describe("User transformation fetch tests", () => {
};
const errMsg = "ERROR";

const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);

expect(mockResolver).toHaveBeenCalledTimes(0);
output.transformedEvents.forEach(ev => {
Expand All @@ -250,7 +250,7 @@ describe("User transformation fetch tests", () => {
const errMsg = "ERROR";

mockResolver.mockRejectedValue('invalid host');
const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);

expect(mockResolver).toHaveBeenCalledTimes(inputData.length);
expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com');
Expand Down Expand Up @@ -279,7 +279,7 @@ describe("User transformation fetch tests", () => {
};
const errMsg = "invalid url, localhost requests are not allowed";

const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);

expect(mockResolver).toHaveBeenCalledTimes(0);
output.transformedEvents.forEach(ev => {
Expand Down Expand Up @@ -308,7 +308,7 @@ describe("User transformation fetch tests", () => {
const errMsg = "request to https://abc.xyz.com/dummyUrl failed, reason: Invalid IP address: cannot use 127.0.0.1 as IP address";

mockResolver.mockResolvedValue(['3.122.122.122', '127.0.0.1']);
const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);

expect(mockResolver).toHaveBeenCalledTimes(inputData.length);
expect(mockResolver).toHaveBeenCalledWith('abc.xyz.com');
Expand Down Expand Up @@ -337,7 +337,7 @@ describe("User transformation fetch tests", () => {
};
const errMsg = "fetch url is required";

const output = await userTransformHandler(inputData, versionId, [], trRevCode, null, true);
const output = await userTransformHandler(inputData, versionId, [], trRevCode, [], true);

output.transformedEvents.forEach(ev => {
expect(ev.errMsg).toEqual(errMsg);
Expand Down

0 comments on commit 67e9277

Please sign in to comment.