mirror of
https://github.com/n8n-io/n8n.git
synced 2026-05-31 16:57:08 +02:00
fix(MongoDB Node): Validate update key value type (#31371)
Some checks failed
CI: Master (Build, Test, Lint) / Build for Github Cache (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (22.22.3) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (24.15.0) (push) Waiting to run
CI: Master (Build, Test, Lint) / Lint (push) Waiting to run
CI: Master (Build, Test, Lint) / Performance (push) Waiting to run
CI: Master (Build, Test, Lint) / Notify Slack on failure (push) Blocked by required conditions
Util: Sync API Docs / sync-public-api (push) Waiting to run
Build: Benchmark Image / build (push) Has been cancelled
Some checks failed
CI: Master (Build, Test, Lint) / Build for Github Cache (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (22.22.3) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (24.15.0) (push) Waiting to run
CI: Master (Build, Test, Lint) / Lint (push) Waiting to run
CI: Master (Build, Test, Lint) / Performance (push) Waiting to run
CI: Master (Build, Test, Lint) / Notify Slack on failure (push) Blocked by required conditions
Util: Sync API Docs / sync-public-api (push) Waiting to run
Build: Benchmark Image / build (push) Has been cancelled
This commit is contained in:
parent
dd4b3ff446
commit
439d260181
|
|
@ -1,12 +1,16 @@
|
|||
import type { INode } from 'n8n-workflow';
|
||||
|
||||
import { prepareItems } from './GenericFunctions';
|
||||
|
||||
const mockNode = { name: 'MongoDB', type: 'n8n-nodes-base.mongoDb' } as INode;
|
||||
|
||||
describe('MongoDB Node: Generic Functions', () => {
|
||||
describe('prepareItems', () => {
|
||||
it('should select fields', () => {
|
||||
const items = [{ json: { name: 'John', age: 30 } }, { json: { name: 'Jane', age: 25 } }];
|
||||
const fields = ['name'];
|
||||
|
||||
const result = prepareItems({ items, fields });
|
||||
const result = prepareItems({ items, fields, node: mockNode });
|
||||
|
||||
expect(result).toEqual([{ name: 'John' }, { name: 'Jane' }]);
|
||||
});
|
||||
|
|
@ -16,7 +20,7 @@ describe('MongoDB Node: Generic Functions', () => {
|
|||
const fields = ['age'];
|
||||
const updateKey = 'name';
|
||||
|
||||
const result = prepareItems({ items, fields, updateKey });
|
||||
const result = prepareItems({ items, fields, updateKey, node: mockNode });
|
||||
|
||||
expect(result).toEqual([
|
||||
{ name: 'John', age: 30 },
|
||||
|
|
@ -29,7 +33,13 @@ describe('MongoDB Node: Generic Functions', () => {
|
|||
const fields = ['user.name'];
|
||||
const useDotNotation = true;
|
||||
|
||||
const result = prepareItems({ items, fields, updateKey: '', useDotNotation });
|
||||
const result = prepareItems({
|
||||
items,
|
||||
fields,
|
||||
updateKey: '',
|
||||
useDotNotation,
|
||||
node: mockNode,
|
||||
});
|
||||
|
||||
expect(result).toEqual([{ user: { name: 'John' } }, { user: { name: 'Jane' } }]);
|
||||
});
|
||||
|
|
@ -50,6 +60,7 @@ describe('MongoDB Node: Generic Functions', () => {
|
|||
useDotNotation,
|
||||
dateFields,
|
||||
isUpdate,
|
||||
node: mockNode,
|
||||
});
|
||||
expect(result).toEqual([
|
||||
{ date: new Date('2023-10-01T00:00:00Z') },
|
||||
|
|
@ -57,6 +68,67 @@ describe('MongoDB Node: Generic Functions', () => {
|
|||
]);
|
||||
});
|
||||
|
||||
describe('updateKey value validation', () => {
|
||||
it('throws when the updateKey value is a plain object', () => {
|
||||
const items = [{ json: { id: { $regex: '^a' }, value: 'x' } }];
|
||||
const args = { items, fields: ['value'], updateKey: 'id', node: mockNode };
|
||||
|
||||
expect(() => prepareItems(args)).toThrow(/must be a string, number, boolean, or date/);
|
||||
});
|
||||
|
||||
it('throws when the updateKey value is an array', () => {
|
||||
const items = [{ json: { id: ['a', 'b'], value: 'x' } }];
|
||||
const args = { items, fields: ['value'], updateKey: 'id', node: mockNode };
|
||||
|
||||
expect(() => prepareItems(args)).toThrow();
|
||||
});
|
||||
|
||||
it('drops items where useDotNotation would resolve the updateKey to a non-scalar', () => {
|
||||
// The data-filter step in prepareItems uses bracket access on the dotted key,
|
||||
// so items whose dot path resolves to an object are excluded before the map loop
|
||||
// runs. No operator-shaped value reaches the driver.
|
||||
const items = [{ json: { user: { id: { $gt: 1 } }, value: 'x' } }];
|
||||
const args = {
|
||||
items,
|
||||
fields: ['value'],
|
||||
updateKey: 'user.id',
|
||||
useDotNotation: true,
|
||||
node: mockNode,
|
||||
};
|
||||
|
||||
const result = prepareItems(args);
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('passes a string updateKey value through unchanged even when its content looks like JSON', () => {
|
||||
const items = [{ json: { id: '{"$regex":"^a"}', value: 'x' } }];
|
||||
const args = { items, fields: ['value'], updateKey: 'id', node: mockNode };
|
||||
|
||||
const result = prepareItems(args);
|
||||
|
||||
expect(result).toEqual([{ id: '{"$regex":"^a"}', value: 'x' }]);
|
||||
});
|
||||
|
||||
it('accepts number, boolean, and null updateKey values', () => {
|
||||
const items = [
|
||||
{ json: { id: 1, value: 'a' } },
|
||||
{ json: { id: true, value: 'b' } },
|
||||
{ json: { id: null, value: 'c' } },
|
||||
];
|
||||
const args = { items, fields: ['value'], updateKey: 'id', node: mockNode };
|
||||
|
||||
expect(() => prepareItems(args)).not.toThrow();
|
||||
});
|
||||
|
||||
it('accepts Date updateKey values', () => {
|
||||
const items = [{ json: { id: new Date('2024-01-01'), value: 'a' } }];
|
||||
const args = { items, fields: ['value'], updateKey: 'id', node: mockNode };
|
||||
|
||||
expect(() => prepareItems(args)).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
it('should handle updates', () => {
|
||||
// Should keep dot notation in result to not overwrite the original values
|
||||
const items = [
|
||||
|
|
@ -73,6 +145,7 @@ describe('MongoDB Node: Generic Functions', () => {
|
|||
useDotNotation,
|
||||
dateFields: [],
|
||||
isUpdate,
|
||||
node: mockNode,
|
||||
});
|
||||
expect(result).toEqual([{ 'user.name': 'John' }, { 'user.name': 'Jane' }]);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -80,6 +80,24 @@ export function validateAndResolveMongoCredentials(
|
|||
}
|
||||
}
|
||||
|
||||
function isScalarUpdateKeyValue(
|
||||
value: unknown,
|
||||
): value is string | number | boolean | bigint | Date | null {
|
||||
if (value === null) return true;
|
||||
const type = typeof value;
|
||||
if (type === 'string' || type === 'number' || type === 'boolean' || type === 'bigint') {
|
||||
return true;
|
||||
}
|
||||
return value instanceof Date;
|
||||
}
|
||||
|
||||
function describeUpdateKeyValueType(value: unknown): string {
|
||||
if (value === null) return 'null';
|
||||
if (Array.isArray(value)) return 'array';
|
||||
if (value instanceof Date) return 'date';
|
||||
return typeof value;
|
||||
}
|
||||
|
||||
export function prepareItems({
|
||||
items,
|
||||
fields,
|
||||
|
|
@ -87,6 +105,7 @@ export function prepareItems({
|
|||
useDotNotation = false,
|
||||
dateFields = [],
|
||||
isUpdate = false,
|
||||
node,
|
||||
}: {
|
||||
items: INodeExecutionData[];
|
||||
fields: string[];
|
||||
|
|
@ -94,6 +113,7 @@ export function prepareItems({
|
|||
useDotNotation?: boolean;
|
||||
dateFields?: string[];
|
||||
isUpdate?: boolean;
|
||||
node: INode;
|
||||
}) {
|
||||
let data = items;
|
||||
|
||||
|
|
@ -104,7 +124,7 @@ export function prepareItems({
|
|||
data = items.filter((item) => item.json[updateKey] !== undefined);
|
||||
}
|
||||
|
||||
const preparedItems = data.map(({ json }) => {
|
||||
const preparedItems = data.map(({ json }, itemIndex) => {
|
||||
const updateItem: IDataObject = {};
|
||||
|
||||
for (const field of fields) {
|
||||
|
|
@ -120,6 +140,17 @@ export function prepareItems({
|
|||
fieldData = new Date(fieldData as string);
|
||||
}
|
||||
|
||||
if (field === updateKey && !isScalarUpdateKeyValue(fieldData)) {
|
||||
throw new NodeOperationError(
|
||||
node,
|
||||
`The value of "${updateKey}" must be a string, number, boolean, or date`,
|
||||
{
|
||||
itemIndex,
|
||||
description: `Got ${describeUpdateKeyValueType(fieldData)} instead. Objects and arrays are not allowed as the match value.`,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
if (useDotNotation && !isUpdate) {
|
||||
set(updateItem, field, fieldData);
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -254,28 +254,25 @@ export class MongoDb implements INodeType {
|
|||
const updateOptions = (this.getNodeParameter('upsert', i) as boolean)
|
||||
? { upsert: true }
|
||||
: undefined;
|
||||
const [item] = prepareItems({
|
||||
items: [items[i]],
|
||||
fields,
|
||||
updateKey,
|
||||
useDotNotation,
|
||||
dateFields,
|
||||
});
|
||||
|
||||
if (!item) {
|
||||
if (this.continueOnFail()) {
|
||||
returnData.push({
|
||||
json: { error: 'Item is missing the updateKey field' },
|
||||
pairedItem: { item: i },
|
||||
});
|
||||
continue;
|
||||
}
|
||||
throw new NodeOperationError(this.getNode(), 'Item is missing the updateKey field', {
|
||||
itemIndex: i,
|
||||
});
|
||||
}
|
||||
|
||||
try {
|
||||
const [item] = prepareItems({
|
||||
items: [items[i]],
|
||||
fields,
|
||||
updateKey,
|
||||
useDotNotation,
|
||||
dateFields,
|
||||
node: this.getNode(),
|
||||
});
|
||||
|
||||
if (!item) {
|
||||
throw new NodeOperationError(
|
||||
this.getNode(),
|
||||
'Item is missing the updateKey field',
|
||||
{ itemIndex: i },
|
||||
);
|
||||
}
|
||||
|
||||
const filter = { [updateKey]: item[updateKey] };
|
||||
if (updateKey === '_id') {
|
||||
filter[updateKey] = new ObjectId(item[updateKey] as string);
|
||||
|
|
@ -321,6 +318,7 @@ export class MongoDb implements INodeType {
|
|||
updateKey,
|
||||
useDotNotation,
|
||||
dateFields,
|
||||
node: this.getNode(),
|
||||
});
|
||||
|
||||
for (const item of updateItems) {
|
||||
|
|
@ -367,29 +365,26 @@ export class MongoDb implements INodeType {
|
|||
const updateOptions = (this.getNodeParameter('upsert', i) as boolean)
|
||||
? { upsert: true }
|
||||
: undefined;
|
||||
const [item] = prepareItems({
|
||||
items: [items[i]],
|
||||
fields,
|
||||
updateKey,
|
||||
useDotNotation,
|
||||
dateFields,
|
||||
isUpdate: true,
|
||||
});
|
||||
|
||||
if (!item) {
|
||||
if (this.continueOnFail()) {
|
||||
returnData.push({
|
||||
json: { error: 'Item is missing the updateKey field' },
|
||||
pairedItem: { item: i },
|
||||
});
|
||||
continue;
|
||||
}
|
||||
throw new NodeOperationError(this.getNode(), 'Item is missing the updateKey field', {
|
||||
itemIndex: i,
|
||||
});
|
||||
}
|
||||
|
||||
try {
|
||||
const [item] = prepareItems({
|
||||
items: [items[i]],
|
||||
fields,
|
||||
updateKey,
|
||||
useDotNotation,
|
||||
dateFields,
|
||||
isUpdate: true,
|
||||
node: this.getNode(),
|
||||
});
|
||||
|
||||
if (!item) {
|
||||
throw new NodeOperationError(
|
||||
this.getNode(),
|
||||
'Item is missing the updateKey field',
|
||||
{ itemIndex: i },
|
||||
);
|
||||
}
|
||||
|
||||
const filter = { [updateKey]: item[updateKey] };
|
||||
if (updateKey === '_id') {
|
||||
filter[updateKey] = new ObjectId(item[updateKey] as string);
|
||||
|
|
@ -436,6 +431,7 @@ export class MongoDb implements INodeType {
|
|||
useDotNotation,
|
||||
dateFields,
|
||||
isUpdate: nodeVersion >= 1.2,
|
||||
node: this.getNode(),
|
||||
});
|
||||
|
||||
for (const item of updateItems) {
|
||||
|
|
@ -488,6 +484,7 @@ export class MongoDb implements INodeType {
|
|||
updateKey: '',
|
||||
useDotNotation,
|
||||
dateFields,
|
||||
node: this.getNode(),
|
||||
});
|
||||
|
||||
if (!insertItem) continue;
|
||||
|
|
@ -561,6 +558,7 @@ export class MongoDb implements INodeType {
|
|||
updateKey: '',
|
||||
useDotNotation,
|
||||
dateFields,
|
||||
node: this.getNode(),
|
||||
});
|
||||
|
||||
const { insertedIds } = await mdb
|
||||
|
|
@ -606,29 +604,26 @@ export class MongoDb implements INodeType {
|
|||
const updateOptions = (this.getNodeParameter('upsert', i) as boolean)
|
||||
? { upsert: true }
|
||||
: undefined;
|
||||
const [item] = prepareItems({
|
||||
items: [items[i]],
|
||||
fields,
|
||||
updateKey,
|
||||
useDotNotation,
|
||||
dateFields,
|
||||
isUpdate: true,
|
||||
});
|
||||
|
||||
if (!item) {
|
||||
if (this.continueOnFail()) {
|
||||
returnData.push({
|
||||
json: { error: 'Item is missing the updateKey field' },
|
||||
pairedItem: { item: i },
|
||||
});
|
||||
continue;
|
||||
}
|
||||
throw new NodeOperationError(this.getNode(), 'Item is missing the updateKey field', {
|
||||
itemIndex: i,
|
||||
});
|
||||
}
|
||||
|
||||
try {
|
||||
const [item] = prepareItems({
|
||||
items: [items[i]],
|
||||
fields,
|
||||
updateKey,
|
||||
useDotNotation,
|
||||
dateFields,
|
||||
isUpdate: true,
|
||||
node: this.getNode(),
|
||||
});
|
||||
|
||||
if (!item) {
|
||||
throw new NodeOperationError(
|
||||
this.getNode(),
|
||||
'Item is missing the updateKey field',
|
||||
{ itemIndex: i },
|
||||
);
|
||||
}
|
||||
|
||||
const filter = { [updateKey]: item[updateKey] };
|
||||
if (updateKey === '_id') {
|
||||
filter[updateKey] = new ObjectId(item[updateKey] as string);
|
||||
|
|
@ -675,6 +670,7 @@ export class MongoDb implements INodeType {
|
|||
useDotNotation,
|
||||
dateFields,
|
||||
isUpdate: nodeVersion >= 1.2,
|
||||
node: this.getNode(),
|
||||
});
|
||||
|
||||
for (const item of updateItems) {
|
||||
|
|
|
|||
|
|
@ -370,6 +370,78 @@ describe('MongoDB CRUD Node', () => {
|
|||
expect(items[2].pairedItem).toEqual({ item: 2 });
|
||||
});
|
||||
|
||||
describe.each(['findOneAndReplace', 'findOneAndUpdate', 'update'])(
|
||||
'%s: non-scalar updateKey value',
|
||||
(operation) => {
|
||||
const itemsWithObjectKey = [
|
||||
{ json: { id: { $regex: '^a' }, value: 'x', collection: 'col1' } },
|
||||
];
|
||||
|
||||
function mockObjectKey(continueOnFail: boolean) {
|
||||
const mock = mockExecuteFunctions(1.3, operation);
|
||||
mock.getInputData.mockReturnValue(itemsWithObjectKey);
|
||||
mock.continueOnFail.mockReturnValue(continueOnFail);
|
||||
mock.getNodeParameter.mockImplementation(
|
||||
(parameterName: string, _itemIndex = 0, fallbackValue?: NodeParameterValueType) => {
|
||||
switch (parameterName) {
|
||||
case 'operation':
|
||||
return operation;
|
||||
case 'collection':
|
||||
return 'col1';
|
||||
case 'fields':
|
||||
return 'value';
|
||||
case 'updateKey':
|
||||
return 'id';
|
||||
case 'upsert':
|
||||
return false;
|
||||
case 'options.useDotNotation':
|
||||
return false;
|
||||
case 'options.dateFields':
|
||||
return '';
|
||||
default:
|
||||
return fallbackValue;
|
||||
}
|
||||
},
|
||||
);
|
||||
return mock;
|
||||
}
|
||||
|
||||
it('throws NodeOperationError when continueOnFail is off', async () => {
|
||||
await expect(node.execute.call(mockObjectKey(false))).rejects.toThrow(
|
||||
/must be a string, number, boolean, or date/,
|
||||
);
|
||||
});
|
||||
|
||||
it('pushes error item with pairedItem when continueOnFail is on', async () => {
|
||||
const [items] = await node.execute.call(mockObjectKey(true));
|
||||
expect(items).toHaveLength(1);
|
||||
expect(items[0].json.error).toMatch(/must be a string, number, boolean, or date/);
|
||||
expect(items[0].pairedItem).toEqual({ item: 0 });
|
||||
});
|
||||
|
||||
it('does not invoke the driver for the affected item', async () => {
|
||||
const findOneAndReplaceSpy = jest.spyOn(Collection.prototype, 'findOneAndReplace');
|
||||
const findOneAndUpdateSpy = jest.spyOn(Collection.prototype, 'findOneAndUpdate');
|
||||
const updateOneSpy = jest.spyOn(Collection.prototype, 'updateOne');
|
||||
findOneAndReplaceSpy.mockResolvedValue(null);
|
||||
findOneAndUpdateSpy.mockResolvedValue(null);
|
||||
updateOneSpy.mockResolvedValue({
|
||||
acknowledged: true,
|
||||
matchedCount: 0,
|
||||
modifiedCount: 0,
|
||||
upsertedCount: 0,
|
||||
upsertedId: null,
|
||||
});
|
||||
|
||||
await node.execute.call(mockObjectKey(true));
|
||||
|
||||
expect(findOneAndReplaceSpy).not.toHaveBeenCalled();
|
||||
expect(findOneAndUpdateSpy).not.toHaveBeenCalled();
|
||||
expect(updateOneSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
describe.each(['findOneAndReplace', 'findOneAndUpdate', 'update'])(
|
||||
'%s: item missing the updateKey field',
|
||||
(operation) => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user