Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(core & function nodes): Update function nodes to work with binary-data-mode 'filesystem'. #3845

Merged
merged 9 commits into from
Sep 11, 2022
Merged

fix(core & function nodes): Update function nodes to work with binary-data-mode 'filesystem'. #3845

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c2e2f88
Initial Fix
rhyswilliamsza Aug 6, 2022
4bb055f
Self-Review #1
rhyswilliamsza Aug 6, 2022
af43ff1
Lint
rhyswilliamsza Aug 6, 2022
03be8f7
Added support for FunctionItem. Minor updates.
rhyswilliamsza Aug 24, 2022
a43e4be
Self-review
rhyswilliamsza Aug 24, 2022
b5e9154
review comments. Added testing.
rhyswilliamsza Aug 25, 2022
812b8aa
Self Review
rhyswilliamsza Aug 25, 2022
7e9bfe1
Fixed memory handling on data manager use.
rhyswilliamsza Aug 26, 2022
4de631f
Fixes for unnecessary memory leaks.
rhyswilliamsza Aug 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/core/src/Interfaces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export interface IExecuteFunctions extends IExecuteFunctionsBase {
mimeType?: string,
): Promise<IBinaryData>;
getBinaryDataBuffer(itemIndex: number, propertyName: string): Promise<Buffer>;
setBinaryDataBuffer(data: IBinaryData, binaryData: Buffer): Promise<IBinaryData>;
request: (uriOrObject: string | IDataObject | any, options?: IDataObject) => Promise<any>; // tslint:disable-line:no-any
requestWithAuthentication(
this: IAllExecuteFunctions,
Expand Down Expand Up @@ -74,6 +75,7 @@ export interface IExecuteFunctions extends IExecuteFunctionsBase {
export interface IExecuteSingleFunctions extends IExecuteSingleFunctionsBase {
helpers: {
getBinaryDataBuffer(propertyName: string, inputIndex?: number): Promise<Buffer>;
setBinaryDataBuffer(data: IBinaryData, binaryData: Buffer): Promise<IBinaryData>;
httpRequest(requestOptions: IHttpRequestOptions): Promise<any>; // tslint:disable-line:no-any
prepareBinaryData(
binaryData: Buffer,
Expand Down
33 changes: 32 additions & 1 deletion packages/core/src/NodeExecuteFunctions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,22 @@ export async function getBinaryDataBuffer(
return BinaryDataManager.getInstance().retrieveBinaryData(binaryData);
}

/**
* Store an incoming IBinaryData & related buffer using the configured binary data manager.
*
* @export
* @param {Buffer} binaryData
* @param {IBinaryData} data
rhyswilliamsza marked this conversation as resolved.
Show resolved Hide resolved
* @returns {Promise<IBinaryData>}
*/
export async function setBinaryDataBuffer(
data: IBinaryData,
binaryData: Buffer,
executionId: string,
): Promise<IBinaryData> {
return BinaryDataManager.getInstance().storeBinaryData(data, binaryData, executionId);
}

/**
* Takes a buffer and converts it into the format n8n uses. It encodes the binary data as
* base64 and adds metadata.
Expand Down Expand Up @@ -874,7 +890,7 @@ export async function prepareBinaryData(
}
}

return BinaryDataManager.getInstance().storeBinaryData(returnData, binaryData, executionId);
return setBinaryDataBuffer(returnData, binaryData, executionId);
}

/**
Expand Down Expand Up @@ -1920,6 +1936,9 @@ export function getExecutePollFunctions(
},
helpers: {
httpRequest,
async setBinaryDataBuffer(data: IBinaryData, binaryData: Buffer): Promise<IBinaryData> {
return setBinaryDataBuffer.call(this, data, binaryData, additionalData.executionId!);
},
async prepareBinaryData(
binaryData: Buffer,
filePath?: string,
Expand Down Expand Up @@ -2091,6 +2110,9 @@ export function getExecuteTriggerFunctions(
additionalCredentialOptions,
);
},
async setBinaryDataBuffer(data: IBinaryData, binaryData: Buffer): Promise<IBinaryData> {
return setBinaryDataBuffer.call(this, data, binaryData, additionalData.executionId!);
},
async prepareBinaryData(
binaryData: Buffer,
filePath?: string,
Expand Down Expand Up @@ -2347,6 +2369,9 @@ export function getExecuteFunctions(
additionalCredentialOptions,
);
},
async setBinaryDataBuffer(data: IBinaryData, binaryData: Buffer): Promise<IBinaryData> {
return setBinaryDataBuffer.call(this, data, binaryData, additionalData.executionId!);
},
async prepareBinaryData(
binaryData: Buffer,
filePath?: string,
Expand Down Expand Up @@ -2589,6 +2614,9 @@ export function getExecuteSingleFunctions(
additionalCredentialOptions,
);
},
async setBinaryDataBuffer(data: IBinaryData, binaryData: Buffer): Promise<IBinaryData> {
return setBinaryDataBuffer.call(this, data, binaryData, additionalData.executionId!);
},
async prepareBinaryData(
binaryData: Buffer,
filePath?: string,
Expand Down Expand Up @@ -3086,6 +3114,9 @@ export function getExecuteWebhookFunctions(
additionalCredentialOptions,
);
},
async setBinaryDataBuffer(data: IBinaryData, binaryData: Buffer): Promise<IBinaryData> {
return setBinaryDataBuffer.call(this, data, binaryData, additionalData.executionId!);
},
async prepareBinaryData(
binaryData: Buffer,
filePath?: string,
Expand Down
27 changes: 24 additions & 3 deletions packages/nodes-base/nodes/FunctionItem/FunctionItem.node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ return item;`,
};

for (let itemIndex = 0; itemIndex < length; itemIndex++) {
const mode = this.getMode();

try {
item = items[itemIndex];

Expand All @@ -83,14 +85,35 @@ return item;`,

// Define the global objects for the custom function
const sandbox = {
/** @deprecated for removal */
rhyswilliamsza marked this conversation as resolved.
Show resolved Hide resolved
getBinaryData: (): IBinaryKeyData | undefined => {
if (mode === 'manual') this.sendMessageToUI("getBinaryData(...) is deprecated and will be removed in a future version. Please consider switching to getBinaryDataAsync(...) instead.");
return item.binary;
},
/** @deprecated for removal */
setBinaryData: async (data: IBinaryKeyData) => {
if (mode === 'manual') this.sendMessageToUI("setBinaryData(...) is deprecated and will be removed in a future version. Please consider switching to setBinaryDataAsync(...) instead.");
item.binary = data;
},
getNodeParameter: this.getNodeParameter,
getWorkflowStaticData: this.getWorkflowStaticData,
helpers: this.helpers,
item: item.json,
setBinaryData: (data: IBinaryKeyData) => {
getBinaryDataAsync: async (): Promise<IBinaryKeyData | undefined> => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getBinaryDataAsync: async ()

The Function node requires one to pass in the item here. It isn't necessary in the FunctionItem node, however perhaps we should align the two for better a better user experience when switching between them? Though - to be honest - I quite like it this way if we think about the nodes independently.

if (item?.binary) {
for (const binaryPropertyName of Object.keys(item.binary)) {
item.binary[binaryPropertyName].data = (await this.helpers.getBinaryDataBuffer(itemIndex, binaryPropertyName))?.toString('base64');
}
}
return item.binary;
},
setBinaryDataAsync: async (data: IBinaryKeyData) => {
if (data) {
for (const binaryPropertyName of Object.keys(data)) {
const binaryItem = data[binaryPropertyName];
data[binaryPropertyName] = (await this.helpers.setBinaryDataBuffer(binaryItem, Buffer.from(binaryItem.data, 'base64')));
}
}
item.binary = data;
},
};
Expand All @@ -99,8 +122,6 @@ return item;`,
const dataProxy = this.getWorkflowDataProxy(itemIndex);
Object.assign(sandbox, dataProxy);

const mode = this.getMode();

const options = {
console: mode === 'manual' ? 'redirect' : 'inherit',
sandbox,
Expand Down