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

JSON:API - meta inside data is lost #3419

Closed
kottenator opened this Issue Jun 22, 2015 · 22 comments

Comments

Projects
None yet
@kottenator

kottenator commented Jun 22, 2015

Hi!

I use ember-data#canary (commit 4c5fcf6). I have the following JSON:API data:

{
    "data": {
        "type": "group",
        "id": "1435d80609114121bf7a0ada912e739d",
        "attributes": {
            "name": "Group Name",
            "country": "United States of America"
        },
        "meta": {
            "countries": [
                "England",
                "Ukraine",
                "United States of America"
            ]
        }
    }
}

After serialization, meta is lost.

If I put meta to the top level of JSON then I can access it via DS.Store.metadataFor("group"). But if I put it inside data (which is allowed by JSON:API v1.0) then it's lost.

It affects both single-record and mutli-record data.

@bmac

This comment has been minimized.

Member

bmac commented Jun 22, 2015

Hi @kottenator, Unfortunately, Ember Data does not support per-record meta.

This is a place where the capabilities of the JSON-API format is ahead of the existing Ember Data APIs for exposing metadata. Before Ember Data 1.13.0 was a limitation of the old internal format Ember Data used for representing serialized records that made per record meta difficult to express. With the move towards JSON API as Ember Data's serialization format Ember Data plans to add support for per record meta objects in the future.

For now, Ember Data only understands a top level meta object on queries and relationships (through related links).

@kottenator

This comment has been minimized.

kottenator commented Jun 22, 2015

No worries, I will make some workaround for now.
Anyway, you're doing great job, thank you for that ;)

@kottenator

This comment has been minimized.

kottenator commented Jun 22, 2015

One more question about metadata:
I've put it on the top level and I can reach it via metadataFor.
But ember-data logs a warning:

DEPRECATION: store.metadataFor() has been deprecated. You can use .get('meta') on relationships and arrays returned from store.query()

I'm trying to do this:

// app/routes/group.js
import Ember from 'ember';

export default Ember.Route.extend({
    model(params) {
        return this.store.find('group', params.id);
    },

    setupController(controller, model) {
        this._super(...arguments);
        controller.set('group', model);
        controller.set('meta', model.get('meta'));
    }
});

But it doesn't work. Could you tell me how to make it work in proper way? (not deprecated)

@wecc

This comment has been minimized.

Contributor

wecc commented Jun 23, 2015

@kottenator For now, Ember Data only understands a top level meta object on queries (store.query) and when fetching relationships (through related links).

@kottenator

This comment has been minimized.

kottenator commented Jun 23, 2015

So I should use deprecated metadataFor(...) to get the top-level meta for now?

@wecc

This comment has been minimized.

Contributor

wecc commented Jun 23, 2015

@kottenator yes, unfortunately. metadata handling will be greatly improved in 2.0 but we just haven't had time to work it out yet. Having JSON-API as out internal format is what makes it possible.

@wecc

This comment has been minimized.

Contributor

wecc commented Jun 30, 2015

I'm closing this as we're tracking the progress of JSON API support in #2905

@rtm

This comment has been minimized.

rtm commented Oct 22, 2015

So metadataFor is deprecated, but with no replacement?

@rich-poole

This comment has been minimized.

rich-poole commented Nov 12, 2015

It seems so, the state of metadata support is pretty poor right now, the docs are out of date and doing anything meaningful seems to rely on deprecated and/or private methods.

@wecc

This comment has been minimized.

Contributor

wecc commented Nov 12, 2015

Metadata is supported on results from store.query() and on belongsTo/hasMany relationships. Global metadata per type didn't really make sense and was removed a while back. Metadata per record and improved metadata on collections will hopefully be available soon.

Feel free to submit issues related to out of date docs, or PRs fixing it. From what I can see http://guides.emberjs.com/v2.1.0/models/handling-metadata/ and http://emberjs.com/api/data/classes/DS.ManyArray.html#property_meta looks up to date to me.

@rtm

This comment has been minimized.

rtm commented Nov 13, 2015

Isn't pagination information a perfect example of per-model metadatda? I myself had a good use case for it which is a little it too involved to go into here. I don't understand why someone would randomly decide that something is not necessary because they can't think of any reasons for it at that moment and so decide to remove it. With regard to metadata on records, I'm a bit unclear here: how does it differ from regular old record data?

@rich-poole

This comment has been minimized.

rich-poole commented Nov 13, 2015

I can't understand why you would allow metadata on store.query('project',{}) but not store.findAll('project'), that just makes no sense to me at all.

Metadata isn't only useful when you have a subset of data and I can't see why arbitrarily removing support for it apart from certain circumstances is a good design decision.

I can just about accept not allowing metadata for individual records but even then, it just seems simpler to respect what the API returns rather than telling people they are doing it wrong if they want to access metadata (especially when we don't all have control over the APIs we use).

@wecc I'll make a note to submit issues for the errors and please take the above as constructive criticism, I love a lot of Ember and just want to help make it a less bumpy ride for those following after.

@frederikbosch

This comment has been minimized.

frederikbosch commented Nov 26, 2015

@rtm That was my first thought too. After looking at my app, eventually the meta data only for .query seems to work well for me. Give it a try.

@BananaNeil

This comment has been minimized.

BananaNeil commented Nov 28, 2015

I agree with @Ricky1981 100%. Please respect my API, I have too many apps using it to add more special support for silly deprecations.

In our use case), we return a csrf_token in the top level meta key of our authenticated user response.

