yellow team mailing list archive

[Matthew Scott <matthew.scott@xxxxxxxxxxxxx>]

Matthew Scott has proposed merging lp:~makyo/juju-gui/endpoints-once into lp:juju-gui.

Requested reviews:
  Juju GUI Hackers (juju-gui)
Related bugs:
  Bug #1084790 in juju-gui: "get_endpoints called too frequently"
  https://bugs.launchpad.net/juju-gui/+bug/1084790

For more details, see:
https://code.launchpad.net/~makyo/juju-gui/endpoints-once/+merge/137273

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://codereview.appspot.com/6858099/

-- 
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.

=== modified file 'app/app.js'
--- app/app.js	2012-11-20 16:55:43 +0000
+++ app/app.js	2012-11-30 17:36:57 +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')) {