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

extractMeta hook not firing in JSONAPISerializer #4161

Closed
JakeDluhy opened this issue Feb 12, 2016 · 16 comments
Closed

extractMeta hook not firing in JSONAPISerializer #4161

JakeDluhy opened this issue Feb 12, 2016 · 16 comments

Comments

@JakeDluhy
Copy link

I'm extending the JSONAPISerializer, and I was trying to use the extractMeta hook. It never gets triggered though. Digging into the source, it seems that _normalizeResponse, where extractMeta is called from in the JSONSerializer, is overwritten and no longer calls it (see below).

// /serializers/json.js
...
_normalizeResponse(store, primaryModelClass, payload, id, requestType, isSingle) {
  let documentHash = {
    data: null,
    included: []
  };

  let meta = this.extractMeta(store, primaryModelClass, payload);
  if (meta) {
    assert('The `meta` returned from `extractMeta` has to be an object, not "' + Ember.typeOf(meta) + '".', Ember.typeOf(meta) === 'object');
    documentHash.meta = meta;
  }

  if (isSingle) {
    let { data, included } = this.normalize(primaryModelClass, payload);
    documentHash.data = data;
    if (included) {
      documentHash.included = included;
    }
  } else {
    let ret = new Array(payload.length);
    for (let i = 0, l = payload.length; i < l; i++) {
      let item = payload[i];
      let { data, included } = this.normalize(primaryModelClass, item);
      if (included) {
        documentHash.included.push(...included);
      }
      ret[i] = data;
    }

    documentHash.data = ret;
  }

  return documentHash;
},
// /serializers/json-api.js
...
_normalizeResponse(store, primaryModelClass, payload, id, requestType, isSingle) {
  let normalizedPayload = this._normalizeDocumentHelper(payload);
  return normalizedPayload;
},

_normalizeDocumentHelper(documentHash) {

  if (Ember.typeOf(documentHash.data) === 'object') {
    documentHash.data = this._normalizeResourceHelper(documentHash.data);
  } else if (Array.isArray(documentHash.data)) {
    let ret = new Array(documentHash.data.length);

    for (let i = 0; i < documentHash.data.length; i++) {
      let data = documentHash.data[i];
      ret[i] = this._normalizeResourceHelper(data);
    }

    documentHash.data = ret;
  }

  if (Array.isArray(documentHash.included)) {
    let ret = new Array(documentHash.included.length);

    for (let i = 0; i < documentHash.included.length; i++) {
      let included = documentHash.included[i];
      ret[i] = this._normalizeResourceHelper(included);
    }

    documentHash.included = ret;
  }

  return documentHash;
}

Maybe this.extractMeta needs to be added to _normalizeDocumentHelper?

@wecc
Copy link
Contributor

wecc commented Feb 12, 2016

Thanks for opening this issue @JakeDluhy!

I think the original reasoning behind omitting the call to extractMeta for the JSONAPISeriaizer was that meta in JSON API already is in the right place – and if it isn't, it's not JSON API. That said, a call to extractMeta in _normalizeDocumentHelper makes sense to me.

Would you be able to explain your use-case to help understanding the situation a bit more?

@JakeDluhy
Copy link
Author

Sure! Essentially I started looking into using extractMeta because (I thought) it was already there, and I wanted to use it to extract data from the links object in the payload. See adopted-ember-addons/ember-infinity#97 for a full explanation of what I did, but ultimately it would look like

extractMeta(store, modelName, payload) {
  this._extractLinks(payload.links);
},

_extractLinks(linksObject) {
  let parser = document.createElement('a');
  let indexRoute = getOwner(this).lookup('route:index');
  ['prev', 'self', 'next', 'last'].forEach((key) => {
    if(linksObject[key]) {
      parser.href = linksObject[key];
      indexRoute.set(`${key}Params`, parser.search)
    } else {
      indexRoute.set(`${key}Params`, null);
    }
  });
}

so essentially I want to be able to set properties on the route corresponding to the query params at the end of my links values, so that I can then use them for pagination.

I suppose that to answer your question, I don't really need extractMeta (I would rather have extractLinks)

@JakeDluhy
Copy link
Author

I actually have another use case, that specifically pertains to the meta data.

We have an group object, and we want to provide information about that object that pertains to the current user (e.g. is the user following that group). JSON API spec would seem to suggest that that information should be in the meta, as it is not attributes on the model. However, we still need to access that information and set it as attributes, and I presume the best place to do that would be in extractMeta.

@acorncom
Copy link
Contributor

Seems related to #2905, where there is ongoing discussion of the same issue. I just ran into this issue myself and would love to help (possibly with updating guides once the code has been written?)

@batjaa
Copy link

batjaa commented Mar 22, 2016

Having the same issue with not being able to customize meta data. Any updates on this?
Would like to be able to serialize the meta data.

@visualjeff
Copy link

visualjeff commented May 13, 2016

I'm pulling back a collection of records (findAll) that includes meta data. How can I read the meta data from this JSON API payload? extractMeta is never called. Can you only pull meta data when you perform a query? Its not clear to me as to when meta data will be processed. I can't really find any documentation related to this so I end up having to step debug my way through for answers. Can someone provide a bit of insight?

UPDATE: For now I override a normalize... hook in the serializer to pull out the meta data.

@JakeDluhy
Copy link
Author

@visualjeff I'm doing the same (overriding normalize)

@pangratz pangratz added the meta label May 27, 2016
@bstro
Copy link

