Merge lp:~benji/juju-gui/megawatcher-propose into lp:juju-gui/experimental

[Benji York]

Wire up the first bits of the megawatcher.

There is still much left to do, but what is here works and is reasonably
well tested.

https://codereview.appspot.com/7783050/

[Jeff Pihach (hatch)]

LGTM thanks! 'land with changes' I would like to see the two tests
mentioned below switched to use done() and then just take a look at the
one which isn't async but possibly should be?

Thanks again!

https://codereview.appspot.com/7783050/diff/1/app/store/env/go.js
File app/store/env/go.js (right):

https://codereview.appspot.com/7783050/diff/1/app/store/env/go.js#newcode77
app/store/env/go.js:77: instance_id: entityInfo.InstanceId // XXX and
more stuff
Can this be camelCased?

https://codereview.appspot.com/7783050/diff/1/test/test_env_go.js
File test/test_env_go.js (right):

https://codereview.appspot.com/7783050/diff/1/test/test_env_go.js#newcode58
test/test_env_go.js:58: assert.isTrue(loginFired);
Last night I did some reading on the best approach to this and I think
that using the done() method is the best way. This way could potentially
have a race condition causing the test to fail.

see: http://visionmedia.github.com/mocha/#asynchronous-code

https://codereview.appspot.com/7783050/diff/1/test/test_env_go.js#newcode72
test/test_env_go.js:72: assert.isTrue(loginFired);
see done() above

https://codereview.appspot.com/7783050/diff/1/test/test_env_go.js#newcode85
test/test_env_go.js:85: conn.msg({RequestId: 1, Response: {}});
conn.msg() is async above but not here?

https://codereview.appspot.com/7783050/diff/1/test/utils.js
File test/utils.js (right):

