Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

We need to support relationships between resources #22

Closed
ekryski opened this issue Nov 17, 2013 · 24 comments
Closed

We need to support relationships between resources #22

ekryski opened this issue Nov 17, 2013 · 24 comments
Labels

Comments

@ekryski
Copy link
Contributor

ekryski commented Nov 17, 2013

I should be able to hit a route like GET /users/:id/posts and get all the users posts.

@chmanie
Copy link

chmanie commented Dec 12, 2013

Yes. That is exactly what I am missing as well.

@daffl
Copy link
Member

daffl commented Dec 13, 2013

We found that pretty tricky to solve because you'll have to somehow model your relationships between services and by doing so you will eventually go down the ORM path (and that's a can of worms at least I personally wouldn't want to touch). What I actually wonder is, if something like:

app.use('/users/:userId/posts', postsService);

Might even already work (with userId being mapped to params.query.userId). If not, that is something we definitely should make happen and then we might as well make it possible to do app.use('/users/:userId/posts', 'posts'); where the second parameter is the service name you want to reference to.

@wclr
Copy link

wclr commented Dec 20, 2013

You could also consider following:

  1. ability to provide name of service and route because route can be different than name (obviously), I've met scenarios when in development people want service URL look like: /api/v1/documents/1323 and in production: /a/1/d/1323.

  2. as you still monkey patching express consider using custom method service to add service:

app.service('posts', '/users/:userId/posts', postsService)

like it is done in https://github.com/visionmedia/express-resource

@daffl
Copy link
Member

daffl commented Dec 20, 2013

  1. This should already be possible with Express I think:
app.use(app.settings.env === 'production' ? 'todos/' : 't/', todoService);

I'm also not sure when I'd want different URIs in development and production (except for maybe changing the root).

  1. We had that before but it didn't feel as nice. Express doesn't do anything with an object plus we are checking for service methods so I think it is fairly safe to monkey patch the application object.

@wclr
Copy link

wclr commented Dec 20, 2013

app.use(app.settings.env === 'production' ? 'todos/' : 't/', todoService);

The problem is that you assume route to be a service name, how should I reference to service in app.lookup(app.settings.env === 'production' ? 'todos/' : 't/') ?

@daffl
Copy link
Member

daffl commented Dec 20, 2013

You could also use the express-alias module. The service names however will (and imho should) stay the same in all cases (app.use('my/service' ) will always be found through app.lookup('my/service') never by its alias).

@daffl
Copy link
Member

daffl commented Jan 2, 2014

So a basic version for associations seems pretty easy to implement as discussed above. Here my suggestion:

Definition and use with REST URIs:

// Both associations should only work if there is a /users service registered already
app.use('/users', userService)
  .use('/posts', postsService)
  .use('/accounts', accountService);

// Pass service name in an array
// Calls postsService.findAll({ userId: <userId> })
app.associate('/users/:userId/posts', ['posts']);

// Calls userService.get(<userId>) then calls
// accountService.get(user.account)
app.associate('/users/:userId/account', 'accounts');

For SocketIO:

The easiest way is probably this:

socket.emit('/users/:userId/posts', { userId: 123 }, function(error, posts) {
});

I understand that

socket.emit('/users/123/posts', function(error, posts) {
});

Might look appealing but in reality you'd probably always end up with annoying string concatenation like socket.emit('/users/' + someId + '/posts') which is why I think using an object instead makes more sense (plus any additional parameters can be passed as service params).

@daffl
Copy link
Member

daffl commented Jan 2, 2014

One more addition: In order to not monkey-patch .use any more, should we call it app.associate?

@ekryski
Copy link
Contributor Author

ekryski commented Jan 3, 2014

@daffl the concept would totally work. For some reason I feel like passing an array for findAll and not an array for get seems convoluted. Although, I'll admit that right now I don't have a more elegant solution.

One more addition: In order to not monkey-patch .use any more, should we call it app.associate?

I think we should totally use app.associate.

