Merge lp:~bac/juju-gui/get-endpoints into lp:juju-gui/experimental

[Brad Crittenden]

Track endpoints as state.

Remove the get_endpoints calls in the environment and the API. Instead
keep the endpoints map up-to-date when services are added,
deleted, and changed.

https://codereview.appspot.com/8275043/

[Brad Crittenden (bac)]

Reviewers: mp+156661_code.launchpad.net,

Message:
Please take a look.

Description:
Track endpoints as state.

Remove the get_endpoints calls in the environment and the API. Instead
keep the endpoints map up-to-date when services are added,
deleted, and changed.

https://code.launchpad.net/~bac/juju-gui/get-endpoints/+merge/156661

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/8275043/

Affected files:
   A [revision details]
   M app/app.js
   M app/models/endpoints.js
   M app/models/models.js
   M app/store/env/fakebackend.js
   M app/store/env/go.js
   M app/store/env/python.js
   M app/store/env/sandbox.js
   M app/views/environment.js
   M app/views/topology/relation.js
   M test/test_app.js
   M test/test_browser_charm_details.js
   M test/test_endpoints.js
   M test/test_env_python.js
   M test/test_environment_view.js
   M test/test_notifications.js

[Jeff Pihach (hatch)]

This is looking great Thanks for all this work! It really cleans up a
lot of code.

I'd like to open up a discussion between a few of us to see if this
should be moved into it's own closure and then the rest of the comments
are pretty trivial.

https://codereview.appspot.com/8275043/diff/2001/app/models/endpoints.js
File app/models/endpoints.js (right):

https://codereview.appspot.com/8275043/diff/2001/app/models/endpoints.js#newcode12
app/models/endpoints.js:12: var models = Y.namespace('juju.models');
I know that this is how it was done previously but it feels like these
methods are just hanging off of juju.models with all of the other models
when they should really be within their own closure of utility methods
to clean up the object. This should also make it easier to test.
Thoughts?

https://codereview.appspot.com/8275043/diff/2001/app/models/endpoints.js#newcode15
app/models/endpoints.js:15: models.endpoints_map = {};
camelCase?

https://codereview.appspot.com/8275043/diff/2001/app/models/endpoints.js#newcode169
app/models/endpoints.js:169: // var requires =
evt.currentTarget.get('requires');
commented out old code that should be removed?

https://codereview.appspot.com/8275043/diff/2001/app/models/endpoints.js#newcode255
app/models/endpoints.js:255: if (true) { // Avoid lint warning.
Is this required so that the linter doesn't complain about the vars
being re-declared below?

https://codereview.appspot.com/8275043/diff/2001/app/models/endpoints.js#newcode256
app/models/endpoints.js:256: var rel = {};
I'm surprised that the linter didn't complain about these vars being
re-declared every loop. Typically `k`, `rel`, `j` would be defined at
the top of the fn with `result` then re-assigned in every loop.

https://codereview.appspot.com/8275043/diff/2001/app/models/models.js
File app/models/models.js (right):

https://codereview.appspot.com/8275043/diff/2001/app/models/models.js#newcode563
app/models/models.js:563: this.endpoints_map = {};
camelCase?

https://codereview.appspot.com/8275043/diff/2001/app/views/topology/relation.js
File app/views/topology/relation.js (right):

https://codereview.appspot.com/8275043/diff/2001/app/views/topology/relation.js#newcode563
app/views/topology/relation.js:563: var endpoints =
models.getEndpoints(service, models.endpoints_map, db);
camelCase endpoints_map

https://codereview.appspot.com/8275043/diff/2001/test/test_endpoints.js
File test/test_endpoints.js (right):

https://codereview.appspot.com/8275043/diff/2001/test/test_endpoints.js#newcode135
test/test_endpoints.js:135: var charm = new models.Charm({id:
'cs:precise/wordpress-2'});
Because this attaches events we should destroy it at the end of every
test and make sure that the events that it attaches are destroyed as
well.

https://codereview.appspot.com/8275043/

[Benjamin Saller (bcsaller)]

This is looking good but I do ask a structural change below. I hope I
explain it well enough (and the reasons for it). If you have questions
let me know.

https://codereview.appspot.com/8275043/diff/2001/app/app.js
File app/app.js (left):

https://codereview.appspot.com/8275043/diff/2001/app/app.js#oldcode478
app/app.js:478:
Nice that this removed so much code from App :)

https://codereview.appspot.com/8275043/diff/2001/app/app.js
File app/app.js (right):

https://codereview.appspot.com/8275043/diff/2001/app/app.js#newcode364
app/app.js:364: this);
This is fine for now but When I have this many subscriptions I'd usually
wrap it in a controller object such that

controller._subscriptions = controller.bind()

and controller.unbind can use the subscription object to remove
listeners.

https://codereview.appspot.com/8275043/diff/2001/app/models/endpoints.js
File app/models/endpoints.js (right):

