From cce1aed0a2a082bf67e3c862352baf6caaaec4d0 Mon Sep 17 00:00:00 2001 From: Sergio-Mira Date: Thu, 26 Nov 2015 13:44:12 +0000 Subject: [PATCH] Prevent triggering has many relationships when there are no changes --- packages/ember-model/lib/has_many.js | 5 +- packages/ember-model/lib/has_many_array.js | 17 ++++-- .../has_many/embedded_objects_load_test.js | 54 +++++++++++++++++++ .../tests/has_many/manipulation_test.js | 48 ++++++++++++++++- 4 files changed, 118 insertions(+), 6 deletions(-) diff --git a/packages/ember-model/lib/has_many.js b/packages/ember-model/lib/has_many.js index aa5652c..94be5e6 100644 --- a/packages/ember-model/lib/has_many.js +++ b/packages/ember-model/lib/has_many.js @@ -34,7 +34,10 @@ Ember.hasMany = function(type, options) { if (!existingArray) { existingArray = this.getHasMany(options.key || propertyKey, type, meta, this.container); } - return existingArray.setObjects(newContentArray); + if (!Ember.isEqual(newContentArray, existingArray)) { + return existingArray.setObjects(newContentArray); + } + return existingArray; } }).meta(meta); }; diff --git a/packages/ember-model/lib/has_many_array.js b/packages/ember-model/lib/has_many_array.js index 9ffefcc..375fa6f 100644 --- a/packages/ember-model/lib/has_many_array.js +++ b/packages/ember-model/lib/has_many_array.js @@ -123,10 +123,19 @@ Ember.ManyArray = Ember.RecordArray.extend({ }, load: function(content) { - Ember.setProperties(this, { - content: content, - originalContent: content.slice() - }); + var currentContent = get(this, 'content'); + var mustUpdateCollection = content.length !== currentContent.length; + for (var i = 0, l = content.length; i < l && !mustUpdateCollection; i++) { + var existingItem = currentContent[i]; + var newItem = content[i]; + mustUpdateCollection = newItem !== existingItem; + } + if (mustUpdateCollection) { + Ember.setProperties(this, { + content: content, + originalContent: content.slice() + }); + } set(this, '_modifiedRecords', []); }, diff --git a/packages/ember-model/tests/has_many/embedded_objects_load_test.js b/packages/ember-model/tests/has_many/embedded_objects_load_test.js index a6bd682..0e58c38 100644 --- a/packages/ember-model/tests/has_many/embedded_objects_load_test.js +++ b/packages/ember-model/tests/has_many/embedded_objects_load_test.js @@ -115,3 +115,57 @@ test("loading embedded data into a parent with deleted children deletes the chil equal(post.get('comments.length'), 1); equal(post.get('comments.firstObject.body'), 'new'); }); + +test("loading collections with same element twice does not trigger observers", function() { + expect(4); + var json = { + id: 1, + title: 'foo', + comments: [{ + id: 1, + text: 'helo' + }] + }; + + var Comment = Ember.Model.extend({ + id: attr(), + text: attr() + }); + Comment.adapter = Ember.RESTAdapter.create(); + Comment.url = '/comments'; + + var Article = Ember.Model.extend({ + title: attr(), + comments: Ember.hasMany(Comment, { key: 'comments', embedded: true }), + + commentsChangeCounter: 0, + commentsChanged: function() { + this.incrementProperty('commentsChangeCounter'); + }.observes('comments.[]') + }); + Article.adapter = Ember.RESTAdapter.create(); + Article.url = '/articles'; + Article.adapter._ajax = function() { + return new Ember.RSVP.Promise(function(resolve) { + resolve(json); + }); + }; + + var article = Article.create(); + Ember.run(article, article.load, json.id, json); + // Force collection to be taken into account (_registerHasManyArray) + var comments = article.get('comments'); + equal(article.get('commentsChangeCounter'), 0, "Inital load didn't triggered observers"); + var json2 = { + id: 1, + title: 'foo', + comments: [{ + id: 1, + text: 'helo here' // <- updated + }] + }; + Ember.run(article, article.load, json.id, json2); + equal(article.get('commentsChangeCounter'), 0, "Load with the same collection didn't triggered observers"); + equal(comments.get('isDirty'), false, "comments should not be dirty"); + deepEqual(Ember.run(comments, comments.mapBy, 'text'), ['helo here']); +}); diff --git a/packages/ember-model/tests/has_many/manipulation_test.js b/packages/ember-model/tests/has_many/manipulation_test.js index 51e6339..9a7978b 100644 --- a/packages/ember-model/tests/has_many/manipulation_test.js +++ b/packages/ember-model/tests/has_many/manipulation_test.js @@ -283,4 +283,50 @@ test("setting a hasMany array with setObjects", function() { equal(article.get('comments.length'), 3, "should be 3 comments after revert"); equal(article.get('comments.isDirty'), false, "should not be dirty after revert"); -}); \ No newline at end of file +}); + +test("setting a has many array with the same array should not trigger its observers", function() { + expect(7); + var json = { + id: 1, + title: 'foo', + comments: [1, 2, 3] + }; + + var Comment = Ember.Model.extend({ + text: attr() + }); + + var Article = Ember.Model.extend({ + title: attr(), + + comments: Ember.hasMany(Comment, { key: 'comments' }), + + commentsChangeCounter: 0, + commentsChanged: function() { + this.incrementProperty('commentsChangeCounter'); + }.observes('comments.[]') + }); + + Comment.adapter = Ember.FixtureAdapter.create(); + Comment.FIXTURES = [ + {id: 1, text: 'uno'}, + {id: 2, text: 'dos'}, + {id: 3, text: 'tres'} + ]; + + var article = Article.create(); + Ember.run(article, article.load, json.id, json); + + equal(article.get('comments.length'), 3, "should be 3 comments"); + + article.set('comments', article.get('comments')); + equal(article.get('comments.length'), 3, "should still be 3 comments"); + equal(article.get('comments.isDirty'), false, "comments should not be dirty"); + equal(article.get('commentsChangeCounter'), 0, "comment observers should not fire"); + + article.set('comments', [Comment.find(3)]); + equal(article.get('comments.length'), 1, "should be 1 comment after set"); + equal(article.get('comments.isDirty'), true, "comments should be dirty after set"); + equal(article.get('commentsChangeCounter'), 1, "comment observers should have fired 1 time"); +});