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

RFC: Ember Data url templates #4

Closed
wants to merge 4 commits into from
Closed

Conversation

amiel
Copy link

@amiel amiel commented Aug 20, 2014

This is my third draft after feedback from @igorT.

There are still some details that need to get ironed out, but I thought it was about time to get community discussion on this.

Thanks!

(RFC content pasted in here by @machty)

  • Start Date: 2014-08-20
  • RFC PR:
  • Ember Issue:

Summary

url-templates improves the extesibility of API endpoint urls in RESTAdapter.

Motivation

I think that Ember Data has a reputation for being hard to configure. I've often
heard it recommended to design the server API around what Ember Data expects.
Considering a lot of thought has gone in to the default RESTAdapter API, this
is sound advice. However, this is a false and damaging reputation. The adapter
and serializer pattern that Ember Data uses makes it incredibly extensible. The
barrier of entry is high though, and it's not obvious how to get the url you need
unless it's a namespace
or something pathForType
can handle. Otherwise it's "override buildURL". RESTSerializer was recently
improved to make handling various JSON structures easier; it's time for url
configuration to be easy too.

Detailed Design

buildURL and associated methods and properties will be moved to a mixin design
to handle url generation only. buildURL will use templates to generate a URL
instead of manually assembling parts. Simple usage example:

export default DS.RESTAdapter.extend({
  namespace: 'api/v1',
  urlTemplate: '{host}{/namespace}/posts{/id}'
});

Resolving template segments

Each dynamic path segment will be resolved on a singleton object based on a urlSegments
object provided in the adapter. This urlSegments object will feel similar to defining
actions on routes and controllers.

// adapter
export default DS.RESTAdapter.extend({
  namespace: 'api/v1',
  pathTemplate: '{/:namespace}/posts{/:post_id}{/:category_name}{/:id}',

  pathSegments: {
    category_name: function(type, id, snapshot, requestType) {
      return _pathForCategory(snapshot.get('category'));
    }

    post_id: function(type, id, snapshot, requestType) {
      return snapshot.get('post.id');
    };
  }
});

Psuedo implementation

export default Adapter.extend({
  buildURL: function(type, id, record, requestType) {
    var template = this.compileTemplate(this.get('urlTemplate'));
    var templateResolver = this.templateResolverFor(type);
    var adapter = this;

    return template.fill(function(name) {
      var result = templateResolver.get(name);

      if (Ember.typeOf(result) === 'function') {
        return result.call(adapter, type, id, record, requestType);
      } else {
        return result;
      }
    });
  },
});

Different URL templates per action

Configuring different urls for different actions becomes fairly trivial with this
system in place:

Usage

// adapter
export default DS.RESTAdapter.extend({
  urlTemplate: '/posts{/:post_id}{/:id}',
  createUrlTemplate: '/comments',
  hasManyUrlTemplate: '/posts/{:post_id}/comments',

  pathSegments: {
    post_id: function(snapshot) {
      return snapshot.get('post.id');
    };
  }
});

Drawbacks

  • Building URLs in this way is likely to be less performant. If this proposal is
    generally accepted, I will run benchmarks.

Alternatives

The main alternative that comes to mind, that would make it easier to configure
urls in the adapter, would be to generally simplify buildURL and create more
hooks.

Unresolved Questions

  • How do we handle generating urls for actions that do not have a single
    record? This includes findAll and findQuery, which have no record, and
    findMany and findHasMany which have a collection of records.

@amiel amiel changed the title Initial path templates proposal RFC: Ember Data path templates Aug 20, 2014
@knownasilya
Copy link
Contributor

How would you see configuring for multiple fetch types, e.g. findHasMany, etc.., using the pathSegments hash? If that was needed.

@amiel
Copy link
Author

amiel commented Aug 20, 2014

@knownasilya Unless I am misunderstanding your question, there is a section on this. Basically, buildURL would get passed an action type and would look up a template specific to that action (hasManyPathTemplate, createPathTemplate, etc).

@knownasilya
Copy link
Contributor

@amiel ah, ok so it would reuse the existing segments in the pathSegments hash.

@amiel
Copy link
Author

amiel commented Aug 20, 2014

@knownasilya Correct, and there might be segments in pathSegments that only apply to some of the templates.

