carbon-io / carbond

MIT License
2 stars 5 forks source link

Options argument passed to findOne gets interpreted as the projection argument #276

Closed tfogo closed 6 years ago

tfogo commented 6 years ago

Say you create subendpoints using MongoDB collections. The endpoints look like this:

/user/:user/contacts/:_id

Attempting to GET an endpoint such as /users/59fb9c861bbbeb40a278800f/contacts/59fb9d001bbbeb40a2788011 will result in an error:

MongoError: Projection cannot have a mix of inclusion and exclusion.

The reason for this is because in Collection.preFindobjectOperation, the options object that is passed to MongoDBCollection.findObject is created. It takes all the properties from req.parameters. In this case it's { projection: undefined, user: "59fb9c861bbbeb40a278800f" }.

This causes an error because the mongodb driver's find method takes three arguments: query, fields, and options. We pass in query and options. So by default, mongodb thinks our options document is a projection document. This is usually okay because if the driver detects any supported options properties in fields, it will assume it's the options parameter instead.

In our case, the options object we pass doesn't contain any supported options properties. It just has user and projection. The mongodb driver fails to parse this as a projection document because it sees it as { projection: 0, user: 1 } which is an illegal projection object.

Solution

Neither the idPathParameter nor the projection property should be in the options object we send to the mongo driver. The idPathParameter (user) does nothing. If a coder names their idPathParameter "hint", or "snapshot", or "limit", it could cause errors by clashing with actual option properties. See NODE-794.

Carbon already correctly renames projection to fields if it contains a projection document. If we rename it to fields even when it's undefined, we'll ensure our options document always gets correctly interpreted as the options argument.

tfogo commented 6 years ago

277 renames projection to fields. It also uses options.driverOptions to be consistent with other collection operations.

Still need to decide what to do with the idPathParameter finding its way into the driver options. @gregbanks you mentioned possibly an excludeParameters property on Collection.CollectionOperationConfig.

tfogo commented 6 years ago

Fixed by Greg