Enhance database object handling with ObjectId normalization and error handling
Some checks failed
farmcontrol/farmcontrol-ws/pipeline/head There was a failure building this commit

- Introduced `refId`, `normalizeFilter`, and `coerceObjectIds` functions in `objectid.js` to standardize ObjectId handling across the database operations.
- Updated `listObjects`, `getObject`, `editObject`, and `newObject` functions to utilize the new normalization methods, improving data integrity and error management.
- Enhanced error handling in socket event handlers to ensure proper feedback on operation failures.
- Refactored `expandObjectIds` and related utility functions to streamline ObjectId processing and improve code clarity.
This commit is contained in:
Tom Butcher 2026-11-07 18:52:45 +00:00
parent bd6038241f
commit 1ae6c3f05c
10 changed files with 337 additions and 81 deletions

View File

@ -14,6 +14,7 @@ import { loadConfig } from '../config.js';
import { formatTraceData, getQueryToCacheKey } from '../utils.js'; import { formatTraceData, getQueryToCacheKey } from '../utils.js';
import { redisServer } from './redis.js'; import { redisServer } from './redis.js';
import { auditLogModel } from './schemas/management/auditlog.schema.js'; import { auditLogModel } from './schemas/management/auditlog.schema.js';
import { coerceObjectIds, normalizeFilter, refId } from './objectid.js';
const config = loadConfig(); const config = loadConfig();
@ -153,17 +154,10 @@ export const listObjects = async ({
if (!sort || sort === '') { if (!sort || sort === '') {
sort = 'createdAt'; sort = 'createdAt';
} }
// Translate parent._id to parent for Mongoose
if (filter['parent._id']) {
filter.parent = filter['parent._id'];
delete filter['parent._id'];
}
// Translate owner._id to owner for Mongoose // `{ job: { _id } }` and socket.io's `{ buffer: ... }` ObjectId form both
if (filter['owner._id']) { // need to be the raw id before Mongoose casts the query.
filter.owner = filter['owner._id']; filter = normalizeFilter(filter);
delete filter['owner._id'];
}
// Use find with population and filter // Use find with population and filter
let query = model.find(filter).sort({ [sort]: sortOrder }); let query = model.find(filter).sort({ [sort]: sortOrder });
@ -217,6 +211,12 @@ export const getObject = async ({
cached = false cached = false
}) => { }) => {
try { try {
id = refId(id);
if (typeof id !== 'string' || id.length === 0) {
logger.error('Invalid object id:', formatTraceData(id));
return undefined;
}
logger.trace( logger.trace(
`Getting object: ${formatTraceData({ `Getting object: ${formatTraceData({
model, model,
@ -275,7 +275,6 @@ export const getObject = async ({
return finalResult; return finalResult;
} catch (error) { } catch (error) {
logger.error('An error retreiving object:', error.message); logger.error('An error retreiving object:', error.message);
throw error;
return undefined; return undefined;
} }
}; };
@ -521,6 +520,12 @@ export const editObject = async ({
recalculate = true recalculate = true
}) => { }) => {
try { try {
id = refId(id);
if (typeof id !== 'string' || id.length === 0) {
return { error: 'Invalid object id.', code: 400 };
}
updateData = coerceObjectIds(model, updateData);
// Determine parentType from model name // Determine parentType from model name
const parentType = model.modelName ? model.modelName : 'unknown'; const parentType = model.modelName ? model.modelName : 'unknown';
// Fetch the and update object // Fetch the and update object
@ -620,6 +625,7 @@ export const newObject = async ({
}) => { }) => {
try { try {
const parentType = model.modelName ? model.modelName : 'unknown'; const parentType = model.modelName ? model.modelName : 'unknown';
newData = coerceObjectIds(model, newData);
const result = await model.create(newData); const result = await model.create(newData);
if (!result || result.length === 0) { if (!result || result.length === 0) {
@ -663,6 +669,10 @@ export const deleteObject = async ({
ownerType = undefined ownerType = undefined
}) => { }) => {
try { try {
id = refId(id);
if (typeof id !== 'string' || id.length === 0) {
return { error: 'Invalid object id.', code: 400 };
}
const parentType = model.modelName ? model.modelName : 'unknown'; const parentType = model.modelName ? model.modelName : 'unknown';
// Delete the object // Delete the object
const result = await model.findByIdAndDelete(id); const result = await model.findByIdAndDelete(id);

View File

@ -1,4 +1,5 @@
import mongoose from 'mongoose'; import mongoose from 'mongoose';
import { refId } from './objectid.js';
import log4js from 'log4js'; import log4js from 'log4js';
import { loadConfig } from '../config.js'; import { loadConfig } from '../config.js';
@ -810,7 +811,10 @@ async function parseFilter(property, value, model = null) {
const isBoolField = isBooleanField(property, model); const isBoolField = isBooleanField(property, model);
if (value?._id !== undefined && value?._id !== null) { if (value?._id !== undefined && value?._id !== null) {
return { [filterProperty]: new mongoose.Types.ObjectId(value._id) }; const id = refId(value._id);
if (typeof id === 'string' && /^[a-fA-F\d]{24}$/.test(id)) {
return { [filterProperty]: new mongoose.Types.ObjectId(id) };
}
} }
// Non-string values pass through; boolean fields only accept actual booleans. // Non-string values pass through; boolean fields only accept actual booleans.

View File

@ -2,6 +2,7 @@ import { connect } from '@nats-io/transport-node';
import log4js from 'log4js'; import log4js from 'log4js';
import { loadConfig } from '../config.js'; import { loadConfig } from '../config.js';
import { formatTraceData } from '../utils.js'; import { formatTraceData } from '../utils.js';
import { stringifyIds } from './objectid.js';
const config = loadConfig(); const config = loadConfig();
const logger = log4js.getLogger('Nats'); const logger = log4js.getLogger('Nats');
@ -69,7 +70,7 @@ class NatsServer {
async publish(subject, data) { async publish(subject, data) {
const client = await this.getClient(); const client = await this.getClient();
const payload = typeof data === 'string' ? data : JSON.stringify(data); const payload = typeof data === 'string' ? data : JSON.stringify(stringifyIds(data));
try { try {
client.publish(subject, this.textEncoder.encode(payload)); client.publish(subject, this.textEncoder.encode(payload));
@ -83,7 +84,7 @@ class NatsServer {
async request(subject, data, timeout = 30000) { async request(subject, data, timeout = 30000) {
const client = await this.getClient(); const client = await this.getClient();
const payload = typeof data === 'string' ? data : JSON.stringify(data); const payload = typeof data === 'string' ? data : JSON.stringify(stringifyIds(data));
try { try {
const response = await client.request( const response = await client.request(
@ -197,7 +198,7 @@ class NatsServer {
const responsePayload = const responsePayload =
typeof response === 'string' typeof response === 'string'
? response ? response
: JSON.stringify(response); : JSON.stringify(stringifyIds(response));
msg.respond(this.textEncoder.encode(responsePayload)); msg.respond(this.textEncoder.encode(responsePayload));
} catch (err) { } catch (err) {
logger.error( logger.error(

169
src/database/objectid.js Normal file
View File

@ -0,0 +1,169 @@
const PURE_REF_KEYS = new Set(['_id', 'id', 'buffer', '$oid']);
function bytesOf(value) {
if (value == null) return null;
if (typeof Buffer !== 'undefined' && Buffer.isBuffer(value)) {
return value.length === 12 ? value : null;
}
if (value instanceof Uint8Array) {
return value.byteLength === 12 ? Buffer.from(value) : null;
}
if (
value &&
value.type === 'Buffer' &&
Array.isArray(value.data) &&
value.data.length === 12
) {
return Buffer.from(value.data);
}
if (typeof value !== 'object' || Array.isArray(value) || value instanceof Date) {
return null;
}
const keys = Object.keys(value);
if (keys.length !== 12) return null;
const bytes = Buffer.alloc(12);
for (let index = 0; index < 12; index += 1) {
if (!Object.prototype.hasOwnProperty.call(value, index)) return null;
const byte = Number(value[index]);
if (!Number.isInteger(byte) || byte < 0 || byte > 255) return null;
bytes[index] = byte;
}
return bytes;
}
// Socket.io's binary encoder copies BSON ObjectIds into `{ buffer: <12 bytes> }`
// whenever the same packet also contains a Buffer. Accept that shape, hex
// strings, `{ _id }`, and `{ $oid }` as the same id.
export function refId(value) {
if (value == null || value === '') return null;
if (typeof value === 'string') return value;
if (typeof value === 'object' && typeof value.toHexString === 'function') {
return value.toHexString();
}
if (typeof value !== 'object' || Array.isArray(value) || value instanceof Date) {
return null;
}
if (typeof value.$oid === 'string') return refId(value.$oid);
const keys = Object.keys(value);
if (
keys.length > 0 &&
keys.every(key => key === 'buffer' || key === 'id' || key === 'type' || key === 'data')
) {
const bytes = bytesOf(value.buffer) || bytesOf(value.id) || bytesOf(value);
if (bytes) return bytes.toString('hex');
}
if (value._id != null) return refId(value._id);
return null;
}
const BINARY_TYPES = new Set(['Buffer', 'Uint8Array', 'ArrayBuffer', 'Blob', 'File']);
function isBinaryPayload(value) {
if (value == null || typeof value !== 'object') return false;
if (typeof Buffer !== 'undefined' && Buffer.isBuffer(value)) return true;
if (value instanceof Uint8Array || value instanceof ArrayBuffer) return true;
const name = value.constructor?.name;
return BINARY_TYPES.has(name);
}
// Object ids go out as 24-character hex strings. Binary file payloads stay binary.
export function stringifyIds(value) {
if (value == null) return value;
if (typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean') {
return value;
}
if (typeof value.toHexString === 'function') return value.toHexString();
if (value instanceof Date || typeof value === 'function') return value;
if (isBinaryPayload(value)) return value;
if (Array.isArray(value)) return value.map(item => stringifyIds(item));
if (typeof value === 'object') {
if (isPureRef(value)) {
const id = refId(value);
if (typeof id === 'string') return id;
}
const copy = {};
for (const [key, child] of Object.entries(value)) {
copy[key] = stringifyIds(child);
}
return copy;
}
return value;
}
export function isPureRef(value) {
if (value == null || typeof value !== 'object') return false;
if (typeof value.toHexString === 'function') return true;
if (Array.isArray(value) || value instanceof Date) return false;
const keys = Object.keys(value);
return keys.length > 0 && keys.every(key => PURE_REF_KEYS.has(key)) && refId(value) != null;
}
function normalizeOperatorValue(value) {
const next = { ...value };
for (const operator of ['$in', '$nin']) {
if (Array.isArray(next[operator])) {
next[operator] = next[operator].map(item => refId(item) ?? item);
}
}
if (next.$eq != null) next.$eq = refId(next.$eq) ?? next.$eq;
if (next.$ne != null) next.$ne = refId(next.$ne) ?? next.$ne;
return next;
}
function normalizeFilterValue(value) {
if (Array.isArray(value)) return value.map(item => refId(item) ?? item);
if (isPureRef(value)) return refId(value);
if (
value &&
typeof value === 'object' &&
!(value instanceof Date) &&
('$in' in value || '$nin' in value || '$eq' in value || '$ne' in value)
) {
return normalizeOperatorValue(value);
}
return value;
}
export function normalizeFilter(filter) {
if (!filter || typeof filter !== 'object' || Array.isArray(filter)) {
return {};
}
const normalized = {};
for (const [key, value] of Object.entries(filter)) {
const targetKey = key.endsWith('._id') ? key.slice(0, -4) : key;
normalized[targetKey] = normalizeFilterValue(value);
}
return normalized;
}
function isObjectIdSchemaPath(schemaType) {
if (!schemaType) return false;
if (schemaType.instance === 'ObjectId' || schemaType.instance === 'ObjectID') {
return true;
}
const caster =
schemaType.caster ?? schemaType.$embeddedSchemaType ?? schemaType.embeddedSchemaType;
return caster?.instance === 'ObjectId' || caster?.instance === 'ObjectID';
}
export function coerceObjectIds(model, data) {
if (!data || typeof data !== 'object' || Array.isArray(data) || !model?.schema) {
return data;
}
const coerced = { ...data };
for (const [key, value] of Object.entries(data)) {
if (!isObjectIdSchemaPath(model.schema.path(key)) || value == null) continue;
if (Array.isArray(value)) {
coerced[key] = value.map(item => refId(item) ?? item);
} else {
coerced[key] = refId(value) ?? value;
}
}
return coerced;
}

View File

@ -6,6 +6,7 @@ import {
aggregateRollups, aggregateRollups,
aggregateRollupsHistory, aggregateRollupsHistory,
} from '../../database.js'; } from '../../database.js';
import { refId } from '../../objectid.js';
const { Schema } = mongoose; const { Schema } = mongoose;
const parentStockModelNames = { const parentStockModelNames = {
@ -60,10 +61,10 @@ const buildParentState = (parentType, parentStock, currentAmount, startingAmount
}; };
const getStockEventTotal = async (parentId, parentType) => { const getStockEventTotal = async (parentId, parentType) => {
if (!parentId) return null; const id = refId(parentId);
if (typeof id !== 'string' || !/^[a-fA-F\d]{24}$/.test(id)) return null;
const objectId = const objectId = new mongoose.Types.ObjectId(id);
parentId instanceof mongoose.Types.ObjectId ? parentId : new mongoose.Types.ObjectId(parentId);
const [result] = await mongoose const [result] = await mongoose
.model('stockEvent') .model('stockEvent')

View File

@ -1,4 +1,5 @@
import { ObjectId } from 'mongodb'; import { ObjectId } from 'mongodb';
import { refId } from './objectid.js';
import { auditLogModel } from './schemas/management/auditlog.schema.js'; import { auditLogModel } from './schemas/management/auditlog.schema.js';
import { notificationModel } from './schemas/misc/notification.schema.js'; import { notificationModel } from './schemas/misc/notification.schema.js';
import { userNotifierModel } from './schemas/misc/usernotifier.schema.js'; import { userNotifierModel } from './schemas/misc/usernotifier.schema.js';
@ -701,48 +702,54 @@ function flatternObjectIds(object) {
return result; return result;
} }
function expandObjectIds(input) { function isIdValue(val) {
// Helper to check if a value is an ObjectId or a 24-char hex string
function isObjectId(val) {
// Check for MongoDB ObjectId instance
if (val instanceof ObjectId) return true; if (val instanceof ObjectId) return true;
// Check for exactly 24 hex characters (no special characters) if (val && typeof val === 'object' && typeof val.toHexString === 'function') return true;
if (typeof val === 'string' && /^[a-fA-F\d]{24}$/.test(val)) return true; if (typeof val === 'string' && /^[a-fA-F\d]{24}$/.test(val)) return true;
return false; return false;
} }
// Recursive function function idString(val) {
if (typeof val === 'string') return val;
if (val && typeof val.toHexString === 'function') return val.toHexString();
return refId(val);
}
function expandObjectIds(input) {
// Recursive function. Ids are always hex strings, never ObjectId buffers.
function expand(value) { function expand(value) {
if (isIdValue(value)) {
return { _id: idString(value) };
}
if (Array.isArray(value)) { if (Array.isArray(value)) {
return value.map(expand); return value.map(expand);
} else if ( }
value && if (value && typeof value === 'object' && !(value instanceof Date)) {
typeof value === 'object' && const result = {};
!(value instanceof ObjectId)
) {
var result = {};
for (const [key, val] of Object.entries(value)) { for (const [key, val] of Object.entries(value)) {
if (key === '_id') { if (key === '_id') {
// Do not expand keys that are already named _id result[key] = isIdValue(val) ? idString(val) : val;
result[key] = val; } else if (isIdValue(val)) {
} else if (isObjectId(val)) { result[key] = { _id: idString(val) };
result[key] = { _id: val };
} else if (Array.isArray(val)) { } else if (Array.isArray(val)) {
result[key] = val.map(expand); result[key] = val.map(expand);
} else if (val instanceof Date) { } else if (val instanceof Date) {
result[key] = val; result[key] = val;
} else if (val && typeof val === 'object') { } else if (val && typeof val === 'object') {
result[key] = expand(val); const nestedId = refId(val);
const keys = Object.keys(val);
const bufferOnly =
nestedId &&
keys.length > 0 &&
keys.every(item => item === 'buffer' || item === 'id' || item === '$oid');
result[key] = bufferOnly ? { _id: nestedId } : expand(val);
} else { } else {
result[key] = val; result[key] = val;
} }
} }
return result; return result;
} else if (isObjectId(value)) {
return { _id: value };
} else {
return value;
} }
return value;
} }
return expand(input); return expand(input);

View File

@ -163,6 +163,7 @@ export class SocketHost {
} }
async handleNewObject(data, callback) { async handleNewObject(data, callback) {
try {
const object = await newObject({ const object = await newObject({
model: getModelByName(data.objectType), model: getModelByName(data.objectType),
newData: data.newData, newData: data.newData,
@ -170,9 +171,14 @@ export class SocketHost {
ownerType: 'host' ownerType: 'host'
}); });
callback(object); callback(object);
} catch (error) {
logger.error('newObject failed:', error);
callback({ error: error.message });
}
} }
async handleEditObject(data, callback) { async handleEditObject(data, callback) {
try {
const object = await editObject({ const object = await editObject({
model: getModelByName(data.objectType), model: getModelByName(data.objectType),
id: data._id, id: data._id,
@ -184,9 +190,14 @@ export class SocketHost {
notify: data?.notify notify: data?.notify
}); });
callback(object); callback(object);
} catch (error) {
logger.error('editObject failed:', error);
callback({ error: error.message });
}
} }
async handleGetObject(data, callback) { async handleGetObject(data, callback) {
try {
const object = await getObject({ const object = await getObject({
model: getModelByName(data.objectType), model: getModelByName(data.objectType),
id: data._id, id: data._id,
@ -194,9 +205,14 @@ export class SocketHost {
populate: data.populate populate: data.populate
}); });
callback(object); callback(object);
} catch (error) {
logger.error('getObject failed:', error);
callback({ error: error.message });
}
} }
async handleListObjects(data, callback) { async handleListObjects(data, callback) {
try {
const object = await listObjects({ const object = await listObjects({
model: getModelByName(data.objectType), model: getModelByName(data.objectType),
id: data._id, id: data._id,
@ -208,6 +224,10 @@ export class SocketHost {
order: data?.order order: data?.order
}); });
callback(object); callback(object);
} catch (error) {
logger.error('listObjects failed:', error);
callback({ error: error.message });
}
} }
async handleObjectEventEvent(data) { async handleObjectEventEvent(data) {

View File

@ -6,6 +6,7 @@ import { loadConfig } from '../config.js';
import { SocketUser } from './socketuser.js'; import { SocketUser } from './socketuser.js';
import { UpdateManager } from '../updates/updatemanager.js'; import { UpdateManager } from '../updates/updatemanager.js';
import { SocketHost } from './sockethost.js'; import { SocketHost } from './sockethost.js';
import { stringIdParser, useStringIds } from './stringidparser.js';
const config = loadConfig(); const config = loadConfig();
@ -21,6 +22,7 @@ export class SocketManager {
// Create Socket.IO server - CORS applies to HTTP long-polling transport // Create Socket.IO server - CORS applies to HTTP long-polling transport
const allowedOrigins = config.server.corsOrigins || ['*']; const allowedOrigins = config.server.corsOrigins || ['*'];
const io = new Server(server, { const io = new Server(server, {
parser: stringIdParser,
maxHttpBufferSize: 1e8, maxHttpBufferSize: 1e8,
cors: { cors: {
origin: (origin, callback) => { origin: (origin, callback) => {
@ -37,6 +39,11 @@ export class SocketManager {
}); });
// Handle user connections // Handle user connections
io.use((socket, next) => {
useStringIds(socket);
next();
});
io.on('connection', async socket => { io.on('connection', async socket => {
const authType = socket.handshake?.auth?.type; const authType = socket.handshake?.auth?.type;
if (authType == 'user') { if (authType == 'user') {

View File

@ -128,7 +128,11 @@ export class SocketUser {
} else { } else {
logger.warn('User is not authenticated.'); logger.warn('User is not authenticated.');
} }
if (typeof callback === 'function') {
callback(result); callback(result);
} else {
this.socket.emit('authenticate_result', result);
}
} }
} }
@ -384,7 +388,10 @@ export class SocketUser {
return; return;
} }
logger.info('Cancelling scan sessions on user disconnect:', sessions.length); logger.info(
'Cancelling scan sessions on user disconnect:',
sessions.length
);
await Promise.all( await Promise.all(
sessions.map(([sessionId, hostId]) => sessions.map(([sessionId, hostId]) =>

View File

@ -0,0 +1,30 @@
import { createRequire } from 'node:module';
import { stringifyIds } from '../database/objectid.js';
const require = createRequire(import.meta.url);
const { Encoder, Decoder } = createRequire(require.resolve('socket.io'))(
'socket.io-parser'
);
class StringIdEncoder extends Encoder {
encode(packet) {
if (packet?.data == null) return super.encode(packet);
return super.encode({ ...packet, data: stringifyIds(packet.data) });
}
}
export const stringIdParser = {
Encoder: StringIdEncoder,
Decoder
};
export function useStringIds(socket) {
socket.use((packet, next) => {
for (let index = 1; index < packet.length; index += 1) {
if (typeof packet[index] !== 'function') {
packet[index] = stringifyIds(packet[index]);
}
}
next();
});
}