https://codereview.appspot.com/8275043/diff/2001/app/models/endpoints.js#newcode29
app/models/endpoints.js:29: models.getEndpoints = function(svc, ep_map,
db) {
This signature doesn't work well anymore, the db may or may not be the
one the service handlers were bound to. By making a controller object we
can reference the db when we bind the handlers and drop this argument
making it consistent.

https://codereview.appspot.com/8275043/diff/2001/app/models/endpoints.js#newcode167
app/models/endpoints.js:167: charm.on('load', Y.bind(function(svcName,
evt) {
It shouldn't matter, but this seems like a case for charm.once(...)
rather than on() as the charm should be static post load and we don't
need to keep the listener active.

https://codereview.appspot.com/8275043/diff/2001/app/models/endpoints.js#newcode181
app/models/endpoints.js:181: models.serviceAddHandler = function(evt) {
If we had a controller here like EndpointManager or something we could
hang the handlers off that rather than models which would be nicer (In
addition to the bind/unbind stuff mentioned before).

https://codereview.appspot.com/8275043/diff/2001/app/models/endpoints.js#newcode254
app/models/endpoints.js:254: for (var k in meta) {
Why is this done in native JS rather than with each() or map()? (I
think) The linter complaining here is because in a loop we want to know
if the current item is owned by the object or part of its prototype, the
higher level constructs can safely handle those cases for us.

https://codereview.appspot.com/8275043/diff/2001/app/store/env/fakebackend.js
File app/store/env/fakebackend.js (left):

https://codereview.appspot.com/8275043/diff/2001/app/store/env/fakebackend.js#oldcode427
app/store/env/fakebackend.js:427: // },
nice catch, thanks for pulling all these from the envs

https://codereview.appspot.com/8275043/diff/2001/app/views/topology/relation.js
File app/views/topology/relation.js (right):

https://codereview.appspot.com/8275043/diff/2001/app/views/topology/relation.js#newcode563
app/views/topology/relation.js:563: var endpoints =
models.getEndpoints(service, models.endpoints_map, db);
as noted before, in causes like this we can't always be sure the db is
the one the listeners were bound to.

https://codereview.appspot.com/8275043/

[Jeff Pihach (hatch)]

LGTM with Ben's changes and those instances in the tests destroyed.

Thanks again, Great work!

https://codereview.appspot.com/8275043/

[Benjamin Saller (bcsaller)]

LGTM

as discussed online the changes I suggested will be pushed to another
branch (and I am fine with that).

https://codereview.appspot.com/8275043/

[lp:~bac/juju-gui/get-endpoints]

472. By Brad Crittenden on 2013-04-02

Removed commented-out code

473. By Brad Crittenden on 2013-04-02

Clean up tests

[Brad Crittenden (bac)]

Thanks for the reviews and the great suggestions. Since none of them
are show stoppers I'll address them in a follow-on branch so this
blocking work can land, albeit a bit unclean.

https://codereview.appspot.com/8275043/diff/2001/app/app.js
File app/app.js (right):

https://codereview.appspot.com/8275043/diff/2001/app/app.js#newcode364
app/app.js:364: this);
Great suggestion Ben. I'll do it in the follow-on branch. A card has
been created with my face on it.

https://codereview.appspot.com/8275043/diff/2001/app/models/endpoints.js
File app/models/endpoints.js (right):

https://codereview.appspot.com/8275043/diff/2001/app/models/endpoints.js#newcode15
app/models/endpoints.js:15: models.endpoints_map = {};
On 2013/04/02 19:56:43, jeff.pihach wrote:
> camelCase?

Done.

https://codereview.appspot.com/8275043/diff/2001/app/models/endpoints.js#newcode254
app/models/endpoints.js:254: for (var k in meta) {
No good reason. I'll clean that up tomorrow.

https://codereview.appspot.com/8275043/diff/2001/app/models/models.js
File app/models/models.js (right):

https://codereview.appspot.com/8275043/diff/2001/app/models/models.js#newcode563
app/models/models.js:563: this.endpoints_map = {};
On 2013/04/02 19:56:43, jeff.pihach wrote:
> camelCase?

Done.

https://codereview.appspot.com/8275043/diff/2001/test/test_endpoints.js
File test/test_endpoints.js (right):

https://codereview.appspot.com/8275043/diff/2001/test/test_endpoints.js#newcode135
test/test_endpoints.js:135: var charm = new models.Charm({id:
'cs:precise/wordpress-2'});
On 2013/04/02 19:56:43, jeff.pihach wrote:
> Because this attaches events we should destroy it at the end of every
test and
> make sure that the events that it attaches are destroyed as well.

Done.

https://codereview.appspot.com/8275043/

[Brad Crittenden (bac)]

*** Submitted:

Track endpoints as state.

Remove the get_endpoints calls in the environment and the API. Instead
keep the endpoints map up-to-date when services are added,
deleted, and changed.

R=jeff.pihach, bcsaller
CC=
https://codereview.appspot.com/8275043

https://codereview.appspot.com/8275043/

=== modified file 'app/app.js'
--- app/app.js	2013-04-01 17:31:13 +0000
+++ app/app.js	2013-04-02 21:29:23 +0000
@@ -262,7 +262,6 @@
 
       // Create a client side database to store state.
       this.db = new models.Database();
-      this.serviceEndpoints = {};
 
       // Optional Landscape integration helper.
       this.landscape = new views.Landscape();
@@ -359,6 +358,14 @@
       this.env.on('delta', this.notifications.generate_notices,
           this.notifications);
 
+      // Handlers for adding and removing services to the service list.
+      this.db.services.after('add', models.serviceAddHandler, this);
+      this.db.services.after('remove', models.serviceRemoveHandler, this);
+      this.db.services.after('*:pendingChange', models.serviceChangeHandler,
+          this);
+      this.db.services.after('*:charmChange', models.serviceChangeHandler,
+          this);
+
       // When the connection resets, reset the db, re-login (a delta will
       // arrive with successful authentication), and redispatch.
       this.env.after('connectedChange', function(ev) {
@@ -451,7 +458,7 @@
     },
 
     /**
-     * On database changes, update the endpoints map and update the view.
+     * On database changes update the view.
      *
      * @method on_database_changed
      */
@@ -461,24 +468,6 @@
       var self = this;
       var active = this.get('activeView');
 
-      // Compare endpoints map against db to see if services have been added.
-      var servicesAdded = this.db.services.some(function(service) {
-        return (self.serviceEndpoints[service.get('id')] === undefined);
-      });
-
-      // If there are new services in the DB, pull an updated endpoints map.
-      if (servicesAdded) {
-        this.updateEndpoints();
-      } else {
-        // If any services have been removed, delete them from the map
-        // rather than updating it as a whole.
-        Y.Object.each(this.serviceEndpoints, function(key, value, obj) {
-          if (self.db.services.getById(key) === null) {
-            delete(self.serviceEndpoints[key]);
-          }
-        });
-      }
-
       // Update Landscape annotations.
       this.landscape.update();
 
@@ -495,27 +484,6 @@
       }
     },
 
-    /**
-     * When services are added, update the endpoints map.
-     *
-     * @method updateEndpoints
-     */
-    updateEndpoints: function(callback) {
-      var self = this;
-
-      // Defensive code to aid tests. Other systems
-      // do not have to mock enough to get_endpoints below.
-      if (!this.env.get('connected')) {
-        return;
-      }
-      self.env.get_endpoints([], function(evt) {
-        self.serviceEndpoints = evt.result;
-        if (Y.Lang.isFunction(callback)) {
-          callback(self.serviceEndpoints);
-        }
-      });
-    },
-
     // Route handlers
 
     /**
@@ -851,13 +819,6 @@
           options = {
             getModelURL: Y.bind(this.getModelURL, this),
             nsRouter: this.nsRouter,
-            /**
-             * A simple closure so changes to the value are available.
-             *
-             * @method show_environment.options.getServiceEndpoints
-             */
-            getServiceEndpoints: function() {
-              return self.serviceEndpoints;},
             loadService: this.loadService,
             landscape: this.landscape,
             db: this.db,

=== modified file 'app/models/endpoints.js'
--- app/models/endpoints.js	2013-02-08 17:36:58 +0000
+++ app/models/endpoints.js	2013-04-02 21:29:23 +0000
@@ -12,10 +12,12 @@
   var models = Y.namespace('juju.models');
   var utils = Y.namespace('juju.views.utils');
 
+  models.endpointsMap = {};
+
   /**
    * Find available relation targets for a service.
    *
-   * @class getEndpoints
+   * @method getEndpoints
    * @param {Object} svc A service object.
    * @param {Object} ep_map A mapping of service name to available endpoints
    *   for the service in the form [{'interface':x, 'name':y, 'role': z}, ...].
@@ -152,4 +154,123 @@
     });
     return targets;
   };
+
+  /**
+   * Setup on('load') handler for a charm.
+   *
+   * @method setupCharmOnLoad
+   * @param {Object} charm The charm to watch.
+   * @param {String} svcName The name of the service the charm is attached.
+   * @return {undefined} Nothing.
+   */
+  var setupCharmOnLoad = function(charm, svcName) {
+    charm.on('load', Y.bind(function(svcName, evt) {
+      addServiceToEndpointsMap(svcName, evt.currentTarget);
+    }, null, svcName));
+  };
+
+  /**
+   * Handle event for a service being added to the services modellist.
+   *
+   * @method serviceAddHandler
+   * @param {Object} evt The event, containing a model object.
+   * @return {undefined} Nothing.
+   */
+  models.serviceAddHandler = function(evt) {
+    var svcName = evt.model.get('id');
+    var charm_id = evt.model.get('charm'),
+        self = this,
+        charm = this.db.charms.getById(charm_id);
+
+    // If the charm doesn't exist, add and load it.
+    if (!Y.Lang.isValue(charm)) {
+      charm = this.db.charms.add({id: charm_id})
+        .load(this.env,
+          // If views are bound to the charm model, firing "update" is
+          // unnecessary, and potentially even mildly harmful.
+          function(err, result) { self.db.fire('update'); });
+    }
+
+    // If the service is not a ghost (i.e. 'pending' is false), process it.
+    if (!evt.model.get('pending')) {
+      if (charm.loaded) {
+        addServiceToEndpointsMap(svcName, charm);
+      } else {
+        setupCharmOnLoad(charm, svcName);
+      }
+    }
+  };
+
+  /**
+   * Handle event for a service transitioning from a ghost to a corporeal
+   * object as indicated by the 'pending' attribute becoming false.  Also
+   * handles changes in the service's charm.
+   *
+   * @method serviceChangeHandler
+   * @param {Object} evt The event, containing the service as the target.
+   * @return {undefined} Nothing.
+   */
+  models.serviceChangeHandler = function(evt) {
+    var charm_id = evt.target.get('charm'),
+        charm = this.db.charms.getById(charm_id),
+        service = evt.target,
+        svcName = service.get('id');
+    // Ensure the service is no longer pending.
+    if (service.get('pending')) {
+      return;
+    }
+    if (charm.loaded) {
+      addServiceToEndpointsMap(svcName, charm);
+    } else {
+      setupCharmOnLoad(charm, svcName);
+    }
+  };
+
+  /**
+   * Handle event for a service removal.
+   *
+   * @method serviceRemoveHandler
+   * @param {Object} evt The event, containing the service as the target.
+   * @return {undefined} Nothing.
+   */
+  models.serviceRemoveHandler = function(evt) {
+    var svcName = evt.model.get('id');
+    delete(models.endpointsMap[svcName]);
+  };
+
+  /**
+   * Flatten the relation metadata.
+   *
+   * @method flatten
+   * @param {Object} meta The relation metadata.
+   * @return {List} A list of objects, where each entry is a hash with a
+   *   'name' key and the key value pairs from the metadata.
+   */
+  var flatten = function(meta) {
+    var result = [];
+    if (Y.Lang.isValue(meta)) {
+      for (var k in meta) {
+        if (true) { // Avoid lint warning.
+          var rel = {};
+          rel.name = k;
+          for (var j in meta[k]) {
+            if (true) { // Avoid lint warning.
+              rel[j] = meta[k][j];
+            }
+          }
+          result.push(rel);
+        }
+      }
+    }
+    return result;
+  };
+
+  var addServiceToEndpointsMap = function(svcName, charm) {
+    models.endpointsMap[svcName] = {};
+    models.endpointsMap[svcName].provides = flatten(charm.get('provides'));
+    models.endpointsMap[svcName].requires = flatten(charm.get('requires'));
+  };
+
+  models.addServiceToEndpointsMap = addServiceToEndpointsMap;
+
 });

=== modified file 'app/models/models.js'
--- app/models/models.js	2013-03-21 21:39:47 +0000
+++ app/models/models.js	2013-04-02 21:29:23 +0000
@@ -560,6 +560,7 @@
       this.relations.reset();
       this.units.reset();
       this.notifications.reset();
+      this.endpointsMap = {};
     },
 
     on_delta: function(delta_evt) {

=== modified file 'app/store/env/fakebackend.js'
--- app/store/env/fakebackend.js	2013-03-21 21:39:47 +0000
+++ app/store/env/fakebackend.js	2013-04-02 21:29:23 +0000
@@ -422,10 +422,6 @@
 
     // },
 
-    // getEndpoints: function() {
-
-    // },
-
     // updateAnnotations: function() {
 
     // },

=== modified file 'app/store/env/go.js'
--- app/store/env/go.js	2013-04-02 15:18:57 +0000
+++ app/store/env/go.js	2013-04-02 21:29:23 +0000
@@ -149,24 +149,6 @@
     },
 
     /**
-      XXX FAKE FAKE FAKE, does not do anything.
-
-      Get the available endpoints (by interface) for a collection of
-      services.
-
-      @method get_endpoints
-      @param {Array} services Zero or more currently deployed services for
-          which the endpoints should be collected.  Specifying an empty array
-          indicates that all deployed services should be analyzed.
-      @param {Function} callback A callable that must be called once the
-         operation is performed.
-      @return {undefined} Sends a message to the server only.
-     */
-    get_endpoints: function(services, callback) {
-    },
-
-
-    /**
      * See "app.store.env.base.BaseEnvironment.dispatch_result".
      *
      * @method dispatch_result

=== modified file 'app/store/env/python.js'
--- app/store/env/python.js	2013-03-27 14:50:52 +0000
+++ app/store/env/python.js	2013-04-02 21:29:23 +0000
@@ -381,23 +381,6 @@
     },
 
     /**
-      Get the available endpoints (by interface) for a collection of
-      services.
-
-      @method get_endpoints
-      @param {Array} services Zero or more currently deployed services for
-          which the endpoints should be collected.  Specifying an empty array
-          indicates that all deployed services should be analyzed.
-      @param {Function} callback A callable that must be called once the
-         operation is performed.
-      @return {undefined} Sends a message to the server only.
-     */
-    get_endpoints: function(services, callback) {
-      this._send_rpc({'op': 'get_endpoints', 'service_names': services},
-                     callback);
-    },
-
-    /**
      * Update the annotations for an entity by name.
      *
      * @param {Object} entity The name of a machine, unit, service, or

=== modified file 'app/store/env/sandbox.js'
--- app/store/env/sandbox.js	2013-04-01 22:32:24 +0000
+++ app/store/env/sandbox.js	2013-04-02 21:29:23 +0000
@@ -341,13 +341,6 @@
     },
 
     /**
-    Handles get_endpoints operations from client.  Called by receive.
-    PLACEHOLDER.  This exists to demo existing functionality.
-    **/
-    performOp_get_endpoints: function(data) {
-    },
-
-    /**
     Handles update_annotations operations from client.  Called by receive.
     PLACEHOLDER.  This exists to demo existing functionality.
     **/

=== modified file 'app/views/environment.js'
--- app/views/environment.js	2013-03-21 15:32:06 +0000
+++ app/views/environment.js	2013-04-02 21:29:23 +0000
@@ -69,7 +69,6 @@
               db: this.get('db'),
               landscape: this.get('landscape'),
               getModelURL: this.get('getModelURL'),
-              getServiceEndpoints: this.get('getServiceEndpoints'),
               container: container,
               nsRouter: this.get('nsRouter')});
             // Bind all the behaviors we need as modules.