@knownasilya
Copy link
Contributor

I really like this proposal, and can see it being one of the things that makes ED very usable in many situations.

@jayphelps
Copy link

Can someone add some usage examples, from the store-side?

e.g. given:

// comment.js
export default DS.RESTAdapter.extend({
  pathTemplate: '/:namespace/posts/:post_id/comments/:id',

  pathSegments: {
    post_id: function(record) {
      return record.get('post.id');
    }
  }
});

How do I find one of these comments via the store methods?
Something like this?

var comment = this.store.find('comment', 1, { post_id: 15 });

@amiel
Copy link
Author

amiel commented Oct 1, 2014

@jayphelps Good question.

I like your idea of

var comment = this.store.find('comment', 1, { post_id: 15 });

This is kind of related to this unanswered question.

@blimmer
Copy link

blimmer commented Oct 1, 2014

I really like this proposal, and would love to see something like this in Ember Data. I've done a lot of reading on this topic today, and have worked around the issue as follows.

For certain actions, we namespace the URL to a particular customer. For instance,

http://endpoint.com/customer/123/settings.json

for populating the settings model.

Because we don't have something as described here in Ember Data, we're doing this hack in pathForType in our adapter:

  pathForType: (type) ->
    switch type
      when 'settings' then "customer/#{@get('session.customer.id')}/settings"

Where session is injected into our adapter and we require the user to be logged in to make this request.

