koa-mongoose icon indicating copy to clipboard operation
koa-mongoose copied to clipboard

Logic Error

Open lmmarsano opened this issue 6 years ago • 1 comments

I wasn't sure exactly what your package did, so I read the source code and spotted a logical programming error: https://github.com/Jackong/koa-mongoose/blob/676741f1676f3104fa320af5d6a6e6f0609f1ab6/lib/mongoose.js#L62 If !options is true, then options is falsy, so it won't have properties and the entire condition is true. If the entire condition is true, then !options is true. In other words, the following are logically equivalent

  • !options
  • !options && (!options.host || !options.port) && !options.uri

This means your condition is actually only testing !options: probably not your intent.

To see this is a problem, we can refactor the domain logic from the rest of your code

const mongoUri = (options) => {
    if (!options && (!options.host || !options.port) && !options.uri) {
        throw new Error('options not found')
    }

    var database = typeof options.database === 'function' ? undefined : options.database

    return options.uri || `mongodb://${options.user ? options.user + ':' + options.pass + '@':''}${options.host}:${options.port}${database ?'/' + database : ''}`
}

and observe

mongoUri({})
\\ "mongodb://undefined:undefined"
mongoUri({host: 'host'})
\\ "mongodb://host:undefined"
mongoUri({port: 0})
\\ "mongodb://undefined:0"

To reason about negations more easily, I apply De Morgan's law to factor them out. The condition I think you were aiming for is

 if (!(options && (options.host && options.port || options.uri))) { 

lmmarsano avatar Jul 15 '18 21:07 lmmarsano

Since this package hasn't been updated in a while I made an updated version of this package where this logic issue isn't present and made a few other changes too - https://github.com/South-Paw/koa-mongoose 👍

South-Paw avatar Jun 13 '19 06:06 South-Paw