Ember Data background fetch RFC

[bmac]

Rendered

[bmac]

I thought about this some more last night and realized the RFC doesn't specify how a real time adapter would be able to signal to the store that it does not need to perform a background fetch. To solve this I think we would need to add 2 methods to the Adapter API.

New Adapter Methods

The first method shouldRefetchRecord would be used to make initial decision weather to refetch the record form the adapter or return the cached record from the store. This could method could be used to implement caching logic (e.g. only refetch this record after the time specified in its cache expires token) or for improved offline support (e.g. always refetch unless there is no internet connection then use cached record),

This record would only be called if the record was already found in the store and is in the loaded state.

{
  /**
   `shouldRefetchRecord` returns true if the adapter determines the record is
   stale and should be refetch. It should return false if the record
   is not stale or some other condition indicates that a fetch should
   not happen at this time (e.g. loss of internet connection). 

   This method is synchronous.

   @method shouldRefetchRecord
   @param {DS.Store} store
   @param {DS.Model} record
   @param {Object} adapterOptions
   @return {Boolean}
   */
  shouldRefetchRecord: function(store, record, adapterOptions),
}

The second method shouldBackgroundUpdate would be used by the store to make the decision to re-fetch the record after it has already been returned to the user. This would allow realtime adapter to opt out of the background fetch if they already are subscribing to changes on the record.

{
  /**
   `shouldBackgroundUpdate` returns true if the store should re fetch a
   record in the store after returning it to the user to ensure the
   record has the most up to date data.

   This method is synchronous.

   @method shouldBackgroundUpdate
   @param {DS.Store} store
   @param {DS.Model} record
   @param {Object} adapterOptions
   @return {Boolean}
  */
  shouldBackgroundUpdate: function(store, record, adapterOptions),
}

Store Find Methods

Currently this rfc proposes using store.fetch to implement this new behavior. I fear this will be confusing since the term "fetch" is already used in Ember Data to tell the store to get the record from the adapter regardless of its caches state.

With the introduction of the new adapter methods mentioned above I think it would be possible for the background fetch behavior to happen in store.find. First for consistency with fetchById and fetchAll the existing find method would be deprecated and replaced with findById and findAll. The new findById and findAll methods would become smarter and collaborate with the adapter using the two new methods mentioned above to achieve the background updating and conditional fetching described in this RFC.

Piggy backing on some of the work done in RFC #27 I think public API for finding records in the store could look like this.

{
  /**
   Use `peekById()` to retrieve a record by it's type and id if present in the
   store. If there's no record matching the type and id `null` will be
   returned.

   This method is synchronous.

   `getById` will be deprecated and renamed to `peekById`.

   @method peekById
   @param {String} type
   @param {String} id
   @return {DS.Model|null} record
   */
  peekById: function(type, id),

  /**
   Use `peekAll()` to retrieve all records present in the store, by type. 

   This method is synchronous.

   `all` will be deprecated and renamed to `peekAll`.

   @method peekAll
   @param {String} type
   @return {DS.RecordArray} records
   */
  peekAll: function(type),

  /**
   Use `findById()` to retrieve a record by it's type and id if present in the
   store.

   If the record is in the store the store will ask the adapter if the
   record is stale. If the record is stale then the store will ask the
   adapter to refetch the record. If the record is not stale then it
   will resolve with the cached record immediately.

   If there's no record matching the type and id the store will ask the
   adapter to fetch the requested record.

   This method is asynchronous.

   `find(type, id)` will be deprecated and replaced by `findById`.

   @method findById
   @param {String} type
   @param {String} id
   @param {Object} adapterOptions
   @return {DS.PromiseObject} record
   */
  findById: function(type, id, adapterOptions),

  /**
   Use `findAll()` to retrieve all the records for a given type.

   Before asking the adapter to find all the records the store will
   first ask the adapter if the existing records it knows about are
   stale. If they are stale then findAll will wait for the adapter to
   find the records. Otherwise it will resolve immediately with all of
   the known records in the store.

   This method is asynchronous.

   `find(type)` will be deprecated and replaced by `findAll`.

   @method findAll
   @param {String} type
   @param {Object} adapterOptions
   @return {DS.RecordArray} records
   */
  findAll: function(type, adapterOptions),

  /**
   Use `fetchById()` to force the store to always fetch the latest
   version of a record from the adapter.

   This method is asynchronous.

   @method fetch
   @param {String} type
   @param {String|Integer} id
   @param {Object} adapterOptions
   @return {DS.PromiseObject} record
   */
  fetchById: function(type, id, adapterOptions),

  /**
   Use `fetchAll()` to force the store to always fetch all records
   by type from the adapter.

   This method is asynchronous.

   @method findAll
   @param {String} type
   @param {Object} adapterOptions
   @return {DS.RecordArray} records
   */
  fetchAll: function(type, adapterOptions),

  /**
   Use `fetchQuery()` to ask the adapter to fetch all records by type
   that match a query object. This method will always fetch new
   records and never resolve with cached records from the store.

   This method is asynchronous.

   `findQuery(type, query)` will be deprecated and replaced by `fetchQuery`.

   @method fetchQuery
   @param {String} type
   @param {Object} query
   @param {Object} adapterOptions
   @return {DS.RecordArray} records
   */
  fetchQuery: function(type, query, adapterOptions),

  /**
   Use `fetchQueryOne()` to ask the adapter to fetch all records by type that match a query object and
   it resolves with the first record returned.

   This method is asynchronous.

   @method fetchQueryOne
   @param {String} type
   @param {Object} query
   @param {Object} adapterOptions
   @return {DS.PromiseObject} record
   */
  fetchQueryOne: function(type, query, adapterOptions),
}

This results in a table that looks like this.

multiplicity	local data	fetch if not local; check for stale if local	fetch always
collection	store.peekAll	store.findAll	store.fetchAll
single object	store.peekById	store.findById	store.fetchById
collection query	Out of scope of this RFC	Out of scope of this RFC	store.fetchQuery
single object query	Out of scope of this RFC	Out of scope of this RFC	store.fetchQueryOne

[tchak]

👍

[tchak]

I am not sure I am a huge fan of the name fetchQueryOne. I think I prefer fetchQueryFirst. It is closer to what it dose.

[fivetanley]

👍

[igorT]

Need to figure out what to do for preloadedData for findById

[bmac]

There has been some talk of changing ById to Record (e.g. fetchById -> fetchRecord). I think that would make fetchQuery -> fetchQueryAll and fetchQueryOne -> fetchQueryRecord.

[thaume]

I second the fact that fetchQueryFirst would make more sense. fetchQueryOne was intended to fetch with an id and the name came from it (when you fetch with an id you always get one result).

Anyway let's go with fetchQueryFirst, I'll update my PR once we've validated this RFC !

[cibernox]

Maybe it's because I'm pretty used to ember-data current behaviour, but I think that return false from shouldBackgroundUpdate would be a better default.

It is retrocompatible with what ED does now and I feel that refetch information that I already have is something you should opt-in to instead of opt-out.

[bmac]

Some of the motivation for the background update to be true by default is the belief that background updates provide a best of both worlds approach for new developers.

As for the retro-compatibility I agree with you there. The current thinking is for this feature will be introduces with shouldBackgroundUpdate set to false an a warning that the default will change in the next major version of Ember Data.

[mmun]

This is done and merged right? I think the ember data team should have access to manage their RFCs or at least get their own repo in ember-data/rfcs

[knownasilya]

Yes, please close this if it's done. 👍

[fivetanley]

Merged and assigned in 10638ee. Thanks!