haydn / json-api-store

An isomorphic JavaScript library that acts as an in memory data store for JSON API data.
http://particlesystem.com/json-api-store
MIT License
73 stars 5 forks source link

Check resource data type instead of using schema relation definition #50

Open ronindesign opened 8 years ago

ronindesign commented 8 years ago

Currently mapping for a returned resource (and it's relationships, etc) is handled explicitly by schema definition. Code paths need to be added to allow for dynamic data type returned (array vs. object) despite what is set specified in the schema definition.

Example:

EventStore.define("event", {
name: Store.attr(),
registration: Store.hasMany()
});

Then any call to MyStore.push(json-api-response) throws on the deserialization logic when calling Array.map() on relational data that is not of type array. This case happens (as per the JSON API spec) anytime the relational data result is a single element.

Despite any schema / implied associations between models, this is a convention outlined by the JSON API spec. Whenever a collection of resources would be returned with only one resource, the resource (object) itself is returned, instead of a collection (array) of resources with a single resource.

Reference:

Alternatives to this would imply discrete discrimination between pluralized resources. E.g., we would need two definitions just to handle .hasOne() vs. .hasMany():

EventStore.define("event", {
name: Store.attr(),
registration: Store.hasOne()
});
EventStore.define("events", {
name: Store.attr(),
registration: Store.hasMany()
});
haydn commented 8 years ago

Ah, interesting. I hadn't realised that.

Do you think this is a reasonable approach?

var store = new Store();

store.define("products", {
  "category": Store.hasOne(),
  "comments": Store.hasMany()
});

store.push({
  "type": "products",
  "id": "1",
  "relationships": {
    "category": {
      "data": [
        { "type": "categories", "id": "34" },
        { "type": "categories", "id": "25" }
      ]
    },
    "comments": {
      "data": { "type": "comments", "id": "93" }
    }
  }
});

store.find("products", "1").category.id === "34"; // uses the first resource in the array
store.find("products", "1").comments[0].id === "93"; // wraps the single resource in an array
ronindesign commented 8 years ago

I think we probably only need a change to the "has-many" logic.

The "has-one" seems to be logical as is, if JSON API response returns an array (i.e. many) resources, the json-api-store's hasOne deserialization process should throw at the following line:

if (data.relationships[name].data === null) {
  return null;
} else if (data.relationships[name].data) {
  // This should throw on accessing since data would be an Array.
  return this.find(data.relationships[name].data.type, data.relationships[name].data.id);
}

It makes sense to throw here because at this point the returned data no longer fits the expected model, and the developer probably wants to know this (i.e. either the data is corrupt or the model is incorrectly constructed on the api resource endpoint).

The "has-many" logic just needs to be expanded to include two response data type cases:

  1. Collection of Resources: data will be an Array, already handled intuitively via .map(), keep this!
  2. Only one related resource: data will be an Object, formatted just like the "has-one" case.

So to handle case (2) in addition to the existing logic, we'd just need a type pre-check to see if the response data is a single object. Currently, we have:

if (data.relationships[name].data === null) {
  return [];
} else if (data.relationships[name].data) {
  return data.relationships[name].data.map((c) => {
    return this.find(c.type, c.id);
  });
}

We can simply check using data.constructor or typeof to see if we actually have an array or a single object:

if (data.relationships[name].data === null) {
  return [];
} else if (data.relationships[name].data) {
  if (data.relationships[name].data.constructor === Object) {
    // Taken directly from the "has-one" deserialization process, adjust as needed...
    return this.find(data.relationships[name].data.type, data.relationships[name].data.id);
  } else if (data.relationships[name].data.constructor === Array)
    // Same as before
    return data.relationships[name].data.map((c) => {
      return this.find(c.type, c.id);
    });
  }
}

Or maybe more succinctly:

// === null actually is false if .data is undefined since: typeof null === "object"
// I've used a '!' which handles multiple 'empty' values
// '... == null' would also work, but is probably not desired
if (!data.relationships[name].data) {
  return [];
} else if (data.relationships[name].data.constructor === Object) {
  // Taken directly from the "has-one" deserialization process, but wrapped in array.
  // This is nice because future code can always expect array and do .forEach()
  return [ this.find(data.relationships[name].data.type, data.relationships[name].data.id) ]
} else if (data.relationships[name].data.constructor === Array) {
  // Same as before
  return data.relationships[name].data.map((c) => {
    return this.find(c.type, c.id);
  });
} else {
  // Throw here if desired, or let .data remain undefined to be handled later
  throw new TypeError("Invalid response data type for 'has-many' relationship '"+name+"', expected Array or Object");
}

Then reverse logic would need to be applied for the serialization process, maybe like so:

serialize: function serialize(resource, data, key) {
  if (resource[key]) {
    data.relationships[name || key] = {
      data: (resource[key].length > 1)
      ? resource[key].map(x => {
          return { type: x.type, id: x.id };
        })
      : resource[key] = {
          // Same as "has-one", but selects first (only) object from the array to conform to JSON API spec.
          type: resource[key][0].type,
          id: resource[key][0].id
        }
    };
  }
}

The final result, as per your example definitions, would be:

var store = new Store();

store.define("products", {
  "category": Store.hasOne(),
  "comments": Store.hasMany()
});

store.push({
  "type": "products",
  "id": "1",
  "relationships": {
    "category": {
      "data": [
        { "type": "categories", "id": "34" },
        { "type": "categories", "id": "25" }
      ]
    },
    "comments": {
      "data": { "type": "comments", "id": "93" }
    }
  }
});

store.find("products", "1").category.id === "34"; // Should throw property access error on Array type.
store.find("products", "1").category[0].id === "34"; // Would work, but setting category as Array should probably throw a TypeError in the first place. 

store.find("products", "1").comments[0].id === "93"; // This is perfect!
store.find("products", "1").comments.id === "93"; // Shouldn't work, ideally hasMany() should always return Array.
store.find("products", "1").comments.forEach(function(key,value) {
  console.log(value); //
}); // Should print 93 as expected.

This way any code later in the stack will be able to always expect and depend on either a directly accessible object for "has-one" xor an enumerable array for "has-many".