vadimdemedes / mongorito

🍹 MongoDB ODM for Node.js apps based on Redux
1.38k stars 90 forks source link

Serialize doesn't check type of value #175

Closed EdenCoder closed 7 years ago

EdenCoder commented 7 years ago

when you pass null to lib/serialize.js it will fail to map the object, we need to check if the value is null.

'use strict';

const mapObject = require('map-obj');

module.exports = obj => {
    return mapObject(obj, (key, value) => {
        if (typeof value.toBSON === 'function') {
            value = value.toBSON();
        }

        if (Array.isArray(value) && value[0] && typeof value[0].toBSON === 'function') {
            value = value.map(item => item.toBSON());
        }

        return [key, value];
    });
};

Should become

'use strict';

const mapObject = require('map-obj');

module.exports = obj => {
    return mapObject(obj, (key, value) => {
                if (!value) return [key, value];

        if (typeof value.toBSON === 'function') {
            value = value.toBSON();
        }

        if (Array.isArray(value) && value[0] && typeof value[0].toBSON === 'function') {
            value = value.map(item => item.toBSON());
        }

        return [key, value];
    });
};

I can create a pull request if it's easier, I already did for the string to object ID issue within findById

vadimdemedes commented 7 years ago

Under which condition would serialize() receive null as an input?

EdenCoder commented 7 years ago

Migrating from mongorito@2.x to mongorito@3.x, there are cases in mongorito v2 where null would be passed as a parameter, for example an empty array is inserted as null.

vadimdemedes commented 7 years ago

Not sure I entirely understand the case you're describing. Could you submit a PR, which contains only the failing test to clearly demonstrate the problem? Would be very helpful!

EdenCoder commented 7 years ago

@vadimdemedes

When you insert an empty array into a field, it's set in the db as null

hlandao commented 7 years ago

@vadimdemedes when a property in mongo is set to null we might get null in serialize

const user = User.findOne({})
user.set('someKey', 'someVal');
user.save()  // if user.anotherKey was null, an exception is raised.
vadimdemedes commented 7 years ago

https://github.com/vadimdemedes/mongorito/pull/182 should fix it. Waiting for @hlandao to fix linter errors and will publish a patch update immediately.

EdenCoder commented 7 years ago

@vadimdemedes this is still breaking some of my production builds, have resorted to editing that file everywhere...

Any chance you can merge that?