Reviewers: mp+137273_code.launchpad.net, Message: Please take a look.
Description: Clean up get_endpoints get_endpoints was being called on every instance of service_add and not being garbage collected. Moved it to on_database_changed and added gc around services being removed. https://code.launchpad.net/~makyo/juju-gui/endpoints-once/+merge/137273 (do not edit description out of merge proposal) Please review this at https://codereview.appspot.com/6858099/ Affected files: A [revision details] M app/app.js Index: [revision details] === added file '[revision details]' --- [revision details] 2012-01-01 00:00:00 +0000 +++ [revision details] 2012-01-01 00:00:00 +0000 @@ -0,0 +1,2 @@ +Old revision: [email protected] +New revision: [email protected] Index: app/app.js === modified file 'app/app.js' --- app/app.js 2012-11-20 16:55:43 +0000 +++ app/app.js 2012-11-30 17:33:27 +0000 @@ -256,8 +256,6 @@ // TODO - Bound views will automatically update this on individual models this.db.on('update', this.on_database_changed, this); - this.db.services.on('add', this.updateEndpoints, this); - this.on('navigate', function(e) { console.log('app navigate', e); }); @@ -310,6 +308,35 @@ */ on_database_changed: function(evt) { Y.log(evt, 'debug', 'App: Database changed'); + + var updateNeeded = false, + self = this; + if (!Y.Lang.isValue(this.serviceEndpoints)) { + this.serviceEndpoints = {}; + } + + // Compare endpoints map against db to see if it needs to be changed. + this.db.services.some(function(service) { + if (self.serviceEndpoints[service.get('id')] === undefined) { + updateNeeded = true; + } + return updateNeeded; + }); + + // If there are new services in the DB, pull an updated endpoints map. + if (updateNeeded) { + this.updateEndpoints(); + } else { + // Check to see if any services have been removed (if there are, and + // new ones also, updateEndpoints will replace the whole map, so only + // do this if needed). + Y.Object.each(this.serviceEndpoints, function(key, value, obj) { + if (self.db.services.getById(key) === null) { + delete(self.serviceEndpoints[key]); + } + }); + } + // Redispatch to current view to update. this.dispatch(); }, @@ -322,9 +349,6 @@ updateEndpoints: function(callback) { var self = this; - if (!Y.Lang.isValue(this.serviceEndpoints)) { - this.serviceEndpoints = {}; - } // Defensive code to aid tests. Other systems // don't have to mock enough to get_endpoints below. if (!this.env.get('connected')) { -- https://code.launchpad.net/~makyo/juju-gui/endpoints-once/+merge/137273 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~makyo/juju-gui/endpoints-once into lp:juju-gui. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
