Data: Ember Data creates duplicate records

Created on 22 Mar 2014  Â·  80Comments  Â·  Source: emberjs/data

Using Ember 1.4, EmberData beta 7 with default REST adapter. I don't have a JSBIN because problem only shows when there's a success reply from the server. Here's the scenario:

  1. create 2 routes, and transition to the 1st route.
  2. create a record: amd = store.createRecord ('myModel', myRecord);
  3. save the newly created record: md.save();
    note - server must return a success for problem to show. My server returned {status, 204}
  4. now transition to route 2 via link-to or something else, then hit the browser's back key to go
    back to route 1.
  5. when the 'back' button is pressed, Ember makes a GET request to the server.
    (why does Ember send a GET message when it already has it in the identity map?)
    Server replies with one instance of the saved record. Ember displays two instances of
    the saved record. Ember inspector also shows two instances.
  6. Now if the browser refresh button is pressed, Ember sends a GET to the server. Server replies
    with 1 record, and everything looks right again.

Most helpful comment

Currently I have a workaround that requires no code in your controllers, routes (or whatever place you put your saving stuff).

Basically you need a mixin that you use on the parent model that contains the dependent child records:

import Ember from 'ember';

export default Ember.Mixin.create({
    save: function() {
        var recordsToDelete = [];
        this.eachRelationship((name, descriptor) => {
            if (descriptor.kind === 'hasMany') {
                this.get(name).forEach((object) => {
                    if (object.get('isNew')) {
                        recordsToDelete.push(object);
                    }
                });
            }
        });

        var promise = this._super(...arguments);
        promise.then(() => {
            recordsToDelete.forEach((record) => {
                record.deleteRecord();
            });
        });
        return promise;
    }
});

Then you can use this mixin in your models, like this:

import DS from 'ember-data';
import DependentRelationships from '../mixins/dependent-relationships';

export default DS.Model.extend(DependentRelationships, {
    tags: DS.hasMany('tag', { async: false })
});

Then when you save the above model, it will take care of the hasMany relationship if it got some new ones embedded. I hope this might be useful for someone, for me it is working well.

this.get('model').save(); // thats all you need to use now, you got some better client code now

@florent-blanvillain it is something like yours approach but it doesn't require you to have that dirty code in your client code.

All 80 comments

I have a bit more information. I tried returning status 201 and 200 (i.e., 200 with no content returned). Both resulted in the promise not fulfilling; 201 should have worked. 200, maybe not since I didn't return any data.

I believe the GET message (i.e., item 5 in my original post) is tied to the problem. Ember Data is confused about what's in the store and sends an unnecessary GET request for data it already has in the store. When the server returned the requested record (the record that was just saved), Ember just inserts it into the cache, resulting in duplicates.

I have found the same problem and without creating any new record, just going to another route and then pressing the back button

There seems to be quite a few problems with EmberData's handling of model relationships. Unfortunately, I'm too far committed to scrap EmberData entirely; but I've given up trying to find workarounds to these problems. I'm just looking forward to the next beta release, which hopefully contains fixes to some of these issues.

Another problem with EmberData (and Ember itself to a certain extend) is that there are lots of obsolete documentations, advises, and examples on the internet that can lead you astray, especially if you're just starting out with Ember. I'm hoping the Ember team will soon provide some decent documentations containing recommendations on how to solve specific problems.

You can try returning the newly-created DB object instead of a 204.

e.g.

200 {comment: {id: 'xxx', a: 'b', b: 'c'}}

You can also tell Ember Data to disregard the server response entirely, which may work:

export default DS.RESTSerializer.extend({
  extractCreateRecord: function() {
    // do nothing
  }
});

Just wanted to chime in a little and say please make sure you're handling the case where the record doesn't create at the API (and/or where you want it revoked by transitioning away from the route where the record exists)... it should rollback on the record otherwise the record will still be in the store. That would definitely lead to "duplicates" appearing in your store from you visiting the same route again...