We rely on metadataFor in our 1.9 app, and now for our_seperate 2.x app, I'm either going to have to duplicate the csrf_token to be mixed in with the user's data, or possibly add some sort of monkey patch to ember data serializer.

@frederikbosch

This comment has been minimized.

frederikbosch commented Nov 30, 2015

@BananaNeil After thinking through, I think you are right on this matter. However, you might reconsider your language. 'Silly deprecations' is not the way we should approach this. By choosing open source you opt-in on features for free, but you also opt-in on decisions that you will not be happy about. Try to convince them to do otherwise, deal with it (by creating a patch) but in any case leave the unnecessary adverbs home.

@wecc

This comment has been minimized.

Contributor

wecc commented Nov 30, 2015

I can't understand why you would allow metadata on store.query('project',{}) but not store.findAll('project'), that just makes no sense to me at all.

The reason for this is that store.findAll(type) always returns the same array of records, and this array is "live" and automatically updated when records are added/removed from the store. Because of this, metadata returned when doing an initial store.findAll(type) call might not be correct for all the records returned by that same call in the future.

store.query(...) on the other hand is made to represent a subset of data and meta in there makes a lot more sense imho.

I agree with @Ricky1981 100%. Please respect my API, I have too many apps using it to add more special support for silly deprecations.

I believe "silly deprecation" is a bit unfair. The internals of Embed Data was heavily refactored and improved in the 1.13 version (with backwards-compatible support). Since we moved to JSON API internally this also allowed us to actually do better meta handling. The previous version was just a global meta store per type, and hacks around that. With the new internals it will be much easier and cleaner to implement great support for meta on different levels of your data. We still have work to do and currently meta is only implemented in query and hasMany/belongsTo relationships.

@rich-poole

This comment has been minimized.

rich-poole commented Nov 30, 2015

I think part of the problem is that as far as I can see, JSON API is very sparsely adopted and it feels a bit like it has been adopted/forced on Ember/Ember Data purely because of Yehuda's involvement and I think that could prove to be a mistake.

Personally, I would have liked to see much more effort put into making it easier to work with non-compliant APIs because we are often forced to do that, especially when using external APIs. That would seem like time better spent rather than making JSON API the "happy path" when I can't find anyone who has implemented on a wide scale.

Just my 2 cents of course.

@neilbaylor

This comment has been minimized.

neilbaylor commented Dec 4, 2015

Hey there

I agree that this sucks

Here's my work around for now

in /serializers/application.js:

import App from '../app';
import { ActiveModelSerializer } from 'active-model-adapter';

App.storeMeta = {};

export default ActiveModelSerializer.extend({
  normalizeResponse(store, primaryModelClass, payload) {
    App.storeMeta[primaryModelClass.modelName] = payload.meta; //ember data only allows meta data on 'query', this adds support for all other methods
    return this._super(...arguments);
  }
});

then in your route you can do this:

    return this.store.filter('myModelType', params, filterFn).then(function (model) {
      model.meta = App.storeMeta['myModelType']; 
      return model;
    });
@frederikbosch

This comment has been minimized.

frederikbosch commented Dec 6, 2015

After rereading things thread, I come to the following conclusions.

  1. People use meta data for different purposes
  2. That can be either meta data related to the requested resources
  3. Or for purposes unrelated to the requested resources
  4. At this moment only the first option is implemented. And more specifically, only with query commands there is a guarantee that the meta data is really related to the returned resources.
  5. People that are using the meta property for data unrelated to the requested resources should reconsider this. The definition of meta data is that it is describing other data. This is not the case for a CSRF Token. Maybe move it to a 'config' property.
  6. People that do not have this opportunity should implement a hack.
  7. Both solutions require extra effort. What I would do is inject a config object (Ember.Object or POJO) into the serializer. Fill it with the extra data coming from http requests (be it in a 'config' or 'meta' property) by overloading the serializer as suggested by @neilbaylor. And then also inject this config object in places where you need it, which probably would be in routes.
@artburkart

This comment has been minimized.

artburkart commented Jan 23, 2016

@Ricky1981 - I have never seen a company implement a JSON API compliant API either, but you must consider the possibility this is because there has never been a standard to follow. By officially adopting JSON API, the ember community is taking an aggressive step toward making web development easier for the whole web dev community.

Customizing serializers and adapters is a very easy task with data 2.x and adding 2-10 lines of code is hardly a chore considering the bang ember and ember data give you in return. Additionally, adopting jsonapi has finally made it possible to bridge ember with other random APIs confidently.

This thread is helpful for understanding the current state of ember data and where it's going. Thank you @wecc for pointing to the relevant documentation.

@rohanpujaris

This comment has been minimized.

rohanpujaris commented Feb 24, 2016

I also agree with @Ricky1981.
But I think it is not a big issue as we can customize serializer completely as per our wish.

@vasilakisfil

This comment has been minimized.

vasilakisfil commented Sep 5, 2016

If JSONAPI adapter wants to be JSONAPI-compliant it should support meta information per record since the spec allows that. Looking forward to it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment