find by ObjectID fails

[myartsev]

Related to #5; if the ID field is an ObjectID, find fails.

For example:
http://localhost:1337/foo/59c84caa76f5f2066ae21d9a
http://localhost:1337/foo/_bsontype,ObjectID,id,Y%EF%BF%BDL....

The id field is always treated as a string in the resulting query.

fortune-mongodb/lib/index.js

Line 122 in 7e8c626

if (ids && ids.length) query[idKey] = { $in: ids }

There is not much to differentiate a serialized ObjectID from a normal string.

One approach would be to attempt to de-serialize the ID into an ObjectID.
If this de-serialization is successful, add the resulting object as an alternative to the lookup query.

    if (ids && ids.length) {
      ids.forEach(id => {
        var objectId
        try {
          objectId = ObjectID(id)
          ids.push(objectId)
        }
        catch(e) { /* id cannot be converted into a valid ObjectID */ }
      })

      query[idKey] = { $in: ids }
    }

This works and all variations are covered:

• http://localhost:1337/foo/59c84caa76f5f2066ae21d9a

  • where the ID could be a valid ObjectID
  • where the ID could be a string

• http://localhost:1337/foo/bar
  or any other string including the IDs that get generated by Fortune itself.

Thoughts?

[gr0uch]

This handles deserialization of ObjectIDs, also need to serialize any client-side provided IDs into ObjectIDs.

[myartsev]

Yes, see #22 for the serialization, I ran into this afterwards.

[gr0uch]

There needs to be a configuration option whether to use ObjectIDs globally or not. The problem is that they can not be mixed, since one must always cast strings to ObjectIDs.

[myartsev]

Agreed, I tried to make it work as a mix, but for various reasons I no longer think that is a good idea / possible.

I was running out of time and I ended up switching over to using strings instead of ObjectIDs for the time being. A configuration to use Object IDs globally is a good idea!