I created a beforeFind lifecycle callback to allow a user to always modify a model's find or findOne queries with some criteria. A good use case would be if you're using a soft-delete system and you'd like to tack on an isDeleted:false to any find query on a model. Please feel free to critique the code/suggest changes, I tried to stick to the style that already exists.

While Q is a great library for Promises, Bluebird promises are almost as fast as callbacks, and even faster than callbacks when using generators, while Q tends to take up a lot of extra memory and runs much slower. Bluebird also has much more detailed stack traces.

Changes:

• Replace Q with Bluebird
• Replace .fail with .catch

This PR makes it so that:

• You can view the diff on a model
• You don't get afterUpdate to trigger on create
• You don't get an update at all, on create (because there's no diff)
• .save() only updates changed fields, in stead of everything
• There's no association cache anymore
• The original values are retrievable on the model.

Notes to self

• ~~.compareModelValues() in save.js can probably go.~~
• ~~.findPrimaryKey() can probably go. Context has a primaryKey property.~~
• ~~Don't forget to set status back to clean after various save operations.~~

About

Support Multitenancy by using dynamically loaded Tenant-specific collections and their corresponding connections.

Example App with Multitenancy

See https://github.com/Glavin001/sails-multitenancy-example

How It Works

The call to Collection.tenant will return a tenant-specific collection. This collection has a scoped identity for each tenant and the tenant name could be a number of string. For instance, Collection with identity test would becoming test<tenant:1> for Test.tenant(1).

The Tenant-specific Collection also has it's own Tenant-specific Connection with an analogous naming convention: connectionName<tenant:tenantId>.

These new Tenant-specific collections and connections are loaded dynamically, on-demand, at runtime. This means that you:

1. do not require waiting to load all X number of connections for all of your tenants, and
2. do not even have to know at server startup what tenants are available. They could even be loaded from a database upon request, without reloading your server!

Related Issues

See #286 and https://github.com/balderdashy/sails-mongo/issues/130 and https://github.com/balderdashy/sails/issues/1857

Usage

Configure Connection

      'my_foo': {
        adapter: 'foo',
        host: 'localhost',
        port: 12345,
        database: 'default_database',

        // === New ===
        isMultiTenant: true, // Enable Multi-Tenancy feature
        defaultTenant: false, // false, <string>, or <number>
        getTenant: function(req, cb) { // For Sailsjs
          return cb(req.params.tenant);
        },
        configForTenant: function(tenantId, config, cb) { // For Waterline
          // Validate Tenant
          tenantId = parseInt(tenantId);
          if (tenantId >= 1 && tenantId < 10) {
            config.database = "tenant-" + tenantId;
            return cb(null, config);
          } else {
            // 
            return cb(new Error("Invalid tenant " + tenantId + "!"));
          }
        }

      }

.tenant API

Currently Supported

• [x] Callback-style

      Collection
        .tenant("1", function(err, TenantCollection) {
          // Now you have a Tenant-specific Collection!
          TenantCollection
            .find({})
            .exec(function(err, results) {
              assert(results.length === 1, 'tenant-1 has 1 record');
              assert(results[0].message === db['tenant-1']['tests'][0].message, 'records are from the tenant-1 database');
              done();
            });
        });

• Promise-style, using
  • [x] Operations
    • Used by finders
  • [x] Deferred
    • Used by count, create, destroy, update

      Collection
        .tenant("1")
        .find({})
        .exec(function(err, result) {
          console.log(err, result);
          done();
        });

Tests:

See https://github.com/Glavin001/waterline/blob/multi-tenant/lib/waterline/query/dql/tenant.js

• [x] Callback-style API
  • [x] find
  • [x] findOne
  • [x] count
  • [x] create
  • [x] destroy
  • [x] update
• [x] Promise-style API (Operation/Deferred)
  • [x] find
  • [x] findOne
  • [x] count
  • [x] create
  • [x] destroy
  • [x] update

TODO:

• [x] Use existing Tenant connection, if previously loaded
• [x] Use existing Tenant Collection, if previously loaded
• [x] Remove debugging console.log
• Add support for Promise-style API (see above)
• Add support to Sailsjs Blueprints for using this Multi-Tenancy feature
  • Sailsjs would use the .getTenant(req, callback) as defined in the configuration
• [x] Improve documentation
• [ ] Joins/associations across multiple multi-tenant Collections
  • Apply the same tenant to both, if either have multi-tenant enabled
• [ ] Allow for the Adapter to change between Tenants
  • See https://github.com/Glavin001/waterline/blob/multi-tenant/lib/waterline/connections/index.js#L67 and https://github.com/Glavin001/waterline/blob/multi-tenant/lib/waterline/query/dql/tenant.js#L182

I think @Globegitter has been pretty busy lately, so this is an offer to implement just the linting part of https://github.com/balderdashy/waterline/pull/935. I am willing to go through and try to fix failing rules, but I would like to get consensus on what the rules should be prior to spending the effort. With the rules from https://github.com/balderdashy/waterline/pull/935 (currently in this PR), I get the following results:

