Add bookshelf validator for PUT/PATCH methods on values in multi-column unique indices

[agerbens]

I wrote a validator a while back that I've used on a couple projects now that is useful for preventing database constraint errors on PUT/PATCH methods changing values in multi-column unique indices. Making this issue to see if we want to include something like it in here.

Here's the basic layout, which could be cleaned up a bit to use array helpers and get rid of lodash:

const _ = require('lodash');

module.exports = (bookshelf, ValidationError) => {
  return (modelName, column, otherWheres, notWheres, message) => {
    return (value, options) => {
      return new Promise((resolve, reject) => {
        const Model = bookshelf.model(modelName);
        const where = {};

        where[column] = value;

        const query = Model.where(where);

        _.forEach(otherWheres, (contextValuePath, column) => {
          const contextValue = _.get(options.context, contextValuePath);
          if (typeof contextValue !== 'undefined') {
            query.where(column, '=', contextValue);
          }
        });

        _.forEach(notWheres, (contextValuePath, column) => {
          const contextValue = _.get(options.context, contextValuePath);
          if (typeof contextValue !== 'undefined') {
            query.where(column, '!=', contextValue);
          }
        });

        query
          .count()
          .then(count => {
            if (count) {
              reject(new ValidationError(message || 'Value must be unique', 'uniqueInContext'));
            } else {
              resolve(value);
            }
          })
          .catch(err => {
            reject(err);
          });
      });
    };
  };
};

An example of it being used is as follows:

{
  payloadProperty: uniqueInContext(
    'model_name',
    'column_name',
    {
      other_columns_in_unique_index: 'path.to.identifier.in.params.or.payload',
    },
    {
      identifiers_for_rows_you_want_to_ignore: 'path.to.identifier.in.params.or.payload',
    },
    'column_name already used'
  )
}

The first object is a list of other columns in the unique index, the 2nd object can be used to identify / ignore a row you want, so you can PUT / PATCH the same column_name

[spruce-bruce]

Seems useful. Could rowNotExistsWhere be updated to do this instead of including a new validator? I wouldn't want to complicate the rowNotExistsWhere signature too much so it might be better separate.

[agerbens]

It's essentially a more flexible rowNotExistsWhere that also allows where not equal to clauses. But like you said, I wouldn't want to rewrite the signature for rowNotExistsWhere. That would definitely be a breaking change / major version bump.

[spruce-bruce]

IMO the major version bump isn't a reason not to do something... it's more like a UX thing.

The api to rowNotExistsWhere is already complicated and easy to mess up and if we make it even more flexible then it gets even harder to use.

I floated an idea that we could have a "bookshelfQuery" validator that just constructs a query that you specify somehow. I couldn't think of a good api for that validator so I dropped it, but it would act as a sort of catchall validator. If your case isn't covered by one of the validators we've built then use bookshelfQuery() to validator you thing. That's sort of tangential and doesn't really impact what we're talking about too much.

I think adding the multi-column-unique (or whatever) validator doesn't seem wrong to me. You've already got code and I definitely wouldn't stonewall this addition. Just need to get this on a release at some point and write a test or two.

I'd like to see it written without the full lodash dependency and just importing get, but that's more of a nitpick.

[areida]

I like aarons approach of a generalized queryValidator, something like

const get = require('lodash/get');

const getObjectionOptions = require('./get-objection-options');
const ValidationError = require('../ValidationError');

module.exports = (Model, wheres = [], message = 'Invalid query', constraintOptions) =>
  async (value, validatorOptions) => {
    const options = getObjectionOptions(validatorOptions, constraintOptions);
    const query = Model.query();

    if (Array.isArray(wheres)) {
      wheres.forEach(where => {
        query.where(where[0], where[1], get(validatorOptions.context, where[2]));
      });
    }

    const rows = await query
      .eagerAlgorithm(Model[options.fetchOptions.eagerAlgorithm])
      .eagerOptions(options.fetchOptions.eagerOptions)
      .eager(options.fetchOptions.eager);

    if (options.shouldExist && !rows.length || rows.length) {
      if (options.shouldExist && options.return404) {
        throw Boom.notFound(message);
      } else {
        throw new ValidationError(message, 'queryValidator');
      }
    }

    return (options.shouldExist && options.convert) ? rows[0] : value;
  };

{
  id: queryValidator(User, [['id', '=', 'params.id']], 'Invalid user'),
}