It would be preferable if we could do this the way @amiel has suggested (expect with a findAll instead of a find:

@store.findAll('settings', { customer: @get('session.customer.id') }

So, anyway, I'm really a big fan of this idea and would love to see this implemented 👍

@garrettlancaster
Copy link

👍

@knownasilya
Copy link
Contributor

Will it also be possible to add query params to the paths? e.g.

pathTemplate: ':namespace/:type?owner=:owner'

Or maybe even something like:

this.store.find('comment', 1, { queryParams: [['owner', 4]] });
pathTemplate: ':namespace/:type/:id'
// api/comments/1?owner=4

And they are added if specified.

@amiel
Copy link
Author

amiel commented Nov 21, 2014

Yes, it will be more like:

pathTemplate: ':namespace/:type/:id'

this.store.find('comment', 1, { owner: 4 });
// api/comments/1?owner=4

@MiguelMadero
Copy link

👍

@nikz
Copy link
Contributor

nikz commented Nov 27, 2014

👍 for this - let me know if there's anything I can do to help with development!

@stefanpenner
Copy link
Member

Doesn't the jsonapi adapter have some support for this? Is this something that must be backed into ember data, or does everything exist for it to be an adapter concern?

@knownasilya
Copy link
Contributor

@stefanpenner at a minimum it should be in the RESTAdapter by default.

@DJM0
Copy link

DJM0 commented Jan 8, 2015

Any update on this? This would potentially stop us moving away from ember-data to a custom adapter.

@amiel
Copy link
Author

amiel commented Jan 9, 2015

It was my intention to implement this. Since I wrote this RFC I have changed companies and am now just getting approval for some open source time. I'll see if I can get started next week. If anyone is interested in pairing on this, let me know. I'll also be at Emberconf, and would love to chat about it there :)

In the mean time, I would love some input on the Unresolved Questions.

@wycats
Copy link
Member

wycats commented Feb 6, 2015

I really like the idea of using URL templates for this problem. However, I think it would be better to use the URL template spec for this purpose. For example, the URL template spec allows you to expand arrays or maps into various structures (like comma-separated lists or query params), which is much more expressive than the :segment syntax.

We might consider supporting the :segment syntax as a shorthand for people familiar with Ember/Rails/etc. routing syntax, but I really like the expressiveness of the template syntax. This is also why we use URL templates in JSON API rather than a simpler ad-hoc solution.

Can people investigate the smallest, most embeddable URL template expander written in JavaScript? If there is none that is particularly small, it shouldn't be too hard to make a new microlibrary, but I bet one already exists.

amiel pushed a commit to amiel/data that referenced this pull request Feb 21, 2015
This is my first step towards emberjs/rfcs#4.

The intent here is to separate url building from the RESTAdapter so that
it can eventually be included in other adapters and hold the logic for the
path templating described in emberjs/rfcs#4.
amiel pushed a commit to amiel/data that referenced this pull request Feb 21, 2015
This is my first step towards emberjs/rfcs#4.

The intent here is to separate url building from the RESTAdapter so that
it can eventually be included in other adapters and hold the logic for the
path templating described in emberjs/rfcs#4.
@mgenev
Copy link

mgenev commented Jun 24, 2015

@amiel thank you, i'm on it, i'll report back

@billpull
Copy link

@amiel add-on working for me thank you.

Anyone on ember-data team have an indication of if this path will be integrated into ember or if this will become deprecated and another nested url solution available?

@knownasilya
Copy link
Contributor

Would love to see an example of this add-on working with the new adapter options..

@amiel
Copy link
Author

amiel commented Jun 24, 2015

@knownasilya what new adapter options are you talking about?

@knownasilya
Copy link
Contributor

emberjs/data#3278

@amiel
Copy link
Author

amiel commented Jun 25, 2015

@knownasilya Oh sweet! I'll have to take a more in depth look. Maybe I missed it; are these options passed to buildURL?

@igorT
Copy link
Member

igorT commented Jun 27, 2015

@billpull the idea is for people to use this addon, and then judge based on the feedback whether it's worth the complexity of brining it to ED itself. As addons now work pretty well, seems sometimes better to opt out of making core bigger and bigger

@amiel
Copy link
Author

amiel commented Jan 4, 2016

FYI: There's a bunch of new documentation up at https://github.com/amiel/ember-data-url-templates/wiki

I'm not sure how the ember-data core team feels about bringing this in to core yet. Here are my thoughts:

First of all, while ember-data-url-templates still feels new (there haven't really been many changes since the first release), it is getting used by a lot of projects (according to ember observer it is in the top 10% of downloads). Also, the amount of code in the actual plugin is very small (< 100 lines).

If that were the end of it, I'd vote strongly for including it in ember-data core immediately.

However, the current templating library (geraintluff/uri-templates), is 4.5k minified and I think that is too much to include in ember-data by default.

geraintluff/uri-templates supports level 4 of RFC6570, which is great for the addon, but maybe not necessary for the basic use-case.

I think ideally ember-data would include a micro-library for supporting RFC6570 level 1 or 2 and that it would be easy to plugin in support for level 4 templates if needed.

@oligriffiths
Copy link

@amiel Hey, this is interesting, just came across this. We've been working on a prototype system to interface with a fairly unstandardised API, with very little convention. Some routes use the correct HTTP verbs, some don't, some require body data, some don't.

I developed a very basic URL scheme mapper, that would resolve a URL based on a provided object (model), a list of changed attributes, and an "action" that is being performed "fetchRecord" etc.

It looks like this could solve some of our problems, but our API is so non-standard some things become very hard. For example, on a post object we have a liked attribute that says whether the current user has liked the post previously. In order to like a post, we set the liked property to true, and save the post. When this happens, we need to send a POST request to a different URL than the one the post usually saves to.

What are your thoughts on this...

@knownasilya
Copy link
Contributor

@oligriffiths you are probably better of using something like https://github.com/mike-north/ember-api-actions, and do model.like() where that likes and saves the model to the like endpoint.

@oligriffiths
Copy link

@knownasilya Thanks ill check that out.

However, we still have an issue that some routes don't map to HTTP verbs, e.g. to delete a post, we send a POST request to /post/delete with data of: id=X.

@knownasilya
Copy link
Contributor

@oligriffiths that would be solved by @amiel's template addon

@amiel
Copy link
Author

amiel commented Jan 6, 2016

@oligriffiths The concept behind this RFC is implemented in https://github.com/amiel/ember-data-url-templates. Based on what you said, I think https://github.com/mike-north/ember-api-actions will be useful to you. You can use ember-data-url-templates along with ember-api-actions.

@oligriffiths
Copy link

@amiel Thanks, they both look excellent, however I'm not sure they cover the following cases:

Custom HTTP verbs for specific actions, e.g. POST to do a DELETE action
Custom request bodies for specific actions, e.g. POST data of id=X for a DELETE action

@amiel
Copy link
Author

amiel commented Jan 7, 2016

@oligriffiths that makes sense. I think you'll want to override deleteRecord then.

@oligriffiths
Copy link

Ok, I'll try working on an extension to handle this case then, along with using those recommended addons

@hoIIer
Copy link

hoIIer commented Jan 9, 2017

What is the status of this? It's been a year since last comment and I've found this while researching how to handle sub-resources with ember-data. It seems like there isn't a clear way to do what we're looking for which is to have a model e.g. User with a relation field User.friends: hasMany('user') where that relation field GET/PUT's it's data from a sub-resource /users/:id/friends

Is that what this rfc/related package aims to handle?

@rmharrison
Copy link

Yes, as far as I understand it, that's exactly what this RFC is intended to do. Back in Oct, I gave a talk [slides, video, sample repo] outlining my understanding of the problem, and the current workarounds available with @amiel's ember-data-url-templates addon.

To my understanding, there are two main hang-ups.

  1. As discussed, one of the hang-ups is the large file size of the uri-templates lib.
  2. Even with path resolution, the question of how to pass data between the route and the model adapter is still open. There isn't an attribute, in for example snapshot, present in all of the dozen or so fetchRecord et al. methods.

@amiel
Copy link
Author

amiel commented Jan 9, 2017

@rmharrison is correct. ember-data-url-templates is the current implementation of this RFC.

@erichonkanen Please let me know if you have any questions or feedback :)

pzuraq referenced this pull request in pzuraq/emberjs-rfcs Nov 27, 2018
updates the design and motivation sections, along with some others
@cbou
Copy link

cbou commented Jan 17, 2019

Since EmberJS 3.2 it's possible to pass an object options to the store which may contains adapterOptions. This was not mention in this PR but I think it's an alternative too.

@hjdivad
Copy link
Member

hjdivad commented Jun 26, 2019

The feature described in this RFC is excellent.

The concerns re: landing this in Ember Data are twofold:

  1. Expanding the adapter API within Ember Data itself; and
  2. The filesize concerns

The Ember Data team is currently working on separating Ember Data's various concerns into separate packages with a more focused API and enabling more features to be "pay as you go" via things like addons and not shipping adapter & serializer implementations that aren't used (and "tree shaking" stuff more generally).

People are encouraged to continue using ember-data-url-templates.

We've discussed this RFC the last couple of weekly meetings (these are open btw; the details are pinned in the public discord channel & folks are encouraged to drop by) and decided to FCP to close with the expectation that users who need this extended functionality can continue to take advantage of the addon.

@hjdivad hjdivad added the FCP to close The core-team that owns this RFC has moved that this RFC be closed. label Jun 26, 2019
@amiel
Copy link
Author

amiel commented Jun 26, 2019

Thanks for the update @hjdivad.

It makes a lot of sense to me to continue to suggest using ember-data-url-templates to people who need this functionality. While I think it is a fairly common use-case, it is arguably not the standard green-field use-case for anyone that has the opportunity to design their own API to follow adapter standards.

If anyone is interested, I could use some help keeping ember-data-url-templates up to date with the latest ember addon standards.

@knownasilya
Copy link
Contributor

Would love to see this recommend in the ED docs for those that hit the limits.

@hjdivad
Copy link
Member

hjdivad commented Jul 16, 2019

@knownasilya this is a good point. I don't think we have a particular system for documenting "recommended addons" for extended use cases but it's something we should consider.

@hjdivad hjdivad closed this Aug 21, 2019
rwjblue pushed a commit that referenced this pull request Jun 12, 2020
kategengler added a commit to kategengler/rfcs-1 that referenced this pull request Aug 25, 2020
@olenderhub
Copy link

olenderhub commented Sep 17, 2020

When you want to use it only in one place, you can do:

// app/adapters/comment.js
import ApplicationAdapter from 'my-app-name/adapters/application';

export default ApplicationAdapter.extend({
  urlForFindAll(modelName, snapshot) {
    return `/${this.urlPrefix()}/posts/${snapshot.adapterOptions.postId}/comments`;
  }
});

then

this.store.findAll('comments', {adapterOptions: {postId: post.id}})

and it will generate e.g.:

http://localhost:3000/api/v1/posts/1/comments (GET)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FCP to close The core-team that owns this RFC has moved that this RFC be closed. Needs Champion T-ember-data RFCs that impact the ember-data library
Projects
None yet
Development

Successfully merging this pull request may close these issues.