=== modified file 'app/views/topology/relation.js'
--- app/views/topology/relation.js	2013-03-28 15:08:38 +0000
+++ app/views/topology/relation.js	2013-04-02 21:29:23 +0000
@@ -560,10 +560,8 @@
       topo.fire('show', { selection: vis.selectAll('.service') });
 
       var db = this.get('component').get('db');
-      var getServiceEndpoints = this.get('component')
-                                    .get('getServiceEndpoints');
-      var endpoints = models.getEndpoints(
-          service, getServiceEndpoints(), db);
+      var endpoints = models.getEndpoints(service, models.endpointsMap, db);
+
       // Transform endpoints into a list of relatable services (to the
       // service).
       var possible_relations = Y.Array.map(

=== modified file 'test/test_app.js'
--- test/test_app.js	2013-03-22 00:00:29 +0000
+++ test/test_app.js	2013-04-02 21:29:23 +0000
@@ -34,12 +34,14 @@
 (function() {
 
   describe('Application basics', function() {
-    var Y, app, container;
+    var Y, app, container, utils, juju, env, conn;
 
     before(function(done) {
       Y = YUI(GlobalConfig).use(
           ['juju-gui', 'juju-tests-utils', 'juju-view-utils'],
           function(Y) {
+            utils = Y.namespace('juju-tests.utils');
+            juju = Y.namespace('juju');
             done();
           });
     });
@@ -54,13 +56,18 @@
           .append(Y.Node.create('<span/>')
             .addClass('provider-type'))
           .hide();
+      conn = new utils.SocketStub();
+      env = juju.newEnvironment({conn: conn});
+      env.connect();
       app = new Y.juju.App(
           { container: container,
-            viewContainer: container});
+            viewContainer: container,
+            env: env});
       injectData(app);
     });
 
     afterEach(function() {
+      app.destroy();
       container.remove(true);
       sessionStorage.setItem('credentials', null);
     });
@@ -77,6 +84,7 @@
           var the_username = 'nehi';
           var the_password = 'moonpie';
           // Replace the existing app.
+          app.destroy();
           app = new Y.juju.App(
               { container: container,
                 user: the_username,
@@ -89,6 +97,7 @@
 
     it('propagates the readOnly option from the configuration', function() {
       // Replace the existing app.
+      app.destroy();
       app = new Y.juju.App({
         container: container,
         readOnly: true,
@@ -108,7 +117,7 @@
       // needed to show them.
       var wordpress = app.db.services.getById('wordpress'),
           wp0 = app.db.units.get_units_for_service(wordpress)[0],
-          wp_charm = app.db.charms.add({id: wordpress.get('charm')});
+          wp_charm = app.db.charms.getById(wordpress.get('charm'));
 
       // 'service/wordpress/' is the primary route,
       // so other URLs are not returned.
@@ -127,6 +136,7 @@
 
     it('should display the configured environment name', function() {
       var environment_name = 'This is the environment name.  Deal with it.';
+      app.destroy();
       app = new Y.juju.App(
           { container: container,
             viewContainer: container,
@@ -138,6 +148,7 @@
 
     it('should show a generic environment name if none configured',
        function() {
+         app.destroy();
          app = new Y.juju.App(
          { container: container,
            viewContainer: container});
@@ -330,7 +341,6 @@
             viewContainer: container,
             env: env,
             charm_store: {} });
-      env.get_endpoints = function() {};
     });
 
     afterEach(function() {
@@ -339,46 +349,19 @@
     });
 
     it('must prefetch charm and service for service pages', function() {
-      injectData(app);
       var _ = expect(
           app.db.charms.getById('cs:precise/wordpress-6')).to.not.exist;
+      injectData(app);
       app.show_service({params: {id: 'wordpress'}, query: {}});
-      // The app made a request of juju for the service info.
-      conn.messages[conn.messages.length - 2].op.should.equal('get_service');
-      // The app also requested juju (not the charm store--see discussion in
-      // app/models/charm.js) for the charm info.
-      conn.last_message().op.should.equal('get_charm');
+      // When the service was added to the service modellist, that triggered
+      // the loading of the charm.
+      conn.messages[conn.messages.length - 2].op.should.equal('get_charm');
+      // The service was later loaded.
+      conn.last_message().op.should.equal('get_service');
       // Tests of the actual load machinery are in the model and env tests, and
       // so are not repeated here.
     });
 
-    it('must request endpoints only when necessary', function() {
-      var get_endpoints_count = 0,
-          tmp_data = {
-            result: [
-              ['service', 'add', {
-                'charm': 'cs:precise/mysql-6',
-                'id': 'mysql2'
-              }],
-              ['unit', 'add', {
-                'machine': 0,
-                'agent-state': 'started',
-                'public-address': '192.168.122.222',
-                'id': 'mysql2/0'
-              }]
-            ],
-            op: 'delta'
-          };
-      env.get_endpoints = function(services, callback) {
-        get_endpoints_count += 1;
-      };
-      // Inject default data, should only get_endpoints once.
-      injectData(app);
-      get_endpoints_count.should.equal(1);
-      // Additional deltas should only call get_endpoints once.
-      app.db.on_delta({ data: tmp_data });
-      get_endpoints_count.should.equal(2);
-    });
   });
 })();
 

=== modified file 'test/test_browser_charm_details.js'
--- test/test_browser_charm_details.js	2013-04-02 17:24:31 +0000
+++ test/test_browser_charm_details.js	2013-04-02 21:29:23 +0000
@@ -162,7 +162,7 @@
       });
 
       view.render(node);
-      Y.one('#bws_hooks').all('ul li a').size().should.eql(2);
+      Y.one('#bws_hooks').all('ul li a').size().should.equal(2);
 
       // Click on the hooks install and the content should update.
       Y.one('#bws_hooks').one('ul li a').simulate('click');
@@ -267,4 +267,3 @@
     });
   });
 })();
