Discussion: No way to do safe update?

[FossPrime]

feathers-mongodb/lib/index.js

Line 219 in fa08323

.then(() => this._findOrGet(id, params))

suppose one ran

service('user').update('id1', { _id: 'id1', updatedAt: '2020' }, { query: {
  updatedAt: '2019-01-01T12:00:00.000Z'
})

The call to mongo would move along fine... but we would get an error after the update has been made because the object changed and the findOrGet would fail. I want to add that protection because I've been having issues with two simultaneous update calls cancelling each other out without warning.

This is a hard problem for security reasons. The only way I see forward is to support $noFetch, which would allow you to receive a write success, or an unambiguous write error. Right now we get a 404 not found error, it's not possible to tell if the error comes from the findOrGet or from the replaceOne call... there is a big difference.

Another option is to have a setting for freeGetFields, these fields would be removed from the findOrGet calls after the update is made. Safe fields like updatedAt, hash, checksum, etc would be removed before the findOrGet call. This would still not work for a safe full document update, where every field is checked for changes by mongo before it is modified.

Perhaps the answer is staring at us write in the face... just return what was given to us, this is an update call after all.

Update: Tried the last option... it hides the errors, but update does not throw an error if nothing changed. :/

// update without making a find call after the update
  _update (id, data, params = {}) {
    if (Array.isArray(data) || id === null) {
      return Promise.reject(
        new errors.BadRequest('Not replacing multiple records. Did you mean `patch`?')
      );
    }

    const { query, options } = super._multiOptions(id, params);

    return this.Model.replaceOne(query, super._normalizeId(id, data), options)
      .then(() => data)
      .then(select(params, this.id))
      .catch(super.errorHandler);
  }

Update #2:

Another option is to $errorOnNoOp. But that would be bespoke and break adapter agnosticism.
Instead of all that... I'll have a bespoke safety check field... than do an $or query on it, for the old value and the new value. Not as safe, but not as disruptive to the architecture.

Update #3:

the updatedAt: { $in: [t1, t2] } solution works well... but it could be better. Separate params.$updateQuery and params.$findQuery param support would allow a great deal of assurance and flexibility, without taking it all the way up to transaction locking and losing all db agnosticism. This should be an adapter level proposal. $softLock: sha256, $softLockSource: 'before' would be fine for sensitive situations.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Apologies if the issue could not be resolved. FeathersJS ecosystem modules are community maintained so there may be a chance that there isn't anybody available to address the issue at the moment. For other ways to get help see here.