When I first started using Ember, I didn't realise I had to provide a deactive in the router that would rollback the model and I ended up with duplicates in my store, too. H2H

Has you model got the proper id?

Yes, model does have the proper id. Also, I've already tried returning 200 with the proper JSON. Returning 200 with JSON fixed the duplication problem in the parent record, but caused duplication in the hasMany record. I have full control over what the server returns, and have spent endless hours trying many different combinations in attempt to find a temporary fix; I finally gave up.

There are bugs in Ember Data; it's as simple as that. As mentioned, I'm just waiting for the next beta release. BTW, canary built as of 3 days ago still has the problem.

@ptmoy2 I'm using ember data. I wonder why I haven't encountered the bugs you're having (I'm using beta 7). Can you jsbin this?

Julian, does the model in your application have a hasMany relationship? Simple (i.e., unrelated) models work fine in Ember Data if server returns proper JSON (including a server-generated id) along with status 200 or 201.

@ptmoy2 Yeah, I have a lot of hasMany and belongTo relationships. It even has recursive relationships (ie parent/child on the same model).

I thought 204 was the correct status for created. That's what my server uses, I'm fairly sure. Using REST Adapter.

My app is in no way simple.

204 sort of works, but leaves the isDirty flag set, which is incorrect (also, if I remember correctly, returning 204 creates a duplicate in the parent record in my app).

This is actually my 3rd Ember app, so I know the basics fairly well. I don't know why you're not seeing the problem, but wouldn't be surprised if there's a combination that actually works. It's just that I haven't found it. My current work around is to return 200 with json, then force a reload of the model, which fixes the store. In my app, the Ember Viewer actually shows 2 records that are identical, so I know it's confused.

My application is really big, so isolating problem into a jsbin is not easy. Based on everything I've read, I speculate problem will be fixed by the time we get out of beta. I don't want to waste more time messing around with this problem during beta. I'll take another look at doing a jsbin if problem still exists in version 1.0.

When I returned 200 with json as described above, this is the exact problem I'm seeing in my app:

http://discuss.emberjs.com/t/proposal-fix-saving-new-embedded-records-creates-duplicates/3677

@ptmoy2 sucks, dude. So I take it nothing there helped you? Just out of curiosity, are you using sideloaded records? I'm only use async: true, so if you are that might be why I haven't seen any of these duplicated record issues. Of course, without seeing some code it's a bit tricky to say! :)

I use sideloaded records when fetching from the server. But I overwrite the serializeHasMany hook in the serializer to send the hasMany records embedded when saving.

Discussion in that link helped a little bit. But I don't really understand the fix involving _clientId. Is _clienId some sort of undocumented hidden identifier I can use?

Supplementary information

I'm appending information from discuss.emberjs.com that I think is helpful:


Currently there is a problem saving _new_ embedded records in a hasMany relationship. Ember has a hard time reconciling the returned embedded records with the ones already in memory and creates duplicates.

The problem:

You have a hasMany relationship:

User = DS.Model.extend({
  name: DS.attr(),
  posts: DS.hasMany('post')
});
  1. You create a new record within that array. this.get('posts').pushObject(newRecord)
  2. You call this.save() on the parent object and it embeds the new object, which has no server assigned id, in the JSON payload.
  3. Your server sees the embedded object and persists it, giving it an id.
  4. Server responds with the embedded object which now has an id.
  5. You normalize your payload so the embedded object is side-loaded (and now has an id)

    {
     "user": {
       "name": "Tim",
       "posts": ["1"] 
     },
     "posts": [
       {
         "id": "1"
         ...
       }
     ]
    }
    
  6. Ember sees this as a new record and pushes it into the store.
  7. The hasMany relationship looks up the reference to this id and appends the record, so it now references the original (no id) and the new record (with an id).

At this point you have duplicate records until you call reload() on the record. Ember has no way of knowing that these two records were supposed to be the same thing.