-

=== modified file 'test/test_endpoints.js'
--- test/test_endpoints.js	2013-01-24 16:46:38 +0000
+++ test/test_endpoints.js	2013-04-02 21:29:23 +0000
@@ -116,3 +116,258 @@
 });
 
 
+describe('Endpoints map', function() {
+  var Y, juju, models;
+
+  beforeEach(function(done) {
+    Y = YUI(GlobalConfig).use(['juju-models',
+                               'juju-tests-utils',
+                               'juju-controllers'],
+    function(Y) {
+      juju = Y.namespace('juju');
+      models = Y.namespace('juju.models');
+      done();
+    });
+  });
+
+  it('should add a service to the map', function() {
+    models.endpointsMap = {};
+    var charm = new models.Charm({id: 'cs:precise/wordpress-2'});
+    charm.set('provides', {
+      url: {
+        'interface': 'http',
+        optional: 'false'
+      },
+      'logging-dir': {
+        'interface': 'logging',
+        scope: 'container'
+      }
+    });
+    charm.set('requires', {
+      db: {
+        'interface': 'mysql',
+        optional: 'false'
+      },
+      cache: {
+        'interface': 'varnish',
+        optional: 'true'
+      }
+    });
+    models.addServiceToEndpointsMap('wordpress', charm);
+    models.endpointsMap.should.eql({wordpress: {
+      provides: [
+        {
+          name: 'url',
+          'interface': 'http',
+          optional: 'false'
+        }, {
+          name: 'logging-dir',
+          'interface': 'logging',
+          scope: 'container'
+        }
+      ],
+      requires: [
+        {
+          name: 'db',
+          'interface': 'mysql',
+          optional: 'false'
+        }, {
+          name: 'cache',
+          'interface': 'varnish',
+          optional: 'true'
+        }
+      ]
+    }});
+    charm.destroy();
+  });
+
+  it('should add a service to the map, requires only', function() {
+    models.endpointsMap = {};
+    var charm = new models.Charm({id: 'cs:precise/wordpress-2'});
+    charm.set('requires', {
+      db: {
+        'interface': 'mysql',
+        optional: 'false'
+      },
+      cache: {
+        'interface': 'varnish',
+        optional: 'true'
+      }
+    });
+    models.addServiceToEndpointsMap('wordpress', charm);
+    models.endpointsMap.should.eql({wordpress: {
+      provides: [],
+      requires: [
+        {
+          name: 'db',
+          'interface': 'mysql',
+          optional: 'false'
+        }, {
+          name: 'cache',
+          'interface': 'varnish',
+          optional: 'true'
+        }
+      ]
+    }});
+    charm.destroy();
+  });
+
+  it('should add a service to the map, provides only', function() {
+    models.endpointsMap = {};
+    var charm = new models.Charm({id: 'cs:precise/wordpress-2'});
+    charm.set('provides', {
+      url: {
+        'interface': 'http',
+        optional: 'false'
+      },
+      'logging-dir': {
+        'interface': 'logging',
+        scope: 'container'
+      }
+    });
+    models.addServiceToEndpointsMap('wordpress', charm);
+    models.endpointsMap.should.eql({wordpress: {
+      requires: [],
+      provides: [
+        {
+          name: 'url',
+          'interface': 'http',
+          optional: 'false'
+        }, {
+          name: 'logging-dir',
+          'interface': 'logging',
+          scope: 'container'
+        }
+      ] }});
+    charm.destroy();
+  });
+
+  it('should add a service to the map, neither provides nor requires',
+     function() {
+       models.endpointsMap = {};
+       var charm = new models.Charm({id: 'cs:precise/wordpress-2'});
+       models.addServiceToEndpointsMap('wordpress', charm);
+       models.endpointsMap.should.eql({wordpress: {
+         requires: [],
+         provides: []}});
+       charm.destroy();
+     });
+
+});
+
+describe('Endpoints map handlers', function() {
+  var Y, juju, utils, models, app, conn, env;
+
+  before(function(done) {
+    Y = YUI(GlobalConfig).use(['juju-gui',
+                               'juju-models',
+                               'juju-tests-utils',
+                               'juju-controllers'],
+    function(Y) {
+      juju = Y.namespace('juju');
+      utils = Y.namespace('juju-tests.utils');
+      models = Y.namespace('juju.models');
+      done();
+    });
+  });
+
+  beforeEach(function() {
+    conn = new utils.SocketStub();
+    env = juju.newEnvironment({conn: conn});
+    env.connect();
+    app = new Y.juju.App({env: env});
+    models.endpointsMap = {};
+  });
+
+  afterEach(function() {
+    env.destroy();
+    app.destroy();
+  });
+
+  it('should not update endpoints map when pending services are added',
+     function() {
+       var service_name = 'wordpress';
+       var charm_id = 'cs:precise/wordpress-2';
+       app.db.charms.add({id: charm_id});
+       var charm = app.db.charms.getById(charm_id);
+       charm.loaded = true;
+       app.db.services.add({
+         id: service_name,
+         pending: true,
+         charm: charm_id});
+       models.endpointsMap.should.eql({});
+       charm.destroy();
+     });
+
+  it('should update endpoints map when non-pending services are added',
+     function() {
+       var service_name = 'wordpress';
+       var charm_id = 'cs:precise/wordpress-2';
+       app.db.charms.add({id: charm_id});
+       var charm = app.db.charms.getById(charm_id);
+       charm.loaded = true;
+       app.db.services.add({
+         id: service_name,
+         pending: true,
+         charm: charm_id});
+       var svc = app.db.services.getById(service_name);
+       svc.set('pending', false);
+       models.endpointsMap.should.eql({wordpress: {
+         requires: [],
+         provides: []}});
+       charm.destroy();
+     });
+
+  it('should update endpoints map when a service\'s charm changes', function() {
+    var service_name = 'wordpress';
+    var charm_id = 'cs:precise/wordpress-2';
+    app.db.charms.add({id: charm_id});
+    var charm = app.db.charms.getById(charm_id);
+    charm.loaded = true;
+    app.db.services.add({
+      id: service_name,
+      pending: true,
+      charm: charm_id});
+    var svc = app.db.services.getById(service_name);
+    svc.set('pending', false);
+    models.endpointsMap.should.eql({wordpress: {
+      requires: [],
+      provides: []}});
+
+    charm_id = 'cs:precise/wordpress-3';
+    app.db.charms.add({id: charm_id});
+    var charm2 = app.db.charms.getById(charm_id);
+    charm2.set('provides', {
+      url: {
+        'interface': 'http'
+      }
+    });
+
+    charm2.loaded = true;
+    svc.set('charm', charm_id);
+    models.endpointsMap.should.eql({wordpress: {
+      requires: [],
+      provides: [
+        {
+          name: 'url',
+          'interface': 'http'
+        }]}});
+    charm.destroy();
+    charm2.destroy();
+  });
+
+  it('should remove service from endpoints map when it is deleted', function() {
+    var service_name = 'wordpress';
+    var charm_id = 'cs:precise/wordpress-2';
+    app.db.charms.add({id: charm_id});
+    app.db.services.add({
+      id: service_name,
+      charm: charm_id});
+    models.endpointsMap = {wordpress: 'foo'};
+    var service = app.db.services.getById(service_name);
+    app.db.services.remove(service);
+    models.endpointsMap.should.eql({});
+    app.db.charms.getById(charm_id).destroy();
+  });
+
+});

