Thank you for the pull request. Do you have examples/tests how this would be used?

I think this is already possible by setting params.sequelize in a before hook. The documentation we are currently working on for this can be found at https://docs.feathersjs.com/v/auk-reorg/api/databases/sequelize.html#paramssequelize.

For background, I was given the task of evaluating a number of frameworks for constructing apis for a number of clients to connect to (apps, websites, browser extensions). One of the bullet points to be investigated was: "How easy is it to query through a relationship?" (EG: Find all people in a group). I can query group and then ask for its people, but I was hoping to get the results with a single query.

The goal of this patch is to allow the "includes" sequelize clause to come from the standard api service endpoint (via url arguments) without needing to do anything else. I couldnt understand how to query through a relationship by the documentation provided for either sequelize or feathers-sequelize . The best I could come up with was the "include" option of query. Once I figured out the correct sequelize incantation, I wanted to just have the feathers-sequelize service return the results of that query.
(Example server and client code below).

Fundamentally I wanted this url:
"/phrases?$limit=100&$skip=0&include[model]=groups&include[through]=phrase_groups&include[where][id]=8"

to correctly be turned into this sequelize call:

// model contains all my sequelize models

model.Phrase.find({
     include:{
       model: model.Group,
       through: model.PhraseGroup,
       where:{id: "8"}
     }}).then(function(data){console.log(data); res = data;})

So feathers service definition:

import feathers_sequelize from 'feathers-sequelize';
import hooks from './hooks';
import _ from 'lodash';

module.exports = function(){
  const app = this;
  const Phrase = app.get('Model.Phrase');
  const Group = app.get('Model.Group');
  const sequelize = app.get('sequelize');
  const options = {
    Model: Phrase,
    paginate: {
      default: 100,
      max: 500
    }
  };

  // Initialize our service with any options it requires
  app.use('/phrases', feathers_sequelize(options));
};

Allows api calls from a client like:

ns.getPhrases = function(opts){
    opts = _.assign({
      '$limit':100,
      '$skip': 0}, opts);
    console.log('Getting Phrases: ', opts);
    return jQuery.get(ns.BASE+'phrases',opts).then(function(data){
      console.log('Got Phrases: ', opts , data);
      return data;
    });
  };
// call to /phrases?$limit=100&$skip=0&include[model]=groups&include[through]=phrase_groups&include[where][id]=8
ns.getPhrases({
    include:{
      model: "groups",
      through:"phrase_groups",
      where:{id: "8"}
    }});

My understanding is that you are saying I could handle this in my applications code (which is true). My counter argument is that this is an obvious translation to sequelize and should be supported by default.
Cheers and thanks for your consideration

This seems like a good idea to me. How good is Sequelize at handling cases where users include malformed params?

I think the biggest safety that sequelize is going to provide here is that all the models and associations referenced have to have been declared in your application (sequelize doesnt seem to ever coerce from a string to a model on its own) and so I think it will only allow well declared relationships.

Perhaps you are correct and this could be too much direct influence over sequelize. I am relatively new to this piece of software and wouldnt want to vouch for the latent security of a random piece of software, but I would also hope that there are no glaring security issues in normal use of the api. I did look through the sequelize code and there is a conformInclude function that seems to be ensuring the "include" argument is sensible and as expected.

https://github.com/sequelize/sequelize/blob/master/lib/model.js#L211

I still think this can already be done through a fairly simple hook:

app.service('messages').hooks({
  before: {
    find(hook) {
      const include = hook.params.query.include;
      hook.params.sequelize = { include };

      delete hook.params.query.include;
    }
  }
});

I'd prefer to have this explicitly added in that hook so that you can modify it to your own requirements, e.g. default includes instead of adding it to the adapter core.

I have no issues with shipping a pre-made hook like this with the adapter though.

@bobbysmith007 does this work if you use that hook? If the hook works, then I definitely agree with @daffl.

I am sure that activating a hook would be fine for my uses. I have not looked into hooks much yet, but if there was an obvious configuration option to do what I was trying to accomplish then thats fine.

I might be able to get around to rewriting the patch in that style, but it might be a bit.

Cheers and thanks

@bobbysmith007 I finished developing quite a complex app using sql, relations etc. I definitely had my problems as well using feathers-sequelize and not being able to pass include options in the client. However, i must say that after all it is possible with hooks only. And after some time it really works quite well. Thus i'm also now quite conservative to introduce such a powerful concept to the client. Really have to think about this, although you are right and the client can also query what is defined in the model already.

e.g. let's say you can always only query all users from a particular project. Then you can define a hook to include user<->M:M<->project where project id=XXX in the trough table. Now the nice thing is you can reuse that in the find, remove and get hooks(well there a pull request from me after it should work in get hooks as well). Basically with that i was able to manage a quite complex security concept, without repeating myself too often.

Thanks again @bobbysmith007 for the pull request (and your input @eikaramba). As per the discussion I am going to close it.