https://codereview.appspot.com/7783050/diff/1/test/utils.js#newcode30
test/utils.js:30: if (!back) {
just FYI - another commonly used convention is
back = back || 1;

https://codereview.appspot.com/7783050/

[Francesco Banconi (frankban)]

This branch looks good, a solid base for the megawatcher stuff.
LGTM with some questions and comments below.
Thank you.

https://codereview.appspot.com/7783050/diff/1/app/store/env/go.js
File app/store/env/go.js (right):

https://codereview.appspot.com/7783050/diff/1/app/store/env/go.js#newcode98
app/store/env/go.js:98: XXX FAKE FAKE FAKE, does not do anything.
Can't we just remove this method?

https://codereview.appspot.com/7783050/diff/1/app/store/env/go.js#newcode148
app/store/env/go.js:148: if (!op.Params) {
Nice, thanks.

https://codereview.appspot.com/7783050/diff/1/app/store/env/go.js#newcode196
app/store/env/go.js:196: deltas.push([kind, operation,
converter(entityInfo)]);
Very nice and simple.

https://codereview.appspot.com/7783050/diff/1/app/store/env/go.js#newcode219
app/store/env/go.js:219: this.fire('_rpc_response', data);
Just curiosity: why an event is preferred here over directly calling
_handleRpcResponse?

https://codereview.appspot.com/7783050/diff/1/test/test_env_go.js
File test/test_env_go.js (right):

https://codereview.appspot.com/7783050/diff/1/test/test_env_go.js#newcode82
test/test_env_go.js:82: it('calls environmentInfo and watchAll ofter
login', function() {
typo: after

https://codereview.appspot.com/7783050/diff/1/test/test_env_go.js#newcode678
test/test_env_go.js:678: assert.isTrue('Id' in msg.Params);
Here we could also test that Params.Id is effectively the answer...

https://codereview.appspot.com/7783050/diff/1/test/test_env_go.js#newcode715
test/test_env_go.js:715: Y = YUI(GlobalConfig).use(['juju-env',
'juju-tests-utils'], function(Y) {
It seems 'juju-tests-utils' is not really required by this test case.
Ditto for all the converter tests below.

https://codereview.appspot.com/7783050/diff/1/undocumented
File undocumented (left):

https://codereview.appspot.com/7783050/diff/1/undocumented#oldcode15
undocumented:15: app/store/env/python.js:371 "get_endpoints"
Thank you.

https://codereview.appspot.com/7783050/

=== modified file 'Makefile'
--- Makefile	2013-03-22 21:22:38 +0000
+++ Makefile	2013-03-26 12:15:49 +0000
@@ -242,7 +242,7 @@
 		app/assets/javascripts/d3.v2.min.js
 
 gjslint: virtualenv/bin/gjslint
-	virtualenv/bin/gjslint --strict --nojsdoc --jslint_error=all \
+	virtualenv/bin/gjslint --unix --strict --nojsdoc --jslint_error=all \
 	    --custom_jsdoc_tags module,main,class,method,event,property,attribute,submodule,namespace,extends,config,constructor,static,final,readOnly,writeOnce,optional,required,param,return,for,type,private,protected,requires,default,uses,example,chainable,deprecated,since,async,beta,bubbles,extension,extensionfor,extension_for \
 		-x $(LINT_IGNORE) $(JSFILES)
 

=== modified file 'app/store/env/go.js'
--- app/store/env/go.js	2013-03-22 16:52:05 +0000
+++ app/store/env/go.js	2013-03-26 12:15:49 +0000
@@ -29,6 +29,56 @@
 
   GoEnvironment.NAME = 'go-env';
 
+  var entityInfoConverters = {
+    /**
+      Convert a Go style entity info into the expected legacy format.
+
+      @param {Object} entityInfo The JSON entity information from Go.
+      @return {Object} The legacy JSON data that the GUI expects.
+     */
+    service: function(entityInfo) {
+      return {
+        id: entityInfo.Name,
+        exposed: entityInfo.Exposed // XXX and more stuff
+      };
+    },
+    /**
+      Convert a Go style entity info into the expected legacy format.
+
+      @param {Object} entityInfo The JSON entity information from Go.
+      @return {Object} The legacy JSON data that the GUI expects.
+     */
+    unit: function(entityInfo) {
+      return {
+        id: entityInfo.Name,
+        service: entityInfo.Service // XXX and more stuff
+      };
+    },
+    /**
+      Convert a Go style entity info into the expected legacy format.
+
+      @param {Object} entityInfo The JSON entity information from Go.
+      @return {Object} The legacy JSON data that the GUI expects.
+     */
+    relation: function(entityInfo) {
+      return {
+        id: entityInfo.Key // XXX and more stuff
+      };
+    },
+    /**
+      Convert a Go style entity info into the expected legacy format.
+
+      @param {Object} entityInfo The JSON entity information from Go.
+      @return {Object} The legacy JSON data that the GUI expects.
+     */
+    machine: function(entityInfo) {
+      return {
+        id: entityInfo.Id,
+        instance_id: entityInfo.InstanceId // XXX and more stuff
+      };
+    }
+  };
+
   Y.extend(GoEnvironment, environments.BaseEnvironment, {
 
     /**
@@ -41,7 +91,26 @@
       // Define the default user name for this environment. It will appear as
       // predefined value in the login mask.
       this.defaultUser = 'user-admin';
-    },
+      this.on('_rpc_response', this._handleRpcResponse, this);
+    },
+
+    /**
+      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".
@@ -76,11 +145,83 @@
         this._txn_callbacks[tid] = callback;
       }
       op.RequestId = tid;
+      if (!op.Params) {
+        op.Params = {};
+      }
       var msg = Y.JSON.stringify(op);
       this.ws.send(msg);
     },
 
     /**
+      Begin watching all Juju status.
+
+      @method _watchAll
+      @private
+      @return {undefined} Sends a message to the server only.
+     */
+    _watchAll: function() {
+      this._send_rpc(
+          {
+            Type: 'Client',
+            Request: 'WatchAll'
+          },
+          function(data) {
+            if (data.Error) {
+              console.log('aiiiiie!'); // retry and eventually alert user XXX
+            } else {
+              this._allWatcherId = data.Response.AllWatcherId;
+              this._next();
+            }
+          }
+      );
+    },
+
+    /**
+      Process an incoming response to an RPC request.
+
+      @method _handleRpcResponse
+      @param {Object} data The data returned by the server.
+      @return {undefined} Nothing.
+     */
+    _handleRpcResponse: function(data) {
+      // We do this early to get a response back fast.  Might be a bad
+      // idea. :-)
+      this._next();
+      // data.Deltas has our stuff.  We need to translate delta
+      // events based on the deltas we got.
+      var deltas = [];
+      data.Response.Deltas.forEach(function(delta) {
+        var kind = delta[0], operation = delta[1], entityInfo = delta[2];
+        var converter = entityInfoConverters[kind];
+        deltas.push([kind, operation, converter(entityInfo)]);
+      });
+      this.fire('delta', {data: {result: deltas}});
+    },
+
+    /**
+      Get the next batch of deltas from the Juju status.
+
+      @method _next
+      @private
+      @return {undefined} Sends a message to the server only.
+     */
+    _next: function() {
+      this._send_rpc({
+        Type: 'AllWatcher',
+        Request: 'Next',
+        Params: {
+          Id: this._allWatcherId
+        }
+      }, function(data) {
+        if (data.Error) {
+          console.log('aiiiiie!'); // XXX
+        } else {
+          this.fire('_rpc_response', data);
+        }
+      });
+    },
+
+    /**
      * React to the results of sending a login message to the server.
      *
      * @method handleLoginEvent
@@ -88,10 +229,12 @@
      * @return {undefined} Nothing.
      */
     handleLogin: function(data) {
+      this.pendingLoginResponse = false;
       this.userIsAuthenticated = !data.Error;
       if (this.userIsAuthenticated) {
         // If login succeeded retrieve the environment info.
         this.environmentInfo();
+        this._watchAll();
       } else {
         // If the credentials were rejected remove them.
         this.setCredentials(null);
@@ -109,7 +252,7 @@
      */
     login: function() {
       // If the user is already authenticated there is nothing to do.
-      if (this.userIsAuthenticated) {
+      if (this.userIsAuthenticated || this.pendingLoginResponse) {
         return;
       }
       var credentials = this.getCredentials();
@@ -122,6 +265,7 @@
             Password: credentials.password
           }
         }, this.handleLogin);
+        this.pendingLoginResponse = true;
       } else {
         console.warn('Attempted login without providing credentials.');
         this.fire('login', {data: {result: false}});
@@ -156,8 +300,7 @@
     environmentInfo: function() {
       this._send_rpc({
         Type: 'Client',
-        Request: 'EnvironmentInfo',
-        Params: {}
+        Request: 'EnvironmentInfo'
       }, this.handleEnvironmentInfo);
     },
 
@@ -587,6 +730,7 @@
   });
 
   environments.GoEnvironment = GoEnvironment;
+  environments.entityInfoConverters = entityInfoConverters;
 
 }, '0.1.0', {
   requires: [

=== modified file 'app/store/env/python.js'
--- app/store/env/python.js	2013-02-26 20:04:10 +0000
+++ app/store/env/python.js	2013-03-26 12:15:49 +0000
@@ -367,6 +367,18 @@
         retry: retry || false}, callback, true);
     },
 
+    /**
+      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);

=== modified file 'test/test_env_go.js'
--- test/test_env_go.js	2013-03-22 16:52:05 +0000
+++ test/test_env_go.js	2013-03-26 12:15:49 +0000
@@ -13,18 +13,16 @@
       });
     });
 
-    beforeEach(function(done) {
+    beforeEach(function() {
       conn = new utils.SocketStub();
       env = juju.newEnvironment({
         conn: conn, user: 'user', password: 'password'
       }, 'go');
       env.connect();
-      done();
     });
 
-    afterEach(function(done)  {
+    afterEach(function()  {
       env.destroy();
-      done();
     });
 
     it('sends the correct login message', function() {
@@ -47,7 +45,7 @@
       assert.isTrue(env.failedAuthentication);
     });
 
-    it('fires a login event on successful login', function(done) {
+    it('fires a login event on successful login', function() {
       var loginFired = false;
       var result;
       env.on('login', function(evt) {
@@ -59,10 +57,9 @@
       conn.msg({RequestId: 1, Response: {}});
       assert.isTrue(loginFired);
       assert.isTrue(result);
-      done();
     });
 
-    it('fires a login event on failed login', function(done) {
+    it('fires a login event on failed login', function() {
       var loginFired = false;
       var result;
       env.on('login', function(evt) {
@@ -74,7 +71,6 @@
       conn.msg({RequestId: 1, Error: 'Invalid user or password'});
       assert.isTrue(loginFired);
       assert.isFalse(result);
-      done();
     });
 
     it('avoids sending login requests without credentials', function() {
@@ -83,20 +79,28 @@
       assert.equal(0, conn.messages.length);
     });
 
-    it('calls environmentInfo on successful login', function(done) {
+    it('calls environmentInfo and watchAll ofter login', function() {
       env.login();
       // Assume login to be the first request.
       conn.msg({RequestId: 1, Response: {}});
-      var last_message = conn.last_message();
+      var environmentInfoMessage = conn.last_message(2);
       // EnvironmentInfo is the second request.
-      var expected = {
+      var environmentInfoExpected = {
         Type: 'Client',
         Request: 'EnvironmentInfo',
         RequestId: 2,
         Params: {}
       };
-      assert.deepEqual(expected, last_message);
-      done();
+      assert.deepEqual(environmentInfoExpected, environmentInfoMessage);
+      var watchAllMessage = conn.last_message();
+      // EnvironmentInfo is the second request.
+      var watchAllExpected = {
+        Type: 'Client',
+        Request: 'WatchAll',
+        RequestId: 3,
+        Params: {}
+      };
+      assert.deepEqual(watchAllExpected, watchAllMessage);
     });
 
     it('sends the correct request for environment info', function() {
@@ -651,6 +655,191 @@
       assert.equal(err, 'service "yoursql" not found');
     });
 
+    it('provides for a missing Params', function() {
+      // If no "Params" are provided in an RPC call an empty one is added.
+      var op = {};
+      env._send_rpc(op);
+      assert.deepEqual(op.Params, {});
+    });
+
+    it('can watch all changes', function() {
+      env._watchAll();
+      msg = conn.last_message();
+      assert.equal(msg.Type, 'Client');
+      assert.equal(msg.Request, 'WatchAll');
+    });
+
+    it('can retrieve the next set of environment changes', function() {
+      // This is normally set by _watchAll, we'll fake it here.
+      env._allWatcherId = 42;
+      env._next();
+      msg = conn.last_message();
+      assert.equal(msg.Type, 'AllWatcher');
+      assert.isTrue('Id' in msg.Params);
+      assert.equal(msg.Request, 'Next');
+    });
+
+    it('fires "_rpc_response" message after an RPC response', function(done) {
+      // We don't want the real response, we just want to be sure the event is
+      // fired.
+      env.detach('_rpc_response');
+      env.on('_rpc_response', function(data) {
+        done();
+      });
+      // Calling this sets up the callback.
+      env._next();
+      env._txn_callbacks[env._counter].call(env, {});
+      // The only test assertion is that done (above) is called.
+    });
+
+    it('fires "delta" when handling an RPC response', function(done) {
+      env.detach('delta');
+      var callbackData = {Response: {Deltas: [['service', 'deploy', {}]]}};
+      env.on('delta', function(data) {
+        console.log(data.result);
+        done();
+      });
+      env._handleRpcResponse(callbackData);
+    });
+
+  });
+
+})();
+
+(function() {
+
+  describe('Go Juju environment service entity converter', function() {
+    var environments, utils, Y, converter, entityInfoConverters;
+
+    before(function(done) {
+      Y = YUI(GlobalConfig).use(['juju-env', 'juju-tests-utils'], function(Y) {
+        environments = Y.namespace('juju.environments');
+        utils = Y.namespace('juju-tests.utils');
+        converter = environments.entityInfoConverters.service;
+        entityInfoConverters = environments.entityInfoConverters;
+        done();
+      });
+    });
+
+
+    it('exists', function() {
+      assert.isTrue('service' in entityInfoConverters);
+    });
+
+    it('converts "Name" to "id"', function() {
+      var converted = converter({Name: 'service name'});
+      assert.isTrue('id' in converted);
+      assert.equal('service name', converted.id);
+    });
+
+    it('converts "Exposed" to "exposed"', function() {
+      var converted = converter({Exposed: true});
+      assert.isTrue('exposed' in converted);
+      assert.isTrue(converted.exposed);
+    });
+
+  });
+
+})();
+
+(function() {
+
+  describe('Go Juju environment unit entity converter', function() {
+    var environments, utils, Y, converter, entityInfoConverters;
+
+    before(function(done) {
+      Y = YUI(GlobalConfig).use(['juju-env', 'juju-tests-utils'], function(Y) {
+        environments = Y.namespace('juju.environments');
+        utils = Y.namespace('juju-tests.utils');
+        converter = environments.entityInfoConverters.unit;
+        entityInfoConverters = environments.entityInfoConverters;
+        done();
+      });
+    });
+
+
+    it('exists', function() {
+      assert.isTrue('unit' in entityInfoConverters);
+    });
+
+    it('converts "Name" to "id"', function() {
+      var converted = converter({Name: 'unit name'});
+      assert.isTrue('id' in converted);
+      assert.equal('unit name', converted.id);
+    });
+
+    it('converts "Service" to "service"', function() {
+      var converted = converter({Service: 'a service'});
+      assert.isTrue('service' in converted);
+      assert.equal('a service', converted.service);
+    });
+
+  });
+
+})();
+
+(function() {
+
+  describe('Go Juju environment relation entity converter', function() {
+    var environments, utils, Y, converter, entityInfoConverters;
+
+    before(function(done) {
+      Y = YUI(GlobalConfig).use(['juju-env', 'juju-tests-utils'], function(Y) {
+        environments = Y.namespace('juju.environments');
+        utils = Y.namespace('juju-tests.utils');
+        converter = environments.entityInfoConverters.relation;
+        entityInfoConverters = environments.entityInfoConverters;
+        done();
+      });
+    });
+
+
+    it('exists', function() {
+      assert.isTrue('relation' in entityInfoConverters);
+    });
+
+    it('converts "Name" to "id"', function() {
+      var converted = converter({Key: 'relation name'});
+      assert.isTrue('id' in converted);
+      assert.equal('relation name', converted.id);
+    });
+
+  });
+
+})();
+
+(function() {
+
+  describe('Go Juju environment machine entity converter', function() {
+    var environments, utils, Y, converter, entityInfoConverters;
+
+    before(function(done) {
+      Y = YUI(GlobalConfig).use(['juju-env', 'juju-tests-utils'], function(Y) {
+        environments = Y.namespace('juju.environments');
+        utils = Y.namespace('juju-tests.utils');
+        converter = environments.entityInfoConverters.machine;
+        entityInfoConverters = environments.entityInfoConverters;
+        done();
+      });
+    });
+
+
+    it('exists', function() {
+      assert.isTrue('machine' in entityInfoConverters);
+    });
+
+    it('converts "Id" to "id"', function() {
+      var converted = converter({Id: 'machine ID'});
+      assert.isTrue('id' in converted);
+      assert.equal('machine ID', converted.id);
+    });
+
+    it('converts "InstanceId" to "instance_id"', function() {
+      var converted = converter({InstanceId: 'instance ID'});
+      assert.isTrue('instance_id' in converted);
+      assert.equal('instance ID', converted.instance_id);
+    });
+
   });
 
 })();

=== modified file 'test/utils.js'
--- test/utils.js	2013-03-21 21:17:03 +0000
+++ test/utils.js	2013-03-26 12:15:49 +0000
@@ -10,7 +10,6 @@
       this.messages = [];
 
       this.close = function() {
-        //console.log('close stub');
         this.messages = [];
       };
 
@@ -24,16 +23,17 @@
       };
 
       this.msg = function(m) {
-        //console.log('serializing env msg', m);
         this.onmessage({'data': Y.JSON.stringify(m)});
       };
 
-      this.last_message = function(m) {
-        return this.messages[this.messages.length - 1];
+      this.last_message = function(back) {
+        if (!back) {
+          back = 1;
+        }
+        return this.messages[this.messages.length - back];
       };
 
       this.send = function(m) {
-        //console.log('socket send', m);
         this.messages.push(Y.JSON.parse(m));
       };
 

=== modified file 'undocumented'
--- undocumented	2013-03-12 11:16:32 +0000
+++ undocumented	2013-03-26 12:15:49 +0000
@@ -12,7 +12,6 @@
 app/store/env/python.js:94 "_dispatch_rpc_result"
 app/store/env/python.js:195 "get_service"
 app/store/env/python.js:86 "_dispatch_event"
-app/store/env/python.js:371 "get_endpoints"
 app/store/env/python.js:191 "get_charm"
 app/store/env/base.js:53 "destructor"
 app/store/env/base.js:38 "initializer"