@sbruchmann
Copy link

+1 for app.associate

@daffl
Copy link
Member

daffl commented Jan 5, 2014

In order to kick off a plugin ecosystem for Feathers and keeping the core lean I'm thinking of making this (as well as session handling) a separate plugin. So you would do

npm install feathers-associations

var feathers = require('feathers');
var associations = require('feathers-associations');

var app = feathers().configure(associations)
  .use('/users', userService)
  .use('/posts', postsService)
  .associate('/users/:userId/posts', ['posts']);

@Glavin001
Copy link
Contributor

+1 for this feature!
I would like to use Feathers on my current side project, Coders Classroom. So far it's very impressive and I like the idea and how simple it is. The addition of relationships would make it that much much powerful and usable for future projects 👍 .

Is there anything I could do to contribute to speed up development of this feature? I like the idea of having Feathers plugins.

@daffl
Copy link
Member

daffl commented Jan 7, 2014

I already refactored the REST provider to make it possible to wrap other services into a middleware externally (it was all within the provider before and not accessible from the outside). So this plugin should happen fairly soon (as well as a 0.3.1 release).
@Glavin001 I can add you to the plugin repository once I set it up so that you can help testing.
A great way to also help would be with examples and articles actually.

@Glavin001
Copy link
Contributor

Oh, that's excellent middleware is already supported then.

Thank you. I'd really like to work on the plugin repository. And I just started learning Mocha for Unit Testing; would that be what you'd recommend using?
I'd be happy to work on documentation for plugins and Feathers.

Edit:
Should we fork a similar Plugins/Components repository as Bower? https://github.com/sindresorhus/bower-components
I'd be up for helping develop something like that, too.

Edit 2:
I'm going to work on a proof-of-concept here:
https://github.com/Glavin001/feathers-components
I am thinking that we can use GitHub-Pages to serve the components browser.
New components could be added via a merged Pull Request.
Should Feathers component repositories contain a feathers.json configure file? Like bower.json?

Edit 3:
Nevermind. We should just use npm install feathers-component-name. I got too ahead of myself there!

@Glavin001
Copy link
Contributor

Any progress on this? I have recently release feathers-mongoose-service (https://github.com/Glavin001/feathers-mongoose-service) and would like to also support relationships / associations.

I'd be happy to help develop the suggested feathers-associations plugin, however I will need a little guidance as I have not yet figured out how I can wrap custom service methods externally into Feathers, as suggested above.

I hope we can get this feature soon! Thank you for an awesome product.

@Glavin001
Copy link
Contributor

For those who are following this issue, @daffl has created the associations plugin: https://github.com/feathersjs/associations

@daffl
Copy link
Member

daffl commented Feb 20, 2014

Closing since development has moved to https://github.com/feathersjs/feathers-associations. First version for RESTful associations should be working already.

@daffl daffl closed this as completed Feb 20, 2014
@jadedevin13
Copy link

what happened to this? feathers association has not been updated since 2 years.

@daffl
Copy link
Member

daffl commented Apr 13, 2016

We have an FAQ item about this at http://docs.feathersjs.com/help/faq.html#how-do-i-return-related-entities

@nikolakanacki
Copy link

@daffl All of the links associated with "associations" are dead. Any fresh ones?

@daffl
Copy link
Member

daffl commented Feb 28, 2018

@carcinocron
Copy link

Philosophically speaking, why is GET /posts?userId=:userId not better than GET /users/:id/posts?

@daffl
Copy link
Member

daffl commented Apr 30, 2018

It is better because it is much easier to create and serialize an object with { userId: 1234 } into a query string with standard tools than having to know how the route is composed and doing some arbitrary string concatenation.

daffl pushed a commit that referenced this issue Aug 21, 2018
daffl pushed a commit that referenced this issue Aug 21, 2018
daffl pushed a commit that referenced this issue Aug 25, 2018
@lock
Copy link

lock bot commented Feb 7, 2019

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue with a link to this issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Feb 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

9 participants