kurko / ember-json-api

JSON API adapter for Ember Data.
MIT License
254 stars 62 forks source link

Links use href for hasMany relations #43

Closed koemeet closed 10 years ago

koemeet commented 10 years ago

Hi,

Currently this library does not support urls for hasMany relations. In the server response, you have to pass all the IDs of the related resource in order to fetch it. Isn't it much better to also support URLs to point the client to the correct API call to fetch the related resources.

This is how you now would return a post with the related comments (these will be fetched async)

{
  "posts": [
    {
      "id": 1,
      "title": "First post",
      "links": {
        "comments": [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 
12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25]
      }
    }
  ]
}

Those IDs could become huge and eventually you will even get an HTTP error for URI to long.

It would be much better to support something like this (using this in my Ember project too, along with this library):

{
  "posts": [
    {
      "id": 1,
      "title": "First post",
      "links": {
        "comments": {
          "href": "/api/v1/post/1/comments"
        }
      }
    }
  ]
}

WDYT? If you guys like it, I can create a pull request right away :+1:

koemeet commented 10 years ago

@kurko Can you look at this, pls?

kurko commented 10 years ago

Yes, @steffenbrem, let's do it! Here are my thoughts of what needs to be done:

Regarding IDs, I think it should be included by default and removed if the developer wants. So, here's what I'd do:

What do you think?


I have https://github.com/kurko/ember-json-api/issues/41 opened to keep track of these specs that we're covering. Please, see the tests on https://github.com/kurko/ember-json-api/tree/master/tests/integration/specs. Each one is a section from the JSONAPI.org page.

koemeet commented 10 years ago

@kurko I think both of them needs to be supported at the same time. That is how I have it implemented it right now, haven't tested it though but it should work fine. If you look at the spec, it is valid to only include a href as a link (Link to spec.). Note that they say: "A 'collection object' contains one or more of the members.". If you want, I can of course include the configuration, but I think it would be better to follow the spec and support both at the same time, won't break anything.

kurko commented 10 years ago

The thing is that not everyone supports those URL, so it could be misleading. But I'm fine with it as long as it's in the README how to disable it.

koemeet commented 10 years ago

@kurko It will be disabled automatically if you do not include the href links inside the response document. So both examples that I showed in my post will still work exactly like you would expect.

kurko commented 10 years ago

Oh my. Please forget everything I said so far lol

—Alexandre de Oliveira

On Thu, Oct 9, 2014 at 1:43 PM, steffenbrem notifications@github.com wrote:

@kurko It will be disabled automatically if you do not include the href links inside the response document.

Reply to this email directly or view it on GitHub: https://github.com/kurko/ember-json-api/issues/43#issuecomment-58557390

koemeet commented 10 years ago

@kurko I have a PR ready in a couple of minutes. Also have 1 test included.

kurko commented 10 years ago

Closing this. #44 implements it.