From f1015fd73aff47ee3328f5046e79dae0d9b1fe68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Wed, 20 Mar 2013 16:26:46 +0100 Subject: [PATCH] updated PreloadStore API so that it is more clear that we are deleting preloaded data once read --- app/assets/javascripts/discourse.js | 5 +- .../javascripts/discourse/models/topic.js | 2 +- .../discourse/models/topic_list.js | 2 +- .../javascripts/discourse/models/user.js | 2 +- .../discourse/routes/application_route.js | 10 ++-- app/assets/javascripts/preload_store.js | 45 +++++++++--------- spec/javascripts/preload_store_spec.js | 46 ++++++++----------- 7 files changed, 51 insertions(+), 61 deletions(-) diff --git a/app/assets/javascripts/discourse.js b/app/assets/javascripts/discourse.js index 1b3b02e645b..beeeadf770f 100644 --- a/app/assets/javascripts/discourse.js +++ b/app/assets/javascripts/discourse.js @@ -184,10 +184,11 @@ Discourse = Ember.Application.createWithMixins({ start: function() { Discourse.bindDOMEvents(); - Discourse.SiteSettings = PreloadStore.getStatic('siteSettings'); + Discourse.SiteSettings = PreloadStore.get('siteSettings'); Discourse.MessageBus.start(); Discourse.KeyValueStore.init("discourse_", Discourse.MessageBus); - + // Make sure we delete preloaded data + PreloadStore.remove('siteSettings'); // Developer specific functions Discourse.Development.setupProbes(); Discourse.Development.observeLiveChanges(); diff --git a/app/assets/javascripts/discourse/models/topic.js b/app/assets/javascripts/discourse/models/topic.js index 4af79a69535..c49c336e9cc 100644 --- a/app/assets/javascripts/discourse/models/topic.js +++ b/app/assets/javascripts/discourse/models/topic.js @@ -430,7 +430,7 @@ Discourse.Topic.reopenClass({ } // Check the preload store. If not, load it via JSON - return PreloadStore.get("topic_" + topicId, function() { + return PreloadStore.getAndRemove("topic_" + topicId, function() { return $.getJSON(url + ".json", data); }).then(function(result) { var first = result.posts.first(); diff --git a/app/assets/javascripts/discourse/models/topic_list.js b/app/assets/javascripts/discourse/models/topic_list.js index 8784d44e28b..10e7799f493 100644 --- a/app/assets/javascripts/discourse/models/topic_list.js +++ b/app/assets/javascripts/discourse/models/topic_list.js @@ -106,7 +106,7 @@ Discourse.TopicList.reopenClass({ Discourse.set('transient.topicsList', null); Discourse.set('transient.topicListScrollPos', null); - return PreloadStore.get("topic_list", function() { return $.getJSON(url) }).then(function(result) { + return PreloadStore.getAndRemove("topic_list", function() { return $.getJSON(url) }).then(function(result) { topic_list.set('topics', Discourse.TopicList.topicsFrom(result)); topic_list.set('can_create_topic', result.topic_list.can_create_topic); topic_list.set('more_topics_url', result.topic_list.more_topics_url); diff --git a/app/assets/javascripts/discourse/models/user.js b/app/assets/javascripts/discourse/models/user.js index 447572b5e1a..93e319871a7 100644 --- a/app/assets/javascripts/discourse/models/user.js +++ b/app/assets/javascripts/discourse/models/user.js @@ -437,7 +437,7 @@ Discourse.User.reopenClass({ find: function(username) { // Check the preload store first - return PreloadStore.get("user_" + username, function() { + return PreloadStore.getAndRemove("user_" + username, function() { return $.ajax({ url: Discourse.getURL("/users/") + username + '.json' }); }).then(function (json) { diff --git a/app/assets/javascripts/discourse/routes/application_route.js b/app/assets/javascripts/discourse/routes/application_route.js index 31abb104ad5..c37be43a089 100644 --- a/app/assets/javascripts/discourse/routes/application_route.js +++ b/app/assets/javascripts/discourse/routes/application_route.js @@ -8,13 +8,13 @@ **/ Discourse.ApplicationRoute = Discourse.Route.extend({ setupController: function(controller) { - var currentUser; - Discourse.set('site', Discourse.Site.create(PreloadStore.getStatic('site'))); - currentUser = PreloadStore.getStatic('currentUser'); + Discourse.set('site', Discourse.Site.create(PreloadStore.get('site'))); + var currentUser = PreloadStore.get('currentUser'); if (currentUser) { Discourse.set('currentUser', Discourse.User.create(currentUser)); } + // make sure we delete preloaded data + PreloadStore.remove('site'); + PreloadStore.remove('currentUser'); } }); - - diff --git a/app/assets/javascripts/preload_store.js b/app/assets/javascripts/preload_store.js index 3e4a51286eb..29b2cb6d819 100644 --- a/app/assets/javascripts/preload_store.js +++ b/app/assets/javascripts/preload_store.js @@ -20,17 +20,17 @@ PreloadStore = { }, /** - To retrieve a key, you provide the key you want, plus a finder to - load it if the key cannot be found. Once the key is used once, it is - removed from the store. So, for example, you can't load a preloaded topic - more than once. + To retrieve a key, you provide the key you want, plus a finder to load + it if the key cannot be found. Once the key is used once, it is removed + from the store. + So, for example, you can't load a preloaded topic more than once. - @method get + @method getAndRemove @param {String} key the key to look up the object with @param {function} finder a function to find the object with @returns {Ember.Deferred} a promise that will eventually be the object we want. **/ - get: function(key, finder) { + getAndRemove: function(key, finder) { var preloadStore = this; return Ember.Deferred.promise(function(promise) { if (preloadStore.data[key]) { @@ -59,28 +59,25 @@ PreloadStore = { }, /** - Does the store contain a particular key? Does not delete. + If we are sure it's preloaded, we don't have to supply a finder. + Just returns undefined if it's not in the store. - @method contains - @param {String} key the key to look up the object with - @returns {Boolean} whether the object exists - **/ - contains: function(key) { - return this.data[key] !== void 0; - }, - - /** - If we are sure it's preloaded, we don't have to supply a finder. Just returns - undefined if it's not in the store. - - @method getStatic + @method get @param {String} key the key to look up the object with @returns {Object} the object from the store **/ - getStatic: function(key) { - var result = this.data[key]; - delete this.data[key]; - return result; + get: function(key) { + return this.data[key]; + }, + + /** + Removes the stored value if the key exists + + @method remove + @param {String} key the key to remove + **/ + remove: function(key) { + if (this.data[key]) delete this.data[key]; } }; diff --git a/spec/javascripts/preload_store_spec.js b/spec/javascripts/preload_store_spec.js index f739b056c6d..02c87edc362 100644 --- a/spec/javascripts/preload_store_spec.js +++ b/spec/javascripts/preload_store_spec.js @@ -6,41 +6,33 @@ describe("PreloadStore", function() { PreloadStore.store('bane', 'evil'); }); - describe("contains", function() { - - it("returns false for a key that doesn't exist", function() { - expect(PreloadStore.contains('joker')).toBe(false); - }); - - it("returns true for a stored key", function() { - expect(PreloadStore.contains('bane')).toBe(true); - }); - - }); - - describe('getStatic', function() { + describe('get', function() { it("returns undefined if the key doesn't exist", function() { - expect(PreloadStore.getStatic('joker')).toBe(void 0); + expect(PreloadStore.get('joker')).toBe(undefined); }); - it("returns the the key if it exists", function() { - expect(PreloadStore.getStatic('bane')).toBe('evil'); - }); - - it("removes the key after being called", function() { - PreloadStore.getStatic('bane'); - expect(PreloadStore.getStatic('bane')).toBe(void 0); + it("returns the value if the key exists", function() { + expect(PreloadStore.get('bane')).toBe('evil'); }); }); - describe('get', function() { + describe('remove', function() { + + it("removes the value if the key exists", function() { + PreloadStore.remove('bane'); + expect(PreloadStore.get('bane')).toBe(undefined); + }); + + }); + + describe('getAndRemove', function() { it("returns a promise that resolves to null", function() { var done, storeResult; done = storeResult = null; - PreloadStore.get('joker').then(function(result) { + PreloadStore.getAndRemove('joker').then(function(result) { done = true; storeResult = result; }); @@ -54,7 +46,7 @@ describe("PreloadStore", function() { var done, finder, storeResult; done = storeResult = null; finder = function() { return 'evil'; }; - PreloadStore.get('joker', finder).then(function(result) { + PreloadStore.getAndRemove('joker', finder).then(function(result) { done = true; storeResult = result; }); @@ -70,7 +62,7 @@ describe("PreloadStore", function() { finder = function() { return Ember.Deferred.promise(function(promise) { promise.resolve('evil'); }); }; - PreloadStore.get('joker', finder).then(function(result) { + PreloadStore.getAndRemove('joker', finder).then(function(result) { done = true; storeResult = result; }); @@ -86,7 +78,7 @@ describe("PreloadStore", function() { finder = function() { return Ember.Deferred.promise(function(promise) { promise.reject('evil'); }); }; - PreloadStore.get('joker', finder).then(null, function(rejectedResult) { + PreloadStore.getAndRemove('joker', finder).then(null, function(rejectedResult) { done = true; storeResult = rejectedResult; }); @@ -99,7 +91,7 @@ describe("PreloadStore", function() { it("returns a promise that resolves to 'evil'", function() { var done, storeResult; done = storeResult = null; - PreloadStore.get('bane').then(function(result) { + PreloadStore.getAndRemove('bane').then(function(result) { done = true; storeResult = result; });