[bugfix beta] invalidate link promise on inverse unload

[hjdivad]

[bugfix beta] Invalidate link on inverse unload

Invalidates link promises when inverse records are unloaded. Subsequent fetches of a hasMany unloaded in this way will load from the link again, rather than loading whatever ids were returned from the prior link load.

[hjdivad]

this includes the test from #5354 (with some tweaks) + fix

[jlami]

Thanks for the extra work on this!

This does sound like it would solve my problems, but I'm not sure if this might have performance penalties. As this would mean one extra request after the unload. That request could be avoided if the unload would be seen as a delete as it is in the sync version.

I'm also a bit concerned that this might mask the problem with rejections that might happen in the race condition that could happen between the lookup of the returned ids that the hasMany query might give and the following record fetches. The records could have vanished server side between these points and the ManyArray does not handle this well.

But like I said, this will probably fix my problems. And I was thinking that returning the whole record as a 'included' data and not only the ids might fix this while fetching the link. But I'm not sure now that ember-data would not still issue the fetch anyway after that. I Will look into that later.

[Herriau]

@hjdivad This has done more harm than good for our use case. We typically use unloadRecord() to have the DS Store "forget" about records that have been implicitly deleted server-side as a side effect of some save(). For our use case the cost of the extra requests to reload all the relationships those unloaded records were involved in is a deal breaker (and completely unnecessary).

This fix has left us with no clean way to mark a DS record as "implicitly" deleted. The work around we are using currently is the following.

// adapters/application.js
export default class ApplicationAdapter extends JSONAPIAdapter {
  async deleteRecord(store, type, snapshot) {
    if (get(snapshot, 'adapterOptions.assumeOnly')) {
      return;
    }

    return await this._super(...arguments);
  }
}

await someRecord.save(); // Has the side effect of removing other record server-side
await someOtherRecord.destroyRecord({ adapterOptions: { assumeOnly: true } });

[runspired]

@Herriau Even if we didn't invalidate the link promise unload here would result in a new request being needed if a relationship came into contact with the record.

unloadRecord is not intended to mean "forget this" or "treat this as deleted" although folks often attempt to use it that way. It literally means "move this record to the not-loaded" state. If nothing in the graph needs to retain a reference to that record, we additionally remove it from the cache.

Your approach of using adapterOptions to explicitly delete the record is a common approach, and you can even take it a step further and put it into the initial deleteRecord call.

record.deleteRecord();
record.save({ adapterOptions: { sideDeletes: ['aRelationship'] } });

class Adapter {
  async deleteRecord(store, { modelName }, snapshot) {
    if (snapshot!.adapterOptions!.isLocalDelete) {
      return { data: null };
    }
    const localDeletions = snapshot.adapterOptions.sideDeletes;
    await fetch(`/${modelName}/${snapshot.id}`, { method: 'DELETE' });
    
   let promises = [];
   let  localDelete = { adapterOptions: { isLocalDelete: true } };
   snapshot.eachRelationship((key, meta) => {
     if (localDeletions.contains(key)) {
       if (meta.kind === 'belongsTo') {
         let record = snapshot.record.belongsTo(key);
         promises.push(record.destroyRecord(localDelete));
       } else {
         let records = snapshot.record.hasMany(key);
         records.forEach(r => promises.push(r.destroyRecord(localDelete)));
       }
     }   
   });
    await all(promises);

    return { data: null };
  }
}

Another common approach to pushing a client-side delete doesn't involve the adapter at all, but instead takes advantage of eliminating the server-state for any potentially entangling relationships prior to unloading so that the record will be cleared from the graph.

function pushDeletion(store, type, id) {
  let record = store.peekRecord(type, id);
  
  if (record !== null) {
    let relationships = {};
    let hasRelationships = false;
    
    record.eachRelationship((name, { kind }) => {
      hasRelationships = true;
      relationships[name] = {
        data: kind === 'hasMany' ? [] : null
      };
    });

    if (hasRelationships) {
      store.push({
        data: {
          type,
          id,
          relationships
        }
      });
    }

    record.unloadRecord();
  }
}