I have a working branch that solves this issue by sending a client specific identifier to the server (defaults to _clientId but is configurable) in any unsaved embedded records.

The server needs to be aware of this special key and pass it back unchanged with any embedded objects.

e.g. Request:

{
  "user": {
    "id": "1",
    "name": "Tim",
    "posts": [
      {
        "_clientId": "ember212",
        "body": "test"
      }
    ]
  }
}

Response:

{
  "user": {
    "id": "1",
    "name": "Tim",
    "posts": [
      {
        "id": "1",
        "_clientId": "ember212",
        "body": "test"
      }
    ]
  }
}

Using the fix I have implemented, Ember is able to reconcile these records and prevent duplicates in the resulting hasMany array.

I was hoping to get some feedback as to the actual implementation.

Ember Data pre 1.0 had an internal client id and a method for looking up a record by client id. It would have been perfect to use in this situation. 1.0 removed this.

Instead, I am using Ember.guidFor(record) as an alternative, but there is no global guid lookup to find an object by guid. Because of this, I have made my own clientId to record lookup in the EmbeddedRecordsMixin called clientIdMap. Once it reconciles a client side record it removes the entry from the map. This works - but has a small problem - Rails ActiveModelSerializer likes to send back a HTTP 204 with no content when you _update_ a record. If this happens, the lookup entryis not removed and over time the clientIdMap will grow and grow.

I don't really like the idea of the clientIdMap.

Is anyone aware of a better (and performant) method for looking up a record that has no id yet?

There are other less attractive options I've already pondered:

  • Store an expiration time on the clientIdMap entry and clean out old records when they expire (yuck)
  • Clear the entire map every time we finish extractSingle -- this will probably fail to work if two responses arrive out of order.
  • Loop through the record's hasMany relationship, looking for the guid (wont scale and the original record is not readily available inside the extractSingle method)

Any thoughts would be welcome.

_Keep in mind, I am trying to keep this solution decoupled from the ActiveModelSerializer._


SOURCE: http://discuss.emberjs.com/t/proposal-fix-saving-new-embedded-records-creates-duplicates/3677

Also, this issue may be related https://github.com/emberjs/data/pull/1656

This is still a big issue in latest Ember data. Anyone maybe found a good temporary workaround (not involving server side changes)? @igorT

@steffenbrem Would you mind trying with master/canary?

