balderdashy / sails

Realtime MVC Framework for Node.js
https://sailsjs.com
MIT License
22.84k stars 1.95k forks source link

Associations #124

Closed mikermcneil closed 10 years ago

mikermcneil commented 11 years ago

Update:

Sails v0.10.0-rc3 is now available on npm.

Upgrading to Sails v0.10 beta:

Install: $ sudo npm install -g sails@beta -g

Docs: beta.sailsjs.org

Also see: https://github.com/balderdashy/waterline-docs

Migration Guide for v0.9.x apps: https://github.com/balderdashy/sails-docs/blob/master/Migration-Guide.md

We're working on improving docs in general, but we could really use your help- please feel free to submit pull requests to https://github.com/balderdashy/sails-docs. The changes go live to beta.sailsjs.org (and eventually sailsjs.org proper, when we set the latest tag on v0.10)

mikermcneil commented 10 years ago

This is all in development so expect breakage and changes until it is pushed to the main branch

@btkostner appreciate that :)

btw everyone, I created a v0.10 list on the Trello board with the remaining line items if you're curious

npfitz commented 10 years ago

Hey guys, I'm playing around with the V0.10 branch right now, and the associations stuff is pretty awesome! I did have a couple questions/thoughts though.

  1. If I have a user object that has an account association, and I populate the account, and I also able to populate one of the accounts attributes? Though I would assume if I'm performing queries like that, I should probably rethink my application structure
  2. I'm having issues using populates on many to many associations using the sails-mysql#associations adapter. I can see that it's creating a table, and when I add an object and call save, it updates that association table, but if I call populate on that association, it returns an empty array. Has anyone gotten this to work?
lwansbrough commented 10 years ago

@npfitz

  1. You want to change that thinking slightly. Sails isn't supporting that functionality currently. What you want to do is create the user and the account independent of one another, then add one to the other. Here's how that might look:
User.create({...}, function(err, user) {
    if(err) return cb(err);
    Account.create({ user: user.id }, function(err, account) {
        if(err) return cb(err);
        // account with user associated to it.
    });

I don't know off the top of my head if user would then have account populated (it should..., at least for 1-1 associations but you never know)

npfitz commented 10 years ago

@Iwansbrough I think we're on the same page. And that does work, I've been using that most of the day today. And even if it weren't, for 1 - 1 associations, you can always just nest callbacks, even if it does start to look ugly.

Any thoughts on why I'm having issues with an n - n association? Basically, I have a user model that has a collection of projects, and a project model that has a collection of users. And it seems to work 1 way... project.populate("users") works fine but the other way around doesn't seem to give me anything :/

lwansbrough commented 10 years ago

@npfitz I haven't noticed this problem with my environment, however I'm using the PostgreSQL adapter.

npfitz commented 10 years ago

@lwansbrough Fair enough. I had some weirdness going on earlier because I messed up my model definitions. If I can't get it to work, Maybe I'll just switch on over to PostreSQL

zetxx commented 10 years ago

Hi to all, i'm trying to accomplish following: I have 2 models, first one is Product and second one is ProductCategory, and theirs corresponding models:

Product

attributes: {

    productName: {
      type: 'string',
      required: true
    },

    productPrice: {
      type: 'float',
      required: true
    },

    productCategory: {
      model: 'ProductCategory'
    }

  }

ProductCategory

attributes: {

    categoryName: {
      type: 'string',
      required: true
    },

    products:{
      collection: 'Product'
    }

  }

and im trying to get all categories and corrensponding association of its products as follows:

ProductCategory
          .find({})
          .populate('products')
          .exec(function(err, ProductCategoryCollection){
........
          });

and the result in ProductCategoryCollection is corect but when i looked into ProductCategoryCollection[0].products there was array of products but without product primary key, is this normal behavior ?

npfitz commented 10 years ago

Haha, I was actually just about to ask this exact same thing. the 1 - n relations don't seem to return the id of the returned collection

particlebanana commented 10 years ago

@zetxx @npfitz we are aware of this issue in Waterline.

I rebased the changes to master into the associations branch and pushed this up to association_master_rebase. @sgress454 has a fix for this issue in there I believe. Once i'm sure everything is good and the tests pass I will move it all into a 0.10 branch and kill off all the feature/WIP branches in Waterline.

This stuff is all coming along but there are still some issues that break. We are working on this and then need to get some docs together and you guys can go crazy playing around with it.

npfitz commented 10 years ago

@particlebanana Awesome, I'll be sure to check it out!

On a side note, is this where you want us to mention issues like these? Or should we be logging these some other way?

Cheers!

particlebanana commented 10 years ago

No this thread is too old and mixed up already. You should make issues on the Waterline project and I have a v0.10 tag.

zetxx commented 10 years ago

@particlebanana thanks :)

mikedevita commented 10 years ago

can someone chime in on how they're handling their associated data in the interim until waterline associations are working 100%?

I've been handling mine on the client side, querying for associated data by the foreign key manually. Not sure if this is the way to go.

oscarvgg commented 10 years ago

@mikedevita that sounds painful. I'm actually using mongoose.

Yes, I might be missing some cool features about Sails, but it's just while waterline's associations are complete.

So, I recommend using a different ORM for now.

thetutlage commented 10 years ago

I would rather suggest dropping sails for time being as they are still developing certain areas of the framework , especially the ORM , you will see a lot of changes with new build.

I would suggest building a boilerplate using Passport, Express and Node-orm2

mikedevita commented 10 years ago

@mikedevita that sounds painful. I'm actually using mongoose.

Yes, I might be missing some cool features about Sails, but it's just while waterline's associations are complete.

So, I recommend using a different ORM for now.

Yeah it kind of is, my other thought was to rewrite the index and index:id routes and the controller to build an adequat json response..

something like

//ClientController

find: function(req, res){
    Client.find(1).done(function(err, client){
        Domain.find({
            clientId: client.id
        });
....
davepreston commented 10 years ago

That's the approach I've used (wiring up the associations manually in the controller). It's made for some pretty deep callback stacks, but it works for now.

softwarequalitycenter commented 10 years ago

Anyone has example of how to use mongodb embedded schema. I thought it would work with associations. Doesn't?

mikermcneil commented 10 years ago

@softwarequalitycenter you can use type:json to store embedded data you don't want to break out into separate models. If you're planning on querying/searching on that embedded data, however, you'll want to break it out into a separate model and take advantage of associations. Hope that helps!

softwarequalitycenter commented 10 years ago

Thanks. Was my plan to not use json if possible. Do url post vars map to schema objects where there are more than 1 schema (embedded) inside the model? I think I need to do a fair bit of controller manipulation there (the default operations dont' support that I guess).

lwansbrough commented 10 years ago

@softwarequalitycenter If I recall correctly, v0.10 supports, for instance, /dogs/10/owner and /dogs/10/parks by default. For embedded documents though, I don't think this is the case. Although I can't understand why you'd want embedded documents vs. relational models - you lose support for associations in most places and it makes the code a lot less portable (whereas if you use associations with multiple models, you can change databases with just a few changes to the config files.

particlebanana commented 10 years ago

@softwarequalitycenter @lwansbrough Waterline currently doesn't support embedded documents on any adapters. For the associations release if an adapter supports native joins they will be used (mysql, postgresql, etc) otherwise in-memory joins are used for all other adapters (mongo, redis, disk, memory, etc).

The schemas that are built are all relational simply for the fact that Waterline works across multiple datastores. It makes sense to normalize them so you can develop on disk and move to postgresql/mongo/whatever in production without changing your code base. If you want to store embedded data on a record you always have type: json which works across all adapters but can't be queried.

canky commented 10 years ago

Is there any approximate release date on the roadmap for the associations (and v.0.10)? Sorry for being impatient. Im just really excited about it. You are doing an amazing job! Thank you for making sails.js

casoetan commented 10 years ago

I've being using sailsjs v0.10 with association branch of sails-mysql and its been fine. Had issues with sails-mongodb thou, but generally mysql has been fine.

But for v0.9.* there is a blog post here http://blog.schneidmaster.com/wiring-associations-in-sailsjs/ that gives a good guide of doing association in the model instead of the controller [@mikedevita]. Hope these helps.

robwormald commented 10 years ago

Wanted to make a suggestion, based on the changes in this commit https://github.com/balderdashy/sails/commit/5f78e81114dfeef9496eb344584f6bdc2fe84493

IMHO, the blueprint methods for associations shouldn't populate anything automatically:

Ideally, you could just pass populations in as a query parameter:

This use case is less useful with lots of relations

Since it's more sensible to do:

...but is useful here

Returning foreign keys by default would also allow lazy loading client-side, which is something waterline in the browser could handle in all sorts of interesting ways in the future.

Discuss?

lwansbrough commented 10 years ago

@robwormald It appears as though the request has to explicitly state which associations should be populated (I'm looking at line 18) which is exactly what you've outlined in your post.

robwormald commented 10 years ago

@lwansbrough i'm fairly sure if that was the case, they'd be coming in via the req.params? (see 82-84)

lwansbrough commented 10 years ago

I'm with @robwormald here, I think opt in populations are a much better plan than blindly populating everything. Would actually be nice if we could specify defaults for the blueprints too, for example:

controllers/User

{
    _config: {
        populate: {
            roles: { ...where / limit / skip in here... }
        }
    }
}
ndhoule commented 10 years ago

I've been using v0.10 with blueprints and noticed that the pub/sub methods don't populate associations. Is that by design, or just something that hasn't been finished quite yet?

EvanBurchard commented 10 years ago

Maybe this will help someone wondering if they should try v0.10 or maybe these are potential steps in the migration guide. Anyways, I had 2 issues, and then gave up.

  1. "Duplicate attribute "href" is not allowed." in my jade templates. Not sure why this was happening.
  2. My models with defaultsTo: "something" and required: true, were getting validation errors. Not sure if it was that defaultsTo doesn't work anymore or if it doesn't play nicely with required: true anymore. In either case, I was using these relevant packages:
    "sails": "git://github.com/balderdashy/sails.git#v0.10",
    "sails-disk": "git://github.com/balderdashy/sails-disk.git#0.10.beta",
    "sails-mongo": "git://github.com/balderdashy/sails-mongo.git#associations",
    "sails-memory": "git://github.com/balderdashy/sails-memory.git#associations",
particlebanana commented 10 years ago

@EvanBurchard I'll make an issue on Waterline for the defaultsTo and required issue. You shouldn't need both in any case though because there will always be a value set for it when using defaultsTo.

mikermcneil commented 10 years ago

@EvanBurchard thanks for documenting!

rjmoggach commented 10 years ago

@EvanBurchard - double check you don't have 'true' in quotes... I had one rogue attribute with 'true' in quotes and it was tough to diagnose

EvanBurchard commented 10 years ago

@mogga not the case this time, but definitely a good thing to check. Thanks for the suggestion. I've definitely done this before. Added to my list of stupid things I sometimes do. Comes in handy when I'm stuck for no good reason.

rjmoggach commented 10 years ago

not sure if this is expected behavior and probably a waterline issue if it is I imagine - working with 1:1 associations for a user and profile - if either side fk attribute is set to null it crashes the populate method...

particlebanana commented 10 years ago

@mogga can you create a Waterline issue for this and I'll tag it 0.10. I'm trying to keep all that stuff there so it's manageable. Thanks!

EvanBurchard commented 10 years ago

I hate to be on the bandwagon of "are we there yet?" and further bloat this thread, but I'm a little confused by the trello board. Does the cutover to master happen:

  1. After the v0.10 tasks card is completed?
  2. After some other known list of tasks is finished?
  3. We don't know yet?
particlebanana commented 10 years ago

@EvanBurchard I can only answer for Waterline but I'm working on docs and getting everything else stable-ish for associations. At that point I will send out a note to the group and on IRC to get a public beta going.

People have been using it some but without docs it's still an exploration in the unknown for most people. Once any major bugs seem to be worked out and it's good Waterline 0.10 will be published to NPM and merged in with master. Once Waterline is good as long as Sails is stable and the hooks and generators are all setup I think we can release Sails 0.10.

If anyone would like to help with Waterline Docs there is a repo Waterline-Docs where I'm starting to document all the various pieces.

gfhuertac commented 10 years ago

I am trying the v0.10 branch, and I got this error...this was reported before, but the error now is on the waterline module, not sails. I checked package version for waterline, and it is using version 0.10.0 Any help?

/usr/lib/node_modules/sails/node_modules/waterline/node_modules/anchor/lib/match.js:50
                throw new Error ('Unknown rule: ' + ruleName);
                      ^
Error: Unknown rule: collection
    at Object.match (/usr/lib/node_modules/sails/node_modules/waterline/node_modules/anchor/lib/match.js:50:9)
    at Anchor.to (/usr/lib/node_modules/sails/node_modules/waterline/node_modules/anchor/index.js:76:45)
    at /usr/lib/node_modules/sails/node_modules/waterline/lib/waterline/core/validations.js:151:31
    at /usr/lib/node_modules/sails/node_modules/async/lib/async.js:119:25
    at /usr/lib/node_modules/sails/node_modules/async/lib/async.js:24:16
    at /usr/lib/node_modules/sails/node_modules/waterline/lib/waterline/core/validations.js:135:64
    at /usr/lib/node_modules/sails/node_modules/async/lib/async.js:111:13
    at Array.forEach (native)
    at _each (/usr/lib/node_modules/sails/node_modules/async/lib/async.js:32:24)
    at Object.async.each (/usr/lib/node_modules/sails/node_modules/async/lib/async.js:110:9)

Models are as follows:

Identity.js

module.exports = {
  attributes: {
    provider  : { type: 'string' },
    uid  : { type: 'string' },
    user: { model: 'User' }
  }
};

User.js

module.exports = {
  attributes: {
    displayName  : { type: 'string' },
    identities  : { collection: 'Identity', via: 'user' }
  }
};
mikedevita commented 10 years ago

Are you using sails-disk. Try another adapter like sails mysql or sails Mongo. On Feb 5, 2014 4:07 AM, "Gonzalo Huerta-Canepa" notifications@github.com wrote:

I am trying the v0.10 branch, and I got this error...this was reported before, but the error now is on the waterline module, not sails. Any help?

/usr/lib/node_modules/sails/node_modules/waterline/node_modules/anchor/lib/match.js:50 throw new Error ('Unknown rule: ' + ruleName); ^ Error: Unknown rule: collection at Object.match (/usr/lib/node_modules/sails/node_modules/waterline/node_modules/anchor/lib/match.js:50:9) at Anchor.to (/usr/lib/node_modules/sails/node_modules/waterline/node_modules/anchor/index.js:76:45) at /usr/lib/node_modules/sails/node_modules/waterline/lib/waterline/core/validations.js:151:31 at /usr/lib/node_modules/sails/node_modules/async/lib/async.js:119:25 at /usr/lib/node_modules/sails/node_modules/async/lib/async.js:24:16 at /usr/lib/node_modules/sails/node_modules/waterline/lib/waterline/core/validations.js:135:64 at /usr/lib/node_modules/sails/node_modules/async/lib/async.js:111:13 at Array.forEach (native) at _each (/usr/lib/node_modules/sails/node_modules/async/lib/async.js:32:24) at Object.async.each (/usr/lib/node_modules/sails/node_modules/async/lib/async.js:110:9)

Reply to this email directly or view it on GitHubhttps://github.com/balderdashy/sails/issues/124#issuecomment-34156511 .

gfhuertac commented 10 years ago

I tried with postgres adapter and same result. Only mysql and mongo are working?

mikedevita commented 10 years ago

The error relates to the identities line of your User.js file. The problem is Anchor doesn't have a collection rule.. https://github.com/balderdashy/anchor/blob/0.10.beta/lib/rules.js

Look at the waterline-docs. https://github.com/balderdashy/waterline-docs/blob/master/associations.md

lwansbrough commented 10 years ago

Try completely uninstalling sails globally. Make sure no dependencies are installed globally as well (/usr/local/lib/node_modules if I recall correctly). Reinstall Sails and use Sails-disk.

gfhuertac commented 10 years ago

OK. I think the issue is another one. I wanted to insert first the identity and then a user with parameter identities: [ identity.id ] But it seems that identities is not created at db level, so it fails. If I do it the other way around (insert user and then an identity with user: user.id) then it works ok.

lwansbrough commented 10 years ago
User.create({...}, function(err, user) {
    user.identities.add(identity.id);
});

That's the solution to your problem I believe.

npfitz commented 10 years ago

So... I couldn't help but notice V0.10 has been merged into master. Has it finally happened? Is this the day we've all been waiting for??

Lujaw commented 10 years ago

I guess so. Had been playing with the v0.10 branch for sometime now. Awesome..

particlebanana commented 10 years ago

I'm going to make one last pass through the adapters and make sure they are good to go with Waterline v0.10 then go ahead and probably publish v0.10 of Waterline to NPM.

unknownArtist commented 10 years ago

@particlebanana @mikermcneil when sailsjs 0.10 is coming out.. ? any specific date?

elathon commented 10 years ago

Is it possible to reference a primary key named something other than 'id'? I'd like to use the tableID naming convention for my primary keys.

I tried:

fk_groupID: {
    model: 'group',
    references: 'groupID'
}

but it doesn't seem to work.