Recursive ObjectID hook. Good, bad or ugly?

[ericuldall]

I think most veteran feathers users are well aware of the issue with querying by _id not coercing data automatically to mongo ObjectID, if you're not get educated on #757

I've decided to implement a hook that should be easily configurable and allow for a slightly less heavy handed implementation, and more flexibility in the client.

It looks like this as of now:

const { ObjectID } = require('mongodb');
module.exports = {
	castObjectIdFromQuery: (field) => {
		return  context => {
			const { query = {} } = context.params;
			
			context.query = iterateObjectKeysToObjectID(query, field);
			console.log(context.query);
			return context;
		};
	},
	castObjectIdFromData: (field) => {
		return  context => {
			const { data = {} } = context;
			
			if(data[field]) {
				if(typeof data[field] === 'string') {
					data[field] = new ObjectID(data[field]);
				} else if(Array.isArray(data[field])) {
					data[field] = data[field].map(id => new ObjectID(id));
				}
			}
			context.data = data;
			return context;
		};
	}
};

function iterateObjectKeysToObjectID (obj, field) {
	for (let key of Object.keys(obj)) {
		if (key === field) {
			obj = convertToObjectID(obj, field);
		} else if ( typeof obj[key] === "object" ) {
			obj[key] = iterateObjectKeysToObjectID(obj[key], field);
		}
	}
	return obj;
}

function convertToObjectID (idObj, field) {
	const otherStringOperators = ['$eq', '$ne'];
	const arrayOperators = ['$in', '$nin'];
	if(idObj[field]) {
		if(typeof idObj[field] === 'string') {
			idObj[field] = new ObjectID(idObj[field]);
		} else {
			// check array operators
			for (let operator of arrayOperators) {
				if (idObj[field][operator]) {
					idObj[field][operator] = 
						idObj[field][operator].map(id => new ObjectID(id));
				}
			}
			// check other string operators
			for (let operator of otherStringOperators) {
				if (idObj[field][operator]) {
					idObj[field][operator] =
						new ObjectID(idObj[field][operator]);
				}
			}
		}
	}
	return idObj;
}

It's a basic recursion concept and currently tested it working with a query like:

{
    $and: [
        {
            _id: '123456789012'
        },
        {
            $or: [
                {
                    _id: '234567890123'
                }
            ]
        }
    ]
}

I know that query doesn't make sense but the _id fields do get successfully converted.

Hook implementation:

before: {
    find: [
        castObjectIdFromQuery('_id'), // allows us to select which keys to look for
        castObjectIdFromQuery('userId'), // should be able to use it as many times as you want (maybe switch to array support in the argument instead?
    ]
}

Really just looking for feedback on if @daffl or anyone else has gone down this rabbit hole and has any life lessons to share on the topic.

Thanks in advance :D

[ericuldall]

I updated it to allow array and it will convert all keys in one pass. Not sure of the exact O complexity, but considering most queries are pretty small I don't think it will be problematic.

function iterateObjectKeysToObjectID (obj, fields) {
    if (obj) {
        for (let key of Object.keys(obj)) {
            if (Array.isArray(fields) && fields.includes(key)) {
                const field = fields[fields.indexOf(key)];
                obj = convertToObjectID(obj, field);
            } else if (key === fields) {
                obj = convertToObjectID(obj, fields);
            } else if ( typeof obj[key] === "object" ) {
                obj[key] = iterateObjectKeysToObjectID(obj[key], fields);
            }
        }
    }
    return obj;
}

function convertToObjectID (idObj, field) {
    const otherStringOperators = ['$eq', '$ne'];
    const arrayOperators = ['$in', '$nin'];
    if(idObj[field]) {
        if(typeof idObj[field] === 'string') {
            idObj[field] = new ObjectID(idObj[field]);
        } else {
            // check array operators
            for (let operator of arrayOperators) {
                if (idObj[field][operator]) {
                    idObj[field][operator] =
                        idObj[field][operator].map(id => new ObjectID(id));
                }
            }
            // check other string operators
            for (let operator of otherStringOperators) {
                if (idObj[field][operator]) {
                    idObj[field][operator] =
                        new ObjectID(idObj[field][operator]);
                }
            }
        }
    }
    return idObj;
}

Implementation:

before: {
    find: [
        castObjectIdFromQuery(['_id', 'userId'])
    ]
}