(Note that all these screenshots are cropped on the right, a few bars are much longer)

Another strategy to avoid bikeshedding and arguing over rules, would be to use eslint-config-standard. With that set of rules, these are the results:

My suggestion would be to override the following rules to more closely align with the existing codebase:

"rules": {
  "space-before-function-paren":  [2, "never"],
  "space-after-keywords":         [2, "never"],
  "semi":                         [2, "always"]
}

The results of these three changes results in something pretty manageable:

What say ye?

This is the same as https://github.com/balderdashy/waterline/pull/954 but with the function signature changed to have cb as the last parameter.

All credit to @nkibler7.

As mentioned by @dmarcelino in https://github.com/balderdashy/waterline/pull/939 a PR against the 0.11 branch.

I know there is the current discussion going on about findOrCreateEach ( https://github.com/balderdashy/waterline/issues/931) so it might not have been the wisest choice to add a findAndModifyEach but I needed a findAndModify functionality (see http://docs.mongodb.org/manual/reference/method/db.collection.findAndModify/) and it seemed like a good solution to put it such a way that other people can use it as well.

Usage is Model.findAndModify(search critieras, values, [options, cb]);

where-as default options look like that:

{
  upsert: false, // on true automatically inserts the values if nothing is found
  new: true, // if false returns the model before it was updated/created
  mergeArrays: false // quite a special case that I needed and I am still testing in my app. When any of the values given contain an array it is merged to the found object rather than just replaced.
}

I added passing unit tests as well as adapter-tests, see https://github.com/balderdashy/waterline-adapter-tests/pull/47 There is something strange going on, even though the findAndModify passes the unit tests and works in my app (via npm i git+https://github.com/Globegitter/waterline.git#findAndModify) the adapter-tests do not pass and throw the same error throughout:

9) Semantic Interface .findAndModfiy() should return a model instance:
     TypeError: undefined is not a function
      at Context.<anonymous> (Git/waterline/node_modules/waterline-adapter-tests/interfaces/semantic/findAndModify.test.js:73:20)
      at Test.Runnable.run (Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runnable.js:194:15)
      at Runner.runTest (Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:355:10)
      at Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:401:12
      at next (Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:281:14)
      at Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:290:7
      at next (Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:234:23)
      at Immediate._onImmediate (Git/waterline/node_modules/waterline-adapter-tests/node_modules/mocha/lib/runner.js:258:5)
      at processImmediate [as _immediateCallback] (timers.js:361:17)

I have no idea why this happens or how to fix this. So apart from if this actually gets merged, anyone got any idea of why that could be?

Also it does not work completely in my app via sails-postgresql, I seem to be getting this error. I am not sure if that is related to my code or to sails-postgresql.

db_1     | ERROR:  syntax error at or near "LIMIT" at character 140
db_1     | STATEMENT:  UPDATE "subject" AS "subject" SET "researchIds" = $1, "text" = $2, "createdAt" = $3, "updatedAt" = $4  WHERE LOWER("subject"."text") = $5  LIMIT 1 RETURNING *

Waterline version: 0.13.0-3 Node version: v7.5.0 NPM version: 4.1.2 Operating system: macOS 10.12.3 (16D32)

Hello,

I'm trying to use latest version of waterline with SailsJS 1.0.0-22, and found a problem with Model.findOrCreate(). It behaves the same as Model.create() rather than Model.find(), i.e it requires {fetch: true} meta to return a created/found object.

Model.findOrCreate({ type: 'dog' }, { name: 'Pretend pet type', type: 'dog' }).then(console.log);
// undefined

Model.findOrCreate({ type: 'dog' }, { name: 'Pretend pet type', type: 'dog' }).meta({fetch: true}).then(console.log);
// { id: 1, name: 'Pretend pet type', type: 'dog' }

Pass criteria to the beforeUpdate, beforeDestroy, and beforeValidate callbacks, before the callback param.

In beforeValidate callbacks, criteria is null for create operations.

Serves as a solution to issue #1004.

NOTE: For backwards compatibility, it now checks if the beforeValidate or beforeCreate callback(s) only accept 3 arguments or beforeDestroy accepts only 2 and if so, passes the callback as the last argument and omits criteria.

Example usage and explanation:

