yellow team mailing list archive

[Matthew Scott <matthew.scott@xxxxxxxxxxxxx>]

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: bcsaller@xxxxxxxxx-20121129035815-ofiq7wv9xlvphfug
+New revision: matthew.scott@xxxxxxxxxxxxx-20121130173327-9cjke8qat9vgoobp

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.