fortunejs/fortune-mongodb

MongoDB adapter for Fortune.

fortune-mongodb's Issues

id field is ObjectID

Did an update form an older version of fortune-mongodb to the latest version, and it appears that there has been a backwards incompatible(ish) change that the ID field returned in objects from the adapter is the raw ObjectID type from the DB. Previously the ID was cast to a string before the application consumed it.

Network dropouts

After a period of network interruption that causes Fortune to disconnect from the DB, the adapter does not attempt to re-connect when the DB is available again.

No errors but no action

I've got Fortune working with JSON-API and that worked very seamlessly but when trying to add the mongodb plugin it seems to never engage. By "engage" I mean I am watching the mongo logs and no connection attempt seems to be made (either initially or when I add data through POST calls to the API). Furthermore, while Fortune is clearly collecting state it never gets persisted to Mongo.

Here's the test script I'm using:

const fortune = require('fortune');
const MongodbAdapter = require('fortune-mongodb');
const Fortune = fortune({
  adapter: {
    type: MongodbAdapter,
    options: {
      url: 'mongodb://localhost:27017/test',
      typeMap: {
        user: 'users',
        group: 'groups'
      }
    }
  }
});
const http = require('http');
const jsonApi = require('fortune-json-api');
const store = fortune({
  serializers: [{
    type: jsonApi,
    options: {},
  }]
});

const listener = fortune.net.http(store);
const server = http.createServer(listener);
const port = 1337;

store.defineType('user', {
  username: {
    type: String,
  },
  key: {
    type: Buffer,
  },
  salt: {
    type: Buffer,
  },
  group: {
    link: 'group',
    inverse: 'users',
    isArray: true,
  },
});

store.defineType('group', {
  name: {
    type: String
  },
  users: {
    link: 'user',
    inverse: 'group',
    isArray: true
  },
});

store.connect().then(() => {
  server.listen(port)
  console.log(`Server is listening on port ${port}...`)
})

I'm using Mongo for the first time today so possibly I'm missing something obvious but would LOVE some help in getting this working.

Connection to Replica Sets

The current design forces you to specify a singular DB server to connect to. If you specify a server that is not the current Primary in the replica set, the adapter does not discover the current Primary from the given Secondary.

DeprecationWarning: current URL string parser is deprecated, and will be removed in a future version. To use the new parser, pass option { useNewUrlParser: true } to MongoClient.connect

Hi,
I know this is just warning but as it states this will eventually mean this adapter will be incompatible with newer version of mongodb.

I know the way to address this is by emitting the db name from the url and passing as a parameter to the Mongo instance decleration.

I have done this before but would be trickier to implement here as I don't know this codebase. I'm willing to take a look and commit though if a PR would be accepted?

Adapter can't handle ObjectID

When using the following option for generateId:

generateId: function(){
    return new mongodb.ObjectID().str;
}

Fortune will correctly save records with an ObjectID. When it tries to find the records the result is always empty. It most likely tries to do it with a string or hex representation of the ObjectId which results in 0 records which needs to be cast to an ObjectID before being sent to MongoDB.

Applicable error message from fortune:
A related record for the field "person" was not found.
With the test case of saving an item with an added relationship to the person.

find by ObjectID fails

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?

TODOs

Mostly new features:

• Implement beginTransaction and endTransaction using the $isolated operator. It doesn't provide rollbacks but it's better than nothing. Using native implementation for MongoDB 4.0+

How to create unique fields

Hi,

How can I create an unique field?

Using unique: true in the resource creation doesn't work.

Thanks!

Relationships references is wrong

The relationships references are wrong in this adapter and returns something like https://gist.github.com/jlcarvalho/bca2502ba26691cb03a7.

The attributes self and related outputs garbage.

Support mongo DB clusters

The current mongo DB adapter doesn't suppport connecting to a mongoDB cluster.

I think the best option is to override the mongo connection adapter, mongodb.js:

this.db = mongoose.createConnection('mongodb://' + (options.username ? options.username + ':' + options.password + '@' : '') + options.host + (options.port ? ':' + options.port : '') + '/' + options.db, options.flags);

To allow the connection string to be specified if desired instead:

var connectionString = options.connectionString;

if (!connectionString) {
connectionString = 'mongodb://' +(options.username ? options.username + ':' + options.password + '@' : '') +
options.host + (options.port ? ':' + options.port : '') + '/' + options.db,
options.flags;
}

console.log("Connection string : %s", connectionString);

//Setup mongoose instance
this.db = mongoose.createConnection(connectionString);

Weird ids

I got this generated id: /f9XL5u+VDM426bToRc1.
So, when i tried to access GET /people//f9XL5u+VDM426bToRc1 i got:

{
    "errors": [
        {
            "title": "NotFoundError",
            "detail": "The field \"f9XL5u+VDM426bToRc1\" is not a link on the type \"people\"."
        }
    ]
}

After some investigation i found this function:

export function generateId () {
  return crypto.randomBytes(15).toString('base64')
}

Running in node shell:

Sometimes base64 returns the / char.
Use hex instead of base64 would be a quick fix.

crypto.randomBytes(15).toString('hex')

Not founding resources

The current version doesn't found the resources on mongo 3.0, it occurs because the adapter uses IDs as strings and not as ObjectID.

Replace ids for ids.map((id)=>new ObjectID(id)) should fix.

Expose the Aggregation framework.

Certain types of queries need the Aggregation framework in order to be constructed.

It would be great if this adapter exposed it and allowed Fortune to utilise it via Fortune.adapter.aggregate() (with the caveat that it is specific to this adapter).

Specify collection name.

I'm not understanding why the type must match the collection name. If I create a type called "card", I would expect the collection to be called "cards". Is there a way to transform or specify the collection for the type "card"?

DeprecationWarning: collection.count is deprecated, and will be removed in a future version. Use collection.countDocuments or collection.estimatedDocumentCount instead

Again just another potential issue with future versions that can be fixed with just a code change (just renaming a key in this instance).

Support for embedded docs (schemas)

Tried to nest a schema to create an embedded doc. Is this only supported for schema-less embedded docs with an array?

Implement `range` and `exists`

Upstream has new query options.

Remove Babel

Babel considered harmful.