// User.js
module.exports = {
  attributes: {
    email: { type: 'email', required: true, unique: true },
    password: { type: 'string', required: true }
  },

  /**
   * Hashes the password if it is provided
   * @param {Object} values The data submitted via the API
   * @param {Function} next The callback to invoke when done, with optional error
   */
  beforeCreate: function (values, next) {
    if (values.password === undefined) return next();
    hashPassword(values.password, function (err, hash) {
      if (err) return next(err);
      values.password = hash;
      next();
    });
  },

  /**
   * Hashes the password if it is provided, but only if the provided
   * password is not the user's existing password hash (safeguards
   * against the record.save() bug)
   * @param {Object} values The values to update
   * @param {Object} criteria The criteria to match the record(s) to update
   * @param {Function} next The callback to invoke when done, with optional error
   */
  beforeUpdate: function (values, criteria, next) {
    if (values.password === undefined) return next();

    // Look up user and only change password if values.password isn't the hash
    User.findOne(criteria).exec(function (err, user) {
      if (err) return next(err);
      if (!user) return next(new Error('User to update does not seem to exist!'));

      if (values.password === user.password) return next();

      hashPassword(values.password, function (err, hash) {
        if (err) return next(err);
        values.password = hash;
        next();
      });
    });
  },

  /**
   * Validates the password against constraints, but only if the user does
   * not exist, or the provided password is not the user's existing password
   * hash (safeguards against the record.save() bug)
   * @param {Object} values The values to update or insert
   * @param {Object|null} criteria The criteria to match the record(s) to
   *   update, or null if creating a new record
   * @param {Function} next The callback to invoke when done, with optional error
   */
  beforeValidate: function (values, criteria, next) {
    if (values.password === undefined) return next();
    if (!criteria) { // create
      return validatePassword(values.password, next);
    } else { // update
      // Look up user to see if values.password is actually just the existing hash
      User.findOne(criteria).exec(function (err, user) {
        if (err) return next(err);
        if (!user) return next(new Error('User to update does not seem to exist!'));

        if (user.password === values.password) return next();

        return validatePassword(values.password, next);
    }
  }
};

/**
 * Returns a hash with embedded salt given a string
 * @param {string} password The string to hash
 * @param {Function} next The callback to invoke with null or hash error and the hash
 */
function hashPassword (password, next) {
  require('bcrypt').hash(password, 10, next);
}

/**
 * Validates a password against the constraints
 * @param {string} password The password to validate
 * @param {Function} next The callback to invoke with null or the validation error
 */
function validatePassword (password, next) {
  if (password.length < 5 || password.length > 20)
    return next(new Error('Password should be 5-20 characters.'));
  next(null);
}

In #795, the async.each was replaced by async.eachSeries to ensure order of creation and make tests pass (for community adapters?), this however introduces a performance penalty which in my opinion and others is not worth:

@tjwebb in https://github.com/balderdashy/sails-postgresql/issues/128#issuecomment-68575680

.eachSeries will slow things down, though, since each create will then be done sequentially. Should the order really matter, here?

@kevinburkeshyp in https://github.com/balderdashy/sails-postgresql/issues/128#issuecomment-95970705:

would async.map help?

@devinivy in https://github.com/balderdashy/waterline-adapter-tests/pull/27#issuecomment-96682857:

I also would like to see Async#map being used as @dmarcelino mentioned.

Pros and cons

Using async.map instead of async.eachSeries will ensure .create([]) returns its results in order (issues balderdashy/sails-postgresql#128 and balderdashy/waterline-adapter-tests#27) without limiting performance. For situations where creation order is sensitive users can still chain independent create() commands in an async.eachSeries calls.

Tests

Lastly, regarding breaking tests, me and @devinivy have been fixing them in the waterline-adapter-tests (examples: balderdashy/waterline-adapter-tests#43, balderdashy/waterline-adapter-tests#52). I think it's best to fix the tests than to restrain waterline performance.

cc: @imevs, @particlebanana, @devinivy, @tjwebb, @brandonsimpson, @kevinburkeshyp, @atiertant

I noticed that every time I called .save() on my models with large association lists, I was waiting seconds for it to finish. In fact, as the association list grew (i.e. posts for a user in a social network setting), performance degraded rapidly. This small feature addition allows an optional options parameter to be passed to .save(), so the new function header is:

model.save(cb, options);

This will allow for more options to be passed to the function in the future, if necessary. Specific to this pull request is the addition of the populate key.

The value can be an array of objects that contain details about the association you want to populate:

// This will only populate the given associations 
var pOpts = [
    {
        key: 'followers',
        query: {
            name: 'Mike',
            limit: 5
        }
    },
    {
        key: 'stream'
    }
];

model.save(cb, { populate: pOpts });

Or the value can be a boolean, where true will populate all associations and false will populate none:

// This will not populate any associations
var pOpts = false;
model.save(cb, { populate: pOpts });

The default value is true so existing code will not be affected by this change. If someone gets to writing the tests before I do, that would be fantastic (I am not familiar with this testing platform and it may take me some time to get to it). Let me know if anything should be changed!

Thanks, Nate Kibler

meta attribute is used for sails-postgresql adapter for setting table schemaName (like client.t_order. "client" is the schemaName and "t_order" is the table name).

Implemented option to call hooks 'beforeUpdate', 'afterUpdate', 'beforeCreate', 'afterCreate', 'beforeDestroy', 'afterDestroy', with Model instance and make this point to model instead of a global scope.

This allows to implement common logic in single place (e.g. config/models.js in Sails).

When a through table has multiple associations behavior was unpredictable because waterline actually created the mapping the with the first model that was not the child relationship.

This fix provides an easy solution.

Currently the populates clause is allowed for plural ("collection") associations but disallowed for singular ("model") associations.

This patch allows only a select or an omit for singular associations.

Fix for selecting specific fields in population for one-to-one relationship

If there is a population for a one-to-one relationship, we should accept only "select" and "omit" attributes.