=== modified file 'test/test_env_python.js'
--- test/test_env_python.js	2013-03-22 12:49:57 +0000
+++ test/test_env_python.js	2013-04-02 21:29:23 +0000
@@ -224,13 +224,6 @@
       assert.equal(env.get('defaultSeries'), defaultSeries);
     });
 
-    it('can get endpoints for a service', function() {
-      env.get_endpoints(['mysql']);
-      msg = conn.last_message();
-      msg.op.should.equal('get_endpoints');
-      msg.service_names.should.eql(['mysql']);
-    });
-
     it('can update annotations', function() {
       var unit_name = 'mysql/0';
       env.update_annotations(unit_name, {name: 'A'});
@@ -375,10 +368,6 @@
       assertOperationAllowed('get_charm', ['cs:precise/haproxy']);
     });
 
-    it('allows retrieving endpoints if the GUI is read-only', function() {
-      assertOperationAllowed('get_endpoints', [['haproxy']]);
-    });
-
     it('allows getting services if the GUI is read-only', function() {
       assertOperationAllowed('get_service', ['haproxy']);
     });

=== modified file 'test/test_environment_view.js'
--- test/test_environment_view.js	2013-03-26 16:09:53 +0000
+++ test/test_environment_view.js	2013-04-02 21:29:23 +0000
@@ -674,7 +674,6 @@
        function() {
          var view = new views.environment({
            container: container,
-           getServiceEndpoints: function() {return {};},
            db: db,
            env: env
          }).render();
@@ -795,15 +794,16 @@
     it('should stop creating a relation if the background is clicked',
         function() {
           var db = new models.Database(),
-              endpoint_map = {'service-1': {requires: [], provides: []}},
+              endpointMap = {'service-1': {requires: [], provides: []}},
               view = new views.environment(
               { container: container,
                 db: db,
-                env: env,
-                getServiceEndpoints: function() {return endpoint_map;}}),
+                env: env}),
               service = new models.Service({ id: 'service-1'});
 
           db.services.add([service]);
+          // Reset the global endpointsMap after adding the service.
+          models.endpointsMap = endpointMap;
           view.render();
 
           // If the user has clicked on the "Add Relation" menu item...

=== modified file 'test/test_notifications.js'
--- test/test_notifications.js	2013-03-20 20:40:10 +0000
+++ test/test_notifications.js	2013-04-02 21:29:23 +0000
@@ -189,12 +189,15 @@
   it('must be able to include and show object links', function() {
     var container = Y.Node.create('<div id="test">'),
         logoNode = Y.Node.create('<div id="nav-brand-env"></div>'),
-        conn = new(Y.namespace('juju-tests.utils')).SocketStub(),
-        env = juju.newEnvironment({conn: conn}),
-        app = new Y.juju.App({env: env, container: container}),
+        conn = new(Y.namespace('juju-tests.utils')).SocketStub();
+    var env = juju.newEnvironment({conn: conn});
+    env.connect();
+    var app = new Y.juju.App({env: env, container: container}),
         db = app.db,
-        mw = db.services.create({id: 'mediawiki',
-                                    name: 'mediawiki'}),
+        mw = db.services.create({
+          id: 'mediawiki',
+          name: 'mediawiki',
+          charm: 'cs:precise/mediawiki-2'}),
         notifications = db.notifications,
         view = new views.NotificationsOverview({
                       container: container,
@@ -202,10 +205,10 @@
                       app: app,
                       env: env,
                       nsRouter: nsRouter}).render();
-    // we use overview here for testing as it defaults
-    // to showing all notices
+    // We use overview here for testing as it defaults
+    // to showing all notices.
 
-    // we can use app's routing table to derive a link
+    // We can use app's routing table to derive a link.
     notifications.create({title: 'Service Down',
       message: 'Your service has an error',
       link: app.getModelURL(mw)
@@ -216,7 +219,6 @@
         '/:gui:/service/mediawiki/');
     link.getHTML().should.contain('View Details');
 
-
     // create a new notice passing the link_title
     notifications.create({title: 'Service Down',
       message: 'Your service has an error',
@@ -241,6 +243,7 @@
           container: container,
           viewContainer: container
         });
+    env.connect();
     var environment_delta = default_env;
 
     var notifications = app.db.notifications,
@@ -250,7 +253,6 @@
           env: app.env,
           nsRouter: nsRouter}).render();
 
-
     app.env.dispatch_result(environment_delta);
 
 
@@ -274,41 +276,45 @@
   it('must properly construct title and message based on level from ' +
      'event data',
      function() {
-       var container = Y.Node.create(
-       '<div id="test" class="container"></div>'),
-       app = new Y.juju.App({
-         container: container,
-         viewContainer: container
-       });
-       var environment_delta = {
-         'result': [
-           ['service', 'add', {
-             'charm': 'cs:precise/wordpress-6',
-             'id': 'wordpress'
-           }],
-           ['service', 'add', {
-             'charm': 'cs:precise/mediawiki-3',
-             'id': 'mediawiki'
-           }],
-           ['service', 'add', {
-             'charm': 'cs:precise/mysql-6',
-             'id': 'mysql'
-           }],
-           ['unit', 'add', {
-             'agent-state': 'install-error',
-             'id': 'wordpress/0'
-           }],
-           ['unit', 'add', {
-             'agent-state': 'error',
-             'public-address': '192.168.122.222',
-             'id': 'mysql/0'
-           }],
-           ['unit', 'add', {
-             'public-address': '192.168.122.222',
-             'id': 'mysql/2'
-           }]
-         ],
-         'op': 'delta'
+        var container = Y.Node.create(
+           '<div id="test" class="container"></div>');
+        var conn = new(Y.namespace('juju-tests.utils')).SocketStub();
+        var env = juju.newEnvironment({conn: conn});
+        env.connect();
+        var app = new Y.juju.App({
+          env: env,
+          container: container,
+          viewContainer: container
+        });
+        var environment_delta = {
+          'result': [
+            ['service', 'add', {
+              'charm': 'cs:precise/wordpress-6',
+              'id': 'wordpress'
+            }],
+            ['service', 'add', {
+              'charm': 'cs:precise/mediawiki-3',
+              'id': 'mediawiki'
+            }],
+            ['service', 'add', {
+              'charm': 'cs:precise/mysql-6',
+              'id': 'mysql'
+            }],
+            ['unit', 'add', {
+              'agent-state': 'install-error',
+              'id': 'wordpress/0'
+            }],
+            ['unit', 'add', {
+              'agent-state': 'error',
+              'public-address': '192.168.122.222',
+              'id': 'mysql/0'
+            }],
+            ['unit', 'add', {
+              'public-address': '192.168.122.222',
+              'id': 'mysql/2'
+            }]
+          ],
+          'op': 'delta'
        };
 
        var notifications = app.db.notifications,