Overhaul store.push and store.pushPayload

text/0000-ember-data-overhaul-store-push.md

@@ -0,0 +1,331 @@
+# ember-data-overhaul-push
+
+- Start Date: 2016-08-01
+- RFC PR: (leave this empty)
+- Ember Issue: (leave this empty)
+
+# Summary
+
+A new `store.normalizePayload(modelName, payload)` is added which normalizes a
+payload into JSON-API format. This normalized payload can then be used with
+`store.push()` or the `push()` methods on the `BelongsTo` and `HasMany`
+references.
+
+Add a `store.pushDocument()` which takes a JSON-API document and returns a
+`DocumentReference` (proposed in
+[RFC#160](https://github.com/pangratz/rfcs/blob/ember-data-links-meta-improvements/text/0000-ember-data-links-meta-improvements.md#dsdocumentreference)).
+
+# Motivation
+
+## Status quo
+
+Consider an application gets the data not only from the adapter, but also from
+a different source (WebSockets for example). Ember Data has a
+`store.pushPayload()` method, which allows to push records into the store
+outside of adapters. It does however - compared to `store.push()` - not return
+the pushed record(s).
+
+The issues with `store.push` and `store.pushPayload` can be summarized as
+follows:
+
+- `store.pushPayload` doesn't return the pushed records (`store.push` does)
+- `store.pushPayload` calls `serializer.pushPayload` which invokes
+  `store.push`, though pushing data into the store shouldn't be the concern of
+  the `serializer`
+- `store.push` and `store.pushPayload` don't support `meta` and `links`
+- the `modelName` passed to `store.pushPayload` is not passed explicitly to the
+  serializers' `pushPayload`, making it difficult for the serializer to know
+  what the primary data is (implicitly this is known by checking which
+  serializer is invoked)
+
+Also `BelongsToReference#push()` and `HasManyReference#push()` expect a
+JSON-API document, but there is currently no easy way to normalize a payload
+into the expected format:
+
+- `serializer.normalizeResponse` is cumbersome, as it takes 5 parameters
+- as stated in the documentation,
+  "[`store.normalize`](http://emberjs.com/api/data/classes/DS.Store.html#method_normalize)
+  converts a JSON payload into the normalized form that `push` expects", but it
+  uses `serializer.normalize`, which only normalizes a single object
+  - since the documentation is misleading, this seems like a bug
+    ([#4285](https://github.com/emberjs/data/issues/4285))
+  - this would be a good name for a method which normalizes a payload, but it's
+    not clear how it can be refactored to support whole payload
+
+
+The [`ds-pushpayload-return`](https://github.com/emberjs/data/pull/4110)
+feature does address the problem of getting the pushed record(s), but there are
+some issues:
+
+- lock down the API for `store.pushPayload` to always return a materialized
+  record(s)
+- once locked down, we cannot refactor for lazy materialization (started in
+  [PR#4272](https://github.com/emberjs/data/pull/4272))
+- apps which heavily use `pushPayload` will then not profit from this
+
+## Proposed solution
+
+- add `store.normalizePayload` which normalizes a payload into JSON-API format
+- add `store.pushDocument` which pushes data and included of a JSON-API
+  document into the store without materialization of the pushed records
+
+# Detailed design
+
+## `store.normalizePayload`
+
+Add a `store.normalizePayload` method which takes an optional `modelName`
+indicating the primary model (and hence which serializer should be used) and
+the payload which should be pushed:
+
+``` js
+/**
+  Normalize a payload into a JSON-API document, using the
+  optional modelName as serializer for the primary data.
+
+  If `modelName` is omitted, the application serializer is
+  used. The `modelName` is passed down to the serializers'
+  `normalizePayload` method - this can be used to determine
+  what the primary data of the normalized payload is.
+
+  @param modelName {String} optional modelName, indicating the
+                            primary model of the payload
+  @param payload {Object} payload which should be normalized
+*/
+normalizePayload(modelName, payload) {}
+```
+
+If a serializer implements `pushPayload`, then the normalizing-a-payload
+functionality is basically already there. The problem is that the `serializer`
+is expected to push the data into the store directly via `store.push`, instead
+of returning the normalized payload. We can make use of this and re-use the
+existing `pushPayload` on the serializer to provide a `normalizePayload` out of
+the box, without immediate actions being required by app and addon authors.
+
+_:zap: WARNING :boom: hacks ahead :boom: WARNING :zap:_
+
+Given this implementation of `normalizePayload` on the store:
+
+``` js
+DS.Store.reopen({
+
+  normalizePayload(modelName, payload) {
+    let serializer = this.serializerFor(modelName);
+
+    if (serializer.normalizePayload) {
+      return serializer.normalizePayload(this, ...arguments);
+    }
+
+    if (serializer.pushPayload) {
+      deprecate("rename `serializer.pushPayload` to `serializer.normalizePayload`" +
+                " and return normalized payload instead of pushing it into the store");
+      return serializer.__normalizePayload(this, payload);
+    }
+
+    assert("serializer needs to implement normalizePayload");
+  }
+
+})
+```
+
+Then the `DS.Serializer` can be adapted as follows to re-use the existing
+payload-normalization behavior:
+
+``` js
+DS.Serializer.reopen({
+
+  __normalizePayload(store, payload) {
+    const originalPushMethod = store.push;
+    let normalizedPayload;
+
+    // temporarily patch store.push, which is invoked in
+    // serializer.pushPayload
+    store.push = function(_normalizedPayload) {
+      normalizedPayload = _normalizedPayload;
+    }
+
+    this.pushPayload(store, payload);
+
+    // restore original store.push
+    store.push = originalPushMethod;
+
+    return normalizedPayload;
+  }
+
+})
+```
+
+Drawback: this only works if `store.push` is used only once within
+`serializer.pushPayload` though. A check could be added to see if `store.push`
+is invoked multiple times within `serializer.pushPayload`. If this is the case,
+then we can't automatically and reliably infer the
+`serializer.normalizePayload` behavior.
+
+The documentation for `DS.Serializer` is updated as follows:
+
+``` js
+/**
+  Normalize a payload into JSON-API format.
+
+  The optional `modelName` indicates which data of the input
+  payload should be the primary data of the normalized payload.
+  If no `modelName` is passed to `store.normalizePayload`, then
+  this parameter is `null`.
+
+  @param store {DS.Store}
+  @param modelName {String} optional name of the primary model
+  @param payload {Object}
+  @return Object JSON-API document
+*/
+normalizePayload(store, modelName, payload)
+```
+
+## `store.pushDocument`
+
+Add a `store.pushDocument` which takes a JSON-API document and returns a
+[`DocumentReference`](https://github.com/pangratz/rfcs/blob/ember-data-links-meta-improvements/text/0000-ember-data-links-meta-improvements.md#dsdocumentreference)):
+
+``` js
+DS.Store.reopen({
+
+  /**
+    Push given JSON-API document into the store, and get the
+    reference to the pushed document.
+
+    Use this method if data should be pushed into the store,
+    without the need for getting the pushed records. This is
+    potentially faster than `store.push`, since unnecessary
+    materialization of records is avoided.
+
+    @param payload {Object} JSON-API document
+    @return {DS.DocumentReference} pushed document
+  */
+  pushDocument(payload)
+
+});
+```
+
+# Code Samples
+
+The new methods can the be used to push a payload into the store and get the
+relevant data:
+
+``` js
+let normalized = store.normalizePayload(payloadFromWebSocket);
+let pushedRecords = store.push(normalized);
+```
+
+Or using references instead to get the associated `meta`:
+
+``` js
+// payload === {
+//   book: {
+//     id: 1,
+//     author: "Tobias Fünke",
+//     _meta: {
+//       revision: "1-tmim"
+//     }
+//   },
+//   family: {
+//     id: 1,
+//     name: "Bluth"
+//   }
+// }
+let normalized = store.normalizePayload("book", payload);
+let docRef = store.pushDocument(normalized); // DS.DocumentReference
+let meta = docRef.meta();
+```
+
+And pushing array data:
+
+``` js
+// payload === {
+//   people: [{
+//     id: 1,
+//     name: "George Michael"
+//   }, {
+//     id: 2,
+//     name: "Buster"
+//   }],
+//   _meta: {
+//     count: 100
+//   }
+// }
+let normalized = store.normalizePayload("person", payload);
+let docRef = store.pushDocument(normalized); // DS.DocumentReference
+```
+# How We Teach This
+
+Update the dedicated section [in the
+guides](https://guides.emberjs.com/v2.7.0/models/pushing-records-into-the-store/)
+to outline how data can be pushed into the `store` besides the adapter. Also,
+the difference between `store.push` and `store.pushRef` and their different use
+cases need to be explained.
+
+Inform addon authors (blog post, ...) about new `normalizePayload` hook on the
+serializer; with the following upgrade path:
+
+- extract normalize functionality within `pushPayload` into `normalizePayload`
+- use `normalizePayload` within `pushPayload`
+- release party :tada: :balloon:
+
+# Drawbacks
+
+The new `serializer#normalizePayload` is another `normalize` hook which needs
+to be implemented by ember data addon authors and app developers. The
+difference to the `normalizeXXXResponse` hooks might not be immediately clear.
+As pointed out [in #3838](https://github.com/emberjs/data/pull/3838#issuecomment-147706227) and
+[in #3676](https://github.com/emberjs/data/issues/3676), there was a
+`serializer#normalizePayload` in `1.13` which has been renamed to
+`normalizeResponse` instead. Re-adding a `normalizePayload` is confusing.
+
+As mentioned already, the patch re-using `serializer.pushPayload` only works if
+`store.push` is used only once. Though a check for multiple used `store.push`es
+within `serializer.pushPayload` can be added, it is not 100% certain that all
+use cases are supported with this.
+
+Since we have a `store.normalizePayload` now, it is possible to normalize a
+payload and get the corresponding models via `store.push`. There is no
+immediate need for `store.pushDocument`. But if a pushing data into the store
+without materializing records should be possible, having a `pushDocument` is
+needed. So it basically it would boil down to this:
+
+- use `store.push` if you want the `DS.Model`s
+- use `store.pushDocument` if you don't need the materialized `DS.Model`s
+
+# Alternatives
+
+As outlined in
+[#3576](https://github.com/emberjs/data/issues/3576#issuecomment-124051628),
+normalizing a payload and pushing it into the store can be done via:
+
+``` js
+let model = store.modelFor(modelName);
+let serializer = store.serializerFor(modelName);
+let normalized = serializer.normalizeSingleResponse(store, model, payload);
+let record = store.push(normalized);
+```
+
+But this is verbose and cumbersome. Also, you need to know beforehand if the
+primary data of the payload represents a single resource or if it describes a
+resource collection.
+
+# Unresolved questions
+
+- should `store.pushPayload` return the pushed records, to be consistent with
+  `store.push`?
+- should `store.pushRef` return array of `RecordReference`s instead? What
+  should happen when different primary types are pushed?
+
+``` js
+store.pushRef({
+  data: [
+    { type: "person", id: 1 },
+    { type: "family", id: 2 }
+  ]
+});
+```
+- materialization mentioned in the added docs might be the first time this is
+  mentioned in public (internal model might be unknown to most ember data
+  users, with good reason)
+- should `store.pushPayload` be deprecated in favor of `store.normalizePayload`
+  and `store.push` or `store.pushRef`, depending on the use case?