bstro commented Sep 29, 2016

@visualjeff @JakeDluhy would either of you mind posting an example of how you worked around this? running into the same frustrating issue.

@JakeDluhy
Copy link
Author

Hey @bstro for my first pass I overrode normalizeResponse like so...

// serializers/activity.js
// In normalizeResponse, grab the payload and set the params values to the index route
// Would probably prefer to do this in extractMeta, but that hook doesn't work for JSONAPISerializer atm
normalizeResponse(store, primaryModelClass, payload, id, requestType) {
  this._extractLinks(payload.links);

  return this._super(store, primaryModelClass, payload, id, requestType);
},
_extractLinks(linksObject) {
    let parser = document.createElement('a');
    let indexRoute = getOwner(this).lookup('route:index');
    ['prev', 'self', 'next', 'last'].forEach((key) => {
      if(linksObject[key]) {
        parser.href = linksObject[key];
        indexRoute.set(`${key}Params`, parser.search)
      } else {
        indexRoute.set(`${key}Params`, null);
      }
    });
}

Accessing the index route isn't ideal, but it worked. This was my first pass, and I think I later refactored it to move what was in links to meta (I no longer have access to the code base I was working in or I'd just grab the snippet). I think it looked more like

normalizeResponse(store, primaryModelClass, payload, id, requestType) {
  this._extractLinks(payload);

  return this._super(store, primaryModelClass, payload, id, requestType);
},
_extractLinks(payload) {
    ['prev', 'self', 'next', 'last'].forEach((key) => {
      if(payload.links[key]) {
        payload.meta[key] = payload.links[key]
      } else {
        payload.meta[key] = null;
      }
    });
}

@alvincrespo
Copy link

Ran into this issue today as well. Was going to take a shot at implementing extractMeta as part of _normalizeDocumentHelper but not entirely sure behind the reasoning.

I think the original reasoning behind omitting the call to extractMeta for the JSONAPISeriaizer was that meta in JSON API already is in the right place – and if it isn't, it's not JSON API.

@wecc Would you mind clarifying that statement?

My use case for why I ran into this today was to use metadata to add some state to a Service instance in my app. I was going to extend my ApplicationSerializer which extends JSONAPISerializer to override extractMeta and then call a method on my service.

@wecc
Copy link
Contributor

wecc commented Oct 16, 2016

@alvincrespo Originally we didn't implement extractMeta in the JSONAPISerializer because if you're using the JSONAPISerializer then you're already using JSON API, and then meta is already in the right place in the document and shouldn't need to be "extracted". Compared to JSONSerializer/RESTSerializer which has many different looking APIs with meta located in multiple ways and therefor makes more sense to extract.

Not defending the decision, just how I recalled the reasoning at that time. I think calling extractMeta for JSONAPISerializer makes sense and should be implemented.

@alvincrespo
Copy link

@wecc Thanks for the summary - I totally appreciate it. Gives me alot of background information. Ill try and tackle this since I think using extractMeta would be awesome to keep consistency across the serializers.

@urbany
Copy link

urbany commented Oct 20, 2016

Hi, I think the extractMeta can be useful. Here is what I was trying to use it for (camelize all meta keys). It makes no sense to use camel-case for everything except the meta.

// In: {"data":[],"meta":{"page":{"number":1,"size":20,"total-entries":0,"total-pages":0}}}
// Out: {"data":[],"meta":{"page":{"number":1,"size":20,"totalEntries":0,"totalPages":0}}}

export default DS.JSONAPISerializer.extend({
  extractMeta: function(store, typeClass, payload) {
    if (payload && payload['meta'] !== undefined) {
      let meta = camelizeObjectKeys(payload.meta);
      delete payload.meta;
      return meta;
    }
  }
});

function camelizeObjectKeys(value) {
  if(Array.isArray(value)) {
    return value.map(camelizeObjectKeys);
  } else if (value !== null && typeof value === 'object') {
    let obj = {};
    Object.keys(value).forEach(
      k => obj[Ember.String.camelize(k)] = camelizeObjectKeys(value[k])
    );
    return obj;
  }
  return value;
}

@pangratz
Copy link
Member

Basically I am 👍 on having such a hook on the json-api serializer. Since there are 4 places where meta can be placed in a JSON-API document (root level, within data of resource object, relationship and link), we might want to consider this, as the context might be relevant?

I tend to think that this might also go into the direction of a normalizeMeta hook instead of extractMeta 🤔

Should this maybe go through a small RFC before this is addressed? @wecc

@wecc
Copy link
Contributor

wecc commented Oct 20, 2016

Me, @pangratz and @igorT talked about this and since meta can be present in multiple places in a JSON API payload (document, resource, relationship, link) having a single extractMeta hook wouldn't be sufficient – would it only apply to document meta? If yes, then how would you munge meta for a relationship? If no, how would the hook know the context? Several new hooks would be required.

So, instead of adding several new meta specifik hooks the proposed solution is to use the hooks that's already there, normalizeResponse() (document), normalize() (resource) and extractRelationship (relationship). We don't yet fully support meta but since we already have serializer hooks for all the different scenarios it makes sense to use them instead.

@JakeDluhy I wouldn't recommend accessing routes in the serializer but using normalizeResponse() like you do is perfectly fine. The problem here is that we don't yet support links, but we're working on that.

@urbany I've added a PR that explains this a bit more and also gives an example to your exact use case over at #4603

@pangratz
Copy link
Member

Closing since #4603 has been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants