feathers

feathers copied to clipboard

The update method is currently breaking when using data that affects the query.

For example,

const dave = await app.service('people').create({ name: 'Dave' })

// Now mutate AND query the name property.
const result = await app
  .service('people')
  .update(dave._id, { name: 'Marshal' }, { query: { name: 'Dave' } })

The problem originates here: https://github.com/feathersjs/feathers/blob/fe072a26b64eaab1d659bc94ba98a404de663e1f/packages/mongodb/src/adapter.ts#L376

We have updated the data, but then this following _findOrGet is using the original query. Because our data mutated a value in that query, the request throws a NotFound (even though the record was updated); This likely needs to be handled similarly to patch/remove.

I am currently working on a mongoJoinQuery for a project that will ultimately end up in feathers-fletching. So I am taking a deep dive in the new Mongo adapter. I will eventually create a PR for this problem and any others I stumble across.

Furthermore, this should probably be a test in the common adapters test suite.

Ah, same problem we had with .patch where it now uses a list of original ids instead. Since update doesn't support multiple updates, should it just remove the query?

return this._findOrGet(id, {
  ...params,
  query: {}
}).catch(errorHandler)

I like the query because it throws a NotFound actually. I often use this for some security benefits where I stub on some extra params in the query serverside.

// Serverside hook after auth hook
const permissionsHook = (context) => {
  context.params.query.organizationId = context.params.user.organizationId
}

// This will throw a NotFound it item with id: 1 is not in my organization
const item = await app.service('items').get(1)

I am taking a deep dive on this new version of the adapter because I need some fixes/features for a particular project. See: https://github.com/feathersjs/feathers/compare/dove...DaddyWarbucks:feathers:mongodb/aggregate

So I will open a PR soon with

• Whatever fix we decide for this problem
• Allow aggregation in the _get method
• Possibly allow params.mongodb to be used with the aggregation framework
• Whatever else I stumble across

Closed via https://github.com/feathersjs/feathers/pull/3366