Igor made a bunch of enhancements recently related to relationship handling (https://github.com/emberjs/data/pull/2576) which I (if we are lucky) may have solved this. Haven't had time to look at it myself though, so I don't know.

@sandstrom Yeah I have the same issue on the master branch. The thing is that ember data does test for duplicated records when you save a record with an embedded hasMany relationship. But the ID in the test is hardcoded, in the real world, you would not know the ID when you create a new Comment on the client side.

Look, here is the test that I was talking about. You can see that all the records are already given an ID. How would you know the ID of a newly created comment on the client side that isn't even created yet. It doesn't make much sense to me.

test("create - relationships are not duplicated", function() {
  var post, comment;

  Post.reopen({ comments: DS.hasMany('comment') });
  Comment.reopen({ post: DS.belongsTo('post') });

  run(function(){
    post = store.createRecord('post', { name: "Tomtomhuda" });
    comment = store.createRecord('comment', { id: 2, name: "Comment title" }); // it should test without the predefined ID.
  });

  ajaxResponse({ post: [{ id: 1, name: "Rails is omakase", comments: [] }] });

  run(post, 'save').then(async(function(post) {
    equal(post.get('comments.length'), 0, "post has 0 comments");
    post.get('comments').pushObject(comment);
    equal(post.get('comments.length'), 1, "post has 1 comment");

    ajaxResponse({
      post: [{ id: 1, name: "Rails is omakase", comments: [2] }],
      comments: [{ id: 2, name: "Comment title" }]
    });

    return post.save();
  })).then(async(function(post) {
     equal(post.get('comments.length'), 1, "post has 1 comment");
  }));
});

When you would create a new Comment without the predefined ID of 2. Than as expected, ember data will fail. Here is a screenshot with the modified test (I only removed "id: 2" from the creation of the comment):

@steffenbrem Thanks for testing, it remains then (unfortunately).

Ember supports both client- and server-generated IDs. But I agree that in this case an additional test, without client-side IDs, would be useful (since this one doesn't capture the issue).

For reference, here is the relevant json-api issue: https://github.com/json-api/json-api/issues/202

One way to solve this is client-side generated IDs (can be coupled with server-side validation of the id).

Another is to use some kind of transientId. I think this is easiest (moving to client-side generated IDs will be cumbersome for many existing applications).

@sandstrom I changed the test for duplicated records in this PR. Hopefully it will be picked up, not sure how difficult it is to solve.

Will take a look in the morning

@igorT Thanks :+1: Appreciate it!

@igorT I am bumping into this problem as well and it is holding up a pull-request I am working on for Ember Data. Have not been able to find a solution for it so a fix would be awesome!

@igorT I think the whole model should be replaced with the one returned by the server, correct? Instead of trying to merge it.

I've run into the same problem with a belongsTo relationship and am struggling to find a workaround. I was hoping to be able to just remove the original record (i.e. the one without the id) but it appears to still have a link to the parent record (stored in _implicitRelationships) which causes the link between the duplicated record (i.e. the one with the id) and the parent record to be removed when the original record is deleted.

Simple(ish) example here: http://emberjs.jsbin.com/fevepogili/1/edit?html,js,output

Steps to recreate:

  • Save a new task with a description and category
  • Delete the first category in the list (which shouldn't be linked to anything anymore)
  • The task in the list is no longer linked to a category

Am I missing something?

I am strongly in favour of @sandstrom 's suggested approach. Given that JSON API has de-milestoned "sideposting" from 1.0, we're going to be out of luck for a while.

This issue is rather tricky because Ember Data doesn't have enough context to know if the relationship returned from the server is the same one that already exists on the record or if the existing relationship is a new client side record that the server doesn't know about yet.

@igorT maybe the solution is to add a new method to the store that adapters can call to mark a record in the created state as saved.

@bmac Yes, you are absolutely right, that's the meat of it. But I think using a transient id, mirrored back by the server, should be pretty straightforward way of solving this.

{
  "type": "articles",
  "id": "1",
  "attributes": {
    "title": "Rails is Omakase"
  },
  "meta": {
    "transientId": "c1001"
  }
}

_(example of what a payload could look like)_

@bmac But you do know that if the server is returning the relationship sideloaded, it will have the most recent data, so you can replace the local data with all the data returned from the server?

I think it is not necessary to pass a client id or transient id to the server. You could also fix this by just replacing all the local records of the relationship with the ones returned from the server. What would be wrong with that approach? I think this is the easiest and cleanest way to do it.

The only issue with this that I can think of is that you lose any references to the locally created relationship instance that you had before saving. I am not sure if this will be a dealbreaker.

@steffenbrem It works if the server always include all of them. It would work for our use-case, but for larger collections the server may not return all associated records (e.g. a blog post with a thousand associated comments).

I have also gotten duplicates in Ember Data with a far simpler use case than most of the examples here. Consider this:

this.socket.on('message', function(data) {
  store.push(store.normalize('message', data));
});

When multiple instances of a message (with the ID set on the server) stream in store.push() doesn't update the existing record but instead creates duplicates (all with the same ID). I'm not 100% certain that it's the same issue as this one but I figured I'd start here. Ember v1.13.3 and Data v1.13.4.

Working around it like this provides the behavior that the documentation states store.push() should have but it's obviously not ideal for many use cases.

this.socket.on('message', function(data) {
  store.peekAll('message').forEach(message => {
    if (message.id === data.id.toString()) {
      store.unloadRecord(message);
    }
  });
  store.push(store.normalize('message', data));
});

@jerel This issue occurs when (1) an existing record without an id exist in the store, (2) this record is saved via a parent-child relationship [embedded] and (3) when returned from the server, with an id, this child document will get duplicated.

Perhaps using hasRecordForId before pushing will be helpful for you.

As a workaround, I just filter out the hasmany embedded records without ids after saving the parent model like this :

// utils/workaround1829.js
export default {
  filterHasmanyDuplicates: function (model, hasmanyName) {
    var recordList = model.get(hasmanyName), i, iterator;
    iterator = function (record) {
      if (record && !record.get('id')) {
        recordList.removeObject(record);
      }
    };
    for (i = 0; i < recordList.get('length'); i++) {
      recordList.forEach(eachFunc);
    }
  }
};

Usage :

// in some controller or route
import workaround1829 from '../utils/workaround1829';
......
  actions: {
    save: function () {
        var model = this.get('model');
        model.save().then(function() (
            workaround1829.filterHasmanyDuplicates(model, 'children');
 .....

I don't know, maybe it can help someone as a simple quick & dirty workaround ?

Currently I have a workaround that requires no code in your controllers, routes (or whatever place you put your saving stuff).

Basically you need a mixin that you use on the parent model that contains the dependent child records:

import Ember from 'ember';

export default Ember.Mixin.create({
    save: function() {
        var recordsToDelete = [];
        this.eachRelationship((name, descriptor) => {
            if (descriptor.kind === 'hasMany') {
                this.get(name).forEach((object) => {
                    if (object.get('isNew')) {
                        recordsToDelete.push(object);
                    }
                });
            }
        });

        var promise = this._super(...arguments);
        promise.then(() => {
            recordsToDelete.forEach((record) => {
                record.deleteRecord();
            });
        });
        return promise;
    }
});

Then you can use this mixin in your models, like this:

import DS from 'ember-data';
import DependentRelationships from '../mixins/dependent-relationships';

export default DS.Model.extend(DependentRelationships, {
    tags: DS.hasMany('tag', { async: false })
});

Then when you save the above model, it will take care of the hasMany relationship if it got some new ones embedded. I hope this might be useful for someone, for me it is working well.

this.get('model').save(); // thats all you need to use now, you got some better client code now

@florent-blanvillain it is something like yours approach but it doesn't require you to have that dirty code in your client code.

@steffenbrem Nice piece, thanks for sharing!

@sandstrom And for everyone else used the previous snippet I posted. I updated it, it contained a little bug. Now it stores all the records pre-save in a temporary array and after the save is completed THEN it will delete the records. The change is in the save method of the mixin, you only need to update that.

@sandstrom I've run into this problem yesterday (although on an older version of ember-data). I came up with a manual solution which works the same way as your mixin. But the problem is the temporary records stay in the store after deletion. You can check with the ember developer tools.

The solution is to call store.unloadRecord(record) after deletion in the then(...) block. The temporary records are gone now.

Update: Sorry, it may be just an artefact of ember developer tools. After playing around some more I cannot reproduce the effect.

I think the simplest way to fix this is to call unloadRecord on records that are isNew after receiving the response while saving the parent model with which they have a hasMany relationship. In theory, everything in the hasMany should have been persisted along with it's parent, so no new records for that relationship should exist in the store. When this isn't the case, you'd need a flag to disable this behaviour on the model for backends that don't include the hasMany records in the response or when the hasMany relationships don't get sent to the server. Something like DS.hasMany('comment', { persist: false })

@k-fish :+1: that is exactly how I implemented it myself too, see the code of a few posts above this.

@steffenbrem haha whoops, I'm comment-reading-fatigued apparently. I think this should probably just be enabled by default (although it might break things now), then you can turn it off on a per-property basis on the parent.

@steffenbrem instead of loading the records into a temporary array, you can actually just index backwards to delete them (i=length; i>=0; i--) and do it in situ.

@k-fish Not sure how you would do that, if it improves the code I would like to know how you would implement it? Not familiar with that algorithm.

Usually you can't delete from an array because it causes concurrent modification problems (problems with where the index points after you remove something), same with inserting. When you are just deleting however, you can start at the end of the array and remove objects.

save: function() {
 return this._super(...arguments).then(() => {
   this.eachRelationship((name, descriptor) => {
     if (descriptor.kind === 'hasMany') {
       for(idx=this.get(name).get('length') - 1; idx>=0; idx--) {
         const object=this.get(name)[idx]; 
         if (object.get('isNew')) {
           object.deleteRecord();
         }
       });
     }
   });
 });
}

Warning; this is just an example, I haven't tested this

As @grayt0r mentions, the fixes mentioned here are not sufficient. They also need to clear up the private property _implicitRelationships for them to actually work.

Here is a more fully featured recursive fix for hasMany and belongsTo relationships:
https://gist.github.com/amk221/dd0eb6d9b72d0e249f5ae359b87b8b58

What's the status of this issue? The bug is becoming really annoying.

@steffenbrem thanks mate.. :+1: the mixin works like a charm

@klclee Np :+1: Still working great for my project

I had a similar issue, not sure if it is related, but I got a pull request for it :
https://github.com/emberjs/data/pull/4154

Looks like in 2.4.x we have to find another workaround

@jcbvm can you try canary? It seems that PR #4154 is not included in 2.4.x and also not in 2.5.0-beta.1, but maybe this fixes the issue you're having...

@pangratz nope, issue still exists on canary

@jcbvm :cry:, thanks for checking though.

Note that PR #4154 expects you to provide a client side generated id. If you look at the test, https://github.com/emberjs/data/pull/4154/files#diff-f5099af3b5cad3fc343f3f4b73eb3c97R441, you can see what I mean.

This issue is more about what to do with embedded records that had NO client side generated id. I am currently not aware of any work for that in core ember data, for now I think the only solution is the code snippet I posted (https://github.com/emberjs/data/issues/1829#issuecomment-134270337).

What that snippet does is get a reference of all records prior to making the API call and after the save is successful (you are now in a state where you have the duplicates), it removes the old records. It is a simple but effective "hack".

Just be careful for nested embedded relationships where save is not called on the model which has the embedded relationships, but a parent one. We have to do the workaround recursively.

This article outlines the current state of saving embedded records with the JSON-API adapter:
http://emberigniter.com/saving-models-relationships-json-api/

It favors transient id to correlate records, which I agree with.

Just thought I'd mention the article since it's related to this issue.

@steffenbrem

Thank you for the workaround! My problem with saving embedded records extends to include not just hasMany relations but belongsTo relations as well. So I modified your workaround to handle that as well. But way more complicated because this.get() on a belongsTo relation returns a promise (unlike forEach on a hasMany relation). I added comments to make sense of it all.

save: function() {
  let hasManyRecords = [], belongsToRecords = {};

  this.eachRelationship((name, descriptor) => {
    if (descriptor.kind === 'hasMany') {
      this.get(name).forEach((record) => {
        if (record.get('isNew')) {
          hasManyRecords.push(record);
        }
      });
    }

    if (descriptor.kind === 'belongsTo' && this.get(name).get('isNew')) {
      belongsToRecords[name] = this.get(name);
    }
  });

  let promise = this._super(...arguments);
  promise.then(() => {

    hasManyRecords.forEach((record) => {
      record.deleteRecord();
    });

    // Since this.get() on a belongsTo relation returns a promise,
    // use a hash promise.
    Ember.RSVP.hash(belongsToRecords).then((hash) => {
      Object.keys(hash).forEach((name) => {
        this.get(name).then((newRecord) => {

          // Duplicate data from the new record.
          const recordId = newRecord.get('id');
          let pushData = {};
          pushData[name] = [ newRecord.toJSON({ includeId:true }) ];

          // Reset the belongsTo relation on the model.
          this.set(name, null);

          // Delete any of the existing records.
          hash[name].deleteRecord();
          newRecord.deleteRecord();

          Ember.run.scheduleOnce('sync', this, () => {
            // Push data to store.
            this.store.pushPayload(pushData);
            const finalRecord = this.store.peekRecord(name, recordId);

            // Since records created with pushPayload are considered dirty,
            // let's rollbackAttributes. All data on record will persist.
            finalRecord.rollbackAttributes();

            // Set the belongsTo relation.
            this.set(name, finalRecord);
          });
        });
      });
    });
  });

  return promise;
}

Still not the perfect solution, so be cautious when using it.

I know there is a proposal for a fix for this at the moment just wanted to share. Was able to sort of 'sidestep' the issue by clearing out the relationships within the model serializer function and then rely on the returned post data from the server to repopulate it. Obviously less than ideal since if the server fails to return the relationships your up a creek.

I can confirm this issue in 2.11.1.

in my case I'm saving a book with 2 pages (2 pages in store). After this saving I transition to a new route that shows the pages. A GET request is triggered and suddenly 4 pages are in the store (including 2 duplicates). The pages from the server have the exact same dataset. So the question is why ember-data does not detect that they are duplicates (using the ID would be enough to detect the duplicates).

It does, it's likely when you saved you didn't add the IDs the server gave to these records to the records you were saving.

@runspired There is a GET and a POST, both return the same json object, both with the same ID. Or did I misunderstand you?

@toovy you did. From what you've said, it seems unlikely that this timing bug (where a new record is received by a GET request or socket update before the response for it's save request has been received) is your bug. If you'd like, ping me in the #ember-data channel on the ember slack and I can spend a little bit of time helping you figure out what's disconnected.

@runspired I've checked the network and you are right, it is an timing issue. In my case a computed recomputes and thus sends the GET directly when saving. In some cases the GET returns before the POST - timing issue. I could resolve it, but nevertheless I would expect ember-data to detect this - I might be wrong. Thanks for your help.

Why can't Ember data prevent this from happening? It is super annoying.

Thanks,

Tal Weiss
+972-54-7476276
Skype: major.tal
Twitter: @majortal

On Feb 22, 2017 5:42 PM, "Tobias Braner" notifications@github.com wrote:

@runspired https://github.com/runspired I've checked the network and
you are right, it is an timing issue. In my case a computed recomputes and
thus sends the GET directly when saving. In some cases the GET returns
before the POST - timing issue. I could resolve it, but nevertheless I
would expect ember-data to detect this - I might be wrong. Thanks for your
help.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/emberjs/data/issues/1829#issuecomment-281706419, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AA9lwUy29OCZj6AbXLFOtNYgpoj0eL45ks5rfFdTgaJpZM4Br7KU
.

I just ran across this - how has this not been prioritized in 3 years? I will try out @steffenbrem 's workaround, though! Thanks for those who have provided clean global solutions like this.

how has this not been prioritized in 3 years

@abobwhite because it's a super edge-y case that only affect folks with a race condition particular to their Application's API structure, the only general solution to which is to implement a client-id that the server would need to know about and reflect, which is something fairly trivial to do in user-space already.

Thanks @runspired. I don't see this as an edge case. I have a model content which defines hasMany to tag-groups which I've defined with the EmbeddedRecordsMixin as embedded: 'always'....these tag-groups don't make sense outside of being inside another model and while I could write a separate API to save/update/delete them by themselves, it seems silly. So the API I consume expects those operations to be done when saving/updating content but relies on the id of tag-group to know what has changed when updating.

I'm confused how this has to do with an app-specific race condition....?

I have a model, content. I have a new tag-group called tagGroup. I add it via content.get('tagGroups').addObject(tagGroup)....then I call content.save(). Pretty straightforward. Am I misunderstanding something?

@abobwhite The main issue is that the local objects you have can't be mapped to the server response (since you don't have ids on your local objects). This makes it very hard to make sure the proper local objects get merged with the new ones coming from the server. Instead ember data is just creating new local objects, were this whole issue is about. Now the best solution I came up with was simply removing the local objects when new ones came in, simply replacing the your old objects. Since the old records gets destroyed, this isn't a good solution to put into Ember Data directly either, it can have nasty side effects.

If you can implement client sided ids like @runspired mentioned, I would go that route, much cleaner! Otherwise you have to remove the old local records and make peace with the fact the whole instance gets replaced.

Thanks @steffenbrem . I will make peace with your solution for now. Thanks for sharing it :)

Thanks @steffenbrem ! This solution works perfect for me:

 _saveSuccess(savedRecord) {
    get(savedRecord, 'hasManyRecords').forEach(record => {
      if (get(record, 'hasDirtyAttributes')) {
         record.deleteRecord();
     }
    });
 }

On [email protected], I have found that the parent.get('children') array is actually correct, but that there are orphaned child objects in the ember-data store. Those child objects have a parent reference that is to a PromiseProxy whose content is the parent. So the following would work for me:

// models/parent.js

save() {
  return this._super().then(() => {
    this.get('store')
    .peekAll('child')
    .filterBy('isNew')
    .filter((child) => child.get('parent.id') === this.get('id'))
    .invoke('unloadRecord');
  });
}

Is there any update on this? This bug has existing for 3 years. Thanks for the workarounds!

@abarax There are discussions in the json-api group which would help here (if/when they land). But it would also be possible to make changes to Ember Data without waiting for JSON api to iron things out (for example via the meta fields).

If you have time for a PR I'm happy to discuss what a likely solution could look like.

I think the thing is that this issue is extremely hard to solve generally as it requires a server side contract, and it's trivial to make your server side contract work correctly in Ember data today if you have one.

it's trivial to make your server side contract work correctly in Ember data today if you have one

It is? Do you mean client-generated GUIDs?

@jamesarosen I think he's saying (1) it's hard to solve without a client/server contract (e.g. updated json-api spec) and (2) if you have a client/server contract in place, the adjustments needed in ember data are trivial.

(please correct me if my interpretation of what you ment is incorrect runspired)

I've come to this bug a couple times now in the last few months via searches, and unfortunately often can't do anything about the way the API works. So I end up needing to do workarounds to make Ember Data "happy" - and workarounds often are buggy or rely on private APIs unfortunately.

Saying "just fix your server API and then it's trivial" isn't always a viable answer - some (many?) don't control both sides, I don't think that can be assumed from people working with Ember Data.

Ember Data works great as long as everything is as expected, and I do appreciate all the hard work.

@localpcguy This PR would add the necessary changes needed in JSON-API (the wire format the Ember Data is usually using) that will enable fixing this Ember Data issue (duplicate records).

https://github.com/json-api/json-api/pull/1244

Feel free to write your support in that PR, or ask if there are any obstacles left before it could be moved into the spec 😄 (I've done that myself, I think it would be great if that PR could be merged)

Well reading all of this right has been pretty interesting 😄
I am writing here to keep updated and to also say that I had this problem myself.
This has been caused for me by using Embedded Records as well as deeply nested hasMany relationships.

A quick and dirty fix for me was doing this after saving:

topLevel.save().then(() => {
        let parent = this.store.peekAll("children");
        let filtered = parent .filter(data => get(data, "id") === null);
        filtered.forEach(item => item.deleteRecord());
}

For embedded records scenarios (POSTing a single primary record, along with one or more embedded child records) and then wiring up the IDs when the response gets back, is still an issue with Ember Data and JSON API.

However, there are some proposed changes to the JSON API spec that would help this. More specifically, the idea is to allow a local:id field in the payload, where a temporary ID could be used. The server would include this in the response payload, which would help Ember Data avoid orphaned models.

If anyone following this thread is interested, the PR is here and open for comments:
https://github.com/json-api/json-api/pull/1244#issuecomment-614661330

Was this page helpful?
0 / 5 - 0 ratings